[wasm] [debugging] Fix --wasm-gdb-remote

The GDB-stub for Wasm debugging (which builds with the flag
v8_enable_wasm_gdb_remote_debugging) doesn't build anymore after a few changes
in the interface of wasm::DebugInfo.
This CL fixes the build, and also adds a few small changes to the protocol.

Change-Id: I250a8c86fd83048434e68cbdc5cb8ae243577393
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2571341Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Philip Pfaffe <pfaffe@chromium.org>
Commit-Queue: Paolo Severini <paolosev@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#71669}

src/debug/wasm/gdb-server/gdb-server.cc

@@ -105,7 +105,9 @@ class TaskRunner {
   std::atomic<bool> is_terminated_;
 };
 
-GdbServer::GdbServer() { task_runner_ = std::make_unique<TaskRunner>(); }
+GdbServer::GdbServer() : has_module_list_changed_(false) {
+  task_runner_ = std::make_unique<TaskRunner>();
+}
 
 template <typename Functor>
 auto GdbServer::RunSyncTask(Functor&& callback) const {
@@ -147,17 +149,20 @@ void GdbServer::RunMessageLoopOnPause() { task_runner_->Run(); }
 
 void GdbServer::QuitMessageLoopOnPause() { task_runner_->Terminate(); }
 
-std::vector<GdbServer::WasmModuleInfo> GdbServer::GetLoadedModules() {
+std::vector<GdbServer::WasmModuleInfo> GdbServer::GetLoadedModules(
+    bool clear_module_list_changed_flag) {
   // Executed in the GDBServerThread.
   std::vector<GdbServer::WasmModuleInfo> modules;
 
-  RunSyncTask([this, &modules]() {
+  RunSyncTask([this, &modules, clear_module_list_changed_flag]() {
     // Executed in the isolate thread.
     for (const auto& pair : scripts_) {
       uint32_t module_id = pair.first;
       const WasmModuleDebug& module_debug = pair.second;
       modules.push_back({module_id, module_debug.GetModuleName()});
     }
+
+    if (clear_module_list_changed_flag) has_module_list_changed_ = false;
   });
   return modules;
 }
@@ -216,14 +221,32 @@ bool GdbServer::GetWasmStackValue(uint32_t frame_index, uint32_t index,
   return result;
 }
 
-uint32_t GdbServer::GetWasmMemory(uint32_t frame_index, uint32_t offset,
+uint32_t GdbServer::GetWasmMemory(uint32_t module_id, uint32_t offset,
                                   uint8_t* buffer, uint32_t size) {
   // Executed in the GDBServerThread.
   uint32_t bytes_read = 0;
-  RunSyncTask([this, &bytes_read, frame_index, offset, buffer, size]() {
+  RunSyncTask([this, &bytes_read, module_id, offset, buffer, size]() {
     // Executed in the isolate thread.
-    bytes_read = WasmModuleDebug::GetWasmMemory(
-        GetTarget().GetCurrentIsolate(), frame_index, offset, buffer, size);
+    WasmModuleDebug* module_debug = nullptr;
+    if (GetModuleDebugHandler(module_id, &module_debug)) {
+      bytes_read = module_debug->GetWasmMemory(GetTarget().GetCurrentIsolate(),
+                                               offset, buffer, size);
+    }
+  });
+  return bytes_read;
+}
+
+uint32_t GdbServer::GetWasmData(uint32_t module_id, uint32_t offset,
+                                uint8_t* buffer, uint32_t size) {
+  // Executed in the GDBServerThread.
+  uint32_t bytes_read = 0;
+  RunSyncTask([this, &bytes_read, module_id, offset, buffer, size]() {
+    // Executed in the isolate thread.
+    WasmModuleDebug* module_debug = nullptr;
+    if (GetModuleDebugHandler(module_id, &module_debug)) {
+      bytes_read = module_debug->GetWasmData(GetTarget().GetCurrentIsolate(),
+                                             offset, buffer, size);
+    }
   });
   return bytes_read;
 }
@@ -305,6 +328,7 @@ void GdbServer::RemoveIsolate(Isolate* isolate) {
     for (auto it = scripts_.begin(); it != scripts_.end();) {
       if (it->second.GetIsolate() == isolate) {
         it = scripts_.erase(it);
+        has_module_list_changed_ = true;
       } else {
         ++it;
       }
@@ -351,6 +375,7 @@ void GdbServer::AddWasmModule(uint32_t module_id,
   v8::Isolate* isolate = wasm_script->GetIsolate();
   scripts_.insert(
       std::make_pair(module_id, WasmModuleDebug(isolate, wasm_script)));
+  has_module_list_changed_ = true;
 
   if (FLAG_wasm_pause_waiting_for_debugger && scripts_.size() == 1) {
     TRACE_GDB_REMOTE("Paused, waiting for a debugger to attach...\n");

src/debug/wasm/gdb-server/gdb-server.h

@@ -44,7 +44,10 @@ class GdbServer {
     uint32_t module_id;
     std::string module_name;
   };
-  std::vector<WasmModuleInfo> GetLoadedModules();
+  std::vector<WasmModuleInfo> GetLoadedModules(
+      bool clear_module_list_changed_flag = false);
+
+  bool HasModuleListChanged() const { return has_module_list_changed_; }
 
   // Queries the value of the {index} global value in the Wasm module identified
   // by {frame_index}.
@@ -64,13 +67,21 @@ class GdbServer {
                          uint32_t buffer_size, uint32_t* size);
 
   // Reads {size} bytes, starting from {offset}, from the Memory instance
-  // associated to the Wasm module identified by {frame_index}.
+  // associated to the Wasm module identified by {module_id}.
   // Returns the number of bytes copied to {buffer}, or 0 is case of error.
   // Note: only one Memory for Module is currently supported.
   //
-  uint32_t GetWasmMemory(uint32_t frame_index, uint32_t offset, uint8_t* buffer,
+  uint32_t GetWasmMemory(uint32_t module_id, uint32_t offset, uint8_t* buffer,
                          uint32_t size);
 
+  // Reads {size} bytes, starting from {offset}, from the first Data segment
+  // in the Wasm module identified by {module_id}.
+  // Returns the number of bytes copied to {buffer}, or 0 is case of error.
+  // Note: only one Memory for Module is currently supported.
+  //
+  uint32_t GetWasmData(uint32_t module_id, uint32_t offset, uint8_t* buffer,
+                       uint32_t size);
+
   // Reads {size} bytes, starting from the low dword of {address}, from the Code
   // space of th Wasm module identified by high dword of {address}.
   // Returns the number of bytes copied to {buffer}, or 0 is case of error.
@@ -176,6 +187,8 @@ class GdbServer {
   // tasks executed in the main (isolate) thread.
   std::unique_ptr<TaskRunner> task_runner_;
 
+  std::atomic<bool> has_module_list_changed_;
+
   //////////////////////////////////////////////////////////////////////////////
   // Always accessed in the isolate thread.
 

src/debug/wasm/gdb-server/target.cc

@@ -37,7 +37,7 @@ Target::Target(GdbServer* gdb_server)
 void Target::InitQueryPropertyMap() {
   // Request LLDB to send packets up to 4000 bytes for bulk transfers.
   query_properties_["Supported"] =
-      "PacketSize=1000;vContSupported-;qXfer:libraries:read+;";
+      "PacketSize=1000;vContSupported-;qXfer:libraries:read+;wasm+;";
 
   query_properties_["Attached"] = "1";
 
@@ -486,7 +486,7 @@ Target::ErrorCode Target::ProcessQueryPacket(const Packet* pkt_in,
   std::string tmp = "Xfer:libraries:read";
   if (!strncmp(str, tmp.data(), tmp.length())) {
     std::vector<GdbServer::WasmModuleInfo> modules =
-        gdb_server_->GetLoadedModules();
+        gdb_server_->GetLoadedModules(true);
     std::string result("l<library-list>");
     for (const auto& module : modules) {
       wasm_addr_t address(module.module_id, 0);
@@ -583,23 +583,44 @@ Target::ErrorCode Target::ProcessQueryPacket(const Packet* pkt_in,
     return ErrorCode::BadFormat;
   }
 
-  // Read Wasm memory.
-  // IN : $qWasmMem:frame_index;addr;len
+  // Read Wasm Memory.
+  // IN : $qWasmMem:module_id;addr;len
   // OUT: $xx..xx
   if (toks[0] == "WasmMem") {
     if (toks.size() == 4) {
-      uint32_t frame_index =
-          static_cast<uint32_t>(strtol(toks[1].data(), nullptr, 10));
-      uint32_t address =
-          static_cast<uint32_t>(strtol(toks[2].data(), nullptr, 16));
-      uint32_t length =
-          static_cast<uint32_t>(strtol(toks[3].data(), nullptr, 16));
+      uint32_t module_id = strtoul(toks[1].data(), nullptr, 10);
+      uint32_t address = strtoul(toks[2].data(), nullptr, 16);
+      uint32_t length = strtoul(toks[3].data(), nullptr, 16);
+      if (length > Transport::kBufSize / 2) {
+        return ErrorCode::BadArgs;
+      }
+      uint8_t buff[Transport::kBufSize];
+      uint32_t read =
+          gdb_server_->GetWasmMemory(module_id, address, buff, length);
+      if (read > 0) {
+        pkt_out->AddBlock(buff, read);
+        return ErrorCode::None;
+      } else {
+        return ErrorCode::Failed;
+      }
+    }
+    return ErrorCode::BadFormat;
+  }
+
+  // Read Wasm Data.
+  // IN : $qWasmData:module_id;addr;len
+  // OUT: $xx..xx
+  if (toks[0] == "WasmData") {
+    if (toks.size() == 4) {
+      uint32_t module_id = strtoul(toks[1].data(), nullptr, 10);
+      uint32_t address = strtoul(toks[2].data(), nullptr, 16);
+      uint32_t length = strtoul(toks[3].data(), nullptr, 16);
       if (length > Transport::kBufSize / 2) {
         return ErrorCode::BadArgs;
       }
       uint8_t buff[Transport::kBufSize];
       uint32_t read =
-          gdb_server_->GetWasmMemory(frame_index, address, buff, length);
+          gdb_server_->GetWasmData(module_id, address, buff, length);
       if (read > 0) {
         pkt_out->AddBlock(buff, read);
         return ErrorCode::None;
@@ -640,6 +661,10 @@ void Target::SetStopReply(Packet* pkt_out) const {
   // Adds 'thread:<tid>;' pair. Note that a terminating ';' is required.
   pkt_out->AddString("thread:");
   pkt_out->AddNumberSep(kThreadId, ';');
+
+  // If the loaded modules have changed since the last stop packet, signals
+  // that.
+  if (gdb_server_->HasModuleListChanged()) pkt_out->AddString("library:;");
 }
 
 void Target::SetStatus(Status status, int8_t signal,

src/debug/wasm/gdb-server/wasm-module-debug.cc

@@ -10,6 +10,7 @@
 #include "src/execution/frames-inl.h"
 #include "src/execution/frames.h"
 #include "src/objects/script.h"
+#include "src/wasm/module-instantiate.h"
 #include "src/wasm/wasm-debug.h"
 #include "src/wasm/wasm-value.h"
 
@@ -237,10 +238,10 @@ bool WasmModuleDebug::GetWasmLocal(Isolate* isolate, uint32_t frame_index,
                                              isolate);
       wasm::NativeModule* native_module = module_object->native_module();
       DebugInfo* debug_info = native_module->GetDebugInfo();
-      if (static_cast<uint32_t>(debug_info->GetNumLocals(
-              isolate, frame_it.frame()->pc())) > index) {
+      if (static_cast<uint32_t>(
+              debug_info->GetNumLocals(frame_it.frame()->pc())) > index) {
         wasm::WasmValue wasm_value = debug_info->GetLocalValue(
-            index, isolate, frame_it.frame()->pc(), frame_it.frame()->fp(),
+            index, frame_it.frame()->pc(), frame_it.frame()->fp(),
             frame_it.frame()->callee_fp());
         return GetWasmValue(wasm_value, buffer, buffer_size, size);
       }
@@ -270,10 +271,10 @@ bool WasmModuleDebug::GetWasmStackValue(Isolate* isolate, uint32_t frame_index,
                                              isolate);
       wasm::NativeModule* native_module = module_object->native_module();
       DebugInfo* debug_info = native_module->GetDebugInfo();
-      if (static_cast<uint32_t>(debug_info->GetStackDepth(
-              isolate, frame_it.frame()->pc())) > index) {
+      if (static_cast<uint32_t>(
+              debug_info->GetStackDepth(frame_it.frame()->pc())) > index) {
         WasmValue wasm_value = debug_info->GetStackValue(
-            index, isolate, frame_it.frame()->pc(), frame_it.frame()->fp(),
+            index, frame_it.frame()->pc(), frame_it.frame()->fp(),
             frame_it.frame()->callee_fp());
         return GetWasmValue(wasm_value, buffer, buffer_size, size);
       }
@@ -282,14 +283,12 @@ bool WasmModuleDebug::GetWasmStackValue(Isolate* isolate, uint32_t frame_index,
   return false;
 }
 
-// static
-uint32_t WasmModuleDebug::GetWasmMemory(Isolate* isolate, uint32_t frame_index,
-                                        uint32_t offset, uint8_t* buffer,
-                                        uint32_t size) {
+uint32_t WasmModuleDebug::GetWasmMemory(Isolate* isolate, uint32_t offset,
+                                        uint8_t* buffer, uint32_t size) {
   HandleScope handles(isolate);
 
   uint32_t bytes_read = 0;
-  Handle<WasmInstanceObject> instance = GetWasmInstance(isolate, frame_index);
+  Handle<WasmInstanceObject> instance = GetFirstWasmInstance();
   if (!instance.is_null()) {
     uint8_t* mem_start = instance->memory_start();
     size_t mem_size = instance->memory_size();
@@ -304,6 +303,34 @@ uint32_t WasmModuleDebug::GetWasmMemory(Isolate* isolate, uint32_t frame_index,
   return bytes_read;
 }
 
+uint32_t WasmModuleDebug::GetWasmData(Isolate* isolate, uint32_t offset,
+                                      uint8_t* buffer, uint32_t size) {
+  HandleScope handles(isolate);
+
+  uint32_t bytes_read = 0;
+  Handle<WasmInstanceObject> instance = GetFirstWasmInstance();
+  if (!instance.is_null()) {
+    Handle<WasmModuleObject> module_object(instance->module_object(), isolate);
+    const wasm::WasmModule* module = module_object->module();
+    if (!module->data_segments.empty()) {
+      const WasmDataSegment& segment = module->data_segments[0];
+      uint32_t data_offset = EvalUint32InitExpr(instance, segment.dest_addr);
+      offset += data_offset;
+
+      uint8_t* mem_start = instance->memory_start();
+      size_t mem_size = instance->memory_size();
+      if (static_cast<uint64_t>(offset) + size <= mem_size) {
+        memcpy(buffer, mem_start + offset, size);
+        bytes_read = size;
+      } else if (offset < mem_size) {
+        bytes_read = static_cast<uint32_t>(mem_size) - offset;
+        memcpy(buffer, mem_start + offset, bytes_read);
+      }
+    }
+  }
+  return bytes_read;
+}
+
 uint32_t WasmModuleDebug::GetWasmModuleBytes(wasm_addr_t wasm_addr,
                                              uint8_t* buffer, uint32_t size) {
   uint32_t bytes_read = 0;
@@ -373,8 +400,6 @@ bool WasmModuleDebug::GetWasmValue(const wasm::WasmValue& wasm_value,
 
     case wasm::kWasmStmt.kind():
     case wasm::kWasmExternRef.kind():
-    case wasm::kWasmFuncRef.kind():
-    case wasm::kWasmExnRef.kind():
     case wasm::kWasmBottom.kind():
     default:
       // Not supported

src/debug/wasm/gdb-server/wasm-module-debug.h

@@ -49,9 +49,15 @@ class WasmModuleDebug {
   // associated to this module.
   // Returns the number of byte copied to {buffer}, or 0 is case of error.
   // Note: only one Memory for Module is currently supported.
-  static uint32_t GetWasmMemory(Isolate* isolate, uint32_t frame_index,
-                                uint32_t offset, uint8_t* buffer,
-                                uint32_t size);
+  uint32_t GetWasmMemory(Isolate* isolate, uint32_t offset, uint8_t* buffer,
+                         uint32_t size);
+
+  // Reads {size} bytes, starting from {offset}, from the first segment
+  // associated to this module.
+  // Returns the number of byte copied to {buffer}, or 0 is case of error.
+  // Note: only one Memory for Module is currently supported.
+  uint32_t GetWasmData(Isolate* isolate, uint32_t offset, uint8_t* buffer,
+                       uint32_t size);
 
   // Gets {size} bytes, starting from {offset}, from the Code space of this
   // module.

src/wasm/module-instantiate.cc

@@ -33,11 +33,6 @@ namespace wasm {
 using base::ReadLittleEndianValue;
 using base::WriteLittleEndianValue;
 
-namespace {
-byte* raw_buffer_ptr(MaybeHandle<JSArrayBuffer> buffer, int offset) {
-  return static_cast<byte*>(buffer.ToHandleChecked()->backing_store()) + offset;
-}
-
 uint32_t EvalUint32InitExpr(Handle<WasmInstanceObject> instance,
                             const WasmInitExpr& expr) {
   switch (expr.kind()) {
@@ -56,6 +51,12 @@ uint32_t EvalUint32InitExpr(Handle<WasmInstanceObject> instance,
   }
 }
 
+namespace {
+
+byte* raw_buffer_ptr(MaybeHandle<JSArrayBuffer> buffer, int offset) {
+  return static_cast<byte*>(buffer.ToHandleChecked()->backing_store()) + offset;
+}
+
 using ImportWrapperQueue = WrapperQueue<WasmImportWrapperCache::CacheKey,
                                         WasmImportWrapperCache::CacheKeyHash>;
 

src/wasm/module-instantiate.h

@@ -16,6 +16,7 @@ namespace internal {
 class Isolate;
 class JSArrayBuffer;
 class JSReceiver;
+class WasmInitExpr;
 class WasmModuleObject;
 class WasmInstanceObject;
 
@@ -37,6 +38,9 @@ bool LoadElemSegment(Isolate* isolate, Handle<WasmInstanceObject> instance,
                      uint32_t table_index, uint32_t segment_index, uint32_t dst,
                      uint32_t src, uint32_t count) V8_WARN_UNUSED_RESULT;
 
+uint32_t EvalUint32InitExpr(Handle<WasmInstanceObject> instance,
+                            const WasmInitExpr& expr);
+
 }  // namespace wasm
 }  // namespace internal
 }  // namespace v8

test/debugging/wasm/gdb-server/gdb_rsp.py

@@ -178,7 +178,7 @@ def AssertReplySignal(reply, signal):
   AssertEquals(ParseThreadStopReply(reply)['signal'], signal)
 
 def ParseThreadStopReply(reply):
-  match = re.match('T([0-9a-f]{2})thread-pcs:([0-9a-f]+);thread:([0-9a-f]+);$', reply)
+  match = re.match('T([0-9a-f]{2})thread-pcs:([0-9a-f]+);thread:([0-9a-f]+);(library:([0-9a-f]*);)?$', reply)
   if not match:
     raise AssertionError('Bad thread stop reply: %r' % reply)
   return {'signal': int(match.group(1), 16),

test/debugging/wasm/gdb-server/memory.py

@@ -48,12 +48,29 @@ class Tests(unittest.TestCase):
       self.assertEqual(result, expected_data)
 
       # Check reading instance memory at a valid range.
-      reply = connection.RspRequest('qWasmMem:0;%x;%x' % (32, 4))
+      module_id = module_load_addr >> 32
+      reply = connection.RspRequest('qWasmMem:%d;%x;%x' % (module_id, 32, 4))
       value = struct.unpack('I', gdb_rsp.DecodeHex(reply))[0]
       self.assertEquals(int(value), 0)
 
       # Check reading instance memory at an invalid range.
-      reply = connection.RspRequest('qWasmMem:0;%x;%x' % (0xf0000000, 4))
+      reply = connection.RspRequest('qWasmMem:%d;%x;%x' % (module_id, 0xf0000000, 4))
+      self.assertEqual(reply, 'E03')
+
+  def test_reading_and_writing_data_section(self):
+    with gdb_rsp.LaunchDebugStub(COMMAND) as connection:
+      module_load_addr = gdb_rsp.GetLoadedModuleAddress(connection)
+      breakpoint_addr = module_load_addr + test_memory.FUNC0_START_ADDR
+      self.RunToWasm(connection, breakpoint_addr)
+
+      # Check reading instance memory at a valid range.
+      module_id = module_load_addr >> 32
+      reply = connection.RspRequest('qWasmData:%d;%x;%x' % (module_id, test_memory.DATA_OFFSET, test_memory.DATA_SIZE))
+      value = struct.unpack('I', gdb_rsp.DecodeHex(reply))[0]
+      self.assertEquals(int(value), test_memory.DATA_CONTENT)
+
+      # Check reading instance memory at an invalid range.
+      reply = connection.RspRequest('qWasmData:%d;%x;%x' % (module_id, 0xf0000000, 4))
       self.assertEqual(reply, 'E03')
 
   def test_wasm_global(self):

test/debugging/wasm/gdb-server/test_files/test_memory.js

@@ -10,6 +10,8 @@ builder.addGlobal(kWasmI32).exportAs('g_n');
 
 builder.addMemory(32, 128).exportMemoryAs('mem')
 
+builder.addDataSegment(0, [0x2a, 0x2b, 0x2c, 0x2d])
+
 var func_a_idx =
     builder.addFunction('wasm_A', kSig_v_i).addBody([kExprNop, kExprNop]).index;
 

test/debugging/wasm/gdb-server/test_files/test_memory.py

@@ -41,3 +41,6 @@ MEM_MAX = 128
 FUNC0_START_ADDR = 0x42
 FUNC1_RETURN_ADDR = 0x59
 FUNC1_START_ADDR = 0x47
+DATA_OFFSET = 0
+DATA_SIZE = 4
+DATA_CONTENT = 0x2d2c2b2a

test/unittests/wasm/wasm-gdbserver-unittest.cc

@@ -167,7 +167,6 @@ class MockTransport : public TransportBase {
   MOCK_METHOD(bool, AcceptConnection, (), (override));
   MOCK_METHOD(bool, Read, (char*, int32_t), (override));
   MOCK_METHOD(bool, Write, (const char*, int32_t), (override));
-  MOCK_METHOD(bool, IsDataAvailable, (), (override));
   MOCK_METHOD(bool, IsDataAvailable, (), (const, override));
   MOCK_METHOD(void, Disconnect, (), (override));
   MOCK_METHOD(void, Close, (), (override));