[Mesa-dev] [PATCH] mesa: fix segfault in glUniformSubroutinesuiv()

[Timothy Arceri]

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;



> > +      }
> > +
> >        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