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

[Andrew Burgess <aburgess@redhat.com>]

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.  */