[liftoff][debug] Update stack on new Liftoff code

After compiling a function with a different set of breakpoints, update
return addresses on the stack so that execution resumes in the new
code.
This allows new breakpoints to take effect immediately, which is the
expected behavior and a prerequisite for stepping.

R=clemensb@chromium.org

Bug: v8:10147
Change-Id: I67eb3b4ce23a1f3b0519935447f8b847ec888ead
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2064218Reviewed-by: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66351}

src/wasm/wasm-debug.cc

@@ -443,6 +443,58 @@ class InterpreterHandle {
   DISALLOW_COPY_AND_ASSIGN(InterpreterHandle);
 };
 
+Address FindNewPC(int offset, WasmCode* old_code, WasmCode* new_code) {
+  Vector<const uint8_t> old_pos_table = old_code->source_positions();
+  Vector<const uint8_t> new_pos_table = new_code->source_positions();
+
+  // Find the source position in the old code.
+  int old_source_pos = -1;
+  for (SourcePositionTableIterator old_it(old_pos_table); !old_it.done();
+       old_it.Advance()) {
+    if (old_it.code_offset() == offset) {
+      old_source_pos = old_it.source_position().ScriptOffset();
+      break;
+    }
+  }
+  DCHECK_LE(0, old_source_pos);
+
+  // Find the matching source position in the new code.
+  SourcePositionTableIterator new_it(new_pos_table);
+  while (!new_it.done() &&
+         new_it.source_position().ScriptOffset() != old_source_pos) {
+    new_it.Advance();
+  }
+  DCHECK(!new_it.done());
+
+  // Each call instruction generates two source positions with the same source
+  // offset: one for the address of the call instruction and one for the
+  // return address. Skip the first one.
+  new_it.Advance();
+  DCHECK(!new_it.done());
+  DCHECK_EQ(new_it.source_position().ScriptOffset(), old_source_pos);
+  return new_code->instruction_start() + new_it.code_offset();
+}
+
+// After installing a Liftoff code object with a different set of breakpoints,
+// update return addresses on the stack so that execution resumes in the new
+// code. The frame layout itself should be independent of breakpoints.
+// TODO(thibaudm): update other threads as well.
+void UpdateReturnAddresses(Isolate* isolate, WasmCode* new_code) {
+  DCHECK(new_code->is_liftoff());
+  for (StackTraceFrameIterator it(isolate); !it.done(); it.Advance()) {
+    if (!it.is_wasm()) continue;
+    WasmCompiledFrame* frame = WasmCompiledFrame::cast(it.frame());
+    if (frame->native_module() != new_code->native_module()) continue;
+    if (frame->function_index() != new_code->index()) continue;
+    WasmCode* old_code = frame->wasm_code();
+    if (!old_code->is_liftoff()) return;
+    int offset = static_cast<int>(frame->pc() - old_code->instruction_start());
+    Address new_pc = FindNewPC(offset, old_code, new_code);
+    PointerAuthentication::ReplacePC(frame->pc_address(), new_pc,
+                                     kSystemPointerSize);
+  }
+}
+
 }  // namespace
 
 Handle<JSObject> GetGlobalScopeObject(Handle<WasmInstanceObject> instance) {
@@ -573,7 +625,7 @@ class DebugInfoImpl {
     return local_names_->GetName(func_index, local_index);
   }
 
-  void SetBreakpoint(int func_index, int offset) {
+  void SetBreakpoint(int func_index, int offset, Isolate* current_isolate) {
     // Hold the mutex while setting the breakpoint. This guards against multiple
     // isolates setting breakpoints at the same time. We don't really support
     // that scenario yet, but concurrently compiling and installing different
@@ -611,8 +663,7 @@ class DebugInfoImpl {
     DCHECK(added);
     USE(added);
 
-    // TODO(clemensb): OSR active frames on the stack (on all threads).
-    USE(new_code);
+    UpdateReturnAddresses(current_isolate, new_code);
   }
 
   void RemoveDebugSideTables(Vector<WasmCode* const> codes) {
@@ -707,8 +758,9 @@ WireBytesRef DebugInfo::GetLocalName(int func_index, int local_index) {
   return impl_->GetLocalName(func_index, local_index);
 }
 
-void DebugInfo::SetBreakpoint(int func_index, int offset) {
-  impl_->SetBreakpoint(func_index, offset);
+void DebugInfo::SetBreakpoint(int func_index, int offset,
+                              Isolate* current_isolate) {
+  impl_->SetBreakpoint(func_index, offset, current_isolate);
 }
 
 void DebugInfo::RemoveDebugSideTables(Vector<WasmCode* const> code) {
@@ -1000,7 +1052,7 @@ bool WasmScript::SetBreakPointForFunction(Handle<Script> script, int func_index,
                                   break_point);
 
   if (FLAG_debug_in_liftoff) {
-    native_module->GetDebugInfo()->SetBreakpoint(func_index, offset);
+    native_module->GetDebugInfo()->SetBreakpoint(func_index, offset, isolate);
   } else {
     // Iterate over all instances and tell them to set this new breakpoint.
     // We do this using the weak list of all instances from the script.

src/wasm/wasm-debug.h

@@ -154,7 +154,7 @@ class DebugInfo {
 
   WireBytesRef GetLocalName(int func_index, int local_index);
 
-  void SetBreakpoint(int func_index, int offset);
+  void SetBreakpoint(int func_index, int offset, Isolate* current_isolate);
 
   void RemoveDebugSideTables(Vector<WasmCode* const>);
 

test/inspector/debugger/wasm-set-breakpoint-liftoff-expected.txt

 Tests stepping through wasm scripts.
 Instantiating.
 Waiting for wasm script (ignoring first non-wasm script).
+Setting breakpoint at offset 38 on script wasm://wasm/0c10a5fe
+Calling main(4)
+Breaking on byte offset 38
+Setting breakpoint at offset 39 on script wasm://wasm/0c10a5fe
 Setting breakpoint at offset 54 on script wasm://wasm/0c10a5fe
 Setting breakpoint at offset 53 on script wasm://wasm/0c10a5fe
 Setting breakpoint at offset 51 on script wasm://wasm/0c10a5fe
 Setting breakpoint at offset 49 on script wasm://wasm/0c10a5fe
 Setting breakpoint at offset 45 on script wasm://wasm/0c10a5fe
 Setting breakpoint at offset 47 on script wasm://wasm/0c10a5fe
-Calling main(4)
-Breaking on byte offset 45
-Breaking on byte offset 47
-Breaking on byte offset 49
-Breaking on byte offset 51
-Breaking on byte offset 53
-Breaking on byte offset 54
+Breaking on byte offset 39
 Breaking on byte offset 45
 Breaking on byte offset 47
 Breaking on byte offset 49
 Breaking on byte offset 51
 Breaking on byte offset 53
 Breaking on byte offset 54
+Breaking on byte offset 38
+Breaking on byte offset 39
 Breaking on byte offset 45
 Breaking on byte offset 47
 Breaking on byte offset 49
 Breaking on byte offset 51
 Breaking on byte offset 53
 Breaking on byte offset 54
+Breaking on byte offset 38
+Breaking on byte offset 39
 Breaking on byte offset 45
 Breaking on byte offset 47
 Breaking on byte offset 49
 Breaking on byte offset 51
 Breaking on byte offset 53
 Breaking on byte offset 54
+Breaking on byte offset 38
+Breaking on byte offset 39
 Breaking on byte offset 45
 Breaking on byte offset 47
 exports.main returned!

test/inspector/debugger/wasm-set-breakpoint-liftoff.js

@@ -11,24 +11,24 @@ utils.load('test/mjsunit/wasm/wasm-module-builder.js');
 
 const builder = new WasmModuleBuilder();
 
-const func_a_idx =
-    builder.addFunction('wasm_A', kSig_v_v).addBody([kExprNop, kExprNop]).index;
+const func_a =
+    builder.addFunction('wasm_A', kSig_v_v).addBody([kExprNop, kExprNop]);
 
 // wasm_B calls wasm_A <param0> times.
 const func_b = builder.addFunction('wasm_B', kSig_v_i)
     .addBody([
       // clang-format off
-      kExprLoop, kWasmStmt,               // while
-        kExprLocalGet, 0,                 // -
-        kExprIf, kWasmStmt,               // if <param0> != 0
-          kExprLocalGet, 0,               // -
-          kExprI32Const, 1,               // -
-          kExprI32Sub,                    // -
-          kExprLocalSet, 0,               // decrease <param0>
-          kExprCallFunction, func_a_idx,  // -
-          kExprBr, 1,                     // continue
-          kExprEnd,                       // -
-        kExprEnd,                         // break
+      kExprLoop, kWasmStmt,                // while
+        kExprLocalGet, 0,                  // -
+        kExprIf, kWasmStmt,                // if <param0> != 0
+          kExprLocalGet, 0,                // -
+          kExprI32Const, 1,                // -
+          kExprI32Sub,                     // -
+          kExprLocalSet, 0,                // decrease <param0>
+          kExprCallFunction, func_a.index, // -
+          kExprBr, 1,                      // continue
+          kExprEnd,                        // -
+        kExprEnd,                          // break
       // clang-format on
     ])
     .exportAs('main');
@@ -53,21 +53,34 @@ const evalWithUrl = (code, url) =>
         .evaluate({'expression': code + '\n//# sourceURL=v8://test/' + url})
         .then(getResult);
 
-function setBreakpoint(offset, script) {
+function setBreakpoint(offset, scriptId, scriptUrl) {
   InspectorTest.log(
-      'Setting breakpoint at offset ' + offset + ' on script ' + script.url);
+      'Setting breakpoint at offset ' + offset + ' on script ' + scriptUrl);
   return Protocol.Debugger
       .setBreakpoint(
-          {'location': {'scriptId': script.scriptId, 'lineNumber': 0, 'columnNumber': offset}})
+          {'location': {'scriptId': scriptId, 'lineNumber': 0, 'columnNumber': offset}})
       .then(getResult);
 }
 
+// Only set breakpoints during the first loop iteration.
+var first_iteration = true;
+
 Protocol.Debugger.onPaused(pause_msg => {
-  let loc = pause_msg.params.callFrames[0].location;
+  let frame = pause_msg.params.callFrames[0];
+  let loc = frame.location;
   if (loc.lineNumber != 0) {
     InspectorTest.log('Unexpected line number: ' + loc.lineNumber);
   }
   InspectorTest.log('Breaking on byte offset ' + loc.columnNumber);
+  if (first_iteration && loc.columnNumber == func_a.body_offset) {
+    // Check that setting breakpoints on active instances of A and B takes
+    // effect immediately.
+    setBreakpoint(func_a.body_offset + 1, loc.scriptId, frame.url);
+    for (offset of [11, 10, 8, 6, 2, 4]) {
+      setBreakpoint(func_b.body_offset + offset, loc.scriptId, frame.url);
+    }
+    first_iteration = false;
+  }
   Protocol.Debugger.resume();
 });
 
@@ -82,9 +95,9 @@ Protocol.Debugger.onPaused(pause_msg => {
       'Waiting for wasm script (ignoring first non-wasm script).');
   // Ignore javascript and full module wasm script, get scripts for functions.
   const [, {params: wasm_script}] = await Protocol.Debugger.onceScriptParsed(2);
-  for (offset of [11, 10, 8, 6, 2, 4]) {
-    await setBreakpoint(func_b.body_offset + offset, wasm_script);
-  }
+  // Set a breakpoint in function A at offset 0. When the debugger hits this
+  // breakpoint, new ones will be added.
+  await setBreakpoint(func_a.body_offset, wasm_script.scriptId, wasm_script.url);
   InspectorTest.log('Calling main(4)');
   await evalWithUrl('instance.exports.main(4)', 'runWasm');
   InspectorTest.log('exports.main returned!');