Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH v2 0/9] gdb: more fixes for Python limited C API support
@ 2026-03-03 16:16 Matthieu Longo
  2026-03-03 16:16 ` [PATCH v2 1/9] gdb: switch tuple object helpers to Python limited API equivalents Matthieu Longo
                   ` (8 more replies)
  0 siblings, 9 replies; 34+ messages in thread
From: Matthieu Longo @ 2026-03-03 16:16 UTC (permalink / raw)
  To: gdb-patches, Tom Tromey; +Cc: Matthieu Longo

This patch series fixes some of the issues encountered while enabling the Python limited C API in GDB.

Diff with revision 1: https://inbox.sourceware.org/gdb-patches/20260217150259.1391855-1-matthieu.longo@arm.com/
- patches 1,3,4 were already present in revision 1, and I addressed comments from Tom Tromey of revision 1.
- patch 2 is a small refactoring derived from patch 1, and independent of others patches.
- patches 5,6 allows simplification in future patches.
- patches 7,8 are follow-ups on https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=9a84753aa7f8b8939cf4eea9c7f1db4b42e171e1, and simplifies the future migration to heap-allocated types.

All changes were tested by building GDB against the unlimited API of Python 3.10, 3.11, 3.12, 3.13 and 3.14, and the limited API of Python 3.14 (no build regression), and no regressions were observed in the testsuite.

Regards,
Matthieu


Matthieu Longo (9):
  gdb: switch tuple object helpers to Python limited API equivalents
  gdb: introduce rgb_color type to simplify existing code
  gdb: switch bytes object helpers to Python limited API equivalents
  gdb: add new helpers for retrieving a type's fully qualified name
  gdb/python: allow ref_ptr<T, Policy>::new_reference to accept subclasses of T
  gdb/python: flatten functions calling PyObject_New and use gdbpy_ref
  gdb/python: accept gdbpy_ref in init helpers and return bool
  gdb/python: add gdbpy_dict_wrapper:allocate_dict helper
  gdb/python: add accessor helpers for __dict__ in Python extension objects

 gdb/Makefile.in                            |  1 +
 gdb/configure                              |  6 +-
 gdb/configure.ac                           |  6 +-
 gdb/python/py-arch.c                       |  3 +-
 gdb/python/py-block.c                      |  3 +-
 gdb/python/py-breakpoint.c                 |  9 ++-
 gdb/python/py-color.c                      | 17 +++--
 gdb/python/py-connection.c                 |  2 +-
 gdb/python/py-corefile.c                   | 55 +++++++-------
 gdb/python/py-disasm.c                     | 17 ++---
 gdb/python/py-event.c                      | 13 ++--
 gdb/python/py-frame.c                      |  4 +-
 gdb/python/py-function.c                   |  3 +-
 gdb/python/py-inferior.c                   | 56 +++++++--------
 gdb/python/py-infthread.c                  | 22 +++---
 gdb/python/py-mi.c                         |  2 +-
 gdb/python/py-micmd.c                      |  2 +-
 gdb/python/py-obj-type.c                   | 84 ++++++++++++++++++++++
 gdb/python/py-obj-type.h                   | 31 ++++++++
 gdb/python/py-objfile.c                    | 38 +++++-----
 gdb/python/py-prettyprint.c                |  5 +-
 gdb/python/py-progspace.c                  | 60 ++++++++--------
 gdb/python/py-ref.h                        | 29 +++++++-
 gdb/python/py-style.c                      | 14 ++--
 gdb/python/py-symbol.c                     | 14 ++--
 gdb/python/py-type.c                       | 25 +++----
 gdb/python/py-unwind.c                     | 16 +++--
 gdb/python/py-utils.c                      |  4 +-
 gdb/python/py-xmethods.c                   | 24 +++++--
 gdb/python/python-internal.h               |  6 +-
 gdb/python/python.c                        |  7 +-
 gdb/testsuite/gdb.python/py-disasm.exp.tcl |  8 +--
 gdb/testsuite/gdb.python/py-unwind.exp     |  2 +-
 gdb/tui/tui-io.c                           |  3 +-
 gdb/ui-style.c                             | 33 ++++-----
 gdb/ui-style.h                             | 38 +++++++++-
 gdb/unittests/style-selftests.c            | 10 +--
 gdbsupport/gdb_ref_ptr.h                   |  5 +-
 38 files changed, 426 insertions(+), 251 deletions(-)
 create mode 100644 gdb/python/py-obj-type.c
 create mode 100644 gdb/python/py-obj-type.h

-- 
2.53.0


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

* [PATCH v2 1/9] gdb: switch tuple object helpers to Python limited API equivalents
  2026-03-03 16:16 [PATCH v2 0/9] gdb: more fixes for Python limited C API support Matthieu Longo
@ 2026-03-03 16:16 ` Matthieu Longo
  2026-03-03 18:09   ` Tom Tromey
  2026-03-03 16:16 ` [PATCH v2 2/9] gdb: introduce rgb_color type to simplify existing code Matthieu Longo
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: Matthieu Longo @ 2026-03-03 16:16 UTC (permalink / raw)
  To: gdb-patches, Tom Tromey; +Cc: Matthieu Longo

* PyTuple_GET_ITEM -> PyTuple_GetItem
* PyTuple_SET_ITEM -> PyTuple_SetItem
* PyTuple_GET_SIZE -> PyTuple_Size

Unlike PyTuple_SET_ITEM(), PyTuple_SetItem() returns an integer: 0 on
success and -1 on error (e.g. IndexError). The existing code must therefore
be updated to handle this new behaviour.

Since processing now stops when PyTuple_SetItem() returns an error, some
resources (such as newly allocated tuples) must be properly deallocated in
error paths. To address this, this patch replaces the use of raw 'PyObject *'
pointers with gdbpy_ref<>, a reference-counted wrapper around 'PyObject *',
which automatically decrements the reference count on early exit.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23830
---
 gdb/python/py-color.c     |  7 ++++---
 gdb/python/py-function.c  |  3 ++-
 gdb/python/py-inferior.c  | 16 +++++++---------
 gdb/python/py-infthread.c |  7 ++++---
 gdb/python/py-symbol.c    | 11 +++++++----
 gdb/python/py-type.c      |  5 +++--
 gdb/python/py-unwind.c    |  8 +++++---
 gdb/python/py-xmethods.c  | 24 ++++++++++++++++++------
 gdb/python/python.c       |  5 +++--
 9 files changed, 53 insertions(+), 33 deletions(-)

diff --git a/gdb/python/py-color.c b/gdb/python/py-color.c
index 0bec51b1e52..24589dba265 100644
--- a/gdb/python/py-color.c
+++ b/gdb/python/py-color.c
@@ -119,14 +119,15 @@ get_attr (PyObject *obj, PyObject *attr_name)
 	    return nullptr;
 	}
 
-      PyObject *comp = PyTuple_New (3);
+      gdbpy_ref<> comp (PyTuple_New (3));
       if (comp == nullptr)
 	return nullptr;
 
       for (int i = 0; i < 3; ++i)
-	PyTuple_SET_ITEM (comp, i, rgb_objects[i].release ());
+	if (PyTuple_SetItem (comp.get (), i, rgb_objects[i].release ()) < 0)
+	  return nullptr;
 
-      return comp;
+      return comp.release ();
     }
 
   return PyObject_GenericGetAttr (obj, attr_name);
diff --git a/gdb/python/py-function.c b/gdb/python/py-function.c
index 3bb81527a9c..23e0be0ea43 100644
--- a/gdb/python/py-function.c
+++ b/gdb/python/py-function.c
@@ -48,7 +48,8 @@ convert_values_to_python (int argc, struct value **argv)
       gdbpy_ref<> elt = value_to_value_object (argv[i]);
       if (elt == NULL)
 	return NULL;
-      PyTuple_SetItem (result.get (), i, elt.release ());
+      if (PyTuple_SetItem (result.get (), i, elt.release ()) < 0)
+	return nullptr;
     }
   return result;
 }
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index ab948d28ebe..ae309620e1f 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -382,7 +382,6 @@ infpy_threads (PyObject *self, PyObject *args)
 {
   int i = 0;
   inferior_object *inf_obj = (inferior_object *) self;
-  PyObject *tuple;
 
   INFPY_REQUIRE_VALID (inf_obj);
 
@@ -395,19 +394,18 @@ infpy_threads (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  tuple = PyTuple_New (inf_obj->threads->size ());
-  if (!tuple)
-    return NULL;
+  gdbpy_ref<> tuple (PyTuple_New (inf_obj->threads->size ()));
+  if (tuple == nullptr)
+    return nullptr;
 
   for (const thread_map_t::value_type &entry : *inf_obj->threads)
     {
-      PyObject *thr = (PyObject *) entry.second.get ();
-      Py_INCREF (thr);
-      PyTuple_SET_ITEM (tuple, i, thr);
-      i = i + 1;
+      auto thr = gdbpy_ref<>::new_reference ((PyObject *) entry.second.get ());
+      if (PyTuple_SetItem (tuple.get (), i++, thr.release ()) < 0)
+	return nullptr;
     }
 
-  return tuple;
+  return tuple.release ();
 }
 
 static PyObject *
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index d75742360d4..652355990ee 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -384,9 +384,10 @@ gdbpy_create_ptid_object (ptid_t ptid)
     return nullptr;
 
   /* Note that these steal references, hence the use of 'release'.  */
-  PyTuple_SET_ITEM (ret.get (), 0, pid_obj.release ());
-  PyTuple_SET_ITEM (ret.get (), 1, lwp_obj.release ());
-  PyTuple_SET_ITEM (ret.get (), 2, tid_obj.release ());
+  if (PyTuple_SetItem (ret.get (), 0, pid_obj.release ()) < 0
+      || PyTuple_SetItem (ret.get (), 1, lwp_obj.release ()) < 0
+      || PyTuple_SetItem (ret.get (), 2, tid_obj.release ()) < 0)
+    return nullptr;
 
   return ret;
 }
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index 15fcbeea9be..fe4d6dac000 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -402,7 +402,7 @@ gdbpy_lookup_symbol (PyObject *self, PyObject *args, PyObject *kw)
   const char *name;
   static const char *keywords[] = { "name", "block", "domain", NULL };
   struct symbol *symbol = NULL;
-  PyObject *block_obj = NULL, *bool_obj;
+  PyObject *block_obj = NULL;
   const struct block *block = NULL;
 
   if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|O!i", keywords, &name,
@@ -450,10 +450,13 @@ gdbpy_lookup_symbol (PyObject *self, PyObject *args, PyObject *kw)
     }
   else
     sym_obj = gdbpy_ref<>::new_reference (Py_None);
-  PyTuple_SET_ITEM (ret_tuple.get (), 0, sym_obj.release ());
 
-  bool_obj = PyBool_FromLong (is_a_field_of_this.type != NULL);
-  PyTuple_SET_ITEM (ret_tuple.get (), 1, bool_obj);
+  if (PyTuple_SetItem (ret_tuple.get (), 0, sym_obj.release ()) < 0)
+    return nullptr;
+
+  gdbpy_ref<> bool_obj (PyBool_FromLong (is_a_field_of_this.type != NULL));
+  if (PyTuple_SetItem (ret_tuple.get (), 1, bool_obj.release ()) < 0)
+    return nullptr;
 
   return ret_tuple.release ();
 }
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index b6807801f7e..7ba77ad1d4a 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -249,8 +249,9 @@ make_fielditem (struct type *type, int i, enum gdbpy_iter_kind kind)
 	gdbpy_ref<> item (PyTuple_New (2));
 	if (item == NULL)
 	  return NULL;
-	PyTuple_SET_ITEM (item.get (), 0, key.release ());
-	PyTuple_SET_ITEM (item.get (), 1, value.release ());
+	if (PyTuple_SetItem (item.get (), 0, key.release ()) < 0
+	    || PyTuple_SetItem (item.get (), 1, value.release ()) < 0)
+	  return nullptr;
 	return item;
       }
     case iter_keys:
diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
index 09af3e7ca2c..9ffa382d093 100644
--- a/gdb/python/py-unwind.c
+++ b/gdb/python/py-unwind.c
@@ -899,11 +899,12 @@ frame_unwind_python::sniff (const frame_info_ptr &this_frame,
 
   /* Verify the return value of _execute_unwinders is a tuple of size 2.  */
   gdb_assert (PyTuple_Check (pyo_execute_ret.get ()));
-  gdb_assert (PyTuple_GET_SIZE (pyo_execute_ret.get ()) == 2);
+  gdb_assert (PyTuple_Size (pyo_execute_ret.get ()) == 2);
 
   if (pyuw_debug)
     {
-      PyObject *pyo_unwinder_name = PyTuple_GET_ITEM (pyo_execute_ret.get (), 1);
+      PyObject *pyo_unwinder_name = PyTuple_GetItem (pyo_execute_ret.get (), 1);
+      gdb_assert (pyo_unwinder_name != nullptr);
       gdb::unique_xmalloc_ptr<char> name
 	= python_string_to_host_string (pyo_unwinder_name);
 
@@ -919,7 +920,8 @@ frame_unwind_python::sniff (const frame_info_ptr &this_frame,
     }
 
   /* Received UnwindInfo, cache data.  */
-  PyObject *pyo_unwind_info = PyTuple_GET_ITEM (pyo_execute_ret.get (), 0);
+  PyObject *pyo_unwind_info = PyTuple_GetItem (pyo_execute_ret.get (), 0);
+  gdb_assert (pyo_unwind_info != nullptr);
   if (!PyObject_TypeCheck (pyo_unwind_info, &unwind_info_object_type))
     error (_("an Unwinder should return gdb.UnwindInfo, not %s."),
 	   Py_TYPE (pyo_unwind_info)->tp_name);
diff --git a/gdb/python/py-xmethods.c b/gdb/python/py-xmethods.c
index 1fafafb4d24..24c8f48e1bd 100644
--- a/gdb/python/py-xmethods.c
+++ b/gdb/python/py-xmethods.c
@@ -458,9 +458,13 @@ python_xmethod_worker::do_get_result_type (value *obj,
       return EXT_LANG_RC_ERROR;
     }
 
-  /* PyTuple_SET_ITEM steals the reference of the element, hence the
+  /* PyTuple_SetItem steals the reference of the element, hence the
      release.  */
-  PyTuple_SET_ITEM (py_arg_tuple.get (), 0, py_value_obj.release ());
+  if (PyTuple_SetItem (py_arg_tuple.get (), 0, py_value_obj.release ()) < 0)
+    {
+      gdbpy_print_stack ();
+      return EXT_LANG_RC_ERROR;
+    }
 
   for (i = 0; i < args.size (); i++)
     {
@@ -471,7 +475,12 @@ python_xmethod_worker::do_get_result_type (value *obj,
 	  gdbpy_print_stack ();
 	  return EXT_LANG_RC_ERROR;
 	}
-      PyTuple_SET_ITEM (py_arg_tuple.get (), i + 1, py_value_arg.release ());
+      if (PyTuple_SetItem (py_arg_tuple.get (), i + 1,
+			   py_value_arg.release ()) < 0)
+	{
+	  gdbpy_print_stack ();
+	  return EXT_LANG_RC_ERROR;
+	}
     }
 
   gdbpy_ref<> py_result_type
@@ -543,9 +552,10 @@ python_xmethod_worker::invoke (struct value *obj,
       error (_("Error while executing Python code."));
     }
 
-  /* PyTuple_SET_ITEM steals the reference of the element, hence the
+  /* PyTuple_SetItem steals the reference of the element, hence the
      release.  */
-  PyTuple_SET_ITEM (py_arg_tuple.get (), 0, py_value_obj.release ());
+  if (PyTuple_SetItem (py_arg_tuple.get (), 0, py_value_obj.release ()) < 0)
+    return nullptr;
 
   for (i = 0; i < args.size (); i++)
     {
@@ -557,7 +567,9 @@ python_xmethod_worker::invoke (struct value *obj,
 	  error (_("Error while executing Python code."));
 	}
 
-      PyTuple_SET_ITEM (py_arg_tuple.get (), i + 1, py_value_arg.release ());
+      if (PyTuple_SetItem (py_arg_tuple.get (), i + 1,
+			   py_value_arg.release ()) < 0)
+	return nullptr;
     }
 
   gdbpy_ref<> py_result (PyObject_CallObject (m_py_worker,
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 8739864a861..5474b8d644f 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1043,8 +1043,9 @@ gdbpy_decode_line (PyObject *self, PyObject *args)
   else
     unparsed = gdbpy_ref<>::new_reference (Py_None);
 
-  PyTuple_SetItem (return_result.get (), 0, unparsed.release ());
-  PyTuple_SetItem (return_result.get (), 1, result.release ());
+  if (PyTuple_SetItem (return_result.get (), 0, unparsed.release ()) < 0
+      || PyTuple_SetItem (return_result.get (), 1, result.release ()) < 0)
+    return nullptr;
 
   return return_result.release ();
 }
-- 
2.53.0


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

* [PATCH v2 2/9] gdb: introduce rgb_color type to simplify existing code
  2026-03-03 16:16 [PATCH v2 0/9] gdb: more fixes for Python limited C API support Matthieu Longo
  2026-03-03 16:16 ` [PATCH v2 1/9] gdb: switch tuple object helpers to Python limited API equivalents Matthieu Longo
@ 2026-03-03 16:16 ` Matthieu Longo
  2026-03-03 18:16   ` Tom Tromey
  2026-03-03 16:16 ` [PATCH v2 3/9] gdb: switch bytes object helpers to Python limited API equivalents Matthieu Longo
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: Matthieu Longo @ 2026-03-03 16:16 UTC (permalink / raw)
  To: gdb-patches, Tom Tromey; +Cc: Matthieu Longo

This patch replaces the raw uint8[3] buffer used to represent RGB values
with a more convenient wrapper, rgb_color, around std::array<uint8_t, 3>.
It also changes the return type of ui_file_style::color::get_rgb to
rgb_color instead of filling a caller-provided buffer, and updates all
callers accordingly.

This expected benefit of this change consists in:
- removing the manual size handling.
- proving accessors without using hard-coded indexes.
- making the API safer.
- simplifying call sites.

This refactoring does not introduce any functional change.
---
 gdb/python/py-color.c           | 10 ++++-----
 gdb/tui/tui-io.c                |  3 +--
 gdb/ui-style.c                  | 33 +++++++++++-----------------
 gdb/ui-style.h                  | 38 ++++++++++++++++++++++++++++++++-
 gdb/unittests/style-selftests.c | 10 ++++-----
 5 files changed, 60 insertions(+), 34 deletions(-)

diff --git a/gdb/python/py-color.c b/gdb/python/py-color.c
index 24589dba265..69c7a2b5860 100644
--- a/gdb/python/py-color.c
+++ b/gdb/python/py-color.c
@@ -108,11 +108,9 @@ get_attr (PyObject *obj, PyObject *attr_name)
   if (color.is_direct ()
       && !PyUnicode_CompareWithASCIIString (attr_name, "components"))
     {
-      uint8_t rgb[3];
-      color.get_rgb (rgb);
-
-      gdbpy_ref<> rgb_objects[3];
-      for (int i = 0; i < 3; ++i)
+      rgb_color rgb = color.get_rgb ();
+      std::array<gdbpy_ref<>, rgb.size ()> rgb_objects;
+      for (auto i = 0u; i < rgb_objects.size (); ++i)
 	{
 	  rgb_objects[i] = gdb_py_object_from_ulongest (rgb[i]);
 	  if (rgb_objects[i] == nullptr)
@@ -123,7 +121,7 @@ get_attr (PyObject *obj, PyObject *attr_name)
       if (comp == nullptr)
 	return nullptr;
 
-      for (int i = 0; i < 3; ++i)
+      for (auto i = 0u; i < rgb_objects.size (); ++i)
 	if (PyTuple_SetItem (comp.get (), i, rgb_objects[i].release ()) < 0)
 	  return nullptr;
 
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index f673fbf36f6..a9a50446e8a 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -255,8 +255,7 @@ get_color (const ui_file_style::color &color, int *result)
 	  int next = color_map.size () + 8;
 	  if (next >= COLORS)
 	    return false;
-	  uint8_t rgb[3];
-	  color.get_rgb (rgb);
+	  rgb_color rgb = color.get_rgb ();
 	  /* We store RGB as 0..255, but curses wants 0..1000.  */
 	  if (init_color (next, rgb[0] * 1000 / 255, rgb[1] * 1000 / 255,
 			  rgb[2] * 1000 / 255) == ERR)
diff --git a/gdb/ui-style.c b/gdb/ui-style.c
index 7ab466e2407..0952b89f4b7 100644
--- a/gdb/ui-style.c
+++ b/gdb/ui-style.c
@@ -166,25 +166,22 @@ ui_file_style::color::to_string () const
 
 /* See ui-style.h.  */
 
-void
-ui_file_style::color::get_rgb (uint8_t *rgb) const
+rgb_color
+ui_file_style::color::get_rgb () const
 {
+  rgb_color rgb;
   if (m_color_space == color_space::RGB_24BIT)
-    {
-      rgb[0] = m_red;
-      rgb[1] = m_green;
-      rgb[2] = m_blue;
-    }
+    rgb = rgb_color (m_red, m_green, m_blue);
   else if (m_color_space == color_space::ANSI_8COLOR
 	   && 0 <= m_value && m_value <= 7)
-    memcpy (rgb, palette_8colors[m_value], 3 * sizeof (uint8_t));
+    memcpy (rgb, palette_8colors[m_value], rgb.size_bytes ());
   else if (m_color_space == color_space::AIXTERM_16COLOR
 	   && 0 <= m_value && m_value <= 15)
-    memcpy (rgb, palette_16colors[m_value], 3 * sizeof (uint8_t));
+    memcpy (rgb, palette_16colors[m_value], rgb.size_bytes ());
   else if (m_color_space != color_space::XTERM_256COLOR)
     gdb_assert_not_reached ("get_rgb called on invalid color");
   else if (0 <= m_value && m_value <= 15)
-    memcpy (rgb, palette_16colors[m_value], 3 * sizeof (uint8_t));
+    memcpy (rgb, palette_16colors[m_value], rgb.size_bytes ());
   else if (m_value >= 16 && m_value <= 231)
     {
       int value = m_value;
@@ -202,12 +199,12 @@ ui_file_style::color::get_rgb (uint8_t *rgb) const
   else if (232 <= m_value && m_value <= 255)
     {
       uint8_t v = (m_value - 232) * 10 + 8;
-      rgb[0] = v;
-      rgb[1] = v;
-      rgb[2] = v;
+      rgb = rgb_color (v, v, v);
     }
   else
     gdb_assert_not_reached ("get_rgb called on invalid color");
+
+  return rgb;
 }
 
 /* See ui-style.h.  */
@@ -227,9 +224,7 @@ ui_file_style::color::approximate (const std::vector<color_space> &spaces) const
 
   if (target_space == color_space::RGB_24BIT)
     {
-      uint8_t rgb[3];
-      get_rgb (rgb);
-      return color (rgb[0], rgb[1], rgb[2]);
+      return color (get_rgb ());
     }
 
   int target_size = 0;
@@ -251,14 +246,12 @@ ui_file_style::color::approximate (const std::vector<color_space> &spaces) const
 
   color result = NONE;
   int best_distance = std::numeric_limits<int>::max ();
-  uint8_t rgb[3];
-  get_rgb (rgb);
+  rgb_color rgb = get_rgb ();
 
   for (int i = 0; i < target_size; ++i)
     {
-      uint8_t c_rgb[3];
       color c (target_space, i);
-      c.get_rgb (c_rgb);
+      rgb_color c_rgb = c.get_rgb ();
       int d_red = std::abs (rgb[0] - c_rgb[0]);
       int d_green = std::abs (rgb[1] - c_rgb[1]);
       int d_blue = std::abs (rgb[2] - c_rgb[2]);
diff --git a/gdb/ui-style.h b/gdb/ui-style.h
index fca9150889b..72349256034 100644
--- a/gdb/ui-style.h
+++ b/gdb/ui-style.h
@@ -54,6 +54,34 @@ extern bool color_space_safe_cast (color_space *result, long c);
 /* Get the number of colors supported by the terminal where GDB is running.  */
 extern int gdb_get_ncolors ();
 
+struct rgb_color
+{
+private:
+  std::array <uint8_t, 3> m_data;
+
+public:
+  constexpr rgb_color ()
+    : m_data {}
+  {}
+  constexpr rgb_color (uint8_t r, uint8_t g, uint8_t b)
+    : m_data {r, g, b}
+  {}
+
+  constexpr uint8_t r () const noexcept { return m_data[0]; }
+  constexpr uint8_t g () const noexcept { return m_data[1]; }
+  constexpr uint8_t b () const noexcept { return m_data[2]; }
+
+  constexpr operator uint8_t *() noexcept { return m_data.data (); }
+  constexpr size_t size () const noexcept { return m_data.size (); }
+  constexpr size_t size_bytes () const noexcept
+  { return m_data.size () * sizeof (decltype (m_data)::value_type); }
+
+  constexpr uint8_t& operator[](std::size_t idx) noexcept
+  { return m_data[idx]; }
+  constexpr const uint8_t& operator[](std::size_t idx) const noexcept
+  { return m_data[idx]; }
+};
+
 /* Styles that can be applied to a ui_file.  */
 struct ui_file_style
 {
@@ -131,6 +159,14 @@ struct ui_file_style
 	       c, range.first, range.second, static_cast<int> (cs));
     }
 
+    color (const rgb_color &rgb)
+      : m_color_space (color_space::RGB_24BIT),
+	m_red (rgb.r ()),
+	m_green (rgb.g ()),
+	m_blue (rgb.b ())
+    {
+    }
+
     color (uint8_t r, uint8_t g, uint8_t b)
       : m_color_space (color_space::RGB_24BIT),
 	m_red (r),
@@ -216,7 +252,7 @@ struct ui_file_style
     /* Fill in RGB with the red/green/blue values for this color.
        This may not be called for basic colors or for the "NONE"
        color.  */
-    void get_rgb (uint8_t *rgb) const;
+    rgb_color get_rgb () const;
 
     /* Append the ANSI terminal escape sequence for this color to STR.
        IS_FG indicates whether this is a foreground or background
diff --git a/gdb/unittests/style-selftests.c b/gdb/unittests/style-selftests.c
index f10a24d4217..9035050bd6c 100644
--- a/gdb/unittests/style-selftests.c
+++ b/gdb/unittests/style-selftests.c
@@ -31,7 +31,7 @@ run_tests ()
 {
   ui_file_style style;
   size_t n_read;
-  uint8_t rgb[3];
+  rgb_color rgb;
 
   SELF_CHECK (style.parse ("\033[m", &n_read));
   SELF_CHECK (n_read == 3);
@@ -94,10 +94,10 @@ run_tests ()
   SELF_CHECK (style.parse ("\033[38;5;112;48;5;249m", &n_read));
   SELF_CHECK (n_read == 20);
   SELF_CHECK (!style.get_foreground ().is_basic ());
-  style.get_foreground ().get_rgb (rgb);
+  rgb = style.get_foreground ().get_rgb ();
   CHECK_RGB (0x87, 0xd7, 0);
   SELF_CHECK (!style.get_background ().is_basic ());
-  style.get_background ().get_rgb (rgb);
+  rgb = style.get_background ().get_rgb ();
   CHECK_RGB (0xb2, 0xb2, 0xb2);
   SELF_CHECK (style.get_intensity () == ui_file_style::NORMAL);
   SELF_CHECK (!style.is_italic ());
@@ -109,10 +109,10 @@ run_tests ()
   SELF_CHECK (style.parse ("\033[38;2;83;84;85;48;2;0;1;254;2;7m", &n_read));
   SELF_CHECK (n_read == 33);
   SELF_CHECK (!style.get_foreground ().is_basic ());
-  style.get_foreground ().get_rgb (rgb);
+  rgb = style.get_foreground ().get_rgb ();
   CHECK_RGB (83, 84, 85);
   SELF_CHECK (!style.get_background ().is_basic ());
-  style.get_background ().get_rgb (rgb);
+  rgb = style.get_background ().get_rgb ();
   CHECK_RGB (0, 1, 254);
   SELF_CHECK (style.get_intensity () == ui_file_style::DIM);
   SELF_CHECK (!style.is_italic ());
-- 
2.53.0


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

* [PATCH v2 3/9] gdb: switch bytes object helpers to Python limited API equivalents
  2026-03-03 16:16 [PATCH v2 0/9] gdb: more fixes for Python limited C API support Matthieu Longo
  2026-03-03 16:16 ` [PATCH v2 1/9] gdb: switch tuple object helpers to Python limited API equivalents Matthieu Longo
  2026-03-03 16:16 ` [PATCH v2 2/9] gdb: introduce rgb_color type to simplify existing code Matthieu Longo
@ 2026-03-03 16:16 ` Matthieu Longo
  2026-03-03 18:03   ` Tom Tromey
  2026-03-03 16:16 ` [PATCH v2 4/9] gdb: add new helpers for retrieving a type's fully qualified name Matthieu Longo
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: Matthieu Longo @ 2026-03-03 16:16 UTC (permalink / raw)
  To: gdb-patches, Tom Tromey; +Cc: Matthieu Longo

* PyBytes_AS_STRING -> PyBytes_AsString
* PyBytes_GET_SIZE -> PyBytes_Size

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23830
---
 gdb/python/py-prettyprint.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c
index eb7ac2c048f..3dcb4fba1e4 100644
--- a/gdb/python/py-prettyprint.c
+++ b/gdb/python/py-prettyprint.c
@@ -318,8 +318,9 @@ print_string_repr (PyObject *printer, const char *hint,
 	      long length;
 	      struct type *type;
 
-	      output = PyBytes_AS_STRING (string.get ());
-	      length = PyBytes_GET_SIZE (string.get ());
+	      output = PyBytes_AsString (string.get ());
+	      gdb_assert (output != nullptr);
+	      length = PyBytes_Size (string.get ());
 	      type = builtin_type (gdbarch)->builtin_char;
 
 	      if (hint && !strcmp (hint, "string"))
-- 
2.53.0


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

* [PATCH v2 4/9] gdb: add new helpers for retrieving a type's fully qualified name
  2026-03-03 16:16 [PATCH v2 0/9] gdb: more fixes for Python limited C API support Matthieu Longo
                   ` (2 preceding siblings ...)
  2026-03-03 16:16 ` [PATCH v2 3/9] gdb: switch bytes object helpers to Python limited API equivalents Matthieu Longo
@ 2026-03-03 16:16 ` Matthieu Longo
  2026-03-03 18:59   ` Tom Tromey
  2026-03-03 16:16 ` [PATCH v2 5/9] gdb/python: allow ref_ptr<T, Policy>::new_reference to accept subclasses of T Matthieu Longo
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: Matthieu Longo @ 2026-03-03 16:16 UTC (permalink / raw)
  To: gdb-patches, Tom Tromey; +Cc: Matthieu Longo

Py_TYPE (self)->tp_name is the traditional idiomatic way to get a Python
type's fully qualified name. However, in the context of the Python
limited API, PyTypeObject is opaque, so the 'tp_name' attribute is no
longer accessible. Additionally, retrieving the type of a Python object
requires Py_TYPE, which is only available as part of the stable API
starting with Python 3.14.

This patch increases minimal Python limited API version from 3.11 to 3.14.
It also introduces two new helpers to retrieve a type's fully qualified
name: gdb_py_tp_name() and gdbpy_py_obj_tp_name(), which extract the fully
qualified name from a PyTypeObject and a PyObject, respectively. Ifdefery
allows these wrappers to select the appropriate API depending on the Python
version and whether the Python limited API is enabled. For any Python
version less than 3.13, gdb_py_tp_name() fallbacks using __qualname__
instead. However, the result may differ slightly in some cases, e.g. the
module name may be missing.

Finally, this patch adapts the existing code to use these wrappers, and
adjusts some test expectations to use the fully qualified name (or
__qualname__ for versions <= 3.13) where it was not previously used.
Note that the corner case where the module name would be missing does not
appear in the testsuite.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23830
---
 gdb/Makefile.in                            |  1 +
 gdb/configure                              |  6 +-
 gdb/configure.ac                           |  6 +-
 gdb/python/py-arch.c                       |  3 +-
 gdb/python/py-block.c                      |  3 +-
 gdb/python/py-breakpoint.c                 |  9 ++-
 gdb/python/py-connection.c                 |  2 +-
 gdb/python/py-corefile.c                   |  2 +-
 gdb/python/py-disasm.c                     | 17 ++---
 gdb/python/py-frame.c                      |  4 +-
 gdb/python/py-infthread.c                  |  2 +-
 gdb/python/py-mi.c                         |  2 +-
 gdb/python/py-micmd.c                      |  2 +-
 gdb/python/py-obj-type.c                   | 84 ++++++++++++++++++++++
 gdb/python/py-obj-type.h                   | 31 ++++++++
 gdb/python/py-style.c                      | 14 ++--
 gdb/python/py-symbol.c                     |  3 +-
 gdb/python/py-type.c                       |  3 +-
 gdb/python/py-unwind.c                     |  8 +--
 gdb/python/py-utils.c                      |  4 +-
 gdb/python/python-internal.h               |  6 +-
 gdb/python/python.c                        |  2 +-
 gdb/testsuite/gdb.python/py-disasm.exp.tcl |  8 +--
 gdb/testsuite/gdb.python/py-unwind.exp     |  2 +-
 24 files changed, 175 insertions(+), 49 deletions(-)
 create mode 100644 gdb/python/py-obj-type.c
 create mode 100644 gdb/python/py-obj-type.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 2aa95be968a..4f5966de25c 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -422,6 +422,7 @@ SUBDIR_PYTHON_SRCS = \
 	python/py-micmd.c \
 	python/py-newobjfileevent.c \
 	python/py-objfile.c \
+	python/py-obj-type.c \
 	python/py-param.c \
 	python/py-prettyprint.c \
 	python/py-progspace.c \
diff --git a/gdb/configure b/gdb/configure
index 12c54521682..9802f41f71f 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -28761,12 +28761,12 @@ fi
 
 
 if test "$enable_py_limited_api" = yes; then
-  # The minimal Python limited API version is currently set to 3.11 for the
-  # support of PyBuffer_FillInfo and PyBuffer_Release.
+  # The minimal Python limited API version is currently set to 3.14 for the
+  # support of Py_TYPE().
   # The choice of the minimal version for the Python limited API won't be frozen
   # until the end of the migration.
 
-$as_echo "#define Py_LIMITED_API 0x030b0000" >>confdefs.h
+$as_echo "#define Py_LIMITED_API 0x030e0000" >>confdefs.h
 
 fi
 
diff --git a/gdb/configure.ac b/gdb/configure.ac
index cf8078e1d89..a3343dec118 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1070,11 +1070,11 @@ AC_ARG_ENABLE([py-limited-api],
 	      [enable_py_limited_api=no])
 
 if test "$enable_py_limited_api" = yes; then
-  # The minimal Python limited API version is currently set to 3.11 for the
-  # support of PyBuffer_FillInfo and PyBuffer_Release.
+  # The minimal Python limited API version is currently set to 3.14 for the
+  # support of Py_TYPE().
   # The choice of the minimal version for the Python limited API won't be frozen
   # until the end of the migration.
-  AC_DEFINE(Py_LIMITED_API, 0x030b0000,
+  AC_DEFINE(Py_LIMITED_API, 0x030e0000,
 	    [Define if GDB should be built against the Python limited C API.])
 fi
 
diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index f40d7da1763..d859d66dc82 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -338,7 +338,8 @@ archpy_repr (PyObject *self)
 
   auto arch_info = gdbarch_bfd_arch_info (gdbarch);
   return PyUnicode_FromFormat ("<%s arch_name=%s printable_name=%s>",
-			       Py_TYPE (self)->tp_name, arch_info->arch_name,
+			       gdbpy_py_obj_tp_name (self),
+			       arch_info->arch_name,
 			       arch_info->printable_name);
 }
 
diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
index 263819e1292..602e86c3766 100644
--- a/gdb/python/py-block.c
+++ b/gdb/python/py-block.c
@@ -485,7 +485,8 @@ blpy_repr (PyObject *self)
       if (++written_symbols < len)
 	str += ", ";
     }
-  return PyUnicode_FromFormat ("<%s %s {%s}>", Py_TYPE (self)->tp_name,
+  return PyUnicode_FromFormat ("<%s %s {%s}>",
+			       gdbpy_py_obj_tp_name (self),
 			       name, str.c_str ());
 }
 
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 408d4b9d857..bdab68ec686 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -1065,9 +1065,11 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
 static PyObject *
 bppy_repr (PyObject *self)
 {
+  const char *tp_name = gdbpy_py_obj_tp_name (self);
+
   const auto bp = (struct gdbpy_breakpoint_object*) self;
   if (bp->bp == nullptr)
-    return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (self)->tp_name);
+    return PyUnicode_FromFormat ("<%s (invalid)>", tp_name);
 
   std::string str = " ";
   if (bp->bp->thread != -1)
@@ -1079,7 +1081,7 @@ bppy_repr (PyObject *self)
   str.pop_back ();
 
   return PyUnicode_FromFormat ("<%s%s number=%d hits=%d%s>",
-			       Py_TYPE (self)->tp_name,
+			       tp_name,
 			       (bp->bp->enable_state == bp_enabled
 				? "" : " disabled"), bp->bp->number,
 			       bp->bp->hit_count, str.c_str ());
@@ -1771,7 +1773,8 @@ bplocpy_repr (PyObject *py_self)
       str += fn_name;
     }
 
-  return PyUnicode_FromFormat ("<%s %s>", Py_TYPE (self)->tp_name,
+  return PyUnicode_FromFormat ("<%s %s>",
+			       gdbpy_py_obj_tp_name (py_self),
 			       str.c_str ());
 }
 
diff --git a/gdb/python/py-connection.c b/gdb/python/py-connection.c
index e2c29f6f180..c0cbfbd710e 100644
--- a/gdb/python/py-connection.c
+++ b/gdb/python/py-connection.c
@@ -201,7 +201,7 @@ connpy_repr (PyObject *obj)
     return gdb_py_invalid_object_repr (obj);
 
   return PyUnicode_FromFormat ("<%s num=%d, what=\"%s\">",
-			       Py_TYPE (obj)->tp_name,
+			       gdbpy_py_obj_tp_name (obj),
 			       target->connection_number,
 			       make_target_connection_string (target).c_str ());
 }
diff --git a/gdb/python/py-corefile.c b/gdb/python/py-corefile.c
index 88fedbd718c..762348121a7 100644
--- a/gdb/python/py-corefile.c
+++ b/gdb/python/py-corefile.c
@@ -362,7 +362,7 @@ cfpy_repr (PyObject *self)
   bfd *core_bfd = get_inferior_core_bfd (obj->inferior);
   gdb_assert (core_bfd != nullptr);
   return PyUnicode_FromFormat ("<%s inferior=%d filename='%s'>",
-			       Py_TYPE (self)->tp_name,
+			       gdbpy_py_obj_tp_name (self),
 			       obj->inferior->num,
 			       bfd_get_filename (core_bfd));
 }
diff --git a/gdb/python/py-disasm.c b/gdb/python/py-disasm.c
index 7635e45db56..1c5661dd307 100644
--- a/gdb/python/py-disasm.c
+++ b/gdb/python/py-disasm.c
@@ -304,7 +304,7 @@ disasmpy_info_repr (PyObject *self)
   const char *arch_name
     = (gdbarch_bfd_arch_info (obj->gdbarch))->printable_name;
   return PyUnicode_FromFormat ("<%s address=%s architecture=%s>",
-			       Py_TYPE (obj)->tp_name,
+			       gdbpy_py_obj_tp_name (self),
 			       core_addr_to_string_nz (obj->address),
 			       arch_name);
 }
@@ -995,7 +995,7 @@ disasmpy_result_init (PyObject *self, PyObject *args, PyObject *kwargs)
     {
       PyErr_Format (PyExc_ValueError,
 		    _("Cannot use 'string' and 'parts' when creating %s."),
-		    Py_TYPE (self)->tp_name);
+		    gdbpy_py_obj_tp_name (self));
       return -1;
     }
 
@@ -1079,7 +1079,7 @@ disasmpy_result_repr (PyObject *self)
   gdb_assert (obj->parts != nullptr);
 
   return PyUnicode_FromFormat ("<%s length=%d string=\"%U\">",
-			       Py_TYPE (obj)->tp_name,
+			       gdbpy_py_obj_tp_name (self),
 			       obj->length,
 			       disasmpy_result_str (self));
 }
@@ -1294,7 +1294,7 @@ gdbpy_print_insn (struct gdbarch *gdbarch, CORE_ADDR memaddr,
       PyErr_Format
 	(PyExc_TypeError,
 	 _("Result from Disassembler must be gdb.DisassemblerResult, not %s."),
-	 Py_TYPE (result.get ())->tp_name);
+	 gdbpy_py_obj_tp_name (result.get ()));
       gdbpy_print_stack ();
       return std::optional<int> (-1);
     }
@@ -1381,8 +1381,9 @@ disasmpy_dealloc_result (PyObject *self)
 static int
 disasmpy_part_init (PyObject *self, PyObject *args, PyObject *kwargs)
 {
-  PyErr_SetString (PyExc_RuntimeError,
-		   _("Cannot create instances of DisassemblerPart."));
+  PyErr_Format (PyExc_RuntimeError,
+		_("Cannot create instances of %s."),
+		gdbpy_py_obj_tp_name (self));
   return -1;
 }
 
@@ -1419,7 +1420,7 @@ disasmpy_text_part_repr (PyObject *self)
   gdb_assert (obj->string != nullptr);
 
   return PyUnicode_FromFormat ("<%s string='%s', style='%s'>",
-			       Py_TYPE (obj)->tp_name,
+			       gdbpy_py_obj_tp_name (self),
 			       obj->string->c_str (),
 			       get_style_name (obj->style));
 }
@@ -1462,7 +1463,7 @@ disasmpy_addr_part_repr (PyObject *self)
   disasm_addr_part_object *obj = (disasm_addr_part_object *) self;
 
   return PyUnicode_FromFormat ("<%s address='%s'>",
-			       Py_TYPE (obj)->tp_name,
+			       gdbpy_py_obj_tp_name (self),
 			       core_addr_to_string_nz (obj->address));
 }
 
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index c8548791f1f..93a3ba3e980 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -95,7 +95,7 @@ frapy_repr (PyObject *self)
 
   const frame_id &fid = frame_obj->frame_id;
   return PyUnicode_FromFormat ("<%s level=%d frame-id=%s>",
-			       Py_TYPE (self)->tp_name,
+			       gdbpy_py_obj_tp_name (self),
 			       frame_relative_level (f_info),
 			       fid.to_string ().c_str ());
 }
@@ -544,7 +544,7 @@ frapy_read_var (PyObject *self, PyObject *args, PyObject *kw)
     {
       PyErr_Format (PyExc_TypeError,
 		    _("argument 1 must be gdb.Symbol or str, not %s"),
-		    Py_TYPE (sym_obj)->tp_name);
+		    gdbpy_py_obj_tp_name (sym_obj));
       return NULL;
     }
 
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index 652355990ee..0f29e9fb7a4 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -355,7 +355,7 @@ thpy_repr (PyObject *self)
 
   thread_info *thr = thread_obj->thread;
   return PyUnicode_FromFormat ("<%s id=%s target-id=\"%s\">",
-			       Py_TYPE (self)->tp_name,
+			       gdbpy_py_obj_tp_name (self),
 			       print_full_thread_id (thr),
 			       target_pid_to_str (thr->ptid).c_str ());
 }
diff --git a/gdb/python/py-mi.c b/gdb/python/py-mi.c
index 5189bda944e..27c79d60636 100644
--- a/gdb/python/py-mi.c
+++ b/gdb/python/py-mi.c
@@ -379,7 +379,7 @@ gdbpy_notify_mi (PyObject *self, PyObject *args, PyObject *kwargs)
       PyErr_Format
 	(PyExc_ValueError,
 	 _("MI notification data must be either None or a dictionary, not %s"),
-	 Py_TYPE (data)->tp_name);
+	 gdbpy_py_obj_tp_name (data));
       return nullptr;
     }
 
diff --git a/gdb/python/py-micmd.c b/gdb/python/py-micmd.c
index 0c820751c56..dac8234f095 100644
--- a/gdb/python/py-micmd.c
+++ b/gdb/python/py-micmd.c
@@ -510,7 +510,7 @@ micmdpy_set_installed (PyObject *self, PyObject *newvalue, void *closure)
     {
       PyErr_Format (PyExc_TypeError,
 		    _("gdb.MICommand.installed must be set to a bool, not %s"),
-		    newvalue == Py_None ? "None" : Py_TYPE(newvalue)->tp_name);
+		    newvalue == Py_None ? "None" : gdbpy_py_obj_tp_name (newvalue));
       return -1;
     }
 
diff --git a/gdb/python/py-obj-type.c b/gdb/python/py-obj-type.c
new file mode 100644
index 00000000000..034ebd26b8d
--- /dev/null
+++ b/gdb/python/py-obj-type.c
@@ -0,0 +1,84 @@
+/* Helpers related to Python object type
+
+   Copyright (C) 2026 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "python-internal.h"
+#include "py-obj-type.h"
+
+/* Return the type's fully qualified name from a PyTypeObject.  */
+const char *
+gdb_py_tp_name (PyTypeObject *py_type) noexcept
+{
+#if PY_VERSION_HEX >= 0x030d0000
+  /* Note: PyType_GetFullyQualifiedName() was added in version 3.13, and is
+     part of the stable ABI since version 3.13.  */
+  PyObject *fully_qualified_name = PyType_GetFullyQualifiedName (py_type);
+  if (fully_qualified_name == nullptr)
+    return nullptr;
+
+  return PyUnicode_AsUTF8AndSize (fully_qualified_name, nullptr);
+
+#elif ! defined (Py_LIMITED_API)
+  /* For non-heap types, the fully qualified name corresponds to tp_name.  */
+  if (! (PyType_GetFlags (py_type) & Py_TPFLAGS_HEAPTYPE))
+    return py_type->tp_name;
+
+  /* In the absence of PyType_GetFullyQualifiedName(), we fallback using
+     __qualname__ instead. However, the result may differ slightly in some
+     cases, e.g. the module name may be missing.  */
+
+# if PY_VERSION_HEX >= 0x030b0000
+  /* Note: PyType_GetQualName() was added in version 3.11.  */
+  PyObject *qualname = PyType_GetQualName (py_type);
+  if (qualname == nullptr)
+    return nullptr;
+
+  return PyUnicode_AsUTF8AndSize (qualname, nullptr);
+
+# else
+  /* In the absence of PyType_GetQualName(), fallback on using PyHeapTypeObject
+     which is not part of the public API.
+     Tested on 3.10 which is the oldest supported version at the time of this
+     writing, i.e. February 2026.  Hopefully, this workaround should go away
+     when the minimum supported Python version is increased above 3.10.  */
+  PyHeapTypeObject *ht = (PyHeapTypeObject *) py_type;
+  if (ht->ht_qualname == nullptr)
+    return nullptr;
+
+  return PyUnicode_AsUTF8AndSize (ht->ht_qualname, nullptr);
+# endif
+
+#else
+  #error "The version of Python you are using does not expose " \
+	 "PyType_GetFullyQualifiedName() as a part of the limited C API."
+#endif
+}
+
+/* Return the type's fully qualified name from a PyObject.  */
+const char *
+gdbpy_py_obj_tp_name (PyObject *self) noexcept
+{
+  /* Note: Py_TYPE () is part of the stable ABI since version 3.14.  */
+#if ! defined (Py_LIMITED_API) \
+    || Py_LIMITED_API >= 0x030e0000
+  return gdb_py_tp_name (Py_TYPE (self));
+#else
+  #error "The version of Python you are using does not expose Py_TYPE() "\
+	 "as a part of the limited C API."
+#endif
+}
diff --git a/gdb/python/py-obj-type.h b/gdb/python/py-obj-type.h
new file mode 100644
index 00000000000..f1b118e287a
--- /dev/null
+++ b/gdb/python/py-obj-type.h
@@ -0,0 +1,31 @@
+/* Helpers related to Python object type
+
+   Copyright (C) 2026 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDB_PYTHON_PY_OBJ_TYPE_H
+#define GDB_PYTHON_PY_OBJ_TYPE_H
+
+/* Return the type's fully qualified name from a PyTypeObject.  */
+extern const char *
+gdb_py_tp_name (PyTypeObject *py_type) noexcept;
+
+/* Return the type's fully qualified name from a PyObject.  */
+extern const char *
+gdbpy_py_obj_tp_name (PyObject *self) noexcept;
+
+#endif /* GDB_PYTHON_PY_OBJ_TYPE_H */
diff --git a/gdb/python/py-style.c b/gdb/python/py-style.c
index aa6eccaadbe..e0b40fe6844 100644
--- a/gdb/python/py-style.c
+++ b/gdb/python/py-style.c
@@ -268,7 +268,7 @@ stylepy_init_from_parts (PyObject *self, PyObject *fg, PyObject *bg,
       PyErr_Format
 	(PyExc_TypeError,
 	 _("'foreground' argument must be gdb.Color or None, not %s."),
-	 Py_TYPE (fg)->tp_name);
+	 gdbpy_py_obj_tp_name (fg));
       return -1;
     }
 
@@ -277,7 +277,7 @@ stylepy_init_from_parts (PyObject *self, PyObject *fg, PyObject *bg,
       PyErr_Format
 	(PyExc_TypeError,
 	 _("'background' argument must be gdb.Color or None, not %s."),
-	 Py_TYPE (bg)->tp_name);
+	 gdbpy_py_obj_tp_name (bg));
       return -1;
     }
 
@@ -484,7 +484,7 @@ stylepy_set_foreground (PyObject *self, PyObject *newvalue, void *closure)
   if (!gdbpy_is_color (newvalue))
     {
       PyErr_Format (PyExc_TypeError, _("value must be gdb.Color, not %s"),
-		    Py_TYPE (newvalue)->tp_name);
+		    gdbpy_py_obj_tp_name (newvalue));
       return -1;
     }
 
@@ -542,7 +542,7 @@ stylepy_set_background (PyObject *self, PyObject *newvalue, void *closure)
   if (!gdbpy_is_color (newvalue))
     {
       PyErr_Format (PyExc_TypeError, _("value must be gdb.Color, not %s"),
-		    Py_TYPE (newvalue)->tp_name);
+		    gdbpy_py_obj_tp_name (newvalue));
       return -1;
     }
 
@@ -624,7 +624,7 @@ stylepy_set_intensity (PyObject *self, PyObject *newvalue, void *closure)
       PyErr_Format
 	(PyExc_TypeError,
 	 _("value must be a Long (a gdb.INTENSITY constant), not %s"),
-	 Py_TYPE (newvalue)->tp_name);
+	 gdbpy_py_obj_tp_name (newvalue));
       return -1;
     }
 
@@ -734,12 +734,12 @@ stylepy_repr (PyObject *self)
 
   if (style_obj->style_name == nullptr)
     return PyUnicode_FromFormat ("<%s fg=%s, bg=%s, intensity=%s>",
-				 Py_TYPE (self)->tp_name,
+				 gdbpy_py_obj_tp_name (self),
 				 fg_str.get (), bg_str.get (),
 				 intensity_str);
   else
     return PyUnicode_FromFormat ("<%s name='%s', fg=%s, bg=%s, intensity=%s>",
-				 Py_TYPE (self)->tp_name,
+				 gdbpy_py_obj_tp_name (self),
 				 style_obj->style_name, fg_str.get (),
 				 bg_str.get (), intensity_str);
 }
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index fe4d6dac000..e74c8e4b368 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -384,7 +384,8 @@ sympy_repr (PyObject *self)
   if (symbol == nullptr)
     return gdb_py_invalid_object_repr (self);
 
-  return PyUnicode_FromFormat ("<%s print_name=%s>", Py_TYPE (self)->tp_name,
+  return PyUnicode_FromFormat ("<%s print_name=%s>",
+			       gdbpy_py_obj_tp_name (self),
 			       symbol->print_name ());
 }
 
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 7ba77ad1d4a..ec4126d0589 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -1084,7 +1084,8 @@ typy_repr (PyObject *self)
   auto py_typename = PyUnicode_Decode (type_name.c_str (), type_name.size (),
 				       host_charset (), NULL);
 
-  return PyUnicode_FromFormat ("<%s code=%s name=%U>", Py_TYPE (self)->tp_name,
+  return PyUnicode_FromFormat ("<%s code=%s name=%U>",
+			       gdbpy_py_obj_tp_name (self),
 			       code, py_typename);
 }
 
diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
index 9ffa382d093..dcf86f7db3d 100644
--- a/gdb/python/py-unwind.c
+++ b/gdb/python/py-unwind.c
@@ -247,7 +247,7 @@ unwind_infopy_repr (PyObject *self)
 
   if (frame == nullptr)
     return PyUnicode_FromFormat ("<%s for an invalid frame>",
-				 Py_TYPE (self)->tp_name);
+				 gdbpy_py_obj_tp_name (self));
 
   std::string saved_reg_names;
   struct gdbarch *gdbarch = pending_frame->gdbarch;
@@ -262,7 +262,7 @@ unwind_infopy_repr (PyObject *self)
     }
 
   return PyUnicode_FromFormat ("<%s frame #%d, saved_regs=(%s)>",
-			       Py_TYPE (self)->tp_name,
+			       gdbpy_py_obj_tp_name (self),
 			       frame_relative_level (frame),
 			       saved_reg_names.c_str ());
 }
@@ -456,7 +456,7 @@ pending_framepy_repr (PyObject *self)
     }
 
   return PyUnicode_FromFormat ("<%s level=%d, sp=%s, pc=%s>",
-			       Py_TYPE (self)->tp_name,
+			       gdbpy_py_obj_tp_name (self),
 			       frame_relative_level (frame),
 			       sp_str,
 			       pc_str);
@@ -924,7 +924,7 @@ frame_unwind_python::sniff (const frame_info_ptr &this_frame,
   gdb_assert (pyo_unwind_info != nullptr);
   if (!PyObject_TypeCheck (pyo_unwind_info, &unwind_info_object_type))
     error (_("an Unwinder should return gdb.UnwindInfo, not %s."),
-	   Py_TYPE (pyo_unwind_info)->tp_name);
+	   gdbpy_py_obj_tp_name (pyo_unwind_info));
 
   {
     unwind_info_object *unwind_info =
diff --git a/gdb/python/py-utils.c b/gdb/python/py-utils.c
index 8283b30db04..47ba1a71ef4 100644
--- a/gdb/python/py-utils.c
+++ b/gdb/python/py-utils.c
@@ -362,7 +362,7 @@ gdb_py_generic_getattro (PyObject *self, PyObject *attr)
      Therefore, we must explicitly raise an AttributeError in this case.  */
   PyErr_Format (PyExc_AttributeError,
 		"'%s' object has no attribute '%s'",
-		Py_TYPE (self)->tp_name,
+		gdbpy_py_obj_tp_name (self),
 		PyUnicode_AsUTF8AndSize (attr, nullptr));
   return nullptr;
 }
@@ -700,5 +700,5 @@ gdbpy_fix_doc_string_indentation (gdb::unique_xmalloc_ptr<char> doc)
 PyObject *
 gdb_py_invalid_object_repr (PyObject *self)
 {
-  return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (self)->tp_name);
+  return PyUnicode_FromFormat ("<%s (invalid)>", gdbpy_py_obj_tp_name (self));
 }
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index fdd353ffbeb..06b9804d01e 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -59,6 +59,7 @@
 #include <Python.h>
 #include <frameobject.h>
 #include "py-ref.h"
+#include "py-obj-type.h"
 
 static_assert (PY_VERSION_HEX >= 0x03040000);
 
@@ -1120,12 +1121,13 @@ gdbpy_type_ready (PyTypeObject *type, PyObject *mod = nullptr)
 {
   if (PyType_Ready (type) < 0)
     return -1;
+  const char *tp_name = gdb_py_tp_name (type);
   if (mod == nullptr)
     {
-      gdb_assert (startswith (type->tp_name, "gdb."));
+      gdb_assert (startswith (tp_name, "gdb."));
       mod = gdb_module;
     }
-  const char *dot = strrchr (type->tp_name, '.');
+  const char *dot = strrchr (tp_name, '.');
   gdb_assert (dot != nullptr);
   return gdb_pymodule_addobject (mod, dot + 1, (PyObject *) type);
 }
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 5474b8d644f..27cb3417c89 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1579,7 +1579,7 @@ gdbpy_write (PyObject *self, PyObject *args, PyObject *kw)
       PyErr_Format
 	(PyExc_TypeError,
 	 _("'style' argument must be gdb.Style or None, not %s."),
-	 Py_TYPE (style_obj)->tp_name);
+	 gdbpy_py_obj_tp_name (style_obj));
       return nullptr;
     }
 
diff --git a/gdb/testsuite/gdb.python/py-disasm.exp.tcl b/gdb/testsuite/gdb.python/py-disasm.exp.tcl
index 07131a44bff..e4391fa59ce 100644
--- a/gdb/testsuite/gdb.python/py-disasm.exp.tcl
+++ b/gdb/testsuite/gdb.python/py-disasm.exp.tcl
@@ -124,7 +124,7 @@ set test_plans \
 	 [list "" "${base_pattern}\r\n.*"] \
 	 [list "GlobalNullDisassembler" "${base_pattern}\r\n.*"] \
 	 [list "ShowInfoRepr" "${base_pattern}\\s+## <gdb.disassembler.DisassembleInfo address=$hex architecture=\[^>\]+>\r\n.*"] \
-	 [list "ShowInfoSubClassRepr" "${base_pattern}\\s+## <MyInfo address=$hex architecture=\[^>\]+>\r\n.*"] \
+	 [list "ShowInfoSubClassRepr" "${base_pattern}\\s+## <ShowInfoSubClassRepr.MyInfo address=$hex architecture=\[^>\]+>\r\n.*"] \
 	 [list "ShowResultRepr" "${base_pattern}\\s+## <gdb.disassembler.DisassemblerResult length=$decimal string=\"\[^\r\n\]+\">\r\n.*"] \
 	 [list "ShowResultStr" "${base_pattern}\\s+## ${nop}\r\n.*"] \
 	 [list "GlobalPreInfoDisassembler" "${base_pattern}\\s+## ad = $hex, ar = ${curr_arch}\r\n.*"] \
@@ -154,10 +154,10 @@ set test_plans \
 		   "Buffer returned from read_memory is sized $decimal instead of the expected $decimal"]] \
 	 [list "ResultOfWrongType" \
 	      [make_exception_pattern "TypeError" \
-		   "Result from Disassembler must be gdb.DisassemblerResult, not Blah."]] \
+		   "Result from Disassembler must be gdb.DisassemblerResult, not ResultOfWrongType.Blah."]] \
 	 [list "ResultOfVeryWrongType" \
 	      [make_exception_pattern "TypeError" \
-		   "Result from Disassembler must be gdb.DisassemblerResult, not Blah."]] \
+		   "Result from Disassembler must be gdb.DisassemblerResult, not ResultOfVeryWrongType.Blah."]] \
 	 [list "ErrorCreatingTextPart_NoArgs" \
 	      [make_exception_pattern "TypeError" \
 		   [missing_arg_pattern "style" 1]]] \
@@ -337,7 +337,7 @@ foreach len {0 -1} {
 foreach type {DisassemblerTextPart DisassemblerAddressPart} {
     gdb_test "python result = gdb.disassembler.${type}()" \
 	[multi_line \
-	     "RuntimeError.*: Cannot create instances of DisassemblerPart\\." \
+	     "RuntimeError.*: Cannot create instances of gdb.disassembler.${type}\\." \
 	     "Error occurred in Python.*"] \
 	 "try to create an instance of ${type}"
 }
diff --git a/gdb/testsuite/gdb.python/py-unwind.exp b/gdb/testsuite/gdb.python/py-unwind.exp
index 8c90da50a1d..784d9dfcb64 100644
--- a/gdb/testsuite/gdb.python/py-unwind.exp
+++ b/gdb/testsuite/gdb.python/py-unwind.exp
@@ -249,7 +249,7 @@ with_test_prefix "bad object unwinder" {
     gdb_test_no_output "python obj = bad_object_unwinder(\"bad-object\")"
     gdb_test_no_output "python gdb.unwinder.register_unwinder(None, obj, replace=True)"
     gdb_test "backtrace" \
-	"Python Exception <class 'gdb.error'>: an Unwinder should return gdb.UnwindInfo, not Blah\\.\r\n.*"
+	"Python Exception <class 'gdb.error'>: an Unwinder should return gdb.UnwindInfo, not bad_object_unwinder.+Blah\\.\r\n.*"
 }
 
 # Gather information about every frame.
-- 
2.53.0


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

* [PATCH v2 5/9] gdb/python: allow ref_ptr<T, Policy>::new_reference to accept subclasses of T
  2026-03-03 16:16 [PATCH v2 0/9] gdb: more fixes for Python limited C API support Matthieu Longo
                   ` (3 preceding siblings ...)
  2026-03-03 16:16 ` [PATCH v2 4/9] gdb: add new helpers for retrieving a type's fully qualified name Matthieu Longo
@ 2026-03-03 16:16 ` Matthieu Longo
  2026-03-03 18:18   ` Tom Tromey
  2026-03-03 16:16 ` [PATCH v2 6/9] gdb/python: flatten functions calling PyObject_New and use gdbpy_ref Matthieu Longo
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: Matthieu Longo @ 2026-03-03 16:16 UTC (permalink / raw)
  To: gdb-patches, Tom Tromey; +Cc: Matthieu Longo

When ref_ptr<T,Policy>::new_reference() is specialized for 'PyObject'
(i.e. gdbpy_ref<>), it currently requires the argument type to be exactly
'PyObject *'. As a result, pointers to subclasses of 'PyObject' must be
explicitly cast before being passed, making call sites unnecessarily
verbose.

This patch makes ref_ptr<T,Policy>::new_reference() a template method
that accepts both T and subclasses of T, performing the cast to 'T *'
internally when needed. This removes redundant casts at call sites
without changing behavior.
---
 gdb/python/py-inferior.c | 2 +-
 gdbsupport/gdb_ref_ptr.h | 5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index ae309620e1f..e959a107858 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -400,7 +400,7 @@ infpy_threads (PyObject *self, PyObject *args)
 
   for (const thread_map_t::value_type &entry : *inf_obj->threads)
     {
-      auto thr = gdbpy_ref<>::new_reference ((PyObject *) entry.second.get ());
+      auto thr = gdbpy_ref<>::new_reference (entry.second.get ());
       if (PyTuple_SetItem (tuple.get (), i++, thr.release ()) < 0)
 	return nullptr;
     }
diff --git a/gdbsupport/gdb_ref_ptr.h b/gdbsupport/gdb_ref_ptr.h
index 3d01bf4441b..647ac42367d 100644
--- a/gdbsupport/gdb_ref_ptr.h
+++ b/gdbsupport/gdb_ref_ptr.h
@@ -150,9 +150,12 @@ class ref_ptr
   }
 
   /* Acquire a new reference and return a ref_ptr that owns it.  */
-  static ref_ptr<T, Policy> new_reference (T *obj)
+  template <class TObj>
+  static ref_ptr<T, Policy> new_reference (TObj *obj)
   {
     Policy::incref (obj);
+    if constexpr (std::is_base_of<T, TObj>::value)
+      return ref_ptr<T, Policy> (static_cast<T *> (obj));
     return ref_ptr<T, Policy> (obj);
   }
 
-- 
2.53.0


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

* [PATCH v2 6/9] gdb/python: flatten functions calling PyObject_New and use gdbpy_ref
  2026-03-03 16:16 [PATCH v2 0/9] gdb: more fixes for Python limited C API support Matthieu Longo
                   ` (4 preceding siblings ...)
  2026-03-03 16:16 ` [PATCH v2 5/9] gdb/python: allow ref_ptr<T, Policy>::new_reference to accept subclasses of T Matthieu Longo
@ 2026-03-03 16:16 ` Matthieu Longo
  2026-03-03 18:22   ` Tom Tromey
  2026-03-03 18:22   ` Tom Tromey
  2026-03-03 16:16 ` [PATCH v2 7/9] gdb/python: accept gdbpy_ref in init helpers and return bool Matthieu Longo
                   ` (2 subsequent siblings)
  8 siblings, 2 replies; 34+ messages in thread
From: Matthieu Longo @ 2026-03-03 16:16 UTC (permalink / raw)
  To: gdb-patches, Tom Tromey; +Cc: Matthieu Longo

This patch aims at systematically using gdbpy_ref<> at all call sites
of PyObject_New(). This prepares for future patches that expect
gdbby_ref<> parameters and affect return handling.
As part of this change, flattening the affected functions so that the
return logic becomes clearer and more flexible to adjust.
---
 gdb/python/py-corefile.c  | 48 +++++++++++++++++++--------------------
 gdb/python/py-inferior.c  | 35 +++++++++++++---------------
 gdb/python/py-progspace.c | 31 +++++++++++++------------
 3 files changed, 57 insertions(+), 57 deletions(-)

diff --git a/gdb/python/py-corefile.c b/gdb/python/py-corefile.c
index 762348121a7..de88a964efb 100644
--- a/gdb/python/py-corefile.c
+++ b/gdb/python/py-corefile.c
@@ -119,33 +119,33 @@ gdbpy_core_file_from_inferior (inferior *inf)
     return gdbpy_ref<>::new_reference (Py_None);
 
   PyObject *result = (PyObject *) cfpy_inferior_corefile_data_key.get (inf);
-  if (result == nullptr)
-    {
-      gdbpy_ref<corefile_object> object
-	(PyObject_New (corefile_object, &corefile_object_type));
-      if (object == nullptr)
-	return nullptr;
+  if (result != nullptr)
+    return gdbpy_ref<>::new_reference (result);
 
-      /* Ensure the 'inferior' field is set to NULL.  If the PyDict_New
-	 call fails then the gdb.Corefile will be discarded and
-	 cfpy_dealloc will be called, which requires that the 'inferior' be
-	 set to NULL.  */
-      object->inferior = nullptr;
-      object->mapped_files = nullptr;
-      object->dict = PyDict_New ();
-      if (object->dict == nullptr)
-	return nullptr;
+  gdbpy_ref<corefile_object> object
+    (PyObject_New (corefile_object, &corefile_object_type));
+  if (object == nullptr)
+    return nullptr;
 
-      /* Now that the gdb.Corefile has been successfully initialised and we
-	 know that it is going to be passed back to the user, move it out
-	 of the invalid state by setting the 'inferior' field to a non NULL
-	 value.  */
-      object->inferior = inf;
-      cfpy_inferior_corefile_data_key.set (inf, object.get ());
-      result = (PyObject *) object.release ();
-    }
+  /* Ensure the 'inferior' field is set to NULL.  If the PyDict_New call fails
+     then the gdb.Corefile will be discarded and cfpy_dealloc will be called,
+     which requires that the 'inferior' be set to NULL.  */
+  object->inferior = nullptr;
+  object->mapped_files = nullptr;
+  object->dict = PyDict_New ();
+  if (object->dict == nullptr)
+    return nullptr;
+
+  /* Now that the gdb.Corefile has been successfully initialised and we know
+     that it is going to be passed back to the user, move it out of the invalid
+     state by setting the 'inferior' field to a non NULL value.  */
+  object->inferior = inf;
+
+  /* PyObject_New initializes the new object with a refcount of 1.  This counts
+     for the reference we are keeping in the inferior corefile data.  */
+  cfpy_inferior_corefile_data_key.set (inf, object.get ());
 
-  return gdbpy_ref<>::new_reference (result);
+  return gdbpy_ref<>::new_reference (object.release ());
 }
 
 /* Return true if OBJ is valid.  */
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index e959a107858..dbf3a536255 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -213,29 +213,26 @@ python_free_objfile (struct objfile *objfile)
 gdbpy_ref<inferior_object>
 inferior_to_inferior_object (struct inferior *inferior)
 {
-  inferior_object *inf_obj;
+  inferior_object *result = infpy_inf_data_key.get (inferior);
+  if (result != nullptr)
+    return gdbpy_ref<inferior_object>::new_reference (result);
 
-  inf_obj = infpy_inf_data_key.get (inferior);
-  if (!inf_obj)
-    {
-      inf_obj = PyObject_New (inferior_object, &inferior_object_type);
-      if (!inf_obj)
-	return NULL;
+  gdbpy_ref<inferior_object> inf_obj
+    (PyObject_New (inferior_object, &inferior_object_type));
+  if (inf_obj == nullptr)
+    return nullptr;
 
-      inf_obj->inferior = inferior;
-      inf_obj->threads = new thread_map_t ();
-      inf_obj->dict = PyDict_New ();
-      if (inf_obj->dict == nullptr)
-	return nullptr;
+  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.  */
-      infpy_inf_data_key.set (inferior, inf_obj);
-    }
+  /* PyObject_New initializes the new object with a refcount of 1.  This counts
+     for the reference we are keeping in the inferior data.  */
+  infpy_inf_data_key.set (inferior, inf_obj.get ());
 
-  /* We are returning a new reference.  */
-  gdb_assert (inf_obj != nullptr);
-  return gdbpy_ref<inferior_object>::new_reference (inf_obj);
+  return gdbpy_ref<inferior_object>::new_reference (inf_obj.release ());
 }
 
 /* Called when a new inferior is created.  Notifies any Python event
diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
index baab3be5ee6..146360d890d 100644
--- a/gdb/python/py-progspace.c
+++ b/gdb/python/py-progspace.c
@@ -585,21 +585,24 @@ gdbpy_ref<>
 pspace_to_pspace_object (struct program_space *pspace)
 {
   PyObject *result = (PyObject *) pspy_pspace_data_key.get (pspace);
-  if (result == NULL)
-    {
-      gdbpy_ref<pspace_object> object
-	((pspace_object *) PyObject_New (pspace_object, &pspace_object_type));
-      if (object == NULL)
-	return NULL;
-      if (!pspy_initialize (object.get ()))
-	return NULL;
-
-      object->pspace = pspace;
-      pspy_pspace_data_key.set (pspace, object.get ());
-      result = (PyObject *) object.release ();
-    }
+  if (result != nullptr)
+    return gdbpy_ref<>::new_reference (result);
+
+  gdbpy_ref<pspace_object> object
+    (PyObject_New (pspace_object, &pspace_object_type));
+  if (object == nullptr)
+    return nullptr;
+
+  if (! pspy_initialize (object.get ()))
+    return nullptr;
+
+  object->pspace = pspace;
+
+  /* PyObject_New initializes the new object with a refcount of 1.  This counts
+     for the reference we are keeping in the pspace data.  */
+  pspy_pspace_data_key.set (pspace, object.get ());
 
-  return gdbpy_ref<>::new_reference (result);
+  return gdbpy_ref<>::new_reference (object.release ());
 }
 
 /* See python-internal.h.  */
-- 
2.53.0


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

* [PATCH v2 7/9] gdb/python: accept gdbpy_ref in init helpers and return bool
  2026-03-03 16:16 [PATCH v2 0/9] gdb: more fixes for Python limited C API support Matthieu Longo
                   ` (5 preceding siblings ...)
  2026-03-03 16:16 ` [PATCH v2 6/9] gdb/python: flatten functions calling PyObject_New and use gdbpy_ref Matthieu Longo
@ 2026-03-03 16:16 ` Matthieu Longo
  2026-03-03 18:24   ` Tom Tromey
  2026-03-03 16:16 ` [PATCH v2 8/9] gdb/python: add gdbpy_dict_wrapper:allocate_dict helper Matthieu Longo
  2026-03-03 16:16 ` [PATCH v2 9/9] gdb/python: add accessor helpers for __dict__ in Python extension objects Matthieu Longo
  8 siblings, 1 reply; 34+ messages in thread
From: Matthieu Longo @ 2026-03-03 16:16 UTC (permalink / raw)
  To: gdb-patches, Tom Tromey; +Cc: Matthieu Longo

Passing 'gdbpy_ref<> &' instead of raw 'PyObject *' to init helpers
makes ownership of PyObject clearer at call sites, and removes
unnecessary '.get()' calls.
Changing the return type from 'int' to 'bool' improves readability
and better expresses the success/failure semantics.
---
 gdb/python/py-objfile.c   | 27 ++++++++++++---------------
 gdb/python/py-progspace.c | 22 +++++++++++-----------
 2 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/gdb/python/py-objfile.c b/gdb/python/py-objfile.c
index 5d7cfe83ec2..feae62c32a8 100644
--- a/gdb/python/py-objfile.c
+++ b/gdb/python/py-objfile.c
@@ -198,36 +198,36 @@ objfpy_dealloc (PyObject *o)
 /* Initialize an objfile_object.
    The result is a boolean indicating success.  */
 
-static int
-objfpy_initialize (objfile_object *self)
+static bool
+objfpy_initialize (gdbpy_ref<objfile_object> &self)
 {
   self->objfile = NULL;
 
   self->dict = PyDict_New ();
   if (self->dict == NULL)
-    return 0;
+    return false;
 
   self->printers = PyList_New (0);
   if (self->printers == NULL)
-    return 0;
+    return false;
 
   self->frame_filters = PyDict_New ();
   if (self->frame_filters == NULL)
-    return 0;
+    return false;
 
   self->frame_unwinders = PyList_New (0);
   if (self->frame_unwinders == NULL)
-    return 0;
+    return false;
 
   self->type_printers = PyList_New (0);
   if (self->type_printers == NULL)
-    return 0;
+    return false;
 
   self->xmethods = PyList_New (0);
   if (self->xmethods == NULL)
-    return 0;
+    return false;
 
-  return 1;
+  return true;
 }
 
 static PyObject *
@@ -235,11 +235,8 @@ objfpy_new (PyTypeObject *type, PyObject *args, PyObject *keywords)
 {
   gdbpy_ref<objfile_object> self ((objfile_object *) type->tp_alloc (type, 0));
 
-  if (self != NULL)
-    {
-      if (!objfpy_initialize (self.get ()))
-	return NULL;
-    }
+  if (self != nullptr && ! objfpy_initialize (self))
+    return nullptr;
 
   return (PyObject *) self.release ();
 }
@@ -682,7 +679,7 @@ objfile_to_objfile_object (struct objfile *objfile)
 	((objfile_object *) PyObject_New (objfile_object, &objfile_object_type));
       if (object == NULL)
 	return NULL;
-      if (!objfpy_initialize (object.get ()))
+      if (! objfpy_initialize (object))
 	return NULL;
 
       object->objfile = objfile;
diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
index 146360d890d..15f7fbead65 100644
--- a/gdb/python/py-progspace.c
+++ b/gdb/python/py-progspace.c
@@ -166,40 +166,40 @@ pspy_dealloc (PyObject *self)
 /* Initialize a pspace_object.
    The result is a boolean indicating success.  */
 
-static int
-pspy_initialize (pspace_object *self)
+static bool
+pspy_initialize (gdbpy_ref<pspace_object> &self)
 {
   self->pspace = NULL;
 
   self->dict = PyDict_New ();
   if (self->dict == NULL)
-    return 0;
+    return false;
 
   self->printers = PyList_New (0);
   if (self->printers == NULL)
-    return 0;
+    return false;
 
   self->frame_filters = PyDict_New ();
   if (self->frame_filters == NULL)
-    return 0;
+    return false;
 
   self->frame_unwinders = PyList_New (0);
   if (self->frame_unwinders == NULL)
-    return 0;
+    return false;
 
   self->type_printers = PyList_New (0);
   if (self->type_printers == NULL)
-    return 0;
+    return false;
 
   self->xmethods = PyList_New (0);
   if (self->xmethods == NULL)
-    return 0;
+    return false;
 
   self->missing_file_handlers = PyList_New (0);
   if (self->missing_file_handlers == nullptr)
-    return 0;
+    return false;
 
-  return 1;
+  return true;
 }
 
 PyObject *
@@ -593,7 +593,7 @@ pspace_to_pspace_object (struct program_space *pspace)
   if (object == nullptr)
     return nullptr;
 
-  if (! pspy_initialize (object.get ()))
+  if (! pspy_initialize (object))
     return nullptr;
 
   object->pspace = pspace;
-- 
2.53.0


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

* [PATCH v2 8/9] gdb/python: add gdbpy_dict_wrapper:allocate_dict helper
  2026-03-03 16:16 [PATCH v2 0/9] gdb: more fixes for Python limited C API support Matthieu Longo
                   ` (6 preceding siblings ...)
  2026-03-03 16:16 ` [PATCH v2 7/9] gdb/python: accept gdbpy_ref in init helpers and return bool Matthieu Longo
@ 2026-03-03 16:16 ` Matthieu Longo
  2026-03-03 18:30   ` Tom Tromey
  2026-03-03 16:16 ` [PATCH v2 9/9] gdb/python: add accessor helpers for __dict__ in Python extension objects Matthieu Longo
  8 siblings, 1 reply; 34+ messages in thread
From: Matthieu Longo @ 2026-03-03 16:16 UTC (permalink / raw)
  To: gdb-patches, Tom Tromey; +Cc: Matthieu Longo

Python extension objects that support __dict__ must inherit from
gdbpy_dict_wrapper, a wrapper class that stores the PyObject
corresponding to the __dict__ attribute.

Currently, management of this dictionary is not centralized, and
each Python extension object implements its own logic to create,
access, and destroy it.

This patch focuses on the allocation of the dictionary, introduces
a new helper function, gdbpy_dict_wrapper::allocate_dict(), and
adapts the existing code to use the helper.
---
 gdb/python/py-corefile.c  |  3 +--
 gdb/python/py-event.c     |  7 +++----
 gdb/python/py-inferior.c  |  3 +--
 gdb/python/py-infthread.c |  7 +++----
 gdb/python/py-objfile.c   |  7 +++----
 gdb/python/py-progspace.c |  3 +--
 gdb/python/py-ref.h       | 11 +++++++++++
 gdb/python/py-type.c      |  9 +++------
 8 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/gdb/python/py-corefile.c b/gdb/python/py-corefile.c
index de88a964efb..5aefd2092c8 100644
--- a/gdb/python/py-corefile.c
+++ b/gdb/python/py-corefile.c
@@ -132,8 +132,7 @@ gdbpy_core_file_from_inferior (inferior *inf)
      which requires that the 'inferior' be set to NULL.  */
   object->inferior = nullptr;
   object->mapped_files = nullptr;
-  object->dict = PyDict_New ();
-  if (object->dict == nullptr)
+  if (! gdbpy_dict_wrapper::allocate_dict (object))
     return nullptr;
 
   /* Now that the gdb.Corefile has been successfully initialised and we know
diff --git a/gdb/python/py-event.c b/gdb/python/py-event.c
index 8fb21b7fdad..47c2ba4c84d 100644
--- a/gdb/python/py-event.c
+++ b/gdb/python/py-event.c
@@ -31,11 +31,10 @@ create_event_object (PyTypeObject *py_type)
 {
   gdbpy_ref<event_object> event_obj (PyObject_New (event_object, py_type));
   if (event_obj == NULL)
-    return NULL;
+    return nullptr;
 
-  event_obj->dict = PyDict_New ();
-  if (!event_obj->dict)
-    return NULL;
+  if (! gdbpy_dict_wrapper::allocate_dict (event_obj))
+    return nullptr;
 
   return gdbpy_ref<> ((PyObject *) event_obj.release ());
 }
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index dbf3a536255..e1cd1b8f9dd 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -224,8 +224,7 @@ 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)
+  if (! gdbpy_dict_wrapper::allocate_dict (inf_obj))
     return nullptr;
 
   /* PyObject_New initializes the new object with a refcount of 1.  This counts
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index 0f29e9fb7a4..ca9a34632d6 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -41,16 +41,15 @@ create_thread_object (struct thread_info *tp)
 
   gdbpy_ref<inferior_object> inf_obj = inferior_to_inferior_object (tp->inf);
   if (inf_obj == NULL)
-    return NULL;
+    return nullptr;
 
   thread_obj.reset (PyObject_New (thread_object, &thread_object_type));
   if (thread_obj == NULL)
-    return NULL;
+    return nullptr;
 
   thread_obj->thread = tp;
   thread_obj->inf_obj = (PyObject *) inf_obj.release ();
-  thread_obj->dict = PyDict_New ();
-  if (thread_obj->dict == nullptr)
+  if (! gdbpy_dict_wrapper::allocate_dict (thread_obj))
     return nullptr;
 
   return thread_obj;
diff --git a/gdb/python/py-objfile.c b/gdb/python/py-objfile.c
index feae62c32a8..289182c794d 100644
--- a/gdb/python/py-objfile.c
+++ b/gdb/python/py-objfile.c
@@ -201,12 +201,11 @@ objfpy_dealloc (PyObject *o)
 static bool
 objfpy_initialize (gdbpy_ref<objfile_object> &self)
 {
-  self->objfile = NULL;
-
-  self->dict = PyDict_New ();
-  if (self->dict == NULL)
+  if (! gdbpy_dict_wrapper::allocate_dict (self))
     return false;
 
+  self->objfile = NULL;
+
   self->printers = PyList_New (0);
   if (self->printers == NULL)
     return false;
diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
index 15f7fbead65..c38fd740286 100644
--- a/gdb/python/py-progspace.c
+++ b/gdb/python/py-progspace.c
@@ -171,8 +171,7 @@ pspy_initialize (gdbpy_ref<pspace_object> &self)
 {
   self->pspace = NULL;
 
-  self->dict = PyDict_New ();
-  if (self->dict == NULL)
+  if (! gdbpy_dict_wrapper::allocate_dict (self))
     return false;
 
   self->printers = PyList_New (0);
diff --git a/gdb/python/py-ref.h b/gdb/python/py-ref.h
index 4ce7e29357d..f67e247cc98 100644
--- a/gdb/python/py-ref.h
+++ b/gdb/python/py-ref.h
@@ -81,6 +81,17 @@ struct gdbpy_dict_wrapper : public PyObject
     auto *wrapper = reinterpret_cast<gdbpy_dict_wrapper *> (self);
     return &wrapper->dict;
   }
+
+  template <class T>
+  static bool
+  allocate_dict (gdbpy_ref<T>& self)
+  {
+    static_assert (std::is_base_of_v<gdbpy_dict_wrapper, T>
+		   || std::is_same_v<PyObject, T>);
+    auto *obj = static_cast<gdbpy_dict_wrapper *> (self.get ());
+    obj->dict = PyDict_New ();
+    return obj->dict != nullptr;
+  }
 };
 
 #endif /* GDB_PYTHON_PY_REF_H */
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index ec4126d0589..a4b046be1ad 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -93,12 +93,9 @@ field_new (void)
   gdbpy_ref<field_object> result (PyObject_New (field_object,
 						&field_object_type));
 
-  if (result != NULL)
-    {
-      result->dict = PyDict_New ();
-      if (!result->dict)
-	return NULL;
-    }
+  if (result != nullptr && ! gdbpy_dict_wrapper::allocate_dict (result))
+    return nullptr;
+
   return (PyObject *) result.release ();
 }
 
-- 
2.53.0


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

* [PATCH v2 9/9] gdb/python: add accessor helpers for __dict__ in Python extension objects
  2026-03-03 16:16 [PATCH v2 0/9] gdb: more fixes for Python limited C API support Matthieu Longo
                   ` (7 preceding siblings ...)
  2026-03-03 16:16 ` [PATCH v2 8/9] gdb/python: add gdbpy_dict_wrapper:allocate_dict helper Matthieu Longo
@ 2026-03-03 16:16 ` Matthieu Longo
  2026-03-03 19:02   ` Tom Tromey
  8 siblings, 1 reply; 34+ messages in thread
From: Matthieu Longo @ 2026-03-03 16:16 UTC (permalink / raw)
  To: gdb-patches, Tom Tromey; +Cc: Matthieu Longo

Python extension objects that support __dict__ must inherit from
gdbpy_dict_wrapper, a wrapper class that stores the PyObject
corresponding to the __dict__ attribute. Because this dictionary
is not managed directly by Python, each extension objects must
provide appropriate accessors so that attributes stored in __dict__
are looked up correctly.

Currently, management of this dictionary is not centralized, and
each Python extension object implements its own logic to create,
access, and destroy it.

A previous patch centralized the creation logic; this patch focuses
on centralizing the access to the dictionary. It introduces two new
macros:
- gdbpy_dict_wrapper_PyGetSetDef, which defines a getter for __dict__.
- gdbpy_dict_wrapper_PyTypeObject_tps, which sets tp_getattro and
  tp_setattro in PyTypeObject to gdb_py_generic_getattro and
  gdb_py_generic_setattro, respectively. These helpers already
  centralizes attribute access for Python extension objects having
  a __dict__.
  Note: this macro will eventually be removed once Python extension
  types are migrated to heap-allocated types.
---
 gdb/python/py-corefile.c  |  6 ++----
 gdb/python/py-event.c     |  6 ++----
 gdb/python/py-inferior.c  |  6 ++----
 gdb/python/py-infthread.c |  6 ++----
 gdb/python/py-objfile.c   |  6 ++----
 gdb/python/py-progspace.c |  6 ++----
 gdb/python/py-ref.h       | 18 ++++++++++++++++--
 gdb/python/py-type.c      |  8 +++-----
 8 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/gdb/python/py-corefile.c b/gdb/python/py-corefile.c
index 5aefd2092c8..1b2c66be6c7 100644
--- a/gdb/python/py-corefile.c
+++ b/gdb/python/py-corefile.c
@@ -500,8 +500,7 @@ GDBPY_INITIALIZE_FILE (gdbpy_initialize_corefile);
 
 static gdb_PyGetSetDef corefile_getset[] =
 {
-  { "__dict__", gdb_py_generic_dict_getter, nullptr,
-    "The __dict__ for the gdb.Corefile.", nullptr },
+  gdbpy_dict_wrapper_PyGetSetDef ("corefile"),
   { "filename", cfpy_get_filename, nullptr,
     "The filename of a valid Corefile object.", nullptr },
   { nullptr }
@@ -537,8 +536,7 @@ PyTypeObject corefile_object_type =
   0,				  /*tp_hash */
   0,				  /*tp_call*/
   0,				  /*tp_str*/
-  gdb_py_generic_getattro,	  /*tp_getattro*/
-  gdb_py_generic_setattro,	  /*tp_setattro*/
+  gdbpy_dict_wrapper_PyTypeObject_tps,
   0,				  /*tp_as_buffer*/
   Py_TPFLAGS_DEFAULT,		  /*tp_flags*/
   "GDB corefile object",	  /* tp_doc */
diff --git a/gdb/python/py-event.c b/gdb/python/py-event.c
index 47c2ba4c84d..1ba1836694e 100644
--- a/gdb/python/py-event.c
+++ b/gdb/python/py-event.c
@@ -100,8 +100,7 @@ GDBPY_INITIALIZE_FILE (gdbpy_initialize_event);
 
 static gdb_PyGetSetDef event_object_getset[] =
 {
-  { "__dict__", gdb_py_generic_dict_getter, NULL,
-    "The __dict__ for this event.", NULL },
+  gdbpy_dict_wrapper_PyGetSetDef ("event"),
   { NULL }
 };
 
@@ -123,8 +122,7 @@ PyTypeObject event_object_type =
   0,                                          /* tp_hash  */
   0,                                          /* tp_call */
   0,                                          /* tp_str */
-  gdb_py_generic_getattro,                    /* tp_getattro */
-  gdb_py_generic_setattro,                    /* tp_setattro */
+  gdbpy_dict_wrapper_PyTypeObject_tps,
   0,                                          /* tp_as_buffer */
   Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,   /* tp_flags */
   "GDB event object",                         /* tp_doc */
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index e1cd1b8f9dd..1c5f0348a0c 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -1033,8 +1033,7 @@ GDBPY_INITIALIZE_FILE (gdbpy_initialize_inferior);
 
 static gdb_PyGetSetDef inferior_object_getset[] =
 {
-  { "__dict__", gdb_py_generic_dict_getter, nullptr,
-    "The __dict__ for this inferior.", nullptr },
+  gdbpy_dict_wrapper_PyGetSetDef ("inferior"),
   { "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 },
@@ -1116,8 +1115,7 @@ PyTypeObject inferior_object_type =
   0,				  /* tp_hash  */
   0,				  /* tp_call */
   0,				  /* tp_str */
-  gdb_py_generic_getattro,	  /* tp_getattro */
-  gdb_py_generic_setattro,	  /* tp_setattro */
+  gdbpy_dict_wrapper_PyTypeObject_tps,
   0,				  /* tp_as_buffer */
   Py_TPFLAGS_DEFAULT,		  /* tp_flags */
   "GDB inferior object",	  /* tp_doc */
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index ca9a34632d6..3b099f7ce53 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -415,8 +415,7 @@ GDBPY_INITIALIZE_FILE (gdbpy_initialize_thread);
 
 static gdb_PyGetSetDef thread_object_getset[] =
 {
-  { "__dict__", gdb_py_generic_dict_getter, nullptr,
-    "The __dict__ for this thread.", nullptr },
+  gdbpy_dict_wrapper_PyGetSetDef ("thread"),
   { "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,
@@ -479,8 +478,7 @@ PyTypeObject thread_object_type =
   0,				  /*tp_hash */
   0,				  /*tp_call*/
   0,				  /*tp_str*/
-  gdb_py_generic_getattro,	  /*tp_getattro*/
-  gdb_py_generic_setattro,	  /*tp_setattro*/
+  gdbpy_dict_wrapper_PyTypeObject_tps,
   0,				  /*tp_as_buffer*/
   Py_TPFLAGS_DEFAULT,		  /*tp_flags*/
   "GDB thread object",		  /* tp_doc */
diff --git a/gdb/python/py-objfile.c b/gdb/python/py-objfile.c
index 289182c794d..493df334059 100644
--- a/gdb/python/py-objfile.c
+++ b/gdb/python/py-objfile.c
@@ -725,8 +725,7 @@ Look up a static-linkage global symbol in this objfile and return it." },
 
 static gdb_PyGetSetDef objfile_getset[] =
 {
-  { "__dict__", gdb_py_generic_dict_getter, NULL,
-    "The __dict__ for this objfile.", NULL },
+  gdbpy_dict_wrapper_PyGetSetDef ("objfile"),
   { "filename", objfpy_get_filename, NULL,
     "The objfile's filename, or None.", NULL },
   { "username", objfpy_get_username, NULL,
@@ -771,8 +770,7 @@ PyTypeObject objfile_object_type =
   0,				  /*tp_hash */
   0,				  /*tp_call*/
   0,				  /*tp_str*/
-  gdb_py_generic_getattro,	  /*tp_getattro*/
-  gdb_py_generic_setattro,	  /*tp_setattro*/
+  gdbpy_dict_wrapper_PyTypeObject_tps,
   0,				  /*tp_as_buffer*/
   Py_TPFLAGS_DEFAULT,		  /*tp_flags*/
   "GDB objfile object",		  /* tp_doc */
diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
index c38fd740286..eedfe1966bb 100644
--- a/gdb/python/py-progspace.c
+++ b/gdb/python/py-progspace.c
@@ -751,8 +751,7 @@ GDBPY_INITIALIZE_FILE (gdbpy_initialize_pspace);
 
 static gdb_PyGetSetDef pspace_getset[] =
 {
-  { "__dict__", gdb_py_generic_dict_getter, NULL,
-    "The __dict__ for this progspace.", NULL },
+  gdbpy_dict_wrapper_PyGetSetDef ("progspace"),
   { "filename", pspy_get_filename, NULL,
     "The filename of the progspace's main symbol file, or None.", nullptr },
   { "symbol_file", pspy_get_symbol_file, nullptr,
@@ -814,8 +813,7 @@ PyTypeObject pspace_object_type =
   0,				  /*tp_hash */
   0,				  /*tp_call*/
   0,				  /*tp_str*/
-  gdb_py_generic_getattro,	  /*tp_getattro*/
-  gdb_py_generic_setattro,	  /*tp_setattro*/
+  gdbpy_dict_wrapper_PyTypeObject_tps,
   0,				  /*tp_as_buffer*/
   Py_TPFLAGS_DEFAULT,		  /*tp_flags*/
   "GDB progspace object",	  /* tp_doc */
diff --git a/gdb/python/py-ref.h b/gdb/python/py-ref.h
index f67e247cc98..6db5ccf4858 100644
--- a/gdb/python/py-ref.h
+++ b/gdb/python/py-ref.h
@@ -54,8 +54,7 @@ template<typename T = PyObject> using gdbpy_ref
    Access to the dict requires a custom getter defined via PyGetSetDef.
      gdb_PyGetSetDef my_object_getset[] =
      {
-       { "__dict__", gdb_py_generic_dict_getter, nullptr,
-	 "The __dict__ for this object.", nullptr },
+       gdbpy_dict_wrapper_PyGetSetDef ("object"),
        ...
        { nullptr }
      };
@@ -82,6 +81,21 @@ struct gdbpy_dict_wrapper : public PyObject
     return &wrapper->dict;
   }
 
+  #define gdbpy_dict_wrapper_PyGetSetDef(object_name)		\
+    {								\
+      "__dict__", /* name */					\
+      (getter) gdb_py_generic_dict_getter,			\
+      (setter) nullptr,						\
+      "The __dict__ for this " object_name ".", /* doc */	\
+      nullptr, /* closure */					\
+    }
+
+  #define gdbpy_dict_wrapper_PyTypeObject_tps				\
+    /*tp_getattro*/							\
+    gdb_py_generic_getattro,						\
+    /*tp_setattro*/							\
+    gdb_py_generic_setattro
+
   template <class T>
   static bool
   allocate_dict (gdbpy_ref<T>& self)
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index a4b046be1ad..c34ebabe511 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -1700,9 +1700,8 @@ PyTypeObject type_object_type =
 
 static gdb_PyGetSetDef field_object_getset[] =
 {
-  { "__dict__", gdb_py_generic_dict_getter, NULL,
-    "The __dict__ for this field.", NULL },
-  { NULL }
+  gdbpy_dict_wrapper_PyGetSetDef ("field"),
+  { nullptr }
 };
 
 PyTypeObject field_object_type =
@@ -1723,8 +1722,7 @@ PyTypeObject field_object_type =
   0,				  /*tp_hash */
   0,				  /*tp_call*/
   0,				  /*tp_str*/
-  gdb_py_generic_getattro,	  /*tp_getattro*/
-  gdb_py_generic_setattro,	  /*tp_setattro*/
+  gdbpy_dict_wrapper_PyTypeObject_tps,
   0,				  /*tp_as_buffer*/
   Py_TPFLAGS_DEFAULT,		  /*tp_flags*/
   "GDB field object",		  /* tp_doc */
-- 
2.53.0


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

* Re: [PATCH v2 3/9] gdb: switch bytes object helpers to Python limited API equivalents
  2026-03-03 16:16 ` [PATCH v2 3/9] gdb: switch bytes object helpers to Python limited API equivalents Matthieu Longo
@ 2026-03-03 18:03   ` Tom Tromey
  0 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2026-03-03 18:03 UTC (permalink / raw)
  To: Matthieu Longo; +Cc: gdb-patches, Tom Tromey

>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:

> * PyBytes_AS_STRING -> PyBytes_AsString
> * PyBytes_GET_SIZE -> PyBytes_Size

> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23830

Ok.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

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

* Re: [PATCH v2 1/9] gdb: switch tuple object helpers to Python limited API equivalents
  2026-03-03 16:16 ` [PATCH v2 1/9] gdb: switch tuple object helpers to Python limited API equivalents Matthieu Longo
@ 2026-03-03 18:09   ` Tom Tromey
  0 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2026-03-03 18:09 UTC (permalink / raw)
  To: Matthieu Longo; +Cc: gdb-patches, Tom Tromey

>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:

> * PyTuple_GET_ITEM -> PyTuple_GetItem
> * PyTuple_SET_ITEM -> PyTuple_SetItem
> * PyTuple_GET_SIZE -> PyTuple_Size

Thanks.

At first I thought:

> +  gdbpy_ref<> bool_obj (PyBool_FromLong (is_a_field_of_this.type != NULL));
> +  if (PyTuple_SetItem (ret_tuple.get (), 1, bool_obj.release ()) < 0)
> +    return nullptr;

... this was wrong, but it turns out that PyBool_FromLong is infallible.

So I think this is ok.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

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

* Re: [PATCH v2 2/9] gdb: introduce rgb_color type to simplify existing code
  2026-03-03 16:16 ` [PATCH v2 2/9] gdb: introduce rgb_color type to simplify existing code Matthieu Longo
@ 2026-03-03 18:16   ` Tom Tromey
  2026-03-04 16:30     ` Matthieu Longo
  0 siblings, 1 reply; 34+ messages in thread
From: Tom Tromey @ 2026-03-03 18:16 UTC (permalink / raw)
  To: Matthieu Longo; +Cc: gdb-patches, Tom Tromey

>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:

> This patch replaces the raw uint8[3] buffer used to represent RGB values
> with a more convenient wrapper, rgb_color, around std::array<uint8_t, 3>.
> It also changes the return type of ui_file_style::color::get_rgb to
> rgb_color instead of filling a caller-provided buffer, and updates all
> callers accordingly.

Nice, thanks.

>    else if (m_color_space == color_space::ANSI_8COLOR
>  	   && 0 <= m_value && m_value <= 7)
> -    memcpy (rgb, palette_8colors[m_value], 3 * sizeof (uint8_t));
> +    memcpy (rgb, palette_8colors[m_value], rgb.size_bytes ());
>    else if (m_color_space == color_space::AIXTERM_16COLOR
>  	   && 0 <= m_value && m_value <= 15)
> -    memcpy (rgb, palette_16colors[m_value], 3 * sizeof (uint8_t));
> +    memcpy (rgb, palette_16colors[m_value], rgb.size_bytes ());
>    else if (m_color_space != color_space::XTERM_256COLOR)
>      gdb_assert_not_reached ("get_rgb called on invalid color");
>    else if (0 <= m_value && m_value <= 15)
> -    memcpy (rgb, palette_16colors[m_value], 3 * sizeof (uint8_t));
> +    memcpy (rgb, palette_16colors[m_value], rgb.size_bytes ());

I wonder if the types of palette_8colors and palette_16colors should
change as well.  Then it seems the memcpy could be an assignment.

> diff --git a/gdb/ui-style.h b/gdb/ui-style.h
> index fca9150889b..72349256034 100644
> --- a/gdb/ui-style.h
> +++ b/gdb/ui-style.h
> @@ -54,6 +54,34 @@ extern bool color_space_safe_cast (color_space *result, long c);
>  /* Get the number of colors supported by the terminal where GDB is running.  */
>  extern int gdb_get_ncolors ();
 
> +struct rgb_color
> +{

Usually in gdb we add a comment before a struct definition that explains
what it is about.

> +  constexpr const uint8_t& operator[](std::size_t idx) const noexcept
> +  { return m_data[idx]; }

The "&" should come after the space here.
Also a space between "[]" and "()".

> +    color (const rgb_color &rgb)
> +      : m_color_space (color_space::RGB_24BIT),
> +	m_red (rgb.r ()),
> +	m_green (rgb.g ()),
> +	m_blue (rgb.b ())
> +    {
> +    }

Probably this constructor should be 'explicit'.  That's the norm in gdb
unless there's a real need for an implicit conversion.

thanks,
Tom

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

* Re: [PATCH v2 5/9] gdb/python: allow ref_ptr<T, Policy>::new_reference to accept subclasses of T
  2026-03-03 16:16 ` [PATCH v2 5/9] gdb/python: allow ref_ptr<T, Policy>::new_reference to accept subclasses of T Matthieu Longo
@ 2026-03-03 18:18   ` Tom Tromey
  2026-03-04 16:56     ` Matthieu Longo
  2026-03-06 11:37     ` Matthieu Longo
  0 siblings, 2 replies; 34+ messages in thread
From: Tom Tromey @ 2026-03-03 18:18 UTC (permalink / raw)
  To: Matthieu Longo; +Cc: gdb-patches, Tom Tromey

>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:

> When ref_ptr<T,Policy>::new_reference() is specialized for 'PyObject'
> (i.e. gdbpy_ref<>), it currently requires the argument type to be exactly
> 'PyObject *'. As a result, pointers to subclasses of 'PyObject' must be
> explicitly cast before being passed, making call sites unnecessarily
> verbose.

> This patch makes ref_ptr<T,Policy>::new_reference() a template method
> that accepts both T and subclasses of T, performing the cast to 'T *'
> internally when needed. This removes redundant casts at call sites
> without changing behavior.

See this series

https://inbox.sourceware.org/gdb-patches/20260221-python-ref-simplif-v1-0-5b06f48d3096@tromey.com/

I can land that this week if it's convenient.

Tom

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

* Re: [PATCH v2 6/9] gdb/python: flatten functions calling PyObject_New and use gdbpy_ref
  2026-03-03 16:16 ` [PATCH v2 6/9] gdb/python: flatten functions calling PyObject_New and use gdbpy_ref Matthieu Longo
@ 2026-03-03 18:22   ` Tom Tromey
  2026-03-09 11:41     ` Matthieu Longo
  2026-03-03 18:22   ` Tom Tromey
  1 sibling, 1 reply; 34+ messages in thread
From: Tom Tromey @ 2026-03-03 18:22 UTC (permalink / raw)
  To: Matthieu Longo; +Cc: gdb-patches, Tom Tromey

>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:

> This patch aims at systematically using gdbpy_ref<> at all call sites
> of PyObject_New(). This prepares for future patches that expect
> gdbby_ref<> parameters and affect return handling.
> As part of this change, flattening the affected functions so that the
> return logic becomes clearer and more flexible to adjust.

Makes sense, thank you.

> -  return gdbpy_ref<>::new_reference (result);
> +  return gdbpy_ref<>::new_reference (object.release ());

If that other series goes in the '.release' can be dropped.

Tom

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

* Re: [PATCH v2 6/9] gdb/python: flatten functions calling PyObject_New and use gdbpy_ref
  2026-03-03 16:16 ` [PATCH v2 6/9] gdb/python: flatten functions calling PyObject_New and use gdbpy_ref Matthieu Longo
  2026-03-03 18:22   ` Tom Tromey
@ 2026-03-03 18:22   ` Tom Tromey
  1 sibling, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2026-03-03 18:22 UTC (permalink / raw)
  To: Matthieu Longo; +Cc: gdb-patches, Tom Tromey

>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:

> This patch aims at systematically using gdbpy_ref<> at all call sites
> of PyObject_New(). This prepares for future patches that expect
> gdbby_ref<> parameters and affect return handling.
> As part of this change, flattening the affected functions so that the
> return logic becomes clearer and more flexible to adjust.

Forgot to add the tag.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

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

* Re: [PATCH v2 7/9] gdb/python: accept gdbpy_ref in init helpers and return bool
  2026-03-03 16:16 ` [PATCH v2 7/9] gdb/python: accept gdbpy_ref in init helpers and return bool Matthieu Longo
@ 2026-03-03 18:24   ` Tom Tromey
  2026-03-09 13:25     ` Matthieu Longo
  0 siblings, 1 reply; 34+ messages in thread
From: Tom Tromey @ 2026-03-03 18:24 UTC (permalink / raw)
  To: Matthieu Longo; +Cc: gdb-patches, Tom Tromey

>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:

> Passing 'gdbpy_ref<> &' instead of raw 'PyObject *' to init helpers
> makes ownership of PyObject clearer at call sites, and removes
> unnecessary '.get()' calls.
> Changing the return type from 'int' to 'bool' improves readability
> and better expresses the success/failure semantics.

Ok.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

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

* Re: [PATCH v2 8/9] gdb/python: add gdbpy_dict_wrapper:allocate_dict helper
  2026-03-03 16:16 ` [PATCH v2 8/9] gdb/python: add gdbpy_dict_wrapper:allocate_dict helper Matthieu Longo
@ 2026-03-03 18:30   ` Tom Tromey
  2026-03-06 12:03     ` Matthieu Longo
  0 siblings, 1 reply; 34+ messages in thread
From: Tom Tromey @ 2026-03-03 18:30 UTC (permalink / raw)
  To: Matthieu Longo; +Cc: gdb-patches, Tom Tromey

>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:

> Python extension objects that support __dict__ must inherit from
> gdbpy_dict_wrapper, a wrapper class that stores the PyObject
> corresponding to the __dict__ attribute.

> Currently, management of this dictionary is not centralized, and
> each Python extension object implements its own logic to create,
> access, and destroy it.

> This patch focuses on the allocation of the dictionary, introduces
> a new helper function, gdbpy_dict_wrapper::allocate_dict(), and
> adapts the existing code to use the helper.

> +  if (! gdbpy_dict_wrapper::allocate_dict (object))
>      return nullptr;

gdb style doesn't put a space after '!' in an expression.
There's a few instances of this.

> +
> +  template <class T>
> +  static bool
> +  allocate_dict (gdbpy_ref<T>& self)

Space before "&"

> +  {
> +    static_assert (std::is_base_of_v<gdbpy_dict_wrapper, T>
> +		   || std::is_same_v<PyObject, T>);
> +    auto *obj = static_cast<gdbpy_dict_wrapper *> (self.get ());
> +    obj->dict = PyDict_New ();
> +    return obj->dict != nullptr;

It seems to me that if this were a non-static method, then it also would
not have to be a template method, like

    bool allocate_dict ()
    {
      // maybe?   gdb_assert (dict == nullptr);
      dict = PyDict_New ();
      return dict != nullptr;
    }

The idea being that inheritance takes care of the details, so this
works:

  gdbpy_ref<field_object> blah;
  if (blah->allocate_dict ()) ...

Unless we have a case where this is needed for plain gdbpy_ref<>?
But based on an earlier patch I would think that's not possible.

Tom

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

* Re: [PATCH v2 4/9] gdb: add new helpers for retrieving a type's fully qualified name
  2026-03-03 16:16 ` [PATCH v2 4/9] gdb: add new helpers for retrieving a type's fully qualified name Matthieu Longo
@ 2026-03-03 18:59   ` Tom Tromey
  2026-03-06 17:49     ` Matthieu Longo
  0 siblings, 1 reply; 34+ messages in thread
From: Tom Tromey @ 2026-03-03 18:59 UTC (permalink / raw)
  To: Matthieu Longo; +Cc: gdb-patches, Tom Tromey

>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:

> +/* Return the type's fully qualified name from a PyTypeObject.  */
> +const char *
> +gdb_py_tp_name (PyTypeObject *py_type) noexcept
> +{
> +#if PY_VERSION_HEX >= 0x030d0000
...
> +#elif ! defined (Py_LIMITED_API)
...
> +#else
> +  #error "The version of Python you are using does not expose " \
> +	 "PyType_GetFullyQualifiedName() as a part of the limited C API."
> +#endif

I think this case can't actually be hit... can it?

My reasoning is that the very first case is taken if either the limited
API is in use (which requires 0x030e0000 as of this patch) or if Python
is new enough.

So, all other cases fall into the non-limited case.

It would be better IMO to remove it if it cannot ever happen.

> +#else
> +  #error "The version of Python you are using does not expose Py_TYPE() "\
> +	 "as a part of the limited C API."

I'd prefer that #error not be indented here.

> +++ b/gdb/python/py-obj-type.h
> @@ -0,0 +1,31 @@
> +/* Helpers related to Python object type
> +
> +/* Return the type's fully qualified name from a PyTypeObject.  */
> +extern const char *
> +gdb_py_tp_name (PyTypeObject *py_type) noexcept;
> +
> +/* Return the type's fully qualified name from a PyObject.  */
> +extern const char *
> +gdbpy_py_obj_tp_name (PyObject *self) noexcept;

Declarations in gdb don't have a newline after the return type.

thanks,
Tom

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

* Re: [PATCH v2 9/9] gdb/python: add accessor helpers for __dict__ in Python extension objects
  2026-03-03 16:16 ` [PATCH v2 9/9] gdb/python: add accessor helpers for __dict__ in Python extension objects Matthieu Longo
@ 2026-03-03 19:02   ` Tom Tromey
  2026-03-06 14:33     ` Matthieu Longo
  0 siblings, 1 reply; 34+ messages in thread
From: Tom Tromey @ 2026-03-03 19:02 UTC (permalink / raw)
  To: Matthieu Longo; +Cc: gdb-patches, Tom Tromey

>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:

> A previous patch centralized the creation logic; this patch focuses
> on centralizing the access to the dictionary. It introduces two new
> macros:

Thanks.

 
> +  #define gdbpy_dict_wrapper_PyGetSetDef(object_name)		\
> +    {								\
> +      "__dict__", /* name */					\
> +      (getter) gdb_py_generic_dict_getter,			\
> +      (setter) nullptr,						\
> +      "The __dict__ for this " object_name ".", /* doc */	\
> +      nullptr, /* closure */					\
> +    }
> +
> +  #define gdbpy_dict_wrapper_PyTypeObject_tps				\
> +    /*tp_getattro*/							\
> +    gdb_py_generic_getattro,						\
> +    /*tp_setattro*/							\
> +    gdb_py_generic_setattro

I wouldn't indent the '#define'.  But I tend to think it would be better
to just use constexpr functions here.

I'd also suggest changing the name, the "PyGetSetDef" suffix is kind of
un-gdb-ish and anyway not really needed for a function, since this will
be clear from the return type.

You can see an example of doing this in a similar-ish context here

https://sourceware.org/pipermail/gdb-patches/2026-February/225253.html

Tom

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

* Re: [PATCH v2 2/9] gdb: introduce rgb_color type to simplify existing code
  2026-03-03 18:16   ` Tom Tromey
@ 2026-03-04 16:30     ` Matthieu Longo
  0 siblings, 0 replies; 34+ messages in thread
From: Matthieu Longo @ 2026-03-04 16:30 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 03/03/2026 18:16, Tom Tromey wrote:
>>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:
> 
>> This patch replaces the raw uint8[3] buffer used to represent RGB values
>> with a more convenient wrapper, rgb_color, around std::array<uint8_t, 3>.
>> It also changes the return type of ui_file_style::color::get_rgb to
>> rgb_color instead of filling a caller-provided buffer, and updates all
>> callers accordingly.
> 
> Nice, thanks.
> 
>>     else if (m_color_space == color_space::ANSI_8COLOR
>>   	   && 0 <= m_value && m_value <= 7)
>> -    memcpy (rgb, palette_8colors[m_value], 3 * sizeof (uint8_t));
>> +    memcpy (rgb, palette_8colors[m_value], rgb.size_bytes ());
>>     else if (m_color_space == color_space::AIXTERM_16COLOR
>>   	   && 0 <= m_value && m_value <= 15)
>> -    memcpy (rgb, palette_16colors[m_value], 3 * sizeof (uint8_t));
>> +    memcpy (rgb, palette_16colors[m_value], rgb.size_bytes ());
>>     else if (m_color_space != color_space::XTERM_256COLOR)
>>       gdb_assert_not_reached ("get_rgb called on invalid color");
>>     else if (0 <= m_value && m_value <= 15)
>> -    memcpy (rgb, palette_16colors[m_value], 3 * sizeof (uint8_t));
>> +    memcpy (rgb, palette_16colors[m_value], rgb.size_bytes ());
> 
> I wonder if the types of palette_8colors and palette_16colors should
> change as well.  Then it seems the memcpy could be an assignment.
> 

Yes, it makes sense. I hadn't looked too much at the code using it.
Ok, I will fix this in the new revision.

>> diff --git a/gdb/ui-style.h b/gdb/ui-style.h
>> index fca9150889b..72349256034 100644
>> --- a/gdb/ui-style.h
>> +++ b/gdb/ui-style.h
>> @@ -54,6 +54,34 @@ extern bool color_space_safe_cast (color_space *result, long c);
>>   /* Get the number of colors supported by the terminal where GDB is running.  */
>>   extern int gdb_get_ncolors ();
>   
>> +struct rgb_color
>> +{
> 
> Usually in gdb we add a comment before a struct definition that explains
> what it is about.
> 

/* Convenient wrapper for RGB color values.  */ ?

>> +  constexpr const uint8_t& operator[](std::size_t idx) const noexcept
>> +  { return m_data[idx]; }
> 
> The "&" should come after the space here.
> Also a space between "[]" and "()".
> 

Fixed.

>> +    color (const rgb_color &rgb)
>> +      : m_color_space (color_space::RGB_24BIT),
>> +	m_red (rgb.r ()),
>> +	m_green (rgb.g ()),
>> +	m_blue (rgb.b ())
>> +    {
>> +    }
> 
> Probably this constructor should be 'explicit'.  That's the norm in gdb
> unless there's a real need for an implicit conversion.

Fixed.

> 
> thanks,
> Tom

Matthieu

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

* Re: [PATCH v2 5/9] gdb/python: allow ref_ptr<T, Policy>::new_reference to accept subclasses of T
  2026-03-03 18:18   ` Tom Tromey
@ 2026-03-04 16:56     ` Matthieu Longo
  2026-03-04 18:55       ` Tom Tromey
  2026-03-06 11:37     ` Matthieu Longo
  1 sibling, 1 reply; 34+ messages in thread
From: Matthieu Longo @ 2026-03-04 16:56 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 03/03/2026 18:18, Tom Tromey wrote:
>>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:
> 
>> When ref_ptr<T,Policy>::new_reference() is specialized for 'PyObject'
>> (i.e. gdbpy_ref<>), it currently requires the argument type to be exactly
>> 'PyObject *'. As a result, pointers to subclasses of 'PyObject' must be
>> explicitly cast before being passed, making call sites unnecessarily
>> verbose.
> 
>> This patch makes ref_ptr<T,Policy>::new_reference() a template method
>> that accepts both T and subclasses of T, performing the cast to 'T *'
>> internally when needed. This removes redundant casts at call sites
>> without changing behavior.
> 
> See this series
> 
> https://inbox.sourceware.org/gdb-patches/20260221-python-ref-simplif-v1-0-5b06f48d3096@tromey.com/
> 
> I can land that this week if it's convenient.
> 
> Tom

Yes please.
I will wait for you to merge it before republishing a new revision of my series.

Matthieu

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

* Re: [PATCH v2 5/9] gdb/python: allow ref_ptr<T, Policy>::new_reference to accept subclasses of T
  2026-03-04 16:56     ` Matthieu Longo
@ 2026-03-04 18:55       ` Tom Tromey
  0 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2026-03-04 18:55 UTC (permalink / raw)
  To: Matthieu Longo; +Cc: Tom Tromey, gdb-patches

>> See this series
>> https://inbox.sourceware.org/gdb-patches/20260221-python-ref-simplif-v1-0-5b06f48d3096@tromey.com/
>> I can land that this week if it's convenient.

> Yes please.
> I will wait for you to merge it before republishing a new revision of my series.

I've checked it in now.

Tom

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

* Re: [PATCH v2 5/9] gdb/python: allow ref_ptr<T, Policy>::new_reference to accept subclasses of T
  2026-03-03 18:18   ` Tom Tromey
  2026-03-04 16:56     ` Matthieu Longo
@ 2026-03-06 11:37     ` Matthieu Longo
  2026-03-06 11:43       ` Matthieu Longo
  2026-03-06 16:47       ` Tom Tromey
  1 sibling, 2 replies; 34+ messages in thread
From: Matthieu Longo @ 2026-03-06 11:37 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 03/03/2026 18:18, Tom Tromey wrote:
>>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:
> 
>> When ref_ptr<T,Policy>::new_reference() is specialized for 'PyObject'
>> (i.e. gdbpy_ref<>), it currently requires the argument type to be exactly
>> 'PyObject *'. As a result, pointers to subclasses of 'PyObject' must be
>> explicitly cast before being passed, making call sites unnecessarily
>> verbose.
> 
>> This patch makes ref_ptr<T,Policy>::new_reference() a template method
>> that accepts both T and subclasses of T, performing the cast to 'T *'
>> internally when needed. This removes redundant casts at call sites
>> without changing behavior.
> 
> See this series
> 
> https://inbox.sourceware.org/gdb-patches/20260221-python-ref-simplif-v1-0-5b06f48d3096@tromey.com/
> 
> I can land that this week if it's convenient.
> 
> Tom

After a deeper examination, I am not sure that your series helps much for this patch.

* Case 1: return value from the registry.

PyObject *result = (PyObject *) cfpy_inferior_corefile_data_key.get (inf);
   if (result != nullptr)
     return gdbpy_ref<>::new_reference (result);

If I had gdbpy_borrowed_ref available, I could replace the code above by something like:
gdbpy_borrowed_ref result = cfpy_inferior_corefile_data_key.get (inf);
if (result != nullptr)
   return result; /* assuming that gdbpy_ref<> would have a constructor taking gdbpy_borrowed_ref. */


* Case 2: registration of the new object in the registry and return the value.

gdbpy_ref<corefile_object> object
     (PyObject_New (corefile_object, &corefile_object_type));
...
cfpy_inferior_corefile_data_key.set (inf, object.get ());
return gdbpy_ref<>::new_reference (object.release ());

I could change the code as follows, but I am not sure that it makes the code more readable.

cfpy_inferior_corefile_data_key.set (inf, decltype (object) (object).release ());
return object;

If the registry was storing gdbpy_ref<corefile_object> instead of 'corefile_object *', I guess that it could be simplified even further to:

cfpy_inferior_corefile_data_key.set (inf, object);
return object;

What do you think ?

However, I am getting further from my original objective, which was only flattening the code for upcoming changes.
All those improvements seem to me out of scope of my original patch.

Matthieu

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

* Re: [PATCH v2 5/9] gdb/python: allow ref_ptr<T, Policy>::new_reference to accept subclasses of T
  2026-03-06 11:37     ` Matthieu Longo
@ 2026-03-06 11:43       ` Matthieu Longo
  2026-03-06 16:47       ` Tom Tromey
  1 sibling, 0 replies; 34+ messages in thread
From: Matthieu Longo @ 2026-03-06 11:43 UTC (permalink / raw)
  To: gdb-patches

On 06/03/2026 11:37, Matthieu Longo wrote:
> On 03/03/2026 18:18, Tom Tromey wrote:
>>>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:
>>
>>> When ref_ptr<T,Policy>::new_reference() is specialized for 'PyObject'
>>> (i.e. gdbpy_ref<>), it currently requires the argument type to be exactly
>>> 'PyObject *'. As a result, pointers to subclasses of 'PyObject' must be
>>> explicitly cast before being passed, making call sites unnecessarily
>>> verbose.
>>
>>> This patch makes ref_ptr<T,Policy>::new_reference() a template method
>>> that accepts both T and subclasses of T, performing the cast to 'T *'
>>> internally when needed. This removes redundant casts at call sites
>>> without changing behavior.
>>
>> See this series
>>
>> https://inbox.sourceware.org/gdb-patches/20260221-python-ref-simplif-v1-0-5b06f48d3096@tromey.com/
>>
>> I can land that this week if it's convenient.
>>
>> Tom
> 
> After a deeper examination, I am not sure that your series helps much for this patch.
> 
> * Case 1: return value from the registry.
> 
> PyObject *result = (PyObject *) cfpy_inferior_corefile_data_key.get (inf);
>    if (result != nullptr)
>      return gdbpy_ref<>::new_reference (result);
> 
> If I had gdbpy_borrowed_ref available, I could replace the code above by something like:
> gdbpy_borrowed_ref result = cfpy_inferior_corefile_data_key.get (inf);
> if (result != nullptr)
>    return result; /* assuming that gdbpy_ref<> would have a constructor taking gdbpy_borrowed_ref. */
> 
> 
> * Case 2: registration of the new object in the registry and return the value.
> 
> gdbpy_ref<corefile_object> object
>      (PyObject_New (corefile_object, &corefile_object_type));
> ...
> cfpy_inferior_corefile_data_key.set (inf, object.get ());
> return gdbpy_ref<>::new_reference (object.release ());
> 
> I could change the code as follows, but I am not sure that it makes the code more readable.
> 
> cfpy_inferior_corefile_data_key.set (inf, decltype (object) (object).release ());
> return object;
> 
> If the registry was storing gdbpy_ref<corefile_object> instead of 'corefile_object *', I guess that it could be simplified even further to:
> 
> cfpy_inferior_corefile_data_key.set (inf, object);
> return object;
> 
> What do you think ?
> 
> However, I am getting further from my original objective, which was only flattening the code for upcoming changes.
> All those improvements seem to me out of scope of my original patch.
> 
> Matthieu

I realized that my response might be unclear.
I forgot to mention that the code I am referencing here is part of the next patch.

Matthieu

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

* Re: [PATCH v2 8/9] gdb/python: add gdbpy_dict_wrapper:allocate_dict helper
  2026-03-03 18:30   ` Tom Tromey
@ 2026-03-06 12:03     ` Matthieu Longo
  0 siblings, 0 replies; 34+ messages in thread
From: Matthieu Longo @ 2026-03-06 12:03 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 03/03/2026 18:30, Tom Tromey wrote:
>>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:
> 
>> Python extension objects that support __dict__ must inherit from
>> gdbpy_dict_wrapper, a wrapper class that stores the PyObject
>> corresponding to the __dict__ attribute.
> 
>> Currently, management of this dictionary is not centralized, and
>> each Python extension object implements its own logic to create,
>> access, and destroy it.
> 
>> This patch focuses on the allocation of the dictionary, introduces
>> a new helper function, gdbpy_dict_wrapper::allocate_dict(), and
>> adapts the existing code to use the helper.
> 
>> +  if (! gdbpy_dict_wrapper::allocate_dict (object))
>>       return nullptr;
> 
> gdb style doesn't put a space after '!' in an expression.
> There's a few instances of this.
> 

Fixed.

>> +
>> +  template <class T>
>> +  static bool
>> +  allocate_dict (gdbpy_ref<T>& self)
> 
> Space before "&"
> 
>> +  {
>> +    static_assert (std::is_base_of_v<gdbpy_dict_wrapper, T>
>> +		   || std::is_same_v<PyObject, T>);
>> +    auto *obj = static_cast<gdbpy_dict_wrapper *> (self.get ());
>> +    obj->dict = PyDict_New ();
>> +    return obj->dict != nullptr;
> 
> It seems to me that if this were a non-static method, then it also would
> not have to be a template method, like
> 
>      bool allocate_dict ()
>      {
>        // maybe?   gdb_assert (dict == nullptr);
>        dict = PyDict_New ();
>        return dict != nullptr;
>      }
> 
> The idea being that inheritance takes care of the details, so this
> works:
> 
>    gdbpy_ref<field_object> blah;
>    if (blah->allocate_dict ()) ...
> 
> Unless we have a case where this is needed for plain gdbpy_ref<>?
> But based on an earlier patch I would think that's not possible.
> 
> Tom

You are right, I needlessly overcomplicated things.
There is no case where this is needed for plain gdbpy_ref<>.
The approach with the inheritance is simpler and cleaner.

Regarding the assertion "gdb_assert (dict == nullptr);", it seems that PyObject_New() does not zero the memory so the assertion can be randomly triggered by garbage. I won't keep it.

Fixed in the next revision.

Matthieu

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

* Re: [PATCH v2 9/9] gdb/python: add accessor helpers for __dict__ in Python extension objects
  2026-03-03 19:02   ` Tom Tromey
@ 2026-03-06 14:33     ` Matthieu Longo
  2026-03-06 16:04       ` Tom Tromey
  0 siblings, 1 reply; 34+ messages in thread
From: Matthieu Longo @ 2026-03-06 14:33 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 03/03/2026 19:02, Tom Tromey wrote:
>>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:
> 
>> A previous patch centralized the creation logic; this patch focuses
>> on centralizing the access to the dictionary. It introduces two new
>> macros:
> 
> Thanks.
> 
>   
>> +  #define gdbpy_dict_wrapper_PyGetSetDef(object_name)		\
>> +    {								\
>> +      "__dict__", /* name */					\
>> +      (getter) gdb_py_generic_dict_getter,			\
>> +      (setter) nullptr,						\
>> +      "The __dict__ for this " object_name ".", /* doc */	\
>> +      nullptr, /* closure */					\
>> +    }
>> +
>> +  #define gdbpy_dict_wrapper_PyTypeObject_tps				\
>> +    /*tp_getattro*/							\
>> +    gdb_py_generic_getattro,						\
>> +    /*tp_setattro*/							\
>> +    gdb_py_generic_setattro
> 
> I wouldn't indent the '#define'.  But I tend to think it would be better
> to just use constexpr functions here.
> 

For gdbpy_dict_wrapper_PyGetSetDef, I tried to use a constexpr function at first, but there is an issue.
I am not aware of an easy way to concatenate strings with a constexpr in C++17.

This code requires C++20.

   static constexpr PyGetSetDef
   configure_dict_getter (std::string_view object_name)
   {
     return {
       "__dict__", /* name */
       (getter) gdb_py_generic_dict_getter,
       (setter) nullptr,
       std::views::join (std::array {
         "The __dict__ for this "sv, object_name, "."sv
       }).data (), /* doc */
       nullptr, /* closure */
     }
   }

Please let me know if you know a way of making it compatible with C++17.

Regarding gdbpy_dict_wrapper_PyTypeObject_tps, I am not sure how to expand the array in the calling code using a constexpr function.
Please let me know if you know a solution for it.

> I'd also suggest changing the name, the "PyGetSetDef" suffix is kind of
> un-gdb-ish and anyway not really needed for a function, since this will
> be clear from the return type.
> 

If the code is convertible to C++17, configure_dict_getter seems a good name.
Otherwise, gdbpy_dict_wrapper_configure_dict_getter ?

Regarding gdbpy_dict_wrapper_PyTypeObject_tps, what about gdbpy_dict_wrapper_getsetattro ?

> You can see an example of doing this in a similar-ish context here
> 
> https://sourceware.org/pipermail/gdb-patches/2026-February/225253.html
> 
> Tom

Matthieu

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

* Re: [PATCH v2 9/9] gdb/python: add accessor helpers for __dict__ in Python extension objects
  2026-03-06 14:33     ` Matthieu Longo
@ 2026-03-06 16:04       ` Tom Tromey
  0 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2026-03-06 16:04 UTC (permalink / raw)
  To: Matthieu Longo; +Cc: Tom Tromey, gdb-patches

>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:

> For gdbpy_dict_wrapper_PyGetSetDef, I tried to use a constexpr function at first, but there is an issue.
> I am not aware of an easy way to concatenate strings with a constexpr in C++17.

Ahh, I missed that part, sorry about that.

> Please let me know if you know a way of making it compatible with C++17.

Maybe it's possible but I wouldn't jump through too many hoops.

> If the code is convertible to C++17, configure_dict_getter seems a good name.
> Otherwise, gdbpy_dict_wrapper_configure_dict_getter ?

> Regarding gdbpy_dict_wrapper_PyTypeObject_tps, what about gdbpy_dict_wrapper_getsetattro ?

These sound reasonable enough to me, thanks.

Tom

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

* Re: [PATCH v2 5/9] gdb/python: allow ref_ptr<T, Policy>::new_reference to accept subclasses of T
  2026-03-06 11:37     ` Matthieu Longo
  2026-03-06 11:43       ` Matthieu Longo
@ 2026-03-06 16:47       ` Tom Tromey
  2026-03-09 11:38         ` Matthieu Longo
  1 sibling, 1 reply; 34+ messages in thread
From: Tom Tromey @ 2026-03-06 16:47 UTC (permalink / raw)
  To: Matthieu Longo; +Cc: Tom Tromey, gdb-patches

>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:

> After a deeper examination, I am not sure that your series helps much for this patch.

> * Case 1: return value from the registry.

> PyObject *result = (PyObject *) cfpy_inferior_corefile_data_key.get (inf);
>   if (result != nullptr)
>     return gdbpy_ref<>::new_reference (result);

> If I had gdbpy_borrowed_ref available, I could replace the code above by something like:
> gdbpy_borrowed_ref result = cfpy_inferior_corefile_data_key.get (inf);
> if (result != nullptr)
>   return result; /* assuming that gdbpy_ref<> would have a constructor taking gdbpy_borrowed_ref. */

Ok, I see.  Sorry about that.

Does the registry need to return a properly-typed one?
Perhaps it can just always return a gdbpy_ref<>.

> * Case 2: registration of the new object in the registry and return the value.

> gdbpy_ref<corefile_object> object
>     (PyObject_New (corefile_object, &corefile_object_type));
> ...
> cfpy_inferior_corefile_data_key.set (inf, object.get ());
> return gdbpy_ref<>::new_reference (object.release ());

Here the release stuff is no longer needed as upcasting works fine:

cfpy_inferior_corefile_data_key.set (...);
return object;

> However, I am getting further from my original objective, which was only flattening the code for upcoming changes.
> All those improvements seem to me out of scope of my original patch.

Yeah.  It's fine to take small steps and not do too much.
So if it's convenient just go ahead; but if you don't mind add a bit of
explanation to the commit message.

thanks,
Tom

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

* Re: [PATCH v2 4/9] gdb: add new helpers for retrieving a type's fully qualified name
  2026-03-03 18:59   ` Tom Tromey
@ 2026-03-06 17:49     ` Matthieu Longo
  2026-03-06 19:45       ` Tom Tromey
  0 siblings, 1 reply; 34+ messages in thread
From: Matthieu Longo @ 2026-03-06 17:49 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 03/03/2026 18:59, Tom Tromey wrote:
>>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:
> 
>> +/* Return the type's fully qualified name from a PyTypeObject.  */
>> +const char *
>> +gdb_py_tp_name (PyTypeObject *py_type) noexcept
>> +{
>> +#if PY_VERSION_HEX >= 0x030d0000
> ...
>> +#elif ! defined (Py_LIMITED_API)
> ...
>> +#else
>> +  #error "The version of Python you are using does not expose " \
>> +	 "PyType_GetFullyQualifiedName() as a part of the limited C API."
>> +#endif
> 
> I think this case can't actually be hit... can it?
> 

It currently can when you build GDB with python 3.11 for instance with --enable-py-limited-api=yes.
Unfortunately, I haven't added the logic to error when someone requests to build GDB against the limited API, but does not have a suitable version of Python.
This #error were a workaround to provide a meaningful message to the developer in this case.

> My reasoning is that the very first case is taken if either the limited
> API is in use (which requires 0x030e0000 as of this patch) or if Python
> is new enough.
> 
> So, all other cases fall into the non-limited case.
> 
> It would be better IMO to remove it if it cannot ever happen.
> 

I agree with you, all other cases should fall into the non-limited case.
To do so, it requires me to fix the configure script to detect the issue before the build (see below).

>> +#else
>> +  #error "The version of Python you are using does not expose Py_TYPE() "\
>> +	 "as a part of the limited C API."
> 
> I'd prefer that #error not be indented here.
> 

Removed in the patch below.

>> +++ b/gdb/python/py-obj-type.h
>> @@ -0,0 +1,31 @@
>> +/* Helpers related to Python object type
>> +
>> +/* Return the type's fully qualified name from a PyTypeObject.  */
>> +extern const char *
>> +gdb_py_tp_name (PyTypeObject *py_type) noexcept;
>> +
>> +/* Return the type's fully qualified name from a PyObject.  */
>> +extern const char *
>> +gdbpy_py_obj_tp_name (PyObject *self) noexcept;
> 
> Declarations in gdb don't have a newline after the return type.
> 

Fixed in the next revision.

> thanks,
> Tom

Here is the fix to remove #error macros, and detect the issue at configure time.
I will split the configure fix from the removal of the #error macros in the next revision.

Matthieu


diff --git a/gdb/configure.ac b/gdb/configure.ac
index a3343dec118..b7ae2f3bb3c 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -933,6 +933,7 @@ else
      # do except assume that the compiler will be able to find those files.
      python_includes=
      python_libs=
+    python_prefix=
      have_python_config=no
    fi

@@ -1062,6 +1063,13 @@ AC_SUBST(PYTHON_CPPFLAGS)
  AC_SUBST(PYTHON_LIBS)
  AM_CONDITIONAL(HAVE_PYTHON, test "${have_libpython}" != no)

+dnl Use --enable-py-limited-api to enable the build of GDB against the Python
+dnl limited API.
+dnl
+dnl no -   Disable the Python limited API.
+dnl yes -  Use the Python limited API to build GDB, error if the selected
+dnl        version of Python is not compatible with the Python limited API.
+
  # Check whether to build GDB against Python limited C API.
  AC_ARG_ENABLE([py-limited-api],
               [AS_HELP_STRING([--enable-py-limited-api],
@@ -1072,10 +1080,14 @@ AC_ARG_ENABLE([py-limited-api],
  if test "$enable_py_limited_api" = yes; then
    # The minimal Python limited API version is currently set to 3.14 for the
    # support of Py_TYPE().
-  # The choice of the minimal version for the Python limited API won't be frozen
-  # until the end of the migration.
-  AC_DEFINE(Py_LIMITED_API, 0x030e0000,
-           [Define if GDB should be built against the Python limited C API.])
+  # The choice of the minimal version for the Python limited API won't be
+  # frozen until the end of the migration.
+  AM_PYTHON_CHECK_VERSION([$python_prog], [3.14], [
+    AC_DEFINE(Py_LIMITED_API, 0x030e0000,
+      [Define if GDB should be built against the Python limited C API.])
+    ],[
+      AC_MSG_ERROR([Python limited API support requires at least Python version 3.14])
+    ])
  fi

  # -------------------- #
diff --git a/gdb/python/py-obj-type.c b/gdb/python/py-obj-type.c
index 034ebd26b8d..ea0b59a8447 100644
--- a/gdb/python/py-obj-type.c
+++ b/gdb/python/py-obj-type.c
@@ -33,7 +33,7 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept

    return PyUnicode_AsUTF8AndSize (fully_qualified_name, nullptr);

-#elif ! defined (Py_LIMITED_API)
+#else /* PY_VERSION_HEX < 0x030d0000 && ! defined (Py_LIMITED_API)  */
    /* For non-heap types, the fully qualified name corresponds to tp_name.  */
    if (! (PyType_GetFlags (py_type) & Py_TPFLAGS_HEAPTYPE))
      return py_type->tp_name;
@@ -62,10 +62,6 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept

    return PyUnicode_AsUTF8AndSize (ht->ht_qualname, nullptr);
  # endif
-
-#else
-  #error "The version of Python you are using does not expose " \
-        "PyType_GetFullyQualifiedName() as a part of the limited C API."
  #endif
  }

@@ -74,11 +70,5 @@ const char *
  gdbpy_py_obj_tp_name (PyObject *self) noexcept
  {
    /* Note: Py_TYPE () is part of the stable ABI since version 3.14.  */
-#if ! defined (Py_LIMITED_API) \
-    || Py_LIMITED_API >= 0x030e0000
    return gdb_py_tp_name (Py_TYPE (self));
-#else
-  #error "The version of Python you are using does not expose Py_TYPE() "\
-        "as a part of the limited C API."
-#endif
  }

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

* Re: [PATCH v2 4/9] gdb: add new helpers for retrieving a type's fully qualified name
  2026-03-06 17:49     ` Matthieu Longo
@ 2026-03-06 19:45       ` Tom Tromey
  0 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2026-03-06 19:45 UTC (permalink / raw)
  To: Matthieu Longo; +Cc: Tom Tromey, gdb-patches

>> I think this case can't actually be hit... can it?

> It currently can when you build GDB with python 3.11 for instance with
> --enable-py-limited-api=yes.  Unfortunately, I haven't added the logic
> to error when someone requests to build GDB against the limited API,
> but does not have a suitable version of Python.  This #error were a
> workaround to provide a meaningful message to the developer in this
> case.

Ah ok, I see.

I guess I was assuming that we'd get an error from "somewhere" (Python I
guess) since gdb would be requesting a limited API with a version newer
than what's available.

> Here is the fix to remove #error macros, and detect the issue at configure time.
> I will split the configure fix from the removal of the #error macros in the next revision.

> +  # The choice of the minimal version for the Python limited API won't be
> +  # frozen until the end of the migration.
> +  AM_PYTHON_CHECK_VERSION([$python_prog], [3.14], [
> +    AC_DEFINE(Py_LIMITED_API, 0x030e0000,
> +      [Define if GDB should be built against the Python limited C API.])
> +    ],[
> +      AC_MSG_ERROR([Python limited API support requires at least Python version 3.14])
> +    ])
>  fi

Seems reasonable, thanks.

FWIW for a speciality configure option like this, I think it's also fine
to bail out at build time, like just a check and a #error in python-internal.h.
But configure failing is definitely a bit friendlier.

thanks,
Tom

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

* Re: [PATCH v2 5/9] gdb/python: allow ref_ptr<T, Policy>::new_reference to accept subclasses of T
  2026-03-06 16:47       ` Tom Tromey
@ 2026-03-09 11:38         ` Matthieu Longo
  0 siblings, 0 replies; 34+ messages in thread
From: Matthieu Longo @ 2026-03-09 11:38 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 06/03/2026 16:47, Tom Tromey wrote:
>>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:
> 
>> After a deeper examination, I am not sure that your series helps much for this patch.
> 
>> * Case 1: return value from the registry.
> 
>> PyObject *result = (PyObject *) cfpy_inferior_corefile_data_key.get (inf);
>>    if (result != nullptr)
>>      return gdbpy_ref<>::new_reference (result);
> 
>> If I had gdbpy_borrowed_ref available, I could replace the code above by something like:
>> gdbpy_borrowed_ref result = cfpy_inferior_corefile_data_key.get (inf);
>> if (result != nullptr)
>>    return result; /* assuming that gdbpy_ref<> would have a constructor taking gdbpy_borrowed_ref. */
> 
> Ok, I see.  Sorry about that.
> 
> Does the registry need to return a properly-typed one?

In my understanding, it does not.
I guess you would like to change

static const registry<inferior>::key<corefile_object,
				     inferior_corefile_deleter>
      cfpy_inferior_corefile_data_key;

to

static const registry<inferior>::key<PyObject,
				     inferior_corefile_deleter>
      cfpy_inferior_corefile_data_key;

But the only impact of this is removing the cast to (PyObject *).
Today we can already write:
   gdbpy_ref<> result (cfpy_inferior_corefile_data_key.get (inf));

I personally don't like this syntax, and would prefer something like:

   gdbpy_borrowed<> key<gdbpy_ref<>, inferior_corefile_deleter>::get (inferior *) const;

And then in this function:

   gdbpy_borrowed<> corefile_obj = cfpy_inferior_corefile_data_key.get (inf);
   if (corefile_obj != nullptr)
     return corefile_obj;

It seems more natural to me.

> Perhaps it can just always return a gdbpy_ref<>.
> 

I am not sure how to do this with the current implementation of registry.

Why not using something like an std::unordered_map<inferior *, gdbpy_ref<corefile_object>> ?
The only impediment seems to be this clean-up routine setting corefile_object->inferior to nullptr.
I am not really understanding why this is required. Could this setting to null be removed completely ?

>> * Case 2: registration of the new object in the registry and return the value.
> 
>> gdbpy_ref<corefile_object> object
>>      (PyObject_New (corefile_object, &corefile_object_type));
>> ...
>> cfpy_inferior_corefile_data_key.set (inf, object.get ());
>> return gdbpy_ref<>::new_reference (object.release ());
> 
> Here the release stuff is no longer needed as upcasting works fine:
> 

The release() is not required for upcasting, but keeping the reference counter
incremented to count for the instance stored inside the registry.

/* Allocation of object via PyObject_New() */
gdbpy_ref<corefile_object> object
     (PyObject_New (corefile_object, &corefile_object_type));
...

/* Save the object in the registry which only holds a raw pointer.
    If this registry was storing gdbpy_ref<> and set() was supporting
    both copy and move semantics, using set(const &gdbpy_ref<>) would
    simplify the code on the return to a simple "return value;".  */
cfpy_inferior_corefile_data_key.set (inf, object.get ());

/* Return a new reference. The copy is not sufficient because the gdbpy_ref<>
    would go out of scope at the end of the function, and the registry would
    potentially hold a dangling pointer as soon as the return value would go
    out of scope somewhere in the calling context. */
return gdbpy_ref<>::new_reference (object.release ());

> cfpy_inferior_corefile_data_key.set (...);
> return object;
> 
>> However, I am getting further from my original objective, which was only flattening the code for upcoming changes.
>> All those improvements seem to me out of scope of my original patch.
> 
> Yeah.  It's fine to take small steps and not do too much.

If we decide to change the implementation of the registry later to store gdbpy_ref<>, the code should simplify itself.
However, as explain before, the replacement of a raw pointer by gdbpy_ref<> does not look trivial.
Consequently, I propose to keep the code as it is for now.

> So if it's convenient just go ahead; but if you don't mind add a bit of
> explanation to the commit message.
> 

If I stick to the original goal of this patch, i.e. allowing ref_ptr<T, Policy>::new_reference to accept subclasses of T, it seems to me that the description is clear enough.
What would you like to add ?

> thanks,
> Tom

Matthieu

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

* Re: [PATCH v2 6/9] gdb/python: flatten functions calling PyObject_New and use gdbpy_ref
  2026-03-03 18:22   ` Tom Tromey
@ 2026-03-09 11:41     ` Matthieu Longo
  0 siblings, 0 replies; 34+ messages in thread
From: Matthieu Longo @ 2026-03-09 11:41 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 03/03/2026 18:22, Tom Tromey wrote:
>>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:
> 
>> This patch aims at systematically using gdbpy_ref<> at all call sites
>> of PyObject_New(). This prepares for future patches that expect
>> gdbby_ref<> parameters and affect return handling.
>> As part of this change, flattening the affected functions so that the
>> return logic becomes clearer and more flexible to adjust.
> 
> Makes sense, thank you.
> 
>> -  return gdbpy_ref<>::new_reference (result);
>> +  return gdbpy_ref<>::new_reference (object.release ());
> 
> If that other series goes in the '.release' can be dropped.
> 
> Tom

As explained in the previous patch, because we still need to account for the reference hold by the registry.

Matthieu

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

* Re: [PATCH v2 7/9] gdb/python: accept gdbpy_ref in init helpers and return bool
  2026-03-03 18:24   ` Tom Tromey
@ 2026-03-09 13:25     ` Matthieu Longo
  0 siblings, 0 replies; 34+ messages in thread
From: Matthieu Longo @ 2026-03-09 13:25 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 03/03/2026 18:24, Tom Tromey wrote:
>>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:
> 
>> Passing 'gdbpy_ref<> &' instead of raw 'PyObject *' to init helpers
>> makes ownership of PyObject clearer at call sites, and removes
>> unnecessary '.get()' calls.
>> Changing the return type from 'int' to 'bool' improves readability
>> and better expresses the success/failure semantics.
> 
> Ok.
> Approved-By: Tom Tromey <tom@tromey.com>
> 
> Tom

Merged.

Matthieu

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

end of thread, other threads:[~2026-03-09 13:27 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-03 16:16 [PATCH v2 0/9] gdb: more fixes for Python limited C API support Matthieu Longo
2026-03-03 16:16 ` [PATCH v2 1/9] gdb: switch tuple object helpers to Python limited API equivalents Matthieu Longo
2026-03-03 18:09   ` Tom Tromey
2026-03-03 16:16 ` [PATCH v2 2/9] gdb: introduce rgb_color type to simplify existing code Matthieu Longo
2026-03-03 18:16   ` Tom Tromey
2026-03-04 16:30     ` Matthieu Longo
2026-03-03 16:16 ` [PATCH v2 3/9] gdb: switch bytes object helpers to Python limited API equivalents Matthieu Longo
2026-03-03 18:03   ` Tom Tromey
2026-03-03 16:16 ` [PATCH v2 4/9] gdb: add new helpers for retrieving a type's fully qualified name Matthieu Longo
2026-03-03 18:59   ` Tom Tromey
2026-03-06 17:49     ` Matthieu Longo
2026-03-06 19:45       ` Tom Tromey
2026-03-03 16:16 ` [PATCH v2 5/9] gdb/python: allow ref_ptr<T, Policy>::new_reference to accept subclasses of T Matthieu Longo
2026-03-03 18:18   ` Tom Tromey
2026-03-04 16:56     ` Matthieu Longo
2026-03-04 18:55       ` Tom Tromey
2026-03-06 11:37     ` Matthieu Longo
2026-03-06 11:43       ` Matthieu Longo
2026-03-06 16:47       ` Tom Tromey
2026-03-09 11:38         ` Matthieu Longo
2026-03-03 16:16 ` [PATCH v2 6/9] gdb/python: flatten functions calling PyObject_New and use gdbpy_ref Matthieu Longo
2026-03-03 18:22   ` Tom Tromey
2026-03-09 11:41     ` Matthieu Longo
2026-03-03 18:22   ` Tom Tromey
2026-03-03 16:16 ` [PATCH v2 7/9] gdb/python: accept gdbpy_ref in init helpers and return bool Matthieu Longo
2026-03-03 18:24   ` Tom Tromey
2026-03-09 13:25     ` Matthieu Longo
2026-03-03 16:16 ` [PATCH v2 8/9] gdb/python: add gdbpy_dict_wrapper:allocate_dict helper Matthieu Longo
2026-03-03 18:30   ` Tom Tromey
2026-03-06 12:03     ` Matthieu Longo
2026-03-03 16:16 ` [PATCH v2 9/9] gdb/python: add accessor helpers for __dict__ in Python extension objects Matthieu Longo
2026-03-03 19:02   ` Tom Tromey
2026-03-06 14:33     ` Matthieu Longo
2026-03-06 16:04       ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox