Don't dynamically recurse when analyzing functions
bug: 38257724
With this CL, dEQP-GLES3.functional.shaders.functions.invalid no longer results
in a stack overflow and system crash.
The fix is to explicitly loop over switch cases in the function call
depth analysis.
Later on, we probably want to change TIntermSwitch traversal to loop
over all cases of the switch statement as well, but that requires
a lot of changes; all traversers need to be changed to not have to loop over
switch statements themselves, which will break expectations and
perhaps functionality that critically depends on seeing/controlling iteration
over switch statement cases.
Not tested for regressions in dEQP-GLES2.functional.shaders.*
Test: dEQP-GLES3.functional.shaders.functions.invalid.dynamic_switch_recursion_vertex:
system crash, hang, or fail -> Pass
Change-Id: I5d13a5f3296579c8818975e103f5ed6e03a47b68
Reviewed-on: https://swiftshader-review.googlesource.com/9789
Reviewed-by: Lingfeng Yang <lfy@google.com>
Reviewed-by: Nicolas Capens <capn@google.com>
Tested-by: Lingfeng Yang <lfy@google.com>
diff --git a/src/OpenGL/compiler/AnalyzeCallDepth.cpp b/src/OpenGL/compiler/AnalyzeCallDepth.cpp
index 60991af..48f0017 100644
--- a/src/OpenGL/compiler/AnalyzeCallDepth.cpp
+++ b/src/OpenGL/compiler/AnalyzeCallDepth.cpp
@@ -14,6 +14,23 @@
#include "AnalyzeCallDepth.h"
+static TIntermSequence::iterator
+traverseCaseBody(AnalyzeCallDepth* analysis,
+ TIntermSequence::iterator& start,
+ const TIntermSequence::iterator& end) {
+ TIntermSequence::iterator current = start;
+ for (++current; current != end; ++current)
+ {
+ (*current)->traverse(analysis);
+ if((*current)->getAsBranchNode()) // Kill, Break, Continue or Return
+ {
+ break;
+ }
+ }
+ return current;
+}
+
+
AnalyzeCallDepth::FunctionNode::FunctionNode(TIntermAggregate *node) : node(node)
{
visit = PreVisit;
@@ -101,6 +118,50 @@
}
}
+bool AnalyzeCallDepth::visitSwitch(Visit visit, TIntermSwitch *node)
+{
+ TIntermTyped* switchValue = node->getInit();
+ TIntermAggregate* opList = node->getStatementList();
+
+ if(!switchValue || !opList)
+ {
+ return false;
+ }
+
+ // TODO: We need to dig into switch statement cases from
+ // visitSwitch for all traversers. Is there a way to
+ // preserve existing functionality while moving the iteration
+ // to the general traverser?
+ TIntermSequence& sequence = opList->getSequence();
+ TIntermSequence::iterator it = sequence.begin();
+ TIntermSequence::iterator defaultIt = sequence.end();
+ for(; it != sequence.end(); ++it)
+ {
+ TIntermCase* currentCase = (*it)->getAsCaseNode();
+ if(currentCase)
+ {
+ TIntermSequence::iterator caseIt = it;
+ TIntermTyped* condition = currentCase->getCondition();
+ if(condition) // non default case
+ {
+ condition->traverse(this);
+ traverseCaseBody(this, caseIt, sequence.end());
+ }
+ else
+ {
+ defaultIt = it; // The default case might not be the last case, keep it for last
+ }
+ }
+ }
+
+ // If there's a default case, traverse it here
+ if(defaultIt != sequence.end())
+ {
+ traverseCaseBody(this, defaultIt, sequence.end());
+ }
+ return false;
+}
+
bool AnalyzeCallDepth::visitAggregate(Visit visit, TIntermAggregate *node)
{
switch(node->getOp())
diff --git a/src/OpenGL/compiler/AnalyzeCallDepth.h b/src/OpenGL/compiler/AnalyzeCallDepth.h
index b2ca07a..76d6522 100644
--- a/src/OpenGL/compiler/AnalyzeCallDepth.h
+++ b/src/OpenGL/compiler/AnalyzeCallDepth.h
@@ -27,6 +27,7 @@
AnalyzeCallDepth(TIntermNode *root);
~AnalyzeCallDepth();
+ virtual bool visitSwitch(Visit, TIntermSwitch*);
virtual bool visitAggregate(Visit, TIntermAggregate*);
unsigned int analyzeCallDepth();
