From patchwork Fri Mar 10 14:55:21 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 66219 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 969673850848 for ; Fri, 10 Mar 2023 14:57:10 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 969673850848 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1678460230; bh=TPpZrz7EKXI7yB3B/uBocCYr3+fL1IyJBVMIUmLjIaY=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=SHJm/9JnCbFyHkos/GZTBK/QNoZiFfstHB7hF4nYMxXndOkAkE/CdpqWpeZTlw7uW 90OjpnOKtSCY5HQxS7p1Q+UyiN0lw60GPgdYSGVCk4MeCwxjKHMctFedkhinxv6JZP fyqBX2wSWwM3nvwDKubYi6P13UR3bGrbVZA+raok= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 99E823858C5E for ; Fri, 10 Mar 2023 14:55:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 99E823858C5E Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-206-TCLsEMVZOkOb93XwAzo8cg-1; Fri, 10 Mar 2023 09:55:57 -0500 X-MC-Unique: TCLsEMVZOkOb93XwAzo8cg-1 Received: by mail-wr1-f69.google.com with SMTP id l5-20020a5d6745000000b002c91cb49949so1104860wrw.14 for ; Fri, 10 Mar 2023 06:55:57 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678460156; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=TPpZrz7EKXI7yB3B/uBocCYr3+fL1IyJBVMIUmLjIaY=; b=tnouVv84zpXLJ+JNx0c3crnPU161JegUjsvc3yQGcO8B8DnpDzHf1Y1lxauN13pnNo 0ekSNA/uqkKex0HYKkuQlcSC+LAiLuDwpkXwtiCRbz7APXvOLAyI6LPaDpWUTahlub41 4Uaj+QPl2Ukji2ecuiKBXxiDIwUgiZvw8ZSNlvs0A6Lr2varlh9TY6ouDj8hNOadmNyQ BNtAV29/VoI3kHR8BteNewTDhLPx4qDURgE4vby99M52ZJsDEAiDmE2EOFzx+g0FdMvc bIHd8isYhcE/84VAmjPh92lyhGvIsfEF5RhZFWDv4B+QdriLe0lu9sG2eNxVws+BrT6K 41cw== X-Gm-Message-State: AO0yUKWw55R/p+WYpoRyl6DO33O3Afhk95ABi6qj4PzKUlqEXsJPYrs2 xtDIC4HclV7n+8ut3vjLNGp65FG45bN/SYp98Bl3bJPbe3YPLV13wWny5Jc+AZYEzfcJeZmeRLW fT0mKoef+L/4C9Fv/1GIkw2+hxR1ITvyQoKLmsGpS2CCWzxQ8IeBjmaqvlQhH9xABoZ07kHkhjy nPukBr6g== X-Received: by 2002:a05:600c:4f02:b0:3eb:39c3:8844 with SMTP id l2-20020a05600c4f0200b003eb39c38844mr3095915wmq.12.1678460155961; Fri, 10 Mar 2023 06:55:55 -0800 (PST) X-Google-Smtp-Source: AK7set9S9YU/HUBsHEMrv71O8y6n6Q+m+kT00YYDPg9obqdGCHWnw5lBofsU9rsKgFrfMecJyramsg== X-Received: by 2002:a05:600c:4f02:b0:3eb:39c3:8844 with SMTP id l2-20020a05600c4f0200b003eb39c38844mr3095902wmq.12.1678460155691; Fri, 10 Mar 2023 06:55:55 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id q20-20020a1cf314000000b003ebff290a52sm106088wmq.28.2023.03.10.06.55.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 10 Mar 2023 06:55:55 -0800 (PST) To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH 04/10] gdb/python: add PENDING_FRAMEPY_REQUIRE_VALID macro in py-unwind.c Date: Fri, 10 Mar 2023 14:55:21 +0000 Message-Id: <9633a6228844ef210896ea73246525b00e131e8e.1678460067.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Andrew Burgess via Gdb-patches From: Andrew Burgess Reply-To: Andrew Burgess Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" 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. --- gdb/python/py-unwind.c | 53 +++++++++++++------------- gdb/testsuite/gdb.python/py-unwind.exp | 19 +++++++++ gdb/testsuite/gdb.python/py-unwind.py | 7 ++++ 3 files changed, 53 insertions(+), 26 deletions(-) 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