[Mesa-stable] [PATCH 1/4] glsl: Fix crash due to negative array index

[Anuj Phogat]

On Wed, Oct 22, 2014 at 1:40 PM, Chris Forbes <chrisf at ijw.co.nz> wrote:
> Patches 1 & 2 are:
>
> Reviewed-by: Chris Forbes <chrisf at ijw.co.nz>
>
> I'm not convinced about patch 3. From the GLSL 4.50 spec, section
> 4.1.9 Arrays p31 (p37 of the PDF):
>
> "It is a compile-time error to declare an array with a size, and then
> later (in the same shader) index the same array with an integral
> constant expression greater than or equal to the declared size."
>
Right. We need the compile error in case of index represented by a
constant expression. I'll drop patches 3 and 4.

Compilation happens successfully in case of index represented by a
non-constant expression.

I will also modify few test cases accordingly in following piglit patch:
"[PATCH] Add shader tests for out of bounds array index"
>
> -- Chris
>
> On Thu, Oct 23, 2014 at 8:33 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> Hi Anuj,
>>
>> Afaics the series is yet to land on master, this I've not pulled it for
>> the stable branch. Is the lack of review holding it back ?
>>
>> Gents can anyone take a look at/review the series ?
>>
>> Thanks
>> Emil
>>
>> On 22/09/14 23:57, Anuj Phogat wrote:
>>> Currently Mesa crashes with a shader like this:
>>>
>>> [fragmnet shader]
>>> float[5] array;
>>> int idx = -2;
>>> void main()
>>> {
>>>    gl_FragColor = vec4(0.0, 1.0, 0.0, array[idx]);
>>> }
>>>
>>> Cc: <mesa-stable at lists.freedesktop.org>
>>> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
>>> ---
>>>  src/glsl/opt_array_splitting.cpp | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/src/glsl/opt_array_splitting.cpp b/src/glsl/opt_array_splitting.cpp
>>> index ebb076b..9e73f3c 100644
>>> --- a/src/glsl/opt_array_splitting.cpp
>>> +++ b/src/glsl/opt_array_splitting.cpp
>>> @@ -295,7 +295,7 @@ ir_array_splitting_visitor::split_deref(ir_dereference **deref)
>>>     ir_constant *constant = deref_array->array_index->as_constant();
>>>     assert(constant);
>>>
>>> -   if (constant->value.i[0] < (int)entry->size) {
>>> +   if (constant->value.i[0] >= 0 && constant->value.i[0] < (int)entry->size) {
>>>        *deref = new(entry->mem_ctx)
>>>        ir_dereference_variable(entry->components[constant->value.i[0]]);
>>>     } else {
>>>
>>
>> _______________________________________________
>> mesa-stable mailing list
>> mesa-stable at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-stable