From 8e2ae00bb4d92e385f78d9a2be192de46d21b242 Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Tue, 22 Apr 2025 23:01:58 +0100 Subject: gdb/python: don't use PyObject_IsInstance in py-unwind.c I've been reviewing all uses of PyObject_IsInstance, and I believe that the use of PyObject_IsInstance in py-unwind.c is not entirely correct. The use of PyObject_IsInstance is in this code in frame_unwind_python::sniff: if (PyObject_IsInstance (pyo_unwind_info, (PyObject *) &unwind_info_object_type) <= 0) error (_("A Unwinder should return gdb.UnwindInfo instance.")); The problem is that PyObject_IsInstance can return -1 to indicate an error, in which case a Python error will have been set. Now, the above code appears to handle this case, it checks for '<= 0', however, frame_unwind_python::sniff has this near the start: gdbpy_enter enter_py (gdbarch); And looking in python.c at 'gdbpy_enter::~gdbpy_enter ()', you'll notice that if an error is set then the error is printed, but also, we get a warning about an unhandled Python exception. Clearly, all exceptions should have been handled by the time the gdbpy_enter destructor is called. I've added a test as part of this commit that exposes this problem, the current output is: (gdb) backtrace Python Exception : error in Blah.__class__ warning: internal error: Unhandled Python exception Python Exception : A Unwinder should return gdb.UnwindInfo instance. #0 corrupt_frame_inner () at /home/andrew/projects/binutils-gdb/build.dev-g/gdb/testsuite/../../../src.dev-g/gdb/test> (gdb) An additional observation is that we use PyObject_IsInstance to check that the return value is a gdb.UnwindInfo, or a sub-class. However, gdb.UnwindInfo lacks the Py_TPFLAGS_BASETYPE flag, and so cannot be sub-classed. As such, PyObject_IsInstance is not really needed, we could use PyObject_TypeCheck instead. The PyObject_TypeCheck function only returns 0 or 1, there is no -1 error case. Switching to PyObject_TypeCheck then, fixes the above problem. There's a new test that exposes the problems that originally existed. Approved-By: Tom Tromey --- gdb/python/py-unwind.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'gdb/python') diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c index ab34971..d43d7e9 100644 --- a/gdb/python/py-unwind.c +++ b/gdb/python/py-unwind.c @@ -929,9 +929,9 @@ frame_unwind_python::sniff (const frame_info_ptr &this_frame, /* Received UnwindInfo, cache data. */ PyObject *pyo_unwind_info = PyTuple_GET_ITEM (pyo_execute_ret.get (), 0); - if (PyObject_IsInstance (pyo_unwind_info, - (PyObject *) &unwind_info_object_type) <= 0) - error (_("A Unwinder should return gdb.UnwindInfo instance.")); + if (!PyObject_TypeCheck (pyo_unwind_info, &unwind_info_object_type)) + error (_("an Unwinder should return gdb.UnwindInfo, not %s."), + Py_TYPE (pyo_unwind_info)->tp_name); { unwind_info_object *unwind_info = -- cgit v1.1