Should have mentioned it does pass piglit, but now that I look I don't see
any tests generating an out-of-bounds array index. I also made my own test
program which does go out of bounds, and I'll submit that for inclusion in
piglit.
On Fri, Nov 2, 2012 at 4:51 PM, Ian Romanick <i...@freedesktop.org> wrote:
> On 11/02/2012 01:12 PM, Frank Henigman wrote:
>
>> validate_uniform_parameters now checks that the array index is
>> valid. This means if an index is out of bounds, glGetUniform* now
>> fails with GL_INVALID_OPERATION, as it should.
>> _mesa_uniform and _mesa_uniform_matrix also call
>> validate_uniform_parameters so the bounds checks there became
>> redundant and were removed.
>>
>> The test in glGetUniformLocation is modified to check array bounds
>> so it now returns GL_INVALID_INDEX (-1) if you ask for the location
>> of a non-existent array element, as it should.
>>
>
> Do we have piglit tests for either of these cases?
>
> Do all of the existing piglit tests that pass still pass with this change?
> It seems like they should...
>
>
> ---
>> src/mesa/main/uniform_query.**cpp | 26 +++++++++++++-------------
>> 1 files changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/mesa/main/uniform_query.**cpp
>> b/src/mesa/main/uniform_query.**cpp
>> index bddb8f9..66a2399 100644
>> --- a/src/mesa/main/uniform_query.**cpp
>> +++ b/src/mesa/main/uniform_query.**cpp
>> @@ -237,11 +237,14 @@ validate_uniform_parameters(**struct gl_context
>> *ctx,
>> return false;
>> }
>>
>> - /* This case should be impossible. The implication is that a call
>> like
>> - * glGetUniformLocation(prog, "foo[8]") was successful but "foo" is
>> not an
>> - * array.
>> + /* If the uniform is an array, check that array_index is in bounds.
>> + * If not an array, check that array_index is zero.
>> + * array_index is unsigned so no need to check for less than zero.
>> */
>> - if (*array_index != 0 && shProg->UniformStorage[*loc].**array_elements
>> == 0) {
>> + unsigned limit = shProg->UniformStorage[*loc].**array_elements;
>> + if (limit == 0)
>> + limit = 1;
>> + if (*array_index >= limit) {
>> _mesa_error(ctx, GL_INVALID_OPERATION, "%s(location=%d)",
>> caller, location);
>> return false;
>> @@ -728,9 +731,6 @@ _mesa_uniform(struct gl_context *ctx, struct
>> gl_shader_program *shProg,
>> * will have already generated an error.
>> */
>> if (uni->array_elements != 0) {
>> - if (offset >= uni->array_elements)
>> - return;
>> -
>> count = MIN2(count, (int) (uni->array_elements - offset));
>> }
>>
>> @@ -885,9 +885,6 @@ _mesa_uniform_matrix(struct gl_context *ctx, struct
>> gl_shader_program *shProg,
>> * will have already generated an error.
>> */
>> if (uni->array_elements != 0) {
>> - if (offset >= uni->array_elements)
>> - return;
>> -
>> count = MIN2(count, (int) (uni->array_elements - offset));
>> }
>>
>> @@ -1021,10 +1018,13 @@ _mesa_get_uniform_location(**struct gl_context
>> *ctx,
>> if (!found)
>> return GL_INVALID_INDEX;
>>
>> - /* Since array_elements is 0 for non-arrays, this causes look-ups of
>> 'a[0]'
>> - * to (correctly) fail if 'a' is not an array.
>> + /* If the uniform is an array, fail if the index is out of bounds.
>> + * (A negative index is caught above.) This also fails if the uniform
>> + * is not an array, but the user is trying to index it, because
>> + * array_elements is zero and offset >= 0.
>> */
>> - if (array_lookup && shProg->UniformStorage[**location].array_elements
>> == 0) {
>> + if (array_lookup
>> + && offset >= shProg->UniformStorage[**location].array_elements)
>> {
>> return GL_INVALID_INDEX;
>> }
>>
>>
>>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
