diff options
| author | Alon Zakai <alonzakai@gmail.com> | 2018-04-07 10:19:58 -0700 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2018-04-07 10:19:58 -0700 |
| commit | e42e1e3d4a5c67c0c066fe397b456ab8d41a78fd (patch) | |
| tree | 6f833f2ede0b626840e2b7b76fec0e712ff0680b /src | |
| parent | 5b5789495a97602869f18d552b2a9e1814edefae (diff) | |
| download | binaryen-e42e1e3d4a5c67c0c066fe397b456ab8d41a78fd.tar.gz binaryen-e42e1e3d4a5c67c0c066fe397b456ab8d41a78fd.tar.bz2 binaryen-e42e1e3d4a5c67c0c066fe397b456ab8d41a78fd.zip | |
Handle literally unreachable brs (#1497)
The optimization in #1495 had a bug which was found by the fuzzer: our binary format parsing will not emit unreachable code (it may be stacky, so we ignore it). However, while parsing it we note breaks that are taken there, and then we removed that code, leading to a state where a break was not taken in the code, but we thought it was.
This PR clarifies the difference between unreachable code in the wasm sense (anything from the start of a block til an unreachable is "reachable") and the literal sense (even that code at the start may not be literally reachable if the block is not reachable), and then we use literal unreachability to know what code will be ignored and therefore we should ignore breaks in.
Diffstat (limited to 'src')
| -rw-r--r-- | src/wasm-binary.h | 18 | ||||
| -rw-r--r-- | src/wasm/wasm-binary.cpp | 22 |
2 files changed, 32 insertions, 8 deletions
diff --git a/src/wasm-binary.h b/src/wasm-binary.h index b8eb86d31..4b4a37888 100644 --- a/src/wasm-binary.h +++ b/src/wasm-binary.h @@ -895,8 +895,22 @@ public: std::vector<Expression*> expressionStack; - bool definitelyUnreachable; // set when we know code is definitely unreachable. this helps parse - // stacky wasm code, which can be unsuitable for our IR when unreachable + // set when we know code is unreachable in the sense of the wasm spec: we are in a block + // and after an unreachable element. + // this helps parse stacky wasm code, which can be unsuitable for our IR when unreachable. + bool unreachableInTheWasmSense; + + // set when the current code being processed will not be emitted in the output, which is the + // case when it is literally unreachable, for example, + // (block $a + // (unreachable) + // (block $b + // ;; code here is reachable in the wasm sense, even though $b as a whole is not + // (unreachable) + // ;; code here is unreachable in the wasm sense + // ) + // ) + bool willBeIgnored; BinaryConsts::ASTNodes lastSeparator = BinaryConsts::End; diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index f70f74c56..e0c75badc 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -1753,6 +1753,7 @@ void WasmBinaryBuilder::readFunctions() { if (debug) std::cerr << "processing function: " << i << std::endl; nextLabel = 0; useDebugLocation = false; + willBeIgnored = false; // process body assert(breakTargetNames.size() == 0); assert(breakStack.empty()); @@ -1957,7 +1958,7 @@ void WasmBinaryBuilder::readGlobals() { void WasmBinaryBuilder::processExpressions() { if (debug) std::cerr << "== processExpressions" << std::endl; - definitelyUnreachable = false; + unreachableInTheWasmSense = false; while (1) { Expression* curr; auto ret = readExpression(curr); @@ -1996,19 +1997,24 @@ void WasmBinaryBuilder::skipUnreachableCode() { // unreachable, and we can ignore anything after it. things after it may pop, // we want to undo that auto savedStack = expressionStack; + // note we are entering unreachable code, and note what the state as before so + // we can restore it + auto before = willBeIgnored; + willBeIgnored = true; // clear the stack. nothing should be popped from there anyhow, just stuff // can be pushed and then popped. Popping past the top of the stack will // result in uneachables being returned expressionStack.clear(); while (1) { - // set the definitelyUnreachable flag each time, as sub-blocks may set and unset it - definitelyUnreachable = true; + // set the unreachableInTheWasmSense flag each time, as sub-blocks may set and unset it + unreachableInTheWasmSense = true; Expression* curr; auto ret = readExpression(curr); if (!curr) { if (debug) std::cerr << "== skipUnreachableCode finished" << std::endl; lastSeparator = ret; - definitelyUnreachable = false; + unreachableInTheWasmSense = false; + willBeIgnored = before; expressionStack = savedStack; return; } @@ -2019,7 +2025,7 @@ void WasmBinaryBuilder::skipUnreachableCode() { Expression* WasmBinaryBuilder::popExpression() { if (debug) std::cerr << "== popExpression" << std::endl; if (expressionStack.empty()) { - if (definitelyUnreachable) { + if (unreachableInTheWasmSense) { // in unreachable code, trying to pop past the polymorphic stack // area results in receiving unreachables if (debug) std::cerr << "== popping unreachable from polymorphic stack" << std::endl; @@ -2460,7 +2466,11 @@ WasmBinaryBuilder::BreakTarget WasmBinaryBuilder::getBreakTarget(int32_t offset) } if (debug) std::cerr << "breaktarget "<< breakStack[index].name << " arity " << breakStack[index].arity << std::endl; auto& ret = breakStack[index]; - breakTargetNames.insert(ret.name); + // if the break is in literally unreachable code, then we will not emit it anyhow, + // so do not note that the target has breaks to it + if (!willBeIgnored) { + breakTargetNames.insert(ret.name); + } return ret; } |