[Mesa-stable] [Fwd: Re: [PATCH 1/3] glsl: on UBO/SSBOs link error, the number of active blocks remains 0]

[Andres Gomez]

It would be good to pick this for 17.0

-------- Forwarded Message --------

Date: Sun, 26 Mar 2017 21:08:27 +1100
Subject: Re: [PATCH 1/3] glsl: on UBO/SSBOs link error, the number of
active blocks remains 0
To: Andres Gomez <agomez at igalia.com>, mesa-dev at lists.freedesktop.org
From: Timothy Arceri <tarceri at itsqueeze.com>
Ok. So lets start again. :)

How about changing the subject to:

glsl: on UBO/SSBOs link error reset the number of active blocks to 0


On 23/02/17 19:55, Andres Gomez wrote:
> Commit f1293b2f9bc3 split apart buffer block arrays but introduced
> also the possibility of a recount of active
> blocks (NumUniformBlocks/NumShaderStorageBlocks) which would be
> incoherent with the actual amount of active
> blocks (UniformBlocks/ShaderStorageBlocks) in the program.
> 
> This could cause a segmentation fault if trying to use the index of a
> block in a link failed program.


How about we simplify this whole commit message to something like:

While it's legal to have an active blocks count > 0 on link failure.
Unless we actually assign memory for the blocks array we can end up 
segfaulting in calls such as glUniformBlockBinding().

To avoid having to NULL check these api calls we simply reset the
block count to 0 if the array was not created.

> 
> Signed-off-by: Andres Gomez <agomez at igalia.com>
> Cc: Timothy Arceri <tarceri at itsqueeze.com>
> ---
>  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 b6f8bc4212e..bb9c101f430 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -1181,6 +1181,7 @@ interstage_cross_validate_uniform_blocks(struct gl_shader_program *prog,
>              for (unsigned k = 0; k <= i; k++) {
>                 delete[] InterfaceBlockStageIndex[k];
>              }

And please add a comment here:


                /* Reset the block count. This will help avoid various
                 * segfaults from api calls that assume the array exists
                 * due to the count being non-zero.
                 */
> +            *num_blks = 0;
>              return false;
>           }
> 
> 

With those this is:

Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>

-- 
Br,

Andres