public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Two more small DAP / Python fixes
@ 2023-10-05 13:34 Tom Tromey
  2023-10-05 13:34 ` [PATCH 1/2] Fix register-setting response from DAP Tom Tromey
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Tom Tromey @ 2023-10-05 13:34 UTC (permalink / raw)
  To: gdb-patches

This series fixes a couple of bugs that a co-worker noticed in the DAP
code.  The fixes themselves are partly in the Python code, and partly
in DAP.

---
Tom Tromey (2):
      Fix register-setting response from DAP
      Handle gdb.LazyString in DAP

 gdb/NEWS                              |  2 +
 gdb/python/lib/gdb/dap/varref.py      |  2 +-
 gdb/python/py-lazy-string.c           | 28 +++++++++++-
 gdb/python/py-value.c                 |  8 +++-
 gdb/testsuite/gdb.dap/lazy-string.c   | 23 ++++++++++
 gdb/testsuite/gdb.dap/lazy-string.exp | 80 +++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.dap/lazy-string.py  | 37 ++++++++++++++++
 gdb/testsuite/gdb.dap/scopes.exp      | 25 +++++++++--
 8 files changed, 199 insertions(+), 6 deletions(-)
---
base-commit: 99b06f8a80cf81f34c5f9b3f8c0e69246f792317
change-id: 20231005-two-dap-fixes-ab1f9fdcf9bc

Best regards,
-- 
Tom Tromey <tromey@adacore.com>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] Fix register-setting response from DAP
  2023-10-05 13:34 [PATCH 0/2] Two more small DAP / Python fixes Tom Tromey
@ 2023-10-05 13:34 ` Tom Tromey
  2023-10-05 13:34 ` [PATCH 2/2] Handle gdb.LazyString in DAP Tom Tromey
  2023-10-16 15:19 ` [PATCH 0/2] Two more small DAP / Python fixes Tom Tromey
  2 siblings, 0 replies; 5+ messages in thread
From: Tom Tromey @ 2023-10-05 13:34 UTC (permalink / raw)
  To: gdb-patches

Andry noticed that given a DAP setExpression request, where the
expression to set is a register, DAP will return the wrong value -- it
will return the old value, not the updated one.

This happens because gdb.Value.assign (which was recently added for
DAP) does not update the value.

In this patch, I chose to have the assign method update the Value
in-place.  It's also possible to have it return a new value, but this
didn't seem very useful to me.
---
 gdb/python/py-value.c            |  8 +++++++-
 gdb/testsuite/gdb.dap/scopes.exp | 25 ++++++++++++++++++++++---
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 245de68e0b9..0bf1d6e0dae 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -905,7 +905,13 @@ valpy_assign (PyObject *self_obj, PyObject *args)
   try
     {
       value_object *self = (value_object *) self_obj;
-      value_assign (self->value, val);
+      value *new_value = value_assign (self->value, val);
+      /* value_as_address returns a new value with the same location
+	 as the old one.  Ensure that this gdb.Value is updated to
+	 reflect the new value.  */
+      new_value->incref ();
+      self->value->decref ();
+      self->value = new_value;
     }
   catch (const gdb_exception &except)
     {
diff --git a/gdb/testsuite/gdb.dap/scopes.exp b/gdb/testsuite/gdb.dap/scopes.exp
index 6937badcca0..c3f62ddd0d6 100644
--- a/gdb/testsuite/gdb.dap/scopes.exp
+++ b/gdb/testsuite/gdb.dap/scopes.exp
@@ -118,8 +118,27 @@ gdb_assert {[llength $deivals] == 2} "dei has two members"
 
 set num [dict get $reg_scope variablesReference]
 # The request succeeding is sufficient.
-dap_check_request_and_response "fetch first register" \
-    "variables" \
-    [format {o variablesReference [i %d] count [i 1]} $num]
+set val [dap_check_request_and_response "fetch first register" \
+	     "variables" \
+	     [format {o variablesReference [i %d] count [i 1]} $num]]
+
+# Try setting the value to something else.
+set val [dict get [lindex $val 0] body variables]
+set name [dict get [lindex $val 0] name]
+set val [dict get [lindex $val 0] value]
+# Just make sure it is different from the original value.
+set val [expr {$val ^ 7}]
+
+# setVariable isn't implemented yet, so use the register name.  Note
+# that we sneak the "$" into the name, written in a slightly funny way
+# to work around apparent TON limitations.
+set response [dap_check_request_and_response "set first register" \
+		  setExpression \
+		  [format {o expression [s \$%s] value [s %d] frameId [i %d]} \
+		       $name $val $frame_id]]
+set response [lindex $response 0]
+
+gdb_assert {[dict get $response body value] == $val} \
+    "setting register yields updated value"
 
 dap_shutdown

-- 
2.40.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/2] Handle gdb.LazyString in DAP
  2023-10-05 13:34 [PATCH 0/2] Two more small DAP / Python fixes Tom Tromey
  2023-10-05 13:34 ` [PATCH 1/2] Fix register-setting response from DAP Tom Tromey
@ 2023-10-05 13:34 ` Tom Tromey
  2023-10-05 16:14   ` Eli Zaretskii
  2023-10-16 15:19 ` [PATCH 0/2] Two more small DAP / Python fixes Tom Tromey
  2 siblings, 1 reply; 5+ messages in thread
From: Tom Tromey @ 2023-10-05 13:34 UTC (permalink / raw)
  To: gdb-patches

Andry pointed out that the DAP code did not properly handle
gdb.LazyString results from a pretty-printer, yielding:

    TypeError: Object of type LazyString is not JSON serializable

This patch fixes the problem, partly with a small patch in varref.py,
but mainly by implementing tp_str for LazyString.
---
 gdb/NEWS                              |  2 +
 gdb/python/lib/gdb/dap/varref.py      |  2 +-
 gdb/python/py-lazy-string.c           | 28 +++++++++++-
 gdb/testsuite/gdb.dap/lazy-string.c   | 23 ++++++++++
 gdb/testsuite/gdb.dap/lazy-string.exp | 80 +++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.dap/lazy-string.py  | 37 ++++++++++++++++
 6 files changed, 170 insertions(+), 2 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index fc4c7c01661..3936a221a20 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -340,6 +340,8 @@ show tui mouse-events
      gdb.Progspace that is either being added to GDB, or removed from
      GDB.
 
+  ** gdb.LazyString now implements the __str__ method.
+
 *** Changes in GDB 13
 
 * MI version 1 is deprecated, and will be removed in GDB 14.
diff --git a/gdb/python/lib/gdb/dap/varref.py b/gdb/python/lib/gdb/dap/varref.py
index 764a8291027..95f00f5fe06 100644
--- a/gdb/python/lib/gdb/dap/varref.py
+++ b/gdb/python/lib/gdb/dap/varref.py
@@ -173,7 +173,7 @@ class VariableReference(BaseReference):
 
     def to_object(self):
         result = super().to_object()
-        result[self.result_name] = self.printer.to_string()
+        result[self.result_name] = str(self.printer.to_string())
         num_children = self.child_count()
         if num_children is not None:
             if (
diff --git a/gdb/python/py-lazy-string.c b/gdb/python/py-lazy-string.c
index 25a22b78af4..03393b0479e 100644
--- a/gdb/python/py-lazy-string.c
+++ b/gdb/python/py-lazy-string.c
@@ -296,6 +296,32 @@ gdbpy_extract_lazy_string (PyObject *string, CORE_ADDR *addr,
   encoding->reset (lazy->encoding ? xstrdup (lazy->encoding) : NULL);
 }
 
+/* __str__ for LazyString.  */
+
+static PyObject *
+stpy_str (PyObject *self)
+{
+  lazy_string_object *str = (lazy_string_object *) self;
+
+  struct value_print_options opts;
+  get_user_print_options (&opts);
+  opts.addressprint = false;
+
+  string_file stream;
+  try
+    {
+      struct type *type = stpy_lazy_string_elt_type (str);
+      val_print_string (type, str->encoding, str->address, str->length,
+			&stream, &opts);
+    }
+  catch (const gdb_exception &exc)
+    {
+      GDB_PY_HANDLE_EXCEPTION (exc);
+    }
+
+  return host_string_to_python_string (stream.c_str ()).release ();
+}
+
 GDBPY_INITIALIZE_FILE (gdbpy_initialize_lazy_string);
 
 \f
@@ -331,7 +357,7 @@ PyTypeObject lazy_string_object_type = {
   0,				  /*tp_as_mapping*/
   0,				  /*tp_hash */
   0,				  /*tp_call*/
-  0,				  /*tp_str*/
+  stpy_str,			  /*tp_str*/
   0,				  /*tp_getattro*/
   0,				  /*tp_setattro*/
   0,				  /*tp_as_buffer*/
diff --git a/gdb/testsuite/gdb.dap/lazy-string.c b/gdb/testsuite/gdb.dap/lazy-string.c
new file mode 100644
index 00000000000..9df48bf2957
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/lazy-string.c
@@ -0,0 +1,23 @@
+/* Copyright 2023 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+main ()
+{
+  const char *the_string = "DEI";
+  return 0;			/* STOP */
+}
diff --git a/gdb/testsuite/gdb.dap/lazy-string.exp b/gdb/testsuite/gdb.dap/lazy-string.exp
new file mode 100644
index 00000000000..10271fc216f
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/lazy-string.exp
@@ -0,0 +1,80 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test printing of gdb.LazyString objects.
+
+require allow_dap_tests
+
+load_lib dap-support.exp
+
+standard_testfile
+
+if {[build_executable ${testfile}.exp $testfile] == -1} {
+    return
+}
+
+set remote_python_file [gdb_remote_download host \
+			    ${srcdir}/${subdir}/${testfile}.py]
+
+save_vars GDBFLAGS {
+    append GDBFLAGS " -iex \"source $remote_python_file\""
+
+    if {[dap_launch $testfile] == ""} {
+	return
+    }
+}
+
+set line [gdb_get_line_number "STOP"]
+set obj [dap_check_request_and_response "set breakpoint by line number" \
+	     setBreakpoints \
+	     [format {o source [o path [%s]] breakpoints [a [o line [i %d]]]} \
+		  [list s $srcfile] $line]]
+set line_bpno [dap_get_breakpoint_number $obj]
+
+dap_check_request_and_response "start inferior" configurationDone
+
+dap_wait_for_event_and_check "stopped at line breakpoint" stopped \
+    "body reason" breakpoint \
+    "body hitBreakpointIds" $line_bpno
+
+set bt [lindex [dap_check_request_and_response "backtrace" stackTrace \
+		    {o threadId [i 1]}] \
+	    0]
+set frame_id [dict get [lindex [dict get $bt body stackFrames] 0] id]
+
+set scopes [dap_check_request_and_response "get scopes" scopes \
+		[format {o frameId [i %d]} $frame_id]]
+set scopes [dict get [lindex $scopes 0] body scopes]
+
+lassign $scopes scope reg_scope
+gdb_assert {[dict get $scope name] == "Locals"} "scope is locals"
+gdb_assert {[dict get $scope namedVariables] == 1} "one var in scope"
+
+set num [dict get $scope variablesReference]
+set refs [lindex [dap_check_request_and_response "fetch variable" \
+		      "variables" \
+		      [format {o variablesReference [i %d] count [i 1]} \
+			   $num]] \
+	      0]
+
+foreach var [dict get $refs body variables] {
+    gdb_assert {[dict get $var name] == "the_string"} "variable name"
+    # The result looks strange here, but only because TON does not
+    # handle the backslash-quote sequence properly when decoding the
+    # JSON.  The actual JSON is: "value": "\"DEI\"".
+    gdb_assert {[dict get $var value] == "\\\"DEI\\\""} "variable value"
+}
+
+dap_shutdown
diff --git a/gdb/testsuite/gdb.dap/lazy-string.py b/gdb/testsuite/gdb.dap/lazy-string.py
new file mode 100644
index 00000000000..a15539e6b5c
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/lazy-string.py
@@ -0,0 +1,37 @@
+# Copyright (C) 2022-2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+
+import gdb
+
+
+class Printer(gdb.ValuePrinter):
+    """Pretty print a string"""
+
+    def __init__(self, val):
+        self._val = val
+
+    def to_string(self):
+        return self._val.lazy_string()
+
+
+def lookup_function(val):
+    typ = val.type
+    if typ.code == gdb.TYPE_CODE_PTR:
+        return Printer(val)
+    return None
+
+
+gdb.pretty_printers.append(lookup_function)

-- 
2.40.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] Handle gdb.LazyString in DAP
  2023-10-05 13:34 ` [PATCH 2/2] Handle gdb.LazyString in DAP Tom Tromey
@ 2023-10-05 16:14   ` Eli Zaretskii
  0 siblings, 0 replies; 5+ messages in thread
From: Eli Zaretskii @ 2023-10-05 16:14 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

> Date: Thu, 05 Oct 2023 07:34:47 -0600
> From: Tom Tromey via Gdb-patches <gdb-patches@sourceware.org>
> 
> Andry pointed out that the DAP code did not properly handle
> gdb.LazyString results from a pretty-printer, yielding:
> 
>     TypeError: Object of type LazyString is not JSON serializable
> 
> This patch fixes the problem, partly with a small patch in varref.py,
> but mainly by implementing tp_str for LazyString.
> ---
>  gdb/NEWS                              |  2 +
>  gdb/python/lib/gdb/dap/varref.py      |  2 +-
>  gdb/python/py-lazy-string.c           | 28 +++++++++++-
>  gdb/testsuite/gdb.dap/lazy-string.c   | 23 ++++++++++
>  gdb/testsuite/gdb.dap/lazy-string.exp | 80 +++++++++++++++++++++++++++++++++++
>  gdb/testsuite/gdb.dap/lazy-string.py  | 37 ++++++++++++++++
>  6 files changed, 170 insertions(+), 2 deletions(-)

Thanks, the NEWS part is okay.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] Two more small DAP / Python fixes
  2023-10-05 13:34 [PATCH 0/2] Two more small DAP / Python fixes Tom Tromey
  2023-10-05 13:34 ` [PATCH 1/2] Fix register-setting response from DAP Tom Tromey
  2023-10-05 13:34 ` [PATCH 2/2] Handle gdb.LazyString in DAP Tom Tromey
@ 2023-10-16 15:19 ` Tom Tromey
  2 siblings, 0 replies; 5+ messages in thread
From: Tom Tromey @ 2023-10-16 15:19 UTC (permalink / raw)
  To: Tom Tromey via Gdb-patches; +Cc: Tom Tromey

>>>>> "Tom" == Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> This series fixes a couple of bugs that a co-worker noticed in the DAP
Tom> code.  The fixes themselves are partly in the Python code, and partly
Tom> in DAP.

I'm pushing this to git master & the gdb 14 branch now.

Tom

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-10-16 15:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-05 13:34 [PATCH 0/2] Two more small DAP / Python fixes Tom Tromey
2023-10-05 13:34 ` [PATCH 1/2] Fix register-setting response from DAP Tom Tromey
2023-10-05 13:34 ` [PATCH 2/2] Handle gdb.LazyString in DAP Tom Tromey
2023-10-05 16:14   ` Eli Zaretskii
2023-10-16 15:19 ` [PATCH 0/2] Two more small DAP / Python fixes Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).