[PATCH v1] gdb/python: fix memory leak in gdb_py_tp_name

[PATCH v1] gdb/python: fix memory leak in gdb_py_tp_name

[Matthieu Longo]

The current implementation of gdb_py_tp_name() leaks a reference to the
object returned by PyType_GetFullyQualifiedName() for Python >= 3.13, and
PyType_GetQualName() for Python >= 3.11.

Managing the strong reference on the returned PyObject* with a gdbpy_ref<>
fixes the reference count, but also causes the temporary object to be
deallocated on function exit. As a consequence, the 'const char *' returned
by PyUnicode_AsUTF8AndSize() becomes dangling and can no longer safely be
returned.

The proposed approach consists in changing gdb_py_tp_name() to return a
std::string, and forcing a copy of the 'const char *' value, statically
or dynamically allocated, stored in a temporary or non-temporary PyObject,
depending on the version of Python it was compiled with.
A unfortunate side effect of this fix is that every call sites where the
tp_name is printed, must now use `.c_str()' because PyErr_Format()
and its siblings cannot handle std::string.
---
 gdb/python/py-arch.c         |  2 +-
 gdb/python/py-block.c        |  2 +-
 gdb/python/py-breakpoint.c   |  6 +++---
 gdb/python/py-connection.c   |  2 +-
 gdb/python/py-corefile.c     |  2 +-
 gdb/python/py-disasm.c       | 14 +++++++-------
 gdb/python/py-frame.c        |  4 ++--
 gdb/python/py-infthread.c    |  2 +-
 gdb/python/py-mi.c           |  2 +-
 gdb/python/py-micmd.c        |  4 +++-
 gdb/python/py-obj-type.c     | 12 ++++++------
 gdb/python/py-obj-type.h     |  4 ++--
 gdb/python/py-style.c        | 14 +++++++-------
 gdb/python/py-symbol.c       |  2 +-
 gdb/python/py-type.c         |  2 +-
 gdb/python/py-unwind.c       |  8 ++++----
 gdb/python/py-utils.c        |  5 +++--
 gdb/python/python-internal.h | 13 ++++++++-----
 gdb/python/python.c          |  2 +-
 19 files changed, 54 insertions(+), 48 deletions(-)

diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index 7a0cb0a2a59..5b3cfbdc876 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -343,7 +343,7 @@ archpy_repr (PyObject *self)
 
   auto arch_info = gdbarch_bfd_arch_info (gdbarch);
   return PyUnicode_FromFormat ("<%s arch_name=%s printable_name=%s>",
-			       gdbpy_py_obj_tp_name (self),
+			       gdbpy_py_obj_tp_name (self).c_str (),
 			       arch_info->arch_name,
 			       arch_info->printable_name);
 }
diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
index 903f800683a..99c487bc875 100644
--- a/gdb/python/py-block.c
+++ b/gdb/python/py-block.c
@@ -527,7 +527,7 @@ blpy_repr (PyObject *self)
 	str += ", ";
     }
   return PyUnicode_FromFormat ("<%s %s {%s}>",
-			       gdbpy_py_obj_tp_name (self),
+			       gdbpy_py_obj_tp_name (self).c_str (),
 			       name, str.c_str ());
 }
 
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index d1fec6a7700..30d5ea471c2 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -1067,7 +1067,7 @@ 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 &tp_name = gdbpy_py_obj_tp_name (self);
 
   const auto bp = (struct gdbpy_breakpoint_object*) self;
   if (bp->bp == nullptr)
@@ -1083,7 +1083,7 @@ bppy_repr (PyObject *self)
   str.pop_back ();
 
   return PyUnicode_FromFormat ("<%s%s number=%d hits=%d%s>",
-			       tp_name,
+			       tp_name.c_str (),
 			       (bp->bp->enable_state == bp_enabled
 				? "" : " disabled"), bp->bp->number,
 			       bp->bp->hit_count, str.c_str ());
@@ -1776,7 +1776,7 @@ bplocpy_repr (PyObject *py_self)
     }
 
   return PyUnicode_FromFormat ("<%s %s>",
-			       gdbpy_py_obj_tp_name (py_self),
+			       gdbpy_py_obj_tp_name (py_self).c_str (),
 			       str.c_str ());
 }
 
diff --git a/gdb/python/py-connection.c b/gdb/python/py-connection.c
index 02330b28068..bc738669b79 100644
--- a/gdb/python/py-connection.c
+++ b/gdb/python/py-connection.c
@@ -203,7 +203,7 @@ connpy_repr (PyObject *obj)
     return gdb_py_invalid_object_repr (obj);
 
   return PyUnicode_FromFormat ("<%s num=%d, what=\"%s\">",
-			       gdbpy_py_obj_tp_name (obj),
+			       gdbpy_py_obj_tp_name (obj).c_str (),
 			       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 fbefbd267f4..fc5b4889fdc 100644
--- a/gdb/python/py-corefile.c
+++ b/gdb/python/py-corefile.c
@@ -400,7 +400,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'>",
-			       gdbpy_py_obj_tp_name (self),
+			       gdbpy_py_obj_tp_name (self).c_str (),
 			       obj->inferior->num,
 			       bfd_get_filename (core_bfd));
 }
diff --git a/gdb/python/py-disasm.c b/gdb/python/py-disasm.c
index e5d7174b7d6..a2abf37a605 100644
--- a/gdb/python/py-disasm.c
+++ b/gdb/python/py-disasm.c
@@ -312,7 +312,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>",
-			       gdbpy_py_obj_tp_name (self),
+			       gdbpy_py_obj_tp_name (self).c_str (),
 			       core_addr_to_string_nz (obj->address),
 			       arch_name);
 }
@@ -1003,7 +1003,7 @@ disasmpy_result_init (PyObject *self, PyObject *args, PyObject *kwargs)
     {
       PyErr_Format (PyExc_ValueError,
 		    _("Cannot use 'string' and 'parts' when creating %s."),
-		    gdbpy_py_obj_tp_name (self));
+		    gdbpy_py_obj_tp_name (self).c_str ());
       return -1;
     }
 
@@ -1087,7 +1087,7 @@ disasmpy_result_repr (PyObject *self)
   gdb_assert (obj->parts != nullptr);
 
   return PyUnicode_FromFormat ("<%s length=%d string=\"%U\">",
-			       gdbpy_py_obj_tp_name (self),
+			       gdbpy_py_obj_tp_name (self).c_str (),
 			       obj->length,
 			       disasmpy_result_str (self));
 }
@@ -1302,7 +1302,7 @@ gdbpy_print_insn (struct gdbarch *gdbarch, CORE_ADDR memaddr,
       PyErr_Format
 	(PyExc_TypeError,
 	 _("Result from Disassembler must be gdb.DisassemblerResult, not %s."),
-	 gdbpy_py_obj_tp_name (result.get ()));
+	 gdbpy_py_obj_tp_name (result.get ()).c_str ());
       gdbpy_print_stack ();
       return std::optional<int> (-1);
     }
@@ -1391,7 +1391,7 @@ disasmpy_part_init (PyObject *self, PyObject *args, PyObject *kwargs)
 {
   PyErr_Format (PyExc_RuntimeError,
 		_("Cannot create instances of %s."),
-		gdbpy_py_obj_tp_name (self));
+		gdbpy_py_obj_tp_name (self).c_str ());
   return -1;
 }
 
@@ -1428,7 +1428,7 @@ disasmpy_text_part_repr (PyObject *self)
   gdb_assert (obj->string != nullptr);
 
   return PyUnicode_FromFormat ("<%s string='%s', style='%s'>",
-			       gdbpy_py_obj_tp_name (self),
+			       gdbpy_py_obj_tp_name (self).c_str (),
 			       obj->string->c_str (),
 			       get_style_name (obj->style));
 }
@@ -1471,7 +1471,7 @@ disasmpy_addr_part_repr (PyObject *self)
   disasm_addr_part_object *obj = (disasm_addr_part_object *) self;
 
   return PyUnicode_FromFormat ("<%s address='%s'>",
-			       gdbpy_py_obj_tp_name (self),
+			       gdbpy_py_obj_tp_name (self).c_str (),
 			       core_addr_to_string_nz (obj->address));
 }
 
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 0a1a0dffbf9..859b8bdbb6e 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -97,7 +97,7 @@ frapy_repr (PyObject *self)
 
   const frame_id &fid = frame_obj->frame_id;
   return PyUnicode_FromFormat ("<%s level=%d frame-id=%s>",
-			       gdbpy_py_obj_tp_name (self),
+			       gdbpy_py_obj_tp_name (self).c_str (),
 			       frame_relative_level (f_info),
 			       fid.to_string ().c_str ());
 }
@@ -545,7 +545,7 @@ frapy_read_var (PyObject *self, PyObject *args, PyObject *kw)
     {
       PyErr_Format (PyExc_TypeError,
 		    _("argument 1 must be gdb.Symbol or str, not %s"),
-		    gdbpy_py_obj_tp_name (sym_obj));
+		    gdbpy_py_obj_tp_name (sym_obj).c_str ());
       return NULL;
     }
 
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index ddb67a284ac..96c736495a8 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -354,7 +354,7 @@ thpy_repr (PyObject *self)
 
   thread_info *thr = thread_obj->thread;
   return PyUnicode_FromFormat ("<%s id=%s target-id=\"%s\">",
-			       gdbpy_py_obj_tp_name (self),
+			       gdbpy_py_obj_tp_name (self).c_str (),
 			       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 1bcb276da8a..73daefecb20 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"),
-	 gdbpy_py_obj_tp_name (data));
+	 gdbpy_py_obj_tp_name (data).c_str ());
       return nullptr;
     }
 
diff --git a/gdb/python/py-micmd.c b/gdb/python/py-micmd.c
index 9ae7fc51bee..9f6e664a5f2 100644
--- a/gdb/python/py-micmd.c
+++ b/gdb/python/py-micmd.c
@@ -511,7 +511,9 @@ 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" : gdbpy_py_obj_tp_name (newvalue));
+		    (newvalue == Py_None
+		     ? "None"
+		     : gdbpy_py_obj_tp_name (newvalue).c_str ()));
       return -1;
     }
 
diff --git a/gdb/python/py-obj-type.c b/gdb/python/py-obj-type.c
index ea0b59a8447..6d428b355b8 100644
--- a/gdb/python/py-obj-type.c
+++ b/gdb/python/py-obj-type.c
@@ -21,17 +21,17 @@
 #include "py-obj-type.h"
 
 /* Return the type's fully qualified name from a PyTypeObject.  */
-const char *
+std::string
 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);
+  gdbpy_ref<> fully_qualified_name (PyType_GetFullyQualifiedName (py_type));
   if (fully_qualified_name == nullptr)
     return nullptr;
 
-  return PyUnicode_AsUTF8AndSize (fully_qualified_name, nullptr);
+  return PyUnicode_AsUTF8AndSize (fully_qualified_name.get (), nullptr);
 
 #else /* PY_VERSION_HEX < 0x030d0000 && ! defined (Py_LIMITED_API)  */
   /* For non-heap types, the fully qualified name corresponds to tp_name.  */
@@ -44,11 +44,11 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept
 
 # if PY_VERSION_HEX >= 0x030b0000
   /* Note: PyType_GetQualName() was added in version 3.11.  */
-  PyObject *qualname = PyType_GetQualName (py_type);
+  gdbpy_ref<> qualname (PyType_GetQualName (py_type));
   if (qualname == nullptr)
     return nullptr;
 
-  return PyUnicode_AsUTF8AndSize (qualname, nullptr);
+  return PyUnicode_AsUTF8AndSize (qualname.get (), nullptr);
 
 # else
   /* In the absence of PyType_GetQualName(), fallback on using PyHeapTypeObject
@@ -66,7 +66,7 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept
 }
 
 /* Return the type's fully qualified name from a PyObject.  */
-const char *
+std::string
 gdbpy_py_obj_tp_name (PyObject *self) noexcept
 {
   /* Note: Py_TYPE () is part of the stable ABI since version 3.14.  */
diff --git a/gdb/python/py-obj-type.h b/gdb/python/py-obj-type.h
index 293647fabfb..ac6c6aa3296 100644
--- a/gdb/python/py-obj-type.h
+++ b/gdb/python/py-obj-type.h
@@ -21,9 +21,9 @@
 #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;
+extern std::string 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;
+extern std::string 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 c45eba60867..a27a1ab75a7 100644
--- a/gdb/python/py-style.c
+++ b/gdb/python/py-style.c
@@ -269,7 +269,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."),
-	 gdbpy_py_obj_tp_name (fg));
+	 gdbpy_py_obj_tp_name (fg).c_str ());
       return -1;
     }
 
@@ -278,7 +278,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."),
-	 gdbpy_py_obj_tp_name (bg));
+	 gdbpy_py_obj_tp_name (bg).c_str ());
       return -1;
     }
 
@@ -485,7 +485,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"),
-		    gdbpy_py_obj_tp_name (newvalue));
+		    gdbpy_py_obj_tp_name (newvalue).c_str ());
       return -1;
     }
 
@@ -543,7 +543,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"),
-		    gdbpy_py_obj_tp_name (newvalue));
+		    gdbpy_py_obj_tp_name (newvalue).c_str ());
       return -1;
     }
 
@@ -625,7 +625,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"),
-	 gdbpy_py_obj_tp_name (newvalue));
+	 gdbpy_py_obj_tp_name (newvalue).c_str ());
       return -1;
     }
 
@@ -735,12 +735,12 @@ stylepy_repr (PyObject *self)
 
   if (style_obj->style_name == nullptr)
     return PyUnicode_FromFormat ("<%s fg=%s, bg=%s, intensity=%s>",
-				 gdbpy_py_obj_tp_name (self),
+				 gdbpy_py_obj_tp_name (self).c_str (),
 				 fg_str.get (), bg_str.get (),
 				 intensity_str);
   else
     return PyUnicode_FromFormat ("<%s name='%s', fg=%s, bg=%s, intensity=%s>",
-				 gdbpy_py_obj_tp_name (self),
+				 gdbpy_py_obj_tp_name (self).c_str (),
 				 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 5b1d843a36f..5840a3f5720 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -384,7 +384,7 @@ sympy_repr (PyObject *self)
     return gdb_py_invalid_object_repr (self);
 
   return PyUnicode_FromFormat ("<%s print_name=%s>",
-			       gdbpy_py_obj_tp_name (self),
+			       gdbpy_py_obj_tp_name (self).c_str (),
 			       symbol->print_name ());
 }
 
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index c002d97d0c6..7831e838481 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -1086,7 +1086,7 @@ typy_repr (PyObject *self)
 				       host_charset (), NULL);
 
   return PyUnicode_FromFormat ("<%s code=%s name=%U>",
-			       gdbpy_py_obj_tp_name (self),
+			       gdbpy_py_obj_tp_name (self).c_str (),
 			       code, py_typename);
 }
 
diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
index 7ec87367d0a..13eced4cecb 100644
--- a/gdb/python/py-unwind.c
+++ b/gdb/python/py-unwind.c
@@ -252,7 +252,7 @@ unwind_infopy_repr (PyObject *self)
 
   if (pending_frame->frame_info == nullptr)
     return PyUnicode_FromFormat ("<%s for an invalid frame>",
-				 gdbpy_py_obj_tp_name (self));
+				 gdbpy_py_obj_tp_name (self).c_str ());
 
   std::string saved_reg_names;
   struct gdbarch *gdbarch = pending_frame->gdbarch;
@@ -268,7 +268,7 @@ unwind_infopy_repr (PyObject *self)
 
   const frame_info_ptr &frame (*pending_frame->frame_info);
   return PyUnicode_FromFormat ("<%s frame #%d, saved_regs=(%s)>",
-			       gdbpy_py_obj_tp_name (self),
+			       gdbpy_py_obj_tp_name (self).c_str (),
 			       frame_relative_level (frame),
 			       saved_reg_names.c_str ());
 }
@@ -464,7 +464,7 @@ pending_framepy_repr (PyObject *self)
     }
 
   return PyUnicode_FromFormat ("<%s level=%d, sp=%s, pc=%s>",
-			       gdbpy_py_obj_tp_name (self),
+			       gdbpy_py_obj_tp_name (self).c_str (),
 			       frame_relative_level (frame),
 			       sp_str,
 			       pc_str);
@@ -939,7 +939,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."),
-	   gdbpy_py_obj_tp_name (pyo_unwind_info));
+	   gdbpy_py_obj_tp_name (pyo_unwind_info).c_str ());
 
   {
     unwind_info_object *unwind_info =
diff --git a/gdb/python/py-utils.c b/gdb/python/py-utils.c
index 96f1cb1ee31..a831b96eb38 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'",
-		gdbpy_py_obj_tp_name (self),
+		gdbpy_py_obj_tp_name (self).c_str (),
 		PyUnicode_AsUTF8AndSize (attr, nullptr));
   return nullptr;
 }
@@ -700,5 +700,6 @@ gdbpy_fix_doc_string_indentation (gdb::unique_xmalloc_ptr<char> doc)
 PyObject *
 gdb_py_invalid_object_repr (PyObject *self)
 {
-  return PyUnicode_FromFormat ("<%s (invalid)>", gdbpy_py_obj_tp_name (self));
+  return PyUnicode_FromFormat ("<%s (invalid)>",
+			       gdbpy_py_obj_tp_name (self).c_str ());
 }
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 10c984bad4b..0cc5fc4ac06 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -1132,15 +1132,18 @@ gdbpy_type_ready (PyTypeObject *type, PyObject *mod = nullptr)
 {
   if (PyType_Ready (type) < 0)
     return -1;
-  const char *tp_name = gdb_py_tp_name (type);
+  const auto &tp_name = gdb_py_tp_name (type);
+  std::string_view tp_name_s = tp_name;
   if (mod == nullptr)
     {
-      gdb_assert (startswith (tp_name, "gdb."));
+      gdb_assert (startswith (tp_name_s, "gdb."));
       mod = gdb_module;
     }
-  const char *dot = strrchr (tp_name, '.');
-  gdb_assert (dot != nullptr);
-  return gdb_pymodule_addobject (mod, dot + 1, (PyObject *) type);
+  const auto pos_dot = tp_name_s.find_last_of ('.');
+  gdb_assert (pos_dot != tp_name_s.npos);
+  return gdb_pymodule_addobject (mod,
+				 tp_name_s.substr (pos_dot + 1).data (),
+				 (PyObject *) type);
 }
 
 /* Poison PyType_Ready.  Only gdbpy_type_ready should be used, to
diff --git a/gdb/python/python.c b/gdb/python/python.c
index e6d58f2f16a..2131c4a0648 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1581,7 +1581,7 @@ gdbpy_write (PyObject *self, PyObject *args, PyObject *kw)
       PyErr_Format
 	(PyExc_TypeError,
 	 _("'style' argument must be gdb.Style or None, not %s."),
-	 gdbpy_py_obj_tp_name (style_obj));
+	 gdbpy_py_obj_tp_name (style_obj).c_str ());
       return nullptr;
     }
 
-- 
2.54.0

Re: [PATCH v1] gdb/python: fix memory leak in gdb_py_tp_name

[Tom Tromey]

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

Matthieu> The proposed approach consists in changing gdb_py_tp_name() to return a
Matthieu> std::string

That seems fine.

Matthieu> A unfortunate side effect of this fix is that every call sites where the
Matthieu> tp_name is printed, must now use `.c_str()' because PyErr_Format()
Matthieu> and its siblings cannot handle std::string.

Yeah, that's unfortunate but normal.

Matthieu>  /* Return the type's fully qualified name from a PyTypeObject.  */
Matthieu> -const char *
Matthieu> +std::string
Matthieu>  gdb_py_tp_name (PyTypeObject *py_type) noexcept
Matthieu>  {
Matthieu>  #if PY_VERSION_HEX >= 0x030d0000
Matthieu>    /* Note: PyType_GetFullyQualifiedName() was added in version 3.13, and is
Matthieu>       part of the stable ABI since version 3.13.  */
Matthieu> -  PyObject *fully_qualified_name = PyType_GetFullyQualifiedName (py_type);
Matthieu> +  gdbpy_ref<> fully_qualified_name (PyType_GetFullyQualifiedName (py_type));
Matthieu>    if (fully_qualified_name == nullptr)
Matthieu>      return nullptr;

First, returning nullptr here seems really weird.  I think this should
cause a crash?  In C++23 this constructor is even deleted.

Second, if PyType_GetFullyQualifiedName fails, this will violate the
invariants gdb normally uses for error handling.

Matthieu> -  return PyUnicode_AsUTF8AndSize (fully_qualified_name, nullptr);
Matthieu> +  return PyUnicode_AsUTF8AndSize (fully_qualified_name.get (), nullptr);
 
I think this can also fail.

Matthieu> +  gdbpy_ref<> qualname (PyType_GetQualName (py_type));
Matthieu>    if (qualname == nullptr)
Matthieu>      return nullptr;
 
Matthieu> -  return PyUnicode_AsUTF8AndSize (qualname, nullptr);
Matthieu> +  return PyUnicode_AsUTF8AndSize (qualname.get (), nullptr);
 
Similar stuff in these spots.

Tom

Re: [PATCH v1] gdb/python: fix memory leak in gdb_py_tp_name

[Matthieu Longo]

On 26/05/2026 17:35, Tom Tromey wrote:
>>>>>> "Matthieu" == Matthieu Longo <matthieu.longo@arm.com> writes:
> 
> Matthieu> The proposed approach consists in changing gdb_py_tp_name() to return a
> Matthieu> std::string
> 
> That seems fine.
> 
> Matthieu> A unfortunate side effect of this fix is that every call sites where the
> Matthieu> tp_name is printed, must now use `.c_str()' because PyErr_Format()
> Matthieu> and its siblings cannot handle std::string.
> 
> Yeah, that's unfortunate but normal.
> 
> Matthieu>  /* Return the type's fully qualified name from a PyTypeObject.  */
> Matthieu> -const char *
> Matthieu> +std::string
> Matthieu>  gdb_py_tp_name (PyTypeObject *py_type) noexcept
> Matthieu>  {
> Matthieu>  #if PY_VERSION_HEX >= 0x030d0000
> Matthieu>    /* Note: PyType_GetFullyQualifiedName() was added in version 3.13, and is
> Matthieu>       part of the stable ABI since version 3.13.  */
> Matthieu> -  PyObject *fully_qualified_name = PyType_GetFullyQualifiedName (py_type);
> Matthieu> +  gdbpy_ref<> fully_qualified_name (PyType_GetFullyQualifiedName (py_type));
> Matthieu>    if (fully_qualified_name == nullptr)
> Matthieu>      return nullptr;
> 
> First, returning nullptr here seems really weird.  I think this should
> cause a crash?  In C++23 this constructor is even deleted.
> 
> Second, if PyType_GetFullyQualifiedName fails, this will violate the
> invariants gdb normally uses for error handling.
> 
> Matthieu> -  return PyUnicode_AsUTF8AndSize (fully_qualified_name, nullptr);
> Matthieu> +  return PyUnicode_AsUTF8AndSize (fully_qualified_name.get (), nullptr);
>   
> I think this can also fail.
> 
> Matthieu> +  gdbpy_ref<> qualname (PyType_GetQualName (py_type));
> Matthieu>    if (qualname == nullptr)
> Matthieu>      return nullptr;
>   
> Matthieu> -  return PyUnicode_AsUTF8AndSize (qualname, nullptr);
> Matthieu> +  return PyUnicode_AsUTF8AndSize (qualname.get (), nullptr);
>   
> Similar stuff in these spots.
> 
> Tom

Sorry, I completely missed those issues while I was changing the code in others places.
I should definitely have spotted them if it was not my thoughtlessness in those last days.

Here below is the diff of gdb_py_tp_name() with fixes for all the issues you previously mentioned.
Thanks for your patience with me, Tom.

Matthieu

--- a/gdb/python/py-obj-type.c
+++ b/gdb/python/py-obj-type.c
@@ -21,17 +21,23 @@
  #include "py-obj-type.h"

  /* Return the type's fully qualified name from a PyTypeObject.  */
-const char *
+std::string
  gdb_py_tp_name (PyTypeObject *py_type) noexcept
  {
+  auto pyobj_to_str = [](PyObject *name) -> std::string
+  {
+    const char *s = PyUnicode_AsUTF8AndSize (name, nullptr);
+    return (s == nullptr) ? "" : s;
+  };
+
  #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);
+  gdbpy_ref<> fully_qualified_name (PyType_GetFullyQualifiedName (py_type));
    if (fully_qualified_name == nullptr)
-    return nullptr;
+    return {};

-  return PyUnicode_AsUTF8AndSize (fully_qualified_name, nullptr);
+  return pyobj_to_str (fully_qualified_name.get ());

  #else /* PY_VERSION_HEX < 0x030d0000 && ! defined (Py_LIMITED_API)  */
    /* For non-heap types, the fully qualified name corresponds to tp_name.  */
@@ -44,11 +50,11 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept

  # if PY_VERSION_HEX >= 0x030b0000
    /* Note: PyType_GetQualName() was added in version 3.11.  */
-  PyObject *qualname = PyType_GetQualName (py_type);
+  gdbpy_ref<> qualname (PyType_GetQualName (py_type));
    if (qualname == nullptr)
-    return nullptr;
+    return {};

-  return PyUnicode_AsUTF8AndSize (qualname, nullptr);
+  return pyobj_to_str (qualname.get ());

  # else
    /* In the absence of PyType_GetQualName(), fallback on using PyHeapTypeObject
@@ -58,15 +64,15 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept
       when the minimum supported Python version is increased above 3.10.  */
    PyHeapTypeObject *ht = (PyHeapTypeObject *) py_type;
    if (ht->ht_qualname == nullptr)
-    return nullptr;
+    return {};

-  return PyUnicode_AsUTF8AndSize (ht->ht_qualname, nullptr);
+  return pyobj_to_str (ht->ht_qualname);
  # endif
  #endif
  }

Re: [PATCH v1] gdb/python: fix memory leak in gdb_py_tp_name

[Tom Tromey]

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

> Sorry, I completely missed those issues while I was changing the code
> in others places.  I should definitely have spotted them if it was not
> my thoughtlessness in those last days.

Don't beat yourself up, it's all just normal stuff.

> -  PyObject *fully_qualified_name = PyType_GetFullyQualifiedName (py_type);
> +  gdbpy_ref<> fully_qualified_name (PyType_GetFullyQualifiedName (py_type));
>    if (fully_qualified_name == nullptr)
> -    return nullptr;
> +    return {};

Something I missed in the initial patch to add gdb_py_tp_name is that if
it fails, then the exception will be set, and the callers don't seem to
be prepared to handle that.

I'm not really sure what to do about it.  I guess the principled thing
would be to change the callers (including those of gdbpy_py_obj_tp_name)
to handle this properly :(

Though if we're really sure that this can only fail pathologically
somehow, I suppose we could add asserts in gdb_py_tp_name itself.

Tom

Re: [PATCH v1] gdb/python: fix memory leak in gdb_py_tp_name

[Matthieu Longo]

On 28/05/2026 17:43, Tom Tromey wrote:
>>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:
>> -  PyObject *fully_qualified_name = PyType_GetFullyQualifiedName (py_type);
>> +  gdbpy_ref<> fully_qualified_name (PyType_GetFullyQualifiedName (py_type));
>>     if (fully_qualified_name == nullptr)
>> -    return nullptr;
>> +    return {};
> 
> Something I missed in the initial patch to add gdb_py_tp_name is that if
> it fails, then the exception will be set, and the callers don't seem to
> be prepared to handle that.
> 
> I'm not really sure what to do about it.  I guess the principled thing
> would be to change the callers (including those of gdbpy_py_obj_tp_name)
> to handle this properly :(
> 
> Though if we're really sure that this can only fail pathologically
> somehow, I suppose we could add asserts in gdb_py_tp_name itself.
> 
> Tom

Indeed that can happen, for instance, when this function is called inside a tp_clear() in the middle 
of a PyObject destruction.
It happened to me when I added some logging inside the object's tp_clear() and wanted to print out 
the type name. It looks like a very special corner case.

If PyType_GetFullyQualifiedName () fails, it will probably set a Python error that has to be 
cleared, or it might cause an assertion to be triggered in CPython, and the program aborts.

Example output:

# Start clearing the object.
Call PyType_GetFullyQualifiedName
# PyType_GetFullyQualifiedName failed while building the fully qualified name
# that is supposed to contain the module name.
AttributeError: __module__
Calling (null)::tp_clear_impl (0x7fcecb849dd2)
B_clear_impl
PyType_GetFullyQualifiedName failed!
AttributeError: __module__
(null): calling MyModule.A::tp_clear
PyType_GetFullyQualifiedName failed!
AttributeError: __module__
Calling (null)::tp_clear_impl (0x7fcecb849996)
A_clear_impl
----

Something like below would at least avoid the general abort, but it would cause logging of errors in 
the middle of the destruction. How useful would it be ?

gdbpy_ref<> fully_qualified_name (PyType_GetFullyQualifiedName (py_type));
if (fully_qualified_name == nullptr)
   {
     PyErr_Print();
     PyErr_Clear();
     return {};
   }

At this point, I don't think that it is super important not to crash because there is a serious 
logic error somewhere if we are unable to get this name. Consequently, an assert might be more 
appropriate.

diff --git a/gdb/python/py-obj-type.c b/gdb/python/py-obj-type.c
index 45f7d8a3d74..a78cfe6bf39 100644
--- a/gdb/python/py-obj-type.c
+++ b/gdb/python/py-obj-type.c
@@ -34,9 +34,7 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept
    /* Note: PyType_GetFullyQualifiedName() was added in version 3.13, and is
       part of the stable ABI since version 3.13.  */
    gdbpy_ref<> fully_qualified_name (PyType_GetFullyQualifiedName (py_type));
-  if (fully_qualified_name == nullptr)
-    return {};
-
+  gdb_assert (fully_qualified_name != nullptr);
    return pyobj_to_str (fully_qualified_name.get ());

  #else /* PY_VERSION_HEX < 0x030d0000 && ! defined (Py_LIMITED_API)  */
@@ -51,9 +49,7 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept
  # if PY_VERSION_HEX >= 0x030b0000
    /* Note: PyType_GetQualName() was added in version 3.11.  */
    gdbpy_ref<> qualname (PyType_GetQualName (py_type));
-  if (qualname == nullptr)
-    return {};
-
+  gdb_assert (qualname != nullptr);
    return pyobj_to_str (qualname.get ());

  # else
@@ -63,9 +59,7 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept
       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 {};
-
+  gdb_assert (ht->ht_qualname != nullptr);
    return pyobj_to_str (ht->ht_qualname);
  # endif
  #endif

Re: [PATCH v1] gdb/python: fix memory leak in gdb_py_tp_name

[Tom Tromey]

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

Matthieu> Indeed that can happen, for instance, when this function is called
Matthieu> inside a tp_clear() in the middle of a PyObject destruction.

Thanks.

Matthieu> Something like below would at least avoid the general abort, but it
Matthieu> would cause logging of errors in the middle of the destruction. How
Matthieu> useful would it be ?

Matthieu> gdbpy_ref<> fully_qualified_name (PyType_GetFullyQualifiedName (py_type));
Matthieu> if (fully_qualified_name == nullptr)
Matthieu>   {
Matthieu>     PyErr_Print();
Matthieu>     PyErr_Clear();
Matthieu>     return {};
Matthieu>   }

Matthieu> At this point, I don't think that it is super important not to crash
Matthieu> because there is a serious logic error somewhere if we are unable to
Matthieu> get this name. Consequently, an assert might be more appropriate.

If we think it could conceivably happen then it should probably just
call gdbpy_print_stack (which is basically the above but with some user
controls) and then return some error string.

Tom

Re: [PATCH v1] gdb/python: fix memory leak in gdb_py_tp_name

[Matthieu Longo]

On 28/05/2026 20:00, Tom Tromey wrote:
>>>>>> "Matthieu" == Matthieu Longo <matthieu.longo@arm.com> writes:
> 
> Matthieu> Indeed that can happen, for instance, when this function is called
> Matthieu> inside a tp_clear() in the middle of a PyObject destruction.
> 
> Thanks.
> 
> Matthieu> Something like below would at least avoid the general abort, but it
> Matthieu> would cause logging of errors in the middle of the destruction. How
> Matthieu> useful would it be ?
> 
> Matthieu> gdbpy_ref<> fully_qualified_name (PyType_GetFullyQualifiedName (py_type));
> Matthieu> if (fully_qualified_name == nullptr)
> Matthieu>   {
> Matthieu>     PyErr_Print();
> Matthieu>     PyErr_Clear();
> Matthieu>     return {};
> Matthieu>   }
> 
> Matthieu> At this point, I don't think that it is super important not to crash
> Matthieu> because there is a serious logic error somewhere if we are unable to
> Matthieu> get this name. Consequently, an assert might be more appropriate.
> 
> If we think it could conceivably happen then it should probably just
> call gdbpy_print_stack (which is basically the above but with some user
> controls) and then return some error string.
> 
> Tom

What about the below ?

Matthieu

diff --git a/gdb/python/py-obj-type.c b/gdb/python/py-obj-type.c
index 45f7d8a3d74..514dbf5c179 100644
--- a/gdb/python/py-obj-type.c
+++ b/gdb/python/py-obj-type.c
@@ -27,7 +27,11 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept
    auto pyobj_to_str = [](PyObject *name) -> std::string
    {
      const char *s = PyUnicode_AsUTF8AndSize (name, nullptr);
-    return (s == nullptr) ? "" : s;
+    if (s != nullptr)
+      return s;
+
+    gdbpy_print_stack ();
+    error (_("Could not convert PyObject to UTF-8 string."));
    };

  #if PY_VERSION_HEX >= 0x030d0000
@@ -35,8 +39,10 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept
       part of the stable ABI since version 3.13.  */
    gdbpy_ref<> fully_qualified_name (PyType_GetFullyQualifiedName (py_type));
    if (fully_qualified_name == nullptr)
-    return {};
-
+    {
+      gdbpy_print_stack ();
+      error (_("Could not get fully qualified name."));
+    }
    return pyobj_to_str (fully_qualified_name.get ());

  #else /* PY_VERSION_HEX < 0x030d0000 && ! defined (Py_LIMITED_API)  */
@@ -52,8 +58,10 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept
    /* Note: PyType_GetQualName() was added in version 3.11.  */
    gdbpy_ref<> qualname (PyType_GetQualName (py_type));
    if (qualname == nullptr)
-    return {};
-
+    {
+      gdbpy_print_stack ();
+      error (_("Could not get qualified name."));
+    }
    return pyobj_to_str (qualname.get ());

  # else
@@ -64,8 +72,10 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept
       when the minimum supported Python version is increased above 3.10.  */
    PyHeapTypeObject *ht = (PyHeapTypeObject *) py_type;
    if (ht->ht_qualname == nullptr)
-    return {};
-
+    {
+      gdbpy_print_stack ();
+      error (_("Could not get qualified name."));
+    }
    return pyobj_to_str (ht->ht_qualname);
  # endif
  #endif

Re: [PATCH v1] gdb/python: fix memory leak in gdb_py_tp_name

[Andrew Burgess]

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

> On 28/05/2026 20:00, Tom Tromey wrote:
>>>>>>> "Matthieu" == Matthieu Longo <matthieu.longo@arm.com> writes:
>> 
>> Matthieu> Indeed that can happen, for instance, when this function is called
>> Matthieu> inside a tp_clear() in the middle of a PyObject destruction.
>> 
>> Thanks.
>> 
>> Matthieu> Something like below would at least avoid the general abort, but it
>> Matthieu> would cause logging of errors in the middle of the destruction. How
>> Matthieu> useful would it be ?
>> 
>> Matthieu> gdbpy_ref<> fully_qualified_name (PyType_GetFullyQualifiedName (py_type));
>> Matthieu> if (fully_qualified_name == nullptr)
>> Matthieu>   {
>> Matthieu>     PyErr_Print();
>> Matthieu>     PyErr_Clear();
>> Matthieu>     return {};
>> Matthieu>   }
>> 
>> Matthieu> At this point, I don't think that it is super important not to crash
>> Matthieu> because there is a serious logic error somewhere if we are unable to
>> Matthieu> get this name. Consequently, an assert might be more appropriate.
>> 
>> If we think it could conceivably happen then it should probably just
>> call gdbpy_print_stack (which is basically the above but with some user
>> controls) and then return some error string.
>> 
>> Tom
>
> What about the below ?
>
> Matthieu
>
> diff --git a/gdb/python/py-obj-type.c b/gdb/python/py-obj-type.c
> index 45f7d8a3d74..514dbf5c179 100644
> --- a/gdb/python/py-obj-type.c
> +++ b/gdb/python/py-obj-type.c
> @@ -27,7 +27,11 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept
>     auto pyobj_to_str = [](PyObject *name) -> std::string
>     {
>       const char *s = PyUnicode_AsUTF8AndSize (name, nullptr);
> -    return (s == nullptr) ? "" : s;
> +    if (s != nullptr)
> +      return s;
> +
> +    gdbpy_print_stack ();
> +    error (_("Could not convert PyObject to UTF-8 string."));

I'm a bit late to this review chain, but...

isn't throwing an error here going to be problematic as gdb_py_tp_name
is called from Python callbacks?  Unless these are catching and handling
exceptions then the error is going to end up trying to pass through
Python's C code.

Of course, with Tom's upcoming safety work that would be fine as all of
these exceptions would be caught and handled correctly.

But for now you likely don't want to have to add try/catch everywhere
gdb_py_tp_name is used.

So maybe just returning something like "<type name unavailable>" would
be better?  This might be better even when the safety work _is_ merged;
if gdb_py_tp_name is called as part of logging then we'd probably rather
log "<type name unavailable>" than have GDB throw an exception?

Thanks,
Andrew


>     };
>
>   #if PY_VERSION_HEX >= 0x030d0000
> @@ -35,8 +39,10 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept
>        part of the stable ABI since version 3.13.  */
>     gdbpy_ref<> fully_qualified_name (PyType_GetFullyQualifiedName (py_type));
>     if (fully_qualified_name == nullptr)
> -    return {};
> -
> +    {
> +      gdbpy_print_stack ();
> +      error (_("Could not get fully qualified name."));
> +    }
>     return pyobj_to_str (fully_qualified_name.get ());
>
>   #else /* PY_VERSION_HEX < 0x030d0000 && ! defined (Py_LIMITED_API)  */
> @@ -52,8 +58,10 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept
>     /* Note: PyType_GetQualName() was added in version 3.11.  */
>     gdbpy_ref<> qualname (PyType_GetQualName (py_type));
>     if (qualname == nullptr)
> -    return {};
> -
> +    {
> +      gdbpy_print_stack ();
> +      error (_("Could not get qualified name."));
> +    }
>     return pyobj_to_str (qualname.get ());
>
>   # else
> @@ -64,8 +72,10 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept
>        when the minimum supported Python version is increased above 3.10.  */
>     PyHeapTypeObject *ht = (PyHeapTypeObject *) py_type;
>     if (ht->ht_qualname == nullptr)
> -    return {};
> -
> +    {
> +      gdbpy_print_stack ();
> +      error (_("Could not get qualified name."));
> +    }
>     return pyobj_to_str (ht->ht_qualname);
>   # endif
>   #endif

Re: [PATCH v1] gdb/python: fix memory leak in gdb_py_tp_name

[Tom Tromey]

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

Andrew> isn't throwing an error here going to be problematic as gdb_py_tp_name
Andrew> is called from Python callbacks?  Unless these are catching and handling
Andrew> exceptions then the error is going to end up trying to pass through
Andrew> Python's C code.

Yeah.

I guess maybe it isn't explicitly stated anywhere (not sure) but the
rule in gdb's Python layer is that gdb exceptions can't propagate into
Python itself.  So if a function is called from Python (say like a
__repr__ implementation), then that function has to be sure that it does
not throw.  And since most things in gdb can throw, this is why at
present we wrap calls into gdb's core in try/catch.

Andrew> So maybe just returning something like "<type name unavailable>" would
Andrew> be better?  This might be better even when the safety work _is_ merged;
Andrew> if gdb_py_tp_name is called as part of logging then we'd probably rather
Andrew> log "<type name unavailable>" than have GDB throw an exception?

Yeah, I think this is what should be done, since IIUC the cases where
this can set the Python exception are pathological in nature.

Or just propagating the Python exception.  Though this would mean using
a return type other than std::string.

Tom

Re: [PATCH v1] gdb/python: fix memory leak in gdb_py_tp_name

[Matthieu Longo]

On 29/05/2026 14:09, Tom Tromey wrote:
>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
> 
> Andrew> isn't throwing an error here going to be problematic as gdb_py_tp_name
> Andrew> is called from Python callbacks?  Unless these are catching and handling
> Andrew> exceptions then the error is going to end up trying to pass through
> Andrew> Python's C code.
> 
> Yeah.
> 
> I guess maybe it isn't explicitly stated anywhere (not sure) but the
> rule in gdb's Python layer is that gdb exceptions can't propagate into
> Python itself.  So if a function is called from Python (say like a
> __repr__ implementation), then that function has to be sure that it does
> not throw.  And since most things in gdb can throw, this is why at
> present we wrap calls into gdb's core in try/catch.
> 
> Andrew> So maybe just returning something like "<type name unavailable>" would
> Andrew> be better?  This might be better even when the safety work _is_ merged;
> Andrew> if gdb_py_tp_name is called as part of logging then we'd probably rather
> Andrew> log "<type name unavailable>" than have GDB throw an exception?
> 
> Yeah, I think this is what should be done, since IIUC the cases where
> this can set the Python exception are pathological in nature.
> 
> Or just propagating the Python exception.  Though this would mean using
> a return type other than std::string.
> 
> Tom

Adopting the approach proposed by Andrew.

Matthieu

  /* Return the type's fully qualified name from a PyTypeObject.  */
-const char *
+std::string
  gdb_py_tp_name (PyTypeObject *py_type) noexcept
  {
+  static const std::string NO_TYPE_NAME = "<type name unavailable>";
+
+  auto pyobj_to_str = [&](PyObject *name) -> std::string
+  {
+    const char *s = PyUnicode_AsUTF8AndSize (name, nullptr);
+    if (s == nullptr)
+      return NO_TYPE_NAME;
+    return s;
+  };
+
  #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);
+  gdbpy_ref<> fully_qualified_name (PyType_GetFullyQualifiedName (py_type));
    if (fully_qualified_name == nullptr)
-    return nullptr;
-
-  return PyUnicode_AsUTF8AndSize (fully_qualified_name, nullptr);
+    return NO_TYPE_NAME;
+  return pyobj_to_str (fully_qualified_name.get ());

  #else /* PY_VERSION_HEX < 0x030d0000 && ! defined (Py_LIMITED_API)  */
    /* For non-heap types, the fully qualified name corresponds to tp_name.  */
@@ -44,11 +53,10 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept

  # if PY_VERSION_HEX >= 0x030b0000
    /* Note: PyType_GetQualName() was added in version 3.11.  */
-  PyObject *qualname = PyType_GetQualName (py_type);
+  gdbpy_ref<> qualname (PyType_GetQualName (py_type));
    if (qualname == nullptr)
-    return nullptr;
-
-  return PyUnicode_AsUTF8AndSize (qualname, nullptr);
+    return NO_TYPE_NAME;
+  return pyobj_to_str (qualname.get ());

  # else
    /* In the absence of PyType_GetQualName(), fallback on using PyHeapTypeObject
@@ -58,15 +66,14 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept
       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);
+    return NO_TYPE_NAME;
+  return pyobj_to_str (ht->ht_qualname);
  # endif
  #endif
  }

Re: [PATCH v1] gdb/python: fix memory leak in gdb_py_tp_name

[Andrew Burgess]

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

> On 29/05/2026 14:09, Tom Tromey wrote:
>>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>> 
>> Andrew> isn't throwing an error here going to be problematic as gdb_py_tp_name
>> Andrew> is called from Python callbacks?  Unless these are catching and handling
>> Andrew> exceptions then the error is going to end up trying to pass through
>> Andrew> Python's C code.
>> 
>> Yeah.
>> 
>> I guess maybe it isn't explicitly stated anywhere (not sure) but the
>> rule in gdb's Python layer is that gdb exceptions can't propagate into
>> Python itself.  So if a function is called from Python (say like a
>> __repr__ implementation), then that function has to be sure that it does
>> not throw.  And since most things in gdb can throw, this is why at
>> present we wrap calls into gdb's core in try/catch.
>> 
>> Andrew> So maybe just returning something like "<type name unavailable>" would
>> Andrew> be better?  This might be better even when the safety work _is_ merged;
>> Andrew> if gdb_py_tp_name is called as part of logging then we'd probably rather
>> Andrew> log "<type name unavailable>" than have GDB throw an exception?
>> 
>> Yeah, I think this is what should be done, since IIUC the cases where
>> this can set the Python exception are pathological in nature.
>> 
>> Or just propagating the Python exception.  Though this would mean using
>> a return type other than std::string.
>> 
>> Tom
>
> Adopting the approach proposed by Andrew.
>
> Matthieu
>
>   /* Return the type's fully qualified name from a PyTypeObject.  */
> -const char *
> +std::string
>   gdb_py_tp_name (PyTypeObject *py_type) noexcept
>   {
> +  static const std::string NO_TYPE_NAME = "<type name unavailable>";
> +
> +  auto pyobj_to_str = [&](PyObject *name) -> std::string
> +  {
> +    const char *s = PyUnicode_AsUTF8AndSize (name, nullptr);
> +    if (s == nullptr)
> +      return NO_TYPE_NAME;

If PyUnicode_AsUTF8AndSize fails then a Python exception will be set.
As we're not going to propagate that back to the actual Python
interpreter, should we not clear the exception at this point?

And the same question each time NO_TYPE_NAME is injected instead of an
error?

Thanks,
Andrew

Re: [PATCH v1] gdb/python: fix memory leak in gdb_py_tp_name

[Tom Tromey]

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

>> +    const char *s = PyUnicode_AsUTF8AndSize (name, nullptr);
>> +    if (s == nullptr)
>> +      return NO_TYPE_NAME;

Andrew> If PyUnicode_AsUTF8AndSize fails then a Python exception will be set.
Andrew> As we're not going to propagate that back to the actual Python
Andrew> interpreter, should we not clear the exception at this point?

Andrew> And the same question each time NO_TYPE_NAME is injected instead of an
Andrew> error?

Also worth noting that normally gdb doesn't simply discard Python
exceptions.  (To be clear, there are a couple of spots I think, but
maybe they are bugs.)

Normally either the exception is propagated, standard Python style; or
it is displayed using gdbpy_print_stack.  This function is used because
it respects some user settings that control what is displayed.

Tom

Re: [PATCH v1] gdb/python: fix memory leak in gdb_py_tp_name

[Matthieu Longo]

On 02/06/2026 15:22, Tom Tromey wrote:
>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
> 
>>> +    const char *s = PyUnicode_AsUTF8AndSize (name, nullptr);
>>> +    if (s == nullptr)
>>> +      return NO_TYPE_NAME;
> 
> Andrew> If PyUnicode_AsUTF8AndSize fails then a Python exception will be set.
> Andrew> As we're not going to propagate that back to the actual Python
> Andrew> interpreter, should we not clear the exception at this point?
> 
> Andrew> And the same question each time NO_TYPE_NAME is injected instead of an
> Andrew> error?
> 
> Also worth noting that normally gdb doesn't simply discard Python
> exceptions.  (To be clear, there are a couple of spots I think, but
> maybe they are bugs.)
> 
> Normally either the exception is propagated, standard Python style; or
> it is displayed using gdbpy_print_stack.  This function is used because
> it respects some user settings that control what is displayed.
> 
> Tom

Addressing the previous comment of Andrew, but using gdbpy_print_stack as recommended by Tom.

Matthieu

diff --git a/gdb/python/py-obj-type.c b/gdb/python/py-obj-type.c
index ea0b59a8447..3ed57d0cbab 100644
--- a/gdb/python/py-obj-type.c
+++ b/gdb/python/py-obj-type.c
@@ -21,20 +21,44 @@
  #include "py-obj-type.h"

  /* Return the type's fully qualified name from a PyTypeObject.  */
-const char *
+std::string
  gdb_py_tp_name (PyTypeObject *py_type) noexcept
  {
+  static const std::string NO_TYPE_NAME = "<type name unavailable>";
+
+  /* This helper should be used for cases when the called CPython function
+     informs the caller that an error occurred, and a Python error was set.  */
+  auto handle_err = [&]() -> std::string
+  {
+    gdbpy_print_stack ();
+    PyErr_Clear ();
+    return NO_TYPE_NAME;
+  };
+
+  /* Convert a PyObject to a UTF-8 encoded string.  */
+  auto pyobj_to_str = [&](PyObject *name) -> std::string
+  {
+    const char *s = PyUnicode_AsUTF8AndSize (name, nullptr);
+    if (s == nullptr)
+      return handle_err ();
+    return s;
+  };
+
  #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);
+  /* Notes:
+     1. PyType_GetFullyQualifiedName() was added in version 3.13, and is
+       part of the stable ABI since version 3.13.
+     2. If an error occurs when looking up the module name (for instance,
+       during the destruction of the object), PyType_GetFullyQualifiedName()
+       returns NULL, and a Python error is set.  */
+  gdbpy_ref<> fully_qualified_name (PyType_GetFullyQualifiedName (py_type));
    if (fully_qualified_name == nullptr)
-    return nullptr;
-
-  return PyUnicode_AsUTF8AndSize (fully_qualified_name, nullptr);
+    return handle_err ();
+  return pyobj_to_str (fully_qualified_name.get ());

  #else /* PY_VERSION_HEX < 0x030d0000 && ! defined (Py_LIMITED_API)  */
-  /* For non-heap types, the fully qualified name corresponds to tp_name.  */
+  /* For non-heap types, the fully qualified name corresponds to tp_name,
+     which can never be NULL.  */
    if (! (PyType_GetFlags (py_type) & Py_TPFLAGS_HEAPTYPE))
      return py_type->tp_name;

@@ -43,12 +67,17 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept
       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);
+  /* Notes:
+     1. PyType_GetQualName() was added in version 3.11.
+     2. On one hand, PyType_GetQualName() relies internally on ht_qualname
+       which is supposed to never be NULL, therefore, does not set any Python
+       error.  On the other hand, PyType_GetQualName() calls internally
+       PyUnicode_AsUTF8AndSize(), which when erroring, sets a Python error
+       and returns NULL.  */
+  gdbpy_ref<> qualname (PyType_GetQualName (py_type));
    if (qualname == nullptr)
-    return nullptr;
-
-  return PyUnicode_AsUTF8AndSize (qualname, nullptr);
+    return handle_err ();
+  return pyobj_to_str (qualname.get ());

  # else
    /* In the absence of PyType_GetQualName(), fallback on using PyHeapTypeObject
@@ -58,15 +87,14 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept
       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);
+    return NO_TYPE_NAME;
+  return pyobj_to_str (ht->ht_qualname);
  # endif
  #endif
  }

  /* Return the type's fully qualified name from a PyObject.  */
-const char *
+std::string
  gdbpy_py_obj_tp_name (PyObject *self) noexcept
  {
    /* Note: Py_TYPE () is part of the stable ABI since version 3.14.  */

Re: [PATCH v1] gdb/python: fix memory leak in gdb_py_tp_name

[Andrew Burgess]

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

> On 02/06/2026 15:22, Tom Tromey wrote:
>>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>> 
>>>> +    const char *s = PyUnicode_AsUTF8AndSize (name, nullptr);
>>>> +    if (s == nullptr)
>>>> +      return NO_TYPE_NAME;
>> 
>> Andrew> If PyUnicode_AsUTF8AndSize fails then a Python exception will be set.
>> Andrew> As we're not going to propagate that back to the actual Python
>> Andrew> interpreter, should we not clear the exception at this point?
>> 
>> Andrew> And the same question each time NO_TYPE_NAME is injected instead of an
>> Andrew> error?
>> 
>> Also worth noting that normally gdb doesn't simply discard Python
>> exceptions.  (To be clear, there are a couple of spots I think, but
>> maybe they are bugs.)
>> 
>> Normally either the exception is propagated, standard Python style; or
>> it is displayed using gdbpy_print_stack.  This function is used because
>> it respects some user settings that control what is displayed.
>> 
>> Tom
>
> Addressing the previous comment of Andrew, but using gdbpy_print_stack
> as recommended by Tom.

LGTM.

Thanks,
Andrew


>
> Matthieu
>
> diff --git a/gdb/python/py-obj-type.c b/gdb/python/py-obj-type.c
> index ea0b59a8447..3ed57d0cbab 100644
> --- a/gdb/python/py-obj-type.c
> +++ b/gdb/python/py-obj-type.c
> @@ -21,20 +21,44 @@
>   #include "py-obj-type.h"
>
>   /* Return the type's fully qualified name from a PyTypeObject.  */
> -const char *
> +std::string
>   gdb_py_tp_name (PyTypeObject *py_type) noexcept
>   {
> +  static const std::string NO_TYPE_NAME = "<type name unavailable>";
> +
> +  /* This helper should be used for cases when the called CPython function
> +     informs the caller that an error occurred, and a Python error was set.  */
> +  auto handle_err = [&]() -> std::string
> +  {
> +    gdbpy_print_stack ();
> +    PyErr_Clear ();
> +    return NO_TYPE_NAME;
> +  };
> +
> +  /* Convert a PyObject to a UTF-8 encoded string.  */
> +  auto pyobj_to_str = [&](PyObject *name) -> std::string
> +  {
> +    const char *s = PyUnicode_AsUTF8AndSize (name, nullptr);
> +    if (s == nullptr)
> +      return handle_err ();
> +    return s;
> +  };
> +
>   #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);
> +  /* Notes:
> +     1. PyType_GetFullyQualifiedName() was added in version 3.13, and is
> +       part of the stable ABI since version 3.13.
> +     2. If an error occurs when looking up the module name (for instance,
> +       during the destruction of the object), PyType_GetFullyQualifiedName()
> +       returns NULL, and a Python error is set.  */
> +  gdbpy_ref<> fully_qualified_name (PyType_GetFullyQualifiedName (py_type));
>     if (fully_qualified_name == nullptr)
> -    return nullptr;
> -
> -  return PyUnicode_AsUTF8AndSize (fully_qualified_name, nullptr);
> +    return handle_err ();
> +  return pyobj_to_str (fully_qualified_name.get ());
>
>   #else /* PY_VERSION_HEX < 0x030d0000 && ! defined (Py_LIMITED_API)  */
> -  /* For non-heap types, the fully qualified name corresponds to tp_name.  */
> +  /* For non-heap types, the fully qualified name corresponds to tp_name,
> +     which can never be NULL.  */
>     if (! (PyType_GetFlags (py_type) & Py_TPFLAGS_HEAPTYPE))
>       return py_type->tp_name;
>
> @@ -43,12 +67,17 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept
>        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);
> +  /* Notes:
> +     1. PyType_GetQualName() was added in version 3.11.
> +     2. On one hand, PyType_GetQualName() relies internally on ht_qualname
> +       which is supposed to never be NULL, therefore, does not set any Python
> +       error.  On the other hand, PyType_GetQualName() calls internally
> +       PyUnicode_AsUTF8AndSize(), which when erroring, sets a Python error
> +       and returns NULL.  */
> +  gdbpy_ref<> qualname (PyType_GetQualName (py_type));
>     if (qualname == nullptr)
> -    return nullptr;
> -
> -  return PyUnicode_AsUTF8AndSize (qualname, nullptr);
> +    return handle_err ();
> +  return pyobj_to_str (qualname.get ());
>
>   # else
>     /* In the absence of PyType_GetQualName(), fallback on using PyHeapTypeObject
> @@ -58,15 +87,14 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept
>        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);
> +    return NO_TYPE_NAME;
> +  return pyobj_to_str (ht->ht_qualname);
>   # endif
>   #endif
>   }
>
>   /* Return the type's fully qualified name from a PyObject.  */
> -const char *
> +std::string
>   gdbpy_py_obj_tp_name (PyObject *self) noexcept
>   {
>     /* Note: Py_TYPE () is part of the stable ABI since version 3.14.  */

Re: [PATCH v1] gdb/python: fix memory leak in gdb_py_tp_name

[Tom Tromey]

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

Matthieu> +  /* This helper should be used for cases when the called CPython function
Matthieu> +     informs the caller that an error occurred, and a Python error was set.  */
Matthieu> +  auto handle_err = [&]() -> std::string
Matthieu> +  {
Matthieu> +    gdbpy_print_stack ();
Matthieu> +    PyErr_Clear ();
Matthieu> +    return NO_TYPE_NAME;

The PyErr_Clear is redundant as gdbpy_print_stack guarantees that no
error is set when it returns.

Tom

Re: [PATCH v1] gdb/python: fix memory leak in gdb_py_tp_name

[Matthieu Longo]

On 05/06/2026 21:49, Tom Tromey wrote:
>>>>>> "Matthieu" == Matthieu Longo <matthieu.longo@arm.com> writes:
> 
> Matthieu> +  /* This helper should be used for cases when the called CPython function
> Matthieu> +     informs the caller that an error occurred, and a Python error was set.  */
> Matthieu> +  auto handle_err = [&]() -> std::string
> Matthieu> +  {
> Matthieu> +    gdbpy_print_stack ();
> Matthieu> +    PyErr_Clear ();
> Matthieu> +    return NO_TYPE_NAME;
> 
> The PyErr_Clear is redundant as gdbpy_print_stack guarantees that no
> error is set when it returns.
> 
> Tom

Since I already merged the previous patch, can I apply this obvious fix ?

Matthieu

commit f8e8c76fbfe2a1b5b9b58add57edb24443d3c660
Author: Matthieu Longo <matthieu.longo@arm.com>
Date:   Mon Jun 8 11:27:44 2026 +0100

     gdb_py_tp_name: remove redundant PyErr_Clear

     gdbpy_print_stack() already clears the error indicator.
     Thus remove the following call to PyErr_Clear().

diff --git a/gdb/python/py-obj-type.c b/gdb/python/py-obj-type.c
index 3ed57d0cbab..bd27b255679 100644
--- a/gdb/python/py-obj-type.c
+++ b/gdb/python/py-obj-type.c
@@ -31,7 +31,6 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept
    auto handle_err = [&]() -> std::string
    {
      gdbpy_print_stack ();
-    PyErr_Clear ();
      return NO_TYPE_NAME;
    };

Re: [PATCH v1] gdb/python: fix memory leak in gdb_py_tp_name

[Andrew Burgess]

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

> On 05/06/2026 21:49, Tom Tromey wrote:
>>>>>>> "Matthieu" == Matthieu Longo <matthieu.longo@arm.com> writes:
>> 
>> Matthieu> +  /* This helper should be used for cases when the called CPython function
>> Matthieu> +     informs the caller that an error occurred, and a Python error was set.  */
>> Matthieu> +  auto handle_err = [&]() -> std::string
>> Matthieu> +  {
>> Matthieu> +    gdbpy_print_stack ();
>> Matthieu> +    PyErr_Clear ();
>> Matthieu> +    return NO_TYPE_NAME;
>> 
>> The PyErr_Clear is redundant as gdbpy_print_stack guarantees that no
>> error is set when it returns.
>> 
>> Tom
>
> Since I already merged the previous patch, can I apply this obvious
> fix ?

Yes please.  Sorry for not spotting this in my previous review.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew


>
> Matthieu
>
> commit f8e8c76fbfe2a1b5b9b58add57edb24443d3c660
> Author: Matthieu Longo <matthieu.longo@arm.com>
> Date:   Mon Jun 8 11:27:44 2026 +0100
>
>      gdb_py_tp_name: remove redundant PyErr_Clear
>
>      gdbpy_print_stack() already clears the error indicator.
>      Thus remove the following call to PyErr_Clear().
>
> diff --git a/gdb/python/py-obj-type.c b/gdb/python/py-obj-type.c
> index 3ed57d0cbab..bd27b255679 100644
> --- a/gdb/python/py-obj-type.c
> +++ b/gdb/python/py-obj-type.c
> @@ -31,7 +31,6 @@ gdb_py_tp_name (PyTypeObject *py_type) noexcept
>     auto handle_err = [&]() -> std::string
>     {
>       gdbpy_print_stack ();
> -    PyErr_Clear ();
>       return NO_TYPE_NAME;
>     };

Re: [PATCH v1] gdb/python: fix memory leak in gdb_py_tp_name

[Tom de Vries]

On 5/26/26 6:04 PM, Matthieu Longo wrote:
> @@ -1067,7 +1067,7 @@ 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 &tp_name = gdbpy_py_obj_tp_name (self);
>   
>     const auto bp = (struct gdbpy_breakpoint_object*) self;
>     if (bp->bp == nullptr)
> @@ -1083,7 +1083,7 @@ bppy_repr (PyObject *self)
>     str.pop_back ();
>   
>     return PyUnicode_FromFormat ("<%s%s number=%d hits=%d%s>",
> -			       tp_name,
> +			       tp_name.c_str (),
>   			       (bp->bp->enable_state == bp_enabled
>   				? "" : " disabled"), bp->bp->number,
>   			       bp->bp->hit_count, str.c_str ());

This updates the second use of tp_name, but not the first, which caused 
a build breaker with clang.  I've submitted a fix ( 
https://sourceware.org/pipermail/gdb-patches/2026-June/227823.html ).

Thanks,
- Tom