[wasm][debug] Fix frame inspection at stack checks

Spill registers before stack checks so that we can inspect them, similar
to traps.

OSR during a stack check is still unsupported and will be fixed in a
follow-up CL.

R=clemensb@chromium.org

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

src/wasm/baseline/liftoff-compiler.cc

@@ -280,9 +280,9 @@ class LiftoffCompiler {
 
   using FullDecoder = WasmFullDecoder<validate, LiftoffCompiler>;
 
-  // For debugging, we need to spill registers before a trap, to be able to
-  // inspect them.
-  struct SpilledRegistersBeforeTrap : public ZoneObject {
+  // For debugging, we need to spill registers before a trap or a stack check to
+  // be able to inspect them.
+  struct SpilledRegistersForInspection : public ZoneObject {
     struct Entry {
       int offset;
       LiftoffRegister reg;
@@ -290,7 +290,7 @@ class LiftoffCompiler {
     };
     ZoneVector<Entry> entries;
 
-    explicit SpilledRegistersBeforeTrap(Zone* zone) : entries(zone) {}
+    explicit SpilledRegistersForInspection(Zone* zone) : entries(zone) {}
   };
 
   struct OutOfLineCode {
@@ -302,13 +302,13 @@ class LiftoffCompiler {
     uint32_t pc;  // for trap handler.
     // These two pointers will only be used for debug code:
     DebugSideTableBuilder::EntryBuilder* debug_sidetable_entry_builder;
-    SpilledRegistersBeforeTrap* spilled_registers;
+    SpilledRegistersForInspection* spilled_registers;
 
     // Named constructors:
     static OutOfLineCode Trap(
         WasmCode::RuntimeStubId s, WasmCodePosition pos, uint32_t pc,
         DebugSideTableBuilder::EntryBuilder* debug_sidetable_entry_builder,
-        SpilledRegistersBeforeTrap* spilled_registers) {
+        SpilledRegistersForInspection* spilled_registers) {
       DCHECK_LT(0, pos);
       return {{},
               {},
@@ -320,10 +320,11 @@ class LiftoffCompiler {
               spilled_registers};
     }
     static OutOfLineCode StackCheck(
-        WasmCodePosition pos, LiftoffRegList regs,
+        WasmCodePosition pos, LiftoffRegList regs_to_save,
+        SpilledRegistersForInspection* spilled_regs,
         DebugSideTableBuilder::EntryBuilder* debug_sidetable_entry_builder) {
-      return {{},   {}, WasmCode::kWasmStackGuard,     pos,
-              regs, 0,  debug_sidetable_entry_builder, nullptr};
+      return {{},           {}, WasmCode::kWasmStackGuard,     pos,
+              regs_to_save, 0,  debug_sidetable_entry_builder, spilled_regs};
     }
   };
 
@@ -503,8 +504,14 @@ class LiftoffCompiler {
   void StackCheck(WasmCodePosition position) {
     DEBUG_CODE_COMMENT("stack check");
     if (!FLAG_wasm_stack_checks || !env_->runtime_exception_support) return;
+    LiftoffRegList regs_to_save = __ cache_state()->used_registers;
+    SpilledRegistersForInspection* spilled_regs = nullptr;
+    if (V8_UNLIKELY(for_debugging_)) {
+      regs_to_save = {};
+      spilled_regs = GetSpilledRegistersForInspection();
+    }
     out_of_line_code_.push_back(OutOfLineCode::StackCheck(
-        position, __ cache_state()->used_registers,
+        position, regs_to_save, spilled_regs,
         RegisterDebugSideTableEntry(DebugSideTableBuilder::kAssumeSpilling)));
     OutOfLineCode& ool = out_of_line_code_.back();
     Register limit_address = __ GetUnusedRegister(kGpReg, {}).gp();
@@ -731,6 +738,14 @@ class LiftoffCompiler {
     DCHECK_EQ(ool->continuation.get()->is_bound(), is_stack_check);
     if (!ool->regs_to_save.is_empty()) __ PopRegisters(ool->regs_to_save);
     if (is_stack_check) {
+      // TODO(thibaudm): If the top frame is OSR'ed during stack check,
+      // execution will resume at the next instruction, skipping the following
+      // register reloads.
+      if (V8_UNLIKELY(ool->spilled_registers != nullptr)) {
+        for (auto& entry : ool->spilled_registers->entries) {
+          __ Fill(entry.reg, entry.offset, entry.type);
+        }
+      }
       __ emit_jump(ool->continuation.get());
     } else {
       __ AssertUnreachable(AbortReason::kUnexpectedReturnFromWasmTrap);
@@ -1944,16 +1959,16 @@ class LiftoffCompiler {
     __ cache_state()->Steal(c->else_state->state);
   }
 
-  SpilledRegistersBeforeTrap* GetSpilledRegistersBeforeTrap() {
+  SpilledRegistersForInspection* GetSpilledRegistersForInspection() {
     DCHECK(for_debugging_);
     // If we are generating debugging code, we really need to spill all
     // registers to make them inspectable when stopping at the trap.
-    auto* spilled =
-        compilation_zone_->New<SpilledRegistersBeforeTrap>(compilation_zone_);
+    auto* spilled = compilation_zone_->New<SpilledRegistersForInspection>(
+        compilation_zone_);
     for (uint32_t i = 0, e = __ cache_state()->stack_height(); i < e; ++i) {
       auto& slot = __ cache_state()->stack_state[i];
       if (!slot.is_reg()) continue;
-      spilled->entries.push_back(SpilledRegistersBeforeTrap::Entry{
+      spilled->entries.push_back(SpilledRegistersForInspection::Entry{
           slot.offset(), slot.reg(), slot.type()});
     }
     return spilled;
@@ -1966,7 +1981,7 @@ class LiftoffCompiler {
     out_of_line_code_.push_back(OutOfLineCode::Trap(
         stub, position, pc,
         RegisterDebugSideTableEntry(DebugSideTableBuilder::kAssumeSpilling),
-        V8_UNLIKELY(for_debugging_) ? GetSpilledRegistersBeforeTrap()
+        V8_UNLIKELY(for_debugging_) ? GetSpilledRegistersForInspection()
                                     : nullptr));
     return out_of_line_code_.back().label.get();
   }

test/inspector/debugger/wasm-stack-check-expected.txt

+Tests pausing a running script and stepping
+Instantiate
+Wait for script
+Got wasm script: wasm://wasm/c84b7cde
+Run
+Expecting to pause at 61
+Paused at offset 61; local: [12]; wasm-expression-stack: []
+Finished!

test/inspector/debugger/wasm-stack-check.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: --allow-natives-syntax
+
+utils.load('test/inspector/wasm-inspector-test.js');
+
+let {session, contextGroup, Protocol} =
+    InspectorTest.start('Tests pausing a running script and stepping');
+
+var builder = new WasmModuleBuilder();
+
+let pause = builder.addImport('imports', 'pause', kSig_v_v);
+let f = builder.addFunction('f', kSig_i_i)
+  .addBody([
+      kExprLocalGet, 0,
+      kExprI32Const, 1,
+      kExprI32Add]);
+let main = builder.addFunction('main', kSig_i_v)
+    .addBody([
+        kExprCallFunction, pause,
+        kExprI32Const, 12, kExprCallFunction, f.index])
+  .exportFunc();
+
+var module_bytes = builder.toArray();
+
+function instantiate(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, imports);
+}
+
+(async function pauseAndStep() {
+  await Protocol.Debugger.enable();
+  InspectorTest.log('Instantiate');
+  const instantiate_code = `var instance = (${instantiate})(${JSON.stringify(module_bytes)}, {'imports': {'pause': () => { %ScheduleBreak() } }});`;
+  WasmInspectorTest.evalWithUrl(instantiate_code, 'instantiate');
+  InspectorTest.log('Wait for script');
+  const [, {params: wasmScript}] = await Protocol.Debugger.onceScriptParsed(2);
+  InspectorTest.log('Got wasm script: ' + wasmScript.url);
+
+  InspectorTest.log('Run');
+  Protocol.Runtime.evaluate({expression: 'instance.exports.main()'});
+  // TODO(thibaudm): Fix source position and OSR at stack checks.
+  InspectorTest.log('Expecting to pause at ' + (f.body_offset - 1));
+  // await waitForPauseAndStep('stepInto');
+  // await waitForPauseAndStep('stepInto');
+  // await waitForPauseAndStep('stepInto');
+  // await waitForPauseAndStep('stepInto');
+  await waitForPauseAndStep('resume');
+  InspectorTest.log('Finished!');
+  InspectorTest.completeTest();
+})();
+
+async function waitForPauseAndStep(stepAction) {
+  const msg = await Protocol.Debugger.oncePaused();
+  await inspect(msg.params.callFrames[0]);
+  Protocol.Debugger[stepAction]();
+}
+
+async function inspect(frame) {
+  let loc = frame.location;
+  let line = [`Paused at offset ${loc.columnNumber}`];
+  // Inspect only the top wasm frame.
+  for (var scope of frame.scopeChain) {
+    if (scope.type == 'module') continue;
+    var scope_properties =
+        await Protocol.Runtime.getProperties({objectId: scope.object.objectId});
+    let str = scope_properties.result.result.map(
+        elem => WasmInspectorTest.getWasmValue(elem.value)).join(', ');
+    line.push(`${scope.type}: [${str}]`);
+  }
+  InspectorTest.log(line.join('; '));
+}