From fe0e84f04bbdf11028f1757ab98b928f21904e93 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Wed, 22 Jan 2025 17:48:33 -0800 Subject: [PATCH 1/2] Fix bug in SIMDLoadStoreLane interpretation The interpreter incorrectly trapped on OOB addresses before evaluating the vector operand. This bug became visible when the vector operand had side effects. Reorder the code to fix the problem. --- src/wasm-interpreter.h | 18 ++++++------ test/lit/exec/simd-load-lane-oob.wast | 42 +++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 9 deletions(-) create mode 100644 test/lit/exec/simd-load-lane-oob.wast diff --git a/src/wasm-interpreter.h b/src/wasm-interpreter.h index d459ffdf5fe..186245ab004 100644 --- a/src/wasm-interpreter.h +++ b/src/wasm-interpreter.h @@ -3812,20 +3812,20 @@ class ModuleRunnerBase : public ExpressionRunner { } Flow visitSIMDLoadStoreLane(SIMDLoadStoreLane* curr) { NOTE_ENTER("SIMDLoadStoreLane"); - Flow flow = self()->visit(curr->ptr); - if (flow.breaking()) { - return flow; + Flow ptrFlow = self()->visit(curr->ptr); + if (ptrFlow.breaking()) { + return ptrFlow; } NOTE_EVAL1(flow); + Flow vecFlow = self()->visit(curr->vec); + if (vecFlow.breaking()) { + return vecFlow; + } auto info = getMemoryInstanceInfo(curr->memory); auto memorySize = info.instance->getMemorySize(info.name); Address addr = info.instance->getFinalAddress( - curr, flow.getSingleValue(), curr->getMemBytes(), memorySize); - flow = self()->visit(curr->vec); - if (flow.breaking()) { - return flow; - } - Literal vec = flow.getSingleValue(); + curr, ptrFlow.getSingleValue(), curr->getMemBytes(), memorySize); + Literal vec = vecFlow.getSingleValue(); switch (curr->op) { case Load8LaneVec128: case Store8LaneVec128: { diff --git a/test/lit/exec/simd-load-lane-oob.wast b/test/lit/exec/simd-load-lane-oob.wast new file mode 100644 index 00000000000..a30ffb91edb --- /dev/null +++ b/test/lit/exec/simd-load-lane-oob.wast @@ -0,0 +1,42 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py --output=fuzz-exec and should not be edited. + +;; RUN: wasm-opt %s -all --fuzz-exec -q -o /dev/null 2>&1 | filecheck %s + +;; Regression test for a bug where the vector operand to SIMDLoadStoreLane was +;; not evaluated if the preceding ptr operand was OOB. + +(module + (memory $mem 1 1) + (global $g (mut i32) (i32.const 0)) + + ;; CHECK: [fuzz-exec] calling oob + ;; CHECK-NEXT: [trap final > memory: 18446744073709551615 > 65536] + (func $oob (export "oob") + (drop + ;; This should trap, but not until after setting the global. + (v128.load64_lane 0 + (i32.const -1) + (block (result v128) + (global.set $g + (i32.const 1) + ) + (v128.const i32x4 0 0 0 0) + ) + ) + ) + ) + + ;; CHECK: [fuzz-exec] calling get + ;; CHECK-NEXT: [fuzz-exec] note result: get => 0 + (func $get (export "get") (result i32) + ;; This should be 1 + (global.get $g) + ) +) +;; CHECK: [fuzz-exec] calling oob +;; CHECK-NEXT: [trap final > memory: 18446744073709551615 > 65536] + +;; CHECK: [fuzz-exec] calling get +;; CHECK-NEXT: [fuzz-exec] note result: get => 0 +;; CHECK-NEXT: [fuzz-exec] comparing get +;; CHECK-NEXT: [fuzz-exec] comparing oob From aa7de7fffc5d688b017bf28cb6a1781cf40dfa8b Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Wed, 22 Jan 2025 17:52:07 -0800 Subject: [PATCH 2/2] fix test output --- test/lit/exec/simd-load-lane-oob.wast | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/lit/exec/simd-load-lane-oob.wast b/test/lit/exec/simd-load-lane-oob.wast index a30ffb91edb..2d0ed4d3af3 100644 --- a/test/lit/exec/simd-load-lane-oob.wast +++ b/test/lit/exec/simd-load-lane-oob.wast @@ -27,7 +27,7 @@ ) ;; CHECK: [fuzz-exec] calling get - ;; CHECK-NEXT: [fuzz-exec] note result: get => 0 + ;; CHECK-NEXT: [fuzz-exec] note result: get => 1 (func $get (export "get") (result i32) ;; This should be 1 (global.get $g) @@ -37,6 +37,6 @@ ;; CHECK-NEXT: [trap final > memory: 18446744073709551615 > 65536] ;; CHECK: [fuzz-exec] calling get -;; CHECK-NEXT: [fuzz-exec] note result: get => 0 +;; CHECK-NEXT: [fuzz-exec] note result: get => 1 ;; CHECK-NEXT: [fuzz-exec] comparing get ;; CHECK-NEXT: [fuzz-exec] comparing oob