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 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