From: Matthieu Longo Date: Tue, 26 May 2026 10:46:29 +0000 (+0100) Subject: gdb/python: fix memory leak in gdb_py_tp_name X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=dafd73bcda60e9a54bdd43c79337dce452b1d8af;p=thirdparty%2Fbinutils-gdb.git gdb/python: fix memory leak in gdb_py_tp_name 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. An 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. Approved-By: Andrew Burgess --- 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 (-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..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 = ""; + + /* 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. */ 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 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; }