Commit 1fb04acc by Lingfeng Yang

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/9789Reviewed-by: 's avatarLingfeng Yang <lfy@google.com> Reviewed-by: 's avatarNicolas Capens <capn@google.com> Tested-by: 's avatarLingfeng Yang <lfy@google.com>
parent 73981b86
...@@ -14,6 +14,23 @@ ...@@ -14,6 +14,23 @@
#include "AnalyzeCallDepth.h" #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) AnalyzeCallDepth::FunctionNode::FunctionNode(TIntermAggregate *node) : node(node)
{ {
visit = PreVisit; visit = PreVisit;
...@@ -101,6 +118,50 @@ AnalyzeCallDepth::~AnalyzeCallDepth() ...@@ -101,6 +118,50 @@ AnalyzeCallDepth::~AnalyzeCallDepth()
} }
} }
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) bool AnalyzeCallDepth::visitAggregate(Visit visit, TIntermAggregate *node)
{ {
switch(node->getOp()) switch(node->getOp())
......
...@@ -27,6 +27,7 @@ public: ...@@ -27,6 +27,7 @@ public:
AnalyzeCallDepth(TIntermNode *root); AnalyzeCallDepth(TIntermNode *root);
~AnalyzeCallDepth(); ~AnalyzeCallDepth();
virtual bool visitSwitch(Visit, TIntermSwitch*);
virtual bool visitAggregate(Visit, TIntermAggregate*); virtual bool visitAggregate(Visit, TIntermAggregate*);
unsigned int analyzeCallDepth(); unsigned int analyzeCallDepth();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment