Initial support for compiler AST validation

src/compiler.gni

@@ -85,6 +85,8 @@ angle_translator_sources = [
   "src/compiler/translator/SymbolUniqueId.h",
   "src/compiler/translator/Types.cpp",
   "src/compiler/translator/Types.h",
+  "src/compiler/translator/ValidateAST.cpp",
+  "src/compiler/translator/ValidateAST.h",
   "src/compiler/translator/ValidateGlobalInitializer.cpp",
   "src/compiler/translator/ValidateGlobalInitializer.h",
   "src/compiler/translator/ValidateLimitations.cpp",

src/compiler/translator/Compiler.cpp

@@ -493,6 +493,10 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
     FoldExpressions(root, &mDiagnostics);
     // Folding should only be able to generate warnings.
     ASSERT(mDiagnostics.numErrors() == 0);
+    if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+    {
+        return false;
+    }
 
     // We prune no-ops to work around driver bugs and to keep AST processing and output simple.
     // The following kinds of no-ops are pruned:
@@ -502,6 +506,10 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
     //      invalid ESSL.
     // After this empty declarations are not allowed in the AST.
     PruneNoOps(root, &mSymbolTable);
+    if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+    {
+        return false;
+    }
 
     // Create the function DAG and check there is no recursion
     if (!initCallDag(root))
@@ -525,6 +533,10 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
     if (!(compileOptions & SH_DONT_PRUNE_UNUSED_FUNCTIONS))
     {
         pruneUnusedFunctions(root);
+        if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+        {
+            return false;
+        }
     }
 
     if (mShaderVersion >= 310 && !ValidateVaryingLocations(root, &mDiagnostics, mShaderType))
@@ -558,29 +570,57 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
     {
         DeclareAndInitBuiltinsForInstancedMultiview(root, mNumViews, mShaderType, compileOptions,
                                                     mOutputType, &mSymbolTable);
+        if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+        {
+            return false;
+        }
     }
 
     // This pass might emit short circuits so keep it before the short circuit unfolding
     if (compileOptions & SH_REWRITE_DO_WHILE_LOOPS)
+    {
         RewriteDoWhile(root, &mSymbolTable);
+        if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+        {
+            return false;
+        }
+    }
 
     if (compileOptions & SH_ADD_AND_TRUE_TO_LOOP_CONDITION)
+    {
         AddAndTrueToLoopCondition(root);
+        if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+        {
+            return false;
+        }
+    }
 
     if (compileOptions & SH_UNFOLD_SHORT_CIRCUIT)
     {
         UnfoldShortCircuitAST(root);
+        if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+        {
+            return false;
+        }
     }
 
     if (compileOptions & SH_REMOVE_POW_WITH_CONSTANT_EXPONENT)
     {
         RemovePow(root, &mSymbolTable);
+        if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+        {
+            return false;
+        }
     }
 
     if (compileOptions & SH_REGENERATE_STRUCT_NAMES)
     {
         RegenerateStructNames gen(&mSymbolTable);
         root->traverse(&gen);
+        if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+        {
+            return false;
+        }
     }
 
     if (mShaderType == GL_VERTEX_SHADER &&
@@ -590,6 +630,10 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
         {
             EmulateGLDrawID(root, &mSymbolTable, &mUniforms,
                             shouldCollectVariables(compileOptions));
+            if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+            {
+                return false;
+            }
         }
     }
 
@@ -599,6 +643,10 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
     {
         EmulateGLFragColorBroadcast(root, mResources.MaxDrawBuffers, &mOutputVariables,
                                     &mSymbolTable, mShaderVersion);
+        if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+        {
+            return false;
+        }
     }
 
     int simplifyScalarized = (compileOptions & SH_SCALARIZE_VEC_AND_MAT_CONSTRUCTOR_ARGS)
@@ -612,17 +660,38 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
                            IntermNodePatternMatcher::kMultiDeclaration |
                                IntermNodePatternMatcher::kArrayLengthMethod | simplifyScalarized,
                            &getSymbolTable());
+    if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+    {
+        return false;
+    }
 
     // Note that separate declarations need to be run before other AST transformations that
     // generate new statements from expressions.
     SeparateDeclarations(root);
+    mValidateASTOptions.validateMultiDeclarations = true;
+    if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+    {
+        return false;
+    }
 
     SplitSequenceOperator(root, IntermNodePatternMatcher::kArrayLengthMethod | simplifyScalarized,
                           &getSymbolTable());
+    if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+    {
+        return false;
+    }
 
     RemoveArrayLengthMethod(root);
+    if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+    {
+        return false;
+    }
 
     RemoveUnreferencedVariables(root, &mSymbolTable);
+    if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+    {
+        return false;
+    }
 
     // In case the last case inside a switch statement is a certain type of no-op, GLSL compilers in
     // drivers may not accept it. In this case we clean up the dead code from the end of switch
@@ -631,6 +700,10 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
     // invalid state. Relies on that PruneNoOps and RemoveUnreferencedVariables have already been
     // run.
     PruneEmptyCases(root);
+    if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+    {
+        return false;
+    }
 
     // Built-in function emulation needs to happen after validateLimitations pass.
     // TODO(jmadill): Remove global pool allocator.
@@ -644,6 +717,10 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
     if (compileOptions & SH_SCALARIZE_VEC_AND_MAT_CONSTRUCTOR_ARGS)
     {
         ScalarizeVecAndMatConstructorArgs(root, mShaderType, highPrecisionSupported, &mSymbolTable);
+        if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+        {
+            return false;
+        }
     }
 
     if (shouldCollectVariables(compileOptions))
@@ -672,6 +749,10 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
         if ((compileOptions & SH_INIT_OUTPUT_VARIABLES) && (mShaderType != GL_COMPUTE_SHADER))
         {
             initializeOutputVariables(root);
+            if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+            {
+                return false;
+            }
         }
     }
 
@@ -680,6 +761,10 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
     if (RemoveInvariant(mShaderType, mShaderVersion, mOutputType, compileOptions))
     {
         RemoveInvariantDeclaration(root);
+        if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+        {
+            return false;
+        }
     }
 
     // gl_Position is always written in compatibility output mode.
@@ -690,6 +775,10 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
     {
         initializeGLPosition(root);
         mGLPositionInitialized = true;
+        if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+        {
+            return false;
+        }
     }
 
     // DeferGlobalInitializers needs to be run before other AST transformations that generate new
@@ -701,6 +790,10 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
     bool canUseLoopsToInitialize = !(compileOptions & SH_DONT_USE_LOOPS_TO_INITIALIZE_VARIABLES);
     DeferGlobalInitializers(root, initializeLocalsAndGlobals, canUseLoopsToInitialize,
                             highPrecisionSupported, &mSymbolTable);
+    if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+    {
+        return false;
+    }
 
     if (initializeLocalsAndGlobals)
     {
@@ -718,30 +811,54 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
                                    IntermNodePatternMatcher::kArrayDeclaration |
                                        IntermNodePatternMatcher::kNamelessStructDeclaration,
                                    &getSymbolTable());
+            if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+            {
+                return false;
+            }
         }
 
         InitializeUninitializedLocals(root, getShaderVersion(), canUseLoopsToInitialize,
                                       highPrecisionSupported, &getSymbolTable());
+        if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+        {
+            return false;
+        }
     }
 
     if (getShaderType() == GL_VERTEX_SHADER && (compileOptions & SH_CLAMP_POINT_SIZE))
     {
         ClampPointSize(root, mResources.MaxPointSize, &getSymbolTable());
+        if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+        {
+            return false;
+        }
     }
 
     if (getShaderType() == GL_FRAGMENT_SHADER && (compileOptions & SH_CLAMP_FRAG_DEPTH))
     {
         ClampFragDepth(root, &getSymbolTable());
+        if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+        {
+            return false;
+        }
     }
 
     if (compileOptions & SH_REWRITE_REPEATED_ASSIGN_TO_SWIZZLED)
     {
         sh::RewriteRepeatedAssignToSwizzled(root);
+        if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+        {
+            return false;
+        }
     }
 
     if (compileOptions & SH_REWRITE_VECTOR_SCALAR_ARITHMETIC)
     {
         VectorizeVectorScalarArithmetic(root, &getSymbolTable());
+        if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+        {
+            return false;
+        }
     }
 
     return true;

src/compiler/translator/Compiler.h

@@ -24,6 +24,7 @@
 #include "compiler/translator/InfoSink.h"
 #include "compiler/translator/Pragma.h"
 #include "compiler/translator/SymbolTable.h"
+#include "compiler/translator/ValidateAST.h"
 #include "third_party/compiler/ArrayBoundsClamper.h"
 
 namespace sh
@@ -277,6 +278,9 @@ class TCompiler : public TShHandleBase
     NameMap mNameMap;
 
     TPragma mPragma;
+
+    // Track what should be validated given passes currently applied.
+    ValidateASTOptions mValidateASTOptions;
 };
 
 //

src/compiler/translator/ValidateAST.cpp

+//
+// Copyright 2019 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+
+#include "compiler/translator/ValidateAST.h"
+
+#include "compiler/translator/Diagnostics.h"
+#include "compiler/translator/tree_util/IntermTraverse.h"
+
+namespace sh
+{
+
+namespace
+{
+
+class ValidateAST : public TIntermTraverser
+{
+  public:
+    static bool validate(TIntermNode *root,
+                         TDiagnostics *diagnostics,
+                         const ValidateASTOptions &options);
+
+    void visitSymbol(TIntermSymbol *node) override;
+    void visitConstantUnion(TIntermConstantUnion *node) override;
+    bool visitSwizzle(Visit visit, TIntermSwizzle *node) override;
+    bool visitBinary(Visit visit, TIntermBinary *node) override;
+    bool visitUnary(Visit visit, TIntermUnary *node) override;
+    bool visitTernary(Visit visit, TIntermTernary *node) override;
+    bool visitIfElse(Visit visit, TIntermIfElse *node) override;
+    bool visitSwitch(Visit visit, TIntermSwitch *node) override;
+    bool visitCase(Visit visit, TIntermCase *node) override;
+    void visitFunctionPrototype(TIntermFunctionPrototype *node) override;
+    bool visitFunctionDefinition(Visit visit, TIntermFunctionDefinition *node) override;
+    bool visitAggregate(Visit visit, TIntermAggregate *node) override;
+    bool visitBlock(Visit visit, TIntermBlock *node) override;
+    bool visitInvariantDeclaration(Visit visit, TIntermInvariantDeclaration *node) override;
+    bool visitDeclaration(Visit visit, TIntermDeclaration *node) override;
+    bool visitLoop(Visit visit, TIntermLoop *node) override;
+    bool visitBranch(Visit visit, TIntermBranch *node) override;
+    void visitPreprocessorDirective(TIntermPreprocessorDirective *node) override;
+
+  private:
+    ValidateAST(TIntermNode *root, TDiagnostics *diagnostics, const ValidateASTOptions &options);
+
+    // Visit as a generic node
+    void visitNode(Visit visit, TIntermNode *node);
+
+    void expectNonNullChildren(Visit visit, TIntermNode *node, size_t least_count);
+
+    bool validateInternal();
+
+    ValidateASTOptions mOptions;
+    TDiagnostics *mDiagnostics;
+
+    // For validateSingleParent:
+    std::map<TIntermNode *, TIntermNode *> mParent;
+    bool mSingleParentFailed = false;
+
+    // For validateNullNodes
+    bool mNullNodesFailed = false;
+};
+
+bool ValidateAST::validate(TIntermNode *root,
+                           TDiagnostics *diagnostics,
+                           const ValidateASTOptions &options)
+{
+    ValidateAST validate(root, diagnostics, options);
+    root->traverse(&validate);
+    return validate.validateInternal();
+}
+
+ValidateAST::ValidateAST(TIntermNode *root,
+                         TDiagnostics *diagnostics,
+                         const ValidateASTOptions &options)
+    : TIntermTraverser(true, false, true, nullptr), mOptions(options), mDiagnostics(diagnostics)
+{
+    if (mOptions.validateSingleParent)
+    {
+        mParent[root] = nullptr;
+    }
+}
+
+void ValidateAST::visitNode(Visit visit, TIntermNode *node)
+{
+    if (visit == PreVisit && mOptions.validateSingleParent)
+    {
+        size_t childCount = node->getChildCount();
+        for (size_t i = 0; i < childCount; ++i)
+        {
+            TIntermNode *child = node->getChildNode(i);
+            if (mParent.find(child) != mParent.end())
+            {
+                // If child is visited twice but through the same parent, the problem is in one of
+                // the ancestors.
+                if (mParent[child] != node)
+                {
+                    mDiagnostics->error(node->getLine(), "Found child with two parents",
+                                        "<validateSingleParent>");
+                    mSingleParentFailed = true;
+                }
+            }
+
+            mParent[child] = node;
+        }
+    }
+}
+
+void ValidateAST::expectNonNullChildren(Visit visit, TIntermNode *node, size_t least_count)
+{
+    if (visit == PreVisit && mOptions.validateNullNodes)
+    {
+        size_t childCount = node->getChildCount();
+        if (childCount < least_count)
+        {
+            mDiagnostics->error(node->getLine(), "Too few children", "<validateNullNodes>");
+            mNullNodesFailed = true;
+        }
+
+        for (size_t i = 0; i < childCount; ++i)
+        {
+            if (node->getChildNode(i) == nullptr)
+            {
+                mDiagnostics->error(node->getLine(), "Found nullptr child", "<validateNullNodes>");
+                mNullNodesFailed = true;
+            }
+        }
+    }
+}
+
+void ValidateAST::visitSymbol(TIntermSymbol *node)
+{
+    visitNode(PreVisit, node);
+}
+
+void ValidateAST::visitConstantUnion(TIntermConstantUnion *node)
+{
+    visitNode(PreVisit, node);
+}
+
+bool ValidateAST::visitSwizzle(Visit visit, TIntermSwizzle *node)
+{
+    visitNode(visit, node);
+    return true;
+}
+
+bool ValidateAST::visitBinary(Visit visit, TIntermBinary *node)
+{
+    visitNode(visit, node);
+    return true;
+}
+
+bool ValidateAST::visitUnary(Visit visit, TIntermUnary *node)
+{
+    visitNode(visit, node);
+    return true;
+}
+
+bool ValidateAST::visitTernary(Visit visit, TIntermTernary *node)
+{
+    visitNode(visit, node);
+    return true;
+}
+
+bool ValidateAST::visitIfElse(Visit visit, TIntermIfElse *node)
+{
+    visitNode(visit, node);
+    return true;
+}
+
+bool ValidateAST::visitSwitch(Visit visit, TIntermSwitch *node)
+{
+    visitNode(visit, node);
+    return true;
+}
+
+bool ValidateAST::visitCase(Visit visit, TIntermCase *node)
+{
+    visitNode(visit, node);
+    return true;
+}
+
+void ValidateAST::visitFunctionPrototype(TIntermFunctionPrototype *node)
+{
+    visitNode(PreVisit, node);
+}
+
+bool ValidateAST::visitFunctionDefinition(Visit visit, TIntermFunctionDefinition *node)
+{
+    visitNode(visit, node);
+    return true;
+}
+
+bool ValidateAST::visitAggregate(Visit visit, TIntermAggregate *node)
+{
+    visitNode(visit, node);
+    expectNonNullChildren(visit, node, 0);
+    return true;
+}
+
+bool ValidateAST::visitBlock(Visit visit, TIntermBlock *node)
+{
+    visitNode(visit, node);
+    expectNonNullChildren(visit, node, 0);
+    return true;
+}
+
+bool ValidateAST::visitInvariantDeclaration(Visit visit, TIntermInvariantDeclaration *node)
+{
+    visitNode(visit, node);
+    return true;
+}
+
+bool ValidateAST::visitDeclaration(Visit visit, TIntermDeclaration *node)
+{
+    visitNode(visit, node);
+    expectNonNullChildren(visit, node, 0);
+    return true;
+}
+
+bool ValidateAST::visitLoop(Visit visit, TIntermLoop *node)
+{
+    visitNode(visit, node);
+    return true;
+}
+
+bool ValidateAST::visitBranch(Visit visit, TIntermBranch *node)
+{
+    visitNode(visit, node);
+    return true;
+}
+
+void ValidateAST::visitPreprocessorDirective(TIntermPreprocessorDirective *node)
+{
+    visitNode(PreVisit, node);
+}
+
+bool ValidateAST::validateInternal()
+{
+    return !mSingleParentFailed && !mNullNodesFailed;
+}
+
+}  // anonymous namespace
+
+bool ValidateAST(TIntermNode *root, TDiagnostics *diagnostics, const ValidateASTOptions &options)
+{
+    return ValidateAST::validate(root, diagnostics, options);
+}
+
+}  // namespace sh

src/compiler/translator/ValidateAST.h

+//
+// Copyright 2019 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+
+#ifndef COMPILER_TRANSLATOR_VALIDATEAST_H_
+#define COMPILER_TRANSLATOR_VALIDATEAST_H_
+
+#include "compiler/translator/BaseTypes.h"
+#include "compiler/translator/Common.h"
+
+namespace sh
+{
+class TDiagnostics;
+class TIntermNode;
+
+// The following options (stored in Compiler) tell the validator what to validate.  Some validations
+// are conditional to certain passes.
+struct ValidateASTOptions
+{
+    // TODO: add support for the flags marked with TODO. http://anglebug.com/2733
+
+    // Check that every node always has only one parent,
+    bool validateSingleParent = true;
+    // Check that all EOpCallFunctionInAST have their corresponding function definitions in the AST,
+    // with matching symbol ids. There should also be at least a prototype declaration before the
+    // function is called.
+    bool validateFunctionCall = true;  // TODO
+    // Check that there are no null nodes where they are not allowed, for example as children of
+    // TIntermDeclaration or TIntermBlock.
+    bool validateNullNodes = true;
+    // Check that symbols that reference variables have consistent qualifiers and symbol ids with
+    // the variable declaration. For example, references to function out parameters should be
+    // EvqOut.
+    bool validateQualifiers = true;  // TODO
+    // Check that variable declarations that can't have initializers don't have initializers
+    // (varyings, uniforms for example).
+    bool validateInitializers = true;  // TODO
+    // Check that there is only one TFunction with each function name referenced in the nodes (no
+    // two TFunctions with the same name, taking internal/non-internal namespaces into account).
+    bool validateUniqueFunctions = true;  // TODO
+    // Check that references to user-defined structs are matched with the corresponding struct
+    // declaration.
+    bool validateStructUsage = true;  // TODO
+    // Check that expression nodes have the correct type considering their operand(s).
+    bool validateExpressionTypes = true;  // TODO
+    // If SeparateDeclarations has been run, check for the absence of multi declarations as well.
+    bool validateMultiDeclarations = false;  // TODO
+};
+
+// Check for errors and output error messages on the context.
+// Returns true if there are no errors.
+bool ValidateAST(TIntermNode *root, TDiagnostics *diagnostics, const ValidateASTOptions &options);
+
+}  // namespace sh
+
+#endif  // COMPILER_TRANSLATOR_VALIDATESWITCH_H_