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 <class 'RuntimeError'>: error in Blah.__class__
  warning: internal error: Unhandled Python exception
  Python Exception <class 'gdb.error'>: 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 <tom@tromey.com>

3 files changed, 30 insertions, 3 deletions

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 =
diff --git a/gdb/testsuite/gdb.python/py-unwind.exp b/gdb/testsuite/gdb.python/py-unwind.exp
index 80eac28..b416c2f 100644
--- a/gdb/testsuite/gdb.python/py-unwind.exp
+++ b/gdb/testsuite/gdb.python/py-unwind.exp
@@ -245,6 +245,13 @@ with_test_prefix "frame-id 'pc' is invalid" {
 	"Python Exception <class 'ValueError'>: invalid literal for int\\(\\) with base 10: 'xyz'\r\n.*"
 }
 
+with_test_prefix "bad object unwinder" {
+    gdb_test_no_output "python obj = bad_object_unwinder(\"bad-object\")"
+    gdb_test_no_output "python gdb.unwinder.register_unwinder(None, obj, replace=True)"
+    gdb_test "backtrace" \
+	"Python Exception <class 'gdb.error'>: an Unwinder should return gdb.UnwindInfo, not Blah\\.\r\n.*"
+}
+
 # Gather information about every frame.
 gdb_test_no_output "python capture_all_frame_information()"
 gdb_test_no_output "python gdb.newest_frame().select()"
diff --git a/gdb/testsuite/gdb.python/py-unwind.py b/gdb/testsuite/gdb.python/py-unwind.py
index 8e65a1a..0faccf2 100644
--- a/gdb/testsuite/gdb.python/py-unwind.py
+++ b/gdb/testsuite/gdb.python/py-unwind.py
@@ -267,4 +267,24 @@ class validating_unwinder(Unwinder):
         return None
 
 
+class bad_object_unwinder(Unwinder):
+    def __init__(self, name):
+        super().__init__(name)
+
+    def __call__(self, pending_frame):
+
+        if pending_frame.level() != 1:
+            return None
+
+        class Blah:
+            def __init__(self):
+                pass
+
+            @property
+            def __class__(self):
+                raise RuntimeError("error in Blah.__class__")
+
+        return Blah()
+
+
 print("Python script imported")