[liftoff] Add initial logic for stepping

Flood functions with breakpoints to prepare them for stepping. With a
small modification to the runtime function, this already implements a
basic step over functionality.

We still cannot resume, step in or step out (including stepping over a
return instruction).

R=clemensb@chromium.org

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

src/debug/debug.cc

@@ -37,6 +37,7 @@
 #include "src/objects/js-promise-inl.h"
 #include "src/objects/slots.h"
 #include "src/snapshot/snapshot.h"
+#include "src/wasm/wasm-debug.h"
 #include "src/wasm/wasm-objects-inl.h"
 
 namespace v8 {
@@ -1004,6 +1005,15 @@ void Debug::PrepareStep(StepAction step_action) {
       return;
     }
   }
+  // Handle stepping in Liftoff code.
+  if (FLAG_debug_in_liftoff && frame->is_wasm_compiled()) {
+    WasmCompiledFrame* wasm_frame = WasmCompiledFrame::cast(frame);
+    wasm::WasmCodeRefScope code_ref_scope;
+    wasm::WasmCode* code = wasm_frame->wasm_code();
+    if (code->is_liftoff()) {
+      wasm_frame->native_module()->GetDebugInfo()->PrepareStep(isolate_);
+    }
+  }
   // If this is wasm, but there are no interpreted frames on top, all we can do
   // is step out.
   if (frame->is_wasm()) step_action = StepOut;

src/runtime/runtime-wasm.cc

@@ -19,6 +19,7 @@
 #include "src/wasm/module-compiler.h"
 #include "src/wasm/wasm-code-manager.h"
 #include "src/wasm/wasm-constants.h"
+#include "src/wasm/wasm-debug.h"
 #include "src/wasm/wasm-engine.h"
 #include "src/wasm/wasm-objects.h"
 #include "src/wasm/wasm-value.h"
@@ -615,6 +616,13 @@ RUNTIME_FUNCTION(Runtime_WasmDebugBreak) {
   // Enter the debugger.
   DebugScope debug_scope(isolate->debug());
 
+  const auto undefined = ReadOnlyRoots(isolate).undefined_value();
+  auto* debug_info = frame_finder.frame()->native_module()->GetDebugInfo();
+  if (debug_info->IsStepping(frame_finder.frame())) {
+    isolate->debug()->OnDebugBreak(isolate->factory()->empty_fixed_array());
+    return undefined;
+  }
+
   // Check whether we hit a breakpoint.
   if (isolate->debug()->break_points_active()) {
     Handle<Script> script(instance->module_object().script(), isolate);
@@ -626,7 +634,7 @@ RUNTIME_FUNCTION(Runtime_WasmDebugBreak) {
     }
   }
 
-  return ReadOnlyRoots(isolate).undefined_value();
+  return undefined;
 }
 
 }  // namespace internal

src/wasm/baseline/liftoff-compiler.cc

@@ -634,7 +634,9 @@ class LiftoffCompiler {
 
   void FinishFunction(FullDecoder* decoder) {
     // All breakpoints (if any) must be emitted by now.
-    DCHECK_NULL(next_breakpoint_ptr_);
+    DCHECK(next_breakpoint_ptr_ == nullptr ||
+           (*next_breakpoint_ptr_ == 0 &&
+            next_breakpoint_ptr_ + 1 == next_breakpoint_end_));
     if (DidAssemblerBailout(decoder)) return;
     for (OutOfLineCode& ool : out_of_line_code_) {
       GenerateOutOfLineCode(&ool);
@@ -655,13 +657,19 @@ class LiftoffCompiler {
   }
 
   void NextInstruction(FullDecoder* decoder, WasmOpcode opcode) {
-    if (V8_UNLIKELY(next_breakpoint_ptr_) &&
-        *next_breakpoint_ptr_ == decoder->position()) {
-      ++next_breakpoint_ptr_;
-      if (next_breakpoint_ptr_ == next_breakpoint_end_) {
-        next_breakpoint_ptr_ = next_breakpoint_end_ = nullptr;
+    if (V8_UNLIKELY(next_breakpoint_ptr_)) {
+      if (*next_breakpoint_ptr_ == 0) {
+        // A single breakpoint at offset 0 indicates stepping.
+        DCHECK_EQ(next_breakpoint_ptr_ + 1, next_breakpoint_end_);
+        EmitBreakpoint(decoder);
+      } else if (*next_breakpoint_ptr_ == decoder->position()) {
+        ++next_breakpoint_ptr_;
+        // TODO(thibaudm): skip unreachable breakpoints.
+        if (next_breakpoint_ptr_ == next_breakpoint_end_) {
+          next_breakpoint_ptr_ = next_breakpoint_end_ = nullptr;
+        }
+        EmitBreakpoint(decoder);
       }
-      EmitBreakpoint(decoder);
     }
     TraceCacheState(decoder);
 #ifdef DEBUG
@@ -2834,6 +2842,8 @@ class LiftoffCompiler {
   uint32_t pc_offset_stack_frame_construction_ = 0;
   // For emitting breakpoint, we store a pointer to the position of the next
   // breakpoint, and a pointer after the list of breakpoints as end marker.
+  // A single breakpoint at offset 0 indicates that we should prepare the
+  // function for stepping by flooding it with breakpoints.
   int* next_breakpoint_ptr_ = nullptr;
   int* next_breakpoint_end_ = nullptr;
 

src/wasm/wasm-debug.cc

@@ -613,23 +613,10 @@ class DebugInfoImpl {
     return local_names_->GetName(func_index, local_index);
   }
 
-  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
-    // Liftoff variants of a function would be problematic.
-    base::MutexGuard guard(&mutex_);
-
-    std::vector<int>& breakpoints = breakpoints_per_function_[func_index];
-    auto insertion_point =
-        std::lower_bound(breakpoints.begin(), breakpoints.end(), offset);
-    if (insertion_point != breakpoints.end() && *insertion_point == offset) {
-      // The breakpoint is already set.
-      return;
-    }
-    breakpoints.insert(insertion_point, offset);
-
+  void RecompileLiftoffWithBreakpoints(int func_index, Vector<int> offsets,
+                                       Isolate* current_isolate) {
     // Recompile the function with Liftoff, setting the new breakpoints.
+    // Not thread-safe. The caller is responsible for locking {mutex_}.
     CompilationEnv env = native_module_->CreateCompilationEnv();
     auto* function = &native_module_->module()->functions[func_index];
     Vector<const uint8_t> wire_bytes = native_module_->wire_bytes();
@@ -637,15 +624,16 @@ class DebugInfoImpl {
                       wire_bytes.begin() + function->code.offset(),
                       wire_bytes.begin() + function->code.end_offset()};
     std::unique_ptr<DebugSideTable> debug_sidetable;
-    WasmCompilationResult result = ExecuteLiftoffCompilation(
-        native_module_->engine()->allocator(), &env, body, func_index, nullptr,
-        nullptr, VectorOf(breakpoints), &debug_sidetable);
+
+    WasmCompilationResult result;
+    result = ExecuteLiftoffCompilation(native_module_->engine()->allocator(),
+                                       &env, body, func_index, nullptr, nullptr,
+                                       offsets, &debug_sidetable);
     DCHECK(result.succeeded());
     DCHECK_NOT_NULL(debug_sidetable);
 
     WasmCodeRefScope wasm_code_ref_scope;
     WasmCode* new_code = native_module_->AddCompiledCode(std::move(result));
-
     bool added =
         debug_side_tables_.emplace(new_code, std::move(debug_sidetable)).second;
     DCHECK(added);
@@ -654,6 +642,54 @@ class DebugInfoImpl {
     UpdateReturnAddresses(current_isolate, new_code);
   }
 
+  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
+    // Liftoff variants of a function would be problematic.
+    base::MutexGuard guard(&mutex_);
+
+    std::vector<int>& breakpoints = breakpoints_per_function_[func_index];
+    // offset == 0 indicates flooding and is handled by the compiler.
+    if (offset != 0) {
+      auto insertion_point =
+          std::lower_bound(breakpoints.begin(), breakpoints.end(), offset);
+      if (insertion_point != breakpoints.end() && *insertion_point == offset) {
+        // The breakpoint is already set.
+        return;
+      }
+      breakpoints.insert(insertion_point, offset);
+    }
+
+    RecompileLiftoffWithBreakpoints(func_index, VectorOf(breakpoints),
+                                    current_isolate);
+  }
+
+  void FloodWithBreakpoints(NativeModule* native_module, int func_index,
+                            Isolate* current_isolate) {
+    base::MutexGuard guard(&mutex_);
+    // 0 is an invalid offset used to indicate flooding.
+    int offset = 0;
+    RecompileLiftoffWithBreakpoints(func_index, Vector<int>(&offset, 1),
+                                    current_isolate);
+  }
+
+  void PrepareStep(Isolate* isolate) {
+    StackTraceFrameIterator it(isolate);
+    DCHECK(!it.done());
+    DCHECK(it.frame()->is_wasm_compiled());
+    WasmCompiledFrame* frame = WasmCompiledFrame::cast(it.frame());
+    if (frame->id() != stepping_frame_) {
+      FloodWithBreakpoints(frame->native_module(), frame->function_index(),
+                           isolate);
+      stepping_frame_ = frame->id();
+    }
+  }
+
+  bool IsStepping(WasmCompiledFrame* frame) {
+    return frame->id() == stepping_frame_;
+  }
+
   void RemoveDebugSideTables(Vector<WasmCode* const> codes) {
     base::MutexGuard guard(&mutex_);
     for (auto* code : codes) {
@@ -749,6 +785,10 @@ class DebugInfoImpl {
   // function).
   std::unordered_map<int, std::vector<int>> breakpoints_per_function_;
 
+  // Store the frame ID when stepping, to avoid breaking in recursive calls of
+  // the same function.
+  StackFrameId stepping_frame_ = NO_ID;
+
   DISALLOW_COPY_AND_ASSIGN(DebugInfoImpl);
 };
 
@@ -771,6 +811,12 @@ void DebugInfo::SetBreakpoint(int func_index, int offset,
   impl_->SetBreakpoint(func_index, offset, current_isolate);
 }
 
+void DebugInfo::PrepareStep(Isolate* isolate) { impl_->PrepareStep(isolate); }
+
+bool DebugInfo::IsStepping(WasmCompiledFrame* frame) {
+  return impl_->IsStepping(frame);
+}
+
 void DebugInfo::RemoveDebugSideTables(Vector<WasmCode* const> code) {
   impl_->RemoveDebugSideTables(code);
 }
@@ -985,7 +1031,7 @@ Handle<Code> WasmDebugInfo::GetCWasmEntry(Handle<WasmDebugInfo> debug_info,
 
 namespace {
 
-// Return the next breakable position after {offset_in_func} in function
+// Return the next breakable position at or after {offset_in_func} in function
 // {func_index}, or 0 if there is none.
 // Note that 0 is never a breakable position in wasm, since the first byte
 // contains the locals count for the function.

src/wasm/wasm-debug.h

@@ -23,6 +23,7 @@ class Handle;
 class JSObject;
 template <typename T>
 class Vector;
+class WasmCompiledFrame;
 class WasmInstanceObject;
 
 namespace wasm {
@@ -146,6 +147,10 @@ class DebugInfo {
 
   void SetBreakpoint(int func_index, int offset, Isolate* current_isolate);
 
+  void PrepareStep(Isolate*);
+
+  bool IsStepping(WasmCompiledFrame*);
+
   void RemoveDebugSideTables(Vector<WasmCode* const>);
 
  private:

test/inspector/debugger/wasm-stepping-liftoff-expected.txt

+Tests stepping through wasm scripts by byte offsets
+Setting up global instance variable.
+Got wasm script: wasm://wasm/befe41aa
+Setting breakpoint on offset 59 (should be propagated to 60, the offset of the call), url wasm://wasm/befe41aa
+{
+    columnNumber : 60
+    lineNumber : 0
+    scriptId : <scriptId>
+}
+Paused at wasm://wasm/befe41aa:0:60
+Debugger.stepOver called
+Paused at wasm://wasm/befe41aa:0:62
+Debugger.stepOver called
+Paused at wasm://wasm/befe41aa:0:46
+Debugger.stepOver called
+Paused at wasm://wasm/befe41aa:0:48
+Debugger.stepOver called
+Paused at wasm://wasm/befe41aa:0:50
+Debugger.stepOver called
+Paused at wasm://wasm/befe41aa:0:52
+Debugger.stepOver called
+Paused at wasm://wasm/befe41aa:0:54
+Debugger.stepOver called
+Paused at wasm://wasm/befe41aa:0:55
+Debugger.stepOver called
+Paused at wasm://wasm/befe41aa:0:57
+Debugger.stepOver called
+Paused at wasm://wasm/befe41aa:0:60
+Debugger.stepOver called
+Finished!

test/inspector/debugger/wasm-stepping-liftoff.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.
+
+// Flags: --debug-in-liftoff
+
+let {session, contextGroup, Protocol} =
+    InspectorTest.start('Tests stepping through wasm scripts by byte offsets');
+
+utils.load('test/mjsunit/wasm/wasm-module-builder.js');
+
+var builder = new WasmModuleBuilder();
+
+var func_a_idx =
+    builder.addFunction('wasm_A', kSig_v_i).addBody([kExprNop, kExprNop]).index;
+
+// wasm_B calls wasm_A <param0> times.
+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>
+          ...wasmI32Const(1024),          // some longer i32 const (2 byte imm)
+          kExprCallFunction, func_a_idx,  // -
+          kExprBr, 1,                     // continue
+          kExprEnd,                       // -
+        kExprEnd,                         // break
+      // clang-format on
+    ])
+    .exportAs('main');
+
+
+var module_bytes = builder.toArray();
+
+function instantiate(bytes) {
+  var buffer = new ArrayBuffer(bytes.length);
+  var view = new Uint8Array(buffer);
+  for (var i = 0; i < bytes.length; ++i) {
+    view[i] = bytes[i] | 0;
+  }
+
+  var module = new WebAssembly.Module(buffer);
+  // Set global variable.
+  instance = new WebAssembly.Instance(module);
+}
+
+(async function test() {
+  for (const action of ['stepInto', 'stepOver', 'stepOut', 'resume'])
+    InspectorTest.logProtocolCommandCalls('Debugger.' + action);
+
+  await Protocol.Debugger.enable();
+  InspectorTest.log('Setting up global instance variable.');
+  Protocol.Runtime.evaluate({
+    expression: `var instance;` +
+        `(${instantiate.toString()})(${JSON.stringify(module_bytes)})`
+  });
+  const [, {params: wasmScript}] = await Protocol.Debugger.onceScriptParsed(2);
+
+  InspectorTest.log('Got wasm script: ' + wasmScript.url);
+
+  // Set the breakpoint on a non-breakable position. This should resolve to the
+  // next instruction.
+  InspectorTest.log(
+      `Setting breakpoint on offset 59 (should be propagated to 60, the ` +
+      `offset of the call), url ${wasmScript.url}`);
+  const bpmsg = await Protocol.Debugger.setBreakpoint({
+    location: {scriptId: wasmScript.scriptId, lineNumber: 0, columnNumber: 59}
+  });
+
+  const actualLocation = bpmsg.result.actualLocation;
+  InspectorTest.logMessage(actualLocation);
+  Protocol.Runtime.evaluate({ expression: 'instance.exports.main(4)' });
+  // Step over 10 times.
+  for (let i = 0; i < 10; ++i) await waitForPauseAndStep('stepOver');
+  InspectorTest.log('Finished!');
+})().catch(reason => InspectorTest.log(`Failed: ${reason}`))
+    .finally(InspectorTest.completeTest);
+
+async function waitForPauseAndStep(stepAction) {
+  const {params: {callFrames}} = await Protocol.Debugger.oncePaused();
+  const topFrame = callFrames[0];
+  InspectorTest.log(
+      `Paused at ${topFrame.url}:${topFrame.location.lineNumber}:${topFrame.location.columnNumber}`);
+  Protocol.Debugger[stepAction]();
+}