[Mesa-dev] [PATCH V3 1/2] glsl: don't attempt to link empty program

[Ian Romanick]

On 01/25/2016 06:33 PM, Timothy Arceri wrote:
> On Mon, 2016-01-25 at 16:58 -0800, Ian Romanick wrote:
>> On 01/25/2016 04:46 PM, Timothy Arceri wrote:
>>> Previously an empty program would go through the entire
>>> link_shaders() function and we would have to be careful
>>> not to cause a segfault.
>>>
>>> In core profile also now set link_status to false by
>>> generating an error, it was previously set to true.
>>>
>>>> From Section 7.3 (PROGRAM OBJECTS) of the OpenGL 4.5 spec:
>>>
>>>    "Linking can fail for a variety of reasons as specified in the
>>>    OpenGL Shading Language Specification, as well as any of the
>>>    following reasons:
>>>
>>>     - No shader objects are attached to program."
>>>
>>> V2: Only generate an error in core profile and add spec quote (Ian)
>>>
>>> V3: generate error in ES too, remove previous check which was only
>>> applying the rule to GL 4.5/ES 3.1 and above. My understand is that
>>> this spec change is clarifying previously undefined behaviour and
>>> therefore should be applied retrospectively. The ES CTS tests for
>>> this are in ES 2 I suspect it was passing because it would have
>>> generated an error for not having both a vertex and fragment
>>> shader.
>>>
>>> Cc: Ian Romanick <idr at freedesktop.org>
>>> Cc: Tapani Pälli <tapani.palli at intel.com>
>>> Cc: Ilia Mirkin <imirkin at alum.mit.edu>
>>> ---
>>>  src/glsl/linker.cpp | 43 ++++++++++++++++++++++-------------------
>>> --
>>>  1 file changed, 22 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
>>> index 6657777..79039a0 100644
>>> --- a/src/glsl/linker.cpp
>>> +++ b/src/glsl/linker.cpp
>>> @@ -4105,14 +4105,34 @@
>>> disable_varying_optimizations_for_sso(struct gl_shader_program
>>> *prog)
>>>  void
>>>  link_shaders(struct gl_context *ctx, struct gl_shader_program
>>> *prog)
>>>  {
>>> +   prog->Validated = false;
>>> +   prog->_Used = false;
>>> +
>>> +   /* Section 7.3 (Program Objects) of the OpenGL 4.5 Core Profile
>>> spec says:
>>> +    *
>>> +    *     "Linking can fail for a variety of reasons as specified
>>> in the
>>> +    *     OpenGL Shading Language Specification, as well as any of
>>> the
>>> +    *     following reasons:
>>> +    *
>>> +    *     - No shader objects are attached to program."
>>> +    *
>>> +    * The Compatibility Profile specification does not list the
>>> error.  In
>>> +    * Compatibility Profile missing shader stages are replaced by
>>> +    * fixed-function.  This applies to the case where all stages
>>> are
>>> +    * missing.
>>> +    */
>>> +   if (prog->NumShaders == 0) {
>>> +      if (ctx->API != API_OPENGL_COMPAT)
>>> +         linker_error(prog, "no shaders attached to the
>>> program\n");
>>> +      return;
>>
>> Does glsl-link-empty-prog-02 still pass?  Since prog->LinkStatus is
>> false, I think that will cause problems.  Won't glUseProgram generate
>> GL_INVALID_OPERATION?
> 
> It does pass, seems something sets the link flag to true before we get
> here. Maybe the fallback code?
> 
> Anyway it makes sense to also move the prog->LinkStatus = true; to the
> top so I've done that locally.

With the 'prog->LinkStatus = true;' moved, this patch is

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

>>> +   }
>>> +
>>>     tfeedback_decl *tfeedback_decls = NULL;
>>>     unsigned num_tfeedback_decls = prog-
>>>> TransformFeedback.NumVarying;
>>>  
>>>     void *mem_ctx = ralloc_context(NULL); // temporary linker
>>> context
>>>  
>>>     prog->LinkStatus = true; /* All error paths will set this to
>>> false */
>>> -   prog->Validated = false;
>>> -   prog->_Used = false;
>>>  
>>>     prog->ARB_fragment_coord_conventions_enable = false;
>>>  
>>> @@ -4162,25 +4182,6 @@ link_shaders(struct gl_context *ctx, struct
>>> gl_shader_program *prog)
>>>     prog->Version = max_version;
>>>     prog->IsES = is_es_prog;
>>>  
>>> -   /* From OpenGL 4.5 Core specification (7.3 Program Objects):
>>> -    *     "Linking can fail for a variety of reasons as specified
>>> in the OpenGL
>>> -    *     Shading Language Specification, as well as any of the
>>> following
>>> -    *     reasons:
>>> -    *
>>> -    *     * No shader objects are attached to program.
>>> -    *
>>> -    *     ..."
>>> -    *
>>> -    *     Same rule applies for OpenGL ES >= 3.1.
>>> -    */
>>> -
>>> -   if (prog->NumShaders == 0 &&
>>> -       ((ctx->API == API_OPENGL_CORE && ctx->Version >= 45) ||
>>> -        (ctx->API == API_OPENGLES2 && ctx->Version >= 31))) {
>>> -      linker_error(prog, "No shader objects are attached to
>>> program.\n");
>>> -      goto done;
>>> -   }
>>> -
>>>     /* Some shaders have to be linked with some other shaders
>>> present.
>>>      */
>>>     if (num_shaders[MESA_SHADER_GEOMETRY] > 0 &&
>>>
>>