[wasm] Fix debug side table for out-of-line code

For out-of-line code, we need to generate the debug side table
information at the point where the out-of-line code is being triggered,
not when it is emitted (at the end of the function).

This CL also adds more tests to check the actual content of the debug
side table in different scenarios.

R=jkummerow@chromium.org

Bug: v8:10019
Change-Id: I7714c86ee7edc4918b5ecc97cbded84c27b00e09
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1967388Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65481}

src/wasm/baseline/liftoff-compiler.cc

@@ -172,30 +172,34 @@ class DebugSideTableBuilder {
     }
 
     int pc_offset() const { return pc_offset_; }
+    void set_pc_offset(int new_pc_offset) { pc_offset_ = new_pc_offset; }
 
    private:
-    const int pc_offset_;
+    int pc_offset_;
     const int stack_height_;
     std::vector<DebugSideTable::Entry::Constant> constants_;
   };
 
-  // Adds a new entry, and returns a reference for modifying that entry. The
-  // reference can only be used until the next call to {NewEntry}.
-  EntryBuilder& NewEntry(int pc_offset, int stack_height) {
-    DCHECK(entries_.empty() || entries_.back().pc_offset() < pc_offset);
+  // Adds a new entry, and returns a pointer to a builder for modifying that
+  // entry.
+  EntryBuilder* NewEntry(int pc_offset, int stack_height) {
     entries_.emplace_back(pc_offset, stack_height);
-    return entries_.back();
+    return &entries_.back();
   }
 
   DebugSideTable GenerateDebugSideTable() {
     std::vector<DebugSideTable::Entry> table_entries;
     table_entries.reserve(entries_.size());
     for (auto& entry : entries_) table_entries.push_back(entry.ToTableEntry());
+    std::sort(table_entries.begin(), table_entries.end(),
+              [](DebugSideTable::Entry& a, DebugSideTable::Entry& b) {
+                return a.pc_offset() < b.pc_offset();
+              });
     return DebugSideTable{std::move(table_entries)};
   }
 
  private:
-  std::vector<EntryBuilder> entries_;
+  std::list<EntryBuilder> entries_;
 };
 
 class LiftoffCompiler {
@@ -231,15 +235,20 @@ class LiftoffCompiler {
     WasmCodePosition position;
     LiftoffRegList regs_to_save;
     uint32_t pc;  // for trap handler.
+    DebugSideTableBuilder::EntryBuilder* debug_sidetable_entry_builder;
 
     // Named constructors:
-    static OutOfLineCode Trap(WasmCode::RuntimeStubId s, WasmCodePosition pos,
-                              uint32_t pc) {
+    static OutOfLineCode Trap(
+        WasmCode::RuntimeStubId s, WasmCodePosition pos, uint32_t pc,
+        DebugSideTableBuilder::EntryBuilder* debug_sidetable_entry_builder) {
       DCHECK_LT(0, pos);
-      return {{}, {}, s, pos, {}, pc};
+      return {{}, {}, s, pos, {}, pc, debug_sidetable_entry_builder};
     }
-    static OutOfLineCode StackCheck(WasmCodePosition pos, LiftoffRegList regs) {
-      return {{}, {}, WasmCode::kWasmStackGuard, pos, regs, 0};
+    static OutOfLineCode StackCheck(
+        WasmCodePosition pos, LiftoffRegList regs,
+        DebugSideTableBuilder::EntryBuilder* debug_sidetable_entry_builder) {
+      return {{},   {}, WasmCode::kWasmStackGuard,    pos,
+              regs, 0,  debug_sidetable_entry_builder};
     }
   };
 
@@ -415,7 +424,8 @@ class LiftoffCompiler {
   void StackCheck(WasmCodePosition position) {
     if (FLAG_wasm_no_stack_checks || !env_->runtime_exception_support) return;
     out_of_line_code_.push_back(
-        OutOfLineCode::StackCheck(position, __ cache_state()->used_registers));
+        OutOfLineCode::StackCheck(position, __ cache_state()->used_registers,
+                                  RegisterDebugSideTableEntry()));
     OutOfLineCode& ool = out_of_line_code_.back();
     Register limit_address = __ GetUnusedRegister(kGpReg).gp();
     LOAD_INSTANCE_FIELD(limit_address, StackLimitAddress, kSystemPointerSize);
@@ -537,7 +547,10 @@ class LiftoffCompiler {
     source_position_table_builder_.AddPosition(
         __ pc_offset(), SourcePosition(ool->position), false);
     __ CallRuntimeStub(ool->stub);
-    RegisterDebugSideTableEntry();
+    DCHECK_EQ(!debug_sidetable_builder_, !ool->debug_sidetable_entry_builder);
+    if (V8_UNLIKELY(ool->debug_sidetable_entry_builder)) {
+      ool->debug_sidetable_entry_builder->set_pc_offset(__ pc_offset());
+    }
     safepoint_table_builder_.DefineSafepoint(&asm_, Safepoint::kNoLazyDeopt);
     DCHECK_EQ(ool->continuation.get()->is_bound(), is_stack_check);
     if (!ool->regs_to_save.is_empty()) __ PopRegisters(ool->regs_to_save);
@@ -1625,7 +1638,8 @@ class LiftoffCompiler {
     DCHECK_EQ(pc != 0, stub == WasmCode::kThrowWasmTrapMemOutOfBounds &&
                            env_->use_trap_handler);
 
-    out_of_line_code_.push_back(OutOfLineCode::Trap(stub, position, pc));
+    out_of_line_code_.push_back(
+        OutOfLineCode::Trap(stub, position, pc, RegisterDebugSideTableEntry()));
     return out_of_line_code_.back().label.get();
   }
 
@@ -1871,16 +1885,17 @@ class LiftoffCompiler {
     __ PushRegister(kWasmI32, result);
   }
 
-  void RegisterDebugSideTableEntry() {
-    if (V8_LIKELY(!debug_sidetable_builder_)) return;
+  DebugSideTableBuilder::EntryBuilder* RegisterDebugSideTableEntry() {
+    if (V8_LIKELY(!debug_sidetable_builder_)) return nullptr;
     int stack_height = static_cast<int>(__ cache_state()->stack_height());
-    auto entry =
+    auto* entry_builder =
         debug_sidetable_builder_->NewEntry(__ pc_offset(), stack_height);
     // Record all constants on the stack.
     for (int idx = 0; idx < stack_height; ++idx) {
       auto& slot = __ cache_state()->stack_state[idx];
-      if (slot.is_const()) entry.SetConstant(idx, slot.i32_const());
+      if (slot.is_const()) entry_builder->SetConstant(idx, slot.i32_const());
     }
+    return entry_builder;
   }
 
   void CallDirect(FullDecoder* decoder,

src/wasm/wasm-debug.h

@@ -8,6 +8,7 @@
 #include <algorithm>
 #include <vector>
 
+#include "src/base/iterator.h"
 #include "src/base/logging.h"
 #include "src/base/macros.h"
 
@@ -85,6 +86,10 @@ class DebugSideTable {
     return &*it;
   }
 
+  auto entries() const {
+    return base::make_iterator_range(entries_.begin(), entries_.end());
+  }
+
   size_t num_entries() const { return entries_.size(); }
 
  private:

test/cctest/wasm/test-liftoff-inspection.cc

@@ -120,6 +120,47 @@ class LiftoffCompileEnvironment {
   Zone zone_;
   TestingModuleBuilder module_builder_;
 };
+
+struct DebugSideTableEntry {
+  int stack_height;
+  std::vector<std::pair<int, int>> constants;
+
+  bool operator==(const DebugSideTableEntry& other) const {
+    return stack_height == other.stack_height && constants == other.constants;
+  }
+};
+
+// Debug builds will print the vector of DebugSideTableEntry.
+#ifdef DEBUG
+std::ostream& operator<<(std::ostream& out, const DebugSideTableEntry& entry) {
+  out << "{stack height " << entry.stack_height << ", constants: {";
+  const char* comma = "";
+  for (auto& c : entry.constants) {
+    out << comma << "{" << c.first << ", " << c.second << "}";
+    comma = ", ";
+  }
+  return out << "}}";
+}
+
+std::ostream& operator<<(std::ostream& out,
+                         const std::vector<DebugSideTableEntry>& entries) {
+  return out << PrintCollection(entries);
+}
+#endif  // DEBUG
+
+void CheckEntries(std::vector<DebugSideTableEntry> expected,
+                  const wasm::DebugSideTable& debug_side_table) {
+  std::vector<DebugSideTableEntry> entries;
+  for (auto& entry : debug_side_table.entries()) {
+    std::vector<std::pair<int, int>> constants;
+    for (int i = 0; i < entry.stack_height(); ++i) {
+      if (entry.IsConstant(i)) constants.emplace_back(i, entry.GetConstant(i));
+    }
+    entries.push_back({entry.stack_height(), std::move(constants)});
+  }
+  CHECK_EQ(expected, entries);
+}
+
 }  // namespace
 
 TEST(Liftoff_deterministic_simple) {
@@ -164,8 +205,11 @@ TEST(Liftoff_debug_side_table_simple) {
   auto debug_side_table = env.GenerateDebugSideTable(
       {kWasmI32}, {kWasmI32, kWasmI32},
       {WASM_I32_ADD(WASM_GET_LOCAL(0), WASM_GET_LOCAL(1))});
-  // 1 entry for the stack check.
-  CHECK_EQ(1, debug_side_table.num_entries());
+  CheckEntries(
+      {
+          {2, {}}  // OOL stack check, stack: {param0, param1}
+      },
+      debug_side_table);
 }
 
 TEST(Liftoff_debug_side_table_call) {
@@ -174,10 +218,28 @@ TEST(Liftoff_debug_side_table_call) {
       {kWasmI32}, {kWasmI32},
       {WASM_I32_ADD(WASM_CALL_FUNCTION(0, WASM_GET_LOCAL(0)),
                     WASM_GET_LOCAL(0))});
-  // 2 entries:
-  // - stack check
-  // - call
-  CHECK_EQ(2, debug_side_table.num_entries());
+  CheckEntries(
+      {
+          {1, {}},  // call, stack: {param0}
+          {1, {}}   // OOL stack check, stack: {param0}
+      },
+      debug_side_table);
+}
+
+TEST(Liftoff_debug_side_table_call_const) {
+  LiftoffCompileEnvironment env;
+  constexpr int kConst = 13;
+  auto debug_side_table = env.GenerateDebugSideTable(
+      {kWasmI32}, {kWasmI32},
+      {WASM_SET_LOCAL(0, WASM_I32V_1(kConst)),
+       WASM_I32_ADD(WASM_CALL_FUNCTION(0, WASM_GET_LOCAL(0)),
+                    WASM_GET_LOCAL(0))});
+  CheckEntries(
+      {
+          {1, {{0, kConst}}},  // call, stack: {kConst}
+          {1, {}}              // OOL stack check, stack: {param0}
+      },
+      debug_side_table);
 }
 
 TEST(Liftoff_debug_side_table_indirect_call) {
@@ -186,23 +248,28 @@ TEST(Liftoff_debug_side_table_indirect_call) {
       {kWasmI32}, {kWasmI32},
       {WASM_I32_ADD(WASM_CALL_INDIRECT(0, WASM_I32V_1(47), WASM_GET_LOCAL(0)),
                     WASM_GET_LOCAL(0))});
-  // 4 entries:
-  // - stack check
-  // - trap for invalid function index
-  // - trap for signature mismatch
-  // - indirect call
-  CHECK_EQ(4, debug_side_table.num_entries());
+  CheckEntries(
+      {
+          {1, {}},         // indirect call, stack: {param0}
+          {1, {}},         // OOL stack check, stack: {param0}
+          {2, {{1, 47}}},  // OOL invalid index, stack: {param0, 47}
+          {2, {{1, 47}}},  // OOL sig mismatch, stack: {param0, 47}
+      },
+      debug_side_table);
 }
 
 TEST(Liftoff_debug_side_table_loop) {
   LiftoffCompileEnvironment env;
+  constexpr int kConst = 42;
   auto debug_side_table = env.GenerateDebugSideTable(
       {kWasmI32}, {kWasmI32},
-      {WASM_LOOP(WASM_BR_IF(0, WASM_GET_LOCAL(0))), WASM_GET_LOCAL(0)});
-  // 2 entries:
-  // - stack check at function entry
-  // - stack check in loop header
-  CHECK_EQ(2, debug_side_table.num_entries());
+      {WASM_I32V_1(kConst), WASM_LOOP(WASM_BR_IF(0, WASM_GET_LOCAL(0)))});
+  CheckEntries(
+      {
+          {1, {}},            // OOL stack check, stack: {param0}
+          {2, {{1, kConst}}}  // OOL loop stack check, stack: {param0, kConst}
+      },
+      debug_side_table);
 }
 
 TEST(Liftoff_debug_side_table_trap) {
@@ -210,11 +277,13 @@ TEST(Liftoff_debug_side_table_trap) {
   auto debug_side_table = env.GenerateDebugSideTable(
       {kWasmI32}, {kWasmI32, kWasmI32},
       {WASM_I32_DIVS(WASM_GET_LOCAL(0), WASM_GET_LOCAL(1))});
-  // 3 entries:
-  // - stack check
-  // - trap for division by zero
-  // - trap for division result unrepresentable
-  CHECK_EQ(3, debug_side_table.num_entries());
+  CheckEntries(
+      {
+          {2, {}},  // OOL stack check, stack: {param0, param1}
+          {2, {}},  // OOL div by zero, stack: {param0, param1}
+          {2, {}}   // OOL unrepresentable div, stack: {param0, param1}
+      },
+      debug_side_table);
 }
 
 }  // namespace wasm