[wasm] First plumbing for debugging in Liftoff

This CL adds a --debug-in-liftoff flag, which takes another path in
{WasmScript::SetBreakPointForFunction}, and sets the breakpoint via
{wasm::DebugInfo} (Liftoff-related) instead of {WasmDebugInfo} (C++
interpreter related).
Actual breakpoint support is not there yet, so the new test which sets
this flag does not currently break anywhere. This will change with a
future CL.

R=thibaudm@chromium.org

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

src/flags/flag-definitions.h

@@ -708,6 +708,10 @@ DEFINE_BOOL(trace_wasm_memory, false,
 DEFINE_INT(wasm_tier_mask_for_testing, 0,
            "bitmask of functions to compile with TurboFan instead of Liftoff")
 
+DEFINE_BOOL(debug_in_liftoff, false,
+            "use Liftoff instead of the C++ interpreter for debugging "
+            "WebAssembly (experimental)")
+
 DEFINE_BOOL(validate_asm, true, "validate asm.js modules before compiling")
 DEFINE_BOOL(suppress_asm_messages, false,
             "don't emit asm.js related messages (for golden file testing)")

src/wasm/baseline/liftoff-compiler.cc

@@ -2404,8 +2404,10 @@ WasmCompilationResult ExecuteLiftoffCompilation(AccountingAllocator* allocator,
 
   Zone zone(allocator, "LiftoffCompilationZone");
   auto call_descriptor = compiler::GetWasmCallDescriptor(&zone, func_body.sig);
-  base::Optional<TimedHistogramScope> liftoff_compile_time_scope(
-      base::in_place, counters->liftoff_compile_time());
+  base::Optional<TimedHistogramScope> liftoff_compile_time_scope;
+  if (counters) {
+    liftoff_compile_time_scope.emplace(counters->liftoff_compile_time());
+  }
   size_t code_size_estimate =
       WasmCodeManager::EstimateLiftoffCodeSize(func_body_size);
   // Allocate the initial buffer a bit bigger to avoid reallocation during code
@@ -2422,21 +2424,24 @@ WasmCompilationResult ExecuteLiftoffCompilation(AccountingAllocator* allocator,
   LiftoffCompiler* compiler = &decoder.interface();
   if (decoder.failed()) compiler->OnFirstError(&decoder);
 
-  // Check that the histogram for the bailout reasons has the correct size.
-  DCHECK_EQ(0, counters->liftoff_bailout_reasons()->min());
-  DCHECK_EQ(kNumBailoutReasons - 1, counters->liftoff_bailout_reasons()->max());
-  DCHECK_EQ(kNumBailoutReasons,
-            counters->liftoff_bailout_reasons()->num_buckets());
-  // Register the bailout reason (can also be {kSuccess}).
-  counters->liftoff_bailout_reasons()->AddSample(
-      static_cast<int>(compiler->bailout_reason()));
-  if (compiler->did_bailout()) {
-    // Liftoff compilation failed.
-    counters->liftoff_unsupported_functions()->Increment();
-    return WasmCompilationResult{};
-  }
-
-  counters->liftoff_compiled_functions()->Increment();
+  if (counters) {
+    // Check that the histogram for the bailout reasons has the correct size.
+    DCHECK_EQ(0, counters->liftoff_bailout_reasons()->min());
+    DCHECK_EQ(kNumBailoutReasons - 1,
+              counters->liftoff_bailout_reasons()->max());
+    DCHECK_EQ(kNumBailoutReasons,
+              counters->liftoff_bailout_reasons()->num_buckets());
+    // Register the bailout reason (can also be {kSuccess}).
+    counters->liftoff_bailout_reasons()->AddSample(
+        static_cast<int>(compiler->bailout_reason()));
+    if (compiler->did_bailout()) {
+      // Liftoff compilation failed.
+      counters->liftoff_unsupported_functions()->Increment();
+      return WasmCompilationResult{};
+    }
+
+    counters->liftoff_compiled_functions()->Increment();
+  }
 
   WasmCompilationResult result;
   compiler->GetCode(&result.code_desc);

src/wasm/wasm-debug.cc

@@ -574,6 +574,42 @@ class DebugInfoImpl {
     return local_names_->GetName(func_index, local_index);
   }
 
+  void SetBreakpoint(int func_index, int offset) {
+    // 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);
+
+    // Recompile the function with Liftoff, setting the new breakpoints.
+    CompilationEnv env = native_module_->CreateCompilationEnv();
+    auto* function = &native_module_->module()->functions[func_index];
+    Vector<const uint8_t> wire_bytes = native_module_->wire_bytes();
+    FunctionBody body{function->sig, function->code.offset(),
+                      wire_bytes.begin() + function->code.offset(),
+                      wire_bytes.begin() + function->code.end_offset()};
+    // TODO(clemensb): Pass breakpoints to Liftoff.
+    WasmCompilationResult result =
+        ExecuteLiftoffCompilation(native_module_->engine()->allocator(), &env,
+                                  body, func_index, nullptr, nullptr);
+    DCHECK(result.succeeded());
+
+    WasmCodeRefScope wasm_code_ref_scope;
+    WasmCode* new_code = native_module_->AddCompiledCode(std::move(result));
+
+    // TODO(clemensb): OSR active frames on the stack (on all threads).
+    USE(new_code);
+  }
+
  private:
   DebugSideTable* GetDebugSideTable(AccountingAllocator* allocator,
                                     int func_index) {
@@ -631,7 +667,7 @@ class DebugInfoImpl {
 
   NativeModule* const native_module_;
 
-  // {mutex_} protects {debug_side_tables_} and {local_names_}.
+  // {mutex_} protects all fields below.
   base::Mutex mutex_;
 
   // DebugSideTable per function, lazily initialized.
@@ -640,6 +676,10 @@ class DebugInfoImpl {
   // Names of locals, lazily decoded from the wire bytes.
   std::unique_ptr<LocalNames> local_names_;
 
+  // Keeps track of the currently set breakpoints (by offset within that
+  // function).
+  std::unordered_map<int, std::vector<int>> breakpoints_per_function_;
+
   DISALLOW_COPY_AND_ASSIGN(DebugInfoImpl);
 };
 
@@ -657,6 +697,10 @@ 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);
+}
+
 }  // namespace wasm
 
 namespace {
@@ -925,34 +969,39 @@ bool WasmScript::SetBreakPointOnFirstBreakableForFunction(
 
 // static
 bool WasmScript::SetBreakPointForFunction(Handle<Script> script, int func_index,
-                                          int breakable_offset,
+                                          int offset,
                                           Handle<BreakPoint> break_point) {
   Isolate* isolate = script->GetIsolate();
 
   DCHECK_LE(0, func_index);
-  DCHECK_NE(0, breakable_offset);
+  DCHECK_NE(0, offset);
 
   // Find the function for this breakpoint.
-  const wasm::WasmModule* module = script->wasm_native_module()->module();
+  wasm::NativeModule* native_module = script->wasm_native_module();
+  const wasm::WasmModule* module = native_module->module();
   const wasm::WasmFunction& func = module->functions[func_index];
 
-  // Insert new break point into break_positions of module object.
-  WasmScript::AddBreakpointToInfo(script, func.code.offset() + breakable_offset,
+  // Insert new break point into {wasm_breakpoint_infos} of the script.
+  WasmScript::AddBreakpointToInfo(script, func.code.offset() + offset,
                                   break_point);
 
-  // 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.
-  Handle<WeakArrayList> weak_instance_list(script->wasm_weak_instance_list(),
-                                           isolate);
-  for (int i = 0; i < weak_instance_list->length(); ++i) {
-    MaybeObject maybe_instance = weak_instance_list->Get(i);
-    if (maybe_instance->IsWeak()) {
-      Handle<WasmInstanceObject> instance(
-          WasmInstanceObject::cast(maybe_instance->GetHeapObjectAssumeWeak()),
-          isolate);
-      Handle<WasmDebugInfo> debug_info =
-          WasmInstanceObject::GetOrCreateDebugInfo(instance);
-      WasmDebugInfo::SetBreakpoint(debug_info, func_index, breakable_offset);
+  if (FLAG_debug_in_liftoff) {
+    native_module->GetDebugInfo()->SetBreakpoint(func_index, offset);
+  } 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.
+    Handle<WeakArrayList> weak_instance_list(script->wasm_weak_instance_list(),
+                                             isolate);
+    for (int i = 0; i < weak_instance_list->length(); ++i) {
+      MaybeObject maybe_instance = weak_instance_list->Get(i);
+      if (maybe_instance->IsWeak()) {
+        Handle<WasmInstanceObject> instance(
+            WasmInstanceObject::cast(maybe_instance->GetHeapObjectAssumeWeak()),
+            isolate);
+        Handle<WasmDebugInfo> debug_info =
+            WasmInstanceObject::GetOrCreateDebugInfo(instance);
+        WasmDebugInfo::SetBreakpoint(debug_info, func_index, offset);
+      }
     }
   }
 

src/wasm/wasm-debug.h

@@ -151,6 +151,8 @@ class DebugInfo {
 
   WireBytesRef GetLocalName(int func_index, int local_index);
 
+  void SetBreakpoint(int func_index, int offset);
+
  private:
   std::unique_ptr<DebugInfoImpl> impl_;
 };

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 54 on script wasm://wasm/18214bfe
+Setting breakpoint at offset 53 on script wasm://wasm/18214bfe
+Setting breakpoint at offset 51 on script wasm://wasm/18214bfe
+Setting breakpoint at offset 49 on script wasm://wasm/18214bfe
+Setting breakpoint at offset 45 on script wasm://wasm/18214bfe
+Setting breakpoint at offset 47 on script wasm://wasm/18214bfe
+Calling main(4)
+exports.main returned!
+Finished!

test/inspector/debugger/wasm-set-breakpoint-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
+
+const {session, contextGroup, Protocol} =
+    InspectorTest.start('Tests stepping through wasm scripts.');
+
+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;
+
+// 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
+      // clang-format on
+    ])
+    .exportAs('main');
+
+const module_bytes = builder.toArray();
+
+function instantiate(bytes) {
+  let buffer = new ArrayBuffer(bytes.length);
+  let view = new Uint8Array(buffer);
+  for (let i = 0; i < bytes.length; ++i) {
+    view[i] = bytes[i] | 0;
+  }
+
+  let module = new WebAssembly.Module(buffer);
+  return new WebAssembly.Instance(module);
+}
+
+const getResult = msg => msg.result || InspectorTest.logMessage(msg);
+
+const evalWithUrl = (code, url) =>
+    Protocol.Runtime
+        .evaluate({'expression': code + '\n//# sourceURL=v8://test/' + url})
+        .then(getResult);
+
+function setBreakpoint(offset, script) {
+  InspectorTest.log(
+      'Setting breakpoint at offset ' + offset + ' on script ' + script.url);
+  return Protocol.Debugger
+      .setBreakpoint(
+          {'location': {'scriptId': script.scriptId, 'lineNumber': 0, 'columnNumber': offset}})
+      .then(getResult);
+}
+
+Protocol.Debugger.onPaused(pause_msg => {
+  let loc = pause_msg.params.callFrames[0].location;
+  if (loc.lineNumber != 0) {
+    InspectorTest.log('Unexpected line number: ' + loc.lineNumber);
+  }
+  InspectorTest.log('Breaking on byte offset ' + loc.columnNumber);
+  Protocol.Debugger.resume();
+});
+
+(async function test() {
+  await Protocol.Debugger.enable();
+  InspectorTest.log('Instantiating.');
+  // Spawn asynchronously:
+  let instantiate_code = 'const instance = (' + instantiate + ')(' +
+      JSON.stringify(module_bytes) + ');';
+  evalWithUrl(instantiate_code, 'instantiate');
+  InspectorTest.log(
+      '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);
+  }
+  InspectorTest.log('Calling main(4)');
+  await evalWithUrl('instance.exports.main(4)', 'runWasm');
+  InspectorTest.log('exports.main returned!');
+  InspectorTest.log('Finished!');
+  InspectorTest.completeTest();
+})();