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

[Matthieu Longo <matthieu.longo@arm.com>]

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