Clamped negative index access.

Clamped negative index access.

Fixed error that allowed negative index for accessing
vector, matrix, and array. Now we report compile error
and clamp the index to 0.

Re-arranged code around it to handle negative index
at the one location.

BUG=crbug.com/239411
TEST=bug test case
R=aedla@chromium.org, kbr@chromium.org

Committed: https://code.google.com/p/angleproject/source/detail?r=2207

[kbr1, 2013-05-13 21:24:49]

LGTM

https://codereview.appspot.com/9193045/diff/1/src/compiler/glslang.y
File src/compiler/glslang.y (right):

https://codereview.appspot.com/9193045/diff/1/src/compiler/glslang.y#newcode293
src/compiler/glslang.y:293: if ($1->getType().getArraySize() == 0) {
What case does this handle? GLSL ES does not support unsized arrays (see spec
1.0.17 section 4.19 "Arrays"); if this branch is attempting to support them,
then perhaps we should simply remove the code and signal an error in this case.
(I see some special-case code for gl_FragData, but it isn't clear whether the
rest of it is needed.)

[Alok Priyadarshi, 2013-05-13 21:41:17]

https://codereview.appspot.com/9193045/diff/1/src/compiler/glslang.y
File src/compiler/glslang.y (right):

https://codereview.appspot.com/9193045/diff/1/src/compiler/glslang.y#newcode293
src/compiler/glslang.y:293: if ($1->getType().getArraySize() == 0) {
I noticed it too and was tempted to delete it in this patch. However we might
need to merge this patch to M28 branch. I will do it and other cleanup in a
separate patch.

[aedla, 2013-05-14 08:16:47]

LGTM, looks somewhat cleaner now.

Not sure why src/compiler/glslang_tab.cpp says "Bad content. Try to upload
again.". Doesn't really matter for review though.

https://codereview.appspot.com/9193045/diff/1/src/compiler/glslang.y
File src/compiler/glslang.y (right):

https://codereview.appspot.com/9193045/diff/1/src/compiler/glslang.y#newcode281
src/compiler/glslang.y:281: if ($1->isArray()) { // constant folding for arrays
Is there a way to create const arrays? I think GLSL ES doesn't support them. I
have only been able to create them by abusing bugs from http://crbug.com/179653

https://codereview.appspot.com/9193045/diff/1/src/compiler/glslang.y#newcode315
src/compiler/glslang.y:315: $$ = context->intermediate.addIndex(EOpIndexDirect,
$1, $3, @2);
This is now executed even with "field selection out of range" error, previously
it wasn't. Is it ok because the index is not used in case of error?

[Alok Priyadarshi, 2013-05-14 17:16:21]

https://codereview.appspot.com/9193045/diff/1/src/compiler/glslang.y
File src/compiler/glslang.y (right):

https://codereview.appspot.com/9193045/diff/1/src/compiler/glslang.y#newcode281
src/compiler/glslang.y:281: if ($1->isArray()) { // constant folding for arrays
You are right! This is dead code then. Will delete in a separate patch.

On 2013/05/14 08:16:47, aedla wrote:
> Is there a way to create const arrays? I think GLSL ES doesn't support them. I
> have only been able to create them by abusing bugs from
http://crbug.com/179653

https://codereview.appspot.com/9193045/diff/1/src/compiler/glslang.y#newcode315
src/compiler/glslang.y:315: $$ = context->intermediate.addIndex(EOpIndexDirect,
$1, $3, @2);
Good catch! Clamped the index to nominal size.

On 2013/05/14 08:16:47, aedla wrote:
> This is now executed even with "field selection out of range" error,
previously
> it wasn't. Is it ok because the index is not used in case of error?

[Alok Priyadarshi, 2013-05-14 17:18:03]

Committed patchset #2 manually as r2207 (presubmit successful).