Translator: Validate negative index after comma

Expressions such as array[something,-1] where not validated because the
index (something,-1) is not a constant, but they can be because only the
right hand side of comma matters.

Bug: chromium:465696600
Change-Id: I0f8dd115b4c3adefb81e9bafe12ee453bb96ebd1
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/7224467
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>

Author: ShabbyX

src/compiler/translator/ParseContext.cpp

@@ -6649,29 +6649,55 @@ TIntermTyped *TParseContext::addIndexExpression(TIntermTyped *baseExpression,
         }
     }
 
-    if (indexConstantUnion)
+    int index                   = 0;
+    bool outOfRangeIndexIsError = false;
+
+    // If the index is using the comma operator, descend to the right-most value, see if that's a
+    // constant.  This is used for validating the index only, we can't constant fold the expression
+    // due to the left-hand-side of the comma.
+    TIntermTyped *commaRHS = indexExpression;
+    while (true)
     {
-        // If an out-of-range index is not qualified as constant, the behavior in the spec is
-        // undefined. This applies even if ANGLE has been able to constant fold it (ANGLE may
-        // constant fold expressions that are not constant expressions). The most compatible way to
-        // handle this case is to report a warning instead of an error and force the index to be in
-        // the correct range.
-        bool outOfRangeIndexIsError = indexExpression->getQualifier() == EvqConst;
-        int index                   = 0;
-        if (indexConstantUnion->getBasicType() == EbtInt)
+        TIntermConstantUnion *constant = commaRHS->getAsConstantUnion();
+        if (constant)
         {
-            index = indexConstantUnion->getIConst(0);
+            // If an out-of-range index is not qualified as constant, the behavior in the spec is
+            // undefined. This applies even if ANGLE has been able to constant fold it (ANGLE may
+            // constant fold expressions that are not constant expressions). The most compatible way
+            // to handle this case is to report a warning instead of an error and force the index to
+            // be in the correct range.
+            outOfRangeIndexIsError = commaRHS->getQualifier() == EvqConst;
+            index                  = 0;
+            if (constant->getBasicType() == EbtInt)
+            {
+                index = constant->getIConst(0);
+            }
+            else if (constant->getBasicType() == EbtUInt)
+            {
+                index = static_cast<int>(constant->getUConst(0));
+            }
+
+            if (index < 0)
+            {
+                outOfRangeError(outOfRangeIndexIsError, location, "index expression is negative",
+                                "[]");
+            }
+            break;
         }
-        else if (indexConstantUnion->getBasicType() == EbtUInt)
+
+        TIntermBinary *asBinary = commaRHS->getAsBinaryNode();
+        if (asBinary == nullptr || asBinary->getOp() != EOpComma)
         {
-            index = static_cast<int>(indexConstantUnion->getUConst(0));
+            break;
         }
+        commaRHS = asBinary->getRight();
+    }
 
+    if (indexConstantUnion)
+    {
         int safeIndex = -1;
-
         if (index < 0)
         {
-            outOfRangeError(outOfRangeIndexIsError, location, "index expression is negative", "[]");
             safeIndex = 0;
         }
 

src/tests/gl_tests/GLSLValidationTest.cpp

@@ -4712,6 +4712,39 @@ void main (void)
                   "GL_EXT_shader_framebuffer_fetch_non_coherent extension is used");
 }
 
+// Ensure that a negative index after a comma generates an error.
+TEST_P(GLSLValidationTest_ES3, NegativeIndexAfterComma)
+{
+    const char kFS[] = R"(#version 300 es
+layout(location = 0) out mediump vec4 o_color;
+uniform mediump float u;
+uniform mediump vec4 u_color[4];
+
+void main (void)
+{
+    o_color = u_color[u,-2];
+})";
+
+    validateError(GL_FRAGMENT_SHADER, kFS, "index expression is negative");
+}
+
+// Ensure that a negative const-variable index after a comma generates an error.
+TEST_P(GLSLValidationTest_ES3, NegativeConstVarIndexAfterComma)
+{
+    const char kFS[] = R"(#version 300 es
+layout(location = 0) out mediump vec4 o_color;
+uniform mediump float u;
+uniform mediump vec4 u_color[4];
+
+void main (void)
+{
+    const int index = -2;
+    o_color = u_color[u,index];
+})";
+
+    validateError(GL_FRAGMENT_SHADER, kFS, "index expression is negative");
+}
+
 // Validate that clip/cull distance extensions are not available in ESSL 100
 TEST_P(GLSLValidationTest, ClipCullDistance)
 {