On Thu, Sep 11, 2014 at 1:27 PM, Erik Faye-Lund <kusmab...@gmail.com> wrote: > On Sat, Aug 2, 2014 at 4:09 AM, Ian Romanick <i...@freedesktop.org> wrote: >> diff --git a/src/mesa/main/uniform_query.cpp >> b/src/mesa/main/uniform_query.cpp >> index 609d94b..7b089fa 100644 >> --- a/src/mesa/main/uniform_query.cpp >> +++ b/src/mesa/main/uniform_query.cpp >> @@ -266,30 +265,32 @@ validate_uniform_parameters(struct gl_context *ctx, >> */ >> if (shProg->UniformRemapTable[location] == >> INACTIVE_UNIFORM_EXPLICIT_LOCATION) >> - return false; >> + return NULL; >> >> - _mesa_uniform_split_location_offset(shProg, location, loc, array_index); >> + unsigned loc; >> + _mesa_uniform_split_location_offset(shProg, location, &loc, array_index); >> + struct gl_uniform_storage *const uni = &shProg->UniformStorage[loc]; >> >> - if (shProg->UniformStorage[*loc].array_elements == 0 && count > 1) { >> + if (uni->array_elements == 0 && count > 1) { > > I'm getting a NULL-pointer deference here when running piglit's > spec/ARB_explicit_uniform_location/arb_explicit_uniform_location-use-of-unused-loc > on yesterday's master. A quick git-log doesn't seem to contain a fix > in today's master either. > > Perhaps something like this is in order? > > - if (uni->array_elements == 0 && count > 1) { > + if (uni && uni->array_elements == 0 && count > 1) {
Actually, that just moves the issue a few lines, as line 282 also dereferences 'uni'. Earlying out by checking for NULL in uni seems more appropriate, but if I do so without setting GL_INVALID_OEPRATION, the piglit test fails. Doing ---8<--- diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp index 4cd2bca..4483948 100644 --- a/src/mesa/main/uniform_query.cpp +++ b/src/mesa/main/uniform_query.cpp @@ -268,6 +268,12 @@ validate_uniform_parameters(struct gl_context *ctx, return NULL; struct gl_uniform_storage *const uni = shProg->UniformRemapTable[location]; + if (!uni) { + _mesa_error(ctx, GL_INVALID_OPERATION, + "%s(nonexistent uniform, location=%d)", + caller, location); + return NULL; + } if (uni->array_elements == 0 && count > 1) { _mesa_error(ctx, GL_INVALID_OPERATION, ---8<--- ...seems to fix both the crash and the test. Looking a bit more closely at the test, it seems somewhat appropriate. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev