Updating Variable::memoryReference to be an lldb::addr_t and creating a new helper for decoding memory references from JSON objects.

Author: ashgti

lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp

@@ -123,10 +123,11 @@ VariablesRequestHandler::Run(const VariablesArguments &arguments) const {
       if (!variable.IsValid())
         break;
 
-      int64_t var_ref =
+      int64_t frame_var_ref =
           dap.variables.InsertVariable(variable, /*is_permanent=*/false);
       variables.emplace_back(CreateVariable(
-          variable, var_ref, hex, dap.configuration.enableAutoVariableSummaries,
+          variable, frame_var_ref, hex,
+          dap.configuration.enableAutoVariableSummaries,
           dap.configuration.enableSyntheticChildDebugging,
           variable_name_counts[GetNonNullVariableName(variable)] > 1));
     }

lldb/tools/lldb-dap/JSONUtils.cpp

@@ -120,6 +120,42 @@ DecodeMemoryReference(llvm::StringRef memoryReference) {
   return addr;
 }
 
+bool DecodeMemoryReference(const llvm::json::Value &v, llvm::StringLiteral key,
+                           lldb::addr_t &out, llvm::json::Path path,
+                           bool required) {
+  const llvm::json::Object *v_obj = v.getAsObject();
+  if (!v_obj) {
+    path.report("expected object");
+    return false;
+  }
+
+  const llvm::json::Value *mem_ref_value = v_obj->get(key);
+  if (!mem_ref_value) {
+    if (!required)
+      return true;
+
+    path.field(key).report("missing value");
+    return false;
+  }
+
+  const std::optional<llvm::StringRef> mem_ref_str =
+      mem_ref_value->getAsString();
+  if (!mem_ref_str) {
+    path.field(key).report("expected string");
+    return false;
+  }
+
+  const std::optional<lldb::addr_t> addr_opt =
+      DecodeMemoryReference(*mem_ref_str);
+  if (!addr_opt) {
+    path.field(key).report("malformed memory reference");
+    return false;
+  }
+
+  out = *addr_opt;
+  return true;
+}
+
 std::vector<std::string> GetStrings(const llvm::json::Object *obj,
                                     llvm::StringRef key) {
   std::vector<std::string> strs;

lldb/tools/lldb-dap/JSONUtils.h

@@ -138,6 +138,30 @@ std::string EncodeMemoryReference(lldb::addr_t addr);
 std::optional<lldb::addr_t>
 DecodeMemoryReference(llvm::StringRef memoryReference);
 
+/// Decodes a memory reference from the given json value.
+///
+/// \param[in] v
+///    A JSON value that we expected to contain the memory reference.
+///
+/// \param[in] key
+///    The key of the memory reference.
+///
+/// \param[out] out
+///    The memory address, if successfully decoded.
+///
+/// \param[in] path
+///    The path for reporting errors.
+///
+/// \param[in] required
+///    Indicates if the key is required to be present, otherwise report an error
+///    if the key is missing.
+///
+/// \return
+///    Returns \b true if the address was decoded successfully.
+bool DecodeMemoryReference(const llvm::json::Value &v, llvm::StringLiteral key,
+                           lldb::addr_t &out, llvm::json::Path path,
+                           bool required);
+
 /// Extract an array of strings for the specified key from an object.
 ///
 /// String values in the array will be extracted without any quotes

lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp

@@ -9,6 +9,7 @@
 #include "Protocol/ProtocolTypes.h"
 #include "JSONUtils.h"
 #include "ProtocolUtils.h"
+#include "lldb/lldb-defines.h"
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
@@ -992,8 +993,9 @@ json::Value toJSON(const Variable &V) {
     result.insert({"namedVariables", V.namedVariables});
   if (V.indexedVariables)
     result.insert({"indexedVariables", V.indexedVariables});
-  if (!V.memoryReference.empty())
-    result.insert({"memoryReference", V.memoryReference});
+  if (V.memoryReference != LLDB_INVALID_ADDRESS)
+    result.insert(
+        {"memoryReference", EncodeMemoryReference(V.memoryReference)});
   if (V.declarationLocationReference)
     result.insert(
         {"declarationLocationReference", V.declarationLocationReference});
@@ -1012,10 +1014,11 @@ bool fromJSON(const json::Value &Param, Variable &V, json::Path Path) {
          O.mapOptional("evaluateName", V.evaluateName) &&
          O.mapOptional("namedVariables", V.namedVariables) &&
          O.mapOptional("indexedVariables", V.indexedVariables) &&
-         O.mapOptional("memoryReference", V.memoryReference) &&
          O.mapOptional("declarationLocationReference",
                        V.declarationLocationReference) &&
-         O.mapOptional("valueLocationReference", V.valueLocationReference);
+         O.mapOptional("valueLocationReference", V.valueLocationReference) &&
+         DecodeMemoryReference(Param, "memoryReference", V.memoryReference,
+                               Path, /*required=*/false);
 }
 
 } // namespace lldb_dap::protocol

lldb/tools/lldb-dap/Protocol/ProtocolTypes.h

@@ -897,7 +897,7 @@ struct Variable {
   ///
   /// This attribute may be returned by a debug adapter if corresponding
   /// capability `supportsMemoryReferences` is true.
-  std::string memoryReference;
+  lldb::addr_t memoryReference = LLDB_INVALID_ADDRESS;
 
   /// A reference that allows the client to request the location where the
   /// variable is declared. This should be present only if the adapter is likely

lldb/tools/lldb-dap/ProtocolUtils.cpp

@@ -299,7 +299,7 @@ Variable CreateVariable(lldb::SBValue v, int64_t var_ref, bool format_hex,
     var.valueLocationReference = PackLocation(var_ref, true);
 
   if (lldb::addr_t addr = v.GetLoadAddress(); addr != LLDB_INVALID_ADDRESS)
-    var.memoryReference = EncodeMemoryReference(addr);
+    var.memoryReference = addr;
 
   return var;
 }

lldb/unittests/DAP/JSONUtilsTest.cpp

@@ -7,9 +7,9 @@
 //===----------------------------------------------------------------------===//
 
 #include "JSONUtils.h"
-#include "lldb/API/SBModule.h"
-#include "lldb/API/SBTarget.h"
+#include "lldb/lldb-defines.h"
 #include "llvm/Support/JSON.h"
+#include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 #include <optional>
 
@@ -182,3 +182,57 @@ TEST(JSONUtilsTest, GetStrings_NestedArray) {
   ASSERT_EQ(result.size(), 1UL);
   EXPECT_EQ(result[0], "string");
 }
+
+TEST(JSONUtilsTest, DecodeMemoryReference) {
+  EXPECT_EQ(DecodeMemoryReference(""), std::nullopt);
+  EXPECT_EQ(DecodeMemoryReference("123"), std::nullopt);
+  EXPECT_EQ(DecodeMemoryReference("0o123"), std::nullopt);
+  EXPECT_EQ(DecodeMemoryReference("0b1010101"), std::nullopt);
+  EXPECT_EQ(DecodeMemoryReference("0x123"), 291u);
+
+  {
+    addr_t addr = LLDB_INVALID_ADDRESS;
+    json::Path::Root root;
+    EXPECT_TRUE(DecodeMemoryReference(json::Object{{"mem_ref", "0x123"}},
+                                      "mem_ref", addr, root,
+                                      /*required=*/true));
+    EXPECT_EQ(addr, 291u);
+  }
+
+  {
+    addr_t addr = LLDB_INVALID_ADDRESS;
+    json::Path::Root root;
+    EXPECT_TRUE(DecodeMemoryReference(json::Object{}, "mem_ref", addr, root,
+                                      /*required=*/false));
+  }
+
+  {
+    addr_t addr = LLDB_INVALID_ADDRESS;
+    json::Path::Root root;
+    EXPECT_FALSE(DecodeMemoryReference(json::Object{}, "mem_ref", addr, root,
+                                       /*required=*/true));
+    EXPECT_THAT_ERROR(root.getError(),
+                      FailedWithMessage("missing value at (root).mem_ref"));
+  }
+
+  {
+    addr_t addr = LLDB_INVALID_ADDRESS;
+    json::Path::Root root;
+    EXPECT_FALSE(DecodeMemoryReference(json::Object{{"mem_ref", 123}},
+                                       "mem_ref", addr, root,
+                                       /*required=*/true));
+    EXPECT_THAT_ERROR(root.getError(),
+                      FailedWithMessage("expected string at (root).mem_ref"));
+  }
+
+  {
+    addr_t addr = LLDB_INVALID_ADDRESS;
+    json::Path::Root root;
+    EXPECT_FALSE(DecodeMemoryReference(json::Object{{"mem_ref", "123"}},
+                                       "mem_ref", addr, root,
+                                       /*required=*/true));
+    EXPECT_THAT_ERROR(
+        root.getError(),
+        FailedWithMessage("malformed memory reference at (root).mem_ref"));
+  }
+}

lldb/unittests/DAP/ProtocolTypesTest.cpp

@@ -919,7 +919,7 @@ TEST(ProtocolTypesTest, Variable) {
   var.evaluateName = "my_name";
   var.namedVariables = 7;
   var.indexedVariables = 7;
-  var.memoryReference = "0x123";
+  var.memoryReference = 291u;
   var.declarationLocationReference = 24;
   var.valueLocationReference = 100;