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 B6D383853804 for ; Wed, 23 Mar 2022 19:11:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B6D383853804 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-601-gVLxgbPLPR-oI3oMnTYJ9w-1; Wed, 23 Mar 2022 15:11:31 -0400 X-MC-Unique: gVLxgbPLPR-oI3oMnTYJ9w-1 Received: by mail-wr1-f70.google.com with SMTP id p16-20020adfc390000000b00204006989c2so819835wrf.5 for ; Wed, 23 Mar 2022 12:11:30 -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=7KxHvEM44zDR2Ui9J3KLB3fvfZ6FB0uJboc2YbjUlVI=; b=0SLL7Sg4WgHgGAKxhKBO2527TWeqPCyB4WND5r5x9MXwnGCjb6v/Gh4Q0QVy9zh/U/ V1dngNaqNobLBzaB6gRaiYFozzEmJwyWAFm5jF4zv3Wt0jJVtJOcjcludYEjKHlz3/RN gQ154RmIxmoVEROyJuS36PCYZJ3gT3wJLu/PtWIKw83qatgoySgintd4IghECo8VPqS9 1QB930eYxEJjNP/5Ab3WIq94BxLFbvM4Ej/RWJbtSrNRZGLSJCwSxZkpYW+ekvsYEa1I lYSNYh3rTiNDp0g691vwSbVOY5yS9Seb8ej9McU7pK1IyqIKItjM7miibQhlyOUvetur Jebw== X-Gm-Message-State: AOAM532OF6oooQ1T5EcrkJqRGOSswfPlEMY45GY95BO/A0qVACu87P03 jysKNRk5KWWrTwuF30ShxVlaZjiu6baQvSw7JR7afAOwH6KTcg+VPiCvH5Xye7jdedbPoy8s0sT CkeXhBeOlDKN8N6mWdjz5eg== X-Received: by 2002:a5d:560e:0:b0:205:7ee9:f4a1 with SMTP id l14-20020a5d560e000000b002057ee9f4a1mr1347745wrv.84.1648062690062; Wed, 23 Mar 2022 12:11:30 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyAT6LsXXD3287KhedDKzkwZ5KhNNKX/SZcbpNGV1cjmZonlAWDANUN/zXkIKvHZlDZOMrtlQ== X-Received: by 2002:a5d:560e:0:b0:205:7ee9:f4a1 with SMTP id l14-20020a5d560e000000b002057ee9f4a1mr1347732wrv.84.1648062689855; Wed, 23 Mar 2022 12:11:29 -0700 (PDT) Received: from localhost (host109-158-45-15.range109-158.btcentralplus.com. [109.158.45.15]) by smtp.gmail.com with ESMTPSA id f18-20020a5d6652000000b001e669ebd528sm561739wrw.91.2022.03.23.12.11.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 23 Mar 2022 12:11:29 -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: <87fsn8scy1.fsf@redhat.com> 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> <87fsn8scy1.fsf@redhat.com> Date: Wed, 23 Mar 2022 19:11:28 +0000 Message-ID: <87czicscv3.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.4 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_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: Wed, 23 Mar 2022 19:11:34 -0000 Andrew Burgess writes: > 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)]" messag= e >>> > 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' '--disa= ble-gold' '--disable-ld' '--disable-gprof' '--disable-gas' '--with-guile' '= CFLAGS=3D-g3 -O0 >>> -fdiagnostics-color=3Dalways -fmax-errors=3D1 -fsanitize=3Daddress' 'CX= XFLAGS=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_V= ECTOR=3D1' 'LDFLAGS=3D-fuse-ld=3Dlld -fsanitize=3Daddress' '--prefix=3D/usr= ' '--enable-ubsan' '-- >>> with-python=3Dpython3' '--with-debuginfod' '--enable-silent-rules' '--e= nable-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. Also, this patch should definitely be back ported to the GDB 12 branch. Thanks, Andrew > > 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 notificatio= ns > =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 -threa= d-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 ha= s > changed, that commit grabs a frame_info pointer before and after an M= I > 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 th= e > 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 obj= ect > 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 a= t > + the top-most level of the stack then we don't want to consider th= is > + 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 w= as > 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