Add unittests to verify invariant doesn't leak

src/compiler/translator/Compiler.cpp

@@ -208,7 +208,6 @@ TIntermNode *TCompiler::compileTreeImpl(const char *const shaderStrings[],
                                         const int compileOptions)
 {
     clearResults();
-    symbolTable.clearInvariantVaryings();
 
     ASSERT(numStrings > 0);
     ASSERT(GetGlobalPoolAllocator());

src/compiler/translator/ParseContext.cpp

@@ -410,12 +410,14 @@ void TParseContext::checkIsScalarInteger(TIntermTyped *node, const char *token)
 
 // Both test, and if necessary spit out an error, to see if we are currently
 // globally scoped.
-void TParseContext::checkIsAtGlobalLevel(const TSourceLoc &line, const char *token)
+bool TParseContext::checkIsAtGlobalLevel(const TSourceLoc &line, const char *token)
 {
     if (!symbolTable.atGlobalLevel())
     {
         error(line, "only allowed at global scope", token);
+        return false;
     }
+    return true;
 }
 
 // For now, keep it simple:  if it starts "gl_", it's reserved, independent
@@ -1672,7 +1674,8 @@ TIntermAggregate *TParseContext::parseInvariantDeclaration(const TSourceLoc &inv
                                                            const TSymbol *symbol)
 {
     // invariant declaration
-    checkIsAtGlobalLevel(invariantLoc, "invariant varying");
+    if (!checkIsAtGlobalLevel(invariantLoc, "invariant varying"))
+        return nullptr;
 
     if (!symbol)
     {

src/compiler/translator/ParseContext.h

@@ -138,7 +138,7 @@ class TParseContext : angle::NonCopyable
     bool checkCanBeLValue(const TSourceLoc &line, const char *op, TIntermTyped *node);
     void checkIsConst(TIntermTyped *node);
     void checkIsScalarInteger(TIntermTyped *node, const char *token);
-    void checkIsAtGlobalLevel(const TSourceLoc &line, const char *token);
+    bool checkIsAtGlobalLevel(const TSourceLoc &line, const char *token);
     bool checkConstructorArguments(const TSourceLoc &line,
                                    TIntermNode *argumentsNode,
                                    const TFunction &function,

src/compiler/translator/SymbolTable.h

@@ -296,6 +296,7 @@ class TSymbolTableLevel
     typedef std::pair<tLevel::iterator, bool> tInsertResult;
 
     TSymbolTableLevel()
+        : mGlobalInvariant(false)
     {
     }
     ~TSymbolTableLevel();
@@ -307,8 +308,22 @@ class TSymbolTableLevel
 
     TSymbol *find(const TString &name) const;
 
+    void addInvariantVarying(const std::string &name)
+    {
+        mInvariantVaryings.insert(name);
+    }
+
+    bool isVaryingInvariant(const std::string &name)
+    {
+        return (mGlobalInvariant || mInvariantVaryings.count(name) > 0);
+    }
+
+    void setGlobalInvariant(bool invariant) { mGlobalInvariant = invariant; }
+
   protected:
     tLevel level;
+    std::set<std::string> mInvariantVaryings;
+    bool mGlobalInvariant;
 };
 
 // Define ESymbolLevel as int rather than an enum since level can go
@@ -326,7 +341,6 @@ class TSymbolTable : angle::NonCopyable
 {
   public:
     TSymbolTable()
-        : mGlobalInvariant(false)
     {
         // The symbol table cannot be used until push() is called, but
         // the lack of an initial call to push() can be used to detect
@@ -348,7 +362,7 @@ class TSymbolTable : angle::NonCopyable
     }
     bool atGlobalLevel() const
     {
-        return currentLevel() <= GLOBAL_LEVEL;
+        return currentLevel() == GLOBAL_LEVEL;
     }
     void push()
     {
@@ -477,25 +491,24 @@ class TSymbolTable : angle::NonCopyable
     // "invariant varying_name;".
     void addInvariantVarying(const std::string &originalName)
     {
-        mInvariantVaryings.insert(originalName);
-    }
-
-    void clearInvariantVaryings()
-    {
-        mInvariantVaryings.clear();
+        ASSERT(atGlobalLevel());
+        table[currentLevel()]->addInvariantVarying(originalName);
     }
-
     // If this returns false, the varying could still be invariant
     // if it is set as invariant during the varying variable
     // declaration - this piece of information is stored in the
     // variable's type, not here.
     bool isVaryingInvariant(const std::string &originalName) const
     {
-      return (mGlobalInvariant ||
-              mInvariantVaryings.count(originalName) > 0);
+        ASSERT(atGlobalLevel());
+        return table[currentLevel()]->isVaryingInvariant(originalName);
     }
 
-    void setGlobalInvariant(bool invariant) { mGlobalInvariant = invariant; }
+    void setGlobalInvariant(bool invariant)
+    {
+        ASSERT(atGlobalLevel());
+        table[currentLevel()]->setGlobalInvariant(invariant);
+    }
 
     static int nextUniqueId()
     {
@@ -525,9 +538,6 @@ class TSymbolTable : angle::NonCopyable
 
     std::set<std::string> mUnmangledBuiltinNames;
 
-    std::set<std::string> mInvariantVaryings;
-    bool mGlobalInvariant;
-
     static int uniqueIdCounter;
 };
 

src/tests/compiler_tests/ShaderVariable_test.cpp

@@ -8,6 +8,7 @@
 //
 
 #include "angle_gl.h"
+#include "compiler/translator/Compiler.h"
 #include "gtest/gtest.h"
 #include "GLSLANG/ShaderLang.h"
 
@@ -49,7 +50,7 @@ TEST(ShaderVariableTest, FindInfoByMappedName)
         uni.fields.push_back(a);
     }
 
-    const ShaderVariable *leafVar = NULL;
+    const ShaderVariable *leafVar = nullptr;
     std::string originalFullName;
 
     std::string mappedFullName = "wrongName";
@@ -250,4 +251,171 @@ TEST(ShaderVariableTest, InvariantDoubleDeleteBug)
     ShDestruct(compiler);
 }
 
+TEST(ShaderVariableTest, IllegalInvariantVarying)
+{
+    ShBuiltInResources resources;
+    ShInitBuiltInResources(&resources);
+
+    ShHandle compiler = ShConstructCompiler(GL_VERTEX_SHADER, SH_GLES2_SPEC,
+                                            SH_GLSL_COMPATIBILITY_OUTPUT, &resources);
+    EXPECT_NE(static_cast<ShHandle>(0), compiler);
+
+    const char *program1[] =
+    {
+        "void foo() {\n"
+        "  vec4 v;\n"
+        "}\n"
+        "varying vec4 v_varying;\n"
+        "invariant v_varying;\n"
+        "void main() {\n"
+        "  foo();\n"
+        "  gl_Position = v_varying;\n"
+        "}"
+    };
+    const char *program2[] =
+    {
+        "varying vec4 v_varying;\n"
+        "void foo() {\n"
+        "  invariant v_varying;\n"
+        "}\n"
+        "void main() {\n"
+        "  foo();\n"
+        "  gl_Position = v_varying;\n"
+        "}"
+    };
+
+    EXPECT_TRUE(ShCompile(compiler, program1, 1, SH_VARIABLES));
+    EXPECT_FALSE(ShCompile(compiler, program2, 1, SH_VARIABLES));
+}
+
+TEST(ShaderVariableTest, InvariantLeakAcrossShaders)
+{
+    ShBuiltInResources resources;
+    ShInitBuiltInResources(&resources);
+
+    ShHandle compiler = ShConstructCompiler(GL_VERTEX_SHADER, SH_GLES2_SPEC,
+                                            SH_GLSL_COMPATIBILITY_OUTPUT, &resources);
+    EXPECT_NE(static_cast<ShHandle>(0), compiler);
+
+    const char *program1[] =
+    {
+        "varying vec4 v_varying;\n"
+        "invariant v_varying;\n"
+        "void main() {\n"
+        "  gl_Position = v_varying;\n"
+        "}"
+    };
+    const char *program2[] =
+    {
+        "varying vec4 v_varying;\n"
+        "void main() {\n"
+        "  gl_Position = v_varying;\n"
+        "}"
+    };
+
+    EXPECT_TRUE(ShCompile(compiler, program1, 1, SH_VARIABLES));
+    const std::vector<sh::Varying> *varyings = ShGetVaryings(compiler);
+    for (const sh::Varying &varying : *varyings)
+    {
+        if (varying.name == "v_varying")
+            EXPECT_TRUE(varying.isInvariant);
+    }
+    EXPECT_TRUE(ShCompile(compiler, program2, 1, SH_VARIABLES));
+    varyings = ShGetVaryings(compiler);
+    for (const sh::Varying &varying : *varyings)
+    {
+        if (varying.name == "v_varying")
+            EXPECT_FALSE(varying.isInvariant);
+    }
+}
+
+TEST(ShaderVariableTest, GlobalInvariantLeakAcrossShaders)
+{
+    ShBuiltInResources resources;
+    ShInitBuiltInResources(&resources);
+
+    ShHandle compiler = ShConstructCompiler(GL_VERTEX_SHADER, SH_GLES2_SPEC,
+                                            SH_GLSL_COMPATIBILITY_OUTPUT, &resources);
+    EXPECT_NE(static_cast<ShHandle>(0), compiler);
+
+    const char *program1[] =
+    {
+        "#pragma STDGL invariant(all)\n"
+        "varying vec4 v_varying;\n"
+        "void main() {\n"
+        "  gl_Position = v_varying;\n"
+        "}"
+    };
+    const char *program2[] =
+    {
+        "varying vec4 v_varying;\n"
+        "void main() {\n"
+        "  gl_Position = v_varying;\n"
+        "}"
+    };
+
+    EXPECT_TRUE(ShCompile(compiler, program1, 1, SH_VARIABLES));
+    const std::vector<sh::Varying> *varyings = ShGetVaryings(compiler);
+    for (const sh::Varying &varying : *varyings)
+    {
+        if (varying.name == "v_varying")
+            EXPECT_TRUE(varying.isInvariant);
+    }
+    EXPECT_TRUE(ShCompile(compiler, program2, 1, SH_VARIABLES));
+    varyings = ShGetVaryings(compiler);
+    for (const sh::Varying &varying : *varyings)
+    {
+        if (varying.name == "v_varying")
+            EXPECT_FALSE(varying.isInvariant);
+    }
+}
+
+TEST(ShaderVariableTest, BuiltinInvariantVarying)
+{
+
+    ShBuiltInResources resources;
+    ShInitBuiltInResources(&resources);
+
+    ShHandle compiler = ShConstructCompiler(GL_VERTEX_SHADER, SH_GLES2_SPEC,
+                                            SH_GLSL_COMPATIBILITY_OUTPUT, &resources);
+    EXPECT_NE(static_cast<ShHandle>(0), compiler);
+
+    const char *program1[] =
+    {
+        "invariant gl_Position;\n"
+        "void main() {\n"
+        "  gl_Position = vec4(0, 0, 0, 0);\n"
+        "}"
+    };
+    const char *program2[] =
+    {
+        "void main() {\n"
+        "  gl_Position = vec4(0, 0, 0, 0);\n"
+        "}"
+    };
+    const char *program3[] =
+    {
+        "void main() {\n"
+        "  invariant gl_Position;\n"
+        "  gl_Position = vec4(0, 0, 0, 0);\n"
+        "}"
+    };
+
+    EXPECT_TRUE(ShCompile(compiler, program1, 1, SH_VARIABLES));
+    const std::vector<sh::Varying> *varyings = ShGetVaryings(compiler);
+    for (const sh::Varying &varying : *varyings)
+    {
+        if (varying.name == "gl_Position")
+            EXPECT_TRUE(varying.isInvariant);
+    }
+    EXPECT_TRUE(ShCompile(compiler, program2, 1, SH_VARIABLES));
+    varyings = ShGetVaryings(compiler);
+    for (const sh::Varying &varying : *varyings)
+    {
+        if (varying.name == "gl_Position")
+            EXPECT_FALSE(varying.isInvariant);
+    }
+    EXPECT_FALSE(ShCompile(compiler, program3, 1, SH_VARIABLES));
+}
+
 }  // namespace sh