[PATCH 0/6] Python __repr__() methods and new __dict__ attributes

[PATCH 0/6] Python __repr__() methods and new __dict__ attributes

[Andrew Burgess]

This series is a bit of a mixed bag of Python API related changes.
Patches #2 and #3 do depend on #1, but #4, #5, and #6 are really quite
separate.  However, they're all pretty small, and are all just
extending existing functionality to cover more cases, so I've thrown
them all in together for now.

---

Andrew Burgess (6):
  gdb/python: hoist common invalid object repr code into py-utils.c
  gdb/python: add gdb.InferiorThread.__repr__() method
  gdb/python: add gdb.Frame.__repr__() method
  gdb/python: remove users ability to create gdb.Progspace objects
  gdb/python: Add gdb.Inferior.__dict__ attribute
  gdb/python: Add gdb.InferiorThread.__dict__ attribute

 gdb/NEWS                                  | 13 +++++
 gdb/doc/python.texi                       | 63 +++++++++++++++++++++++
 gdb/python/py-arch.c                      |  2 +-
 gdb/python/py-block.c                     |  2 +-
 gdb/python/py-breakpoint.c                |  4 +-
 gdb/python/py-connection.c                |  2 +-
 gdb/python/py-frame.c                     | 19 ++++++-
 gdb/python/py-inferior.c                  | 15 +++++-
 gdb/python/py-infthread.c                 | 36 +++++++++++--
 gdb/python/py-objfile.c                   |  2 +-
 gdb/python/py-progspace.c                 | 16 +-----
 gdb/python/py-symbol.c                    |  2 +-
 gdb/python/py-type.c                      |  3 +-
 gdb/python/py-unwind.c                    |  2 +-
 gdb/python/py-utils.c                     |  8 +++
 gdb/python/python-internal.h              | 13 +++++
 gdb/testsuite/gdb.python/py-frame.exp     |  8 +++
 gdb/testsuite/gdb.python/py-inferior.exp  | 52 ++++++++++++++++++-
 gdb/testsuite/gdb.python/py-infthread.exp |  3 +-
 gdb/testsuite/gdb.python/py-progspace.exp |  6 +++
 20 files changed, 238 insertions(+), 33 deletions(-)


base-commit: b7a5722ebdd24a0d15d56e96d30a649ea1d7b0ee
-- 
2.25.4

[PATCH 1/6] gdb/python: hoist common invalid object repr code into py-utils.c

[Andrew Burgess]

Many object types now have a __repr__() function implementation.  A
common pattern is that, if an object is invalid, we print its
representation as: <TYPENAME (invalid)>.

I thought it might be a good idea to move the formatting of this
specific representation into a utility function, and then update all
of our existing code to call the new function.

The only place where I haven't made use of the new function is in
unwind_infopy_repr, where we currently return a different string.
This case is a little different as the UnwindInfo is invalid because
it references a frame, and it is the frame itself which is invalid.
That said, I think it would be fine to switch to using the standard
format; if the UnwindInfo references an invalid frame, then the
UnwindInfo is itself invalid.  But changing this would be an actual
change in behaviour, while all the other changes in this commit are
just refactoring.
---
 gdb/python/py-arch.c         | 2 +-
 gdb/python/py-block.c        | 2 +-
 gdb/python/py-breakpoint.c   | 4 ++--
 gdb/python/py-connection.c   | 2 +-
 gdb/python/py-inferior.c     | 2 +-
 gdb/python/py-objfile.c      | 2 +-
 gdb/python/py-symbol.c       | 2 +-
 gdb/python/py-type.c         | 3 +--
 gdb/python/py-unwind.c       | 2 +-
 gdb/python/py-utils.c        | 8 ++++++++
 gdb/python/python-internal.h | 9 +++++++++
 11 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index ac519331f18..b7d861d27ad 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -326,7 +326,7 @@ archpy_repr (PyObject *self)
 {
   const auto gdbarch = arch_object_to_gdbarch (self);
   if (gdbarch == nullptr)
-    return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (self)->tp_name);
+    return gdb_py_invalid_object_repr (self);
 
   auto arch_info = gdbarch_bfd_arch_info (gdbarch);
   return PyUnicode_FromFormat ("<%s arch_name=%s printable_name=%s>",
diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
index dd6d6d278a0..34be4664144 100644
--- a/gdb/python/py-block.c
+++ b/gdb/python/py-block.c
@@ -425,7 +425,7 @@ blpy_repr (PyObject *self)
 {
   const auto block = block_object_to_block (self);
   if (block == nullptr)
-    return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (self)->tp_name);
+    return gdb_py_invalid_object_repr (self);
 
   const auto name = block->function () ?
     block->function ()->print_name () : "<anonymous>";
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 5155d41e675..9b5e023cb09 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -1750,8 +1750,8 @@ bplocpy_repr (PyObject *py_self)
 {
   const auto self = (gdbpy_breakpoint_location_object *) py_self;
   if (self->owner == nullptr || self->owner->bp == nullptr
-    || self->owner->bp != self->bp_loc->owner)
-    return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (self)->tp_name);
+      || self->owner->bp != self->bp_loc->owner)
+    return gdb_py_invalid_object_repr (py_self);
 
   const auto enabled = self->bp_loc->enabled ? "enabled" : "disabled";
 
diff --git a/gdb/python/py-connection.c b/gdb/python/py-connection.c
index 3df12b435bb..d288a74cb2b 100644
--- a/gdb/python/py-connection.c
+++ b/gdb/python/py-connection.c
@@ -204,7 +204,7 @@ connpy_repr (PyObject *obj)
   process_stratum_target *target = self->target;
 
   if (target == nullptr)
-    return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (obj)->tp_name);
+    return gdb_py_invalid_object_repr (obj);
 
   return PyUnicode_FromFormat ("<%s num=%d, what=\"%s\">",
 			       Py_TYPE (obj)->tp_name,
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index ed153d668ac..929d8bd17b5 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -800,7 +800,7 @@ infpy_repr (PyObject *obj)
   inferior *inf = self->inferior;
 
   if (inf == nullptr)
-    return PyUnicode_FromString ("<gdb.Inferior (invalid)>");
+    return gdb_py_invalid_object_repr (obj);
 
   return PyUnicode_FromFormat ("<gdb.Inferior num=%d, pid=%d>",
 			       inf->num, inf->pid);
diff --git a/gdb/python/py-objfile.c b/gdb/python/py-objfile.c
index bb5d0d92aba..4f5e5cda5e6 100644
--- a/gdb/python/py-objfile.c
+++ b/gdb/python/py-objfile.c
@@ -537,7 +537,7 @@ objfpy_repr (PyObject *self_)
   objfile *obj = self->objfile;
 
   if (obj == nullptr)
-    return PyUnicode_FromString ("<gdb.Objfile (invalid)>");
+    return gdb_py_invalid_object_repr (self_);
 
   return PyUnicode_FromFormat ("<gdb.Objfile filename=%s>",
 			       objfile_name (obj));
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index 99724cfc95b..014442bf147 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -385,7 +385,7 @@ sympy_repr (PyObject *self)
 {
   const auto symbol = symbol_object_to_symbol (self);
   if (symbol == nullptr)
-    return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (self)->tp_name);
+    return gdb_py_invalid_object_repr (self);
 
   return PyUnicode_FromFormat ("<%s print_name=%s>", Py_TYPE (self)->tp_name,
 			       symbol->print_name ());
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index bfaa6d24d94..27c7b78096b 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -1083,8 +1083,7 @@ typy_repr (PyObject *self)
 {
   const auto type = type_object_to_type (self);
   if (type == nullptr)
-    return PyUnicode_FromFormat ("<%s (invalid)>",
-				 Py_TYPE (self)->tp_name);
+    return gdb_py_invalid_object_repr (self);
 
   const char *code = pyty_codes[type->code ()].name;
   string_file type_name;
diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
index f12485c22b7..70c33724cbc 100644
--- a/gdb/python/py-unwind.c
+++ b/gdb/python/py-unwind.c
@@ -425,7 +425,7 @@ pending_framepy_repr (PyObject *self)
   frame_info_ptr frame = pending_frame->frame_info;
 
   if (frame == nullptr)
-    return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (self)->tp_name);
+    return gdb_py_invalid_object_repr (self);
 
   const char *sp_str = nullptr;
   const char *pc_str = nullptr;
diff --git a/gdb/python/py-utils.c b/gdb/python/py-utils.c
index f1ca9ea0a5d..c29291004d2 100644
--- a/gdb/python/py-utils.c
+++ b/gdb/python/py-utils.c
@@ -597,3 +597,11 @@ gdbpy_fix_doc_string_indentation (gdb::unique_xmalloc_ptr<char> doc)
 
   return doc;
 }
+
+/* See python-internal.h.  */
+
+PyObject *
+gdb_py_invalid_object_repr (PyObject *self)
+{
+  return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (self)->tp_name);
+}
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 14e15574685..8ff9af650c2 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -897,6 +897,15 @@ int gdb_pymodule_addobject (PyObject *module, const char *name,
 			    PyObject *object)
   CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
 
+
+/* Return a Python string (str) object that represents SELF.  SELF can be
+   any object type, but should be in an "invalid" state.  What "invalid"
+   means is up to the caller.  The returned string will take the form
+   "<TYPENAME (invalid)>", without the quotes, and with TYPENAME replaced
+   with the type of SELF.  */
+
+PyObject *gdb_py_invalid_object_repr (PyObject *self);
+
 struct varobj_iter;
 struct varobj;
 std::unique_ptr<varobj_iter> py_varobj_get_iterator
-- 
2.25.4

[PATCH 2/6] gdb/python: add gdb.InferiorThread.__repr__() method

[Andrew Burgess]

Add a gdb.InferiorThread.__repr__() method.  Before this patch we
would see output like this:

  (gdb) pi
  >>> gdb.selected_thread()
  <gdb.InferiorThread object at 0x7f4dcc49b970>

After this patch, we now see:

  (gdb) pi
  >>> gdb.selected_thread()
  <gdb.InferiorThread id=1.2 target-id="Thread 0x7ffff7da1700 (LWP 458134)">

More verbose, but, I hope, more useful.

If the gdb.InferiorThread becomes invalid, then we will see:

  (gdb) pi
  >>> invalid_thread_variable
  <gdb.InferiorThread (invalid)>

Which is inline with how other invalid objects are displayed.
---
 gdb/python/py-infthread.c                 | 19 ++++++++++++++++++-
 gdb/testsuite/gdb.python/py-inferior.exp  | 16 +++++++++++++++-
 gdb/testsuite/gdb.python/py-infthread.exp |  3 ++-
 3 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index 00d7171de64..2d892b10b69 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -313,6 +313,23 @@ thpy_thread_handle (PyObject *self, PyObject *args)
   return object;
 }
 
+/* Implement repr() for gdb.InferiorThread.  */
+
+static PyObject *
+thpy_repr (PyObject *self)
+{
+  thread_object *thread_obj = (thread_object *) self;
+
+  if (thread_obj->thread == nullptr)
+    return gdb_py_invalid_object_repr (self);
+
+  thread_info *thr = thread_obj->thread;
+  return PyUnicode_FromFormat ("<%s id=%s target-id=\"%s\">",
+			       Py_TYPE (self)->tp_name,
+			       print_full_thread_id (thr),
+			       target_pid_to_str (thr->ptid).c_str ());
+}
+
 /* Return a reference to a new Python object representing a ptid_t.
    The object is a tuple containing (pid, lwp, tid). */
 PyObject *
@@ -429,7 +446,7 @@ PyTypeObject thread_object_type =
   0,				  /*tp_getattr*/
   0,				  /*tp_setattr*/
   0,				  /*tp_compare*/
-  0,				  /*tp_repr*/
+  thpy_repr,			  /*tp_repr*/
   0,				  /*tp_as_number*/
   0,				  /*tp_as_sequence*/
   0,				  /*tp_as_mapping*/
diff --git a/gdb/testsuite/gdb.python/py-inferior.exp b/gdb/testsuite/gdb.python/py-inferior.exp
index 6fbcdd6822f..5a221f800c3 100644
--- a/gdb/testsuite/gdb.python/py-inferior.exp
+++ b/gdb/testsuite/gdb.python/py-inferior.exp
@@ -78,7 +78,9 @@ gdb_test "python print ('result = %s' % (i0.connection_num == i0.connection.num)
     "Inferior.connection_num equals Inferior.connection.num"
 gdb_test "python print ('result = %s' % i0.pid)" " = \[0-9\]+" "test Inferior.pid"
 gdb_test "python print ('result = %s' % i0.was_attached)" " = False" "test Inferior.was_attached"
-gdb_test "python print (i0.threads ())" "\\(<gdb.InferiorThread object at 0x\[\[:xdigit:\]\]+>,\\)" "test Inferior.threads"
+gdb_test "python print (i0.threads ())" \
+    "\\(<gdb.InferiorThread id=${decimal}\\.${decimal} target-id=\"\[^\r\n\]*\">,\\)" \
+    "test Inferior.threads"
 
 gdb_test "python print (i0.progspace)" "<gdb.Progspace object at $hex>"
 gdb_test "python print (i0.progspace == gdb.progspaces()\[0\])" "True"
@@ -89,11 +91,23 @@ gdb_breakpoint check_threads
 gdb_continue_to_breakpoint "cont to check_threads" ".*pthread_barrier_wait.*"
 gdb_test "python print (len (i0.threads ()))" "\r\n9" "test Inferior.threads 2"
 
+# Grab the last thread from the list.  This thread object will become
+# invalid when the corresponding thread exits.
+gdb_test_no_output "python last_thread = i0.threads()\[-1\]"
+gdb_test "python print(last_thread)" \
+    "<gdb.InferiorThread id=${decimal}\\.${decimal} target-id=\"\[^\r\n\]*\">" \
+    "test repr of a valid thread"
+
 # Proceed to the next test.
 
 gdb_breakpoint [gdb_get_line_number "Break here."]
 gdb_continue_to_breakpoint "cont to Break here." ".*Break here\..*"
 
+# Check the repr() for an invalid gdb.InferiorThread object.
+gdb_test "python print(last_thread)" \
+    "<gdb.InferiorThread \\(invalid\\)>" \
+    "test repr of an invalid thread"
+
 # Test memory read and write operations.
 
 gdb_py_test_silent_cmd "python addr = gdb.selected_frame ().read_var ('str')" \
diff --git a/gdb/testsuite/gdb.python/py-infthread.exp b/gdb/testsuite/gdb.python/py-infthread.exp
index 0b10ce9ff77..49622c8ac28 100644
--- a/gdb/testsuite/gdb.python/py-infthread.exp
+++ b/gdb/testsuite/gdb.python/py-infthread.exp
@@ -57,7 +57,8 @@ gdb_py_test_silent_cmd "python gdb.selected_thread ().inferior" "test InferiorTh
 
 
 gdb_py_test_silent_cmd "python t0 = gdb.selected_thread ()" "test gdb.selected_thread" 1
-gdb_test "python print (t0)" "\\<gdb.InferiorThread object at 0x\[\[:xdigit:\]\]+>" "verify InferiorThread object"
+gdb_test "python print (t0)" \
+    "<gdb.InferiorThread id=${decimal}\\.${decimal} target-id=\"\[^\r\n\]*\">"
 gdb_test "python print ('result = %s' % t0.num)" " = 1" "test InferiorThread.num"
 gdb_test "python print ('result = %s' % t0.global_num)" " = 1" "test InferiorThread.global_num"
 gdb_test "python print ('result = %s' % str (t0.ptid))" " = \\(\[0-9\]+, \[0-9\]+, \[0-9\]+\\)" "test InferiorThread.ptid"
-- 
2.25.4

[PATCH 3/6] gdb/python: add gdb.Frame.__repr__() method

[Andrew Burgess]

Add a gdb.Frame.__repr__() method.  Before this patch we would see
output like this:

  (gdb) pi
  >>> gdb.selected_frame()
  <gdb.Frame object at 0x7fa8cc2df270>

After this patch, we now see:

  (gdb) pi
  >>> gdb.selected_frame()
  <gdb.Frame level=0 frame-id={stack=0x7ffff7da0ed0,code=0x000000000040115d,!special}>

More verbose, but, I hope, more useful.

If the gdb.Frame becomes invalid, then we will see:

  (gdb) pi
  >>> invalid_frame_variable
  <gdb.Frame (invalid)>

which is inline with how other invalid objects are displayed.
---
 gdb/python/py-frame.c                 | 19 ++++++++++++++++++-
 gdb/testsuite/gdb.python/py-frame.exp |  8 ++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index f38bf5d8237..ee465ed64a1 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -84,6 +84,23 @@ frapy_str (PyObject *self)
   return PyUnicode_FromString (fid.to_string ().c_str ());
 }
 
+/* Implement repr() for gdb.Frame.  */
+
+static PyObject *
+frapy_repr (PyObject *self)
+{
+  frame_object *frame_obj = (frame_object *) self;
+  frame_info_ptr f_info = frame_find_by_id (frame_obj->frame_id);
+  if (f_info == nullptr)
+    return gdb_py_invalid_object_repr (self);
+
+  const frame_id &fid = frame_obj->frame_id;
+  return PyUnicode_FromFormat ("<%s level=%d frame-id=%s>",
+			       Py_TYPE (self)->tp_name,
+			       frame_relative_level (f_info),
+			       fid.to_string ().c_str ());
+}
+
 /* Implementation of gdb.Frame.is_valid (self) -> Boolean.
    Returns True if the frame corresponding to the frame_id of this
    object still exists in the inferior.  */
@@ -840,7 +857,7 @@ PyTypeObject frame_object_type = {
   0,				  /* tp_getattr */
   0,				  /* tp_setattr */
   0,				  /* tp_compare */
-  0,				  /* tp_repr */
+  frapy_repr,			  /* tp_repr */
   0,				  /* tp_as_number */
   0,				  /* tp_as_sequence */
   0,				  /* tp_as_mapping */
diff --git a/gdb/testsuite/gdb.python/py-frame.exp b/gdb/testsuite/gdb.python/py-frame.exp
index 16177c8a5f8..6162696948f 100644
--- a/gdb/testsuite/gdb.python/py-frame.exp
+++ b/gdb/testsuite/gdb.python/py-frame.exp
@@ -36,6 +36,10 @@ gdb_breakpoint [gdb_get_line_number "Block break here."]
 gdb_continue_to_breakpoint "Block break here."
 gdb_py_test_silent_cmd "python bf1 = gdb.selected_frame ()" "get frame" 0
 
+# Test Frame.__repr__() method for a valid Frame object.
+gdb_test "python print (repr(bf1))" "<gdb\\.Frame level=0 frame-id=\\{\[^\r\n\]+\\}>" \
+    "test __repr__ for gdb.Frame on a valid frame"
+
 # Test Frame.architecture() method.
 gdb_py_test_silent_cmd "python show_arch_str = gdb.execute(\"show architecture\", to_string=True)" "show arch" 0
 gdb_test "python print (bf1.architecture().name() in show_arch_str)" "True" "test Frame.architecture()"
@@ -100,6 +104,10 @@ gdb_py_test_silent_cmd "python bframe = gdb.selected_frame()" \
     "get bottommost frame" 0
 gdb_test "up" ".*" ""
 
+# Test Frame.__repr__() method for an invalid Frame object.
+gdb_test "python print (repr(bf1))" "<gdb\\.Frame \\(invalid\\)>" \
+    "test __repr__ for gdb.Frame on an invalid frame"
+
 gdb_py_test_silent_cmd "python f1 = gdb.selected_frame ()" "get second frame" 0
 gdb_py_test_silent_cmd "python f0 = f1.newer ()" "get first frame" 0
 gdb_py_test_silent_cmd "python f2 = f1.older ()" "get last frame" 0
-- 
2.25.4

[PATCH 4/6] gdb/python: remove users ability to create gdb.Progspace objects

[Andrew Burgess]

I noticed that it is possible for the user to create a new
gdb.Progspace object, like this:

  (gdb) pi
  >>> p = gdb.Progspace()
  >>> p
  <gdb.Progspace object at 0x7ffad4219c10>
  >>> p.is_valid()
  False

As the new gdb.Progspace object is not associated with an actual C++
program_space object within GDB core, then the new gdb.Progspace is
created invalid, and there is no way in which the new object can ever
become valid.

Nor do I believe there's anywhere in the Python API where it makes
sense to consume an invalid gdb.Progspace created in this way, for
example, the gdb.Progspace could be passed as the locus to
register_type_printer, but all that would happen is that the
registered printer would never be used.

In this commit I propose to remove the ability to create new
gdb.Progspace objects.  Attempting to do so now gives an error, like
this:

  (gdb) pi
  >>> gdb.Progspace()
  Traceback (most recent call last):
    File "<stdin>", line 1, in <module>
  TypeError: cannot create 'gdb.Progspace' instances

Of course, there is a small risk here that some existing user code
might break ... but if that happens I don't believe the user code can
have been doing anything useful, so I see this as a small risk.
---
 gdb/NEWS                                  |  5 +++++
 gdb/python/py-progspace.c                 | 16 +---------------
 gdb/testsuite/gdb.python/py-progspace.exp |  6 ++++++
 3 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 11cd6c0663e..36443c38aca 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -87,6 +87,11 @@ show remote thread-options-packet
   ** New function gdb.interrupt(), that interrupts GDB as if the user
      typed control-c.
 
+  ** It is no longer possible to create new gdb.Progspace object using
+     'gdb.Progspace()', this will result in a TypeError.  Progspace
+     objects can still be obtained through calling other API
+     functions, for example 'gdb.current_progspace()'.
+
 * Debugger Adapter Protocol changes
 
   ** GDB now emits the "process" event.
diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
index 0797ef1fa6b..bfc6ff50f01 100644
--- a/gdb/python/py-progspace.c
+++ b/gdb/python/py-progspace.c
@@ -210,20 +210,6 @@ pspy_initialize (pspace_object *self)
   return 1;
 }
 
-static PyObject *
-pspy_new (PyTypeObject *type, PyObject *args, PyObject *keywords)
-{
-  gdbpy_ref<pspace_object> self ((pspace_object *) type->tp_alloc (type, 0));
-
-  if (self != NULL)
-    {
-      if (!pspy_initialize (self.get ()))
-	return NULL;
-    }
-
-  return (PyObject *) self.release ();
-}
-
 PyObject *
 pspy_get_printers (PyObject *o, void *ignore)
 {
@@ -859,5 +845,5 @@ PyTypeObject pspace_object_type =
   offsetof (pspace_object, dict), /* tp_dictoffset */
   0,				  /* tp_init */
   0,				  /* tp_alloc */
-  pspy_new,			  /* tp_new */
+  0,				  /* tp_new */
 };
diff --git a/gdb/testsuite/gdb.python/py-progspace.exp b/gdb/testsuite/gdb.python/py-progspace.exp
index befd6433e47..5f6a9577f82 100644
--- a/gdb/testsuite/gdb.python/py-progspace.exp
+++ b/gdb/testsuite/gdb.python/py-progspace.exp
@@ -53,6 +53,12 @@ gdb_py_test_silent_cmd "python progspace.random_attribute = 42" \
 gdb_test "python print (progspace.random_attribute)" "42" \
     "Verify set of random attribute in progspace"
 
+# Check that we can't create new (invalid) gdb.Progspace objects.
+gdb_test "python gdb.Progspace()" \
+    [multi_line "TypeError: cannot create 'gdb.Progspace' instances" \
+	 "Error while executing Python code\\."] \
+    "check for error when calling gdb.Progspace() directly"
+
 if {![runto_main]} {
     return
 }
-- 
2.25.4

[PATCH 5/6] gdb/python: Add gdb.Inferior.__dict__ attribute

[Andrew Burgess]

The gdb.Objfile, gdb.Progspace, and gdb.Type Python types already have
a __dict__ attribute, which allows users to create user defined
attributes within the objects.  This is useful if the user wants to
cache information within an object.

This commit adds the same functionality to the gdb.Inferior type.

After this commit there is a new gdb.Inferior.__dict__ attribute,
which is a dictionary.  A user can, for example, do this:

  (gdb) pi
  >>> i = gdb.selected_inferior()
  >>> i._user_attribute = 123
  >>> i._user_attribute
  123
  >>>

There's a new test included.
---
 gdb/NEWS                                 |  4 +++
 gdb/doc/python.texi                      | 32 ++++++++++++++++++++++++
 gdb/python/py-inferior.c                 | 13 +++++++++-
 gdb/testsuite/gdb.python/py-inferior.exp | 19 ++++++++++++++
 4 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 36443c38aca..500d5ab7160 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -92,6 +92,10 @@ show remote thread-options-packet
      objects can still be obtained through calling other API
      functions, for example 'gdb.current_progspace()'.
 
+  ** User defined attributes can be added to a gdb.Inferior object,
+     these will be stored in the object's new Inferior.__dict__
+     attribute.
+
 * Debugger Adapter Protocol changes
 
   ** GDB now emits the "process" event.
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index d74defeec0c..721f0100178 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -3667,6 +3667,38 @@
 string.
 @end defun
 
+One may add arbitrary attributes to @code{gdb.Inferior} objects in the
+usual Python way.  This is useful if, for example, one needs to do
+some extra record keeping associated with the inferior.
+
+In this contrived example we record the time when an inferior last
+stopped:
+
+@smallexample
+(gdb) python
+import datetime
+
+def thread_stopped(event):
+    if event.inferior_thread is not None:
+        thread = event.inferior_thread
+    else:
+        thread = gdb.selected_thread()
+    inferior = thread.inferior
+    inferior._last_stop_time = datetime.datetime.today()
+
+gdb.events.stop.connect(thread_stopped)
+(gdb) file /tmp/hello
+Reading symbols from /tmp/hello...
+(gdb) start
+Temporary breakpoint 1 at 0x401198: file /tmp/hello.c, line 18.
+Starting program: /tmp/hello
+
+Temporary breakpoint 1, main () at /tmp/hello.c:18
+18	  printf ("Hello World\n");
+(gdb) python print(gdb.selected_inferior()._last_stop_time)
+2024-01-04 14:48:41.347036
+@end smallexample
+
 @node Events In Python
 @subsubsection Events In Python
 @cindex inferior events in Python
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 929d8bd17b5..3f14bc31459 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -46,6 +46,10 @@ struct inferior_object
   /* thread_object instances under this inferior.  This owns a
      reference to each object it contains.  */
   thread_map_t *threads;
+
+  /* Dictionary holding user-added attributes.
+     This is the __dict__ attribute of the object.  */
+  PyObject *dict;
 };
 
 extern PyTypeObject inferior_object_type
@@ -241,6 +245,9 @@ inferior_to_inferior_object (struct inferior *inferior)
 
       inf_obj->inferior = inferior;
       inf_obj->threads = new thread_map_t ();
+      inf_obj->dict = PyDict_New ();
+      if (inf_obj->dict == nullptr)
+	return nullptr;
 
       /* PyObject_New initializes the new object with a refcount of 1.  This
 	 counts for the reference we are keeping in the inferior data.  */
@@ -981,6 +988,8 @@ infpy_dealloc (PyObject *obj)
      function is called.  */
   gdb_assert (inf_obj->inferior == nullptr);
 
+  Py_XDECREF (inf_obj->dict);
+
   Py_TYPE (obj)->tp_free (obj);
 }
 
@@ -1038,6 +1047,8 @@ GDBPY_INITIALIZE_FILE (gdbpy_initialize_inferior);
 
 static gdb_PyGetSetDef inferior_object_getset[] =
 {
+  { "__dict__", gdb_py_generic_dict, nullptr,
+    "The __dict__ for this inferior.", &inferior_object_type },
   { "arguments", infpy_get_args, infpy_set_args,
     "Arguments to this program.", nullptr },
   { "num", infpy_get_num, NULL, "ID of inferior, as assigned by GDB.", NULL },
@@ -1135,7 +1146,7 @@ PyTypeObject inferior_object_type =
   0,				  /* tp_dict */
   0,				  /* tp_descr_get */
   0,				  /* tp_descr_set */
-  0,				  /* tp_dictoffset */
+  offsetof (inferior_object, dict), /* tp_dictoffset */
   0,				  /* tp_init */
   0				  /* tp_alloc */
 };
diff --git a/gdb/testsuite/gdb.python/py-inferior.exp b/gdb/testsuite/gdb.python/py-inferior.exp
index 5a221f800c3..0e00636fa1c 100644
--- a/gdb/testsuite/gdb.python/py-inferior.exp
+++ b/gdb/testsuite/gdb.python/py-inferior.exp
@@ -85,6 +85,15 @@ gdb_test "python print (i0.threads ())" \
 gdb_test "python print (i0.progspace)" "<gdb.Progspace object at $hex>"
 gdb_test "python print (i0.progspace == gdb.progspaces()\[0\])" "True"
 
+# Add a user defined attribute to the inferior, and check the
+# attribute can be read back.
+gdb_test_no_output "python i0._user_attr = 123" \
+    "add a user defined attribute to the inferior object"
+gdb_test "python print(i0._user_attr)" \
+    "123" "read back user defined attribute from i0"
+gdb_test "python print(gdb.inferiors()\[0\]._user_attr)" \
+    "123" "read back user defined attribute from gdb.inferiors"
+
 # Test the number of inferior threads.
 
 gdb_breakpoint check_threads
@@ -127,6 +136,16 @@ gdb_test "print str" " = \"hallo, testsuite\"" \
 # correct inferior.
 set num [add_inferior]
 
+# Confirm the new inferior doesn't have the user defined attribute,
+# but that the first inferior does still have the attribute.
+gdb_test "python print(gdb.inferiors()\[1\]._user_attr)" \
+    [multi_line \
+	 "AttributeError: 'gdb\\.Inferior' object has no attribute '_user_attr'" \
+	 "Error while executing Python code\\."] \
+    "check new inferior doesn't have user defined attribute"
+gdb_test "python print(gdb.inferiors()\[0\]._user_attr)" \
+    "123" "read back user defined attribute again"
+
 # Test memory search.
 
 set hex_number {0x[0-9a-fA-F][0-9a-fA-F]*}
-- 
2.25.4

[PATCH 6/6] gdb/python: Add gdb.InferiorThread.__dict__ attribute

[Andrew Burgess]

The gdb.Objfile, gdb.Progspace, gdb.Type, and gdb.Inferior Python
types already have a __dict__ attribute, which allows users to create
user defined attributes within the objects.  This is useful if the
user wants to cache information within an object.

This commit adds the same functionality to the gdb.InferiorThread
type.

After this commit there is a new gdb.InferiorThread.__dict__
attribute, which is a dictionary.  A user can, for example, do this:

  (gdb) pi
  >>> t = gdb.selected_thread()
  >>> t._user_attribute = 123
  >>> t._user_attribute
  123
  >>>

There's a new test included.
---
 gdb/NEWS                                 |  4 +++
 gdb/doc/python.texi                      | 31 ++++++++++++++++++++++++
 gdb/python/py-infthread.c                | 17 +++++++++++--
 gdb/python/python-internal.h             |  4 +++
 gdb/testsuite/gdb.python/py-inferior.exp | 17 +++++++++++++
 5 files changed, 71 insertions(+), 2 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 500d5ab7160..c4862a8beb6 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -96,6 +96,10 @@ show remote thread-options-packet
      these will be stored in the object's new Inferior.__dict__
      attribute.
 
+  ** User defined attributes can be added to a gdb.InferiorThread
+     object, these will be stored in the object's new
+     InferiorThread.__dict__ attribute.
+
 * Debugger Adapter Protocol changes
 
   ** GDB now emits the "process" event.
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 721f0100178..ecba7cfa89c 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -4173,6 +4173,37 @@
 a @code{gdb.Type} for the handle type.
 @end defun
 
+One may add arbitrary attributes to @code{gdb.InferiorThread} objects
+in the usual Python way.  This is useful if, for example, one needs to
+do some extra record keeping associated with the thread.
+
+In this contrived example we record the time when a thread last
+stopped:
+
+@smallexample
+(gdb) python
+import datetime
+
+def thread_stopped(event):
+    if event.inferior_thread is not None:
+        thread = event.inferior_thread
+    else:
+        thread = gdb.selected_thread()
+    thread._last_stop_time = datetime.datetime.today()
+
+gdb.events.stop.connect(thread_stopped)
+(gdb) file /tmp/hello
+Reading symbols from /tmp/hello...
+(gdb) start
+Temporary breakpoint 1 at 0x401198: file /tmp/hello.c, line 18.
+Starting program: /tmp/hello
+
+Temporary breakpoint 1, main () at /tmp/hello.c:18
+18	  printf ("Hello World\n");
+(gdb) python print(gdb.selected_thread()._last_stop_time)
+2024-01-04 14:48:41.347036
+@end smallexample
+
 @node Recordings In Python
 @subsubsection Recordings In Python
 @cindex recordings in python
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index 2d892b10b69..fa24ef3b9ca 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -49,6 +49,10 @@ create_thread_object (struct thread_info *tp)
   if (thread_obj == NULL)
     return NULL;
 
+  thread_obj->dict = PyDict_New ();
+  if (thread_obj->dict == nullptr)
+    return nullptr;
+
   thread_obj->thread = tp;
   thread_obj->inf_obj = (PyObject *) inf_obj.release ();
 
@@ -58,7 +62,14 @@ create_thread_object (struct thread_info *tp)
 static void
 thpy_dealloc (PyObject *self)
 {
-  Py_DECREF (((thread_object *) self)->inf_obj);
+  thread_object *thr_obj = (thread_object *) self;
+
+  gdb_assert (thr_obj->inf_obj != nullptr);
+  gdb_assert (thr_obj->dict != nullptr);
+
+  Py_DECREF (thr_obj->inf_obj);
+  Py_DECREF (thr_obj->dict);
+
   Py_TYPE (self)->tp_free (self);
 }
 
@@ -394,6 +405,8 @@ GDBPY_INITIALIZE_FILE (gdbpy_initialize_thread);
 
 static gdb_PyGetSetDef thread_object_getset[] =
 {
+  { "__dict__", gdb_py_generic_dict, nullptr,
+    "The __dict__ for this thread.", &thread_object_type },
   { "name", thpy_get_name, thpy_set_name,
     "The name of the thread, as set by the user or the OS.", NULL },
   { "details", thpy_get_details, NULL,
@@ -471,7 +484,7 @@ PyTypeObject thread_object_type =
   0,				  /* tp_dict */
   0,				  /* tp_descr_get */
   0,				  /* tp_descr_set */
-  0,				  /* tp_dictoffset */
+  offsetof (thread_object, dict), /* tp_dictoffset */
   0,				  /* tp_init */
   0				  /* tp_alloc */
 };
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 8ff9af650c2..e01557edeb7 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -356,6 +356,10 @@ struct thread_object
 
   /* The Inferior object to which this thread belongs.  */
   PyObject *inf_obj;
+
+  /* Dictionary holding user-added attributes.  This is the __dict__
+     attribute of the object.  */
+  PyObject *dict;
 };
 
 struct inferior_object;
diff --git a/gdb/testsuite/gdb.python/py-inferior.exp b/gdb/testsuite/gdb.python/py-inferior.exp
index 0e00636fa1c..d1cd29f734b 100644
--- a/gdb/testsuite/gdb.python/py-inferior.exp
+++ b/gdb/testsuite/gdb.python/py-inferior.exp
@@ -107,6 +107,19 @@ gdb_test "python print(last_thread)" \
     "<gdb.InferiorThread id=${decimal}\\.${decimal} target-id=\"\[^\r\n\]*\">" \
     "test repr of a valid thread"
 
+# Add a user defined attribute to this thread, check the attribute can
+# be read back, and check the attribute is not present on other
+# threads.
+gdb_test_no_output "python last_thread._user_attribute = 123" \
+    "add user defined attribute to InferiorThread object"
+gdb_test "python print(last_thread._user_attribute)" "123" \
+    "read back user defined attribute"
+gdb_test "python print(i0.threads ()\[0\]._user_attribute)" \
+    [multi_line \
+	 "AttributeError: 'gdb\\.InferiorThread' object has no attribute '_user_attribute'" \
+	 "Error while executing Python code\\."] \
+    "attempt to read non-existent user defined attribute"
+
 # Proceed to the next test.
 
 gdb_breakpoint [gdb_get_line_number "Break here."]
@@ -117,6 +130,10 @@ gdb_test "python print(last_thread)" \
     "<gdb.InferiorThread \\(invalid\\)>" \
     "test repr of an invalid thread"
 
+# Check the user defined attribute is still present on the invalid thread object.
+gdb_test "python print(last_thread._user_attribute)" "123" \
+    "check user defined attribute on an invalid InferiorThread object"
+
 # Test memory read and write operations.
 
 gdb_py_test_silent_cmd "python addr = gdb.selected_frame ().read_var ('str')" \
-- 
2.25.4

Re: [PATCH 4/6] gdb/python: remove users ability to create gdb.Progspace objects

[Eli Zaretskii]

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: Andrew Burgess <aburgess@redhat.com>
> Date: Fri,  5 Jan 2024 11:48:33 +0000
> 
>  gdb/NEWS                                  |  5 +++++
>  gdb/python/py-progspace.c                 | 16 +---------------
>  gdb/testsuite/gdb.python/py-progspace.exp |  6 ++++++
>  3 files changed, 12 insertions(+), 15 deletions(-)

The NEWS part is OK, thanks.

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

Re: [PATCH 6/6] gdb/python: Add gdb.InferiorThread.__dict__ attribute

[Eli Zaretskii]

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: Andrew Burgess <aburgess@redhat.com>
> Date: Fri,  5 Jan 2024 11:48:35 +0000
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 500d5ab7160..c4862a8beb6 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -96,6 +96,10 @@ show remote thread-options-packet
>       these will be stored in the object's new Inferior.__dict__
>       attribute.
>  
> +  ** User defined attributes can be added to a gdb.InferiorThread
> +     object, these will be stored in the object's new
> +     InferiorThread.__dict__ attribute.

This part is OK.

> +@smallexample
> +(gdb) python
> +import datetime
> +
> +def thread_stopped(event):
> +    if event.inferior_thread is not None:
> +        thread = event.inferior_thread
> +    else:
> +        thread = gdb.selected_thread()
> +    thread._last_stop_time = datetime.datetime.today()
> +
> +gdb.events.stop.connect(thread_stopped)
> +(gdb) file /tmp/hello
> +Reading symbols from /tmp/hello...
> +(gdb) start
> +Temporary breakpoint 1 at 0x401198: file /tmp/hello.c, line 18.
> +Starting program: /tmp/hello
> +
> +Temporary breakpoint 1, main () at /tmp/hello.c:18
> +18	  printf ("Hello World\n");
> +(gdb) python print(gdb.selected_thread()._last_stop_time)
> +2024-01-04 14:48:41.347036
> +@end smallexample

When you have an @example that consists of more than just a couple of
lines, and has empty lines in it, it is a good idea to divide it into
separate parts using @group..@end group.  This will make sure that in
the printed manual such @group's will never be split across pages,
thus making it easier to read the example.

Thanks.

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

Re: [PATCH 5/6] gdb/python: Add gdb.Inferior.__dict__ attribute

[Eli Zaretskii]

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: Andrew Burgess <aburgess@redhat.com>
> Date: Fri,  5 Jan 2024 11:48:34 +0000
> 
> The gdb.Objfile, gdb.Progspace, and gdb.Type Python types already have
> a __dict__ attribute, which allows users to create user defined
> attributes within the objects.  This is useful if the user wants to
> cache information within an object.
> 
> This commit adds the same functionality to the gdb.Inferior type.
> 
> After this commit there is a new gdb.Inferior.__dict__ attribute,
> which is a dictionary.  A user can, for example, do this:
> 
>   (gdb) pi
>   >>> i = gdb.selected_inferior()
>   >>> i._user_attribute = 123
>   >>> i._user_attribute
>   123
>   >>>
> 
> There's a new test included.
> ---
>  gdb/NEWS                                 |  4 +++
>  gdb/doc/python.texi                      | 32 ++++++++++++++++++++++++
>  gdb/python/py-inferior.c                 | 13 +++++++++-
>  gdb/testsuite/gdb.python/py-inferior.exp | 19 ++++++++++++++
>  4 files changed, 67 insertions(+), 1 deletion(-)

Is this patch different from the 6/6 one? if so, how?

In any case, my comments are the same as in 6/6, thanks.

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

[PATCH] gdb/python: New InferiorThread.ptid_string attribute

[Andrew Burgess]

This commit adds a new InferiorThread.ptid_string attribute.  This
read-only attribute contains the string returned by target_pid_to_str,
which actually converts a ptid (not pid) to a string.

This is the string that appears (at least in part) in the output of
'info threads' in the 'Target Id' column, but also in the thread
exited message that GDB prints.

Having access to this string from Python is useful for allowing
extensions identify threads in a similar way to how GDB core would
identify the thread.
---
 gdb/NEWS                                  |  4 ++++
 gdb/doc/python.texi                       |  7 +++++++
 gdb/python/py-infthread.c                 | 20 ++++++++++++++++++++
 gdb/testsuite/gdb.python/py-infthread.exp |  8 ++++++++
 4 files changed, 39 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index 11cd6c0663e..60e58daa501 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -87,6 +87,10 @@ show remote thread-options-packet
   ** New function gdb.interrupt(), that interrupts GDB as if the user
      typed control-c.
 
+  ** New gdb.InferiorThread.ptid_string attribute.  This read-only
+     attribute contains the string that appears in the 'Target Id'
+     column of the 'info threads' command output.
+
 * Debugger Adapter Protocol changes
 
   ** GDB now emits the "process" event.
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index d74defeec0c..da37348d663 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -4084,6 +4084,13 @@
 does not  use that identifier.
 @end defvar
 
+@defvar InferiorThread.ptid_string
+This read-only attribute contains a string representing
+@code{InferiorThread.ptid}.  This is the string that @value{GDBN} uses
+in the @samp{Target Id} column in the @kbd{info threads} output
+(@pxref{info_threads,,@samp{info threads}}).
+@end defvar
+
 @defvar InferiorThread.inferior
 The inferior this thread belongs to.  This attribute is represented as
 a @code{gdb.Inferior} object.  This attribute is not writable.
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index 00d7171de64..7307b9e9da9 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -185,6 +185,23 @@ thpy_get_ptid (PyObject *self, void *closure)
   return gdbpy_create_ptid_object (thread_obj->thread->ptid);
 }
 
+/* Implement gdb.InferiorThread.ptid_string attribute.  */
+
+static PyObject *
+thpy_get_ptid_string (PyObject *self, void *closure)
+{
+  thread_object *thread_obj = (thread_object *) self;
+  THPY_REQUIRE_VALID (thread_obj);
+  ptid_t ptid = thread_obj->thread->ptid;
+
+  /* Select the correct inferior before calling a target_* function.  */
+  scoped_restore_current_thread restore_thread;
+  switch_to_inferior_no_thread (thread_obj->thread->inf);
+  std::string ptid_str = target_pid_to_str (ptid);
+
+  return PyUnicode_FromString (ptid_str.c_str ());
+}
+
 /* Getter for InferiorThread.inferior -> Inferior.  */
 
 static PyObject *
@@ -388,6 +405,9 @@ static gdb_PyGetSetDef thread_object_getset[] =
     "Global number of the thread, as assigned by GDB.", NULL },
   { "ptid", thpy_get_ptid, NULL, "ID of the thread, as assigned by the OS.",
     NULL },
+  { "ptid_string", thpy_get_ptid_string, nullptr,
+    "A string representing ptid, as used by, for example, 'info threads'.",
+    nullptr },
   { "inferior", thpy_get_inferior, NULL,
     "The Inferior object this thread belongs to.", NULL },
 
diff --git a/gdb/testsuite/gdb.python/py-infthread.exp b/gdb/testsuite/gdb.python/py-infthread.exp
index 0b10ce9ff77..3b07d84143f 100644
--- a/gdb/testsuite/gdb.python/py-infthread.exp
+++ b/gdb/testsuite/gdb.python/py-infthread.exp
@@ -62,6 +62,14 @@ gdb_test "python print ('result = %s' % t0.num)" " = 1" "test InferiorThread.num
 gdb_test "python print ('result = %s' % t0.global_num)" " = 1" "test InferiorThread.global_num"
 gdb_test "python print ('result = %s' % str (t0.ptid))" " = \\(\[0-9\]+, \[0-9\]+, \[0-9\]+\\)" "test InferiorThread.ptid"
 
+# Test the InferiorThread.ptid_string attribute.  We don't test the
+# actual string contents as they vary based on target, but we check
+# that we get back a non-empty string.
+gdb_test "python print(type(t0.ptid_string))" "<class 'str'>" \
+    "check that InferiorThread.ptid_string is a string"
+gdb_test "python print(t0.ptid_string)" ".+" \
+    "check that InferiorThread.ptid_string is non-empty"
+
 gdb_py_test_silent_cmd "python i0 = t0.inferior" "test InferiorThread.inferior" 1
 gdb_test "python print ('result = %s' % i0.num)" " = 1" "test Inferior.num"
 

base-commit: b7a5722ebdd24a0d15d56e96d30a649ea1d7b0ee
-- 
2.25.4

Re: [PATCH] gdb/python: New InferiorThread.ptid_string attribute

[Eli Zaretskii]

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: Andrew Burgess <aburgess@redhat.com>
> Date: Tue,  9 Jan 2024 17:32:44 +0000
> 
> This commit adds a new InferiorThread.ptid_string attribute.  This
> read-only attribute contains the string returned by target_pid_to_str,
> which actually converts a ptid (not pid) to a string.
> 
> This is the string that appears (at least in part) in the output of
> 'info threads' in the 'Target Id' column, but also in the thread
> exited message that GDB prints.
> 
> Having access to this string from Python is useful for allowing
> extensions identify threads in a similar way to how GDB core would
> identify the thread.
> ---
>  gdb/NEWS                                  |  4 ++++
>  gdb/doc/python.texi                       |  7 +++++++
>  gdb/python/py-infthread.c                 | 20 ++++++++++++++++++++
>  gdb/testsuite/gdb.python/py-infthread.exp |  8 ++++++++
>  4 files changed, 39 insertions(+)

The documentation parts are okay, thanks.

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

Re: [PATCH] gdb/python: New InferiorThread.ptid_string attribute

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> +  /* Select the correct inferior before calling a target_* function.  */
Andrew> +  scoped_restore_current_thread restore_thread;
Andrew> +  switch_to_inferior_no_thread (thread_obj->thread->inf);
Andrew> +  std::string ptid_str = target_pid_to_str (ptid);

Python-layer code that calls into gdb should normally be wrapped in a
try/catch, because any exception propagation will be catastrophic.

Tom

Re: [PATCH 1/6] gdb/python: hoist common invalid object repr code into py-utils.c

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> The only place where I haven't made use of the new function is in
Andrew> unwind_infopy_repr, where we currently return a different string.
Andrew> This case is a little different as the UnwindInfo is invalid because
Andrew> it references a frame, and it is the frame itself which is invalid.
Andrew> That said, I think it would be fine to switch to using the standard
Andrew> format; if the UnwindInfo references an invalid frame, then the
Andrew> UnwindInfo is itself invalid.  But changing this would be an actual
Andrew> change in behaviour, while all the other changes in this commit are
Andrew> just refactoring.

I think this patch is fine as-is, but FWIW I don't think we need to
worry about code that relies on the precise content of the repr of an
invalid object.  IOW, I think it's fine to change unwind_infopy_repr as
well.

Tom

Re: [PATCH 5/6] gdb/python: Add gdb.Inferior.__dict__ attribute

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> +  ** User defined attributes can be added to a gdb.Inferior object,
Andrew> +     these will be stored in the object's new Inferior.__dict__
Andrew> +     attribute.

Perhaps we should document that gdb may add new attributes from time to
time, but only ones starting with lower-case letters.

Tom

Re: [PATCH 6/6] gdb/python: Add gdb.InferiorThread.__dict__ attribute

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> +  thread_obj->dict = PyDict_New ();
Andrew> +  if (thread_obj->dict == nullptr)
Andrew> +    return nullptr;

If this fails...

Andrew>  static void
Andrew>  thpy_dealloc (PyObject *self)
Andrew>  {
Andrew> -  Py_DECREF (((thread_object *) self)->inf_obj);
Andrew> +  thread_object *thr_obj = (thread_object *) self;
Andrew> +
Andrew> +  gdb_assert (thr_obj->inf_obj != nullptr);
Andrew> +  gdb_assert (thr_obj->dict != nullptr);
Andrew> +
Andrew> +  Py_DECREF (thr_obj->inf_obj);
Andrew> +  Py_DECREF (thr_obj->dict);

... won't this be called and then crash because dict is NULL?

I am not really sure TBH but other code, like your change in
py-inferior.c, seems to think this.

I suspect Py_XDECREF should be used instead.

The current call to Py_DECREF isn't a bug because create_thread_object
avoids creating the thread object when 'inf_obj' would be null.  But,
the new patch doesn't do this.

Tom

Re: [PATCH 6/6] gdb/python: Add gdb.InferiorThread.__dict__ attribute

[Andrew Burgess]

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
> Andrew> +  thread_obj->dict = PyDict_New ();
> Andrew> +  if (thread_obj->dict == nullptr)
> Andrew> +    return nullptr;
>
> If this fails...
>
> Andrew>  static void
> Andrew>  thpy_dealloc (PyObject *self)
> Andrew>  {
> Andrew> -  Py_DECREF (((thread_object *) self)->inf_obj);
> Andrew> +  thread_object *thr_obj = (thread_object *) self;
> Andrew> +
> Andrew> +  gdb_assert (thr_obj->inf_obj != nullptr);
> Andrew> +  gdb_assert (thr_obj->dict != nullptr);
> Andrew> +
> Andrew> +  Py_DECREF (thr_obj->inf_obj);
> Andrew> +  Py_DECREF (thr_obj->dict);
>
> ... won't this be called and then crash because dict is NULL?
>
> I am not really sure TBH but other code, like your change in
> py-inferior.c, seems to think this.
>
> I suspect Py_XDECREF should be used instead.
>
> The current call to Py_DECREF isn't a bug because create_thread_object
> avoids creating the thread object when 'inf_obj' would be null.  But,
> the new patch doesn't do this.

Thanks for spotting this.  I'll get this fixed up.

Andrew

[PATCHv2 0/8] Python __repr__() methods and new __dict__ attributes

[Andrew Burgess]

In V2:

  - Fixed problems in thpy_dealloc that Tom pointed out,

  - Used @group...@end group in the docs as suggested by Eli,

  - Added patches #7 and #8, which are both docs only patches.  The
    first adds advice for naming attributes as Tom suggested, the
    second is just some doc updates that I noticed in passing.

--

This series is a bit of a mixed bag of Python API related changes.
Patches #2 and #3 do depend on #1, but #4, #5, and #6 are really quite
separate.  However, they're all pretty small, and are all just
extending existing functionality to cover more cases, so I've thrown
them all in together for now.

--

Andrew Burgess (8):
  gdb/python: hoist common invalid object repr code into py-utils.c
  gdb/python: add gdb.InferiorThread.__repr__() method
  gdb/python: add gdb.Frame.__repr__() method
  gdb/python: remove users ability to create gdb.Progspace objects
  gdb/python: Add gdb.Inferior.__dict__ attribute
  gdb/python: Add gdb.InferiorThread.__dict__ attribute
  gdb/doc: add some notes on selecting suitable attribute names
  gdb/doc: update examples in gdb.Progspace and gdb.Objfile docs

 gdb/NEWS                                  |  13 +++
 gdb/doc/python.texi                       | 111 ++++++++++++++++++++--
 gdb/python/py-arch.c                      |   2 +-
 gdb/python/py-block.c                     |   2 +-
 gdb/python/py-breakpoint.c                |   4 +-
 gdb/python/py-connection.c                |   2 +-
 gdb/python/py-frame.c                     |  19 +++-
 gdb/python/py-inferior.c                  |  15 ++-
 gdb/python/py-infthread.c                 |  34 ++++++-
 gdb/python/py-objfile.c                   |   2 +-
 gdb/python/py-progspace.c                 |  16 +---
 gdb/python/py-symbol.c                    |   2 +-
 gdb/python/py-type.c                      |   3 +-
 gdb/python/py-unwind.c                    |   2 +-
 gdb/python/py-utils.c                     |   8 ++
 gdb/python/python-internal.h              |  13 +++
 gdb/testsuite/gdb.python/py-frame.exp     |   8 ++
 gdb/testsuite/gdb.python/py-inferior.exp  |  52 +++++++++-
 gdb/testsuite/gdb.python/py-infthread.exp |   3 +-
 gdb/testsuite/gdb.python/py-progspace.exp |   6 ++
 20 files changed, 277 insertions(+), 40 deletions(-)


base-commit: b7a5722ebdd24a0d15d56e96d30a649ea1d7b0ee
-- 
2.25.4

[PATCHv2 1/8] gdb/python: hoist common invalid object repr code into py-utils.c

[Andrew Burgess]

Many object types now have a __repr__() function implementation.  A
common pattern is that, if an object is invalid, we print its
representation as: <TYPENAME (invalid)>.

I thought it might be a good idea to move the formatting of this
specific representation into a utility function, and then update all
of our existing code to call the new function.

The only place where I haven't made use of the new function is in
unwind_infopy_repr, where we currently return a different string.
This case is a little different as the UnwindInfo is invalid because
it references a frame, and it is the frame itself which is invalid.
That said, I think it would be fine to switch to using the standard
format; if the UnwindInfo references an invalid frame, then the
UnwindInfo is itself invalid.  But changing this would be an actual
change in behaviour, while all the other changes in this commit are
just refactoring.
---
 gdb/python/py-arch.c         | 2 +-
 gdb/python/py-block.c        | 2 +-
 gdb/python/py-breakpoint.c   | 4 ++--
 gdb/python/py-connection.c   | 2 +-
 gdb/python/py-inferior.c     | 2 +-
 gdb/python/py-objfile.c      | 2 +-
 gdb/python/py-symbol.c       | 2 +-
 gdb/python/py-type.c         | 3 +--
 gdb/python/py-unwind.c       | 2 +-
 gdb/python/py-utils.c        | 8 ++++++++
 gdb/python/python-internal.h | 9 +++++++++
 11 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index ac519331f18..b7d861d27ad 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -326,7 +326,7 @@ archpy_repr (PyObject *self)
 {
   const auto gdbarch = arch_object_to_gdbarch (self);
   if (gdbarch == nullptr)
-    return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (self)->tp_name);
+    return gdb_py_invalid_object_repr (self);
 
   auto arch_info = gdbarch_bfd_arch_info (gdbarch);
   return PyUnicode_FromFormat ("<%s arch_name=%s printable_name=%s>",
diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
index dd6d6d278a0..34be4664144 100644
--- a/gdb/python/py-block.c
+++ b/gdb/python/py-block.c
@@ -425,7 +425,7 @@ blpy_repr (PyObject *self)
 {
   const auto block = block_object_to_block (self);
   if (block == nullptr)
-    return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (self)->tp_name);
+    return gdb_py_invalid_object_repr (self);
 
   const auto name = block->function () ?
     block->function ()->print_name () : "<anonymous>";
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 5155d41e675..9b5e023cb09 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -1750,8 +1750,8 @@ bplocpy_repr (PyObject *py_self)
 {
   const auto self = (gdbpy_breakpoint_location_object *) py_self;
   if (self->owner == nullptr || self->owner->bp == nullptr
-    || self->owner->bp != self->bp_loc->owner)
-    return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (self)->tp_name);
+      || self->owner->bp != self->bp_loc->owner)
+    return gdb_py_invalid_object_repr (py_self);
 
   const auto enabled = self->bp_loc->enabled ? "enabled" : "disabled";
 
diff --git a/gdb/python/py-connection.c b/gdb/python/py-connection.c
index 3df12b435bb..d288a74cb2b 100644
--- a/gdb/python/py-connection.c
+++ b/gdb/python/py-connection.c
@@ -204,7 +204,7 @@ connpy_repr (PyObject *obj)
   process_stratum_target *target = self->target;
 
   if (target == nullptr)
-    return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (obj)->tp_name);
+    return gdb_py_invalid_object_repr (obj);
 
   return PyUnicode_FromFormat ("<%s num=%d, what=\"%s\">",
 			       Py_TYPE (obj)->tp_name,
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index ed153d668ac..929d8bd17b5 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -800,7 +800,7 @@ infpy_repr (PyObject *obj)
   inferior *inf = self->inferior;
 
   if (inf == nullptr)
-    return PyUnicode_FromString ("<gdb.Inferior (invalid)>");
+    return gdb_py_invalid_object_repr (obj);
 
   return PyUnicode_FromFormat ("<gdb.Inferior num=%d, pid=%d>",
 			       inf->num, inf->pid);
diff --git a/gdb/python/py-objfile.c b/gdb/python/py-objfile.c
index bb5d0d92aba..4f5e5cda5e6 100644
--- a/gdb/python/py-objfile.c
+++ b/gdb/python/py-objfile.c
@@ -537,7 +537,7 @@ objfpy_repr (PyObject *self_)
   objfile *obj = self->objfile;
 
   if (obj == nullptr)
-    return PyUnicode_FromString ("<gdb.Objfile (invalid)>");
+    return gdb_py_invalid_object_repr (self_);
 
   return PyUnicode_FromFormat ("<gdb.Objfile filename=%s>",
 			       objfile_name (obj));
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index 99724cfc95b..014442bf147 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -385,7 +385,7 @@ sympy_repr (PyObject *self)
 {
   const auto symbol = symbol_object_to_symbol (self);
   if (symbol == nullptr)
-    return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (self)->tp_name);
+    return gdb_py_invalid_object_repr (self);
 
   return PyUnicode_FromFormat ("<%s print_name=%s>", Py_TYPE (self)->tp_name,
 			       symbol->print_name ());
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index bfaa6d24d94..27c7b78096b 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -1083,8 +1083,7 @@ typy_repr (PyObject *self)
 {
   const auto type = type_object_to_type (self);
   if (type == nullptr)
-    return PyUnicode_FromFormat ("<%s (invalid)>",
-				 Py_TYPE (self)->tp_name);
+    return gdb_py_invalid_object_repr (self);
 
   const char *code = pyty_codes[type->code ()].name;
   string_file type_name;
diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
index f12485c22b7..70c33724cbc 100644
--- a/gdb/python/py-unwind.c
+++ b/gdb/python/py-unwind.c
@@ -425,7 +425,7 @@ pending_framepy_repr (PyObject *self)
   frame_info_ptr frame = pending_frame->frame_info;
 
   if (frame == nullptr)
-    return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (self)->tp_name);
+    return gdb_py_invalid_object_repr (self);
 
   const char *sp_str = nullptr;
   const char *pc_str = nullptr;
diff --git a/gdb/python/py-utils.c b/gdb/python/py-utils.c
index f1ca9ea0a5d..c29291004d2 100644
--- a/gdb/python/py-utils.c
+++ b/gdb/python/py-utils.c
@@ -597,3 +597,11 @@ gdbpy_fix_doc_string_indentation (gdb::unique_xmalloc_ptr<char> doc)
 
   return doc;
 }
+
+/* See python-internal.h.  */
+
+PyObject *
+gdb_py_invalid_object_repr (PyObject *self)
+{
+  return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (self)->tp_name);
+}
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 14e15574685..8ff9af650c2 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -897,6 +897,15 @@ int gdb_pymodule_addobject (PyObject *module, const char *name,
 			    PyObject *object)
   CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
 
+
+/* Return a Python string (str) object that represents SELF.  SELF can be
+   any object type, but should be in an "invalid" state.  What "invalid"
+   means is up to the caller.  The returned string will take the form
+   "<TYPENAME (invalid)>", without the quotes, and with TYPENAME replaced
+   with the type of SELF.  */
+
+PyObject *gdb_py_invalid_object_repr (PyObject *self);
+
 struct varobj_iter;
 struct varobj;
 std::unique_ptr<varobj_iter> py_varobj_get_iterator
-- 
2.25.4

[PATCH] gdb/python: New InferiorThread.ptid_string attribute

[Andrew Burgess]

This commit adds a new InferiorThread.ptid_string attribute.  This
read-only attribute contains the string returned by target_pid_to_str,
which actually converts a ptid (not pid) to a string.

This is the string that appears (at least in part) in the output of
'info threads' in the 'Target Id' column, but also in the thread
exited message that GDB prints.

Having access to this string from Python is useful for allowing
extensions identify threads in a similar way to how GDB core would
identify the thread.
---
 gdb/NEWS                                  |  4 ++++
 gdb/doc/python.texi                       |  7 +++++++
 gdb/python/py-infthread.c                 | 20 ++++++++++++++++++++
 gdb/testsuite/gdb.python/py-infthread.exp |  8 ++++++++
 4 files changed, 39 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index 11cd6c0663e..60e58daa501 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -87,6 +87,10 @@ show remote thread-options-packet
   ** New function gdb.interrupt(), that interrupts GDB as if the user
      typed control-c.
 
+  ** New gdb.InferiorThread.ptid_string attribute.  This read-only
+     attribute contains the string that appears in the 'Target Id'
+     column of the 'info threads' command output.
+
 * Debugger Adapter Protocol changes
 
   ** GDB now emits the "process" event.
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index d74defeec0c..da37348d663 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -4084,6 +4084,13 @@
 does not  use that identifier.
 @end defvar
 
+@defvar InferiorThread.ptid_string
+This read-only attribute contains a string representing
+@code{InferiorThread.ptid}.  This is the string that @value{GDBN} uses
+in the @samp{Target Id} column in the @kbd{info threads} output
+(@pxref{info_threads,,@samp{info threads}}).
+@end defvar
+
 @defvar InferiorThread.inferior
 The inferior this thread belongs to.  This attribute is represented as
 a @code{gdb.Inferior} object.  This attribute is not writable.
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index 00d7171de64..7307b9e9da9 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -185,6 +185,23 @@ thpy_get_ptid (PyObject *self, void *closure)
   return gdbpy_create_ptid_object (thread_obj->thread->ptid);
 }
 
+/* Implement gdb.InferiorThread.ptid_string attribute.  */
+
+static PyObject *
+thpy_get_ptid_string (PyObject *self, void *closure)
+{
+  thread_object *thread_obj = (thread_object *) self;
+  THPY_REQUIRE_VALID (thread_obj);
+  ptid_t ptid = thread_obj->thread->ptid;
+
+  /* Select the correct inferior before calling a target_* function.  */
+  scoped_restore_current_thread restore_thread;
+  switch_to_inferior_no_thread (thread_obj->thread->inf);
+  std::string ptid_str = target_pid_to_str (ptid);
+
+  return PyUnicode_FromString (ptid_str.c_str ());
+}
+
 /* Getter for InferiorThread.inferior -> Inferior.  */
 
 static PyObject *
@@ -388,6 +405,9 @@ static gdb_PyGetSetDef thread_object_getset[] =
     "Global number of the thread, as assigned by GDB.", NULL },
   { "ptid", thpy_get_ptid, NULL, "ID of the thread, as assigned by the OS.",
     NULL },
+  { "ptid_string", thpy_get_ptid_string, nullptr,
+    "A string representing ptid, as used by, for example, 'info threads'.",
+    nullptr },
   { "inferior", thpy_get_inferior, NULL,
     "The Inferior object this thread belongs to.", NULL },
 
diff --git a/gdb/testsuite/gdb.python/py-infthread.exp b/gdb/testsuite/gdb.python/py-infthread.exp
index 0b10ce9ff77..3b07d84143f 100644
--- a/gdb/testsuite/gdb.python/py-infthread.exp
+++ b/gdb/testsuite/gdb.python/py-infthread.exp
@@ -62,6 +62,14 @@ gdb_test "python print ('result = %s' % t0.num)" " = 1" "test InferiorThread.num
 gdb_test "python print ('result = %s' % t0.global_num)" " = 1" "test InferiorThread.global_num"
 gdb_test "python print ('result = %s' % str (t0.ptid))" " = \\(\[0-9\]+, \[0-9\]+, \[0-9\]+\\)" "test InferiorThread.ptid"
 
+# Test the InferiorThread.ptid_string attribute.  We don't test the
+# actual string contents as they vary based on target, but we check
+# that we get back a non-empty string.
+gdb_test "python print(type(t0.ptid_string))" "<class 'str'>" \
+    "check that InferiorThread.ptid_string is a string"
+gdb_test "python print(t0.ptid_string)" ".+" \
+    "check that InferiorThread.ptid_string is non-empty"
+
 gdb_py_test_silent_cmd "python i0 = t0.inferior" "test InferiorThread.inferior" 1
 gdb_test "python print ('result = %s' % i0.num)" " = 1" "test Inferior.num"
 

base-commit: b7a5722ebdd24a0d15d56e96d30a649ea1d7b0ee
-- 
2.25.4

[PATCHv2 2/8] gdb/python: add gdb.InferiorThread.__repr__() method

[Andrew Burgess]

Add a gdb.InferiorThread.__repr__() method.  Before this patch we
would see output like this:

  (gdb) pi
  >>> gdb.selected_thread()
  <gdb.InferiorThread object at 0x7f4dcc49b970>

After this patch, we now see:

  (gdb) pi
  >>> gdb.selected_thread()
  <gdb.InferiorThread id=1.2 target-id="Thread 0x7ffff7da1700 (LWP 458134)">

More verbose, but, I hope, more useful.

If the gdb.InferiorThread becomes invalid, then we will see:

  (gdb) pi
  >>> invalid_thread_variable
  <gdb.InferiorThread (invalid)>

Which is inline with how other invalid objects are displayed.
---
 gdb/python/py-infthread.c                 | 19 ++++++++++++++++++-
 gdb/testsuite/gdb.python/py-inferior.exp  | 16 +++++++++++++++-
 gdb/testsuite/gdb.python/py-infthread.exp |  3 ++-
 3 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index 00d7171de64..2d892b10b69 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -313,6 +313,23 @@ thpy_thread_handle (PyObject *self, PyObject *args)
   return object;
 }
 
+/* Implement repr() for gdb.InferiorThread.  */
+
+static PyObject *
+thpy_repr (PyObject *self)
+{
+  thread_object *thread_obj = (thread_object *) self;
+
+  if (thread_obj->thread == nullptr)
+    return gdb_py_invalid_object_repr (self);
+
+  thread_info *thr = thread_obj->thread;
+  return PyUnicode_FromFormat ("<%s id=%s target-id=\"%s\">",
+			       Py_TYPE (self)->tp_name,
+			       print_full_thread_id (thr),
+			       target_pid_to_str (thr->ptid).c_str ());
+}
+
 /* Return a reference to a new Python object representing a ptid_t.
    The object is a tuple containing (pid, lwp, tid). */
 PyObject *
@@ -429,7 +446,7 @@ PyTypeObject thread_object_type =
   0,				  /*tp_getattr*/
   0,				  /*tp_setattr*/
   0,				  /*tp_compare*/
-  0,				  /*tp_repr*/
+  thpy_repr,			  /*tp_repr*/
   0,				  /*tp_as_number*/
   0,				  /*tp_as_sequence*/
   0,				  /*tp_as_mapping*/
diff --git a/gdb/testsuite/gdb.python/py-inferior.exp b/gdb/testsuite/gdb.python/py-inferior.exp
index 6fbcdd6822f..5a221f800c3 100644
--- a/gdb/testsuite/gdb.python/py-inferior.exp
+++ b/gdb/testsuite/gdb.python/py-inferior.exp
@@ -78,7 +78,9 @@ gdb_test "python print ('result = %s' % (i0.connection_num == i0.connection.num)
     "Inferior.connection_num equals Inferior.connection.num"
 gdb_test "python print ('result = %s' % i0.pid)" " = \[0-9\]+" "test Inferior.pid"
 gdb_test "python print ('result = %s' % i0.was_attached)" " = False" "test Inferior.was_attached"
-gdb_test "python print (i0.threads ())" "\\(<gdb.InferiorThread object at 0x\[\[:xdigit:\]\]+>,\\)" "test Inferior.threads"
+gdb_test "python print (i0.threads ())" \
+    "\\(<gdb.InferiorThread id=${decimal}\\.${decimal} target-id=\"\[^\r\n\]*\">,\\)" \
+    "test Inferior.threads"
 
 gdb_test "python print (i0.progspace)" "<gdb.Progspace object at $hex>"
 gdb_test "python print (i0.progspace == gdb.progspaces()\[0\])" "True"
@@ -89,11 +91,23 @@ gdb_breakpoint check_threads
 gdb_continue_to_breakpoint "cont to check_threads" ".*pthread_barrier_wait.*"
 gdb_test "python print (len (i0.threads ()))" "\r\n9" "test Inferior.threads 2"
 
+# Grab the last thread from the list.  This thread object will become
+# invalid when the corresponding thread exits.
+gdb_test_no_output "python last_thread = i0.threads()\[-1\]"
+gdb_test "python print(last_thread)" \
+    "<gdb.InferiorThread id=${decimal}\\.${decimal} target-id=\"\[^\r\n\]*\">" \
+    "test repr of a valid thread"
+
 # Proceed to the next test.
 
 gdb_breakpoint [gdb_get_line_number "Break here."]
 gdb_continue_to_breakpoint "cont to Break here." ".*Break here\..*"
 
+# Check the repr() for an invalid gdb.InferiorThread object.
+gdb_test "python print(last_thread)" \
+    "<gdb.InferiorThread \\(invalid\\)>" \
+    "test repr of an invalid thread"
+
 # Test memory read and write operations.
 
 gdb_py_test_silent_cmd "python addr = gdb.selected_frame ().read_var ('str')" \
diff --git a/gdb/testsuite/gdb.python/py-infthread.exp b/gdb/testsuite/gdb.python/py-infthread.exp
index 0b10ce9ff77..49622c8ac28 100644
--- a/gdb/testsuite/gdb.python/py-infthread.exp
+++ b/gdb/testsuite/gdb.python/py-infthread.exp
@@ -57,7 +57,8 @@ gdb_py_test_silent_cmd "python gdb.selected_thread ().inferior" "test InferiorTh
 
 
 gdb_py_test_silent_cmd "python t0 = gdb.selected_thread ()" "test gdb.selected_thread" 1
-gdb_test "python print (t0)" "\\<gdb.InferiorThread object at 0x\[\[:xdigit:\]\]+>" "verify InferiorThread object"
+gdb_test "python print (t0)" \
+    "<gdb.InferiorThread id=${decimal}\\.${decimal} target-id=\"\[^\r\n\]*\">"
 gdb_test "python print ('result = %s' % t0.num)" " = 1" "test InferiorThread.num"
 gdb_test "python print ('result = %s' % t0.global_num)" " = 1" "test InferiorThread.global_num"
 gdb_test "python print ('result = %s' % str (t0.ptid))" " = \\(\[0-9\]+, \[0-9\]+, \[0-9\]+\\)" "test InferiorThread.ptid"
-- 
2.25.4

[PATCHv2 3/8] gdb/python: add gdb.Frame.__repr__() method

[Andrew Burgess]

Add a gdb.Frame.__repr__() method.  Before this patch we would see
output like this:

  (gdb) pi
  >>> gdb.selected_frame()
  <gdb.Frame object at 0x7fa8cc2df270>

After this patch, we now see:

  (gdb) pi
  >>> gdb.selected_frame()
  <gdb.Frame level=0 frame-id={stack=0x7ffff7da0ed0,code=0x000000000040115d,!special}>

More verbose, but, I hope, more useful.

If the gdb.Frame becomes invalid, then we will see:

  (gdb) pi
  >>> invalid_frame_variable
  <gdb.Frame (invalid)>

which is inline with how other invalid objects are displayed.
---
 gdb/python/py-frame.c                 | 19 ++++++++++++++++++-
 gdb/testsuite/gdb.python/py-frame.exp |  8 ++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index f38bf5d8237..ee465ed64a1 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -84,6 +84,23 @@ frapy_str (PyObject *self)
   return PyUnicode_FromString (fid.to_string ().c_str ());
 }
 
+/* Implement repr() for gdb.Frame.  */
+
+static PyObject *
+frapy_repr (PyObject *self)
+{
+  frame_object *frame_obj = (frame_object *) self;
+  frame_info_ptr f_info = frame_find_by_id (frame_obj->frame_id);
+  if (f_info == nullptr)
+    return gdb_py_invalid_object_repr (self);
+
+  const frame_id &fid = frame_obj->frame_id;
+  return PyUnicode_FromFormat ("<%s level=%d frame-id=%s>",
+			       Py_TYPE (self)->tp_name,
+			       frame_relative_level (f_info),
+			       fid.to_string ().c_str ());
+}
+
 /* Implementation of gdb.Frame.is_valid (self) -> Boolean.
    Returns True if the frame corresponding to the frame_id of this
    object still exists in the inferior.  */
@@ -840,7 +857,7 @@ PyTypeObject frame_object_type = {
   0,				  /* tp_getattr */
   0,				  /* tp_setattr */
   0,				  /* tp_compare */
-  0,				  /* tp_repr */
+  frapy_repr,			  /* tp_repr */
   0,				  /* tp_as_number */
   0,				  /* tp_as_sequence */
   0,				  /* tp_as_mapping */
diff --git a/gdb/testsuite/gdb.python/py-frame.exp b/gdb/testsuite/gdb.python/py-frame.exp
index 16177c8a5f8..6162696948f 100644
--- a/gdb/testsuite/gdb.python/py-frame.exp
+++ b/gdb/testsuite/gdb.python/py-frame.exp
@@ -36,6 +36,10 @@ gdb_breakpoint [gdb_get_line_number "Block break here."]
 gdb_continue_to_breakpoint "Block break here."
 gdb_py_test_silent_cmd "python bf1 = gdb.selected_frame ()" "get frame" 0
 
+# Test Frame.__repr__() method for a valid Frame object.
+gdb_test "python print (repr(bf1))" "<gdb\\.Frame level=0 frame-id=\\{\[^\r\n\]+\\}>" \
+    "test __repr__ for gdb.Frame on a valid frame"
+
 # Test Frame.architecture() method.
 gdb_py_test_silent_cmd "python show_arch_str = gdb.execute(\"show architecture\", to_string=True)" "show arch" 0
 gdb_test "python print (bf1.architecture().name() in show_arch_str)" "True" "test Frame.architecture()"
@@ -100,6 +104,10 @@ gdb_py_test_silent_cmd "python bframe = gdb.selected_frame()" \
     "get bottommost frame" 0
 gdb_test "up" ".*" ""
 
+# Test Frame.__repr__() method for an invalid Frame object.
+gdb_test "python print (repr(bf1))" "<gdb\\.Frame \\(invalid\\)>" \
+    "test __repr__ for gdb.Frame on an invalid frame"
+
 gdb_py_test_silent_cmd "python f1 = gdb.selected_frame ()" "get second frame" 0
 gdb_py_test_silent_cmd "python f0 = f1.newer ()" "get first frame" 0
 gdb_py_test_silent_cmd "python f2 = f1.older ()" "get last frame" 0
-- 
2.25.4

[PATCHv2 4/8] gdb/python: remove users ability to create gdb.Progspace objects

[Andrew Burgess]

I noticed that it is possible for the user to create a new
gdb.Progspace object, like this:

  (gdb) pi
  >>> p = gdb.Progspace()
  >>> p
  <gdb.Progspace object at 0x7ffad4219c10>
  >>> p.is_valid()
  False

As the new gdb.Progspace object is not associated with an actual C++
program_space object within GDB core, then the new gdb.Progspace is
created invalid, and there is no way in which the new object can ever
become valid.

Nor do I believe there's anywhere in the Python API where it makes
sense to consume an invalid gdb.Progspace created in this way, for
example, the gdb.Progspace could be passed as the locus to
register_type_printer, but all that would happen is that the
registered printer would never be used.

In this commit I propose to remove the ability to create new
gdb.Progspace objects.  Attempting to do so now gives an error, like
this:

  (gdb) pi
  >>> gdb.Progspace()
  Traceback (most recent call last):
    File "<stdin>", line 1, in <module>
  TypeError: cannot create 'gdb.Progspace' instances

Of course, there is a small risk here that some existing user code
might break ... but if that happens I don't believe the user code can
have been doing anything useful, so I see this as a small risk.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
---
 gdb/NEWS                                  |  5 +++++
 gdb/python/py-progspace.c                 | 16 +---------------
 gdb/testsuite/gdb.python/py-progspace.exp |  6 ++++++
 3 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 11cd6c0663e..36443c38aca 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -87,6 +87,11 @@ show remote thread-options-packet
   ** New function gdb.interrupt(), that interrupts GDB as if the user
      typed control-c.
 
+  ** It is no longer possible to create new gdb.Progspace object using
+     'gdb.Progspace()', this will result in a TypeError.  Progspace
+     objects can still be obtained through calling other API
+     functions, for example 'gdb.current_progspace()'.
+
 * Debugger Adapter Protocol changes
 
   ** GDB now emits the "process" event.
diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
index 0797ef1fa6b..bfc6ff50f01 100644
--- a/gdb/python/py-progspace.c
+++ b/gdb/python/py-progspace.c
@@ -210,20 +210,6 @@ pspy_initialize (pspace_object *self)
   return 1;
 }
 
-static PyObject *
-pspy_new (PyTypeObject *type, PyObject *args, PyObject *keywords)
-{
-  gdbpy_ref<pspace_object> self ((pspace_object *) type->tp_alloc (type, 0));
-
-  if (self != NULL)
-    {
-      if (!pspy_initialize (self.get ()))
-	return NULL;
-    }
-
-  return (PyObject *) self.release ();
-}
-
 PyObject *
 pspy_get_printers (PyObject *o, void *ignore)
 {
@@ -859,5 +845,5 @@ PyTypeObject pspace_object_type =
   offsetof (pspace_object, dict), /* tp_dictoffset */
   0,				  /* tp_init */
   0,				  /* tp_alloc */
-  pspy_new,			  /* tp_new */
+  0,				  /* tp_new */
 };
diff --git a/gdb/testsuite/gdb.python/py-progspace.exp b/gdb/testsuite/gdb.python/py-progspace.exp
index befd6433e47..5f6a9577f82 100644
--- a/gdb/testsuite/gdb.python/py-progspace.exp
+++ b/gdb/testsuite/gdb.python/py-progspace.exp
@@ -53,6 +53,12 @@ gdb_py_test_silent_cmd "python progspace.random_attribute = 42" \
 gdb_test "python print (progspace.random_attribute)" "42" \
     "Verify set of random attribute in progspace"
 
+# Check that we can't create new (invalid) gdb.Progspace objects.
+gdb_test "python gdb.Progspace()" \
+    [multi_line "TypeError: cannot create 'gdb.Progspace' instances" \
+	 "Error while executing Python code\\."] \
+    "check for error when calling gdb.Progspace() directly"
+
 if {![runto_main]} {
     return
 }
-- 
2.25.4

[PATCHv2 5/8] gdb/python: Add gdb.Inferior.__dict__ attribute

[Andrew Burgess]

The gdb.Objfile, gdb.Progspace, and gdb.Type Python types already have
a __dict__ attribute, which allows users to create user defined
attributes within the objects.  This is useful if the user wants to
cache information within an object.

This commit adds the same functionality to the gdb.Inferior type.

After this commit there is a new gdb.Inferior.__dict__ attribute,
which is a dictionary.  A user can, for example, do this:

  (gdb) pi
  >>> i = gdb.selected_inferior()
  >>> i._user_attribute = 123
  >>> i._user_attribute
  123
  >>>

There's a new test included.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
---
 gdb/NEWS                                 |  4 +++
 gdb/doc/python.texi                      | 36 ++++++++++++++++++++++++
 gdb/python/py-inferior.c                 | 13 ++++++++-
 gdb/testsuite/gdb.python/py-inferior.exp | 19 +++++++++++++
 4 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 36443c38aca..500d5ab7160 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -92,6 +92,10 @@ show remote thread-options-packet
      objects can still be obtained through calling other API
      functions, for example 'gdb.current_progspace()'.
 
+  ** User defined attributes can be added to a gdb.Inferior object,
+     these will be stored in the object's new Inferior.__dict__
+     attribute.
+
 * Debugger Adapter Protocol changes
 
   ** GDB now emits the "process" event.
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index d74defeec0c..0e6eeeeb15d 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -3667,6 +3667,42 @@
 string.
 @end defun
 
+One may add arbitrary attributes to @code{gdb.Inferior} objects in the
+usual Python way.  This is useful if, for example, one needs to do
+some extra record keeping associated with the inferior.
+
+In this contrived example we record the time when an inferior last
+stopped:
+
+@smallexample
+@group
+(@value{GDBP}) python
+import datetime
+
+def thread_stopped(event):
+    if event.inferior_thread is not None:
+        thread = event.inferior_thread
+    else:
+        thread = gdb.selected_thread()
+    inferior = thread.inferior
+    inferior._last_stop_time = datetime.datetime.today()
+
+gdb.events.stop.connect(thread_stopped)
+@end group
+@group
+(@value{GDBP}) file /tmp/hello
+Reading symbols from /tmp/hello...
+(@value{GDBP}) start
+Temporary breakpoint 1 at 0x401198: file /tmp/hello.c, line 18.
+Starting program: /tmp/hello
+
+Temporary breakpoint 1, main () at /tmp/hello.c:18
+18	  printf ("Hello World\n");
+(@value{GDBP}) python print(gdb.selected_inferior()._last_stop_time)
+2024-01-04 14:48:41.347036
+@end group
+@end smallexample
+
 @node Events In Python
 @subsubsection Events In Python
 @cindex inferior events in Python
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 929d8bd17b5..3f14bc31459 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -46,6 +46,10 @@ struct inferior_object
   /* thread_object instances under this inferior.  This owns a
      reference to each object it contains.  */
   thread_map_t *threads;
+
+  /* Dictionary holding user-added attributes.
+     This is the __dict__ attribute of the object.  */
+  PyObject *dict;
 };
 
 extern PyTypeObject inferior_object_type
@@ -241,6 +245,9 @@ inferior_to_inferior_object (struct inferior *inferior)
 
       inf_obj->inferior = inferior;
       inf_obj->threads = new thread_map_t ();
+      inf_obj->dict = PyDict_New ();
+      if (inf_obj->dict == nullptr)
+	return nullptr;
 
       /* PyObject_New initializes the new object with a refcount of 1.  This
 	 counts for the reference we are keeping in the inferior data.  */
@@ -981,6 +988,8 @@ infpy_dealloc (PyObject *obj)
      function is called.  */
   gdb_assert (inf_obj->inferior == nullptr);
 
+  Py_XDECREF (inf_obj->dict);
+
   Py_TYPE (obj)->tp_free (obj);
 }
 
@@ -1038,6 +1047,8 @@ GDBPY_INITIALIZE_FILE (gdbpy_initialize_inferior);
 
 static gdb_PyGetSetDef inferior_object_getset[] =
 {
+  { "__dict__", gdb_py_generic_dict, nullptr,
+    "The __dict__ for this inferior.", &inferior_object_type },
   { "arguments", infpy_get_args, infpy_set_args,
     "Arguments to this program.", nullptr },
   { "num", infpy_get_num, NULL, "ID of inferior, as assigned by GDB.", NULL },
@@ -1135,7 +1146,7 @@ PyTypeObject inferior_object_type =
   0,				  /* tp_dict */
   0,				  /* tp_descr_get */
   0,				  /* tp_descr_set */
-  0,				  /* tp_dictoffset */
+  offsetof (inferior_object, dict), /* tp_dictoffset */
   0,				  /* tp_init */
   0				  /* tp_alloc */
 };
diff --git a/gdb/testsuite/gdb.python/py-inferior.exp b/gdb/testsuite/gdb.python/py-inferior.exp
index 5a221f800c3..0e00636fa1c 100644
--- a/gdb/testsuite/gdb.python/py-inferior.exp
+++ b/gdb/testsuite/gdb.python/py-inferior.exp
@@ -85,6 +85,15 @@ gdb_test "python print (i0.threads ())" \
 gdb_test "python print (i0.progspace)" "<gdb.Progspace object at $hex>"
 gdb_test "python print (i0.progspace == gdb.progspaces()\[0\])" "True"
 
+# Add a user defined attribute to the inferior, and check the
+# attribute can be read back.
+gdb_test_no_output "python i0._user_attr = 123" \
+    "add a user defined attribute to the inferior object"
+gdb_test "python print(i0._user_attr)" \
+    "123" "read back user defined attribute from i0"
+gdb_test "python print(gdb.inferiors()\[0\]._user_attr)" \
+    "123" "read back user defined attribute from gdb.inferiors"
+
 # Test the number of inferior threads.
 
 gdb_breakpoint check_threads
@@ -127,6 +136,16 @@ gdb_test "print str" " = \"hallo, testsuite\"" \
 # correct inferior.
 set num [add_inferior]
 
+# Confirm the new inferior doesn't have the user defined attribute,
+# but that the first inferior does still have the attribute.
+gdb_test "python print(gdb.inferiors()\[1\]._user_attr)" \
+    [multi_line \
+	 "AttributeError: 'gdb\\.Inferior' object has no attribute '_user_attr'" \
+	 "Error while executing Python code\\."] \
+    "check new inferior doesn't have user defined attribute"
+gdb_test "python print(gdb.inferiors()\[0\]._user_attr)" \
+    "123" "read back user defined attribute again"
+
 # Test memory search.
 
 set hex_number {0x[0-9a-fA-F][0-9a-fA-F]*}
-- 
2.25.4

[PATCHv2 6/8] gdb/python: Add gdb.InferiorThread.__dict__ attribute

[Andrew Burgess]

The gdb.Objfile, gdb.Progspace, gdb.Type, and gdb.Inferior Python
types already have a __dict__ attribute, which allows users to create
user defined attributes within the objects.  This is useful if the
user wants to cache information within an object.

This commit adds the same functionality to the gdb.InferiorThread
type.

After this commit there is a new gdb.InferiorThread.__dict__
attribute, which is a dictionary.  A user can, for example, do this:

  (gdb) pi
  >>> t = gdb.selected_thread()
  >>> t._user_attribute = 123
  >>> t._user_attribute
  123
  >>>

There's a new test included.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
---
 gdb/NEWS                                 |  4 +++
 gdb/doc/python.texi                      | 35 ++++++++++++++++++++++++
 gdb/python/py-infthread.c                | 15 ++++++++--
 gdb/python/python-internal.h             |  4 +++
 gdb/testsuite/gdb.python/py-inferior.exp | 17 ++++++++++++
 5 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 500d5ab7160..c4862a8beb6 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -96,6 +96,10 @@ show remote thread-options-packet
      these will be stored in the object's new Inferior.__dict__
      attribute.
 
+  ** User defined attributes can be added to a gdb.InferiorThread
+     object, these will be stored in the object's new
+     InferiorThread.__dict__ attribute.
+
 * Debugger Adapter Protocol changes
 
   ** GDB now emits the "process" event.
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 0e6eeeeb15d..2eed332fe59 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -4177,6 +4177,41 @@
 a @code{gdb.Type} for the handle type.
 @end defun
 
+One may add arbitrary attributes to @code{gdb.InferiorThread} objects
+in the usual Python way.  This is useful if, for example, one needs to
+do some extra record keeping associated with the thread.
+
+In this contrived example we record the time when a thread last
+stopped:
+
+@smallexample
+@group
+(@value{GDBP}) python
+import datetime
+
+def thread_stopped(event):
+    if event.inferior_thread is not None:
+        thread = event.inferior_thread
+    else:
+        thread = gdb.selected_thread()
+    thread._last_stop_time = datetime.datetime.today()
+
+gdb.events.stop.connect(thread_stopped)
+@end group
+@group
+(@value{GDBP}) file /tmp/hello
+Reading symbols from /tmp/hello...
+(@value{GDBP}) start
+Temporary breakpoint 1 at 0x401198: file /tmp/hello.c, line 18.
+Starting program: /tmp/hello
+
+Temporary breakpoint 1, main () at /tmp/hello.c:18
+18	  printf ("Hello World\n");
+(@value{GDBP}) python print(gdb.selected_thread()._last_stop_time)
+2024-01-04 14:48:41.347036
+@end group
+@end smallexample
+
 @node Recordings In Python
 @subsubsection Recordings In Python
 @cindex recordings in python
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index 2d892b10b69..d1fb5bd4160 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -51,6 +51,9 @@ create_thread_object (struct thread_info *tp)
 
   thread_obj->thread = tp;
   thread_obj->inf_obj = (PyObject *) inf_obj.release ();
+  thread_obj->dict = PyDict_New ();
+  if (thread_obj->dict == nullptr)
+    return nullptr;
 
   return thread_obj;
 }
@@ -58,7 +61,13 @@ create_thread_object (struct thread_info *tp)
 static void
 thpy_dealloc (PyObject *self)
 {
-  Py_DECREF (((thread_object *) self)->inf_obj);
+  thread_object *thr_obj = (thread_object *) self;
+
+  gdb_assert (thr_obj->inf_obj != nullptr);
+
+  Py_DECREF (thr_obj->inf_obj);
+  Py_XDECREF (thr_obj->dict);
+
   Py_TYPE (self)->tp_free (self);
 }
 
@@ -394,6 +403,8 @@ GDBPY_INITIALIZE_FILE (gdbpy_initialize_thread);
 
 static gdb_PyGetSetDef thread_object_getset[] =
 {
+  { "__dict__", gdb_py_generic_dict, nullptr,
+    "The __dict__ for this thread.", &thread_object_type },
   { "name", thpy_get_name, thpy_set_name,
     "The name of the thread, as set by the user or the OS.", NULL },
   { "details", thpy_get_details, NULL,
@@ -471,7 +482,7 @@ PyTypeObject thread_object_type =
   0,				  /* tp_dict */
   0,				  /* tp_descr_get */
   0,				  /* tp_descr_set */
-  0,				  /* tp_dictoffset */
+  offsetof (thread_object, dict), /* tp_dictoffset */
   0,				  /* tp_init */
   0				  /* tp_alloc */
 };
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 8ff9af650c2..e01557edeb7 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -356,6 +356,10 @@ struct thread_object
 
   /* The Inferior object to which this thread belongs.  */
   PyObject *inf_obj;
+
+  /* Dictionary holding user-added attributes.  This is the __dict__
+     attribute of the object.  */
+  PyObject *dict;
 };
 
 struct inferior_object;
diff --git a/gdb/testsuite/gdb.python/py-inferior.exp b/gdb/testsuite/gdb.python/py-inferior.exp
index 0e00636fa1c..d1cd29f734b 100644
--- a/gdb/testsuite/gdb.python/py-inferior.exp
+++ b/gdb/testsuite/gdb.python/py-inferior.exp
@@ -107,6 +107,19 @@ gdb_test "python print(last_thread)" \
     "<gdb.InferiorThread id=${decimal}\\.${decimal} target-id=\"\[^\r\n\]*\">" \
     "test repr of a valid thread"
 
+# Add a user defined attribute to this thread, check the attribute can
+# be read back, and check the attribute is not present on other
+# threads.
+gdb_test_no_output "python last_thread._user_attribute = 123" \
+    "add user defined attribute to InferiorThread object"
+gdb_test "python print(last_thread._user_attribute)" "123" \
+    "read back user defined attribute"
+gdb_test "python print(i0.threads ()\[0\]._user_attribute)" \
+    [multi_line \
+	 "AttributeError: 'gdb\\.InferiorThread' object has no attribute '_user_attribute'" \
+	 "Error while executing Python code\\."] \
+    "attempt to read non-existent user defined attribute"
+
 # Proceed to the next test.
 
 gdb_breakpoint [gdb_get_line_number "Break here."]
@@ -117,6 +130,10 @@ gdb_test "python print(last_thread)" \
     "<gdb.InferiorThread \\(invalid\\)>" \
     "test repr of an invalid thread"
 
+# Check the user defined attribute is still present on the invalid thread object.
+gdb_test "python print(last_thread._user_attribute)" "123" \
+    "check user defined attribute on an invalid InferiorThread object"
+
 # Test memory read and write operations.
 
 gdb_py_test_silent_cmd "python addr = gdb.selected_frame ().read_var ('str')" \
-- 
2.25.4

[PATCHv2 7/8] gdb/doc: add some notes on selecting suitable attribute names

[Andrew Burgess]

In previous commits I've added Object.__dict__ support to gdb.Inferior
and gdb.InferiorThread, this is similar to the existing support for
gdb.Objfile and gdb.Progspace.

This commit extends the documentation to offer the user some guidance
on selecting good names for their custom attributes so they
can (hopefully) avoid conflicting with any future attributes that GDB
might add.

The rules I've proposed are:

  1. Don't start user attributes with a lower case letter, all the
  current GDB attributes start with a lower case letter, and I suspect
  all future attributes would also start with a lower case letter, and

  2. Don't start user attributes with a double underscore, this risks
  conflicting with Python built in attributes (e.g. __dict__) - though
  clearly the user would need to start and end with a double
  underscore, but it seemed easier just to say no double underscores.

I'm doing this as a separate commit as I've updated the docs for the
existing gdb.Objfile and gdb.Progspace so they all reference a single
paragraph on selecting attribute names.
---
 gdb/doc/python.texi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 2eed332fe59..40803274a27 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -3671,6 +3671,13 @@
 usual Python way.  This is useful if, for example, one needs to do
 some extra record keeping associated with the inferior.
 
+@anchor{choosing attribute names}
+When selecting a name for a new attribute, avoid starting the new
+attribute name with a lower case letter; future attributes added by
+@value{GDBN} will start with a lower case letter.  Additionally, avoid
+starting attribute names with two underscore characters, as these
+could clash with Python builtin attribute names.
+
 In this contrived example we record the time when an inferior last
 stopped:
 
@@ -4181,6 +4188,9 @@
 in the usual Python way.  This is useful if, for example, one needs to
 do some extra record keeping associated with the thread.
 
+See @ref{choosing attribute names} for guidance on selecting a
+suitable name for new attributes.
+
 In this contrived example we record the time when a thread last
 stopped:
 
@@ -5396,6 +5406,9 @@
 This is useful if, for example, one needs to do some extra record keeping
 associated with the program space.
 
+See @ref{choosing attribute names} for guidance on selecting a
+suitable name for new attributes.
+
 In this contrived example, we want to perform some processing when
 an objfile with a certain symbol is loaded, but we only want to do
 this once because it is expensive.  To achieve this we record the results
@@ -5556,6 +5569,9 @@
 This is useful if, for example, one needs to do some extra record keeping
 associated with the objfile.
 
+See @ref{choosing attribute names} for guidance on selecting a
+suitable name for new attributes.
+
 In this contrived example we record the time when @value{GDBN}
 loaded the objfile.
 
-- 
2.25.4

[PATCHv2 8/8] gdb/doc: update examples in gdb.Progspace and gdb.Objfile docs

[Andrew Burgess]

This commit updates the Python example code in the gdb.Progspace and
gdb.Objfile sections of the docs.  Changes made:

  1. Use @value{GDBP} for the GDB prompt rather than
  hard-coding (gdB),

  2. Use @group...@end group to split the example code into
  unbreakable chunks, and

  3. Add parenthesis to the Python print() calls in the examples.  In
  Python 2 it was OK to drop the parenthesis, but now GDB is Python 3
  only, example code should include the parenthesis.
---
 gdb/doc/python.texi | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 40803274a27..0b240c76dce 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -5416,13 +5416,16 @@
 will be loaded.
 
 @smallexample
-(gdb) python
+(@value{GDBP}) python
+@group
 def clear_objfiles_handler(event):
     event.progspace.expensive_computation = None
 def expensive(symbol):
     """A mock routine to perform an "expensive" computation on symbol."""
     print ("Computing the answer to the ultimate question ...")
     return 42
+@end group
+@group
 def new_objfile_handler(event):
     objfile = event.new_objfile
     progspace = objfile.progspace
@@ -5437,15 +5440,18 @@
 gdb.events.clear_objfiles.connect(clear_objfiles_handler)
 gdb.events.new_objfile.connect(new_objfile_handler)
 end
-(gdb) file /tmp/hello
+@end group
+@group
+(@value{GDBP}) file /tmp/hello
 Reading symbols from /tmp/hello...
 Computing the answer to the ultimate question ...
-(gdb) python print gdb.current_progspace().expensive_computation
+(@value{GDBP}) python print(gdb.current_progspace().expensive_computation)
 42
-(gdb) run
+(@value{GDBP}) run
 Starting program: /tmp/hello
 Hello.
 [Inferior 1 (process 4242) exited normally]
+@end group
 @end smallexample
 
 @node Objfiles In Python
@@ -5576,17 +5582,21 @@
 loaded the objfile.
 
 @smallexample
-(gdb) python
+@group
+(@value{GDBP}) python
 import datetime
 def new_objfile_handler(event):
     # Set the time_loaded attribute of the new objfile.
     event.new_objfile.time_loaded = datetime.datetime.today()
 gdb.events.new_objfile.connect(new_objfile_handler)
 end
-(gdb) file ./hello
+@end group
+@group
+(@value{GDBP}) file ./hello
 Reading symbols from ./hello...
-(gdb) python print gdb.objfiles()[0].time_loaded
+(@value{GDBP}) python print(gdb.objfiles()[0].time_loaded)
 2014-10-09 11:41:36.770345
+@end group
 @end smallexample
 
 A @code{gdb.Objfile} object has the following methods:
-- 
2.25.4

Re: [PATCHv2 7/8] gdb/doc: add some notes on selecting suitable attribute names

[Eli Zaretskii]

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: Andrew Burgess <aburgess@redhat.com>
> Date: Wed, 10 Jan 2024 15:54:44 +0000
> 
> In previous commits I've added Object.__dict__ support to gdb.Inferior
> and gdb.InferiorThread, this is similar to the existing support for
> gdb.Objfile and gdb.Progspace.
> 
> This commit extends the documentation to offer the user some guidance
> on selecting good names for their custom attributes so they
> can (hopefully) avoid conflicting with any future attributes that GDB
> might add.
> 
> The rules I've proposed are:
> 
>   1. Don't start user attributes with a lower case letter, all the
>   current GDB attributes start with a lower case letter, and I suspect
>   all future attributes would also start with a lower case letter, and
> 
>   2. Don't start user attributes with a double underscore, this risks
>   conflicting with Python built in attributes (e.g. __dict__) - though
>   clearly the user would need to start and end with a double
>   underscore, but it seemed easier just to say no double underscores.
> 
> I'm doing this as a separate commit as I've updated the docs for the
> existing gdb.Objfile and gdb.Progspace so they all reference a single
> paragraph on selecting attribute names.
> ---
>  gdb/doc/python.texi | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)

Thanks.

> +See @ref{choosing attribute names} for guidance on selecting a

Why not @xref?  And please add a comma after the right brace, there
are still old versions of Texinfo that insist on that.

> +See @ref{choosing attribute names} for guidance on selecting a
> +suitable name for new attributes.

Likewise.

> +See @ref{choosing attribute names} for guidance on selecting a
> +suitable name for new attributes.

And likewise.

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

Re: [PATCHv2 8/8] gdb/doc: update examples in gdb.Progspace and gdb.Objfile docs

[Eli Zaretskii]

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: Andrew Burgess <aburgess@redhat.com>
> Date: Wed, 10 Jan 2024 15:54:45 +0000
> 
> This commit updates the Python example code in the gdb.Progspace and
> gdb.Objfile sections of the docs.  Changes made:
> 
>   1. Use @value{GDBP} for the GDB prompt rather than
>   hard-coding (gdB),
> 
>   2. Use @group...@end group to split the example code into
>   unbreakable chunks, and
> 
>   3. Add parenthesis to the Python print() calls in the examples.  In
>   Python 2 it was OK to drop the parenthesis, but now GDB is Python 3
>   only, example code should include the parenthesis.
> ---
>  gdb/doc/python.texi | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)

This is okay, thanks.

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

Re: [PATCHv2 0/8] Python __repr__() methods and new __dict__ attributes

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> In V2:
Andrew>   - Fixed problems in thpy_dealloc that Tom pointed out,

Andrew>   - Used @group...@end group in the docs as suggested by Eli,

Andrew>   - Added patches #7 and #8, which are both docs only patches.  The
Andrew>     first adds advice for naming attributes as Tom suggested, the
Andrew>     second is just some doc updates that I noticed in passing.

Thanks, this looks good & addresses my comments.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCHv2 7/8] gdb/doc: add some notes on selecting suitable attribute names

[Andrew Burgess]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Andrew Burgess <aburgess@redhat.com>
>> Cc: Andrew Burgess <aburgess@redhat.com>
>> Date: Wed, 10 Jan 2024 15:54:44 +0000
>> 
>> In previous commits I've added Object.__dict__ support to gdb.Inferior
>> and gdb.InferiorThread, this is similar to the existing support for
>> gdb.Objfile and gdb.Progspace.
>> 
>> This commit extends the documentation to offer the user some guidance
>> on selecting good names for their custom attributes so they
>> can (hopefully) avoid conflicting with any future attributes that GDB
>> might add.
>> 
>> The rules I've proposed are:
>> 
>>   1. Don't start user attributes with a lower case letter, all the
>>   current GDB attributes start with a lower case letter, and I suspect
>>   all future attributes would also start with a lower case letter, and
>> 
>>   2. Don't start user attributes with a double underscore, this risks
>>   conflicting with Python built in attributes (e.g. __dict__) - though
>>   clearly the user would need to start and end with a double
>>   underscore, but it seemed easier just to say no double underscores.
>> 
>> I'm doing this as a separate commit as I've updated the docs for the
>> existing gdb.Objfile and gdb.Progspace so they all reference a single
>> paragraph on selecting attribute names.
>> ---
>>  gdb/doc/python.texi | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>
> Thanks.
>
>> +See @ref{choosing attribute names} for guidance on selecting a
>
> Why not @xref?  And please add a comma after the right brace, there
> are still old versions of Texinfo that insist on that.

We discussed this once before:

  https://inbox.sourceware.org/gdb-patches/83y1ib6j5x.fsf@gnu.org

TLDR: @xref doesn't add a 'See' prefix in info output.  Emacs works
around this when displaying an info output apparently.

But... I just don't care enough for such a detail, so I've switched to
@xref and added the missing comma.  I'll _try_ to remember to use @xref
in future.  Apologies when I inevitably forget -- but at least you know
why I keep reverting back now!

Updated patch is below.

Thanks,
Andrew

>
>> +See @ref{choosing attribute names} for guidance on selecting a
>> +suitable name for new attributes.
>
> Likewise.
>
>> +See @ref{choosing attribute names} for guidance on selecting a
>> +suitable name for new attributes.
>
> And likewise.
>
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>


---

commit 5c78b7b7f1cb9476c168f904b11883df29557a0b
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Wed Jan 10 15:42:55 2024 +0000

    gdb/doc: add some notes on selecting suitable attribute names
    
    In previous commits I've added Object.__dict__ support to gdb.Inferior
    and gdb.InferiorThread, this is similar to the existing support for
    gdb.Objfile and gdb.Progspace.
    
    This commit extends the documentation to offer the user some guidance
    on selecting good names for their custom attributes so they
    can (hopefully) avoid conflicting with any future attributes that GDB
    might add.
    
    The rules I've proposed are:
    
      1. Don't start user attributes with a lower case letter, all the
      current GDB attributes start with a lower case letter, and I suspect
      all future attributes would also start with a lower case letter, and
    
      2. Don't start user attributes with a double underscore, this risks
      conflicting with Python built in attributes (e.g. __dict__) - though
      clearly the user would need to start and end with a double
      underscore, but it seemed easier just to say no double underscores.
    
    I'm doing this as a separate commit as I've updated the docs for the
    existing gdb.Objfile and gdb.Progspace so they all reference a single
    paragraph on selecting attribute names.
    
    Reviewed-By: Eli Zaretskii <eliz@gnu.org>

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 2eed332fe59..71ae012ee1c 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -3671,6 +3671,13 @@
 usual Python way.  This is useful if, for example, one needs to do
 some extra record keeping associated with the inferior.
 
+@anchor{choosing attribute names}
+When selecting a name for a new attribute, avoid starting the new
+attribute name with a lower case letter; future attributes added by
+@value{GDBN} will start with a lower case letter.  Additionally, avoid
+starting attribute names with two underscore characters, as these
+could clash with Python builtin attribute names.
+
 In this contrived example we record the time when an inferior last
 stopped:
 
@@ -4181,6 +4188,9 @@
 in the usual Python way.  This is useful if, for example, one needs to
 do some extra record keeping associated with the thread.
 
+@xref{choosing attribute names}, for guidance on selecting a suitable
+name for new attributes.
+
 In this contrived example we record the time when a thread last
 stopped:
 
@@ -5396,6 +5406,9 @@
 This is useful if, for example, one needs to do some extra record keeping
 associated with the program space.
 
+@xref{choosing attribute names}, for guidance on selecting a suitable
+name for new attributes.
+
 In this contrived example, we want to perform some processing when
 an objfile with a certain symbol is loaded, but we only want to do
 this once because it is expensive.  To achieve this we record the results
@@ -5556,6 +5569,9 @@
 This is useful if, for example, one needs to do some extra record keeping
 associated with the objfile.
 
+@xref{choosing attribute names}, for guidance on selecting a suitable
+name for new attributes.
+
 In this contrived example we record the time when @value{GDBN}
 loaded the objfile.
 

Re: [PATCHv2 7/8] gdb/doc: add some notes on selecting suitable attribute names

[Eli Zaretskii]

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: gdb-patches@sourceware.org
> Date: Thu, 11 Jan 2024 10:48:43 +0000
> 
> But... I just don't care enough for such a detail, so I've switched to
> @xref and added the missing comma.  I'll _try_ to remember to use @xref
> in future.  Apologies when I inevitably forget -- but at least you know
> why I keep reverting back now!
> 
> Updated patch is below.

Thanks.  As these are mechanical changes, the result must be okay.

[PUSHED] gdb/python: New InferiorThread.ptid_string attribute

[Andrew Burgess]

I realised that I've been traiding emails between myself and Tom
fixing the last few nits in this patch, and somewhere along the line I
managed to drop gdb-patches from the CC list.

Here's the version I've now pushed.  It's not massively different from
the original.

Thanks,
Andrew

---

This commit adds a new InferiorThread.ptid_string attribute.  This
read-only attribute contains the string returned by target_pid_to_str,
which actually converts a ptid (not pid) to a string.

This is the string that appears (at least in part) in the output of
'info threads' in the 'Target Id' column, but also in the thread
exited message that GDB prints.

Having access to this string from Python is useful for allowing
extensions identify threads in a similar way to how GDB core would
identify the thread.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
---
 gdb/NEWS                                  |  4 ++++
 gdb/doc/python.texi                       |  7 ++++++
 gdb/python/py-infthread.c                 | 27 +++++++++++++++++++++++
 gdb/testsuite/gdb.python/py-infthread.exp |  8 +++++++
 4 files changed, 46 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index 96a27be0fb2..76bc131e9c9 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -91,6 +91,10 @@ show remote thread-options-packet
   ** New function gdb.interrupt(), that interrupts GDB as if the user
      typed control-c.
 
+  ** New gdb.InferiorThread.ptid_string attribute.  This read-only
+     attribute contains the string that appears in the 'Target Id'
+     column of the 'info threads' command output.
+
 * Debugger Adapter Protocol changes
 
   ** GDB now emits the "process" event.
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index d74defeec0c..da37348d663 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -4084,6 +4084,13 @@
 does not  use that identifier.
 @end defvar
 
+@defvar InferiorThread.ptid_string
+This read-only attribute contains a string representing
+@code{InferiorThread.ptid}.  This is the string that @value{GDBN} uses
+in the @samp{Target Id} column in the @kbd{info threads} output
+(@pxref{info_threads,,@samp{info threads}}).
+@end defvar
+
 @defvar InferiorThread.inferior
 The inferior this thread belongs to.  This attribute is represented as
 a @code{gdb.Inferior} object.  This attribute is not writable.
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index 00d7171de64..632984d9ce9 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -185,6 +185,30 @@ thpy_get_ptid (PyObject *self, void *closure)
   return gdbpy_create_ptid_object (thread_obj->thread->ptid);
 }
 
+/* Implement gdb.InferiorThread.ptid_string attribute.  */
+
+static PyObject *
+thpy_get_ptid_string (PyObject *self, void *closure)
+{
+  thread_object *thread_obj = (thread_object *) self;
+  THPY_REQUIRE_VALID (thread_obj);
+  ptid_t ptid = thread_obj->thread->ptid;
+
+  try
+    {
+      /* Select the correct inferior before calling a target_* function.  */
+      scoped_restore_current_thread restore_thread;
+      switch_to_inferior_no_thread (thread_obj->thread->inf);
+      std::string ptid_str = target_pid_to_str (ptid);
+      return PyUnicode_FromString (ptid_str.c_str ());
+    }
+  catch (const gdb_exception &except)
+    {
+      GDB_PY_HANDLE_EXCEPTION (except);
+      return nullptr;
+    }
+}
+
 /* Getter for InferiorThread.inferior -> Inferior.  */
 
 static PyObject *
@@ -388,6 +412,9 @@ static gdb_PyGetSetDef thread_object_getset[] =
     "Global number of the thread, as assigned by GDB.", NULL },
   { "ptid", thpy_get_ptid, NULL, "ID of the thread, as assigned by the OS.",
     NULL },
+  { "ptid_string", thpy_get_ptid_string, nullptr,
+    "A string representing ptid, as used by, for example, 'info threads'.",
+    nullptr },
   { "inferior", thpy_get_inferior, NULL,
     "The Inferior object this thread belongs to.", NULL },
 
diff --git a/gdb/testsuite/gdb.python/py-infthread.exp b/gdb/testsuite/gdb.python/py-infthread.exp
index 0b10ce9ff77..3b07d84143f 100644
--- a/gdb/testsuite/gdb.python/py-infthread.exp
+++ b/gdb/testsuite/gdb.python/py-infthread.exp
@@ -62,6 +62,14 @@ gdb_test "python print ('result = %s' % t0.num)" " = 1" "test InferiorThread.num
 gdb_test "python print ('result = %s' % t0.global_num)" " = 1" "test InferiorThread.global_num"
 gdb_test "python print ('result = %s' % str (t0.ptid))" " = \\(\[0-9\]+, \[0-9\]+, \[0-9\]+\\)" "test InferiorThread.ptid"
 
+# Test the InferiorThread.ptid_string attribute.  We don't test the
+# actual string contents as they vary based on target, but we check
+# that we get back a non-empty string.
+gdb_test "python print(type(t0.ptid_string))" "<class 'str'>" \
+    "check that InferiorThread.ptid_string is a string"
+gdb_test "python print(t0.ptid_string)" ".+" \
+    "check that InferiorThread.ptid_string is non-empty"
+
 gdb_py_test_silent_cmd "python i0 = t0.inferior" "test InferiorThread.inferior" 1
 gdb_test "python print ('result = %s' % i0.num)" " = 1" "test Inferior.num"
 

base-commit: 322ffd247e4e95a0d5a1b11ad1ef91f3378e6174
-- 
2.25.4

Re: [PATCHv2 0/8] Python __repr__() methods and new __dict__ attributes

[Andrew Burgess]

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
> Andrew> In V2:
> Andrew>   - Fixed problems in thpy_dealloc that Tom pointed out,
>
> Andrew>   - Used @group...@end group in the docs as suggested by Eli,
>
> Andrew>   - Added patches #7 and #8, which are both docs only patches.  The
> Andrew>     first adds advice for naming attributes as Tom suggested, the
> Andrew>     second is just some doc updates that I noticed in passing.
>
> Thanks, this looks good & addresses my comments.
> Approved-By: Tom Tromey <tom@tromey.com>

Pushed.

Thanks,
Andrew

Re: [PATCHv2 0/8] Python __repr__() methods and new __dict__ attributes

[Tom de Vries]

On 1/12/24 14:44, Andrew Burgess wrote:
> Tom Tromey <tom@tromey.com> writes:
> 
>>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>>
>> Andrew> In V2:
>> Andrew>   - Fixed problems in thpy_dealloc that Tom pointed out,
>>
>> Andrew>   - Used @group...@end group in the docs as suggested by Eli,
>>
>> Andrew>   - Added patches #7 and #8, which are both docs only patches.  The
>> Andrew>     first adds advice for naming attributes as Tom suggested, the
>> Andrew>     second is just some doc updates that I noticed in passing.
>>
>> Thanks, this looks good & addresses my comments.
>> Approved-By: Tom Tromey <tom@tromey.com>
> 
> Pushed.
> 

Hi Andrew,

I'm seeing this new FAIL:
...
FAIL: gdb.python/py-inferior.exp: test repr of an invalid thread
...

Filed as https://sourceware.org/bugzilla/show_bug.cgi?id=31238 .

Thanks,
- Tom

Re: [PATCHv2 0/8] Python __repr__() methods and new __dict__ attributes

[Andrew Burgess]

Tom de Vries <tdevries@suse.de> writes:

> On 1/12/24 14:44, Andrew Burgess wrote:
>> Tom Tromey <tom@tromey.com> writes:
>> 
>>>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>>>
>>> Andrew> In V2:
>>> Andrew>   - Fixed problems in thpy_dealloc that Tom pointed out,
>>>
>>> Andrew>   - Used @group...@end group in the docs as suggested by Eli,
>>>
>>> Andrew>   - Added patches #7 and #8, which are both docs only patches.  The
>>> Andrew>     first adds advice for naming attributes as Tom suggested, the
>>> Andrew>     second is just some doc updates that I noticed in passing.
>>>
>>> Thanks, this looks good & addresses my comments.
>>> Approved-By: Tom Tromey <tom@tromey.com>
>> 
>> Pushed.
>> 
>
> Hi Andrew,
>
> I'm seeing this new FAIL:
> ...
> FAIL: gdb.python/py-inferior.exp: test repr of an invalid thread
> ...
>
> Filed as https://sourceware.org/bugzilla/show_bug.cgi?id=31238 .

I pushed this patch which I this should address this issue.  Let me know
if you are still seeing failures.

Thanks,
Andrew

---

commit 98138c62cd7f721af132f9b24f274332fd8bf079
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Fri Jan 12 16:08:14 2024 +0000

    gdb/testsuite: fix failure in gdb.python/py-inferior.exp
    
    After this commit:
    
      commit 1925bba80edd37c2ef90ef1d2c599dfc2fc17f72
      Date:   Thu Jan 4 10:01:24 2024 +0000
    
          gdb/python: add gdb.InferiorThread.__repr__() method
    
    failures were reported for gdb.python/py-inferior.exp.
    
    The test grabs a gdb.InferiorThread object representing an inferior
    thread, and then, later in the test, expects this Python object to
    become invalid when the inferior thread has exited.
    
    The gdb.InferiorThread object was obtained from the list returned by
    calling gdb.Inferior.threads().
    
    The mistake I made in the original commit was to assume that the order
    of the threads returned from gdb.Inferior.threads() somehow reflected
    the thread creation order.  Specifically, I was expecting the main
    thread to be first in the list, and "other" threads to appear ... not
    first.
    
    However, the gdb.Inferior.threads() function creates a list and
    populates it from a map.  The order of the threads in the returned
    list has no obvious relationship to the thread creation order, and can
    vary from host to host.
    
    On my machine the ordering was as I expected, so the test passed for
    me.  For others the ordering was not as expected, and it just happened
    that we ended up recording the gdb.InferiorThread for the main thread.
    
    As the main thread doesn't exit (until the test is over), the
    gdb.InferiorThread object never became invalid, and the test failed.
    
    Fixed in this commit by taking more care to correctly find a non-main
    thread.  I do this by recording the main thread early on (when there
    is only one inferior thread), and then finding any thread that is not
    this main thread.
    
    Then, once all of the secondary threads have exited, I know that the
    second InferiorThread object I found should now be invalid.
    
    The test still passes for me, and I believe this should fix the issue
    for everyone else too.
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31238

diff --git a/gdb/testsuite/gdb.python/py-inferior.exp b/gdb/testsuite/gdb.python/py-inferior.exp
index 04231dd893d..2968e027812 100644
--- a/gdb/testsuite/gdb.python/py-inferior.exp
+++ b/gdb/testsuite/gdb.python/py-inferior.exp
@@ -94,27 +94,33 @@ gdb_test "python print(i0._user_attr)" \
 gdb_test "python print(gdb.inferiors()\[0\]._user_attr)" \
     "123" "read back user defined attribute from gdb.inferiors"
 
+# Record the main thread, and check its __repr__ while we're at it.
+gdb_test_no_output "python main_thread = gdb.inferiors()\[0\].threads()\[0\]"
+gdb_test "python print(main_thread)" \
+    "<gdb.InferiorThread id=${decimal}\\.${decimal} target-id=\"\[^\r\n\]*\">" \
+
 # Test the number of inferior threads.
 
 gdb_breakpoint check_threads
 gdb_continue_to_breakpoint "cont to check_threads" ".*pthread_barrier_wait.*"
 gdb_test "python print (len (i0.threads ()))" "\r\n9" "test Inferior.threads 2"
 
-# Grab the last thread from the list.  This thread object will become
-# invalid when the corresponding thread exits.
-gdb_test_no_output "python last_thread = i0.threads()\[-1\]"
-gdb_test "python print(last_thread)" \
+# Grab a worker thread from the thread list.  A worker thread is the
+# first thread that is not the main thread.  The worker thread object
+# will become invalid when the corresponding thread exits.
+gdb_test_no_output "python worker_thread = next(filter(lambda thr : thr != main_thread, i0.threads()))"
+gdb_test "python print(worker_thread)" \
     "<gdb.InferiorThread id=${decimal}\\.${decimal} target-id=\"\[^\r\n\]*\">" \
     "test repr of a valid thread"
 
-# Add a user defined attribute to this thread, check the attribute can
-# be read back, and check the attribute is not present on other
-# threads.
-gdb_test_no_output "python last_thread._user_attribute = 123" \
+# Add a user defined attribute to the worker thread, check the
+# attribute can be read back, and check the attribute is not present
+# on the main thread.
+gdb_test_no_output "python worker_thread._user_attribute = 123" \
     "add user defined attribute to InferiorThread object"
-gdb_test "python print(last_thread._user_attribute)" "123" \
+gdb_test "python print(worker_thread._user_attribute)" "123" \
     "read back user defined attribute"
-gdb_test "python print(i0.threads ()\[0\]._user_attribute)" \
+gdb_test "python print(main_thread._user_attribute)" \
     [multi_line \
 	 "AttributeError: 'gdb\\.InferiorThread' object has no attribute '_user_attribute'" \
 	 "Error while executing Python code\\."] \
@@ -126,12 +132,11 @@ gdb_breakpoint [gdb_get_line_number "Break here."]
 gdb_continue_to_breakpoint "cont to Break here." ".*Break here\..*"
 
 # Check the repr() for an invalid gdb.InferiorThread object.
-gdb_test "python print(last_thread)" \
-    "<gdb.InferiorThread \\(invalid\\)>" \
+gdb_test "python print(worker_thread)" "<gdb.InferiorThread \\(invalid\\)>" \
     "test repr of an invalid thread"
 
 # Check the user defined attribute is still present on the invalid thread object.
-gdb_test "python print(last_thread._user_attribute)" "123" \
+gdb_test "python print(worker_thread._user_attribute)" "123" \
     "check user defined attribute on an invalid InferiorThread object"
 
 # Test memory read and write operations.