Linker: Walk the call graph to report an error on missing bodies.
authorJohn Kessenich <cepheus@frii.com>
Fri, 9 Dec 2016 04:01:59 +0000 (21:01 -0700)
committerJohn Kessenich <cepheus@frii.com>
Fri, 9 Dec 2016 06:22:21 +0000 (23:22 -0700)
SPIRV/GlslangToSpv.cpp
Test/baseResults/100scope.vert.out
Test/baseResults/110scope.vert.out
Test/baseResults/300scope.vert.out
Test/baseResults/430scope.vert.out
Test/baseResults/missingBodies.vert.out [new file with mode: 0755]
Test/missingBodies.vert [new file with mode: 0644]
glslang/Include/revision.h
glslang/MachineIndependent/linkValidate.cpp
glslang/MachineIndependent/localintermediate.h
gtests/Link.FromFile.cpp

index 7abd04d..15af947 100755 (executable)
@@ -1352,7 +1352,7 @@ bool TGlslangToSpvTraverser::visitAggregate(glslang::TVisit visit, glslang::TInt
             // anything else gets there, so visit out of order, doing them all now.
             makeGlobalInitializers(node->getAsAggregate()->getSequence());
 
-            // Initializers are done, don't want to visit again, but functions link objects need to be processed,
+            // Initializers are done, don't want to visit again, but functions and link objects need to be processed,
             // so do them manually.
             visitFunctions(node->getAsAggregate()->getSequence());
 
@@ -2634,7 +2634,7 @@ void TGlslangToSpvTraverser::visitFunctions(const glslang::TIntermSequence& glsl
 {
     for (int f = 0; f < (int)glslFunctions.size(); ++f) {
         glslang::TIntermAggregate* node = glslFunctions[f]->getAsAggregate();
-        if (node && (node->getOp() == glslang::EOpFunction || node->getOp() == glslang ::EOpLinkerObjects))
+        if (node && (node->getOp() == glslang::EOpFunction || node->getOp() == glslang::EOpLinkerObjects))
             node->traverse(this);
     }
 }
index 2b542b0..44132bd 100644 (file)
@@ -128,6 +128,8 @@ ERROR: node is still EOpNull!
 
 Linked vertex stage:
 
+ERROR: Linking vertex stage: No function definition (body) found: 
+    g(
 
 Shader version: 100
 ERROR: node is still EOpNull!
index dd53250..e71bba3 100644 (file)
@@ -130,6 +130,10 @@ ERROR: node is still EOpNull!
 
 Linked vertex stage:
 
+ERROR: Linking vertex stage: No function definition (body) found: 
+    sin(f1;
+ERROR: Linking vertex stage: No function definition (body) found: 
+    g(
 
 Shader version: 110
 ERROR: node is still EOpNull!
index 2a9a945..d2428e0 100644 (file)
@@ -131,6 +131,8 @@ ERROR: node is still EOpNull!
 
 Linked vertex stage:
 
+ERROR: Linking vertex stage: No function definition (body) found: 
+    g(
 
 Shader version: 300
 ERROR: node is still EOpNull!
index 0a097a2..f1b9594 100644 (file)
@@ -127,6 +127,8 @@ ERROR: node is still EOpNull!
 
 Linked vertex stage:
 
+ERROR: Linking vertex stage: No function definition (body) found: 
+    g(
 
 Shader version: 430
 ERROR: node is still EOpNull!
diff --git a/Test/baseResults/missingBodies.vert.out b/Test/baseResults/missingBodies.vert.out
new file mode 100755 (executable)
index 0000000..7cf4734
--- /dev/null
@@ -0,0 +1,117 @@
+missingBodies.vert
+Warning, version 450 is not yet complete; most version-specific features are present, but some are missing.
+
+Shader version: 450
+0:? Sequence
+0:4  Function Definition: foo( (global void)
+0:4    Function Parameters: 
+0:4    Sequence
+0:4      Function Call: bar( (global void)
+0:8  Function Definition: C(i1;i1; (global void)
+0:8    Function Parameters: 
+0:8      '' (in int)
+0:8      '' (in int)
+0:10  Function Definition: A( (global void)
+0:10    Function Parameters: 
+0:10    Sequence
+0:10      Function Call: B( (global void)
+0:10      Function Call: C(i1; (global void)
+0:10        Constant:
+0:10          1 (const int)
+0:10      Function Call: C(b1; (global void)
+0:10        Constant:
+0:10          true (const bool)
+0:10      Function Call: C(i1;i1; (global void)
+0:10        Constant:
+0:10          1 (const int)
+0:10        Constant:
+0:10          2 (const int)
+0:12  Function Definition: main( (global void)
+0:12    Function Parameters: 
+0:14    Sequence
+0:14      Function Call: foo( (global void)
+0:15      Function Call: C(b1; (global void)
+0:15        Constant:
+0:15          true (const bool)
+0:20  Sequence
+0:20    move second child to first child (temp int)
+0:20      'f1' (global int)
+0:20      Function Call: ret1( (global int)
+0:22  Function Definition: ret2( (global int)
+0:22    Function Parameters: 
+0:22    Sequence
+0:22      Branch: Return with expression
+0:22        Constant:
+0:22          3 (const int)
+0:24  Sequence
+0:24    move second child to first child (temp int)
+0:24      'f2' (global int)
+0:24      Function Call: ret2( (global int)
+0:?   Linker Objects
+0:?     'f1' (global int)
+0:?     'f2' (global int)
+0:?     'gl_VertexID' (gl_VertexId int VertexId)
+0:?     'gl_InstanceID' (gl_InstanceId int InstanceId)
+
+
+Linked vertex stage:
+
+ERROR: Linking vertex stage: No function definition (body) found: 
+    ret1(
+ERROR: Linking vertex stage: No function definition (body) found: 
+    C(b1;
+ERROR: Linking vertex stage: No function definition (body) found: 
+    bar(
+
+Shader version: 450
+0:? Sequence
+0:4  Function Definition: foo( (global void)
+0:4    Function Parameters: 
+0:4    Sequence
+0:4      Function Call: bar( (global void)
+0:8  Function Definition: C(i1;i1; (global void)
+0:8    Function Parameters: 
+0:8      '' (in int)
+0:8      '' (in int)
+0:10  Function Definition: A( (global void)
+0:10    Function Parameters: 
+0:10    Sequence
+0:10      Function Call: B( (global void)
+0:10      Function Call: C(i1; (global void)
+0:10        Constant:
+0:10          1 (const int)
+0:10      Function Call: C(b1; (global void)
+0:10        Constant:
+0:10          true (const bool)
+0:10      Function Call: C(i1;i1; (global void)
+0:10        Constant:
+0:10          1 (const int)
+0:10        Constant:
+0:10          2 (const int)
+0:12  Function Definition: main( (global void)
+0:12    Function Parameters: 
+0:14    Sequence
+0:14      Function Call: foo( (global void)
+0:15      Function Call: C(b1; (global void)
+0:15        Constant:
+0:15          true (const bool)
+0:20  Sequence
+0:20    move second child to first child (temp int)
+0:20      'f1' (global int)
+0:20      Function Call: ret1( (global int)
+0:22  Function Definition: ret2( (global int)
+0:22    Function Parameters: 
+0:22    Sequence
+0:22      Branch: Return with expression
+0:22        Constant:
+0:22          3 (const int)
+0:24  Sequence
+0:24    move second child to first child (temp int)
+0:24      'f2' (global int)
+0:24      Function Call: ret2( (global int)
+0:?   Linker Objects
+0:?     'f1' (global int)
+0:?     'f2' (global int)
+0:?     'gl_VertexID' (gl_VertexId int VertexId)
+0:?     'gl_InstanceID' (gl_InstanceId int InstanceId)
+
diff --git a/Test/missingBodies.vert b/Test/missingBodies.vert
new file mode 100644 (file)
index 0000000..04b7181
--- /dev/null
@@ -0,0 +1,24 @@
+#version 450\r
+\r
+void bar();\r
+void foo() { bar(); }\r
+\r
+void B();\r
+void C(int);\r
+void C(int, int) { }\r
+void C(bool);\r
+void A() { B(); C(1); C(true); C(1, 2); }\r
+\r
+void main()\r
+{\r
+    foo();\r
+    C(true);\r
+}\r
+\r
+int ret1();\r
+\r
+int f1 = ret1();\r
+\r
+int ret2() { return 3; }\r
+\r
+int f2 = ret2();\r
index c53a14d..62856cc 100644 (file)
@@ -2,5 +2,5 @@
 // For the version, it uses the latest git tag followed by the number of commits.
 // For the date, it uses the current date (when then script is run).
 
-#define GLSLANG_REVISION "Overload400-PrecQual.1676"
-#define GLSLANG_DATE "05-Dec-2016"
+#define GLSLANG_REVISION "Overload400-PrecQual.1686"
+#define GLSLANG_DATE "08-Dec-2016"
index 050fdae..8e14bbb 100644 (file)
@@ -94,7 +94,7 @@ void TIntermediate::merge(TInfoSink& infoSink, TIntermediate& unit)
     callGraph.insert(callGraph.end(), unit.callGraph.begin(), unit.callGraph.end());
 
     if (originUpperLeft != unit.originUpperLeft || pixelCenterInteger != unit.pixelCenterInteger)
-        error(infoSink, "gl_FragCoord redeclarations must match across shaders\n");
+        error(infoSink, "gl_FragCoord redeclarations must match across shaders");
 
     if (! earlyFragmentTests)
         earlyFragmentTests = unit.earlyFragmentTests;
@@ -390,8 +390,9 @@ void TIntermediate::finalCheck(TInfoSink& infoSink)
     if (numPushConstants > 1)
         error(infoSink, "Only one push_constant block is allowed per stage");
 
-    // recursion checking
+    // recursion and missing body checking
     checkCallGraphCycles(infoSink);
+    checkCallGraphBodies(infoSink);
 
     // overlap/alias/missing I/O, etc.
     inOutLocationCheck(infoSink);
@@ -502,7 +503,7 @@ void TIntermediate::finalCheck(TInfoSink& infoSink)
 //
 void TIntermediate::checkCallGraphCycles(TInfoSink& infoSink)
 {
-    // Reset everything, once.
+    // Clear fields we'll use for this.
     for (TGraph::iterator call = callGraph.begin(); call != callGraph.end(); ++call) {
         call->visited = false;
         call->currentPath = false;
@@ -578,6 +579,69 @@ void TIntermediate::checkCallGraphCycles(TInfoSink& infoSink)
 }
 
 //
+// See which functions are reachable from the entry point and which have bodies.
+// Reachable ones with missing bodies are errors.
+//
+void TIntermediate::checkCallGraphBodies(TInfoSink& infoSink)
+{
+    // Clear fields we'll use for this.
+    for (TGraph::iterator call = callGraph.begin(); call != callGraph.end(); ++call) {
+        call->visited = false;
+        call->calleeBodyPosition = -1;
+    }
+    
+    // The top level of the AST includes function definitions (bodies).
+    // Compare these to function calls in the call graph.
+    // We'll end up knowing which have bodies, and if so,
+    // how to map the call-graph node to the location in the AST.
+    TIntermSequence &functionSequence = getTreeRoot()->getAsAggregate()->getSequence();
+    for (int f = 0; f < (int)functionSequence.size(); ++f) {
+        glslang::TIntermAggregate* node = functionSequence[f]->getAsAggregate();
+        if (node && (node->getOp() == glslang::EOpFunction)) {
+            for (TGraph::iterator call = callGraph.begin(); call != callGraph.end(); ++call) {
+                if (call->callee == node->getName())
+                    call->calleeBodyPosition = f;
+            }
+        }
+    }
+
+    // Start call-graph traversal by visiting the entry point nodes.
+    for (TGraph::iterator call = callGraph.begin(); call != callGraph.end(); ++call) {
+        if (call->caller.compare(getEntryPointMangledName().c_str()) == 0)
+            call->visited = true;
+    }
+
+    // Propagate 'visited' through the call-graph to every part of the graph it
+    // can reach (seeded with the entry-point setting above).
+    bool changed;
+    do {
+        changed = false;
+        for (auto call1 = callGraph.begin(); call1 != callGraph.end(); ++call1) {
+            if (call1->visited) {
+                for (TGraph::iterator call2 = callGraph.begin(); call2 != callGraph.end(); ++call2) {
+                    if (! call2->visited) {
+                        if (call1->callee == call2->caller) {
+                            changed = true;
+                            call2->visited = true;
+                        }
+                    }
+                }
+            }
+        }
+    } while (changed);
+
+    // Any call-graph node set to visited but without a callee body is an error.
+    for (TGraph::iterator call = callGraph.begin(); call != callGraph.end(); ++call) {
+        if (call->visited) {
+            if (call->calleeBodyPosition == -1) {
+                error(infoSink, "No function definition (body) found: ");
+                infoSink.info << "    " << call->callee << "\n";
+            }
+        }
+    }
+}
+
+//
 // Satisfy rules for location qualifiers on inputs and outputs
 //
 void TIntermediate::inOutLocationCheck(TInfoSink& infoSink)
index 028de06..a16383f 100644 (file)
@@ -67,7 +67,9 @@ struct TVectorFields {
 // by TIntermediate.
 //
 
-// Used for detecting recursion:  A "call" is a pair: <caller, callee>.
+// Used for call-graph algorithms for detecting recursion, missing bodies, and dead bodies.
+// A "call" is a pair: <caller, callee>.
+// There can be duplicates. General assumption is the list is small.
 struct TCall {
     TCall(const TString& pCaller, const TString& pCallee) : caller(pCaller), callee(pCallee) { }
     TString caller;
@@ -75,6 +77,7 @@ struct TCall {
     bool visited;
     bool currentPath;
     bool errorGiven;
+    int calleeBodyPosition;
 };
 
 // A generic 1-D range.
@@ -393,6 +396,7 @@ protected:
     void mergeImplicitArraySizes(TType&, const TType&);
     void mergeErrorCheck(TInfoSink&, const TIntermSymbol&, const TIntermSymbol&, bool crossStage);
     void checkCallGraphCycles(TInfoSink&);
+    void checkCallGraphBodies(TInfoSink&);
     void inOutLocationCheck(TInfoSink&);
     TIntermSequence& findLinkerObjects() const;
     bool userOutputUsed() const;
index 2651a1e..ab845bf 100644 (file)
@@ -99,6 +99,7 @@ INSTANTIATE_TEST_CASE_P(
         {"150.tesc", "150.tese", "400.tesc", "400.tese", "410.tesc", "420.tesc", "420.tese"},
         {"max_vertices_0.geom"},
         {"es-link1.frag", "es-link2.frag"},
+        {"missingBodies.vert"}
     })),
 );
 // clang-format on