[wasm][devtools] Fix reported function body offsets

The DevTools frontend doesn't want the Wasm module's understanding of
function body offsets (i.e. including locals), but the ranges of
offsets where breakpoints can be set (i.e. only where instructions are).
This patch adjusts the reported offsets accordingly.
A consequence is that we have to report full (start,end) pairs for each
function, instead of being able to dedupe end1==start2 etc.

Bug: v8:12917
Change-Id: I0c7d2d96435cdac2c4553647b7bcc8783bc1798b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3780526
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Philip Pfaffe <pfaffe@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81887}

include/js_protocol.pdl

@@ -261,8 +261,8 @@ domain Debugger
       optional string streamId
       # The total number of lines in the disassembly text.
       integer totalNumberOfLines
-      # The offsets of all function bodies plus one additional entry pointing
-      # one by past the end of the last function.
+      # The offsets of all function bodies, in the format [start1, end1,
+      # start2, end2, ...] where all ends are exclusive.
       array of integer functionBodyOffsets
       # The first chunk of disassembly.
       WasmDisassemblyChunk chunk

src/debug/debug-interface.cc

@@ -847,27 +847,8 @@ int WasmScript::GetContainingFunction(int byte_offset) const {
   return i::wasm::GetContainingWasmFunction(module, byte_offset);
 }
 
-void WasmScript::GetAllFunctionStarts(std::vector<int>& starts) const {
-  i::DisallowGarbageCollection no_gc;
-  i::Handle<i::Script> script = Utils::OpenHandle(this);
-  DCHECK_EQ(i::Script::TYPE_WASM, script->type());
-  i::wasm::NativeModule* native_module = script->wasm_native_module();
-  const i::wasm::WasmModule* module = native_module->module();
-  size_t num_functions = module->functions.size();
-  starts.resize(num_functions + 1);
-  for (size_t i = 0; i < num_functions; i++) {
-    const i::wasm::WasmFunction& f = module->functions[i];
-    starts[i] = f.code.offset();
-  }
-  if (num_functions > 0) {
-    starts[num_functions] =
-        module->functions[num_functions - 1].code.end_offset();
-  } else {
-    starts[0] = 0;
-  }
-}
-
-void WasmScript::Disassemble(DisassemblyCollector* collector) {
+void WasmScript::Disassemble(DisassemblyCollector* collector,
+                             std::vector<int>* function_body_offsets) {
   i::DisallowGarbageCollection no_gc;
   i::Handle<i::Script> script = Utils::OpenHandle(this);
   DCHECK_EQ(i::Script::TYPE_WASM, script->type());
@@ -875,7 +856,7 @@ void WasmScript::Disassemble(DisassemblyCollector* collector) {
   const i::wasm::WasmModule* module = native_module->module();
   i::wasm::ModuleWireBytes wire_bytes(native_module->wire_bytes());
   i::wasm::Disassemble(module, wire_bytes, native_module->GetNamesProvider(),
-                       collector);
+                       collector, function_body_offsets);
 }
 
 uint32_t WasmScript::GetFunctionHash(int function_index) {

src/debug/debug-interface.h

@@ -258,11 +258,9 @@ class WasmScript : public Script {
 
   std::pair<int, int> GetFunctionRange(int function_index) const;
   int GetContainingFunction(int byte_offset) const;
-  // For N functions, {starts} will have N+1 entries: the last is the offset of
-  // the first byte after the end of the last function.
-  void GetAllFunctionStarts(std::vector<int>& starts) const;
 
-  void Disassemble(DisassemblyCollector* collector);
+  void Disassemble(DisassemblyCollector* collector,
+                   std::vector<int>* function_body_offsets);
 
   uint32_t GetFunctionHash(int function_index);
 

src/inspector/v8-debugger-agent-impl.cc

@@ -1214,11 +1214,10 @@ Response V8DebuggerAgentImpl::disassembleWasmModule(
   }
   std::unique_ptr<DisassemblyCollectorImpl> collector =
       std::make_unique<DisassemblyCollectorImpl>();
-  script->Disassemble(collector.get());
+  std::vector<int> functionBodyOffsets;
+  script->Disassemble(collector.get(), &functionBodyOffsets);
   *out_totalNumberOfLines =
       static_cast<int>(collector->total_number_of_lines());
-  std::vector<int> functionBodyOffsets;
-  script->GetAllFunctionStarts(functionBodyOffsets);
   *out_functionBodyOffsets =
       std::make_unique<protocol::Array<int>>(std::move(functionBodyOffsets));
   // Even an empty module would disassemble to "(module)", never to zero lines.

src/inspector/v8-debugger-script.cc

@@ -98,18 +98,13 @@ class ActualScript : public V8DebuggerScript {
     return v8::Just(String16(external_url.data(), external_url.size()));
   }
 
-  void GetAllFunctionStarts(std::vector<int>& starts) const override {
+  void Disassemble(v8::debug::DisassemblyCollector* collector,
+                   std::vector<int>* function_body_offsets) const override {
     v8::HandleScope scope(m_isolate);
     v8::Local<v8::debug::Script> script = this->script();
     DCHECK(script->IsWasm());
-    v8::debug::WasmScript::Cast(*script)->GetAllFunctionStarts(starts);
-  }
-
-  void Disassemble(v8::debug::DisassemblyCollector* collector) const override {
-    v8::HandleScope scope(m_isolate);
-    v8::Local<v8::debug::Script> script = this->script();
-    DCHECK(script->IsWasm());
-    v8::debug::WasmScript::Cast(*script)->Disassemble(collector);
+    v8::debug::WasmScript::Cast(*script)->Disassemble(collector,
+                                                      function_body_offsets);
   }
 #endif  // V8_ENABLE_WEBASSEMBLY
 

src/inspector/v8-debugger-script.h

@@ -105,9 +105,8 @@ class V8DebuggerScript {
   getDebugSymbolsType() const = 0;
   virtual v8::Maybe<String16> getExternalDebugSymbolsURL() const = 0;
   void removeWasmBreakpoint(int id);
-  virtual void GetAllFunctionStarts(std::vector<int>& starts) const = 0;
-  virtual void Disassemble(
-      v8::debug::DisassemblyCollector* collector) const = 0;
+  virtual void Disassemble(v8::debug::DisassemblyCollector* collector,
+                           std::vector<int>* function_body_offsets) const = 0;
 #endif  // V8_ENABLE_WEBASSEMBLY
 
  protected:

src/wasm/wasm-disassembler-impl.h

@@ -90,7 +90,8 @@ class V8_EXPORT_PRIVATE FunctionBodyDisassembler
         names_(names) {}
 
   void DecodeAsWat(MultiLineStringBuilder& out, Indentation indentation,
-                   FunctionHeader include_header = kPrintHeader);
+                   FunctionHeader include_header = kPrintHeader,
+                   uint32_t* first_instruction_offset = nullptr);
 
   void DecodeGlobalInitializer(StringBuilder& out);
 
@@ -128,14 +129,13 @@ class V8_EXPORT_PRIVATE FunctionBodyDisassembler
 
 class ModuleDisassembler {
  public:
-  enum ByteOffsets { kSkipByteOffsets = false, kIncludeByteOffsets = true };
-
-  V8_EXPORT_PRIVATE ModuleDisassembler(MultiLineStringBuilder& out,
-                                       const WasmModule* module,
-                                       NamesProvider* names,
-                                       const ModuleWireBytes wire_bytes,
-                                       ByteOffsets byte_offsets,
-                                       AccountingAllocator* allocator);
+  V8_EXPORT_PRIVATE ModuleDisassembler(
+      MultiLineStringBuilder& out, const WasmModule* module,
+      NamesProvider* names, const ModuleWireBytes wire_bytes,
+      AccountingAllocator* allocator,
+      // When non-nullptr, doubles as a sentinel that bytecode offsets should be
+      // stored for each line of disassembly.
+      std::vector<int>* function_body_offsets = nullptr);
   V8_EXPORT_PRIVATE ~ModuleDisassembler();
 
   V8_EXPORT_PRIVATE void PrintTypeDefinition(uint32_t type_index,
@@ -165,6 +165,7 @@ class ModuleDisassembler {
   const byte* start_;
   Zone zone_;
   std::unique_ptr<OffsetsProvider> offsets_;
+  std::vector<int>* function_body_offsets_;
 };
 
 }  // namespace wasm

src/wasm/wasm-disassembler.cc

@@ -20,11 +20,12 @@ namespace wasm {
 
 void Disassemble(const WasmModule* module, ModuleWireBytes wire_bytes,
                  NamesProvider* names,
-                 v8::debug::DisassemblyCollector* collector) {
+                 v8::debug::DisassemblyCollector* collector,
+                 std::vector<int>* function_body_offsets) {
   MultiLineStringBuilder out;
   AccountingAllocator allocator;
-  ModuleDisassembler md(out, module, names, wire_bytes,
-                        ModuleDisassembler::kIncludeByteOffsets, &allocator);
+  ModuleDisassembler md(out, module, names, wire_bytes, &allocator,
+                        function_body_offsets);
   md.PrintModule({0, 2});
   out.ToDisassemblyCollector(collector);
 }
@@ -149,7 +150,8 @@ void PrintSignatureOneLine(StringBuilder& out, const FunctionSig* sig,
 
 void FunctionBodyDisassembler::DecodeAsWat(MultiLineStringBuilder& out,
                                            Indentation indentation,
-                                           FunctionHeader include_header) {
+                                           FunctionHeader include_header,
+                                           uint32_t* first_instruction_offset) {
   out_ = &out;
   int base_indentation = indentation.current();
   // Print header.
@@ -184,6 +186,7 @@ void FunctionBodyDisassembler::DecodeAsWat(MultiLineStringBuilder& out,
   }
   consume_bytes(locals_length);
   out.set_current_line_bytecode_offset(pc_offset());
+  if (first_instruction_offset) *first_instruction_offset = pc_offset();
 
   // Main loop.
   while (pc_ < end_) {
@@ -662,16 +665,17 @@ ModuleDisassembler::ModuleDisassembler(MultiLineStringBuilder& out,
                                        const WasmModule* module,
                                        NamesProvider* names,
                                        const ModuleWireBytes wire_bytes,
-                                       ByteOffsets byte_offsets,
-                                       AccountingAllocator* allocator)
+                                       AccountingAllocator* allocator,
+                                       std::vector<int>* function_body_offsets)
     : out_(out),
       module_(module),
       names_(names),
       wire_bytes_(wire_bytes),
       start_(wire_bytes_.start()),
       zone_(allocator, "disassembler zone"),
-      offsets_(new OffsetsProvider()) {
-  if (byte_offsets == kIncludeByteOffsets) {
+      offsets_(new OffsetsProvider()),
+      function_body_offsets_(function_body_offsets) {
+  if (function_body_offsets != nullptr) {
     offsets_->CollectOffsets(module, wire_bytes_.start(), wire_bytes_.end(),
                              allocator);
   }
@@ -920,6 +924,11 @@ void ModuleDisassembler::PrintModule(Indentation indentation) {
   if (out_.length() != 0) out_.NextLine(0);
 
   // XI. Code / function bodies.
+  if (function_body_offsets_ != nullptr) {
+    size_t num_defined_functions =
+        module_->functions.size() - module_->num_imported_functions;
+    function_body_offsets_->reserve(num_defined_functions * 2);
+  }
   for (uint32_t i = module_->num_imported_functions;
        i < module_->functions.size(); i++) {
     const WasmFunction* func = &module_->functions[i];
@@ -935,7 +944,13 @@ void ModuleDisassembler::PrintModule(Indentation indentation) {
     FunctionBodyDisassembler d(&zone_, module_, i, &detected, func->sig,
                                code.begin(), code.end(), func->code.offset(),
                                names_);
-    d.DecodeAsWat(out_, indentation, FunctionBodyDisassembler::kSkipHeader);
+    uint32_t first_instruction_offset;
+    d.DecodeAsWat(out_, indentation, FunctionBodyDisassembler::kSkipHeader,
+                  &first_instruction_offset);
+    if (function_body_offsets_ != nullptr) {
+      function_body_offsets_->push_back(first_instruction_offset);
+      function_body_offsets_->push_back(d.pc_offset());
+    }
   }
 
   // XII. Data

src/wasm/wasm-disassembler.h

@@ -24,7 +24,8 @@ class NamesProvider;
 
 void Disassemble(const WasmModule* module, ModuleWireBytes wire_bytes,
                  NamesProvider* names,
-                 v8::debug::DisassemblyCollector* collector);
+                 v8::debug::DisassemblyCollector* collector,
+                 std::vector<int>* function_body_offsets);
 
 }  // namespace wasm
 }  // namespace internal

test/inspector/debugger/wasm-scripts-disassembly-expected.txt

@@ -14,7 +14,7 @@ bytecode:
 0x02 0x66 0x32 0x02 0x08 0x01 0x00 0x01  ;; offset 72..79
 0x01 0x03 0x78 0x79 0x7a
 streamid: undefined
-functionBodyOffsets: 28,40,44
+functionBodyOffsets: 33,39,41,44
 totalNumberOfLines: 13
 lines:
 (module $moduleName

tools/wasm/module-inspector.cc

@@ -795,8 +795,7 @@ class FormatConverter {
 
     // Print any types that were used by the function.
     out.NextLine(0);
-    ModuleDisassembler md(out, module(), names(), wire_bytes_,
-                          ModuleDisassembler::kSkipByteOffsets, &allocator_);
+    ModuleDisassembler md(out, module(), names(), wire_bytes_, &allocator_);
     for (uint32_t type_index : d.used_types()) {
       md.PrintTypeDefinition(type_index, {0, 1},
                              NamesProvider::kIndexAsComment);
@@ -805,8 +804,7 @@ class FormatConverter {
 
   void WatForModule(MultiLineStringBuilder& out) {
     DCHECK(ok_);
-    ModuleDisassembler md(out, module(), names(), wire_bytes_,
-                          ModuleDisassembler::kSkipByteOffsets, &allocator_);
+    ModuleDisassembler md(out, module(), names(), wire_bytes_, &allocator_);
     md.PrintModule({0, 2});
   }