Skip to content

Commit d0ed83f

Browse files
aheejinaheejin
committed
Implement bulk memory semantic changes
This implements recent bulk memory spec changes (WebAssembly/bulk-memory-operations#126) in Binaryen. Now `data.drop` is equivalent to shrinking a segment size to 0, and dropping already dropped segments or active segments (which are thought to be dropped in the beginning) is treated as a no-op. And all bounds checking is performed in advance, so partial copying/filling/initializing does not occur. I tried to implement `visitDataDrop` in the interpreter as `segment.data.clear();`, which is exactly what the revised spec says. I didn't end up doing that because this also deletes all contents from active segments, and there are cases we shouldn't do that: - `wasm-ctor-eval` shouldn't delete active segments, because it will store the changed contents back into segments - When `--fuzz-exec` is given to `wasm-opt`, it runs the module and compare the execution call results before and after transformations. But if running a module will nullify all active segments, applying any transformation to the module or re-running it does not make any sense. Fixes WebAssembly#2528.
1 parent d883009 commit d0ed83f

File tree

5 files changed

+35
-39
lines changed

5 files changed

+35
-39
lines changed

‎src/passes/MemoryPacking.cpp‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -142,12 +142,6 @@ struct MemoryPacking : public Pass {
142142
changed = true;
143143
}
144144
}
145-
void visitDataDrop(DataDrop* curr) {
146-
if (!getModule()->memory.segments[curr->segment].isPassive) {
147-
ExpressionManipulator::unreachable(curr);
148-
changed = true;
149-
}
150-
}
151145
void doWalkFunction(Function* func) {
152146
changed = false;
153147
super::doWalkFunction(func);

‎src/wasm-interpreter.h‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1902,17 +1902,21 @@ template<typename GlobalManager, typename SubType> class ModuleInstanceBase {
19021902
Memory::Segment& segment = instance.wasm.memory.segments[curr->segment];
19031903

19041904
if (instance.droppedSegments.count(curr->segment)) {
1905-
trap("memory.init of dropped segment");
1905+
trap("out of bounds segment access in memory.init");
19061906
}
19071907

19081908
Address destVal(uint32_t(dest.value.geti32()));
19091909
Address offsetVal(uint32_t(offset.value.geti32()));
19101910
Address sizeVal(uint32_t(size.value.geti32()));
19111911

1912+
if ((uint64_t)offsetVal + sizeVal > segment.data.size()) {
1913+
trap("out of bounds segment access in memory.init");
1914+
}
1915+
if ((uint64_t)destVal + sizeVal >
1916+
(uint64_t)instance.wasm.memory.max * Memory::kPageSize) {
1917+
trap("out of bounds memory access in memory.init");
1918+
}
19121919
for (size_t i = 0; i < sizeVal; ++i) {
1913-
if (offsetVal + i >= segment.data.size()) {
1914-
trap("out of bounds segment access in memory.init");
1915-
}
19161920
Literal addr(uint32_t(destVal + i));
19171921
instance.externalInterface->store8(instance.getFinalAddress(addr, 1),
19181922
segment.data[offsetVal + i]);
@@ -1921,9 +1925,6 @@ template<typename GlobalManager, typename SubType> class ModuleInstanceBase {
19211925
}
19221926
Flow visitDataDrop(DataDrop* curr) {
19231927
NOTE_ENTER("DataDrop");
1924-
if (instance.droppedSegments.count(curr->segment)) {
1925-
trap("data.drop of dropped segment");
1926-
}
19271928
instance.droppedSegments.insert(curr->segment);
19281929
return {};
19291930
}
@@ -1948,6 +1949,13 @@ template<typename GlobalManager, typename SubType> class ModuleInstanceBase {
19481949
Address sourceVal(uint32_t(source.value.geti32()));
19491950
Address sizeVal(uint32_t(size.value.geti32()));
19501951

1952+
if ((uint64_t)sourceVal + sizeVal >
1953+
(uint64_t)instance.wasm.memory.max * Memory::kPageSize ||
1954+
(uint64_t)destVal + sizeVal >
1955+
(uint64_t)instance.wasm.memory.max * Memory::kPageSize) {
1956+
trap("out of bounds segment access in memory.copy");
1957+
}
1958+
19511959
int64_t start = 0;
19521960
int64_t end = sizeVal;
19531961
int step = 1;
@@ -1958,9 +1966,6 @@ template<typename GlobalManager, typename SubType> class ModuleInstanceBase {
19581966
step = -1;
19591967
}
19601968
for (int64_t i = start; i != end; i += step) {
1961-
if (i + destVal >= std::numeric_limits<uint32_t>::max()) {
1962-
trap("Out of bounds memory access");
1963-
}
19641969
instance.externalInterface->store8(
19651970
instance.getFinalAddress(Literal(uint32_t(destVal + i)), 1),
19661971
instance.externalInterface->load8s(
@@ -1988,6 +1993,10 @@ template<typename GlobalManager, typename SubType> class ModuleInstanceBase {
19881993
Address destVal(uint32_t(dest.value.geti32()));
19891994
Address sizeVal(uint32_t(size.value.geti32()));
19901995

1996+
if ((uint64_t)destVal + sizeVal >
1997+
(uint64_t)instance.wasm.memory.max * Memory::kPageSize) {
1998+
trap("out of bounds memory access in memory.fill");
1999+
}
19912000
uint8_t val(value.value.geti32());
19922001
for (size_t i = 0; i < sizeVal; ++i) {
19932002
instance.externalInterface->store8(

‎test/passes/memory-packing_all-features.txt‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,24 +22,20 @@
2222
(type $none_=>_none (func))
2323
(memory $0 1 1)
2424
(func $foo (; 0 ;)
25-
(block
26-
(drop
27-
(i32.const 0)
28-
)
29-
(drop
30-
(i32.const 0)
31-
)
32-
(drop
33-
(i32.const 0)
34-
)
35-
(unreachable)
25+
(drop
26+
(i32.const 0)
27+
)
28+
(drop
29+
(i32.const 0)
30+
)
31+
(drop
32+
(i32.const 0)
3633
)
3734
(unreachable)
3835
)
3936
(func $bar (; 1 ;)
4037
(drop
4138
(loop $loop-in (result i32)
42-
(unreachable)
4339
(i32.const 42)
4440
)
4541
)

‎test/passes/memory-packing_all-features.wast‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,10 @@
2626
(i32.const 0)
2727
(i32.const 0)
2828
)
29-
(data.drop 0)
3029
)
3130
(func $bar
3231
(drop
3332
(loop (result i32)
34-
(data.drop 0)
3533
(i32.const 42)
3634
)
3735
)

‎test/spec/bulk-memory.wast‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,9 @@
101101
(invoke "copy" (i32.const 0x10000) (i32.const 0) (i32.const 0))
102102
(invoke "copy" (i32.const 0) (i32.const 0x10000) (i32.const 0))
103103

104-
;; OK copying 0 bytes outside of memory.
105-
(invoke "copy" (i32.const 0x10001) (i32.const 0) (i32.const 0))
106-
(invoke "copy" (i32.const 0) (i32.const 0x10001) (i32.const 0))
104+
;; Copying 0 bytes outside of memory limit is NOT allowed.
105+
(assert_trap (invoke "copy" (i32.const 0x10001) (i32.const 0) (i32.const 0)))
106+
(assert_trap (invoke "copy" (i32.const 0) (i32.const 0x10001) (i32.const 0)))
107107

108108
;; memory.init
109109
(module
@@ -138,9 +138,9 @@
138138
(invoke "init" (i32.const 0x10000) (i32.const 0) (i32.const 0))
139139
(invoke "init" (i32.const 0) (i32.const 4) (i32.const 0))
140140

141-
;; OK writing 0 bytes outside of memory or segment.
141+
;; Writing 0 bytes outside of memory / segment limit is NOT allowed.
142142
(invoke "init" (i32.const 0x10001) (i32.const 0) (i32.const 0))
143-
(invoke "init" (i32.const 0) (i32.const 5) (i32.const 0))
143+
(assert_trap (invoke "init" (i32.const 0) (i32.const 5) (i32.const 0)))
144144

145145
;; data.drop
146146
(module
@@ -157,9 +157,8 @@
157157
(memory.init 1 (i32.const 0) (i32.const 0) (i32.const 0)))
158158
)
159159

160+
;; It is OK to drop the same segment multiple times or drop an active segment
160161
(invoke "init_passive")
161162
(invoke "drop_passive")
162-
(assert_trap (invoke "drop_passive") "data segment dropped")
163-
(assert_trap (invoke "init_passive") "data segment dropped")
164-
(assert_trap (invoke "drop_active") "data segment dropped")
165-
(assert_trap (invoke "init_active") "data segment dropped")
163+
(invoke "drop_passive")
164+
(invoke "drop_active")

0 commit comments

Comments
 (0)