AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Report recursion from within CheckProgramUnrolledSize.

Browse Source 
This allows us to remove the static-recursion analysis pass entirely,
while still providing the same results.

Change-Id: If1564cd4df55be86ca4e0bf53ecc094ba76007df
Bug: skia:12396
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445296
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This commit is contained in:
John Stiles 2021-09-02 14:33:08 -04:00 committed by SkCQ
parent 070ff2fd94
commit 98ddea09f4
5 changed files with 17 additions and 132 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

137
src/sksl/SkSLAnalysis.cpp
View File

@ -603,126 +603,6 @@ bool Analysis::CallsSampleOutsideMain(const Program& program) {
return visitor.visit(program);
}
bool Analysis::DetectStaticRecursion(SkSpan<std::unique_ptr<ProgramElement>> programElements,
ErrorReporter& errors) {
using Function = const FunctionDeclaration;
using CallSet = std::unordered_set<Function*>;
using CallGraph = std::unordered_map<Function*, CallSet>;
class CallGraphVisitor : public ProgramVisitor {
public:
CallGraphVisitor(CallGraph* calls) : fCallGraph(calls), fCurrentFunctionCalls(nullptr) {}
bool visitExpression(const Expression& e) override {
if (e.is<FunctionCall>()) {
fCurrentFunctionCalls->insert(&e.as<FunctionCall>().function());
}
return INHERITED::visitExpression(e);
}
bool visitProgramElement(const ProgramElement& p) override {
if (p.is<FunctionDefinition>()) {
Function* fn = &p.as<FunctionDefinition>().declaration();
SkASSERT(fCallGraph->count(fn) == 0);
SkASSERT(fCurrentFunctionCalls == nullptr);
CallSet currentFunctionCalls;
fCurrentFunctionCalls = &currentFunctionCalls;
INHERITED::visitProgramElement(p);
fCurrentFunctionCalls = nullptr;
fCallGraph->insert({fn, std::move(currentFunctionCalls)});
}
return false;
}
CallGraph* fCallGraph;
CallSet* fCurrentFunctionCalls;
using INHERITED = ProgramVisitor;
};
CallGraph callGraph;
CallGraphVisitor visitor{&callGraph};
for (const auto& pe : programElements) {
visitor.visitProgramElement(*pe);
}
class CycleFinder {
public:
CycleFinder(CallGraph* calls) : fCallGraph(calls) {}
bool containsCycle() {
for (const auto& [caller, callees] : *fCallGraph) {
SkASSERT(fStack.empty());
if (this->dfsHelper(caller)) {
return true;
}
}
return false;
}
const std::vector<Function*>& cycle() const { return fStack; }
private:
bool dfsHelper(Function* fn) {
SkASSERT(std::find(fStack.begin(), fStack.end(), fn) == fStack.end());
auto iter = fCallGraph->find(fn);
if (iter != fCallGraph->end()) {
fStack.push_back(fn);
for (Function* calledFn : iter->second) {
auto it = std::find(fStack.begin(), fStack.end(), calledFn);
if (it != fStack.end()) {
// Cycle detected. It includes the functions from 'it' to the end of fStack
fStack.erase(fStack.begin(), it);
return true;
}
if (this->dfsHelper(calledFn)) {
return true;
}
}
fStack.pop_back();
}
return false;
}
const CallGraph* fCallGraph;
std::vector<Function*> fStack;
};
CycleFinder cycleFinder{&callGraph};
if (cycleFinder.containsCycle()) {
// Get the description of each function participating in the cycle
std::vector<String> fnNames;
for (Function* fn : cycleFinder.cycle()) {
fnNames.push_back(fn->description());
}
// Find the lexicographically first function description, so we generate stable errors
std::vector<String>::iterator cycleStart = std::min_element(fnNames.begin(), fnNames.end());
ptrdiff_t startIndex = std::distance(fnNames.begin(), cycleStart);
// Construct a list of the functions participating in the cycle (including the "start"
// at both the beginning and end):
String cycleDescription;
for (size_t i = 0; i <= fnNames.size(); ++i) {
cycleDescription += "\n\t" + fnNames[(i + startIndex) % fnNames.size()];
}
// Go back to the original data to find the offset of the cycle start's declaration
Function* cycleStartFn = cycleFinder.cycle()[startIndex];
errors.error(cycleStartFn->fOffset,
"potential recursion (function call cycle) not allowed:" + cycleDescription);
return true;
}
return false;
}
bool Analysis::CheckProgramUnrolledSize(const Program& program) {
// We check the size of strict-ES2 programs since SkVM will completely unroll them.
// Note that we *cannot* safely check the program size of non-ES2 code at this time, as it is
@ -760,9 +640,16 @@ bool Analysis::CheckProgramUnrolledSize(const Program& program) {
if (iter->second == kUnknownCost) {
// If the function is present in the map with an unknown cost, we're
// recursively processing it--in other words, we found a cycle in the code.
/* fContext.fErrors->error(pe.fOffset, "potential recursion (function call "
"cycle) not allowed:"); */
// Unwind our stack into a string.
String msg = "\n\t" + decl->description();
for (auto unwind = fStack.rbegin(); unwind != fStack.rend(); ++unwind) {
msg = "\n\t" + (*unwind)->description() + msg;
if (*unwind == decl) {
break;
}
}
msg = "potential recursion (function call cycle) not allowed:" + msg;
fContext.fErrors->error(pe.fOffset, std::move(msg));
fFunctionSize = iter->second = 0;
return true;
}
@ -772,10 +659,13 @@ bool Analysis::CheckProgramUnrolledSize(const Program& program) {
}
// Calculate the function cost and store it in our cache.
fStack.push_back(decl);
fFunctionSize = 0;
fUnrollFactor = 1;
bool result = INHERITED::visitProgramElement(pe);
iter->second = fFunctionSize;
fStack.pop_back();
return result;
}
@ -865,6 +755,7 @@ bool Analysis::CheckProgramUnrolledSize(const Program& program) {
int fFunctionSize;
int fUnrollFactor;
std::unordered_map<const FunctionDeclaration*, int> fFunctionCostMap;
std::vector<const FunctionDeclaration*> fStack;
};
// Process every function in our program.

5
src/sksl/SkSLAnalysis.h
View File

@ -54,11 +54,6 @@ struct Analysis {
static bool CallsSampleOutsideMain(const Program& program);
/**
* Does the function call graph of the program include any cycles? If so, emits an error.
*/
static bool DetectStaticRecursion(SkSpan<std::unique_ptr<ProgramElement>> programElements,
ErrorReporter& errors);
/**
* Computes the size of the program in a completely flattened state--loops fully unrolled,
* function calls inlined--and rejects programs that exceed an arbitrary upper bound. This is

1
src/sksl/SkSLCompiler.cpp
View File

@ -821,7 +821,6 @@ bool Compiler::finalize(Program& program) {
}
if (fContext->fConfig->strictES2Mode()) {
Analysis::DetectStaticRecursion(SkMakeSpan(program.ownedElements()), this->errorReporter());
Analysis::CheckProgramUnrolledSize(program);
}

2
tests/sksl/runtime_errors/IllegalRecursionComplex.skvm
View File

@ -1,6 +1,6 @@
### Compilation failed:
error: 6: potential recursion (function call cycle) not allowed:
error: 16: potential recursion (function call cycle) not allowed:
void f_one(int n)
void f_two(int n)
void f_three(int n)

4
tests/sksl/runtime_errors/IllegalRecursionMutual.skvm
View File

@ -1,7 +1,7 @@
### Compilation failed:
error: 4: potential recursion (function call cycle) not allowed:
bool is_even(int n)
error: 5: potential recursion (function call cycle) not allowed:
bool is_odd(int n)
bool is_even(int n)
bool is_odd(int n)
1 error