mirror of
https://github.com/espressif/binutils-gdb.git
synced 2025-12-19 01:19:41 +08:00
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 <tom@tromey.com>
This commit is contained in:
@@ -1311,12 +1311,13 @@ gdbpy_print_insn (struct gdbarch *gdbarch, CORE_ADDR memaddr,
|
||||
return {};
|
||||
}
|
||||
|
||||
/* Check the result is a DisassemblerResult (or a sub-class). */
|
||||
if (!PyObject_IsInstance (result.get (),
|
||||
(PyObject *) &disasm_result_object_type))
|
||||
/* Check the result is a DisassemblerResult. */
|
||||
if (!PyObject_TypeCheck (result.get (), &disasm_result_object_type))
|
||||
{
|
||||
PyErr_SetString (PyExc_TypeError,
|
||||
_("Result is not a DisassemblerResult."));
|
||||
PyErr_Format
|
||||
(PyExc_TypeError,
|
||||
_("Result from Disassembler must be gdb.DisassemblerResult, not %s."),
|
||||
Py_TYPE (result.get ())->tp_name);
|
||||
gdbpy_print_stack ();
|
||||
return std::optional<int> (-1);
|
||||
}
|
||||
|
||||
@@ -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]]] \
|
||||
|
||||
@@ -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."""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user