[Mesa-dev] [PATCH 1/2] glsl: make sure builtins are initialized before getting the shader

[Ilia Mirkin]

On Sun, Feb 7, 2016 at 3:49 AM, Timothy Arceri
<timothy.arceri at collabora.com> wrote:
> On Sat, 2016-02-06 at 17:10 -0500, Ilia Mirkin wrote:
>
> It would be have been nice to have some reasoning in here,
> the scenario its fixing etc.
>
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> Cc: mesa-stable at lists.freedesktop.org
>> ---
>>  src/compiler/glsl/linker.cpp | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/src/compiler/glsl/linker.cpp
>> b/src/compiler/glsl/linker.cpp
>> index 4776ffa..f1ac53a 100644
>> --- a/src/compiler/glsl/linker.cpp
>> +++ b/src/compiler/glsl/linker.cpp
>> @@ -2125,6 +2125,7 @@ link_intrastage_shaders(void *mem_ctx,
>>
>>        if (ok) {
>>           memcpy(linking_shaders, shader_list, num_shaders *
>> sizeof(gl_shader *));
>> +         _mesa_glsl_initialize_builtin_functions();
>
> Doesn't this defeat the purpose of glReleaseShaderCompiler() as its
> currently implemented in mesa? Although thinking about it I can't think
> of any better alternative.

Yeah, not sure. It does seem like glReleaseShaderCompiler should be
effective before glLinkProgram while this will, effectively, bring all
that data back in. But.... I'm just trying to fix crashes here :)

>
> If you add something to the commit message describing why we would need
> to make sure this is initialize here i.e the problem scenario.

Sure, I can do that. At first I read this as adding a comment to the
code, but all the other call sites just do it without comment as well.
However something in the commit message makes sense... how about:

"The builtin function shader is part of the builtin state, released
when glReleaseShaderCompiler is called. We must ensure that the
builtins have been (re)initialized before attempting to link with the
builtin shader."

>
> Reviewed-by: Timothy Arceri <timothy.arceri at collabora.com>
>
>
>>           linking_shaders[num_shaders] =
>> _mesa_glsl_get_builtin_function_shader();
>>
>>           ok = link_function_calls(prog, linked, linking_shaders,
>> num_shaders + 1);