Skip to content

Commit

Permalink
Fix a fuzz issue with WebAssembly#6984 (WebAssembly#6988)
Browse files Browse the repository at this point in the history
When I refactored the optimizeDroppedBlock logic in WebAssembly#6982, I didn't move the
unreachability check with that code, which was wrong. When that function
was called from another place in WebAssembly#6984, the fuzzer found an issue.

Diff without whitespace is smaller. This reverts almost all the test updates
from WebAssembly#6984 - those changes were on blocks with unreachable children.
The change was safe on them, but in general removing a block value in the
presence of unreachable code is tricky, so it's best to avoid it.

The testcase is a little bizarre, but it's the one the fuzzer found and I can't
find a way to generate a better one (other than to reduce it, which I did).
  • Loading branch information
kripken authored Oct 7, 2024
1 parent 0be8d5e commit bcdedab
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 52 deletions.
27 changes: 13 additions & 14 deletions src/passes/MergeBlocks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,11 @@ static bool optimizeDroppedBlock(Drop* drop,
PassOptions& options,
BranchUtils::BranchSeekerCache& branchInfo) {
assert(drop->value == block);
if (hasUnreachableChild(block)) {
// Don't move around unreachable code, as it can change types (leave it for
// DCE).
return false;
}
if (block->name.is()) {
// There may be breaks: see if we can remove their values.
Expression* expression = block;
Expand Down Expand Up @@ -261,20 +266,14 @@ static void optimizeBlock(Block* curr,
// drop into the block, and remove br values. This allows more merging.
if (auto* drop = list[i]->dynCast<Drop>()) {
childBlock = drop->value->dynCast<Block>();
if (childBlock) {
if (hasUnreachableChild(childBlock)) {
// don't move around unreachable code, as it can change types
// dce should have been run anyhow
continue;
}
if (optimizeDroppedBlock(
drop, childBlock, *module, passOptions, branchInfo)) {
child = list[i] = childBlock;
more = true;
changed = true;
} else {
childBlock = nullptr;
}
if (childBlock &&
optimizeDroppedBlock(
drop, childBlock, *module, passOptions, branchInfo)) {
child = list[i] = childBlock;
more = true;
changed = true;
} else {
childBlock = nullptr;
}
} else if ((loop = list[i]->dynCast<Loop>())) {
// We can merge a loop's "tail" - if the body is a block and has
Expand Down
9 changes: 5 additions & 4 deletions test/lit/passes/merge-blocks.wast
Original file line number Diff line number Diff line change
Expand Up @@ -405,11 +405,12 @@
)

;; CHECK: (func $toplevel (type $4)
;; CHECK-NEXT: (block $label
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (i32.const 42)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block $label (result i32)
;; CHECK-NEXT: (br $label
;; CHECK-NEXT: (i32.const 42)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (br $label)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $toplevel
Expand Down
7 changes: 6 additions & 1 deletion test/lit/passes/monomorphize-drop.wast
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,12 @@

;; CAREFUL: (func $return-normal_4 (type $1)
;; CAREFUL-NEXT: (drop
;; CAREFUL-NEXT: (call $import)
;; CAREFUL-NEXT: (block
;; CAREFUL-NEXT: (drop
;; CAREFUL-NEXT: (call $import)
;; CAREFUL-NEXT: )
;; CAREFUL-NEXT: (return)
;; CAREFUL-NEXT: )
;; CAREFUL-NEXT: )
;; CAREFUL-NEXT: )

Expand Down
10 changes: 7 additions & 3 deletions test/passes/Oz_fuzz-exec_all-features.txt
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,14 @@
(nop)
)
(func $br-on_non_null-2 (type $void_func)
(call $log
(i32.const 1)
(drop
(block
(call $log
(i32.const 1)
)
(unreachable)
)
)
(unreachable)
)
(func $cast-on-func (type $void_func)
(call $log
Expand Down
29 changes: 19 additions & 10 deletions test/passes/merge-blocks.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,11 @@
)
)
(func $drop-block-br
(block $x
(drop
(i32.const 1)
)
(br $x)
(drop
(drop
(block $x (result i32)
(br $x
(i32.const 1)
)
(i32.const 0)
)
)
Expand Down Expand Up @@ -78,9 +77,15 @@
)
)
(func $drop-block-squared-iloop
(block $label$0
(loop $label$2
(br $label$2)
(drop
(block $label$0 (result i32)
(drop
(block $label$1
(loop $label$2
(br $label$2)
)
)
)
)
)
)
Expand All @@ -104,7 +109,11 @@
(func $loop-block-drop-block-return
(loop $label$4
(block $label$5
(return)
(drop
(block $label$6 (result i32)
(return)
)
)
)
)
)
Expand Down
64 changes: 44 additions & 20 deletions test/passes/remove-unused-names_merge-blocks_all-features.txt
Original file line number Diff line number Diff line change
Expand Up @@ -286,11 +286,15 @@
(i32.const 30)
)
(drop
(i32.const 10)
)
(i32.add
(unreachable)
(i32.const 20)
(block
(drop
(i32.const 10)
)
(i32.add
(unreachable)
(i32.const 20)
)
)
)
(drop
(i32.const 20)
Expand Down Expand Up @@ -818,30 +822,40 @@
)
(func $drop-unreachable (type $2) (result i32)
(local $0 i32)
(unreachable)
(drop
(block (result i32)
(unreachable)
)
)
(unreachable)
)
(func $concrete_finale_in_unreachable (type $5) (result f64)
(block
(unreachable)
(drop
(drop
(block (result f64)
(unreachable)
(f64.const 6.322092475576799e-96)
)
)
(f64.const -1)
)
(func $dont-move-unreachable (type $3)
(loop $label$0
(br $label$0)
(drop
(i32.const 1)
(block (result i32)
(br $label$0)
(i32.const 1)
)
)
)
)
(func $dont-move-unreachable-last (type $3)
(loop $label$0
(call $dont-move-unreachable-last)
(br $label$0)
(drop
(block (result i32)
(call $dont-move-unreachable-last)
(br $label$0)
)
)
)
)
(func $move-around-unreachable-in-middle (type $3)
Expand All @@ -862,10 +876,16 @@
)
(func $drop-unreachable-block-with-concrete-final (type $3)
(drop
(return)
)
(drop
(i32.const -452)
(block (result i32)
(drop
(block
(drop
(return)
)
)
)
(i32.const -452)
)
)
)
(func $merging-with-unreachable-in-middle (type $2) (result i32)
Expand All @@ -881,9 +901,13 @@
)
(func $remove-br-after-unreachable (type $3)
(block $label$9
(block
(return)
(br $label$9)
(drop
(block
(block
(return)
(br $label$9)
)
)
)
)
)
Expand Down

0 comments on commit bcdedab

Please sign in to comment.