Skip to content

Commit

Permalink
Fix bug in SIMDLoadStoreLane interpretation (#7237)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tlively authored Jan 23, 2025
1 parent 7074d87 commit ff423ae
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 9 deletions.
18 changes: 9 additions & 9 deletions src/wasm-interpreter.h
Original file line number Diff line number Diff line change
Expand Up @@ -3812,20 +3812,20 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
}
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: {
Expand Down
42 changes: 42 additions & 0 deletions test/lit/exec/simd-load-lane-oob.wast
Original file line number Diff line number Diff line change
@@ -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 => 1
(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 => 1
;; CHECK-NEXT: [fuzz-exec] comparing get
;; CHECK-NEXT: [fuzz-exec] comparing oob

0 comments on commit ff423ae

Please sign in to comment.