From: Andrew Burgess Date: Thu, 9 Mar 2023 10:58:54 +0000 (+0000) Subject: gdb/python: add PENDING_FRAMEPY_REQUIRE_VALID macro in py-unwind.c X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=44d9b0a174b08f283001f01aaf84102ba0a2726a;p=binutils-gdb.git gdb/python: add PENDING_FRAMEPY_REQUIRE_VALID macro in py-unwind.c This commit copies the pattern that is present in many other py-*.c files: having a single macro to check that the Python object is still valid. This cleans up the code a little throughout the py-unwind.c file. Some of the exception messages will change slightly with this commit, though the type of the exceptions is still ValueError in all cases. I started writing some tests for this change and immediately ran into a problem: GDB would crash. It turns out that the PendingFrame objects are not being marked as invalid! In pyuw_sniffer where the pending frames are created, we make use of a scoped_restore to invalidate the pending frame objects. However, this only restores the pending_frame_object::frame_info field to its previous value -- and it turns out we never actually give this field an initial value, it's left undefined. So, when the scoped_restore (called invalidate_frame) performs its cleanup, it actually restores the frame_info field to an undefined value. If this undefined value is not nullptr then any future accesses to the PendingFrame object result in undefined behaviour and most likely, a crash. As part of this commit I now initialize the frame_info field, which ensures all the new tests now pass. Reviewed-By: Tom Tromey --- diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c index ee776bf8dea..12b14616363 100644 --- a/gdb/python/py-unwind.c +++ b/gdb/python/py-unwind.c @@ -52,6 +52,17 @@ show_pyuw_debug (struct ui_file *file, int from_tty, #define PYUW_SCOPED_DEBUG_ENTER_EXIT \ scoped_debug_enter_exit (pyuw_debug, "py-unwind") +/* Require a valid pending frame. */ +#define PENDING_FRAMEPY_REQUIRE_VALID(pending_frame) \ + do { \ + if ((pending_frame)->frame_info == nullptr) \ + { \ + PyErr_SetString (PyExc_ValueError, \ + _("gdb.PendingFrame is invalid.")); \ + return nullptr; \ + } \ + } while (0) + struct pending_frame_object { PyObject_HEAD @@ -215,21 +226,20 @@ unwind_infopy_str (PyObject *self) } /* Create UnwindInfo instance for given PendingFrame and frame ID. - Sets Python error and returns NULL on error. */ + Sets Python error and returns NULL on error. + + The PYO_PENDING_FRAME object must be valid. */ static PyObject * pyuw_create_unwind_info (PyObject *pyo_pending_frame, struct frame_id frame_id) { + gdb_assert (((pending_frame_object *) pyo_pending_frame)->frame_info + != nullptr); + unwind_info_object *unwind_info - = PyObject_New (unwind_info_object, &unwind_info_object_type); + = PyObject_New (unwind_info_object, &unwind_info_object_type); - if (((pending_frame_object *) pyo_pending_frame)->frame_info == NULL) - { - PyErr_SetString (PyExc_ValueError, - "Attempting to use stale PendingFrame"); - return NULL; - } unwind_info->frame_id = frame_id; Py_INCREF (pyo_pending_frame); unwind_info->pending_frame = pyo_pending_frame; @@ -365,15 +375,11 @@ static PyObject * pending_framepy_read_register (PyObject *self, PyObject *args) { pending_frame_object *pending_frame = (pending_frame_object *) self; + PENDING_FRAMEPY_REQUIRE_VALID (pending_frame); + int regnum; PyObject *pyo_reg_id; - if (pending_frame->frame_info == NULL) - { - PyErr_SetString (PyExc_ValueError, - "Attempting to read register from stale PendingFrame"); - return NULL; - } if (!PyArg_UnpackTuple (args, "read_register", 1, 1, &pyo_reg_id)) return NULL; if (!gdbpy_parse_register_id (pending_frame->gdbarch, pyo_reg_id, ®num)) @@ -417,6 +423,8 @@ pending_framepy_create_unwind_info (PyObject *self, PyObject *args) CORE_ADDR pc; CORE_ADDR special; + PENDING_FRAMEPY_REQUIRE_VALID ((pending_frame_object *) self); + if (!PyArg_ParseTuple (args, "O:create_unwind_info", &pyo_frame_id)) return NULL; if (!pyuw_object_attribute_to_pointer (pyo_frame_id, "sp", &sp)) @@ -451,12 +459,8 @@ pending_framepy_architecture (PyObject *self, PyObject *args) { pending_frame_object *pending_frame = (pending_frame_object *) self; - if (pending_frame->frame_info == NULL) - { - PyErr_SetString (PyExc_ValueError, - "Attempting to read register from stale PendingFrame"); - return NULL; - } + PENDING_FRAMEPY_REQUIRE_VALID (pending_frame); + return gdbarch_to_arch_object (pending_frame->gdbarch); } @@ -467,12 +471,8 @@ pending_framepy_level (PyObject *self, PyObject *args) { pending_frame_object *pending_frame = (pending_frame_object *) self; - if (pending_frame->frame_info == NULL) - { - PyErr_SetString (PyExc_ValueError, - "Attempting to read stack level from stale PendingFrame"); - return NULL; - } + PENDING_FRAMEPY_REQUIRE_VALID (pending_frame); + int level = frame_relative_level (pending_frame->frame_info); return gdb_py_object_from_longest (level).release (); } @@ -538,6 +538,7 @@ pyuw_sniffer (const struct frame_unwind *self, frame_info_ptr this_frame, return 0; } pfo->gdbarch = gdbarch; + pfo->frame_info = nullptr; scoped_restore invalidate_frame = make_scoped_restore (&pfo->frame_info, this_frame); diff --git a/gdb/testsuite/gdb.python/py-unwind.exp b/gdb/testsuite/gdb.python/py-unwind.exp index 337e5dc2504..3e214ee0f45 100644 --- a/gdb/testsuite/gdb.python/py-unwind.exp +++ b/gdb/testsuite/gdb.python/py-unwind.exp @@ -142,3 +142,22 @@ gdb_test "python obj = simple_unwinder(True)" \ [multi_line \ "TypeError: incorrect type for name: " \ "Error while executing Python code\\."] + +# Now register the simple_unwinder with a valid name, and use the +# unwinder to capture a PendingFrame object. +gdb_test_no_output "python obj = simple_unwinder(\"simple\")" +gdb_test_no_output "python gdb.unwinder.register_unwinder(None, obj)" +check_for_broken_backtrace "backtrace to capture a PendingFrame object" + +# Call methods on the captured gdb.PendingFrame and check we see the +# expected error. +gdb_test_no_output "python pf = captured_pending_frame" +foreach cmd {"pf.read_register(\"pc\")" \ + "pf.create_unwind_info(None)" \ + "pf.architecture()" \ + "pf.level()"} { + gdb_test "python $cmd" \ + [multi_line \ + "ValueError: gdb\\.PendingFrame is invalid\\." \ + "Error while executing Python code\\."] +} diff --git a/gdb/testsuite/gdb.python/py-unwind.py b/gdb/testsuite/gdb.python/py-unwind.py index edd2e30eb9b..b30e843e7e5 100644 --- a/gdb/testsuite/gdb.python/py-unwind.py +++ b/gdb/testsuite/gdb.python/py-unwind.py @@ -133,12 +133,19 @@ class TestUnwinder(Unwinder): global_test_unwinder = TestUnwinder() gdb.unwinder.register_unwinder(None, global_test_unwinder, True) +# This is filled in by the simple_unwinder class. +captured_pending_frame = None + class simple_unwinder(Unwinder): def __init__(self, name): super().__init__(name) def __call__(self, pending_frame): + global captured_pending_frame + + if captured_pending_frame is None: + captured_pending_frame = pending_frame return None