summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlon Zakai <azakai@google.com>2020-07-28 11:08:17 -0700
committerGitHub <noreply@github.com>2020-07-28 11:08:17 -0700
commit63d60fef3b07a343e21fb4bb8227c4e674633704 (patch)
tree38237feec2af4db045d387adbc03931239f5ec63
parent26f240c72dd62ed8a39f7466df99e51ec34487aa (diff)
downloadbinaryen-63d60fef3b07a343e21fb4bb8227c4e674633704.tar.gz
binaryen-63d60fef3b07a343e21fb4bb8227c4e674633704.tar.bz2
binaryen-63d60fef3b07a343e21fb4bb8227c4e674633704.zip
Fix the side effects of data.drop (#2996)
We marked it as readsMemory so that it could be reordered with various things, except for memory.init. However, the fuzzer found that's not quite right, as it has a global side effect - memory.inits that run later can notice that. So it can't be reordered with anything that might affect global side effects from happening, as in the testcase added here (an instruction that may trap cannot be reordered with a data.drop, as it may prevent the data.drop from happening and changing global state). There may be a way to optimize this more carefully that would allow more optimizations, but as this is a rare instruction I'm not sure it's worth more work.
-rw-r--r--src/ir/effects.h6
-rw-r--r--test/passes/simplify-locals_all-features.txt27
-rw-r--r--test/passes/simplify-locals_all-features.wast18
-rw-r--r--test/passes/simplify-locals_all-features_disable-exception-handling.txt8
4 files changed, 51 insertions, 8 deletions
diff --git a/src/ir/effects.h b/src/ir/effects.h
index f6c87a4e5..9d6e90936 100644
--- a/src/ir/effects.h
+++ b/src/ir/effects.h
@@ -392,8 +392,10 @@ struct EffectAnalyzer
}
}
void visitDataDrop(DataDrop* curr) {
- // prevent reordering with memory.init
- readsMemory = true;
+ // data.drop does not actually write memory, but it does alter the size of
+ // a segment, which can be noticeable later by memory.init, so we need to
+ // mark it as having a global side effect of some kind.
+ writesMemory = true;
if (!ignoreImplicitTraps) {
implicitTrap = true;
}
diff --git a/test/passes/simplify-locals_all-features.txt b/test/passes/simplify-locals_all-features.txt
index fbb1f87ed..34efb8726 100644
--- a/test/passes/simplify-locals_all-features.txt
+++ b/test/passes/simplify-locals_all-features.txt
@@ -1837,13 +1837,15 @@
)
(func $data-drop-load
(local $x i32)
- (nop)
- (data.drop 0)
- (drop
+ (local.set $x
(i32.load
(i32.const 0)
)
)
+ (data.drop 0)
+ (drop
+ (local.get $x)
+ )
)
(func $data-drop-store
(local $x i32)
@@ -2019,3 +2021,22 @@
)
)
)
+(module
+ (type $none_=>_i32 (func (result i32)))
+ (memory $0 (shared 1 1))
+ (data passive "data")
+ (export "foo" (func $0))
+ (func $0 (result i32)
+ (local $0 i32)
+ (block $block (result i32)
+ (local.set $0
+ (i32.rem_u
+ (i32.const 0)
+ (i32.const 0)
+ )
+ )
+ (data.drop 0)
+ (local.get $0)
+ )
+ )
+)
diff --git a/test/passes/simplify-locals_all-features.wast b/test/passes/simplify-locals_all-features.wast
index a8e1a161f..3edffca7e 100644
--- a/test/passes/simplify-locals_all-features.wast
+++ b/test/passes/simplify-locals_all-features.wast
@@ -1782,3 +1782,21 @@
)
)
)
+;; data.drop has global side effects
+(module
+ (memory $0 (shared 1 1))
+ (data passive "data")
+ (func "foo" (result i32)
+ (local $0 i32)
+ (block (result i32)
+ (local.set $0
+ (i32.rem_u ;; will trap, so cannot be reordered to the end
+ (i32.const 0)
+ (i32.const 0)
+ )
+ )
+ (data.drop 0) ;; has global side effects that may be noticed later
+ (local.get $0)
+ )
+ )
+)
diff --git a/test/passes/simplify-locals_all-features_disable-exception-handling.txt b/test/passes/simplify-locals_all-features_disable-exception-handling.txt
index 7be68af6d..0d035234b 100644
--- a/test/passes/simplify-locals_all-features_disable-exception-handling.txt
+++ b/test/passes/simplify-locals_all-features_disable-exception-handling.txt
@@ -1831,13 +1831,15 @@
)
(func $data-drop-load
(local $x i32)
- (nop)
- (data.drop 0)
- (drop
+ (local.set $x
(i32.load
(i32.const 0)
)
)
+ (data.drop 0)
+ (drop
+ (local.get $x)
+ )
)
(func $data-drop-store
(local $x i32)