From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 8AF9E3858D3C for ; Thu, 24 Mar 2022 18:53:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8AF9E3858D3C Received: from mail-ed1-f72.google.com (mail-ed1-f72.google.com [209.85.208.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-179-MLXZxSDjMpCkv2nhx0em5Q-1; Thu, 24 Mar 2022 14:52:59 -0400 X-MC-Unique: MLXZxSDjMpCkv2nhx0em5Q-1 Received: by mail-ed1-f72.google.com with SMTP id c31-20020a509fa2000000b004190d43d28fso3519086edf.9 for ; Thu, 24 Mar 2022 11:52:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=97yQmkzKsxDct6HhGwZrGC2vUiazft0Qr305C4bsPjo=; b=kHN+RrbckT+ak0WTcSn3YAEdzgfT+GAcye77Rdi7pUOkGSM3HixNqImj7A+8HGt02W zuVw9BiNMY1CWurl26I3wt0Q/qBNanDt49X0TQk3lHcd2Pe/dHWmJZj6CbJXhbextd+M HdIOx/7w5SaNfC8o92dkPJUDpbSRnQSL2bklmkdZqT4KplNQYvJ7dYtBk0/xS/MJoEUO 8cRg7ExCPq+ZemzaYWUDXSKsOtMUhPEJIUVSnU9nOLhBISr37imxfNt5+BTiv/EI+rqn lEwup3khIsLNYFhLo68Lokw3QuxkYX7N1r0yZRPKGeofvcq0NIBqg2cV90IiGC5Ud3xy boYA== X-Gm-Message-State: AOAM5338MpCLYbIU+fD2+0QeZSPIJtHF544L4woitT6HAX20LhYP9PrH 82ilG99XhBUVWwEtCohi9OjjB+LrC0kL5vlVhMvE59HevwTGIzuaG730kE3GsFs407pOSB9Bzzd xNoc3SRLhLJZJZchB4q6ox23PlwkOq6+ODmIkUrayRmx9lPvV4VotmnxnIY7Fmn5xMkM8EKLSjg == X-Received: by 2002:a17:906:9f28:b0:6e0:153d:dc29 with SMTP id fy40-20020a1709069f2800b006e0153ddc29mr7136267ejc.756.1648147978263; Thu, 24 Mar 2022 11:52:58 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxfQQp97T9bvfa3wKWI297nFTnwPW3q/BmEOutHOu4FlGNS/GwXsz3Lz4IpQO4pmOrLQh2Hdg== X-Received: by 2002:a17:906:9f28:b0:6e0:153d:dc29 with SMTP id fy40-20020a1709069f2800b006e0153ddc29mr7136246ejc.756.1648147977971; Thu, 24 Mar 2022 11:52:57 -0700 (PDT) Received: from localhost (92.40.179.86.threembb.co.uk. [92.40.179.86]) by smtp.gmail.com with ESMTPSA id q9-20020a170906770900b006d20acf7e2bsm1473008ejm.200.2022.03.24.11.52.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 24 Mar 2022 11:52:57 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCHv2] gdb/mi: fix use after free of frame_info causing spurious notifications Date: Thu, 24 Mar 2022 18:52:54 +0000 Message-Id: <20220324185254.2582384-1-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 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-10.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_ABUSEAT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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-List-Received-Date: Thu, 24 Mar 2022 18:53:16 -0000 Simon, Thanks for the feedback. I believe this addresses all your points. What do you think? Thanks, Andrew --- In commit: commit a2757c4ed693cef4ecc4dcdcb2518353eb6b3c3f Date: Wed Mar 16 15:08:22 2022 +0000 gdb/mi: consistently notify user when GDB/MI client uses -thread-select 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). 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. This is not safe. 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. This was leading to random test failures for some folk, see: https://sourceware.org/pipermail/gdb-patches/2022-March/186867.html 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. --- gdb/mi/mi-main.c | 43 +++++++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index abd033b22ae..28d4afc28c0 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -1974,27 +1974,50 @@ 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); + } /* Return true if the user selected context has changed since this object was created. */ bool has_changed () const { - return ((m_previous_ptid != null_ptid - && inferior_ptid != null_ptid - && m_previous_ptid != inferior_ptid) - || m_previous_frame != deprecated_safe_get_selected_frame ()); + /* Did the selected thread change? */ + if (m_previous_ptid != null_ptid && inferior_ptid != null_ptid + && m_previous_ptid != 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 != 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 != -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; - /* 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 + 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; }; static void -- 2.25.4