diff options
author | Alon Zakai <azakai@google.com> | 2024-10-07 13:43:08 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-10-07 20:43:08 +0000 |
commit | bcdedab9a97dbea1c2ee151ca1013e3896f16c8a (patch) | |
tree | cfc99561a85a40379711d96ab5612269197681cf /src/passes | |
parent | 0be8d5e07e5f893705bc36c5d09676ee65d3466f (diff) | |
download | binaryen-bcdedab9a97dbea1c2ee151ca1013e3896f16c8a.tar.gz binaryen-bcdedab9a97dbea1c2ee151ca1013e3896f16c8a.tar.bz2 binaryen-bcdedab9a97dbea1c2ee151ca1013e3896f16c8a.zip |
Fix a fuzz issue with #6984 (#6988)
When I refactored the optimizeDroppedBlock logic in #6982, I didn't move the
unreachability check with that code, which was wrong. When that function
was called from another place in #6984, the fuzzer found an issue.
Diff without whitespace is smaller. This reverts almost all the test updates
from #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).
Diffstat (limited to 'src/passes')
-rw-r--r-- | src/passes/MergeBlocks.cpp | 27 |
1 files changed, 13 insertions, 14 deletions
diff --git a/src/passes/MergeBlocks.cpp b/src/passes/MergeBlocks.cpp index 12f67329d..7518c73ca 100644 --- a/src/passes/MergeBlocks.cpp +++ b/src/passes/MergeBlocks.cpp @@ -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; @@ -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 |