[PATCH] Respect supportsMemoryReferences in DAP

[PATCH] Respect supportsMemoryReferences in DAP

[Tom Tromey]

I noticed that the support for memoryReference in the "variables"
output is gated on the client "supportsMemoryReferences" capability.

This patch implements this and makes some other changes to the DAP
memory reference code:

* A small refactoring to VariableReference to avoid "del".

* Don't use the address of a variable as its memoryReference -- only
  emit this for pointer types.  There's no spec support for the
  previous approach.

* Use strip_typedefs to handle typedefs of pointers.

Note that this code still ignores the constraint that a
memoryReference for a "variables" response that says that this should
only be used for pointer-to-function.  "evaluate" does not have this
constraint and so it seemed needless to me.  I filed a DAP bug report
about this:

    https://github.com/microsoft/debug-adapter-protocol/issues/414
---
 gdb/python/lib/gdb/dap/evaluate.py |  7 ++-----
 gdb/python/lib/gdb/dap/varref.py   | 23 +++++++++++++++++++----
 gdb/testsuite/gdb.dap/memory.c     |  2 ++
 gdb/testsuite/gdb.dap/memory.exp   |  2 ++
 gdb/testsuite/lib/dap-support.exp  |  3 ++-
 5 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/evaluate.py b/gdb/python/lib/gdb/dap/evaluate.py
index 63e80331b24..ae8cacc3e0f 100644
--- a/gdb/python/lib/gdb/dap/evaluate.py
+++ b/gdb/python/lib/gdb/dap/evaluate.py
@@ -55,12 +55,9 @@ class _SetResult(VariableReference):
     def __init__(self, value):
         super().__init__(None, value, "value")
 
-    def to_object(self):
-        result = super().to_object()
+    def add_memory_reference(self, result, addr):
         # This is not specified in the setExpression result.
-        if "memoryReference" in result:
-            del result["memoryReference"]
-        return result
+        pass
 
 
 # Helper function to perform an assignment.
diff --git a/gdb/python/lib/gdb/dap/varref.py b/gdb/python/lib/gdb/dap/varref.py
index 213151fd3d3..4537fbacf8b 100644
--- a/gdb/python/lib/gdb/dap/varref.py
+++ b/gdb/python/lib/gdb/dap/varref.py
@@ -150,6 +150,14 @@ class VariableReference(BaseReference):
             self.count = num_children
         return self.count
 
+    def add_memory_reference(self, result, addr):
+        """Add a memoryReference to the RESULT dictionary.
+
+        ADDR is the address of the memory.
+        The caller ensures that the client capability is set.
+        This may be overridden by subclasses."""
+        result["memoryReference"] = hex(int(addr))
+
     def to_object(self):
         result = super().to_object()
         result[self.result_name] = self.printer.to_string()
@@ -162,10 +170,17 @@ class VariableReference(BaseReference):
                 result["indexedVariables"] = num_children
             else:
                 result["namedVariables"] = num_children
-        if self.value.type.code == gdb.TYPE_CODE_PTR:
-            result["memoryReference"] = hex(int(self.value))
-        elif self.value.address is not None:
-            result["memoryReference"] = hex(int(self.value.address))
+        if client_bool_capability("supportsMemoryReferences"):
+            # "evaluate" allows a memory reference for any pointer
+            # type, while "variables" seems to only allow it for
+            # pointer-to-function or pointer-to-method.  However, the
+            # restriction seems strange.  This is filed as:
+            # https://github.com/microsoft/debug-adapter-protocol/issues/414
+            # Meanwhile, allow pointers.  The same issue brings up the
+            # idea of using the variable's address here, but for the
+            # time being we don't.
+            if self.value.type.strip_typedefs().code == gdb.TYPE_CODE_PTR:
+                self.add_memory_reference(result, self.value)
         if client_bool_capability("supportsVariableType"):
             result["type"] = str(self.value.type)
         return result
diff --git a/gdb/testsuite/gdb.dap/memory.c b/gdb/testsuite/gdb.dap/memory.c
index 3b9f6138abe..630e23dcf01 100644
--- a/gdb/testsuite/gdb.dap/memory.c
+++ b/gdb/testsuite/gdb.dap/memory.c
@@ -19,6 +19,8 @@
 
 uint32_t thirty_two = 7;
 
+uint32_t *thirty_two_p = &thirty_two;
+
 int main ()
 {
   return 0;			/* BREAK */
diff --git a/gdb/testsuite/gdb.dap/memory.exp b/gdb/testsuite/gdb.dap/memory.exp
index ab0516d6b3d..d702d5b5dee 100644
--- a/gdb/testsuite/gdb.dap/memory.exp
+++ b/gdb/testsuite/gdb.dap/memory.exp
@@ -47,6 +47,8 @@ set obj [dap_check_request_and_response "evaluate global" \
 	     evaluate {o expression [s thirty_two]}]
 dap_match_values "global value" [lindex $obj 0] "body result" 7
 
+set obj [dap_check_request_and_response "evaluate global pointer" \
+	     evaluate {o expression [s thirty_two_p]}]
 set addr [dict get [lindex $obj 0] body memoryReference]
 
 set obj [dap_check_request_and_response "read memory" \
diff --git a/gdb/testsuite/lib/dap-support.exp b/gdb/testsuite/lib/dap-support.exp
index e3750e1d016..4183526a320 100644
--- a/gdb/testsuite/lib/dap-support.exp
+++ b/gdb/testsuite/lib/dap-support.exp
@@ -233,7 +233,8 @@ proc _dap_initialize {name} {
     return [dap_check_request_and_response $name initialize \
 		{o clientID [s "gdb testsuite"] \
 		     supportsVariableType [l true] \
-		     supportsVariablePaging [l true]}]
+		     supportsVariablePaging [l true] \
+		     supportsMemoryReferences [l true]}]
 }
 
 # Start gdb, send a DAP initialize request, and then a launch request
-- 
2.40.1