From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1726) id 84C793858C50; Tue, 29 Mar 2022 10:09:33 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 84C793858C50 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Andrew Burgess To: gdb-cvs@sourceware.org Subject: [binutils-gdb/gdb-12-branch] gdb/mi: fix use after free of frame_info causing spurious notifications X-Act-Checkin: binutils-gdb X-Git-Author: Andrew Burgess X-Git-Refname: refs/heads/gdb-12-branch X-Git-Oldrev: 7685884a38572266216f12069e86f9c7fcdedc14 X-Git-Newrev: 95c82cbf7e78506e7cc2ec249399ec80bba80ed3 Message-Id: <20220329100933.84C793858C50@sourceware.org> Date: Tue, 29 Mar 2022 10:09:33 +0000 (GMT) X-BeenThere: gdb-cvs@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 29 Mar 2022 10:09:33 -0000 https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D95c82cbf7e78= 506e7cc2ec249399ec80bba80ed3 commit 95c82cbf7e78506e7cc2ec249399ec80bba80ed3 Author: Andrew Burgess Date: Wed Mar 23 19:00:35 2022 +0000 gdb/mi: fix use after free of frame_info causing spurious notifications =20 In commit: =20 commit a2757c4ed693cef4ecc4dcdcb2518353eb6b3c3f Date: Wed Mar 16 15:08:22 2022 +0000 =20 gdb/mi: consistently notify user when GDB/MI client uses -thread-= select =20 Changes were made to GDB to address some inconsistencies in when notifications are sent from a MI terminal to a CLI terminal (when multiple terminals are in use, see new-ui command). =20 Unfortunately, in order to track when the currently selected frame has changed, that commit grabs a frame_info pointer before and after an MI command has executed, and compares the pointers to see if the frame has changed. =20 This is not safe. =20 If the frame cache is deleted for any reason then the frame_info pointer captured before the command started, is no longer valid, and any comparisons based on that pointer are undefined. =20 This was leading to random test failures for some folk, see: =20 https://sourceware.org/pipermail/gdb-patches/2022-March/186867.html =20 This commit changes GDB so we no longer hold frame_info pointers, but instead store the frame_id and frame_level, this is safe even when the frame cache is flushed. Diff: --- gdb/mi/mi-main.c | 44 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index abd033b22ae..91eb8d1edd8 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -1974,27 +1974,51 @@ struct user_selected_context { /* Constructor. */ user_selected_context () - : m_previous_ptid (inferior_ptid), - m_previous_frame (deprecated_safe_get_selected_frame ()) - { /* Nothing. */ } + : m_previous_ptid (inferior_ptid) + { + save_selected_frame (&m_previous_frame_id, &m_previous_frame_level); + } =20 /* Return true if the user selected context has changed since this object was created. */ bool has_changed () const { - return ((m_previous_ptid !=3D null_ptid - && inferior_ptid !=3D null_ptid - && m_previous_ptid !=3D inferior_ptid) - || m_previous_frame !=3D deprecated_safe_get_selected_frame ()); + /* Did the selected thread change? */ + if (m_previous_ptid !=3D null_ptid && inferior_ptid !=3D null_ptid + && m_previous_ptid !=3D inferior_ptid) + return true; + + /* Grab details of the currently selected frame, for comparison. */ + frame_id current_frame_id; + int current_frame_level; + save_selected_frame (¤t_frame_id, ¤t_frame_level); + + /* Did the selected frame level change? */ + if (current_frame_level !=3D m_previous_frame_level) + return true; + + /* Did the selected frame id change? If the innermost frame is + selected then the level will be -1, and the frame-id will be + null_frame_id. As comparing null_frame_id with itself always + reports not-equal, we only do the equality test if we have something + other than the innermost frame selected. */ + if (current_frame_level !=3D -1 + && !frame_id_eq (current_frame_id, m_previous_frame_id)) + return true; + + /* Nothing changed! */ + return false; } private: /* The previously selected thread. This might be null_ptid if there was no previously selected thread. */ ptid_t m_previous_ptid; =20 - /* The previously selected frame. This might be nullptr if there was no - previously selected frame. */ - frame_info *m_previous_frame; + /* The previously selected frame. If the innermost frame is selected, or + no frame is selected, then the frame_id will be null_frame_id, and the + level will be -1. */ + frame_id m_previous_frame_id; + int m_previous_frame_level; }; =20 static void