Skip to content

Commit c80d7cc

Browse files
kripkenradekdoulik
kripken
authored and
radekdoulik
committed
Implement dropping of active Element Segments (WebAssembly#6343)
Also rename the existing droppedSegments to droppedDataSegments for clarity.
1 parent b606e83 commit c80d7cc

File tree

2 files changed

+121
-10
lines changed

2 files changed

+121
-10
lines changed

‎src/wasm-interpreter.h‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2696,7 +2696,8 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
26962696
// stack traces.
26972697
std::vector<Name> functionStack;
26982698

2699-
std::unordered_set<Name> droppedSegments;
2699+
std::unordered_set<Name> droppedDataSegments;
2700+
std::unordered_set<Name> droppedElementSegments;
27002701

27012702
struct TableInterfaceInfo {
27022703
// The external interface in which the table is defined.
@@ -2746,6 +2747,8 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
27462747
Flow ret = self()->visit(segment->data[i]);
27472748
extInterface->tableStore(tableName, offset + i, ret.getSingleValue());
27482749
}
2750+
2751+
droppedElementSegments.insert(segment->name);
27492752
});
27502753
}
27512754

@@ -3630,7 +3633,7 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
36303633
Address offsetVal(uint32_t(offset.getSingleValue().geti32()));
36313634
Address sizeVal(uint32_t(size.getSingleValue().geti32()));
36323635

3633-
if (offsetVal + sizeVal > 0 && droppedSegments.count(curr->segment)) {
3636+
if (offsetVal + sizeVal > 0 && droppedDataSegments.count(curr->segment)) {
36343637
trap("out of bounds segment access in memory.init");
36353638
}
36363639
if ((uint64_t)offsetVal + sizeVal > segment->data.size()) {
@@ -3652,7 +3655,7 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
36523655
}
36533656
Flow visitDataDrop(DataDrop* curr) {
36543657
NOTE_ENTER("DataDrop");
3655-
droppedSegments.insert(curr->segment);
3658+
droppedDataSegments.insert(curr->segment);
36563659
return {};
36573660
}
36583661
Flow visitMemoryCopy(MemoryCopy* curr) {
@@ -3768,7 +3771,7 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
37683771
const auto& seg = *wasm.getDataSegment(curr->segment);
37693772
auto elemBytes = element.getByteSize();
37703773
auto end = offset + size * elemBytes;
3771-
if ((size != 0ull && droppedSegments.count(curr->segment)) ||
3774+
if ((size != 0ull && droppedDataSegments.count(curr->segment)) ||
37723775
end > seg.data.size()) {
37733776
trap("out of bounds segment access in array.new_data");
37743777
}
@@ -3797,10 +3800,12 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
37973800

37983801
const auto& seg = *wasm.getElementSegment(curr->segment);
37993802
auto end = offset + size;
3800-
// TODO: Handle dropped element segments once we support those.
38013803
if (end > seg.data.size()) {
38023804
trap("out of bounds segment access in array.new_elem");
38033805
}
3806+
if (end > 0 && droppedElementSegments.count(curr->segment)) {
3807+
trap("out of bounds segment access in array.new_elem");
3808+
}
38043809
contents.reserve(size);
38053810
for (Index i = offset; i < end; ++i) {
38063811
auto val = self()->visit(seg.data[i]).getSingleValue();
@@ -3848,7 +3853,7 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
38483853
if (offsetVal + readSize > seg->data.size()) {
38493854
trap("out of bounds segment access in array.init_data");
38503855
}
3851-
if (offsetVal + sizeVal > 0 && droppedSegments.count(curr->segment)) {
3856+
if (offsetVal + sizeVal > 0 && droppedDataSegments.count(curr->segment)) {
38523857
trap("out of bounds segment access in array.init_data");
38533858
}
38543859
for (size_t i = 0; i < sizeVal; i++) {
@@ -3891,11 +3896,13 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
38913896
Module& wasm = *self()->getModule();
38923897

38933898
auto* seg = wasm.getElementSegment(curr->segment);
3894-
if ((uint64_t)offsetVal + sizeVal > seg->data.size()) {
3895-
trap("out of bounds segment access in array.init");
3899+
auto max = (uint64_t)offsetVal + sizeVal;
3900+
if (max > seg->data.size()) {
3901+
trap("out of bounds segment access in array.init_elem");
3902+
}
3903+
if (max > 0 && droppedElementSegments.count(curr->segment)) {
3904+
trap("out of bounds segment access in array.init_elem");
38963905
}
3897-
// TODO: Check whether the segment has been dropped once we support
3898-
// dropping element segments.
38993906
for (size_t i = 0; i < sizeVal; i++) {
39003907
// TODO: This is not correct because it does not preserve the identity
39013908
// of references in the table! ArrayNew suffers the same problem.

‎test/lit/exec/array.wast‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,14 @@
55
(module
66
(type $array (array (mut i8)))
77

8+
(type $array-func (array (mut funcref)))
9+
10+
(table $table 10 10 funcref)
11+
12+
(elem $active (i32.const 0) $func)
13+
14+
(elem $passive $func)
15+
816
;; CHECK: [fuzz-exec] calling func
917
;; CHECK-NEXT: [fuzz-exec] note result: func => 1
1018
(func $func (export "func") (result i32)
@@ -14,7 +22,103 @@
1422
(return (i32.const 2))
1523
)
1624
)
25+
26+
;; CHECK: [fuzz-exec] calling new_active
27+
;; CHECK-NEXT: [trap out of bounds segment access in array.new_elem]
28+
(func $new_active (export "new_active")
29+
;; Even though this is reading 0 items, offset 1 is out of bounds in that
30+
;; dropped element segment, and this traps.
31+
(drop
32+
(array.new_elem $array-func $active
33+
(i32.const 1)
34+
(i32.const 0)
35+
)
36+
)
37+
)
38+
39+
;; CHECK: [fuzz-exec] calling new_active_in_bounds
40+
(func $new_active_in_bounds (export "new_active_in_bounds")
41+
;; Even though this is dropped, we read 0 from offset 0, which is ok.
42+
(drop
43+
(array.new_elem $array-func $active
44+
(i32.const 0)
45+
(i32.const 0)
46+
)
47+
)
48+
)
49+
50+
;; CHECK: [fuzz-exec] calling new_passive
51+
(func $new_passive (export "new_passive")
52+
;; Using the passive segment here works.
53+
(drop
54+
(array.new_elem $array-func $passive
55+
(i32.const 1)
56+
(i32.const 0)
57+
)
58+
)
59+
)
60+
61+
;; CHECK: [fuzz-exec] calling init_active
62+
;; CHECK-NEXT: [trap out of bounds segment access in array.init_elem]
63+
(func $init_active (export "init_active")
64+
;; Even though this is reading 0 items, offset 1 is out of bounds in that
65+
;; dropped element segment, and this traps.
66+
(array.init_elem $array-func $active
67+
(array.new_default $array-func
68+
(i32.const 100)
69+
)
70+
(i32.const 50)
71+
(i32.const 1)
72+
(i32.const 0)
73+
)
74+
)
75+
76+
;; CHECK: [fuzz-exec] calling init_active_in_bounds
77+
(func $init_active_in_bounds (export "init_active_in_bounds")
78+
;; Even though this is dropped, we read 0 from offset 0, which is ok.
79+
(array.init_elem $array-func $active
80+
(array.new_default $array-func
81+
(i32.const 100)
82+
)
83+
(i32.const 50)
84+
(i32.const 0)
85+
(i32.const 0)
86+
)
87+
)
88+
89+
;; CHECK: [fuzz-exec] calling init_passive
90+
(func $init_passive (export "init_passive")
91+
;; This works ok.
92+
(array.init_elem $array-func $passive
93+
(array.new_default $array-func
94+
(i32.const 100)
95+
)
96+
(i32.const 50)
97+
(i32.const 1)
98+
(i32.const 0)
99+
)
100+
)
17101
)
18102
;; CHECK: [fuzz-exec] calling func
19103
;; CHECK-NEXT: [fuzz-exec] note result: func => 1
104+
105+
;; CHECK: [fuzz-exec] calling new_active
106+
;; CHECK-NEXT: [trap out of bounds segment access in array.new_elem]
107+
108+
;; CHECK: [fuzz-exec] calling new_active_in_bounds
109+
110+
;; CHECK: [fuzz-exec] calling new_passive
111+
112+
;; CHECK: [fuzz-exec] calling init_active
113+
;; CHECK-NEXT: [trap out of bounds segment access in array.init_elem]
114+
115+
;; CHECK: [fuzz-exec] calling init_active_in_bounds
116+
117+
;; CHECK: [fuzz-exec] calling init_passive
20118
;; CHECK-NEXT: [fuzz-exec] comparing func
119+
;; CHECK-NEXT: [fuzz-exec] comparing init_active
120+
;; CHECK-NEXT: [fuzz-exec] comparing init_active_in_bounds
121+
;; CHECK-NEXT: [fuzz-exec] comparing init_passive
122+
;; CHECK-NEXT: [fuzz-exec] comparing new_active
123+
;; CHECK-NEXT: [fuzz-exec] comparing new_active_in_bounds
124+
;; CHECK-NEXT: [fuzz-exec] comparing new_passive

0 commit comments

Comments
 (0)