Skip to content

Commit

Permalink
Implement dropping of active Element Segments (#6343)
Browse files Browse the repository at this point in the history 
Also rename the existing droppedSegments to droppedDataSegments for clarity.
  • Loading branch information
@kripken
kripken authored Feb 23, 2024
1 parent 2fc33e6 commit 7cb213c
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 10 deletions.
27 changes: 17 additions & 10 deletions src/wasm-interpreter.h
View file
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
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

0 comments on commit 7cb213c

Please sign in to comment.