[wasm-simd] Fix simd 128 load and store memarg reading in interpreter

Add a new test SimdLoadStoreLoadMemargOffset to test this, without this fix
this test would have failed.

Bug: v8:9753
Change-Id: I119adda8e3c6c7adb0ad4023298bbce9c0c64a01
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1811457
Commit-Queue: Zhi An Ng <zhin@chromium.org>
Reviewed-by: Deepti Gandluri <gdeepti@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63914}

src/wasm/wasm-interpreter.cc

@@ -1663,9 +1663,15 @@ class ThreadImpl {
 
   template <typename ctype, typename mtype>
   bool ExecuteLoad(Decoder* decoder, InterpreterCode* code, pc_t pc,
-                   int* const len, MachineRepresentation rep) {
-    MemoryAccessImmediate<Decoder::kNoValidate> imm(decoder, code->at(pc),
-                                                    sizeof(ctype));
+                   int* const len, MachineRepresentation rep,
+                   int prefix_len = 0) {
+    // Some opcodes have a prefix byte, and MemoryAccessImmediate assumes that
+    // the memarg is 1 byte from pc. We don't increment pc at the caller,
+    // because we want to keep pc to the start of the operation to keep trap
+    // reporting and tracing accurate, otherwise those will report at the middle
+    // of an opcode.
+    MemoryAccessImmediate<Decoder::kNoValidate> imm(
+        decoder, code->at(pc + prefix_len), sizeof(ctype));
     uint32_t index = Pop().to<uint32_t>();
     Address addr = BoundsCheckMem<mtype>(imm.offset, index);
     if (!addr) {
@@ -1690,9 +1696,15 @@ class ThreadImpl {
 
   template <typename ctype, typename mtype>
   bool ExecuteStore(Decoder* decoder, InterpreterCode* code, pc_t pc,
-                    int* const len, MachineRepresentation rep) {
-    MemoryAccessImmediate<Decoder::kNoValidate> imm(decoder, code->at(pc),
-                                                    sizeof(ctype));
+                    int* const len, MachineRepresentation rep,
+                    int prefix_len = 0) {
+    // Some opcodes have a prefix byte, and MemoryAccessImmediate assumes that
+    // the memarg is 1 byte from pc. We don't increment pc at the caller,
+    // because we want to keep pc to the start of the operation to keep trap
+    // reporting and tracing accurate, otherwise those will report at the middle
+    // of an opcode.
+    MemoryAccessImmediate<Decoder::kNoValidate> imm(
+        decoder, code->at(pc + prefix_len), sizeof(ctype));
     ctype val = Pop().to<ctype>();
 
     uint32_t index = Pop().to<uint32_t>();
@@ -2433,10 +2445,12 @@ class ThreadImpl {
 #undef REPLACE_LANE_CASE
       case kExprS128LoadMem:
         return ExecuteLoad<Simd128, Simd128>(decoder, code, pc, len,
-                                             MachineRepresentation::kSimd128);
+                                             MachineRepresentation::kSimd128,
+                                             /*prefix_len=*/1);
       case kExprS128StoreMem:
         return ExecuteStore<Simd128, Simd128>(decoder, code, pc, len,
-                                              MachineRepresentation::kSimd128);
+                                              MachineRepresentation::kSimd128,
+                                              /*prefix_len=*/1);
 #define SHIFT_CASE(op, name, stype, count, expr) \
   case kExpr##op: {                              \
     uint32_t shift = Pop().to<uint32_t>();       \

test/cctest/wasm/test-run-wasm-simd.cc

@@ -445,8 +445,12 @@ bool ExpectFused(ExecutionTier tier) {
 
 #define WASM_SIMD_LOAD_MEM(index) \
   index, WASM_SIMD_OP(kExprS128LoadMem), ZERO_ALIGNMENT, ZERO_OFFSET
+#define WASM_SIMD_LOAD_MEM_OFFSET(offset, index) \
+  index, WASM_SIMD_OP(kExprS128LoadMem), ZERO_ALIGNMENT, offset
 #define WASM_SIMD_STORE_MEM(index, val) \
   index, val, WASM_SIMD_OP(kExprS128StoreMem), ZERO_ALIGNMENT, ZERO_OFFSET
+#define WASM_SIMD_STORE_MEM_OFFSET(offset, index, val) \
+  index, val, WASM_SIMD_OP(kExprS128StoreMem), ZERO_ALIGNMENT, offset
 
 #define WASM_SIMD_F64x2_QFMA(a, b, c) a, b, c, WASM_SIMD_OP(kExprF64x2Qfma)
 #define WASM_SIMD_F64x2_QFMS(a, b, c) a, b, c, WASM_SIMD_OP(kExprF64x2Qfms)
@@ -3112,6 +3116,29 @@ WASM_SIMD_TEST(SimdLoadStoreLoad) {
   }
 }
 
+WASM_SIMD_TEST(SimdLoadStoreLoadMemargOffset) {
+  WasmRunner<int32_t> r(execution_tier, lower_simd);
+  int32_t* memory =
+      r.builder().AddMemoryElems<int32_t>(kWasmPageSize / sizeof(int32_t));
+  constexpr byte offset_1 = 4;
+  constexpr byte offset_2 = 8;
+  // Load from memory at offset_1, store to offset_2, load from offset_2, and
+  // extract first lane. We use non-zero memarg offsets to test offset decoding.
+  BUILD(
+      r,
+      WASM_SIMD_STORE_MEM_OFFSET(
+          offset_2, WASM_ZERO, WASM_SIMD_LOAD_MEM_OFFSET(offset_1, WASM_ZERO)),
+      WASM_SIMD_I32x4_EXTRACT_LANE(
+          0, WASM_SIMD_LOAD_MEM_OFFSET(offset_2, WASM_ZERO)));
+
+  FOR_INT32_INPUTS(i) {
+    int32_t expected = i;
+    // Index 1 of memory (int32_t) will be bytes 4 to 8.
+    r.builder().WriteMemory(&memory[1], expected);
+    CHECK_EQ(expected, r.Call());
+  }
+}
+
 #if V8_TARGET_ARCH_X64 || V8_TARGET_ARCH_IA32 || V8_TARGET_ARCH_ARM64 || \
     V8_TARGET_ARCH_ARM
 #define WASM_SIMD_ANYTRUE_TEST(format, lanes, max, param_type)                \
@@ -3277,7 +3304,9 @@ WASM_SIMD_TEST_NO_LOWERING(I16x8GtUMixed) {
 #undef WASM_SIMD_I8x16_REPLACE_LANE
 #undef WASM_SIMD_S8x16_SHUFFLE_OP
 #undef WASM_SIMD_LOAD_MEM
+#undef WASM_SIMD_LOAD_MEM_OFFSET
 #undef WASM_SIMD_STORE_MEM
+#undef WASM_SIMD_STORE_MEM_OFFSET
 #undef WASM_SIMD_SELECT_TEST
 #undef WASM_SIMD_NON_CANONICAL_SELECT_TEST
 #undef WASM_SIMD_COMPILED_TEST