[wasm] Load register values from DebugBreak frame

This implements inspection of live registers on breakpoints in Liftoff.
To that end, the frame pointer of the WasmDebugBreak frame is remembered
when iterating the stack. Based on a platform-specific implementation of
{WasmDebugBreakFrameConstants}, the offset of the respective register
within that frame is computed, and the value is read from the frame.

As a drive-by, the wasm debug side table is storing register codes as
liftoff codes, which can also store register pairs (needed for i64 on
32-bit platforms, and for SIMD, which is not supported yet).

R=jkummerow@chromium.org
CC=thibaudm@chromium.org

Bug: v8:10222
Change-Id: I01b669baf56430e100cd46cc46f210121ea679da
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2102574Reviewed-by: Simon Zünd <szuend@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66719}

src/debug/debug-scope-iterator.cc

@@ -176,7 +176,7 @@ v8::Local<v8::Object> DebugWasmScopeIterator::GetObject() {
       wasm::DebugInfo* debug_info =
           WasmCompiledFrame::cast(frame_)->native_module()->GetDebugInfo();
       return Utils::ToLocal(debug_info->GetLocalScopeObject(
-          isolate_, frame_->pc(), frame_->fp()));
+          isolate_, frame_->pc(), frame_->fp(), frame_->callee_fp()));
     }
     default:
       return {};

src/execution/arm/frame-constants-arm.h

@@ -52,6 +52,27 @@ class WasmDebugBreakFrameConstants : public TypedFrameConstants {
       base::bits::CountPopulation(kPushedGpRegs);
   static constexpr int kNumPushedFpRegisters =
       kLastPushedFpReg - kFirstPushedFpReg + 1;
+
+  static constexpr int kLastPushedGpRegisterOffset =
+      -TypedFrameConstants::kFixedFrameSizeFromFp -
+      kSystemPointerSize * kNumPushedGpRegisters;
+  static constexpr int kLastPushedFpRegisterOffset =
+      kLastPushedGpRegisterOffset - kDoubleSize * kNumPushedFpRegisters;
+
+  // Offsets are fp-relative.
+  static int GetPushedGpRegisterOffset(int reg_code) {
+    DCHECK_NE(0, kPushedGpRegs & (1 << reg_code));
+    uint32_t lower_regs = kPushedGpRegs & ((uint32_t{1} << reg_code) - 1);
+    return kLastPushedGpRegisterOffset +
+           base::bits::CountPopulation(lower_regs) * kSystemPointerSize;
+  }
+
+  static int GetPushedFpRegisterOffset(int reg_code) {
+    DCHECK_LE(kFirstPushedFpReg, reg_code);
+    DCHECK_GE(kLastPushedFpReg, reg_code);
+    return kLastPushedFpRegisterOffset +
+           (reg_code - kFirstPushedFpReg) * kDoubleSize;
+  }
 };
 
 }  // namespace internal

src/execution/arm64/frame-constants-arm64.h

@@ -104,6 +104,28 @@ class WasmDebugBreakFrameConstants : public TypedFrameConstants {
       base::bits::CountPopulation(kPushedGpRegs);
   static constexpr int kNumPushedFpRegisters =
       base::bits::CountPopulation(kPushedFpRegs);
+
+  static constexpr int kLastPushedGpRegisterOffset =
+      // Header is padded to 16 byte (see {MacroAssembler::EnterFrame}).
+      -RoundUp<16>(TypedFrameConstants::kFixedFrameSizeFromFp) -
+      kSystemPointerSize * kNumPushedGpRegisters;
+  static constexpr int kLastPushedFpRegisterOffset =
+      kLastPushedGpRegisterOffset - kDoubleSize * kNumPushedFpRegisters;
+
+  // Offsets are fp-relative.
+  static int GetPushedGpRegisterOffset(int reg_code) {
+    DCHECK_NE(0, kPushedGpRegs & (1 << reg_code));
+    uint32_t lower_regs = kPushedGpRegs & ((uint32_t{1} << reg_code) - 1);
+    return kLastPushedGpRegisterOffset +
+           base::bits::CountPopulation(lower_regs) * kSystemPointerSize;
+  }
+
+  static int GetPushedFpRegisterOffset(int reg_code) {
+    DCHECK_NE(0, kPushedFpRegs & (1 << reg_code));
+    uint32_t lower_regs = kPushedFpRegs & ((uint32_t{1} << reg_code) - 1);
+    return kLastPushedFpRegisterOffset +
+           base::bits::CountPopulation(lower_regs) * kDoubleSize;
+  }
 };
 
 }  // namespace internal

src/execution/frames.cc

@@ -896,6 +896,7 @@ void StandardFrame::ComputeCallerState(State* state) const {
   state->fp = caller_fp();
   state->pc_address = ResolveReturnAddressLocation(
       reinterpret_cast<Address*>(ComputePCAddress(fp())));
+  state->callee_fp = fp();
   state->callee_pc_address = pc_address();
   state->constant_pool_address =
       reinterpret_cast<Address*>(ComputeConstantPoolAddress(fp()));

src/execution/frames.h

@@ -118,6 +118,7 @@ class StackFrame {
     Address sp = kNullAddress;
     Address fp = kNullAddress;
     Address* pc_address = nullptr;
+    Address callee_fp = kNullAddress;
     Address* callee_pc_address = nullptr;
     Address* constant_pool_address = nullptr;
   };
@@ -217,6 +218,7 @@ class StackFrame {
   // Accessors.
   Address sp() const { return state_.sp; }
   Address fp() const { return state_.fp; }
+  Address callee_fp() const { return state_.callee_fp; }
   inline Address callee_pc() const;
   Address caller_sp() const { return GetCallerStackPointer(); }
 

src/execution/ia32/frame-constants-ia32.h

@@ -60,6 +60,26 @@ class WasmDebugBreakFrameConstants : public TypedFrameConstants {
       base::bits::CountPopulation(kPushedGpRegs);
   static constexpr int kNumPushedFpRegisters =
       base::bits::CountPopulation(kPushedFpRegs);
+
+  static constexpr int kLastPushedGpRegisterOffset =
+      -kFixedFrameSizeFromFp - kNumPushedGpRegisters * kSystemPointerSize;
+  static constexpr int kLastPushedFpRegisterOffset =
+      kLastPushedGpRegisterOffset - kNumPushedFpRegisters * kSimd128Size;
+
+  // Offsets are fp-relative.
+  static int GetPushedGpRegisterOffset(int reg_code) {
+    DCHECK_NE(0, kPushedGpRegs & (1 << reg_code));
+    uint32_t lower_regs = kPushedGpRegs & ((uint32_t{1} << reg_code) - 1);
+    return kLastPushedGpRegisterOffset +
+           base::bits::CountPopulation(lower_regs) * kSystemPointerSize;
+  }
+
+  static int GetPushedFpRegisterOffset(int reg_code) {
+    DCHECK_NE(0, kPushedFpRegs & (1 << reg_code));
+    uint32_t lower_regs = kPushedFpRegs & ((uint32_t{1} << reg_code) - 1);
+    return kLastPushedFpRegisterOffset +
+           base::bits::CountPopulation(lower_regs) * kSimd128Size;
+  }
 };
 
 }  // namespace internal

src/execution/x64/frame-constants-x64.h

@@ -69,6 +69,26 @@ class WasmDebugBreakFrameConstants : public TypedFrameConstants {
       base::bits::CountPopulation(kPushedGpRegs);
   static constexpr int kNumPushedFpRegisters =
       base::bits::CountPopulation(kPushedFpRegs);
+
+  static constexpr int kLastPushedGpRegisterOffset =
+      -kFixedFrameSizeFromFp - kNumPushedGpRegisters * kSystemPointerSize;
+  static constexpr int kLastPushedFpRegisterOffset =
+      kLastPushedGpRegisterOffset - kNumPushedFpRegisters * kSimd128Size;
+
+  // Offsets are fp-relative.
+  static int GetPushedGpRegisterOffset(int reg_code) {
+    DCHECK_NE(0, kPushedGpRegs & (1 << reg_code));
+    uint32_t lower_regs = kPushedGpRegs & ((uint32_t{1} << reg_code) - 1);
+    return kLastPushedGpRegisterOffset +
+           base::bits::CountPopulation(lower_regs) * kSystemPointerSize;
+  }
+
+  static int GetPushedFpRegisterOffset(int reg_code) {
+    DCHECK_NE(0, kPushedFpRegs & (1 << reg_code));
+    uint32_t lower_regs = kPushedFpRegs & ((uint32_t{1} << reg_code) - 1);
+    return kLastPushedFpRegisterOffset +
+           base::bits::CountPopulation(lower_regs) * kSimd128Size;
+  }
 };
 
 }  // namespace internal

src/wasm/baseline/liftoff-compiler.cc

@@ -208,8 +208,7 @@ class DebugSideTableBuilder {
           DCHECK_NE(kDidSpill, assume_spilling);
           if (assume_spilling == kAllowRegisters) {
             values[i].kind = DebugSideTable::Entry::kRegister;
-            values[i].reg_code =
-                slot.is_fp_reg() ? slot.fp_reg().code() : slot.gp_reg().code();
+            values[i].reg_code = slot.reg().liftoff_code();
             break;
           }
           DCHECK_EQ(kAssumeSpilling, assume_spilling);

src/wasm/baseline/liftoff-register.h

@@ -159,11 +159,17 @@ class LiftoffRegister {
     DCHECK_EQ(reg, fp());
   }
 
-  static LiftoffRegister from_liftoff_code(uint32_t code) {
-    DCHECK_LE(0, code);
-    DCHECK_GT(kAfterMaxLiftoffRegCode, code);
-    DCHECK_EQ(code, static_cast<storage_t>(code));
-    return LiftoffRegister(code);
+  static LiftoffRegister from_liftoff_code(int code) {
+    LiftoffRegister reg{static_cast<storage_t>(code)};
+    // Check that the code is correct by round-tripping through the
+    // reg-class-specific constructor.
+    DCHECK(
+        (reg.is_gp() && code == LiftoffRegister{reg.gp()}.liftoff_code()) ||
+        (reg.is_fp() && code == LiftoffRegister{reg.fp()}.liftoff_code()) ||
+        (reg.is_gp_pair() &&
+         code == ForPair(reg.low_gp(), reg.high_gp()).liftoff_code()) ||
+        (reg.is_fp_pair() && code == ForFpPair(reg.low_fp()).liftoff_code()));
+    return reg;
   }
 
   static LiftoffRegister from_code(RegClass rc, int code) {
@@ -257,8 +263,8 @@ class LiftoffRegister {
   }
 
   int liftoff_code() const {
-    DCHECK(is_gp() || is_fp());
-    return code_;
+    STATIC_ASSERT(sizeof(int) >= sizeof(storage_t));
+    return static_cast<int>(code_);
   }
 
   RegClass reg_class() const {

src/wasm/wasm-debug.cc

@@ -17,6 +17,7 @@
 #include "src/heap/factory.h"
 #include "src/utils/identity-map.h"
 #include "src/wasm/baseline/liftoff-compiler.h"
+#include "src/wasm/baseline/liftoff-register.h"
 #include "src/wasm/module-decoder.h"
 #include "src/wasm/wasm-code-manager.h"
 #include "src/wasm/wasm-interpreter.h"
@@ -536,8 +537,8 @@ class DebugInfoImpl {
   explicit DebugInfoImpl(NativeModule* native_module)
       : native_module_(native_module) {}
 
-  Handle<JSObject> GetLocalScopeObject(Isolate* isolate, Address pc,
-                                       Address fp) {
+  Handle<JSObject> GetLocalScopeObject(Isolate* isolate, Address pc, Address fp,
+                                       Address debug_break_fp) {
     Handle<JSObject> local_scope_object =
         isolate->factory()->NewJSObjectWithNullProto();
 
@@ -571,7 +572,8 @@ class DebugInfoImpl {
                  .ToHandle(&name)) {
           name = PrintFToOneByteString<true>(isolate, "var%d", i);
         }
-        WasmValue value = GetValue(debug_side_table_entry, i, fp);
+        WasmValue value =
+            GetValue(debug_side_table_entry, i, fp, debug_break_fp);
         Handle<Object> value_obj = WasmValueToValueObject(isolate, value);
         // {name} can be a string representation of an element index.
         LookupIterator::Key lookup_key{isolate, name};
@@ -596,7 +598,7 @@ class DebugInfoImpl {
                           NONE);
     int value_count = debug_side_table_entry->num_values();
     for (int i = num_locals; i < value_count; ++i) {
-      WasmValue value = GetValue(debug_side_table_entry, i, fp);
+      WasmValue value = GetValue(debug_side_table_entry, i, fp, debug_break_fp);
       Handle<Object> value_obj = WasmValueToValueObject(isolate, value);
       JSObject::AddDataElement(stack_obj, static_cast<uint32_t>(i - num_locals),
                                value_obj, NONE);
@@ -729,7 +731,8 @@ class DebugInfoImpl {
   // Get the value of a local (including parameters) or stack value. Stack
   // values follow the locals in the same index space.
   WasmValue GetValue(const DebugSideTable::Entry* debug_side_table_entry,
-                     int index, Address stack_frame_base) const {
+                     int index, Address stack_frame_base,
+                     Address debug_break_fp) const {
     ValueType type = debug_side_table_entry->value_type(index);
     if (debug_side_table_entry->is_constant(index)) {
       DCHECK(type == kWasmI32 || type == kWasmI64);
@@ -740,20 +743,35 @@ class DebugInfoImpl {
     }
 
     if (debug_side_table_entry->is_register(index)) {
-      // TODO(clemensb): Implement by loading from the frame of the
-      // WasmDebugBreak builtin. The current values are just placeholders.
-      switch (type.kind()) {
-        case ValueType::kI32:
-          return WasmValue(int32_t{-11});
-        case ValueType::kI64:
-          return WasmValue(int64_t{-11});
-        case ValueType::kF32:
-          return WasmValue(float{-11});
-        case ValueType::kF64:
-          return WasmValue(double{-11});
-        default:
-          UNIMPLEMENTED();
+      LiftoffRegister reg = LiftoffRegister::from_liftoff_code(
+          debug_side_table_entry->register_code(index));
+      auto gp_addr = [debug_break_fp](Register reg) {
+        return debug_break_fp +
+               WasmDebugBreakFrameConstants::GetPushedGpRegisterOffset(
+                   reg.code());
+      };
+      if (reg.is_gp_pair()) {
+        DCHECK_EQ(kWasmI64, type);
+        uint32_t low_word = ReadUnalignedValue<uint32_t>(gp_addr(reg.low_gp()));
+        uint32_t high_word =
+            ReadUnalignedValue<uint32_t>(gp_addr(reg.high_gp()));
+        return WasmValue((uint64_t{high_word} << 32) | low_word);
+      }
+      if (reg.is_gp()) {
+        return type == kWasmI32
+                   ? WasmValue(ReadUnalignedValue<uint32_t>(gp_addr(reg.gp())))
+                   : WasmValue(ReadUnalignedValue<uint64_t>(gp_addr(reg.gp())));
       }
+      // TODO(clemensb/zhin): Fix this for SIMD.
+      DCHECK(reg.is_fp() || reg.is_fp_pair());
+      if (reg.is_fp_pair()) UNIMPLEMENTED();
+      Address spilled_addr =
+          debug_break_fp +
+          WasmDebugBreakFrameConstants::GetPushedFpRegisterOffset(
+              reg.fp().code());
+      return type == kWasmF32
+                 ? WasmValue(ReadUnalignedValue<float>(spilled_addr))
+                 : WasmValue(ReadUnalignedValue<double>(spilled_addr));
     }
 
     // Otherwise load the value from the stack.
@@ -803,8 +821,9 @@ DebugInfo::DebugInfo(NativeModule* native_module)
 DebugInfo::~DebugInfo() = default;
 
 Handle<JSObject> DebugInfo::GetLocalScopeObject(Isolate* isolate, Address pc,
-                                                Address fp) {
-  return impl_->GetLocalScopeObject(isolate, pc, fp);
+                                                Address fp,
+                                                Address debug_break_fp) {
+  return impl_->GetLocalScopeObject(isolate, pc, fp, debug_break_fp);
 }
 
 WireBytesRef DebugInfo::GetLocalName(int func_index, int local_index) {

src/wasm/wasm-debug.h

@@ -141,7 +141,10 @@ class DebugInfo {
   explicit DebugInfo(NativeModule*);
   ~DebugInfo();
 
-  Handle<JSObject> GetLocalScopeObject(Isolate*, Address pc, Address fp);
+  // {fp} is the frame pointer of the Liftoff frame, {debug_break_fp} that of
+  // the {WasmDebugBreak} frame (if any).
+  Handle<JSObject> GetLocalScopeObject(Isolate*, Address pc, Address fp,
+                                       Address debug_break_fp);
 
   WireBytesRef GetLocalName(int func_index, int local_index);
 

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

@@ -57,7 +57,7 @@ at wasm_B (0:47):
  - scope (global):
  - scope (local):
    locals: "var0": 3 (number)
-   stack: "0": -11 (number)
+   stack: "0": 3 (number)
 at (anonymous) (0:17):
    -- skipped
 Paused:
@@ -77,7 +77,7 @@ at wasm_B (0:51):
  - scope (global):
  - scope (local):
    locals: "var0": 3 (number)
-   stack: "0": -11 (number)
+   stack: "0": 3 (number)
 at (anonymous) (0:17):
    -- skipped
 Paused:
@@ -87,7 +87,7 @@ at wasm_B (0:53):
  - scope (global):
  - scope (local):
    locals: "var0": 3 (number)
-   stack: "0": -11 (number), "1": 1 (number)
+   stack: "0": 3 (number), "1": 1 (number)
 at (anonymous) (0:17):
    -- skipped
 Paused:
@@ -97,7 +97,7 @@ at wasm_B (0:54):
  - scope (global):
  - scope (local):
    locals: "var0": 3 (number)
-   stack: "0": -11 (number)
+   stack: "0": 2 (number)
 at (anonymous) (0:17):
    -- skipped
 Paused:
@@ -145,7 +145,7 @@ at wasm_B (0:47):
  - scope (global):
  - scope (local):
    locals: "var0": 2 (number)
-   stack: "0": -11 (number)
+   stack: "0": 2 (number)
 at (anonymous) (0:17):
    -- skipped
 Paused:
@@ -165,7 +165,7 @@ at wasm_B (0:51):
  - scope (global):
  - scope (local):
    locals: "var0": 2 (number)
-   stack: "0": -11 (number)
+   stack: "0": 2 (number)
 at (anonymous) (0:17):
    -- skipped
 Paused:
@@ -175,7 +175,7 @@ at wasm_B (0:53):
  - scope (global):
  - scope (local):
    locals: "var0": 2 (number)
-   stack: "0": -11 (number), "1": 1 (number)
+   stack: "0": 2 (number), "1": 1 (number)
 at (anonymous) (0:17):
    -- skipped
 Paused:
@@ -185,7 +185,7 @@ at wasm_B (0:54):
  - scope (global):
  - scope (local):
    locals: "var0": 2 (number)
-   stack: "0": -11 (number)
+   stack: "0": 1 (number)
 at (anonymous) (0:17):
    -- skipped
 Paused:
@@ -233,7 +233,7 @@ at wasm_B (0:47):
  - scope (global):
  - scope (local):
    locals: "var0": 1 (number)
-   stack: "0": -11 (number)
+   stack: "0": 1 (number)
 at (anonymous) (0:17):
    -- skipped
 Paused:
@@ -253,7 +253,7 @@ at wasm_B (0:51):
  - scope (global):
  - scope (local):
    locals: "var0": 1 (number)
-   stack: "0": -11 (number)
+   stack: "0": 1 (number)
 at (anonymous) (0:17):
    -- skipped
 Paused:
@@ -263,7 +263,7 @@ at wasm_B (0:53):
  - scope (global):
  - scope (local):
    locals: "var0": 1 (number)
-   stack: "0": -11 (number), "1": 1 (number)
+   stack: "0": 1 (number), "1": 1 (number)
 at (anonymous) (0:17):
    -- skipped
 Paused:
@@ -273,7 +273,7 @@ at wasm_B (0:54):
  - scope (global):
  - scope (local):
    locals: "var0": 1 (number)
-   stack: "0": -11 (number)
+   stack: "0": 0 (number)
 at (anonymous) (0:17):
    -- skipped
 Paused:
@@ -321,7 +321,7 @@ at wasm_B (0:47):
  - scope (global):
  - scope (local):
    locals: "var0": 0 (number)
-   stack: "0": -11 (number)
+   stack: "0": 0 (number)
 at (anonymous) (0:17):
    -- skipped
 Paused: