From f3488673f2e2ba94683dde07919d433be9a8ce0c Mon Sep 17 00:00:00 2001
From: Clemens Backes <[email protected]>
Date: Wed, 03 Jun 2026 18:13:22 +0200
Subject: [PATCH] [wasm][debug] Fix return address calculation during OSR patching

In UpdateReturnAddresses, the topmost Wasm frame was unconditionally
treated as being at a breakpoint (using kAfterBreakpoint semantics).
However, the frame could also be at a standard call site (e.g. calling
into JS). If there is a dead breakpoint at the same byte offset as the
call site, kAfterBreakpoint incorrectly picks the dead breakpoint's
code offset, leading to a miscalculated return address.

This CL fixes this by checking if there is a WASM_DEBUG_BREAK frame
above the Wasm frame on the stack. If so, we are actually at a
breakpoint. Otherwise, we treat it as a regular call site.

Also consolidate Wasm debugger regression tests into a new directory
test/debugger/regress/wasm/ and update debugger.status accordingly.

[email protected]

Fixed: 517316174
Change-Id: Ib4718a863d5f358a78c500dec6af4a3ebacca981
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7894740
Commit-Queue: Clemens Backes <[email protected]>
Reviewed-by: Jakob Kummerow <[email protected]>
Cr-Commit-Position: refs/heads/main@{#107766}
---

diff --git a/src/wasm/wasm-debug.cc b/src/wasm/wasm-debug.cc
index 1ae12eb..bb8d6f6 100644
--- a/src/wasm/wasm-debug.cc
+++ b/src/wasm/wasm-debug.cc
@@ -725,37 +725,24 @@
   // code. The frame layout itself should be independent of breakpoints.
   void UpdateReturnAddresses(Isolate* isolate, WasmCode* new_code,
                              StackFrameId stepping_frame) {
-    auto matches = [new_code](WasmFrame* frame) {
-      return frame->native_module() == new_code->native_module() &&
-             FrameSummary::GetTop(frame).AsWasm().function_index() ==
-                 static_cast<uint32_t>(new_code->index()) &&
-             frame->wasm_code()->is_liftoff();
-    };
-
-    // 1. Find and handle the first Wasm frame (potential breakpoint resume).
-    DebuggableStackFrameIterator it(isolate);
-    while (!it.done() && !it.is_wasm()) it.Advance();
-
-    if (!it.done()) {
-      WasmFrame* frame = WasmFrame::cast(it.frame());
-      // We still need the flooded function for stepping.
-      if (frame->id() != stepping_frame &&
-#if V8_ENABLE_DRUMBRAKE
-          // TODO([email protected]) - Implement for Wasm interpreter.
-          !it.is_wasm_interpreter_entry() &&
-#endif
-          matches(frame)) {
-        UpdateReturnAddress(frame, new_code, kAfterBreakpoint);
-      }
-      it.Advance();
-    }
-
-    // 2. Handle all remaining frames (always at call sites).
+    StackFrameIterator it(isolate);
     for (; !it.done(); it.Advance()) {
-      if (!it.is_wasm()) continue;
+      bool at_breakpoint = it.frame()->is_wasm_debug_break();
+      if (at_breakpoint) {
+        it.Advance();
+        CHECK(!it.done());
+      }
+      if (!it.frame()->is_wasm()) continue;
+#if V8_ENABLE_DRUMBRAKE
+      if (it.frame()->is_wasm_interpreter_entry()) continue;
+#endif
       WasmFrame* frame = WasmFrame::cast(it.frame());
-      if (!matches(frame)) continue;
-      UpdateReturnAddress(frame, new_code, kAfterWasmCall);
+      if (frame->native_module() != new_code->native_module()) continue;
+      WasmCode* code = frame->wasm_code();
+      if (!code->is_liftoff() || code->index() != new_code->index()) continue;
+      if (frame->id() == stepping_frame) continue;
+      UpdateReturnAddress(frame, new_code,
+                          at_breakpoint ? kAfterBreakpoint : kAfterWasmCall);
     }
   }
 
diff --git a/test/debugger/debugger.status b/test/debugger/debugger.status
index 3f1459c..492e1e7 100644
--- a/test/debugger/debugger.status
+++ b/test/debugger/debugger.status
@@ -131,8 +131,7 @@
 # TODO(v8:7777): Change this once wasm is supported in jitless mode.
 ['not has_webassembly or variant == jitless', {
   'debug/wasm/*': [SKIP],
-  'regress/regress-crbug-840288': [SKIP],
-  'regress/regress-crbug-1032042': [SKIP],
+  'regress/wasm/*': [SKIP],
 }],  # not has_webassembly or variant == jitless
 
 ##############################################################################
diff --git a/test/debugger/regress/regress-crbug-1032042.js b/test/debugger/regress/regress-crbug-1032042.js
deleted file mode 100644
index b92735f..0000000
--- a/test/debugger/regress/regress-crbug-1032042.js
+++ /dev/null
@@ -1,44 +0,0 @@
-// Copyright 2019 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 Debug = new DebugWrapper();
-Debug.enable();
-
-// Record the ID of the first script reported. This is to ignore
-// the (now deprecated) fake scripts that are generated for every
-// Wasm module.
-let scriptId;
-Debug.setListener((eventType, execState, eventData, data) => {
-    assertEquals(Debug.DebugEvent.AfterCompile, eventType);
-    if (scriptId === undefined) scriptId = eventData.scriptId;
-});
-
-// Create a simple Wasm script, which will be caught by the event listener.
-const builder = new WasmModuleBuilder();
-builder.addFunction('sub', kSig_i_ii)
-// input is 2 args of type int and output is int
-.addBody([
-  kExprLocalGet, 0, // local.get i0
-  kExprLocalGet, 1, // local.get i1
-  kExprI32Sub]) // i32.sub i0 i1
-.exportFunc();
-const instance = builder.instantiate();
-
-// By now we should have recorded the ID of the Wasm script above.
-assertNotEquals(undefined, scriptId);
-
-// Disable and re-enable the Debugger and collect the reported
-// script IDs.
-const scriptIds = new Set();
-Debug.disable();
-Debug.setListener((eventType, execState, eventData, data) => {
-    assertEquals(Debug.DebugEvent.AfterCompile, eventType);
-    scriptIds.add(eventData.scriptId);
-});
-Debug.enable();
-
-// Make sure the Wasm script was reported.
-assertTrue(scriptIds.has(scriptId));
diff --git a/test/debugger/regress/regress-crbug-840288.js b/test/debugger/regress/regress-crbug-840288.js
deleted file mode 100644
index 6d34928..0000000
--- a/test/debugger/regress/regress-crbug-840288.js
+++ /dev/null
@@ -1,27 +0,0 @@
-// Copyright 2018 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");
-
-Debug = debug.Debug
-
-function listener(event, exec_state, event_data, data) {
-  if (event == Debug.DebugEvent.AfterCompile) {
-    // The actual source doesn't matter, just don't crash.
-    var source = event_data.script().source();
-    // Source will be empty for the script representing the entire module,
-    // disassembly for the script representing just the function.
-    assertTrue(source == "func $main\nend\n" || source == "");
-  }
-};
-
-// Add the debug event listener.
-Debug.setListener(listener);
-
-var builder = new WasmModuleBuilder();
-builder.addFunction('main', kSig_v_v).addBody([]).exportFunc();
-var promise = WebAssembly.compile(builder.toBuffer());
-
-// Clear the debug listener only after the event fired.
-promise.then(() => Debug.setListener(null), assertUnreachable);
diff --git a/test/debugger/regress/wasm/regress-517316174.js b/test/debugger/regress/wasm/regress-517316174.js
new file mode 100644
index 0000000..23c398d
--- /dev/null
+++ b/test/debugger/regress/wasm/regress-517316174.js
@@ -0,0 +1,58 @@
+// Copyright 2026 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();
+const num_params = 100;
+// Use a large signature to maximize the call instruction size.
+const sig_idx = builder.addType(

diff --git a/test/debugger/regress/regress-crbug-1032042.js b/test/debugger/regress/regress-crbug-1032042.js
deleted file mode 100644
index b92735f..0000000
--- a/test/debugger/regress/regress-crbug-1032042.js
+++ /dev/null
@@ -1,44 +0,0 @@
-// Copyright 2019 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 Debug = new DebugWrapper();
-Debug.enable();
-
-// Record the ID of the first script reported. This is to ignore
-// the (now deprecated) fake scripts that are generated for every
-// Wasm module.
-let scriptId;
-Debug.setListener((eventType, execState, eventData, data) => {
-    assertEquals(Debug.DebugEvent.AfterCompile, eventType);
-    if (scriptId === undefined) scriptId = eventData.scriptId;
-});
-
-// Create a simple Wasm script, which will be caught by the event listener.
-const builder = new WasmModuleBuilder();
-builder.addFunction('sub', kSig_i_ii)
-// input is 2 args of type int and output is int
-.addBody([
-  kExprLocalGet, 0, // local.get i0
-  kExprLocalGet, 1, // local.get i1
-  kExprI32Sub]) // i32.sub i0 i1
-.exportFunc();
-const instance = builder.instantiate();
-
-// By now we should have recorded the ID of the Wasm script above.
-assertNotEquals(undefined, scriptId);
-
-// Disable and re-enable the Debugger and collect the reported
-// script IDs.
-const scriptIds = new Set();
-Debug.disable();
-Debug.setListener((eventType, execState, eventData, data) => {
-    assertEquals(Debug.DebugEvent.AfterCompile, eventType);
-    scriptIds.add(eventData.scriptId);
-});
-Debug.enable();
-
-// Make sure the Wasm script was reported.
-assertTrue(scriptIds.has(scriptId));
diff --git a/test/debugger/regress/regress-crbug-840288.js b/test/debugger/regress/regress-crbug-840288.js
deleted file mode 100644
index 6d34928..0000000
--- a/test/debugger/regress/regress-crbug-840288.js
+++ /dev/null
@@ -1,27 +0,0 @@
-// Copyright 2018 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");
-
-Debug = debug.Debug
-
-function listener(event, exec_state, event_data, data) {
-  if (event == Debug.DebugEvent.AfterCompile) {
-    // The actual source doesn't matter, just don't crash.
-    var source = event_data.script().source();
-    // Source will be empty for the script representing the entire module,
-    // disassembly for the script representing just the function.
-    assertTrue(source == "func $main\nend\n" || source == "");
-  }
-};
-
-// Add the debug event listener.
-Debug.setListener(listener);
-
-var builder = new WasmModuleBuilder();
-builder.addFunction('main', kSig_v_v).addBody([]).exportFunc();
-var promise = WebAssembly.compile(builder.toBuffer());
-
-// Clear the debug listener only after the event fired.
-promise.then(() => Debug.setListener(null), assertUnreachable);
diff --git a/test/debugger/regress/wasm/regress-517316174.js b/test/debugger/regress/wasm/regress-517316174.js
new file mode 100644
index 0000000..23c398d
--- /dev/null
+++ b/test/debugger/regress/wasm/regress-517316174.js
@@ -0,0 +1,58 @@
+// Copyright 2026 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();
+const num_params = 100;
+// Use a large signature to maximize the call instruction size.
+const sig_idx = builder.addType(
+    makeSig(new Array(num_params).fill(kWasmI64), []));
+const imp_idx = builder.addImport('m', 'js', sig_idx);
+builder.addTable(kWasmFuncRef, 1);
+builder.addActiveElementSegment(0, [kExprI32Const, 0], [imp_idx]);
+
+const func_main = builder.addFunction('main', kSig_v_v)
+    .addBody([
+      kExprNop, // Offset 1
+      ...new Array(num_params).fill([kExprI64Const, 0]).flat(),
+      kExprI32Const, 0, // table index
+      kExprCallIndirect, sig_idx, 0,
+      kExprI64Const, 0,
+      kExprDrop,
+      kExprI64Const, 0,
+      kExprDrop,
+    ])
+    .exportFunc();
+
+const instance = builder.instantiate({
+  m: {
+    js: (...args) => {
+      if (globalThis.recompiled) return;
+      globalThis.recompiled = true;
+
+      // Trigger UpdateReturnAddresses(isolate, ..., kAfterBreakpoint)
+      // for the topmost Wasm frame ('main') which is currently at the call
+      // site.
+      debug.Debug.clearBreakPoint(bp_call);
+
+      // Set a new breakpoint to force recompilation.
+      debug.Debug.setBreakPoint(instance.exports.main, 0, 0);
+    }
+  }
+});
+
+// Calculate the offset of the call_indirect instruction.
+// 1 (nop) + 100 * 2 (i64.const 0) + 2 (i32.const 0) = 203.
+const call_offset = 1 + num_params * 2 + 2;
+
+// Breakpoint 1: At nop (offset 1). This ensures 'UpdateReturnAddresses'
+// has some existing state to work with and triggers the buggy path.
+debug.Debug.setBreakPoint(instance.exports.main, 0, 1);
+
+// Breakpoint 2: Specifically at the call site.
+const bp_call =
+    debug.Debug.setBreakPoint(instance.exports.main, 0, call_offset);
+
+instance.exports.main();
diff --git a/test/debugger/regress/wasm/regress-crbug-1032042.js b/test/debugger/regress/wasm/regress-crbug-1032042.js
new file mode 100644
index 0000000..b92735f
--- /dev/null
+++ b/test/debugger/regress/wasm/regress-crbug-1032042.js
@@ -0,0 +1,44 @@
+// Copyright 2019 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 Debug = new DebugWrapper();
+Debug.enable();
+
+// Record the ID of the first script reported. This is to ignore
+// the (now deprecated) fake scripts that are generated for every
+// Wasm module.
+let scriptId;
+Debug.setListener((eventType, execState, eventData, data) => {
+    assertEquals(Debug.DebugEvent.AfterCompile, eventType);
+    if (scriptId === undefined) scriptId = eventData.scriptId;
+});
+
+// Create a simple Wasm script, which will be caught by the event listener.
+const builder = new WasmModuleBuilder();
+builder.addFunction('sub', kSig_i_ii)
+// input is 2 args of type int and output is int
+.addBody([
+  kExprLocalGet, 0, // local.get i0
+  kExprLocalGet, 1, // local.get i1
+  kExprI32Sub]) // i32.sub i0 i1
+.exportFunc();
+const instance = builder.instantiate();
+
+// By now we should have recorded the ID of the Wasm script above.
+assertNotEquals(undefined, scriptId);
+
+// Disable and re-enable the Debugger and collect the reported
+// script IDs.
+const scriptIds = new Set();
+Debug.disable();
+Debug.setListener((eventType, execState, eventData, data) => {
+    assertEquals(Debug.DebugEvent.AfterCompile, eventType);
+    scriptIds.add(eventData.scriptId);
+});
+Debug.enable();
+
+// Make sure the Wasm script was reported.
+assertTrue(scriptIds.has(scriptId));
diff --git a/test/debugger/regress/wasm/regress-crbug-840288.js b/test/debugger/regress/wasm/regress-crbug-840288.js
new file mode 100644
index 0000000..6d34928
--- /dev/null
+++ b/test/debugger/regress/wasm/regress-crbug-840288.js
@@ -0,0 +1,27 @@
+// Copyright 2018 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");
+
+Debug = debug.Debug
+
+function listener(event, exec_state, event_data, data) {
+  if (event == Debug.DebugEvent.AfterCompile) {
+    // The actual source doesn't matter, just don't crash.
+    var source = event_data.script().source();
+    // Source will be empty for the script representing the entire module,
+    // disassembly for the script representing just the function.
+    assertTrue(source == "func $main\nend\n" || source == "");
+  }
+};
+
+// Add the debug event listener.
+Debug.setListener(listener);
+
+var builder = new WasmModuleBuilder();
+builder.addFunction('main', kSig_v_v).addBody([]).exportFunc();
+var promise = WebAssembly.compile(builder.toBuffer());
+
+// Clear the debug listener only after the event fired.
+promise.then(() => Debug.setListener(null), assertUnreachable);