Skip to content

Commit

Permalink
Fix a code-folding fuzz bug (WebAssembly#1282)
Browse files Browse the repository at this point in the history
* fix a code-folding bug where when merging function-level tails, we moved code out of where it could reach a break target - we must not move code if it has a break target not enclosed in itself. the EffectAnalyzer already had the functionality for that, move the code around a little there to make that clearer too
  • Loading branch information
kripken authored Nov 18, 2017
1 parent da0c455 commit 8a69640
Show file tree
Hide file tree
Showing 4 changed files with 134 additions and 2 deletions.
3 changes: 3 additions & 0 deletions src/ir/effects.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ struct EffectAnalyzer : public PostWalker<EffectAnalyzer> {
bool hasSideEffects() { return hasGlobalSideEffects() || localsWritten.size() > 0 || branches || implicitTrap; }
bool hasAnything() { return branches || calls || accessesLocal() || readsMemory || writesMemory || accessesGlobal() || implicitTrap || isAtomic; }

// check if we break to anything external from ourselves
bool hasExternalBreakTargets() { return !breakNames.empty(); }

// checks if these effects would invalidate another set (e.g., if we write, we invalidate someone that reads, they can't be moved past us)
bool invalidates(EffectAnalyzer& other) {
if (branches || other.branches
Expand Down
14 changes: 12 additions & 2 deletions src/passes/CodeFolding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
#include "wasm-builder.h"
#include "ir/utils.h"
#include "ir/branch-utils.h"
#include "ir/effects.h"
#include "ir/label-utils.h"

namespace wasm {
Expand Down Expand Up @@ -474,9 +475,18 @@ struct CodeFolding : public WalkerPass<ControlFlowWalker<CodeFolding>> {
};
// let's see if we can merge deeper than num, to num + 1
auto next = tails;
// remove tails that are too short
// remove tails that are too short, or that we hit an item we can't handle
next.erase(std::remove_if(next.begin(), next.end(), [&](Tail& tail) {
return effectiveSize(tail) < num + 1;
if (effectiveSize(tail) < num + 1) return true;
auto* newItem = getItem(tail, num);
// ignore tails that break to outside blocks. we want to move code to
// the very outermost position, so such code cannot be moved
// TODO: this should not be a problem in *non*-terminating tails,
// but double-verify that
if (EffectAnalyzer(getPassOptions(), newItem).hasExternalBreakTargets()) {
return true;
}
return false;
}), next.end());
// if we have enough to investigate, do so
if (next.size() >= 2) {
Expand Down
57 changes: 57 additions & 0 deletions test/passes/code-folding.txt
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,61 @@
(f32.const 0)
)
)
(func $break-target-outside-of-return-merged-code (; 4 ;) (type $1)
(block $label$A
(if
(unreachable)
(block $block
(block $block0
(block $label$B
(if
(unreachable)
(br_table $label$A $label$B
(unreachable)
)
)
)
(return)
)
)
(block $block2
(block $label$C
(if
(unreachable)
(br_table $label$A $label$C
(unreachable)
)
)
)
(return)
)
)
)
)
(func $break-target-inside-all-good (; 5 ;) (type $1)
(block $folding-inner0
(block $label$A
(if
(unreachable)
(block $block
(block $block4
(br $folding-inner0)
)
)
(block $block6
(br $folding-inner0)
)
)
)
)
(block $label$B
(if
(unreachable)
(br_table $label$B $label$B
(unreachable)
)
)
)
(return)
)
)
62 changes: 62 additions & 0 deletions test/passes/code-folding.wast
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,67 @@
)
)
)
(func $break-target-outside-of-return-merged-code
(block $label$A
(if
(unreachable)
(block
(block
(block $label$B
(if
(unreachable)
(br_table $label$A $label$B
(unreachable)
)
)
)
(return)
)
)
(block
(block $label$C
(if
(unreachable)
(br_table $label$A $label$C ;; this all looks mergeable, but $label$A is outside
(unreachable)
)
)
)
(return)
)
)
)
)
(func $break-target-inside-all-good
(block $label$A
(if
(unreachable)
(block
(block
(block $label$B
(if
(unreachable)
(br_table $label$B $label$B
(unreachable)
)
)
)
(return)
)
)
(block
(block $label$C
(if
(unreachable)
(br_table $label$C $label$C ;; this all looks mergeable, and is, B ~~ C
(unreachable)
)
)
)
(return)
)
)
)
)
)

0 comments on commit 8a69640

Please sign in to comment.