Add function recursion testing to the link-time validation.

Test/baseResults/recurse1.vert.out

+recurse1.vert
+
+0:? Sequence
+0:3  Function Definition: main( (void)
+0:3    Function Parameters: 
+0:9  Function Definition: self( (void)
+0:9    Function Parameters: 
+0:11    Sequence
+0:11      Function Call: self( (void)
+0:16  Function Definition: foo(f1; (void)
+0:16    Function Parameters: 
+0:16      '' (in float)
+0:18    Sequence
+0:18      Function Call: bar(i1; (float)
+0:18        Constant:
+0:18          2 (const int)
+0:21  Function Definition: bar(i1; (float)
+0:21    Function Parameters: 
+0:21      '' (in int)
+0:23    Sequence
+0:23      Function Call: foo(f1; (void)
+0:23        Constant:
+0:23          4.200000
+0:25      Branch: Return with expression
+0:25        Constant:
+0:25          3.200000
+0:32  Function Definition: A( (void)
+0:32    Function Parameters: 
+0:32    Sequence
+0:32      Function Call: B( (void)
+0:33  Function Definition: C( (void)
+0:33    Function Parameters: 
+0:33    Sequence
+0:33      Function Call: D( (void)
+0:34  Function Definition: B( (void)
+0:34    Function Parameters: 
+0:34    Sequence
+0:34      Function Call: C( (void)
+0:35  Function Definition: D( (void)
+0:35    Function Parameters: 
+0:35    Sequence
+0:35      Function Call: A( (void)
+0:41  Function Definition: AT( (void)
+0:41    Function Parameters: 
+0:41    Sequence
+0:41      Function Call: BT( (void)
+0:41      Function Call: BT( (void)
+0:41      Function Call: BT( (void)
+0:42  Function Definition: CT( (void)
+0:42    Function Parameters: 
+0:42    Sequence
+0:42      Function Call: DT( (void)
+0:42      Function Call: AT( (void)
+0:42      Function Call: DT( (void)
+0:42      Function Call: BT( (void)
+0:43  Function Definition: BT( (void)
+0:43    Function Parameters: 
+0:43    Sequence
+0:43      Function Call: CT( (void)
+0:43      Function Call: CT( (void)
+0:43      Function Call: CT( (void)
+0:44  Function Definition: DT( (void)
+0:44    Function Parameters: 
+0:44    Sequence
+0:44      Function Call: AT( (void)
+0:?   Linker Objects
+0:?     'gl_VertexID' (gl_VertexId int)
+0:?     'gl_InstanceID' (gl_InstanceId int)
+
+recurse1.frag
+
+0:? Sequence
+0:5  Function Definition: main( (void)
+0:5    Function Parameters: 
+0:11  Function Definition: cfoo(f1; (void)
+0:11    Function Parameters: 
+0:11      '' (in float)
+0:13    Sequence
+0:13      Function Call: cbar(i1; (float)
+0:13        Constant:
+0:13          2 (const int)
+0:20  Function Definition: CA( (void)
+0:20    Function Parameters: 
+0:20    Sequence
+0:20      Function Call: CB( (void)
+0:21  Function Definition: CC( (void)
+0:21    Function Parameters: 
+0:21    Sequence
+0:21      Function Call: CD( (void)
+0:27  Function Definition: CAT( (void)
+0:27    Function Parameters: 
+0:27    Sequence
+0:27      Function Call: CBT( (void)
+0:27      Function Call: CBT( (void)
+0:27      Function Call: CBT( (void)
+0:28  Function Definition: CCT( (void)
+0:28    Function Parameters: 
+0:28    Sequence
+0:28      Function Call: CDT( (void)
+0:28      Function Call: CDT( (void)
+0:28      Function Call: CBT( (void)
+0:?   Linker Objects
+
+recurse2.frag
+
+0:? Sequence
+0:9  Function Definition: cbar(i1; (float)
+0:9    Function Parameters: 
+0:9      '' (in int)
+0:11    Sequence
+0:11      Function Call: cfoo(f1; (void)
+0:11        Constant:
+0:11          4.200000
+0:13      Branch: Return with expression
+0:13        Constant:
+0:13          3.200000
+0:20  Function Definition: CB( (void)
+0:20    Function Parameters: 
+0:20    Sequence
+0:20      Function Call: CC( (void)
+0:21  Function Definition: CD( (void)
+0:21    Function Parameters: 
+0:21    Sequence
+0:21      Function Call: CA( (void)
+0:27  Function Definition: CBT( (void)
+0:27    Function Parameters: 
+0:27    Sequence
+0:27      Function Call: CCT( (void)
+0:27      Function Call: CCT( (void)
+0:27      Function Call: CCT( (void)
+0:28  Function Definition: CDT( (void)
+0:28    Function Parameters: 
+0:28    Sequence
+0:28      Function Call: CAT( (void)
+0:?   Linker Objects
+
+
+Linked vertex stage:
+
+ERROR: Linking vertex stage: Recursion detected:
+    DT( calling AT(
+ERROR: Linking vertex stage: Recursion detected:
+    AT( calling BT(
+ERROR: Linking vertex stage: Recursion detected:
+    BT( calling CT(
+ERROR: Linking vertex stage: Recursion detected:
+    D( calling A(
+ERROR: Linking vertex stage: Recursion detected:
+    bar(i1; calling foo(f1;
+ERROR: Linking vertex stage: Recursion detected:
+    self( calling self(
+
+Linked fragment stage:
+
+ERROR: Linking fragment stage: Recursion detected:
+    CCT( calling CBT(
+ERROR: Linking fragment stage: Recursion detected:
+    CBT( calling CCT(
+ERROR: Linking fragment stage: Recursion detected:
+    CC( calling CD(
+ERROR: Linking fragment stage: Recursion detected:
+    cfoo(f1; calling cbar(i1;
+
+0:? Sequence
+0:5  Function Definition: main( (void)
+0:5    Function Parameters: 
+0:11  Function Definition: cfoo(f1; (void)
+0:11    Function Parameters: 
+0:11      '' (in float)
+0:13    Sequence
+0:13      Function Call: cbar(i1; (float)
+0:13        Constant:
+0:13          2 (const int)
+0:20  Function Definition: CA( (void)
+0:20    Function Parameters: 
+0:20    Sequence
+0:20      Function Call: CB( (void)
+0:21  Function Definition: CC( (void)
+0:21    Function Parameters: 
+0:21    Sequence
+0:21      Function Call: CD( (void)
+0:27  Function Definition: CAT( (void)
+0:27    Function Parameters: 
+0:27    Sequence
+0:27      Function Call: CBT( (void)
+0:27      Function Call: CBT( (void)
+0:27      Function Call: CBT( (void)
+0:28  Function Definition: CCT( (void)
+0:28    Function Parameters: 
+0:28    Sequence
+0:28      Function Call: CDT( (void)
+0:28      Function Call: CDT( (void)
+0:28      Function Call: CBT( (void)
+0:9  Function Definition: cbar(i1; (float)
+0:9    Function Parameters: 
+0:9      '' (in int)
+0:11    Sequence
+0:11      Function Call: cfoo(f1; (void)
+0:11        Constant:
+0:11          4.200000
+0:13      Branch: Return with expression
+0:13        Constant:
+0:13          3.200000
+0:20  Function Definition: CB( (void)
+0:20    Function Parameters: 
+0:20    Sequence
+0:20      Function Call: CC( (void)
+0:21  Function Definition: CD( (void)
+0:21    Function Parameters: 
+0:21    Sequence
+0:21      Function Call: CA( (void)
+0:27  Function Definition: CBT( (void)
+0:27    Function Parameters: 
+0:27    Sequence
+0:27      Function Call: CCT( (void)
+0:27      Function Call: CCT( (void)
+0:27      Function Call: CCT( (void)
+0:28  Function Definition: CDT( (void)
+0:28    Function Parameters: 
+0:28    Sequence
+0:28      Function Call: CAT( (void)
+0:?   Linker Objects
+

Test/recurse1.frag

+#version 330 core
+
+// cross-unit recursion
+
+void main() {}
+
+// two-level recursion
+
+float cbar(int);
+
+void cfoo(float)
+{
+	cbar(2);
+}
+
+// four-level, out of order
+
+void CB();
+void CD();
+void CA() { CB(); }
+void CC() { CD(); }
+
+// high degree
+
+void CBT();
+void CDT();
+void CAT() { CBT(); CBT(); CBT(); }
+void CCT() { CDT(); CDT(); CBT(); }

Test/recurse1.vert

+#version 330 core
+
+void main() {}
+
+float bar(int);
+
+// direct recursion
+
+void self()
+{
+    self();
+}
+
+// two-level recursion
+
+void foo(float)
+{
+	bar(2);
+}
+
+float bar(int)
+{
+	foo(4.2);
+
+	return 3.2;
+}
+
+// four-level, out of order
+
+void B();
+void D();
+void A() { B(); }
+void C() { D(); }
+void B() { C(); }
+void D() { A(); }
+
+// high degree
+
+void BT();
+void DT();
+void AT() { BT(); BT(); BT(); }
+void CT() { DT(); AT(); DT(); BT(); }
+void BT() { CT(); CT(); CT(); }
+void DT() { AT(); }

Test/recurse2.frag

+#version 330 core
+
+// cross-unit recursion
+
+// two-level recursion
+
+void cfoo(float);
+
+float cbar(int)
+{
+	cfoo(4.2);
+
+	return 3.2;
+}
+
+// four-level, out of order
+
+void CA();
+void CC();
+void CB() { CC(); }
+void CD() { CA(); }
+
+// high degree
+
+void CAT();
+void CCT();
+void CBT() { CCT(); CCT(); CCT(); }
+void CDT() { CAT(); }

Test/runtests

@@ -35,6 +35,7 @@ function runLinkTest {
 runLinkTest mains1.frag mains2.frag noMain1.geom noMain2.geom
 runLinkTest noMain.vert mains.frag
 runLinkTest link1.frag link2.frag link3.frag
+runLinkTest recurse1.vert recurse1.frag recurse2.frag
 
 #
 # multi-threaded test

Todo.txt

@@ -20,27 +20,27 @@ Link Validation
 	  - 4.4: A stage contains two different blocks, each with no instance name, where the blocks contain a member with the same name.
     Intra-stage linking
       + exactly one main
-      + type consistency check of uniforms, globals, ins, and outs
-      - value checking of global const initializers
-      - value checking of uniform initializers
-      + location match
-      - component/binding/index/offset match check
-      - location/component aliasing (except desktop vertex shader inputs)
-      - location layout range/overlap semantics
-      - geometry shader input array sizes and input layout qualifier declaration
-      - compute shader layout(local_size_*) matching
-      + mixed es/non-es profiles
-      - recursion for both functions and subroutines
-        - Even the potential for recursion through subroutine uniforms is an error.
-      - block matching
-        - matching redeclarations of interface blocks
-      - read or write to both gl_ClipVertex and gl_ClipDistance
-      - write to only one of gl_FragColor, gl_FragData, or user-declared
+      + Non ES: type consistency check of uniforms, globals, ins, and outs
+      + Non ES: value checking of global const initializers
+      + Non ES: value checking of uniform initializers
+      + Non ES: location match
+      - location aliasing/overlap (except desktop vertex shader inputs)
+      + recursion for functions
+      - Non ES: block matching
+      - Non ES: component/binding/index/offset match check
+      - Non ES: geometry shader input array sizes and input layout qualifier declaration
+      - Non ES: compute shader layout(local_size_*) matching
+      + mixed es/non-es profiles are an error
+      - Non ES: Even the potential for recursion through subroutine uniforms is an error.
+      - Non ES: matching redeclarations of interface blocks
+      - Non ES: read or write to both gl_ClipVertex and gl_ClipDistance
+      - Non ES: write to only one of gl_FragColor, gl_FragData, or user-declared
       - 4.3: Be clear that early_fragment_tests is only needed in one fragment-stage compilation unit.
       - 4.3: Be clear that implicit array sizing is only within a stage, not cross stage.
       - 4.4: overlapping transform/feedback offsets, offset/stride overflow checks, and stride matching
 	  - 4.4: If gl_FragCoord is redeclared in any fragment shader in a program, it must be redeclared in all the fragment shaders in that program that have a static use gl_FragCoord
 	  - 4.4: An interface contains two different blocks, each with no instance name, where the blocks contain a member with the same name.
+	  - 4.4: component aliasing (except desktop vertex shader inputs)
 
 Shader Functionality to Implement/Finish
     ESSL 3.0

glslang/MachineIndependent/Intermediate.cpp

@@ -900,11 +900,31 @@ void TIntermediate::addSymbolLinkageNode(TIntermAggregate*& linkage, const TVari
 }
 
 //
+// Add a caller->callee relationship to the call graph.
+// Assumes the strings are unique per signature.
+//
+void TIntermediate::addToCallGraph(TInfoSink& infoSink, const TString& caller, const TString& callee)
+{
+    // Duplicates are okay, but faster to not keep them, and they come grouped by caller,
+    // as long as new ones are push on the same end we check on for duplicates
+    for (TGraph::const_iterator call = callGraph.begin(); call != callGraph.end(); ++call) {
+        if (call->caller != caller)
+            break;
+        if (call->callee == callee)
+            return;
+    }
+
+    callGraph.push_front(TCall(caller, callee));
+}
+
+//
 // Merge the information from 'unit' into 'this'
 //
 void TIntermediate::merge(TInfoSink& infoSink, TIntermediate& unit)
 {
     numMains += unit.numMains;
+    numErrors += unit.numErrors;
+    callGraph.insert(callGraph.end(), unit.callGraph.begin(), unit.callGraph.end());
 
     if (profile != EEsProfile && unit.profile == EEsProfile ||
         profile == EEsProfile && unit.profile != EEsProfile)
@@ -990,10 +1010,89 @@ void TIntermediate::mergeLinkerObjects(TInfoSink& infoSink, TIntermSequence& lin
     }
 }
 
+//
+// Do final link-time error checking of a complete (merged) intermediate representation.
+// (Most error checking was done during merging).
+//
 void TIntermediate::errorCheck(TInfoSink& infoSink)
 {   
     if (numMains < 1)
         error(infoSink, "Missing entry point: Each stage requires one \"void main()\" entry point");
+
+    checkCallGraphCycles(infoSink);
+}
+
+//
+// See if the call graph contains any static recursion, which is disallowed
+// by the specification.
+//
+void TIntermediate::checkCallGraphCycles(TInfoSink& infoSink)
+{
+    // Reset everything, once.
+    for (TGraph::iterator call = callGraph.begin(); call != callGraph.end(); ++call) {
+        call->visited = false;
+        call->subGraph = false;
+        call->errorGiven = false;
+    }
+
+    //
+    // Loop, looking for a new connected subgraph.  One subgraph is handled per loop iteration.
+    //
+
+    TCall* newRoot;
+    do {
+        // See if we have unvisited parts of the graph.
+        newRoot = 0;
+        for (TGraph::iterator call = callGraph.begin(); call != callGraph.end(); ++call) {
+            if (! call->visited) {
+                newRoot = &(*call);
+                break;
+            }
+        }
+
+        // If not, we are done.
+        if (! newRoot)
+            break;
+
+        // Otherwise, we found a new subgraph, process it:
+        // See what all can be reached by this new root, and if any of 
+        // that is recursive.  This is done by marking processed calls as active,
+        // and if a new call is found that is already active, we looped, 
+        // thereby detecting recursion.
+        std::list<TCall*> stack;
+        stack.push_back(newRoot);
+        newRoot->subGraph = true;
+        while (! stack.empty()) {
+            // get a caller
+            TCall* call = stack.back();
+            stack.pop_back();
+
+            // Add to the stack all the callees of the last subgraph node popped from the stack.
+            // This algorithm  always terminates, because only subGraph == false causes a push
+            // and all pushes change subGraph to true, and nothing changes subGraph to false.
+            for (TGraph::iterator child = callGraph.begin(); child != callGraph.end(); ++child) {
+                if (call->callee == child->caller) {
+                    if (child->subGraph) {
+                        if (! child->errorGiven) {
+                            error(infoSink, "Recursion detected:");
+                            infoSink.info << "    " << call->callee << " calling " << child->callee << "\n";
+                            child->errorGiven = true;
+                        }
+                    } else {
+                        child->subGraph = true;
+                        stack.push_back(&(*child));
+                    }
+                }
+            }
+        }  // end while, meaning nothing left to process in this subtree
+
+        // Mark all the subGraph nodes as visited, closing out this subgraph.
+        for (TGraph::iterator call = callGraph.begin(); call != callGraph.end(); ++call) {
+            if (call->subGraph)
+                call->visited = true;
+        }
+
+    } while (newRoot);  // redundant loop check; should always exit via the 'break' above
 }
 
 void TIntermediate::error(TInfoSink& infoSink, const char* message)

glslang/MachineIndependent/ParseHelper.cpp

@@ -653,10 +653,14 @@ TIntermTyped* TParseContext::handleDotDereference(TSourceLoc loc, TIntermTyped*
 }
 
 //
-// Handle seeing a function prototype in the grammar.
+// Handle seeing a function prototype in the grammar.  This includes what may
+// become a full definition, as a full definition looks like a prototype 
+// followed by a body.  The body is handled after this function
+// returns, when present.
 //
 TIntermAggregate* TParseContext::handleFunctionPrototype(TSourceLoc loc, TFunction& function)
 {
+    currentCaller = function.getMangledName();
     TSymbol* symbol = symbolTable.find(function.getMangledName());
     TFunction* prevDec = symbol ? symbol->getAsFunction() : 0;
 
@@ -685,7 +689,7 @@ TIntermAggregate* TParseContext::handleFunctionPrototype(TSourceLoc loc, TFuncti
     functionReturnsValue = false;
 
     //
-    // Raise error message if main function takes any parameters or return anything other than void
+    // Raise error message if main function takes any parameters or returns anything other than void
     //
     if (function.getName() == "main") {
         if (function.getParamCount() > 0)
@@ -806,16 +810,18 @@ TIntermTyped* TParseContext::handleFunctionCall(TSourceLoc loc, TFunction* fnCal
                     return 0;
                 }
             } else {
-                // This is a real function call
+                // This is a function call not mapped to built-in operation
                 result = intermediate.setAggregateOperator(intermAggregate, EOpFunctionCall, fnCandidate->getReturnType(), loc);
-
-                // this is how we know whether the given function is a builtIn function or a user defined function
-                // if builtIn == false, it's a userDefined -> could be an overloaded builtIn function also
-                // if builtIn == true, it's definitely a builtIn function with EOpNull
-                if (!builtIn)
-                    result->getAsAggregate()->setUserDefined();
                 result->getAsAggregate()->setName(fnCandidate->getMangledName());
 
+                // this is how we know whether the given function is a built-in function or a user-defined function
+                // if builtIn == false, it's a userDefined -> could be an overloaded built-in function also
+                // if builtIn == true, it's definitely a built-in function with EOpNull
+                if (! builtIn) {
+                    result->getAsAggregate()->setUserDefined();
+                    intermediate.addToCallGraph(infoSink, currentCaller, fnCandidate->getMangledName());
+                }
+                
                 TStorageQualifier qual;
                 TQualifierList& qualifierList = result->getAsAggregate()->getQualifierList();
                 for (int i = 0; i < fnCandidate->getParamCount(); ++i) {

glslang/MachineIndependent/ParseHelper.h

@@ -213,6 +213,7 @@ protected:
     TQualifier globalUniformDefaults;
     TQualifier globalInputDefaults;
     TQualifier globalOutputDefaults;
+    TString currentCaller;
     // TODO: desktop functionality: track use of gl_FragDepth before redeclaration
 };
 

glslang/MachineIndependent/localintermediate.h

@@ -99,6 +99,7 @@ public:
     void addSymbolLinkageNode(TIntermAggregate*& linkage, TSymbolTable&, const TString&);
     void addSymbolLinkageNode(TIntermAggregate*& linkage, const TVariable&);
 
+    void addToCallGraph(TInfoSink&, const TString& caller, const TString& callee);
     void merge(TInfoSink&, TIntermediate&);
     void errorCheck(TInfoSink&);
 
@@ -110,6 +111,7 @@ protected:
     void mergeLinkerObjects(TInfoSink&, TIntermSequence& linkerObjects, const TIntermSequence& unitLinkerObjects);
     void error(TInfoSink& infoSink, const char*);
     void linkErrorCheck(TInfoSink&, const TIntermSymbol&, const TIntermSymbol&, bool crossStage);
+    void checkCallGraphCycles(TInfoSink&);
 
 protected:
     EShLanguage language;
@@ -119,6 +121,18 @@ protected:
     int numMains;
     int numErrors;
 
+    // for detecting recursion:  pair is <caller, callee>
+    struct TCall {
+        TCall(const TString& pCaller, const TString& pCallee) : caller(pCaller), callee(pCallee) { }
+        TString caller;
+        TString callee;
+        bool visited;
+        bool subGraph;
+        bool errorGiven;
+    };
+    typedef std::list<TCall> TGraph;
+    TGraph callGraph;
+
 private:
     void operator=(TIntermediate&); // prevent assignments
 };