translator: Fix validation sometimes modifying builtins.

src/compiler/translator/IntermNode.cpp

@@ -64,12 +64,12 @@ bool ValidateMultiplication(TOperator op, const TType &left, const TType &right)
 }
 
 bool CompareStructure(const TType& leftNodeType,
-                      TConstantUnion *rightUnionArray,
-                      TConstantUnion *leftUnionArray);
+                      const TConstantUnion *rightUnionArray,
+                      const TConstantUnion *leftUnionArray);
 
 bool CompareStruct(const TType &leftNodeType,
-                   TConstantUnion *rightUnionArray,
-                   TConstantUnion *leftUnionArray)
+                   const TConstantUnion *rightUnionArray,
+                   const TConstantUnion *leftUnionArray)
 {
     const TFieldList &fields = leftNodeType.getStruct()->fields();
 
@@ -102,8 +102,8 @@ bool CompareStruct(const TType &leftNodeType,
 }
 
 bool CompareStructure(const TType &leftNodeType,
-                      TConstantUnion *rightUnionArray,
-                      TConstantUnion *leftUnionArray)
+                      const TConstantUnion *rightUnionArray,
+                      const TConstantUnion *leftUnionArray)
 {
     if (leftNodeType.isArray())
     {

src/compiler/translator/IntermNode.h

@@ -270,7 +270,8 @@ class TIntermConstantUnion : public TIntermTyped
 
     virtual bool hasSideEffects() const { return false; }
 
-    TConstantUnion *getUnionArrayPointer() const { return mUnionArrayPointer; }
+    const TConstantUnion *getUnionArrayPointer() const { return mUnionArrayPointer; }
+    TConstantUnion *getUnionArrayPointer() { return mUnionArrayPointer; }
 
     int getIConst(size_t index) const
     {
@@ -289,6 +290,12 @@ class TIntermConstantUnion : public TIntermTyped
         return mUnionArrayPointer ? mUnionArrayPointer[index].getBConst() : false;
     }
 
+    void replaceConstantUnion(TConstantUnion *safeConstantUnion)
+    {
+        // Previous union pointer freed on pool deallocation.
+        mUnionArrayPointer = safeConstantUnion;
+    }
+
     virtual TIntermConstantUnion *getAsConstantUnion()  { return this; }
     virtual void traverse(TIntermTraverser *);
     virtual bool replaceChildNode(TIntermNode *, TIntermNode *) { return false; }

src/compiler/translator/Intermediate.cpp

@@ -361,9 +361,9 @@ TIntermCase *TIntermediate::addCase(
 //
 
 TIntermConstantUnion *TIntermediate::addConstantUnion(
-    TConstantUnion *unionArrayPointer, const TType &t, const TSourceLoc &line)
+    TConstantUnion *constantUnion, const TType &type, const TSourceLoc &line)
 {
-    TIntermConstantUnion *node = new TIntermConstantUnion(unionArrayPointer, t);
+    TIntermConstantUnion *node = new TIntermConstantUnion(constantUnion, type);
     node->setLine(line);
 
     return node;

src/compiler/translator/Intermediate.h

@@ -49,7 +49,8 @@ class TIntermediate
         TIntermTyped *condition, const TSourceLoc &line);
     TIntermTyped *addComma(
         TIntermTyped *left, TIntermTyped *right, const TSourceLoc &);
-    TIntermConstantUnion *addConstantUnion(TConstantUnion *, const TType &, const TSourceLoc &);
+    TIntermConstantUnion *addConstantUnion(
+        TConstantUnion *constantUnion, const TType &type, const TSourceLoc &line);
     // TODO(zmo): Get rid of default value.
     bool parseConstTree(const TSourceLoc &, TIntermNode *, TConstantUnion *,
                         TOperator, TType, bool singleConstantParam = false);

src/compiler/translator/ParseContext.cpp

@@ -1814,7 +1814,7 @@ TIntermTyped* TParseContext::addConstVectorNode(TVectorFields& fields, TIntermTy
     TIntermTyped* typedNode;
     TIntermConstantUnion* tempConstantNode = node->getAsConstantUnion();
 
-    TConstantUnion *unionArray;
+    const TConstantUnion *unionArray;
     if (tempConstantNode) {
         unionArray = tempConstantNode->getUnionArrayPointer();
 
@@ -1868,7 +1868,7 @@ TIntermTyped* TParseContext::addConstMatrixNode(int index, TIntermTyped* node, c
     }
 
     if (tempConstantNode) {
-         TConstantUnion* unionArray = tempConstantNode->getUnionArrayPointer();
+         TConstantUnion *unionArray = tempConstantNode->getUnionArrayPointer();
          int size = tempConstantNode->getType().getCols();
          typedNode = intermediate.addConstantUnion(&unionArray[size*index], tempConstantNode->getType(), line);
     } else {
@@ -2202,6 +2202,8 @@ TIntermTyped* TParseContext::addIndexExpression(TIntermTyped *baseExpression, co
         }
         else
         {
+            int safeIndex = -1;
+
             if (baseExpression->isArray())
             {
                 if (index >= baseExpression->getType().getArraySize())
@@ -2211,13 +2213,13 @@ TIntermTyped* TParseContext::addIndexExpression(TIntermTyped *baseExpression, co
                     std::string extraInfo = extraInfoStream.str();
                     error(location, "", "[", extraInfo.c_str());
                     recover();
-                    index = baseExpression->getType().getArraySize() - 1;
+                    safeIndex = baseExpression->getType().getArraySize() - 1;
                 }
                 else if (baseExpression->getQualifier() == EvqFragData && index > 0 && !isExtensionEnabled("GL_EXT_draw_buffers"))
                 {
                     error(location, "", "[", "array indexes for gl_FragData must be zero when GL_EXT_draw_buffers is disabled");
                     recover();
-                    index = 0;
+                    safeIndex = 0;
                 }
             }
             else if ((baseExpression->isVector() || baseExpression->isMatrix()) && baseExpression->getType().getNominalSize() <= index)
@@ -2227,10 +2229,18 @@ TIntermTyped* TParseContext::addIndexExpression(TIntermTyped *baseExpression, co
                 std::string extraInfo = extraInfoStream.str();
                 error(location, "", "[", extraInfo.c_str());
                 recover();
-                index = baseExpression->getType().getNominalSize() - 1;
+                safeIndex = baseExpression->getType().getNominalSize() - 1;
+            }
+
+            // Don't modify the data of the previous constant union, because it can point
+            // to builtins, like gl_MaxDrawBuffers. Instead use a new sanitized object.
+            if (safeIndex != -1)
+            {
+                TConstantUnion *safeConstantUnion = new TConstantUnion();
+                safeConstantUnion->setIConst(safeIndex);
+                indexConstantUnion->replaceConstantUnion(safeConstantUnion);
             }
 
-            indexConstantUnion->getUnionArrayPointer()->setIConst(index);
             indexedExpression = intermediate.addIndex(EOpIndexDirect, baseExpression, indexExpression, location);
         }
     }

src/compiler/translator/parseConst.cpp

@@ -177,7 +177,7 @@ void TConstTraverser::visitConstantUnion(TIntermConstantUnion *node)
     if (!mSingleConstantParam)
     {
         size_t objectSize = node->getType().getObjectSize();
-        TConstantUnion *rightUnionArray = node->getUnionArrayPointer();
+        const TConstantUnion *rightUnionArray = node->getUnionArrayPointer();
         for (size_t i=0; i < objectSize; i++)
         {
             if (mIndex >= instanceSize)
@@ -189,7 +189,7 @@ void TConstTraverser::visitConstantUnion(TIntermConstantUnion *node)
     else
     {
         size_t totalSize = mIndex + mSize;
-        TConstantUnion *rightUnionArray = node->getUnionArrayPointer();
+        const TConstantUnion *rightUnionArray = node->getUnionArrayPointer();
         if (!mIsDiagonalMatrixInit)
         {
             int count = 0;

src/tests/compiler_tests/ShaderExtension_test.cpp

@@ -81,7 +81,7 @@ class ShaderExtensionTest : public testing::Test
     {
         bool success = ShCompile(mCompiler, shaderStrings, stringCount, 0);
         const std::string& compileLog = ShGetInfoLog(mCompiler);
-        EXPECT_EQ(success, expectation) << compileLog;
+        EXPECT_EQ(expectation, success) << compileLog;
     }
 
   protected:
@@ -195,3 +195,23 @@ TEST_F(ShaderExtensionTest, TextureLODExtResetsInternalStates)
     TestShaderExtension(&TextureLODShdr[1], 1, false);
     TestShaderExtension(TextureLODShdr, 2, true);
 }
+
+// Test a bug where we could modify the value of a builtin variable.
+TEST_F(ShaderExtensionTest, BuiltinRewritingBug)
+{
+    mResources.MaxDrawBuffers = 4;
+    mResources.EXT_draw_buffers = 1;
+    InitializeCompiler();
+
+    const std::string &shaderString =
+        "#extension GL_EXT_draw_buffers : require\n"
+        "precision mediump float;\n"
+        "void main() {\n"
+        "    gl_FragData[gl_MaxDrawBuffers] = vec4(0.0);\n"
+        "}";
+
+    const char *shaderStrings[] = { shaderString.c_str() };
+
+    TestShaderExtension(shaderStrings, 1, false);
+    TestShaderExtension(shaderStrings, 1, false);
+}