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

Adrian Vogelsgesang via lldb-commits lldb-commits at lists.llvm.org
Mon Sep 16 05:44:55 PDT 2024


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

>From 88b48a5df0153a44276f14872c48e10639dcb673 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/9] [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              | 177 +++++++++++++++++-
 7 files changed, 367 insertions(+), 6 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 c6417760f17a2b..78c0f6233413b6 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 f175079c6f1fb5..5641cee43605d5 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -1180,6 +1180,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
@@ -1239,8 +1252,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": {
@@ -1348,6 +1369,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 f8fec22d7aa0ea..68c338423ad7e7 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -441,6 +441,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 c2ebc9a96a9a29..55d5cb7b73e906 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>
@@ -1541,6 +1542,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" ]
 //        }
@@ -1606,6 +1617,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));
@@ -1875,6 +1890,8 @@ void request_initialize(const llvm::json::Object &request) {
   // The debug adapter supports stepping granularities (argument `granularity`)
   // for the stepping requests.
   body.try_emplace("supportsSteppingGranularity", true);
+  // The debug adapter support for instruction breakpoint.
+  body.try_emplace("supportsInstructionBreakpoints", true);
 
   llvm::json::Array completion_characters;
   completion_characters.emplace_back(".");
@@ -1916,8 +1933,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 support for instruction breakpoint.
-  body.try_emplace("supportsInstructionBreakpoints", 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;
@@ -3775,8 +3792,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()));
     }
@@ -4201,6 +4222,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
@@ -4499,7 +4668,7 @@ void RegisterRequestCallbacks() {
   g_dap.RegisterRequestCallback("threads", request_threads);
   g_dap.RegisterRequestCallback("variables", request_variables);
   g_dap.RegisterRequestCallback("disassemble", request_disassemble);
-  // Instruction breakpoint request
+  g_dap.RegisterRequestCallback("readMemory", request_readMemory);
   g_dap.RegisterRequestCallback("setInstructionBreakpoints",
                                 request_setInstructionBreakpoints);
   // Custom requests

>From ea52649e84867b55eb04cca9566e631c16b81abd 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/9] 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 5641cee43605d5..4e6502ac33317f 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"],
@@ -1180,17 +1179,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": {
@@ -1369,9 +1366,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 68c338423ad7e7..43fb0c5d95300d 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -445,7 +445,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 55d5cb7b73e906..ffe03d54e8cde0 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1617,10 +1617,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));
@@ -3794,10 +3792,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()));
     }
@@ -4324,7 +4320,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()) {
@@ -4335,15 +4331,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 8822955cdb3315ab4a932322ea659cda12192c43 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/9] 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 ffe03d54e8cde0..ded0027520f98f 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -4331,6 +4331,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 201cacb1ec9b54bfa1b0a6b2c96514342c552b5c 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/9] 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 ded0027520f98f..40de2a1e4aebc2 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -4322,12 +4322,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;
   }
@@ -4344,14 +4344,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 05f4a3b0a3e3d5c8aacde3058ac1db71046f68c2 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/9] 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):
         """

>From cd708b96a5aa0f2b0948d6f9045a9d2dc24f7e6f Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang <avogelsgesang at salesforce.com>
Date: Fri, 16 Aug 2024 11:49:30 +0000
Subject: [PATCH 6/9] Introduce `EncodeMemoryReference` &
 `DecodeMemoryReference`

---
 lldb/tools/lldb-dap/JSONUtils.cpp | 18 +++++++++++++++++-
 lldb/tools/lldb-dap/JSONUtils.h   |  7 +++++++
 lldb/tools/lldb-dap/lldb-dap.cpp  | 22 ++++++++++++----------
 3 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 4e6502ac33317f..376fafdab7c1ac 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -112,6 +112,22 @@ bool ObjectContainsKey(const llvm::json::Object &obj, llvm::StringRef key) {
   return obj.find(key) != obj.end();
 }
 
+std::string EncodeMemoryReference(lldb::addr_t addr) {
+  return "0x" + llvm::utohexstr(addr);
+}
+
+std::optional<lldb::addr_t>
+DecodeMemoryReference(llvm::StringRef memoryReference) {
+  if (!memoryReference.starts_with("0x"))
+     return std::nullopt;
+
+  lldb::addr_t addr;
+  if (memoryReference.consumeInteger(0, addr))
+     return std::nullopt;
+
+  return addr;
+}
+
 std::vector<std::string> GetStrings(const llvm::json::Object *obj,
                                     llvm::StringRef key) {
   std::vector<std::string> strs;
@@ -1367,7 +1383,7 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference,
     object.try_emplace("variablesReference", (int64_t)0);
 
   if (std::optional<lldb::addr_t> addr = GetMemoryReference(v))
-    object.try_emplace("memoryReference", "0x" + llvm::utohexstr(*addr));
+    object.try_emplace("memoryReference", EncodeMemoryReference(*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 43fb0c5d95300d..6102ac53e6c989 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -131,6 +131,13 @@ int64_t GetSigned(const llvm::json::Object *obj, llvm::StringRef key,
 ///     \b True if the key exists in the \a obj, \b False otherwise.
 bool ObjectContainsKey(const llvm::json::Object &obj, llvm::StringRef key);
 
+/// Encodes a memory reference
+std::string EncodeMemoryReference(lldb::addr_t addr);
+
+/// Decodes a memory reference
+std::optional<lldb::addr_t>
+DecodeMemoryReference(llvm::StringRef memoryReference);
+
 /// Extract an array of strings for the specified key from an object.
 ///
 /// String values in the array will be extracted without any quotes
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 40de2a1e4aebc2..fd25f2a3a9f50d 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1618,7 +1618,7 @@ void request_evaluate(const llvm::json::Object &request) {
         body.try_emplace("variablesReference", (int64_t)0);
       }
       if (std::optional<lldb::addr_t> addr = GetMemoryReference(value))
-        body.try_emplace("memoryReference", "0x" + llvm::utohexstr(*addr));
+        body.try_emplace("memoryReference", EncodeMemoryReference(*addr));
     }
   }
   response.try_emplace("body", std::move(body));
@@ -3793,7 +3793,7 @@ void request_setVariable(const llvm::json::Object &request) {
       body.try_emplace("variablesReference", newVariablesReference);
 
       if (std::optional<lldb::addr_t> addr = GetMemoryReference(variable))
-        body.try_emplace("memoryReference", "0x" + llvm::utohexstr(*addr));
+        body.try_emplace("memoryReference", EncodeMemoryReference(*addr));
     } else {
       EmplaceSafeString(body, "message", std::string(error.GetCString()));
     }
@@ -4090,17 +4090,18 @@ void request_variables(const llvm::json::Object &request) {
 void request_disassemble(const llvm::json::Object &request) {
   llvm::json::Object response;
   FillResponse(request, response);
-  auto arguments = request.getObject("arguments");
+  auto *arguments = request.getObject("arguments");
 
-  auto memoryReference = GetString(arguments, "memoryReference");
-  lldb::addr_t addr_ptr;
-  if (memoryReference.consumeInteger(0, addr_ptr)) {
+  llvm::StringRef memoryReference = GetString(arguments, "memoryReference");
+  auto addr_opt = DecodeMemoryReference(memoryReference);
+  if (!addr_opt.has_value()) {
     response["success"] = false;
     response["message"] =
         "Malformed memory reference: " + memoryReference.str();
     g_dap.SendJSON(llvm::json::Value(std::move(response)));
     return;
   }
+  lldb::addr_t addr_ptr = *addr_opt;
 
   addr_ptr += GetSigned(arguments, "instructionOffset", 0);
   lldb::SBAddress addr(addr_ptr, g_dap.target);
@@ -4299,7 +4300,7 @@ void request_disassemble(const llvm::json::Object &request) {
 void request_readMemory(const llvm::json::Object &request) {
   llvm::json::Object response;
   FillResponse(request, response);
-  auto arguments = request.getObject("arguments");
+  auto* arguments = request.getObject("arguments");
 
   lldb::SBProcess process = g_dap.target.GetProcess();
   if (!process.IsValid()) {
@@ -4309,15 +4310,16 @@ void request_readMemory(const llvm::json::Object &request) {
     return;
   }
 
-  auto memoryReference = GetString(arguments, "memoryReference");
-  lldb::addr_t addr;
-  if (memoryReference.consumeInteger(0, addr)) {
+  llvm::StringRef memoryReference = GetString(arguments, "memoryReference");
+  auto addr_opt = DecodeMemoryReference(memoryReference);
+  if (!addr_opt.has_value()) {
     response["success"] = false;
     response["message"] =
         "Malformed memory reference: " + memoryReference.str();
     g_dap.SendJSON(llvm::json::Value(std::move(response)));
     return;
   }
+  lldb::addr_t addr = *addr_opt;
 
   addr += GetSigned(arguments, "offset", 0);
   const uint64_t requested_count = GetUnsigned(arguments, "count", 0);

>From 3b7e326d1fec1e8f3354c55e23c4e633a79833cb Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang <avogelsgesang at salesforce.com>
Date: Fri, 16 Aug 2024 12:01:53 +0000
Subject: [PATCH 7/9] Do not provide memory references for arrays.
 `Dereference` did not work anyway

---
 lldb/tools/lldb-dap/JSONUtils.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 376fafdab7c1ac..942bf228aec663 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -1196,7 +1196,7 @@ std::string VariableDescription::GetResult(llvm::StringRef context) {
 }
 
 std::optional<lldb::addr_t> GetMemoryReference(lldb::SBValue v) {
-  if (!v.GetType().IsPointerType() && !v.GetType().IsArrayType())
+  if (!v.GetType().IsPointerType())
     return std::nullopt;
 
   lldb::SBValue deref = v.Dereference();

>From 12f37b6a57c06b2bddb0f3397913d21f05552518 Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang <avogelsgesang at salesforce.com>
Date: Mon, 19 Aug 2024 22:32:31 +0000
Subject: [PATCH 8/9] request_read_memory -> request_readMemory

---
 .../lldbsuite/test/tools/lldb-dap/dap_server.py    |  2 +-
 .../API/tools/lldb-dap/memory/TestDAP_memory.py    | 14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)

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 78c0f6233413b6..d6a386975c8fb9 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,7 +691,7 @@ def request_disassemble(
         for inst in instructions:
             self.disassembled_instructions[inst["address"]] = inst
 
-    def request_read_memory(self, memoryReference, offset, count):
+    def request_readMemory(self, memoryReference, offset, count):
         args_dict = {
             "memoryReference": memoryReference,
             "offset": offset,
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 536b4afb8e0719..f865c423a3da31 100644
--- a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
+++ b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
@@ -12,9 +12,9 @@
 
 
 class TestDAP_memory(lldbdap_testcase.DAPTestCaseBase):
-    def test_read_memory(self):
+    def test_readMemory(self):
         """
-        Tests the 'read_memory' request
+        Tests the 'readMemory' request
         """
         program = self.getBuildArtifact("a.out")
         self.build_and_launch(program)
@@ -30,26 +30,26 @@ def test_read_memory(self):
         rawptr_ref = locals["rawptr"]["memoryReference"]
 
         # We can read the complete string
-        mem = self.dap_server.request_read_memory(rawptr_ref, 0, 5)["body"]
+        mem = self.dap_server.request_readMemory(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"]
+        mem = self.dap_server.request_readMemory(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"]
+        mem = self.dap_server.request_readMemory(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)
+        mem = self.dap_server.request_readMemory(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)
+        mem = self.dap_server.request_readMemory("0x0", 0, 6)
         self.assertEqual(mem["success"], False)
         self.assertEqual(mem["message"], "Memory region is not readable")
 

>From 6b23fbff90717e54563eaac0f18a6b28ae345cee Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang <avogelsgesang at salesforce.com>
Date: Mon, 16 Sep 2024 08:09:10 +0000
Subject: [PATCH 9/9] Provide non-dereferenced memory references

---
 .../tools/lldb-dap/memory/TestDAP_memory.py   | 101 +++++++-----------
 lldb/test/API/tools/lldb-dap/memory/main.cpp  |   1 -
 lldb/tools/lldb-dap/JSONUtils.cpp             |  15 +--
 lldb/tools/lldb-dap/JSONUtils.h               |   6 --
 lldb/tools/lldb-dap/lldb-dap.cpp              |   8 +-
 5 files changed, 47 insertions(+), 84 deletions(-)

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 f865c423a3da31..8bee70e50dcad9 100644
--- a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
+++ b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
@@ -12,9 +12,9 @@
 
 
 class TestDAP_memory(lldbdap_testcase.DAPTestCaseBase):
-    def test_readMemory(self):
+    def test_memory_refs_variables(self):
         """
-        Tests the 'readMemory' request
+        Tests memory references for evaluate
         """
         program = self.getBuildArtifact("a.out")
         self.build_and_launch(program)
@@ -27,33 +27,13 @@ def test_readMemory(self):
         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_readMemory(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_readMemory(rawptr_ref, 2, 3)["body"]
-        self.assertEqual(b64decode(mem["data"]), b"ad\0")
-
-        # Use a negative offset
-        mem = self.dap_server.request_readMemory(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_readMemory(rawptr_ref, 0, 0)
-        self.assertEqual(mem["success"], True)
-        self.assertEqual(mem["body"]["data"], "")
 
-        # Reads at offset 0x0 fail
-        mem = self.dap_server.request_readMemory("0x0", 0, 6)
-        self.assertEqual(mem["success"], False)
-        self.assertEqual(mem["message"], "Memory region is not readable")
+        # Pointers should have memory-references
+        self.assertIn("memoryReference", locals["rawptr"].keys())
+        # Non-pointers should also have memory-references
+        self.assertIn("memoryReference", locals["not_a_ptr"].keys())
 
-    def test_memory_refs_variables(self):
+    def test_memory_refs_evaluate(self):
         """
         Tests memory references for evaluate
         """
@@ -67,21 +47,14 @@ def test_memory_refs_variables(self):
         )
         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(),
+            self.dap_server.request_evaluate("rawptr")["body"].keys(),
         )
-        # Non-pointers should not have memory-references
-        self.assertNotIn("memoryReference", locals["not_a_ptr"].keys())
 
-    def test_memory_refs_evaluate(self):
+    def test_memory_refs_set_variable(self):
         """
-        Tests memory references for evaluate
+        Tests memory references for `setVariable`
         """
         program = self.getBuildArtifact("a.out")
         self.build_and_launch(program)
@@ -93,21 +66,17 @@ def test_memory_refs_evaluate(self):
         )
         self.continue_to_next_stop()
 
-        # Pointers contain memory references
+        ptr_value = self.get_local_as_int("rawptr")
         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(),
+            self.dap_server.request_setVariable(1, "rawptr", ptr_value + 2)[
+                "body"
+            ].keys(),
         )
 
-    def test_memory_refs_set_variable(self):
+    def test_readMemory(self):
         """
-        Tests memory references for `setVariable`
+        Tests the 'readMemory' request
         """
         program = self.getBuildArtifact("a.out")
         self.build_and_launch(program)
@@ -119,17 +88,29 @@ def test_memory_refs_set_variable(self):
         )
         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(),
-        )
+        ptr_deref = self.dap_server.request_evaluate("*rawptr")["body"]
+        memref = ptr_deref["memoryReference"]
 
-        # Non-pointer expressions don't include a memory reference
-        self.assertNotIn(
-            "memoryReference",
-            self.dap_server.request_setVariable(1, "not_a_ptr", 42)["body"].keys(),
-        )
+        # We can read the complete string
+        mem = self.dap_server.request_readMemory(memref, 0, 5)["body"]
+        self.assertEqual(mem["unreadableBytes"], 0)
+        self.assertEqual(b64decode(mem["data"]), b"dead\0")
+
+        # Use an offset
+        mem = self.dap_server.request_readMemory(memref, 2, 3)["body"]
+        self.assertEqual(b64decode(mem["data"]), b"ad\0")
+
+        # Use a negative offset
+        mem = self.dap_server.request_readMemory(memref, -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_readMemory(memref, 0, 0)
+        self.assertEqual(mem["success"], True)
+        self.assertEqual(mem["body"]["data"], "")
+
+        # Reads at offset 0x0 fail
+        mem = self.dap_server.request_readMemory("0x0", 0, 6)
+        self.assertEqual(mem["success"], False)
+        self.assertEqual(mem["message"], "Memory region is not readable")
diff --git a/lldb/test/API/tools/lldb-dap/memory/main.cpp b/lldb/test/API/tools/lldb-dap/memory/main.cpp
index 807b010d2434d6..14ac1ad95e330f 100644
--- a/lldb/test/API/tools/lldb-dap/memory/main.cpp
+++ b/lldb/test/API/tools/lldb-dap/memory/main.cpp
@@ -4,7 +4,6 @@
 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 942bf228aec663..4c07a8f4770179 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -1195,17 +1195,6 @@ std::string VariableDescription::GetResult(llvm::StringRef context) {
   return description.trim().str();
 }
 
-std::optional<lldb::addr_t> GetMemoryReference(lldb::SBValue v) {
-  if (!v.GetType().IsPointerType())
-    return std::nullopt;
-
-  lldb::SBValue deref = v.Dereference();
-  lldb::addr_t load_addr = deref.GetLoadAddress();
-  if (load_addr != LLDB_INVALID_ADDRESS)
-    return load_addr;
-  return std::nullopt;
-}
-
 // "Variable": {
 //   "type": "object",
 //   "description": "A Variable is a name/value pair. Optionally a variable
@@ -1382,8 +1371,8 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference,
   else
     object.try_emplace("variablesReference", (int64_t)0);
 
-  if (std::optional<lldb::addr_t> addr = GetMemoryReference(v))
-    object.try_emplace("memoryReference", EncodeMemoryReference(*addr));
+  if (lldb::addr_t addr = v.GetLoadAddress(); addr != LLDB_INVALID_ADDRESS)
+    object.try_emplace("memoryReference", EncodeMemoryReference(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 6102ac53e6c989..e44101f98103d6 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -448,12 +448,6 @@ 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.
-std::optional<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 fd25f2a3a9f50d..f4c77f2b0eabe0 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1617,8 +1617,8 @@ void request_evaluate(const llvm::json::Object &request) {
       } else {
         body.try_emplace("variablesReference", (int64_t)0);
       }
-      if (std::optional<lldb::addr_t> addr = GetMemoryReference(value))
-        body.try_emplace("memoryReference", EncodeMemoryReference(*addr));
+      if (lldb::addr_t addr = value.GetLoadAddress(); addr != LLDB_INVALID_ADDRESS)
+        body.try_emplace("memoryReference", EncodeMemoryReference(addr));
     }
   }
   response.try_emplace("body", std::move(body));
@@ -3792,8 +3792,8 @@ void request_setVariable(const llvm::json::Object &request) {
             variable, /*is_permanent=*/false);
       body.try_emplace("variablesReference", newVariablesReference);
 
-      if (std::optional<lldb::addr_t> addr = GetMemoryReference(variable))
-        body.try_emplace("memoryReference", EncodeMemoryReference(*addr));
+      if (lldb::addr_t addr = variable.GetLoadAddress(); addr != LLDB_INVALID_ADDRESS)
+        body.try_emplace("memoryReference", EncodeMemoryReference(addr));
     } else {
       EmplaceSafeString(body, "message", std::string(error.GetCString()));
     }



More information about the lldb-commits mailing list