On 10/03/17 18:53, Andres Gomez wrote:
On Fri, 2017-03-10 at 14:28 +1100, Timothy Arceri wrote:
On 10/03/17 08:46, Andres Gomez wrote:
On Fri, 2017-03-10 at 08:32 +1100, Timothy Arceri wrote:
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.
Where exactly does this segfault happen?
interstage_cross_validate_uniform_blocks() should exit linking because
we returned false.
It does exit, the segfault is not happening when running this code but
when using the link failed program later, as commented.
I caught this by using piglit's shader runner. In the "init_test"
function the last action is to call "setup_ubos":
https://cgit.freedesktop.org/piglit/tree/tests/shaders/shader_runner.c#n3704
This is done regardless the link status gotten previously. At
"setup_ubos" we ask first for the active UBOs:
https://cgit.freedesktop.org/piglit/tree/tests/shaders/shader_runner.c#n2652
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.
Of course, you could argue what are you doing using a link failed
program ☺ but the fact is that it should not return a value for the
active UBOs other than 0 and, most importantly, it shouldn't crash.
I hope this clarifies your doubt.
I'm not sure this is a bug. The section 7.3. (PROGRAM OBJECTS) of the
OpenGL 4.5 spec says:
"If a program is not linked successfully, the link may have failed for
a number of reasons, including cases where the program required more
resources than supported by the implementation. Implementations are
permitted, but not required, to record lists of resources that would
have been considered active had the program linked successfully."
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.
In other words, I think this is still a bug. The solution could be
different but, with current implementation, the most straight forward
is the patch I provided, setting the active UBOs to 0, which is
coherent with the rest of the internal state saved for the program.
IMO you are trying to fix something the spec doesn't say needs fixing. I
would be surprised if this was the only value that is reported as non
zero on failure. If we really wanted to reset things to zero on a link
failure we should have a helper that does it for all values on a link
failure, not one off cases like this.
Last, this patch is solving the "regression" that was caused by Commit
f1293b2f9bc3.
This is not a regression, it just happens to now return the value that
it would be if the link didn't fail, which as far as I can tell is
allowed by the spec.
I set you to review because you are the author of that commit.
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
