From 268ec4dd267d602606ee90f4b7e72df3d6496d54 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 23 Feb 2024 13:21:03 -0800 Subject: [PATCH 1/4] work --- src/wasm-interpreter.h | 19 +++++++----- test/lit/exec/array.wast | 63 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 7 deletions(-) diff --git a/src/wasm-interpreter.h b/src/wasm-interpreter.h index 409bec9d82f..229b043240c 100644 --- a/src/wasm-interpreter.h +++ b/src/wasm-interpreter.h @@ -2696,7 +2696,8 @@ class ModuleRunnerBase : public ExpressionRunner { // stack traces. std::vector functionStack; - std::unordered_set droppedSegments; + std::unordered_set droppedDataSegments; + std::unordered_set droppedElementSegments; struct TableInterfaceInfo { // The external interface in which the table is defined. @@ -2746,6 +2747,8 @@ class ModuleRunnerBase : public ExpressionRunner { Flow ret = self()->visit(segment->data[i]); extInterface->tableStore(tableName, offset + i, ret.getSingleValue()); } + + droppedElementSegments.insert(segment->name); }); } @@ -3630,7 +3633,7 @@ class ModuleRunnerBase : public ExpressionRunner { 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()) { @@ -3652,7 +3655,7 @@ class ModuleRunnerBase : public ExpressionRunner { } Flow visitDataDrop(DataDrop* curr) { NOTE_ENTER("DataDrop"); - droppedSegments.insert(curr->segment); + droppedDataSegments.insert(curr->segment); return {}; } Flow visitMemoryCopy(MemoryCopy* curr) { @@ -3768,7 +3771,7 @@ class ModuleRunnerBase : public ExpressionRunner { 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"); } @@ -3797,9 +3800,11 @@ class ModuleRunnerBase : public ExpressionRunner { 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"); + trap("out of bounds segment access in array.init_elem"); + } + if (end > 0 && droppedElementSegments.count(curr->segment)) { + trap("out of bounds segment access in array.init_elem"); } contents.reserve(size); for (Index i = offset; i < end; ++i) { @@ -3848,7 +3853,7 @@ class ModuleRunnerBase : public ExpressionRunner { 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++) { diff --git a/test/lit/exec/array.wast b/test/lit/exec/array.wast index fb6a8b34616..d389d2b7110 100644 --- a/test/lit/exec/array.wast +++ b/test/lit/exec/array.wast @@ -5,6 +5,18 @@ (module (type $array (array (mut i8))) + (type $array-func (array funcref)) + + (memory $mem 10 10) + + (data $data (i32.const 0) "a") + + (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) @@ -14,7 +26,58 @@ (return (i32.const 2)) ) ) + + ;; CHECK: [fuzz-exec] calling new_active + ;; CHECK-NEXT: waka 1 : 1 : 1 + ;; CHECK-NEXT: [trap out of bounds segment access in array.init_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 + ;; CHECK-NEXT: waka 0 : 1 : 1 + (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 + ;; CHECK-NEXT: waka 1 : 1 : 0 + (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 func ;; CHECK-NEXT: [fuzz-exec] note result: func => 1 + +;; CHECK: [fuzz-exec] calling new_active +;; CHECK-NEXT: waka 1 : 1 : 1 +;; CHECK-NEXT: [trap out of bounds segment access in array.init_elem] + +;; CHECK: [fuzz-exec] calling new_active_in_bounds +;; CHECK-NEXT: waka 0 : 1 : 1 + +;; CHECK: [fuzz-exec] calling new_passive +;; CHECK-NEXT: waka 1 : 1 : 0 ;; CHECK-NEXT: [fuzz-exec] comparing func +;; CHECK-NEXT: [fuzz-exec] comparing new_active +;; CHECK-NEXT: [fuzz-exec] comparing new_active_in_bounds +;; CHECK-NEXT: [fuzz-exec] comparing new_passive From 123ca1208ccaae66cf5478dc9b04768061db78c7 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 23 Feb 2024 13:23:35 -0800 Subject: [PATCH 2/4] fix --- src/wasm-interpreter.h | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/wasm-interpreter.h b/src/wasm-interpreter.h index 229b043240c..f1b081e5103 100644 --- a/src/wasm-interpreter.h +++ b/src/wasm-interpreter.h @@ -3801,10 +3801,10 @@ class ModuleRunnerBase : public ExpressionRunner { const auto& seg = *wasm.getElementSegment(curr->segment); auto end = offset + size; if (end > seg.data.size()) { - trap("out of bounds segment access in array.init_elem"); + 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.init_elem"); + trap("out of bounds segment access in array.new_elem"); } contents.reserve(size); for (Index i = offset; i < end; ++i) { @@ -3896,11 +3896,13 @@ class ModuleRunnerBase : public ExpressionRunner { 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. From 5f15b5407d890420547b2db9a8191db3db0281b3 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 23 Feb 2024 13:28:55 -0800 Subject: [PATCH 3/4] work --- test/lit/exec/array.wast | 63 ++++++++++++++++++++++++++++++++++------ 1 file changed, 54 insertions(+), 9 deletions(-) diff --git a/test/lit/exec/array.wast b/test/lit/exec/array.wast index d389d2b7110..f9632505262 100644 --- a/test/lit/exec/array.wast +++ b/test/lit/exec/array.wast @@ -5,7 +5,7 @@ (module (type $array (array (mut i8))) - (type $array-func (array funcref)) + (type $array-func (array (mut funcref))) (memory $mem 10 10) @@ -28,8 +28,7 @@ ) ;; CHECK: [fuzz-exec] calling new_active - ;; CHECK-NEXT: waka 1 : 1 : 1 - ;; CHECK-NEXT: [trap out of bounds segment access in array.init_elem] + ;; 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. @@ -42,7 +41,6 @@ ) ;; CHECK: [fuzz-exec] calling new_active_in_bounds - ;; CHECK-NEXT: waka 0 : 1 : 1 (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 @@ -54,7 +52,6 @@ ) ;; CHECK: [fuzz-exec] calling new_passive - ;; CHECK-NEXT: waka 1 : 1 : 0 (func $new_passive (export "new_passive") ;; Using the passive segment here works. (drop @@ -64,20 +61,68 @@ ) ) ) + + ;; 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: waka 1 : 1 : 1 -;; CHECK-NEXT: [trap out of bounds segment access in array.init_elem] +;; CHECK-NEXT: [trap out of bounds segment access in array.new_elem] ;; CHECK: [fuzz-exec] calling new_active_in_bounds -;; CHECK-NEXT: waka 0 : 1 : 1 ;; CHECK: [fuzz-exec] calling new_passive -;; CHECK-NEXT: waka 1 : 1 : 0 + +;; 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 From 00fb47055053102b3238dfa740cbd82fb8d3aff8 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 23 Feb 2024 13:32:27 -0800 Subject: [PATCH 4/4] oops, no need for memory/data in test --- test/lit/exec/array.wast | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/lit/exec/array.wast b/test/lit/exec/array.wast index f9632505262..ff10c555ce8 100644 --- a/test/lit/exec/array.wast +++ b/test/lit/exec/array.wast @@ -7,10 +7,6 @@ (type $array-func (array (mut funcref))) - (memory $mem 10 10) - - (data $data (i32.const 0) "a") - (table $table 10 10 funcref) (elem $active (i32.const 0) $func)