Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement dropping of active Element Segments #6343

Merged
merged 4 commits into from
Feb 23, 2024
Merged

Implement dropping of active Element Segments #6343

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
268ec4d
work
kripken Feb 23, 2024
123ca12
fix
kripken Feb 23, 2024
5f15b54
work
kripken Feb 23, 2024
00fb470
oops, no need for memory/data in test
kripken Feb 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 17 additions & 10 deletions src/wasm-interpreter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2696,7 +2696,8 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
// stack traces.
std::vector<Name> functionStack;

std::unordered_set<Name> droppedSegments;
std::unordered_set<Name> droppedDataSegments;
std::unordered_set<Name> droppedElementSegments;

struct TableInterfaceInfo {
// The external interface in which the table is defined.
Expand Down Expand Up @@ -2746,6 +2747,8 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
Flow ret = self()->visit(segment->data[i]);
extInterface->tableStore(tableName, offset + i, ret.getSingleValue());
}

droppedElementSegments.insert(segment->name);
});
}

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

if (offsetVal + sizeVal > 0 && droppedSegments.count(curr->segment)) {
if (offsetVal + sizeVal > 0 && droppedDataSegments.count(curr->segment)) {
trap("out of bounds segment access in memory.init");
}
if ((uint64_t)offsetVal + sizeVal > segment->data.size()) {
Expand All @@ -3652,7 +3655,7 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
}
Flow visitDataDrop(DataDrop* curr) {
NOTE_ENTER("DataDrop");
droppedSegments.insert(curr->segment);
droppedDataSegments.insert(curr->segment);
return {};
}
Flow visitMemoryCopy(MemoryCopy* curr) {
Expand Down Expand Up @@ -3768,7 +3771,7 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
const auto& seg = *wasm.getDataSegment(curr->segment);
auto elemBytes = element.getByteSize();
auto end = offset + size * elemBytes;
if ((size != 0ull && droppedSegments.count(curr->segment)) ||
if ((size != 0ull && droppedDataSegments.count(curr->segment)) ||
end > seg.data.size()) {
trap("out of bounds segment access in array.new_data");
}
Expand Down Expand Up @@ -3797,10 +3800,12 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {

const auto& seg = *wasm.getElementSegment(curr->segment);
auto end = offset + size;
// TODO: Handle dropped element segments once we support those.
if (end > seg.data.size()) {
trap("out of bounds segment access in array.new_elem");
}
if (end > 0 && droppedElementSegments.count(curr->segment)) {
trap("out of bounds segment access in array.new_elem");
}
contents.reserve(size);
for (Index i = offset; i < end; ++i) {
auto val = self()->visit(seg.data[i]).getSingleValue();
Expand Down Expand Up @@ -3848,7 +3853,7 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
if (offsetVal + readSize > seg->data.size()) {
trap("out of bounds segment access in array.init_data");
}
if (offsetVal + sizeVal > 0 && droppedSegments.count(curr->segment)) {
if (offsetVal + sizeVal > 0 && droppedDataSegments.count(curr->segment)) {
trap("out of bounds segment access in array.init_data");
}
for (size_t i = 0; i < sizeVal; i++) {
Expand Down Expand Up @@ -3891,11 +3896,13 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
Module& wasm = *self()->getModule();

auto* seg = wasm.getElementSegment(curr->segment);
if ((uint64_t)offsetVal + sizeVal > seg->data.size()) {
trap("out of bounds segment access in array.init");
auto max = (uint64_t)offsetVal + sizeVal;
if (max > seg->data.size()) {
trap("out of bounds segment access in array.init_elem");
}
if (max > 0 && droppedElementSegments.count(curr->segment)) {
trap("out of bounds segment access in array.init_elem");
}
// TODO: Check whether the segment has been dropped once we support
// dropping element segments.
for (size_t i = 0; i < sizeVal; i++) {
// TODO: This is not correct because it does not preserve the identity
// of references in the table! ArrayNew suffers the same problem.
Expand Down
104 changes: 104 additions & 0 deletions test/lit/exec/array.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@
(module
(type $array (array (mut i8)))

(type $array-func (array (mut funcref)))

(table $table 10 10 funcref)

(elem $active (i32.const 0) $func)

(elem $passive $func)

;; CHECK: [fuzz-exec] calling func
;; CHECK-NEXT: [fuzz-exec] note result: func => 1
(func $func (export "func") (result i32)
Expand All @@ -14,7 +22,103 @@
(return (i32.const 2))
)
)

;; CHECK: [fuzz-exec] calling new_active
;; CHECK-NEXT: [trap out of bounds segment access in array.new_elem]
(func $new_active (export "new_active")
;; Even though this is reading 0 items, offset 1 is out of bounds in that
;; dropped element segment, and this traps.
(drop
(array.new_elem $array-func $active
(i32.const 1)
(i32.const 0)
)
)
)

;; CHECK: [fuzz-exec] calling new_active_in_bounds
(func $new_active_in_bounds (export "new_active_in_bounds")
;; Even though this is dropped, we read 0 from offset 0, which is ok.
(drop
(array.new_elem $array-func $active
(i32.const 0)
(i32.const 0)
)
)
)

;; CHECK: [fuzz-exec] calling new_passive
(func $new_passive (export "new_passive")
;; Using the passive segment here works.
(drop
(array.new_elem $array-func $passive
(i32.const 1)
(i32.const 0)
)
)
)

;; CHECK: [fuzz-exec] calling init_active
;; CHECK-NEXT: [trap out of bounds segment access in array.init_elem]
(func $init_active (export "init_active")
;; Even though this is reading 0 items, offset 1 is out of bounds in that
;; dropped element segment, and this traps.
(array.init_elem $array-func $active
(array.new_default $array-func
(i32.const 100)
)
(i32.const 50)
(i32.const 1)
(i32.const 0)
)
)

;; CHECK: [fuzz-exec] calling init_active_in_bounds
(func $init_active_in_bounds (export "init_active_in_bounds")
;; Even though this is dropped, we read 0 from offset 0, which is ok.
(array.init_elem $array-func $active
(array.new_default $array-func
(i32.const 100)
)
(i32.const 50)
(i32.const 0)
(i32.const 0)
)
)

;; CHECK: [fuzz-exec] calling init_passive
(func $init_passive (export "init_passive")
;; This works ok.
(array.init_elem $array-func $passive
(array.new_default $array-func
(i32.const 100)
)
(i32.const 50)
(i32.const 1)
(i32.const 0)
)
)
)
;; CHECK: [fuzz-exec] calling func
;; CHECK-NEXT: [fuzz-exec] note result: func => 1

;; CHECK: [fuzz-exec] calling new_active
;; CHECK-NEXT: [trap out of bounds segment access in array.new_elem]

;; CHECK: [fuzz-exec] calling new_active_in_bounds

;; CHECK: [fuzz-exec] calling new_passive

;; CHECK: [fuzz-exec] calling init_active
;; CHECK-NEXT: [trap out of bounds segment access in array.init_elem]

;; CHECK: [fuzz-exec] calling init_active_in_bounds

;; CHECK: [fuzz-exec] calling init_passive
;; CHECK-NEXT: [fuzz-exec] comparing func
;; CHECK-NEXT: [fuzz-exec] comparing init_active
;; CHECK-NEXT: [fuzz-exec] comparing init_active_in_bounds
;; CHECK-NEXT: [fuzz-exec] comparing init_passive
;; CHECK-NEXT: [fuzz-exec] comparing new_active
;; CHECK-NEXT: [fuzz-exec] comparing new_active_in_bounds
;; CHECK-NEXT: [fuzz-exec] comparing new_passive
Loading