diff options
author | Alon Zakai <azakai@google.com> | 2021-02-23 00:59:27 +0000 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-02-22 16:59:27 -0800 |
commit | b3b6a81c6a74db656d8f2c07b91c4a0ba4b28734 (patch) | |
tree | 07f7199f9f9c3761ca74a89af36630f5f3f7f7f6 | |
parent | 84eb631330cbeb2afc80df2c2c856a2392dc66bb (diff) | |
download | binaryen-b3b6a81c6a74db656d8f2c07b91c4a0ba4b28734.tar.gz binaryen-b3b6a81c6a74db656d8f2c07b91c4a0ba4b28734.tar.bz2 binaryen-b3b6a81c6a74db656d8f2c07b91c4a0ba4b28734.zip |
[Wasm Exceptions] Fix cfg-traversal on linking the basic block after a call (#3594)
This was an unfortunate case of the order of execution of call
arguments. link(self->currBasicBlock, self->startBasicBlock()) would
run the call first, which sets currBasicBlock, so we'd end up with
the same value for both parameters.
Without this fix, the testcase would drop the result of the call,
as it thought it had no uses.
Also improve debug logging here a tiny bit.
Found by emscripten-core/emscripten#13485
-rw-r--r-- | src/cfg/cfg-traversal.h | 6 | ||||
-rw-r--r-- | test/passes/coalesce-locals_all-features.txt | 21 | ||||
-rw-r--r-- | test/passes/coalesce-locals_all-features.wast | 19 |
3 files changed, 44 insertions, 2 deletions
diff --git a/src/cfg/cfg-traversal.h b/src/cfg/cfg-traversal.h index fe85f0b9f..02a1970df 100644 --- a/src/cfg/cfg-traversal.h +++ b/src/cfg/cfg-traversal.h @@ -267,7 +267,8 @@ struct CFGWalker : public ControlFlowWalker<SubType, VisitorType> { doEndThrowingInst(self, currp); if (!self->unwindCatchStack.empty()) { // exception not thrown. link to the continuation BB - self->link(self->currBasicBlock, self->startBasicBlock()); + auto* last = self->currBasicBlock; + self->link(last, self->startBasicBlock()); } } @@ -477,7 +478,8 @@ struct CFGWalker : public ControlFlowWalker<SubType, VisitorType> { generateDebugIds(); for (auto& block : basicBlocks) { assert(debugIds.count(block.get()) > 0); - std::cout << " block " << debugIds[block.get()] << ":\n"; + std::cout << " block " << debugIds[block.get()] << " (" << block.get() + << "):\n"; block->contents.dump(static_cast<SubType*>(this)->getFunction()); for (auto& in : block->in) { assert(debugIds.count(in) > 0); diff --git a/test/passes/coalesce-locals_all-features.txt b/test/passes/coalesce-locals_all-features.txt new file mode 100644 index 000000000..f532f5d0e --- /dev/null +++ b/test/passes/coalesce-locals_all-features.txt @@ -0,0 +1,21 @@ +(module + (type $none_=>_i32 (func (result i32))) + (type $i32_=>_i32 (func (param i32) (result i32))) + (export "foo" (func $1)) + (func $bar (result i32) + (i32.const 1984) + ) + (func $1 (param $0 i32) (result i32) + (try $try + (do + (local.set $0 + (call $bar) + ) + ) + (catch_all + (unreachable) + ) + ) + (local.get $0) + ) +) diff --git a/test/passes/coalesce-locals_all-features.wast b/test/passes/coalesce-locals_all-features.wast new file mode 100644 index 000000000..ff7f04f0c --- /dev/null +++ b/test/passes/coalesce-locals_all-features.wast @@ -0,0 +1,19 @@ +(module + (func $bar (result i32) + (i32.const 1984) + ) + (func "foo" (param $0 i32) (result i32) + (local $1 i32) + (try + (do + (local.set $1 + (call $bar) ;; the call may or may not throw, so we may reach the get of $1 + ) + ) + (catch_all + (unreachable) + ) + ) + (local.get $1) + ) +) |