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.133.124]) by sourceware.org (Postfix) with ESMTPS id F22883858C2C for ; Wed, 23 Mar 2022 19:09:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org F22883858C2C Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-86-i4U9hfmDOtyu0ot4ptIAlw-1; Wed, 23 Mar 2022 15:09:45 -0400 X-MC-Unique: i4U9hfmDOtyu0ot4ptIAlw-1 Received: by mail-wr1-f70.google.com with SMTP id p18-20020adfba92000000b001e8f7697cc7so807781wrg.20 for ; Wed, 23 Mar 2022 12:09:45 -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:subject:in-reply-to:references:date :message-id:mime-version:content-transfer-encoding; bh=b4U4jtAtpyzI16TKI+ixWU8FsB6lU8z4a+Keeao8orQ=; b=wG/rh7FU7/es/Ttd1q8B1dPb8a1kZvAp2ct4eBOE9GB6fnpGrFoEjLjK+hzBVhCSMJ z2uRQuhN9Ee4mvFudEnJVlHh8iQa6jhkjN9f577KWb7CVV9mg0bkq3cRrBDnBl9UEr/Q b/iI0jBS4/6OO4GJTZS32+eH5mKvcKPiVoPec6z/1bjeTWU5uxK/W3tn4HrSLfyYVGqp h8rDaDbAT+Y1QaNdcAI5bT/swJ4vnEgowM7cvouyTcGSLkBh+OvYvihbyACDsBSXBAVP Ar+35OZhAIvGwgEvh4dvSgkiFpvMVhyo7HaWXtgo9PQlUvo4F1HXht8sgIIJlzzO636l Mo8A== X-Gm-Message-State: AOAM531UQRqIhNjxBGmRAEBU2Y61MpTmthroHHC81toS0OP4TChHuasu 2Lf3/cs0Psc8ARRj+mImIqU9oFl48+f2sO0ub/TOfu7f6H6pzTNZdI7CSazk09WX3AgfDNZmoy7 OHY7NtebiCfgW03r1BBWg4w== X-Received: by 2002:a1c:c907:0:b0:37b:f983:5d4e with SMTP id f7-20020a1cc907000000b0037bf9835d4emr1480073wmb.174.1648062584489; Wed, 23 Mar 2022 12:09:44 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzoPzaS/DFdiMxTkR8L4dygCIr5k1/w5gUVn80ugk0F4JRd5pNCZDA3Hkxzlc0xnrnUbF814w== X-Received: by 2002:a1c:c907:0:b0:37b:f983:5d4e with SMTP id f7-20020a1cc907000000b0037bf9835d4emr1480060wmb.174.1648062584231; Wed, 23 Mar 2022 12:09:44 -0700 (PDT) Received: from localhost (host109-158-45-15.range109-158.btcentralplus.com. [109.158.45.15]) by smtp.gmail.com with ESMTPSA id u8-20020a5d4348000000b00203dbfa4ff2sm570614wrr.34.2022.03.23.12.09.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 23 Mar 2022 12:09:43 -0700 (PDT) From: Andrew Burgess To: Jan Vrany , Simon Marchi , Simon Marchi , gdb-patches@sourceware.org Subject: Re: [pushed] gdb/mi: consistently notify user when GDB/MI client uses -thread-select In-Reply-To: References: <20220316150914.1254897-1-jan.vrany@labware.com> <78570a7f-f6c0-5e1b-4178-c9ce401feab7@simark.ca> <897bd508133687e4d30994deecdd87786d71334d.camel@labware.com> <0bcb4286c01054c30c3539c76bd6426ff3b333eb.camel@labware.com> <14d27757ce39bcc7a19f74508e689acfa7fa33c2.camel@labware.com> <82992b26-8010-d7cd-8c27-5e07350d43ab@polymtl.ca> Date: Wed, 23 Mar 2022 19:09:42 +0000 Message-ID: <87fsn8scy1.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-12.3 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_H5, 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: Wed, 23 Mar 2022 19:09:50 -0000 Jan Vrany via Gdb-patches writes: > On Wed, 2022-03-23 at 11:16 -0400, Simon Marchi wrote: >> > I gave it a go but I'm sorry: everything seem to work as expected=20 >> > here. I do not get "[Switching to thread 1 (process 1891548)]" message >> > on CLI when -thread-select already selected thread.=20 >>=20 >> Ok, strange. >>=20 >> > I must be doing something differently. What commit is failing for you? >> > What's your ./configure incantation? >>=20 >> My configure arguments are: >>=20 >> 'CC=3Dccache gcc-11' 'CXX=3Dccache g++-11' '--disable-binutils' '--disab= le-gold' '--disable-ld' '--disable-gprof' '--disable-gas' '--with-guile' 'C= FLAGS=3D-g3 -O0 >> -fdiagnostics-color=3Dalways -fmax-errors=3D1 -fsanitize=3Daddress' 'CXX= FLAGS=3D-std=3Dc++11 -g3 -O0 -fdiagnostics-color=3Dalways -fmax-errors=3D1 = -fsanitize=3Daddress - >> D_GLIBCXX_DEBUG=3D1 -D_GLIBCXX_DEBUG_PEDANTIC=3D1 -D_GLIBCXX_SANITIZE_VE= CTOR=3D1' 'LDFLAGS=3D-fuse-ld=3Dlld -fsanitize=3Daddress' '--prefix=3D/usr'= '--enable-ubsan' '-- >> with-python=3Dpython3' '--with-debuginfod' '--enable-silent-rules' '--en= able-werror' >>=20 > > Thanks! Now it is failing for me too!=20 > I just recompiled GDB using the above command. I'll have a=C2=A0 > look tomorrow.=20 These failures are totally my fault. Sorry! The patch below should fix the problem. Let me know what you think. Thanks, Andrew --- commit fcac0d17e64a18e0600372c2bed51f9e2d9e9d6a 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 --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index abd033b22ae..4f32bb58939 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -1974,27 +1974,40 @@ 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 objec= t was created. */ bool has_changed () const { + /* 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); + + /* If we end up trying to compare two invalid frame-id's then these + will always report themselves as not equal. However, if we are at + the top-most level of the stack then we don't want to consider this + as a frame change. */ return ((m_previous_ptid !=3D null_ptid =09 && inferior_ptid !=3D null_ptid =09 && m_previous_ptid !=3D inferior_ptid) -=09 || m_previous_frame !=3D deprecated_safe_get_selected_frame ()); +=09 || current_frame_level !=3D m_previous_frame_level +=09 || (current_frame_level !=3D -1 +=09=09&& !frame_id_eq (current_frame_id, m_previous_frame_id))); } 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. The frame_id might be null_frame_id + if no frame is currently selected. */ + frame_id m_previous_frame_id; + int m_previous_frame_level; }; =20 static void