[wasm][liftoff][arm64] Fix DropExceptionValueAtOffset

jakobkummerow · V8 LUCI CQ · commit 910cb91733dc · 2024-06-04T15:44:00.000Z
We cannot exit the iteration early, we must update all entries in the cache state. Fixed: 343748812 Change-Id: I8353acb7bd0edc4b979db92e44d24cb9028fd92b Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5596273 Reviewed-by: Clemens Backes <clemensb@chromium.org> Commit-Queue: Clemens Backes <clemensb@chromium.org> Auto-Submit: Jakob Kummerow <jkummerow@chromium.org> Commit-Queue: Jakob Kummerow <jkummerow@chromium.org> Cr-Commit-Position: refs/heads/main@{#94244}
diff --git a/src/wasm/baseline/liftoff-assembler.cc b/src/wasm/baseline/liftoff-assembler.cc
@@ -435,12 +435,13 @@ void LiftoffAssembler::DropExceptionValueAtOffset(int offset) {
        slot != end; ++slot) {
     *slot = *(slot + 1);
     stack_offset = NextSpillOffset(slot->kind(), stack_offset);
-    // Padding could allow us to exit early.
-    if (slot->offset() == stack_offset) break;
-    if (slot->is_stack()) {
-      MoveStackValue(stack_offset, slot->offset(), slot->kind());
+    // Padding could cause some spill offsets to remain the same.
+    if (slot->offset() != stack_offset) {
+      if (slot->is_stack()) {
+        MoveStackValue(stack_offset, slot->offset(), slot->kind());
+      }
+      slot->set_offset(stack_offset);
     }
-    slot->set_offset(stack_offset);
   }
   cache_state_.stack_state.pop_back();
 }
diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status
@@ -1773,6 +1773,7 @@
   'regress/wasm/regress-341875188': [SKIP],
   'regress/wasm/regress-344484969': [SKIP],
   'regress/wasm/regress-341947685': [SKIP],
+  'regress/wasm/regress-343748812': [SKIP],
   'regress/wasm/regress-crbug-1338980': [SKIP],
   'regress/wasm/regress-crbug-1355070': [SKIP],
   'regress/wasm/regress-crbug-1356718': [SKIP],
diff --git a/test/mjsunit/regress/wasm/regress-343748812.js b/test/mjsunit/regress/wasm/regress-343748812.js
@@ -0,0 +1,30 @@
+// Copyright 2024 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
+
+const builder = new WasmModuleBuilder();
+let $sig0 = builder.addType(kSig_v_v);
+let $sig7 = builder.addType(
+    makeSig([], [ kWasmExternRef, kWasmS128, kWasmExternRef ]));
+let $func0 = builder.addImport('imports', 'func0', $sig0);
+builder.addFunction("main", $sig0).exportFunc()
+  .addLocals(kWasmExternRef, 3)
+  .addBody([
+    kExprTry, $sig7,
+      kExprCallFunction, $func0,
+      kExprUnreachable,
+    kExprCatchAll,
+      kExprRefNull, kExternRefCode,
+      ...wasmS128Const([0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]),
+      kExprRefNull, kExternRefCode,
+    kExprEnd,
+    kExprDrop,
+    kExprDrop,
+    kExprDrop,
+  ]);
+
+var instance = builder.instantiate({'imports': { 'func0': () => {} }});
+
+assertThrows(instance.exports.main, WebAssembly.RuntimeError, /unreachable/);

Original file line number	Diff line number	Diff line change
`@@ -435,12 +435,13 @@ void LiftoffAssembler::DropExceptionValueAtOffset(int offset) {`
`435`	`435`	`slot != end; ++slot) {`
`436`	`436`	`slot = (slot + 1);`
`437`	`437`	`stack_offset = NextSpillOffset(slot->kind(), stack_offset);`
`438`		`- // Padding could allow us to exit early.`
`439`		`- if (slot->offset() == stack_offset) break;`
`440`		`- if (slot->is_stack()) {`
`441`		`- MoveStackValue(stack_offset, slot->offset(), slot->kind());`
	`438`	`+ // Padding could cause some spill offsets to remain the same.`
	`439`	`+ if (slot->offset() != stack_offset) {`
	`440`	`+ if (slot->is_stack()) {`
	`441`	`+ MoveStackValue(stack_offset, slot->offset(), slot->kind());`
	`442`	`+ }`
	`443`	`+ slot->set_offset(stack_offset);`
`442`	`444`	`}`
`443`		`- slot->set_offset(stack_offset);`
`444`	`445`	`}`
`445`	`446`	`cache_state_.stack_state.pop_back();`
`446`	`447`	`}`