Skip to content

Commit 8a69640

Browse files
authored
Fix a code-folding fuzz bug (#1282)
* 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
1 parent da0c455 commit 8a69640

File tree

4 files changed

+134
-2
lines changed

4 files changed

+134
-2
lines changed

src/ir/effects.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@ struct EffectAnalyzer : public PostWalker<EffectAnalyzer> {
6262
bool hasSideEffects() { return hasGlobalSideEffects() || localsWritten.size() > 0 || branches || implicitTrap; }
6363
bool hasAnything() { return branches || calls || accessesLocal() || readsMemory || writesMemory || accessesGlobal() || implicitTrap || isAtomic; }
6464

65+
// check if we break to anything external from ourselves
66+
bool hasExternalBreakTargets() { return !breakNames.empty(); }
67+
6568
// 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)
6669
bool invalidates(EffectAnalyzer& other) {
6770
if (branches || other.branches

src/passes/CodeFolding.cpp

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
#include "wasm-builder.h"
6262
#include "ir/utils.h"
6363
#include "ir/branch-utils.h"
64+
#include "ir/effects.h"
6465
#include "ir/label-utils.h"
6566

6667
namespace wasm {
@@ -474,9 +475,18 @@ struct CodeFolding : public WalkerPass<ControlFlowWalker<CodeFolding>> {
474475
};
475476
// let's see if we can merge deeper than num, to num + 1
476477
auto next = tails;
477-
// remove tails that are too short
478+
// remove tails that are too short, or that we hit an item we can't handle
478479
next.erase(std::remove_if(next.begin(), next.end(), [&](Tail& tail) {
479-
return effectiveSize(tail) < num + 1;
480+
if (effectiveSize(tail) < num + 1) return true;
481+
auto* newItem = getItem(tail, num);
482+
// ignore tails that break to outside blocks. we want to move code to
483+
// the very outermost position, so such code cannot be moved
484+
// TODO: this should not be a problem in *non*-terminating tails,
485+
// but double-verify that
486+
if (EffectAnalyzer(getPassOptions(), newItem).hasExternalBreakTargets()) {
487+
return true;
488+
}
489+
return false;
480490
}), next.end());
481491
// if we have enough to investigate, do so
482492
if (next.size() >= 2) {

test/passes/code-folding.txt

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,4 +49,61 @@
4949
(f32.const 0)
5050
)
5151
)
52+
(func $break-target-outside-of-return-merged-code (; 4 ;) (type $1)
53+
(block $label$A
54+
(if
55+
(unreachable)
56+
(block $block
57+
(block $block0
58+
(block $label$B
59+
(if
60+
(unreachable)
61+
(br_table $label$A $label$B
62+
(unreachable)
63+
)
64+
)
65+
)
66+
(return)
67+
)
68+
)
69+
(block $block2
70+
(block $label$C
71+
(if
72+
(unreachable)
73+
(br_table $label$A $label$C
74+
(unreachable)
75+
)
76+
)
77+
)
78+
(return)
79+
)
80+
)
81+
)
82+
)
83+
(func $break-target-inside-all-good (; 5 ;) (type $1)
84+
(block $folding-inner0
85+
(block $label$A
86+
(if
87+
(unreachable)
88+
(block $block
89+
(block $block4
90+
(br $folding-inner0)
91+
)
92+
)
93+
(block $block6
94+
(br $folding-inner0)
95+
)
96+
)
97+
)
98+
)
99+
(block $label$B
100+
(if
101+
(unreachable)
102+
(br_table $label$B $label$B
103+
(unreachable)
104+
)
105+
)
106+
)
107+
(return)
108+
)
52109
)

test/passes/code-folding.wast

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,5 +52,67 @@
5252
)
5353
)
5454
)
55+
(func $break-target-outside-of-return-merged-code
56+
(block $label$A
57+
(if
58+
(unreachable)
59+
(block
60+
(block
61+
(block $label$B
62+
(if
63+
(unreachable)
64+
(br_table $label$A $label$B
65+
(unreachable)
66+
)
67+
)
68+
)
69+
(return)
70+
)
71+
)
72+
(block
73+
(block $label$C
74+
(if
75+
(unreachable)
76+
(br_table $label$A $label$C ;; this all looks mergeable, but $label$A is outside
77+
(unreachable)
78+
)
79+
)
80+
)
81+
(return)
82+
)
83+
)
84+
)
85+
)
86+
(func $break-target-inside-all-good
87+
(block $label$A
88+
(if
89+
(unreachable)
90+
(block
91+
(block
92+
(block $label$B
93+
(if
94+
(unreachable)
95+
(br_table $label$B $label$B
96+
(unreachable)
97+
)
98+
)
99+
)
100+
(return)
101+
)
102+
)
103+
(block
104+
(block $label$C
105+
(if
106+
(unreachable)
107+
(br_table $label$C $label$C ;; this all looks mergeable, and is, B ~~ C
108+
(unreachable)
109+
)
110+
)
111+
)
112+
(return)
113+
)
114+
)
115+
)
116+
)
55117
)
56118

0 commit comments

Comments
 (0)