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

[Andres Gomez]

On Sun, 2017-03-12 at 10:07 +1100, Timothy Arceri wrote:
> 
> On 12/03/17 00:29, Andres Gomez wrote:
> > On Sat, 2017-03-11 at 23:24 +1100, Timothy Arceri wrote:
> > > 
> > > On 10/03/17 18:53, Andres Gomez wrote:
> > > > 
> > > > According with that text it would be OK to report a different number
> > > > than 0 when asking for the active UBOs to the failed link program but
> > > > it still will be not OK that, when trying to use that active UBOs, we
> > > > would get a SIGSEV.
> > > 
> > > That is is because of an application bug (not checking that link was
> > > successful) not a Mesa bug.
> > 
> > I disagree, the one crashing is Mesa.
> 
> Then I'll repeat my initial question. Where exactly does it crash? What 
> OpenGL call? Is it glGetActiveUniformBlockiv? If so that looks like a bug.

Yes, glGetActiveUniformBlockiv, as I answered before:

"And if there is any, we make use of them:
https://cgit.freedesktop.org/piglit/tree/tests/shaders/shader_runner.c#n2662

That's when the SIGSEV happens."

> If we want to fix this bug we should really be adding a piglit test to 
> go with it.

I recently added a test for that:
https://cgit.freedesktop.org/piglit/tree/tests/spec/arb_shading_language_420pack/linker/different-bindings-uniform-blocks-instanced.shader_test

That test will SIGSEV because of the described reason once the patch 3
in this series lands, which fixes the actual problem for which that
test was created:
https://patchwork.freedesktop.org/patch/140481/

AFAIK, there is no other way of triggering this crash.

> The sensible thing to do would be to NULL check the uniform block array 
> in this function.

I think the best solution is what it is understood already now in the
code, which is that the index of active uniform blocks is always
coherent with the block array size.

Setting the index to 0, which is coherent and doesn't make any harm, is
the easiest and most straight forward solution.

Anyway, we can NULL check but, then, I don't know exactly which is the
behavior that you expect from the glGetActiveUniformBlockiv call (?).

-- 
Br,

Andres