[Lldb-commits] [lldb] [lldb-dap] Support inspecting memory (PR #104317)

Adrian Vogelsgesang via lldb-commits lldb-commits at lists.llvm.org
Thu Aug 15 04:01:54 PDT 2024


https://github.com/vogelsgesang updated https://github.com/llvm/llvm-project/pull/104317

>From a5b4f6e7e105d36b82f9de588d2705ad3d622953 Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang <avogelsgesang at salesforce.com>
Date: Wed, 14 Aug 2024 11:52:40 +0000
Subject: [PATCH 1/5] [lldb-dap] Support inspecting memory

Adds support for the `readMemory` request which allows VS-Code to
inspect memory. Also, add `memoryReference` to variablesa and `evaluate`
responses, such that the binary view can be opened from the variables
view and from the "watch" pane.
---
 .../test/tools/lldb-dap/dap_server.py         |  13 ++
 lldb/test/API/tools/lldb-dap/memory/Makefile  |   3 +
 .../tools/lldb-dap/memory/TestDAP_memory.py   | 135 ++++++++++++++
 lldb/test/API/tools/lldb-dap/memory/main.cpp  |  10 +
 lldb/tools/lldb-dap/JSONUtils.cpp             |  29 ++-
 lldb/tools/lldb-dap/JSONUtils.h               |   6 +
 lldb/tools/lldb-dap/lldb-dap.cpp              | 172 +++++++++++++++++-
 7 files changed, 365 insertions(+), 3 deletions(-)
 create mode 100644 lldb/test/API/tools/lldb-dap/memory/Makefile
 create mode 100644 lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
 create mode 100644 lldb/test/API/tools/lldb-dap/memory/main.cpp

diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index a324af57b61df3..da10e8ab5c57c2 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -691,6 +691,19 @@ def request_disassemble(
         for inst in instructions:
             self.disassembled_instructions[inst["address"]] = inst
 
+    def request_read_memory(self, memoryReference, offset, count):
+        args_dict = {
+            "memoryReference": memoryReference,
+            "offset": offset,
+            "count": count,
+        }
+        command_dict = {
+            "command": "readMemory",
+            "type": "request",
+            "arguments": args_dict,
+        }
+        return self.send_recv(command_dict)
+
     def request_evaluate(self, expression, frameIndex=0, threadId=None, context=None):
         stackFrame = self.get_stackFrame(frameIndex=frameIndex, threadId=threadId)
         if stackFrame is None:
diff --git a/lldb/test/API/tools/lldb-dap/memory/Makefile b/lldb/test/API/tools/lldb-dap/memory/Makefile
new file mode 100644
index 00000000000000..99998b20bcb050
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/memory/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
new file mode 100644
index 00000000000000..f950d5eecda671
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
@@ -0,0 +1,135 @@
+"""
+Test lldb-dap memory support
+"""
+
+from base64 import b64decode
+import dap_server
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbdap_testcase
+import os
+
+
+class TestDAP_memory(lldbdap_testcase.DAPTestCaseBase):
+    def test_read_memory(self):
+        """
+        Tests the 'read_memory' request
+        """
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+        source = "main.cpp"
+        self.source_path = os.path.join(os.getcwd(), source)
+        self.set_source_breakpoints(
+            source,
+            [line_number(source, "// Breakpoint")],
+        )
+        self.continue_to_next_stop()
+
+        locals = {l["name"]: l for l in self.dap_server.get_local_variables()}
+        rawptr_ref = locals["rawptr"]["memoryReference"]
+
+        # We can read the complete string
+        mem = self.dap_server.request_read_memory(rawptr_ref, 0, 5)["body"]
+        self.assertEqual(mem["unreadableBytes"], 0)
+        self.assertEqual(b64decode(mem["data"]), b"dead\0")
+
+        # Use an offset
+        mem = self.dap_server.request_read_memory(rawptr_ref, 2, 3)["body"]
+        self.assertEqual(b64decode(mem["data"]), b"ad\0")
+
+        # Use a negative offset
+        mem = self.dap_server.request_read_memory(rawptr_ref, -1, 6)["body"]
+        self.assertEqual(b64decode(mem["data"])[1:], b"dead\0")
+
+        # Reads of size 0 are successful
+        # VS-Code sends those in order to check if a `memoryReference` can actually be dereferenced.
+        mem = self.dap_server.request_read_memory(rawptr_ref, 0, 0)
+        self.assertEqual(mem["success"], True)
+        self.assertEqual(mem["body"]["data"], "")
+
+        # Reads at offset 0x0 fail
+        mem = self.dap_server.request_read_memory("0x0", 0, 6)
+        self.assertEqual(mem["success"], False)
+        self.assertTrue(mem["message"].startswith("Unable to read memory: "))
+
+    def test_memory_refs_variables(self):
+        """
+        Tests memory references for evaluate
+        """
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+        source = "main.cpp"
+        self.source_path = os.path.join(os.getcwd(), source)
+        self.set_source_breakpoints(
+            source,
+            [line_number(source, "// Breakpoint")],
+        )
+        self.continue_to_next_stop()
+
+        locals = {l["name"]: l for l in self.dap_server.get_local_variables()}
+
+        # Pointers should have memory-references
+        self.assertIn("memoryReference", locals["rawptr"].keys())
+        # Smart pointers also have memory-references
+        self.assertIn(
+            "memoryReference",
+            self.dap_server.get_local_variable_child("smartptr", "pointer").keys(),
+        )
+        # Non-pointers should not have memory-references
+        self.assertNotIn("memoryReference", locals["not_a_ptr"].keys())
+
+    def test_memory_refs_evaluate(self):
+        """
+        Tests memory references for evaluate
+        """
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+        source = "main.cpp"
+        self.source_path = os.path.join(os.getcwd(), source)
+        self.set_source_breakpoints(
+            source,
+            [line_number(source, "// Breakpoint")],
+        )
+        self.continue_to_next_stop()
+
+        # Pointers contain memory references
+        self.assertIn(
+            "memoryReference",
+            self.dap_server.request_evaluate("rawptr + 1")["body"].keys(),
+        )
+
+        # Non-pointer expressions don't include a memory reference
+        self.assertNotIn(
+            "memoryReference",
+            self.dap_server.request_evaluate("1 + 3")["body"].keys(),
+        )
+
+    def test_memory_refs_set_variable(self):
+        """
+        Tests memory references for `setVariable`
+        """
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+        source = "main.cpp"
+        self.source_path = os.path.join(os.getcwd(), source)
+        self.set_source_breakpoints(
+            source,
+            [line_number(source, "// Breakpoint")],
+        )
+        self.continue_to_next_stop()
+
+        # Pointers contain memory references
+        ptr_value = self.get_local_as_int("rawptr")
+        self.assertIn(
+            "memoryReference",
+            self.dap_server.request_setVariable(1, "rawptr", ptr_value + 2)[
+                "body"
+            ].keys(),
+        )
+
+        # Non-pointer expressions don't include a memory reference
+        self.assertNotIn(
+            "memoryReference",
+            self.dap_server.request_setVariable(1, "not_a_ptr", 42)["body"].keys(),
+        )
diff --git a/lldb/test/API/tools/lldb-dap/memory/main.cpp b/lldb/test/API/tools/lldb-dap/memory/main.cpp
new file mode 100644
index 00000000000000..807b010d2434d6
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/memory/main.cpp
@@ -0,0 +1,10 @@
+#include <iostream>
+#include <memory>
+
+int main() {
+  int not_a_ptr = 666;
+  const char *rawptr = "dead";
+  std::unique_ptr<int> smartptr(new int(42));
+  // Breakpoint
+  return 0;
+}
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index a8b85f55939e17..b340c4e4a29f9c 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -1085,6 +1085,19 @@ std::string VariableDescription::GetResult(llvm::StringRef context) {
   return description.trim().str();
 }
 
+lldb::addr_t GetMemoryReference(lldb::SBValue v) {
+  if (!v.GetType().IsPointerType() && !v.GetType().IsArrayType()) {
+    return LLDB_INVALID_ADDRESS;
+  }
+
+  lldb::SBValue deref = v.Dereference();
+  if (!deref.IsValid()) {
+    return LLDB_INVALID_ADDRESS;
+  }
+
+  return deref.GetLoadAddress();
+}
+
 // "Variable": {
 //   "type": "object",
 //   "description": "A Variable is a name/value pair. Optionally a variable
@@ -1144,8 +1157,16 @@ std::string VariableDescription::GetResult(llvm::StringRef context) {
 //                       can use this optional information to present the
 //                       children in a paged UI and fetch them in chunks."
 //     }
-//
-//
+//     "memoryReference": {
+//        "type": "string",
+//        "description": "A memory reference associated with this variable.
+//                        For pointer type variables, this is generally a
+//                        reference to the memory address contained in the
+//                        pointer. For executable data, this reference may later
+//                        be used in a `disassemble` request. This attribute may
+//                        be returned by a debug adapter if corresponding
+//                        capability `supportsMemoryReferences` is true."
+//      },
 //     "$__lldb_extensions": {
 //       "description": "Unofficial extensions to the protocol",
 //       "properties": {
@@ -1253,6 +1274,10 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference,
   else
     object.try_emplace("variablesReference", (int64_t)0);
 
+  if (lldb::addr_t addr = GetMemoryReference(v); addr != LLDB_INVALID_ADDRESS) {
+    object.try_emplace("memoryReference", "0x" + llvm::utohexstr(addr));
+  }
+
   object.try_emplace("$__lldb_extensions", desc.GetVariableExtensionsJSON());
   return llvm::json::Value(std::move(object));
 }
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index 1515f5ba2e5f4d..c619712c2c682b 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -411,6 +411,12 @@ struct VariableDescription {
   std::string GetResult(llvm::StringRef context);
 };
 
+/// Get the corresponding `memoryReference` for a value.
+///
+/// According to the DAP documentation, the `memoryReference` should
+/// refer to the pointee, not to the address of the pointer itself.
+lldb::addr_t GetMemoryReference(lldb::SBValue v);
+
 /// Create a "Variable" object for a LLDB thread object.
 ///
 /// This function will fill in the following keys in the returned
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index ea84f31aec3a6c..d138a79d4488e5 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -9,6 +9,7 @@
 #include "DAP.h"
 #include "Watchpoint.h"
 #include "lldb/API/SBMemoryRegionInfo.h"
+#include "llvm/Support/Base64.h"
 
 #include <cassert>
 #include <climits>
@@ -1346,6 +1347,16 @@ void request_completions(const llvm::json::Object &request) {
 //                              present the variables in a paged UI and fetch
 //                              them in chunks."
 //            }
+//            "memoryReference": {
+//               "type": "string",
+//                "description": "A memory reference to a location appropriate
+//                                for this result. For pointer type eval
+//                                results, this is generally a reference to the
+//                                memory address contained in the pointer. This
+//                                attribute may be returned by a debug adapter
+//                                if corresponding capability
+//                                `supportsMemoryReferences` is true."
+//             },
 //          },
 //          "required": [ "result", "variablesReference" ]
 //        }
@@ -1411,6 +1422,10 @@ void request_evaluate(const llvm::json::Object &request) {
       } else {
         body.try_emplace("variablesReference", (int64_t)0);
       }
+      if (lldb::addr_t addr = GetMemoryReference(value);
+          addr != LLDB_INVALID_ADDRESS) {
+        body.try_emplace("memoryReference", "0x" + llvm::utohexstr(addr));
+      }
     }
   }
   response.try_emplace("body", std::move(body));
@@ -1718,6 +1733,8 @@ void request_initialize(const llvm::json::Object &request) {
   body.try_emplace("supportsLogPoints", true);
   // The debug adapter supports data watchpoints.
   body.try_emplace("supportsDataBreakpoints", true);
+  // The debug adapter supports the `readMemory` request.
+  body.try_emplace("supportsReadMemoryRequest", true);
 
   // Put in non-DAP specification lldb specific information.
   llvm::json::Object lldb_json;
@@ -3600,8 +3617,12 @@ void request_setVariable(const llvm::json::Object &request) {
       if (variable.MightHaveChildren())
         newVariablesReference = g_dap.variables.InsertExpandableVariable(
             variable, /*is_permanent=*/false);
-
       body.try_emplace("variablesReference", newVariablesReference);
+
+      if (lldb::addr_t addr = GetMemoryReference(variable);
+          addr != LLDB_INVALID_ADDRESS) {
+        body.try_emplace("memoryReference", "0x" + llvm::utohexstr(addr));
+      }
     } else {
       EmplaceSafeString(body, "message", std::string(error.GetCString()));
     }
@@ -4028,6 +4049,154 @@ void request_disassemble(const llvm::json::Object &request) {
   response.try_emplace("body", std::move(body));
   g_dap.SendJSON(llvm::json::Value(std::move(response)));
 }
+
+// "ReadMemoryRequest": {
+//   "allOf": [ { "$ref": "#/definitions/Request" }, {
+//     "type": "object",
+//     "description": "Reads bytes from memory at the provided location. Clients
+//                     should only call this request if the corresponding
+//                     capability `supportsReadMemoryRequest` is true.",
+//     "properties": {
+//       "command": {
+//         "type": "string",
+//         "enum": [ "readMemory" ]
+//       },
+//       "arguments": {
+//         "$ref": "#/definitions/ReadMemoryArguments"
+//       }
+//     },
+//     "required": [ "command", "arguments" ]
+//   }]
+// },
+// "ReadMemoryArguments": {
+//   "type": "object",
+//   "description": "Arguments for `readMemory` request.",
+//   "properties": {
+//     "memoryReference": {
+//       "type": "string",
+//       "description": "Memory reference to the base location from which data
+//                       should be read."
+//     },
+//     "offset": {
+//       "type": "integer",
+//       "description": "Offset (in bytes) to be applied to the reference
+//                       location before reading data. Can be negative."
+//     },
+//     "count": {
+//       "type": "integer",
+//       "description": "Number of bytes to read at the specified location and
+//                       offset."
+//     }
+//   },
+//   "required": [ "memoryReference", "count" ]
+// },
+// "ReadMemoryResponse": {
+//   "allOf": [ { "$ref": "#/definitions/Response" }, {
+//     "type": "object",
+//     "description": "Response to `readMemory` request.",
+//     "properties": {
+//       "body": {
+//         "type": "object",
+//         "properties": {
+//           "address": {
+//             "type": "string",
+//             "description": "The address of the first byte of data returned.
+//                             Treated as a hex value if prefixed with `0x`, or
+//                             as a decimal value otherwise."
+//           },
+//           "unreadableBytes": {
+//             "type": "integer",
+//             "description": "The number of unreadable bytes encountered after
+//                             the last successfully read byte.\nThis can be
+//                             used to determine the number of bytes that should
+//                             be skipped before a subsequent
+//             `readMemory` request succeeds."
+//           },
+//           "data": {
+//             "type": "string",
+//             "description": "The bytes read from memory, encoded using base64.
+//                             If the decoded length of `data` is less than the
+//                             requested `count` in the original `readMemory`
+//                             request, and `unreadableBytes` is zero or
+//                             omitted, then the client should assume it's
+//                             reached the end of readable memory."
+//           }
+//         },
+//         "required": [ "address" ]
+//       }
+//     }
+//   }]
+// },
+void request_readMemory(const llvm::json::Object &request) {
+  llvm::json::Object response;
+  FillResponse(request, response);
+  auto arguments = request.getObject("arguments");
+
+  lldb::SBProcess process = g_dap.target.GetProcess();
+  if (!process.IsValid()) {
+    response["success"] = false;
+    response["message"] = "No process running";
+    g_dap.SendJSON(llvm::json::Value(std::move(response)));
+    return;
+  }
+
+  auto memoryReference = GetString(arguments, "memoryReference");
+  lldb::addr_t addr;
+  if (memoryReference.consumeInteger(0, addr)) {
+    response["success"] = false;
+    response["message"] =
+        "Malformed memory reference: " + memoryReference.str();
+    g_dap.SendJSON(llvm::json::Value(std::move(response)));
+    return;
+  }
+
+  addr += GetSigned(arguments, "offset", 0);
+  const auto requested_count = GetUnsigned(arguments, "count", 0);
+  lldb::SBMemoryRegionInfo region_info;
+  lldb::SBError memRegError = process.GetMemoryRegionInfo(addr, region_info);
+  if (memRegError.Fail()) {
+    response["success"] = false;
+    EmplaceSafeString(response, "message",
+                      "Unable to find memory region: " +
+                          std::string(memRegError.GetCString()));
+    g_dap.SendJSON(llvm::json::Value(std::move(response)));
+    return;
+  }
+  const auto available_count =
+      std::min(requested_count, region_info.GetRegionEnd() - addr);
+  const auto unavailable_count = requested_count - available_count;
+
+  std::vector<uint8_t> buf;
+  buf.resize(available_count);
+  if (available_count > 0) {
+    lldb::SBError memReadError;
+    auto bytes_read =
+        process.ReadMemory(addr, buf.data(), available_count, memReadError);
+    if (memReadError.Fail()) {
+      response["success"] = false;
+      EmplaceSafeString(response, "message",
+                        "Unable to read memory: " +
+                            std::string(memReadError.GetCString()));
+      g_dap.SendJSON(llvm::json::Value(std::move(response)));
+      return;
+    }
+    if (bytes_read != available_count) {
+      response["success"] = false;
+      EmplaceSafeString(response, "message", "Unexpected, short read");
+      g_dap.SendJSON(llvm::json::Value(std::move(response)));
+      return;
+    }
+  }
+
+  llvm::json::Object body;
+  std::string formatted_addr = "0x" + llvm::utohexstr(addr);
+  body.try_emplace("address", formatted_addr);
+  body.try_emplace("data", llvm::encodeBase64(buf));
+  body.try_emplace("unreadableBytes", unavailable_count);
+  response.try_emplace("body", std::move(body));
+  g_dap.SendJSON(llvm::json::Value(std::move(response)));
+}
+
 // A request used in testing to get the details on all breakpoints that are
 // currently set in the target. This helps us to test "setBreakpoints" and
 // "setFunctionBreakpoints" requests to verify we have the correct set of
@@ -4078,6 +4247,7 @@ void RegisterRequestCallbacks() {
   g_dap.RegisterRequestCallback("threads", request_threads);
   g_dap.RegisterRequestCallback("variables", request_variables);
   g_dap.RegisterRequestCallback("disassemble", request_disassemble);
+  g_dap.RegisterRequestCallback("readMemory", request_readMemory);
   // Custom requests
   g_dap.RegisterRequestCallback("compileUnits", request_compileUnits);
   g_dap.RegisterRequestCallback("modules", request_modules);

>From 0dd51978ab8b1869a1e0a41792efeaf5195edd85 Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang <avogelsgesang at salesforce.com>
Date: Thu, 15 Aug 2024 10:19:39 +0000
Subject: [PATCH 2/5] Address code style comments

---
 lldb/tools/lldb-dap/JSONUtils.cpp | 24 ++++++++++--------------
 lldb/tools/lldb-dap/JSONUtils.h   |  2 +-
 lldb/tools/lldb-dap/lldb-dap.cpp  | 20 ++++++++------------
 3 files changed, 19 insertions(+), 27 deletions(-)

diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index b340c4e4a29f9c..12a851c28dcba1 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -690,8 +690,7 @@ std::optional<llvm::json::Value> CreateSource(lldb::SBFrame &frame) {
 //     "instructionPointerReference": {
 // 	     "type": "string",
 // 	     "description": "A memory reference for the current instruction
-// pointer
-//                       in this frame."
+//                         pointer in this frame."
 //     },
 //     "moduleId": {
 //       "type": ["integer", "string"],
@@ -1085,17 +1084,15 @@ std::string VariableDescription::GetResult(llvm::StringRef context) {
   return description.trim().str();
 }
 
-lldb::addr_t GetMemoryReference(lldb::SBValue v) {
-  if (!v.GetType().IsPointerType() && !v.GetType().IsArrayType()) {
-    return LLDB_INVALID_ADDRESS;
-  }
+std::optional<lldb::addr_t> GetMemoryReference(lldb::SBValue v) {
+  if (!v.GetType().IsPointerType() && !v.GetType().IsArrayType())
+    return std::nullopt;
 
   lldb::SBValue deref = v.Dereference();
-  if (!deref.IsValid()) {
-    return LLDB_INVALID_ADDRESS;
-  }
-
-  return deref.GetLoadAddress();
+  lldb::addr_t load_addr = deref.GetLoadAddress();
+  if (load_addr != LLDB_INVALID_ADDRESS)
+    return load_addr;
+  return std::nullopt;
 }
 
 // "Variable": {
@@ -1274,9 +1271,8 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference,
   else
     object.try_emplace("variablesReference", (int64_t)0);
 
-  if (lldb::addr_t addr = GetMemoryReference(v); addr != LLDB_INVALID_ADDRESS) {
-    object.try_emplace("memoryReference", "0x" + llvm::utohexstr(addr));
-  }
+  if (std::optional<lldb::addr_t> addr = GetMemoryReference(v))
+    object.try_emplace("memoryReference", "0x" + llvm::utohexstr(*addr));
 
   object.try_emplace("$__lldb_extensions", desc.GetVariableExtensionsJSON());
   return llvm::json::Value(std::move(object));
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index c619712c2c682b..9f213a053f84cd 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -415,7 +415,7 @@ struct VariableDescription {
 ///
 /// According to the DAP documentation, the `memoryReference` should
 /// refer to the pointee, not to the address of the pointer itself.
-lldb::addr_t GetMemoryReference(lldb::SBValue v);
+std::optional<lldb::addr_t> GetMemoryReference(lldb::SBValue v);
 
 /// Create a "Variable" object for a LLDB thread object.
 ///
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index d138a79d4488e5..c77733667eb6d2 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1422,10 +1422,8 @@ void request_evaluate(const llvm::json::Object &request) {
       } else {
         body.try_emplace("variablesReference", (int64_t)0);
       }
-      if (lldb::addr_t addr = GetMemoryReference(value);
-          addr != LLDB_INVALID_ADDRESS) {
-        body.try_emplace("memoryReference", "0x" + llvm::utohexstr(addr));
-      }
+      if (std::optional<lldb::addr_t> addr = GetMemoryReference(value))
+        body.try_emplace("memoryReference", "0x" + llvm::utohexstr(*addr));
     }
   }
   response.try_emplace("body", std::move(body));
@@ -3619,10 +3617,8 @@ void request_setVariable(const llvm::json::Object &request) {
             variable, /*is_permanent=*/false);
       body.try_emplace("variablesReference", newVariablesReference);
 
-      if (lldb::addr_t addr = GetMemoryReference(variable);
-          addr != LLDB_INVALID_ADDRESS) {
-        body.try_emplace("memoryReference", "0x" + llvm::utohexstr(addr));
-      }
+      if (std::optional<lldb::addr_t> addr = GetMemoryReference(variable))
+        body.try_emplace("memoryReference", "0x" + llvm::utohexstr(*addr));
     } else {
       EmplaceSafeString(body, "message", std::string(error.GetCString()));
     }
@@ -4151,7 +4147,7 @@ void request_readMemory(const llvm::json::Object &request) {
   }
 
   addr += GetSigned(arguments, "offset", 0);
-  const auto requested_count = GetUnsigned(arguments, "count", 0);
+  const uint64_t requested_count = GetUnsigned(arguments, "count", 0);
   lldb::SBMemoryRegionInfo region_info;
   lldb::SBError memRegError = process.GetMemoryRegionInfo(addr, region_info);
   if (memRegError.Fail()) {
@@ -4162,15 +4158,15 @@ void request_readMemory(const llvm::json::Object &request) {
     g_dap.SendJSON(llvm::json::Value(std::move(response)));
     return;
   }
-  const auto available_count =
+  const uint64_t available_count =
       std::min(requested_count, region_info.GetRegionEnd() - addr);
-  const auto unavailable_count = requested_count - available_count;
+  const uint64_t unavailable_count = requested_count - available_count;
 
   std::vector<uint8_t> buf;
   buf.resize(available_count);
   if (available_count > 0) {
     lldb::SBError memReadError;
-    auto bytes_read =
+    uint64_t bytes_read =
         process.ReadMemory(addr, buf.data(), available_count, memReadError);
     if (memReadError.Fail()) {
       response["success"] = false;

>From afe88fae3bd6da1e5708e75555c11d1d2e84c6f7 Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang <avogelsgesang at salesforce.com>
Date: Thu, 15 Aug 2024 10:39:38 +0000
Subject: [PATCH 3/5] Check memory region readability

---
 lldb/tools/lldb-dap/lldb-dap.cpp | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index c77733667eb6d2..d31911d26c1023 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -4158,6 +4158,12 @@ void request_readMemory(const llvm::json::Object &request) {
     g_dap.SendJSON(llvm::json::Value(std::move(response)));
     return;
   }
+  if (!region_info.IsReadable()) {
+    response["success"] = false;
+    response.try_emplace("message", "Memory region is not readable");
+    g_dap.SendJSON(llvm::json::Value(std::move(response)));
+    return;
+  }
   const uint64_t available_count =
       std::min(requested_count, region_info.GetRegionEnd() - addr);
   const uint64_t unavailable_count = requested_count - available_count;

>From 490b919e5ccb7eb8f7dcb1da40a2532b81b97650 Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang <avogelsgesang at salesforce.com>
Date: Thu, 15 Aug 2024 10:41:09 +0000
Subject: [PATCH 4/5] Fix variable naming

---
 lldb/tools/lldb-dap/lldb-dap.cpp | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index d31911d26c1023..f2f580bc48a665 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -4149,12 +4149,12 @@ void request_readMemory(const llvm::json::Object &request) {
   addr += GetSigned(arguments, "offset", 0);
   const uint64_t requested_count = GetUnsigned(arguments, "count", 0);
   lldb::SBMemoryRegionInfo region_info;
-  lldb::SBError memRegError = process.GetMemoryRegionInfo(addr, region_info);
-  if (memRegError.Fail()) {
+  lldb::SBError memreg_error = process.GetMemoryRegionInfo(addr, region_info);
+  if (memreg_error.Fail()) {
     response["success"] = false;
     EmplaceSafeString(response, "message",
                       "Unable to find memory region: " +
-                          std::string(memRegError.GetCString()));
+                          std::string(memreg_error.GetCString()));
     g_dap.SendJSON(llvm::json::Value(std::move(response)));
     return;
   }
@@ -4171,14 +4171,14 @@ void request_readMemory(const llvm::json::Object &request) {
   std::vector<uint8_t> buf;
   buf.resize(available_count);
   if (available_count > 0) {
-    lldb::SBError memReadError;
+    lldb::SBError memread_error;
     uint64_t bytes_read =
-        process.ReadMemory(addr, buf.data(), available_count, memReadError);
-    if (memReadError.Fail()) {
+        process.ReadMemory(addr, buf.data(), available_count, memread_error);
+    if (memread_error.Fail()) {
       response["success"] = false;
       EmplaceSafeString(response, "message",
                         "Unable to read memory: " +
-                            std::string(memReadError.GetCString()));
+                            std::string(memread_error.GetCString()));
       g_dap.SendJSON(llvm::json::Value(std::move(response)));
       return;
     }

>From 40f7b588c8cf9e2bfb86f1fd5cc900f978e072a8 Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang <avogelsgesang at salesforce.com>
Date: Thu, 15 Aug 2024 10:53:28 +0000
Subject: [PATCH 5/5] Adjust test case

---
 lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
index f950d5eecda671..536b4afb8e0719 100644
--- a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
+++ b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
@@ -51,7 +51,7 @@ def test_read_memory(self):
         # Reads at offset 0x0 fail
         mem = self.dap_server.request_read_memory("0x0", 0, 6)
         self.assertEqual(mem["success"], False)
-        self.assertTrue(mem["message"].startswith("Unable to read memory: "))
+        self.assertEqual(mem["message"], "Memory region is not readable")
 
     def test_memory_refs_variables(self):
         """



More information about the lldb-commits mailing list