[wasm][debug] Implement stepping out from JS to wasm

This specific case was not implemented or tested before. Implementing it
actually simplifies some of the existing logic, since StepOut can now
reuse the generic logic in debug.cc for all cases (Wasm->Wasm, Wasm->JS,
JS->Wasm).

Drive-by:
1) Fix typo ("skip" -> "step").
2) Move the check for Liftoff code from debug.cc to wasm-debug.cc, where
   it fits better.
3) Remove a TODO which is done already.

R=thibaudm@chromium.org, szuend@chromium.org

Bug: chromium:1145176
Change-Id: I415ca1d8bacef5b21bf1dafd9e16417ec2d12c7c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2560719
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71428}

src/debug/debug.cc

@@ -1096,16 +1096,17 @@ void Debug::PrepareStep(StepAction step_action) {
     thread_local_.last_frame_count_ = current_frame_count;
     // No longer perform the current async step.
     clear_suspended_generator();
-  } else if (frame->is_wasm()) {
-    // Handle stepping in Liftoff code.
+  } else if (frame->is_wasm() && step_action != StepOut) {
+    // Handle stepping in wasm.
     WasmFrame* wasm_frame = WasmFrame::cast(frame);
-    wasm::WasmCodeRefScope code_ref_scope;
-    wasm::WasmCode* code = wasm_frame->wasm_code();
-    if (code->is_liftoff()) {
-      auto* debug_info = wasm_frame->native_module()->GetDebugInfo();
-      debug_info->PrepareStep(wasm_frame);
+    auto* debug_info = wasm_frame->native_module()->GetDebugInfo();
+    if (debug_info->PrepareStep(wasm_frame)) {
+      UpdateHookOnFunctionCall();
+      return;
     }
-    // In case the wasm code returns, prepare the next frame (if JS) to break.
+    // If the wasm code is not debuggable or will return after this step
+    // (indicated by {PrepareStep} returning false), then step out of that frame
+    // instead.
     step_action = StepOut;
     UpdateHookOnFunctionCall();
   }
@@ -1131,8 +1132,15 @@ void Debug::PrepareStep(StepAction step_action) {
       bool in_current_frame = true;
       for (; !frames_it.done(); frames_it.Advance()) {
         if (frames_it.frame()->is_wasm()) {
-          in_current_frame = false;
-          continue;
+          if (in_current_frame) {
+            in_current_frame = false;
+            continue;
+          }
+          // Handle stepping out into Wasm.
+          WasmFrame* wasm_frame = WasmFrame::cast(frames_it.frame());
+          auto* debug_info = wasm_frame->native_module()->GetDebugInfo();
+          debug_info->PrepareStepOutTo(wasm_frame);
+          return;
         }
         JavaScriptFrame* frame = JavaScriptFrame::cast(frames_it.frame());
         if (last_step_action() == StepIn) {
@@ -1146,7 +1154,7 @@ void Debug::PrepareStep(StepAction step_action) {
           Handle<SharedFunctionInfo> info = infos.back();
           infos.pop_back();
           if (in_current_frame) {
-            // We want to skip out, so skip the current frame.
+            // We want to step out, so skip the current frame.
             in_current_frame = false;
             continue;
           }
@@ -1162,7 +1170,6 @@ void Debug::PrepareStep(StepAction step_action) {
       thread_local_.target_frame_count_ = current_frame_count;
       V8_FALLTHROUGH;
     case StepIn:
-      // TODO(clemensb): Implement stepping from JS into wasm.
       FloodWithOneShot(shared);
       break;
   }

src/wasm/wasm-debug.cc

@@ -488,7 +488,6 @@ class DebugInfoImpl {
   void FloodWithBreakpoints(WasmFrame* frame, ReturnLocation return_location) {
     // 0 is an invalid offset used to indicate flooding.
     int offset = 0;
-    WasmCodeRefScope wasm_code_ref_scope;
     DCHECK(frame->wasm_code()->is_liftoff());
     // Generate an additional source position for the current byte offset.
     base::MutexGuard guard(&mutex_);
@@ -499,22 +498,20 @@ class DebugInfoImpl {
     per_isolate_data_[frame->isolate()].stepping_frame = frame->id();
   }
 
-  void PrepareStep(WasmFrame* frame) {
-    Isolate* isolate = frame->isolate();
-
-    // If we are at a return instruction, then any stepping action is equivalent
-    // to StepOut, and we need to flood the parent function.
-    if (IsAtReturn(frame) || isolate->debug()->last_step_action() == StepOut) {
-      StackTraceFrameIterator it(isolate, isolate->debug()->break_frame_id());
-      DCHECK(!it.done());
-      DCHECK(it.frame()->is_wasm());
-      it.Advance();
-      if (it.done() || !it.frame()->is_wasm()) return;
-      frame = WasmFrame::cast(it.frame());
-      FloodWithBreakpoints(frame, kAfterWasmCall);
-    } else {
-      FloodWithBreakpoints(frame, kAfterBreakpoint);
-    }
+  bool PrepareStep(WasmFrame* frame) {
+    WasmCodeRefScope wasm_code_ref_scope;
+    wasm::WasmCode* code = frame->wasm_code();
+    if (!code->is_liftoff()) return false;  // Cannot step in TurboFan code.
+    if (IsAtReturn(frame)) return false;    // Will return after this step.
+    FloodWithBreakpoints(frame, kAfterBreakpoint);
+    return true;
+  }
+
+  void PrepareStepOutTo(WasmFrame* frame) {
+    WasmCodeRefScope wasm_code_ref_scope;
+    wasm::WasmCode* code = frame->wasm_code();
+    if (!code->is_liftoff()) return;  // Cannot step out to TurboFan code.
+    FloodWithBreakpoints(frame, kAfterWasmCall);
   }
 
   void ClearStepping(Isolate* isolate) {
@@ -874,7 +871,13 @@ void DebugInfo::SetBreakpoint(int func_index, int offset,
   impl_->SetBreakpoint(func_index, offset, current_isolate);
 }
 
-void DebugInfo::PrepareStep(WasmFrame* frame) { impl_->PrepareStep(frame); }
+bool DebugInfo::PrepareStep(WasmFrame* frame) {
+  return impl_->PrepareStep(frame);
+}
+
+void DebugInfo::PrepareStepOutTo(WasmFrame* frame) {
+  impl_->PrepareStepOutTo(frame);
+}
 
 void DebugInfo::ClearStepping(Isolate* isolate) {
   impl_->ClearStepping(isolate);

src/wasm/wasm-debug.h

@@ -170,7 +170,11 @@ class V8_EXPORT_PRIVATE DebugInfo {
 
   void SetBreakpoint(int func_index, int offset, Isolate* current_isolate);
 
-  void PrepareStep(WasmFrame*);
+  // Returns true if we stay inside the passed frame (or a called frame) after
+  // the step. False if the frame will return after the step.
+  bool PrepareStep(WasmFrame*);
+
+  void PrepareStepOutTo(WasmFrame*);
 
   void ClearStepping(Isolate*);
 

test/debugger/debug/wasm/stepping-from-js.js

@@ -36,7 +36,13 @@ const expected_breaks = [
   `imported:${import_line_nr + 1}:2`,   // debugger;
   `imported:${import_line_nr + 2}:2`,   // return 7;
   `imported:${import_line_nr + 2}:11`,  // return 7;
-  'sub:1:58', 'sub:1:60', 'sub:1:62', 'sub:1:63', 'main:1:72'
+  'main:1:68',                          // i32.const 3
+  'main:1:70',                          // call 'sub'
+  'sub:1:58',                           // local.get i0
+  'sub:1:60',                           // local.get i1
+  'sub:1:62',                           // i32.sub
+  'sub:1:63',                           // end
+  'main:1:72'                           // end
 ];
 let error;
 function onBreak(event, exec_state, data) {

test/inspector/debugger/wasm-stepping-out-from-js-expected.txt

+Tests stepping out from javascript to a wasm caller
+Instantiating.
+Running exports.main.
+>>> First round
+Paused at:
+function pauseAlternating() {
+  if (pause) #debugger;
+  pause = !pause;
+
+Paused at:
+Script wasm://wasm/15df010e byte offset 53: Wasm opcode 0x0b (kExprEnd)
+Paused at:
+instance.exports.main()#
+
+exports.main returned.
+After stepping out of the last script, we should stop right at the beginning of the next script.
+>>> Second round
+Paused at:
+#instance.exports.main()
+
+exports.main returned.
+The next cycle should work as before (stopping at the "debugger" statement), after stopping at script entry.
+>>> Third round
+Paused at:
+#instance.exports.main()
+
+Paused at:
+function pauseAlternating() {
+  if (pause) #debugger;
+  pause = !pause;
+
+Paused at:
+Script wasm://wasm/15df010e byte offset 53: Wasm opcode 0x0b (kExprEnd)
+Paused at:
+instance.exports.main()#
+
+exports.main returned.

test/inspector/debugger/wasm-stepping-out-from-js.js

+// Copyright 2020 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.
+
+utils.load('test/inspector/wasm-inspector-test.js');
+
+let {session, contextGroup, Protocol} =
+    InspectorTest.start('Tests stepping out from javascript to a wasm caller');
+session.setupScriptMap();
+
+let builder = new WasmModuleBuilder();
+
+let pause = builder.addImport('imp', 'pause', kSig_v_v);
+let func = builder.addFunction('wasm_main', kSig_v_v)
+               .addBody([kExprCallFunction, pause])
+               .exportAs('main');
+
+let module_bytes = builder.toArray();
+
+Protocol.Debugger.onPaused(async message => {
+  InspectorTest.log('Paused at:');
+  var frames = message.params.callFrames;
+  await session.logSourceLocation(frames[0].location);
+  await Protocol.Debugger.stepOut();
+});
+
+contextGroup.addScript(`
+let pause = true;
+function pauseAlternating() {
+  if (pause) debugger;
+  pause = !pause;
+}
+`);
+
+(async function Test() {
+  await Protocol.Debugger.enable();
+  InspectorTest.log('Instantiating.');
+  const instantiate_code =
+      `const instance = (${WasmInspectorTest.instantiateFromBuffer})(${
+          JSON.stringify(module_bytes)}, {imp: {pause: pauseAlternating}});`;
+  WasmInspectorTest.evalWithUrl(instantiate_code, 'instantiate');
+  const [, {params: wasmScript}] = await Protocol.Debugger.onceScriptParsed(2);
+  const scriptId = wasmScript.scriptId;
+
+  InspectorTest.log('Running exports.main.');
+  InspectorTest.log('>>> First round');
+  await Protocol.Runtime.evaluate({expression: 'instance.exports.main()'});
+  InspectorTest.log('exports.main returned.');
+
+  InspectorTest.log('After stepping out of the last script, we should stop right at the beginning of the next script.');
+  InspectorTest.log('>>> Second round');
+  await Protocol.Runtime.evaluate({expression: 'instance.exports.main()'});
+  InspectorTest.log('exports.main returned.');
+
+  InspectorTest.log('The next cycle should work as before (stopping at the "debugger" statement), after stopping at script entry.');
+  InspectorTest.log('>>> Third round');
+  await Protocol.Runtime.evaluate({expression: 'instance.exports.main()'});
+  InspectorTest.log('exports.main returned.');
+
+  InspectorTest.completeTest();
+})();

test/inspector/wasm-inspector-test.js

@@ -12,14 +12,14 @@ WasmInspectorTest.evalWithUrl = (code, url) =>
         .evaluate({'expression': code + '\n//# sourceURL=v8://test/' + url})
         .then(printIfFailure);
 
-WasmInspectorTest.instantiateFromBuffer = function(bytes) {
+WasmInspectorTest.instantiateFromBuffer = function(bytes, imports) {
   var buffer = new ArrayBuffer(bytes.length);
   var view = new Uint8Array(buffer);
   for (var i = 0; i < bytes.length; ++i) {
     view[i] = bytes[i] | 0;
   }
   const module = new WebAssembly.Module(buffer);
-  return new WebAssembly.Instance(module);
+  return new WebAssembly.Instance(module, imports);
 }
 
 WasmInspectorTest.instantiate = async function(bytes, instance_name = 'instance') {