From dff7f6c72d05d8a378dc346e6900ba7c4eba759f Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Tue, 22 Apr 2025 21:31:02 +0100 Subject: gdb/python: stop using PyObject_IsInstance in py-disasm.c The PyObject_IsInstance function can return -1 for errors, 0 to indicate false, and 1 to indicate true. I noticed in python/py-disasm.c that we treat the result of PyObject_IsInstance as a bool. This means that if PyObject_IsInstance returns -1, then this will be treated as true. The consequence of this is that we will invoke undefined behaviour by treating the result from the _print_insn call as if it was a DisassemblerResult object, even though PyObject_IsInstance raised an error, and the result might not be of the required type. I could fix this by taking the -1 result into account, however, gdb.DisassemblerResult cannot be sub-classed, the type doesn't have the Py_TPFLAGS_BASETYPE flag. As such, we can switch to using PyObject_TypeCheck instead, which only return 0 or 1, with no error case. I have also taken the opportunity to improve the error message emitted if the result has the wrong type. Better error message make debugging issues easier. I've added a test which exposes the problem when using PyObject_IsInstance, and I've updated the existing test for the improved error message. Approved-By: Tom Tromey --- gdb/testsuite/gdb.python/py-disasm.exp.tcl | 5 ++++- gdb/testsuite/gdb.python/py-disasm.py | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) (limited to 'gdb/testsuite/gdb.python') diff --git a/gdb/testsuite/gdb.python/py-disasm.exp.tcl b/gdb/testsuite/gdb.python/py-disasm.exp.tcl index 938326d..c5099ba 100644 --- a/gdb/testsuite/gdb.python/py-disasm.exp.tcl +++ b/gdb/testsuite/gdb.python/py-disasm.exp.tcl @@ -152,7 +152,10 @@ set test_plans \ "Buffer returned from read_memory is sized $decimal instead of the expected $decimal"]] \ [list "ResultOfWrongType" \ [make_exception_pattern "TypeError" \ - "Result is not a DisassemblerResult."]] \ + "Result from Disassembler must be gdb.DisassemblerResult, not Blah."]] \ + [list "ResultOfVeryWrongType" \ + [make_exception_pattern "TypeError" \ + "Result from Disassembler must be gdb.DisassemblerResult, not Blah."]] \ [list "ErrorCreatingTextPart_NoArgs" \ [make_exception_pattern "TypeError" \ [missing_arg_pattern "style" 1]]] \ diff --git a/gdb/testsuite/gdb.python/py-disasm.py b/gdb/testsuite/gdb.python/py-disasm.py index 32d6aa7..9761337 100644 --- a/gdb/testsuite/gdb.python/py-disasm.py +++ b/gdb/testsuite/gdb.python/py-disasm.py @@ -294,6 +294,24 @@ class ResultOfWrongType(TestDisassembler): return self.Blah(1, "ABC") +class ResultOfVeryWrongType(TestDisassembler): + """Return something that is not a DisassemblerResult from disassemble + method. The thing returned will raise an exception if used in an + isinstance() call, or in PyObject_IsInstance from C++. + """ + + class Blah: + def __init__(self): + pass + + @property + def __class__(self): + raise RuntimeError("error from __class__ in Blah") + + def disassemble(self, info): + return self.Blah() + + class TaggingDisassembler(TestDisassembler): """A simple disassembler that just tags the output.""" -- cgit v1.1