[Mesa-dev] [PATCH] mesa: fix segfault in glUniformSubroutinesuiv()
Ilia Mirkin
imirkin at alum.mit.edu
Sun Jan 17 14:38:57 PST 2016
On Sun, Jan 17, 2016 at 5:34 PM, Timothy Arceri
<timothy.arceri at collabora.com> wrote:
> On Sun, 2016-01-17 at 08:27 -0800, Jason Ekstrand wrote:
>>
>> On Jan 16, 2016 9:15 PM, "Timothy Arceri" <
>> timothy.arceri at collabora.com> wrote:
>> >
>> > From Section 7.9 (SUBROUTINE UNIFORM VARIABLES) of the OpenGL
>> > 4.5 Core spec:
>> >
>> > "The command
>> >
>> > void UniformSubroutinesuiv(enum shadertype, sizei count,
>> > const uint *indices);
>> >
>> > will load all active subroutine uniforms for shader stage
>> > shadertype with subroutine indices from indices, storing
>> > indices[i] into the uniform at location i. The indices for
>> > any locations between zero and the value of
>> > ACTIVE_SUBROUTINE_UNIFORM_LOCATIONS minus one which are not
>> > used will be ignored."
>> >
>> > Cc: "11.0 11.1" mesa-stable at lists.freedesktop.org
>> > https://bugs.freedesktop.org/show_bug.cgi?id=93731
>> > ---
>> > src/mesa/main/shaderapi.c | 14 ++++++++++++++
>> > 1 file changed, 14 insertions(+)
>> >
>> > diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
>> > index cdc85f3..918c5f9 100644
>> > --- a/src/mesa/main/shaderapi.c
>> > +++ b/src/mesa/main/shaderapi.c
>> > @@ -2530,6 +2530,13 @@ _mesa_UniformSubroutinesuiv(GLenum
>> shadertype, GLsizei count,
>> > i = 0;
>> > do {
>> > struct gl_uniform_storage *uni = sh
>> ->SubroutineUniformRemapTable[i];
>> > + if (uni == NULL) {
>> > + if (++i < count)
>> > + continue;
>> > + else
>> > + break;
>> Just
>> i++;
>> continue;
>> Will do the same thing and without repeating the looping condition or
>> making the user think about pre- vs. post-increment. I guess it does
>> make them think about continues in a do-while, but you can't win
>> everything. :-)
>
> I don't feel great about implementing the possibility of an infinite
> loop. Sure the code above should work but this stuff doesn't really
> have extensive tests.
>
> What about at the very least:
>
> i++;
> assert(i < count);
> continue;
>
You'd have to stick the assert before i++.
>
>
>> > + }
>> > +
>> > int uni_count = uni->array_elements ? uni->array_elements :
>> 1;
>> > int j, k;
>> >
>> > @@ -2557,6 +2564,13 @@ _mesa_UniformSubroutinesuiv(GLenum
>> shadertype, GLsizei count,
>> > i = 0;
>> > do {
>> > struct gl_uniform_storage *uni = sh
>> ->SubroutineUniformRemapTable[i];
>> > + if (uni == NULL) {
>> > + if (++i < count)
>> > + continue;
>> > + else
>> > + break;
>> > + }
>> > +
>> > int uni_count = uni->array_elements ? uni->array_elements :
>> 1;
>> >
>> > memcpy(&uni->storage[0], &indices[i],
>> > --
>> > 2.4.3
>> >
>> > _______________________________________________
>> > mesa-dev mailing list
>> > mesa-dev at lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list