[PATCH] gdb/DAP Few bug fixes & Evaluate Array Watch vars

[PATCH] gdb/DAP Few bug fixes & Evaluate Array Watch vars

[Simon Farre]

This patch fixes a few bugs.

First of all, name of VariableReferences must always be of string type.
This patch makes sure that this is the case by formatting the name. If
(when) the name is an integer, this will cause clients to fail or throw
errors.

Fixes a bug in NoOpArrayPrinter that calculated children to be N, but
only ever retrieves N-1 children, which makes Python at some time later
(during fetch_children -> fetch_one_child(N) ) raise an exception (out
of list index) which makes the entire request go bad.

The result[self.result_name] also f-strings the printer.to_string()
value, because this can potentially be a LazyString (which is a Python
object, not a string) and is not serializable by json.dumps.
---
 gdb/python/lib/gdb/dap/evaluate.py | 6 +++---
 gdb/python/lib/gdb/dap/varref.py   | 8 ++++++--
 gdb/python/lib/gdb/printing.py     | 2 +-
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/evaluate.py b/gdb/python/lib/gdb/dap/evaluate.py
index af7bf43afd0..382ba00877f 100644
--- a/gdb/python/lib/gdb/dap/evaluate.py
+++ b/gdb/python/lib/gdb/dap/evaluate.py
@@ -26,8 +26,8 @@ from .varref import find_variable, VariableReference
 
 
 class EvaluateResult(VariableReference):
-    def __init__(self, value):
-        super().__init__(None, value, "result")
+    def __init__(self, expr, value):
+        super().__init__(expr, value, "result")
 
 
 # Helper function to evaluate an expression in a certain frame.
@@ -39,7 +39,7 @@ def _evaluate(expr, frame_id):
         frame.select()
         global_context = False
     val = gdb.parse_and_eval(expr, global_context=global_context)
-    ref = EvaluateResult(val)
+    ref = EvaluateResult(expr, val)
     return ref.to_object()
 
 
diff --git a/gdb/python/lib/gdb/dap/varref.py b/gdb/python/lib/gdb/dap/varref.py
index 23f18d647c3..9f6068b017e 100644
--- a/gdb/python/lib/gdb/dap/varref.py
+++ b/gdb/python/lib/gdb/dap/varref.py
@@ -66,7 +66,7 @@ class BaseReference:
             "variablesReference": self.ref,
         }
         if self.name is not None:
-            result["name"] = self.name
+            result["name"] = f"{self.name}"
         return result
 
     def no_children(self):
@@ -152,7 +152,11 @@ class VariableReference(BaseReference):
 
     def to_object(self):
         result = super().to_object()
-        result[self.result_name] = self.printer.to_string()
+        # If we're a primitive, gdb.Value f-strings just fine
+        # We also must f-string printer.to_string() as it can be a LazyString
+        result[self.result_name] = (
+            f"{self.value}" if self.ref == 0 else f"{self.printer.to_string()}"
+        )
         num_children = self.child_count()
         if num_children is not None:
             if (
diff --git a/gdb/python/lib/gdb/printing.py b/gdb/python/lib/gdb/printing.py
index ab62ef3e45c..353427d000a 100644
--- a/gdb/python/lib/gdb/printing.py
+++ b/gdb/python/lib/gdb/printing.py
@@ -298,7 +298,7 @@ class NoOpArrayPrinter:
         return "array"
 
     def children(self):
-        for i in range(self.low, self.high):
+        for i in range(self.low, self.high + 1):
             yield (i, self.value[i])
 
 
-- 
2.40.1

Re: [PATCH] gdb/DAP Few bug fixes & Evaluate Array Watch vars

[Simon Farre]

[-- Attachment #1: Type: text/plain, Size: 347 bytes --]

What was left out of the commit message is that this patch of bug fixes 
enables the following to happen, in a DA-client, for a "watch" 
evaluateRequest and we want to watch

structlinetable
{
intnitems;
structlinetable_entryitem[1];
};


m_linetable.item[3]@3: ...
   1: ...
   2: ...
   3: ...

i.e. retrieve a range and display it's values.

Re: [PATCH] gdb/DAP Few bug fixes & Evaluate Array Watch vars

[Tom Tromey]

>>>>> "Simon" == Simon Farre via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> First of all, name of VariableReferences must always be of string type.

Do you mean if it is not None?

If so then the EvaluateResult change isn't needed.
But if None shouldn't be accepted, then BaseReference.to_object could be
changed.

Did this cause a problem somewhere?

Simon> The result[self.result_name] also f-strings the printer.to_string()
Simon> value, because this can potentially be a LazyString (which is a Python
Simon> object, not a string) and is not serializable by json.dumps.

We've been avoiding f-strings because some LTS distros are stuck on
versions of Python without them.  I think using str() is fine though.

Tom

Re: [PATCH] gdb/DAP Few bug fixes & Evaluate Array Watch vars

[Simon Farre]

No, not necessarily, if we for instance is retrieving children of an 
array, or the expression
el[3]@7; then each "name" of the children will be, 1,2,3 ..., i.e. 
numbers. This will make displaying any of them in VSCode not work.

On 6/20/23 16:49, Tom Tromey wrote:
>>>>>> "Simon" == Simon Farre via Gdb-patches <gdb-patches@sourceware.org> writes:
> Simon> First of all, name of VariableReferences must always be of string type.
>
> Do you mean if it is not None?
>
> If so then the EvaluateResult change isn't needed.
> But if None shouldn't be accepted, then BaseReference.to_object could be
> changed.
>
> Did this cause a problem somewhere?
>
> Simon> The result[self.result_name] also f-strings the printer.to_string()
> Simon> value, because this can potentially be a LazyString (which is a Python
> Simon> object, not a string) and is not serializable by json.dumps.
>
> We've been avoiding f-strings because some LTS distros are stuck on
> versions of Python without them.  I think using str() is fine though.
>
> Tom