From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id GZbVJV0x7WJuSSEAWB0awg (envelope-from ) for ; Fri, 05 Aug 2022 11:03:57 -0400 Received: by simark.ca (Postfix, from userid 112) id 8E2151EA05; Fri, 5 Aug 2022 11:03:57 -0400 (EDT) Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=xsgm5SN6; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,NICE_REPLY_A,RDNS_DYNAMIC, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 17AFB1E9EB for ; Fri, 5 Aug 2022 11:03:57 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 3C4C1385736F for ; Fri, 5 Aug 2022 15:03:56 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 3C4C1385736F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1659711836; bh=cxz78KVYgEn/JpFXw2GbryfU/ypSDw40ANE/PukzEB4=; h=Date:Subject:To:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=xsgm5SN6MnqQxebo4CIVJKXHA00N9IiiFXpemAQdXIGxNqebx0euOjc59HrFvNAfY +Owtfd6RwT3GEXJ2aFomdSr2Be/PsrS+fNdfXnnj8TyVWDT4NvAn1wVOQHOdIX/msM BqMxQSjfquFd1SsVCVMtADtw3XwDjnHXVQbHQ/0g= Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) by sourceware.org (Postfix) with ESMTPS id 01A9E3858C53 for ; Fri, 5 Aug 2022 15:03:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 01A9E3858C53 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id D2D8D20DDD; Fri, 5 Aug 2022 15:03:25 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id AC99913A9C; Fri, 5 Aug 2022 15:03:25 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id GpiqKD0x7WLTVAAAMHmgww (envelope-from ); Fri, 05 Aug 2022 15:03:25 +0000 Message-ID: <8f62af78-0305-65e3-b728-282c310a7cae@suse.de> Date: Fri, 5 Aug 2022 17:03:25 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH 1/3] gdb/varobj: Do not invalidate locals in varobj_invalidate_iter Content-Language: en-US To: Lancelot SIX , gdb-patches@sourceware.org References: <20220804130231.2126565-1-lancelot.six@amd.com> <20220804130231.2126565-2-lancelot.six@amd.com> In-Reply-To: <20220804130231.2126565-2-lancelot.six@amd.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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: , From: Tom de Vries via Gdb-patches Reply-To: Tom de Vries Cc: lsix@lancelotsix.com Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" On 8/4/22 15:02, Lancelot SIX wrote: > The varobj_invalidate_iter function has logic to invalidate any local > varobj it can find. However since bc20e562ec0 "gdb/varobj: Fix use > after free in varobj" all varobj containing references to an objfile are > cleared when the objfile goes out of scope. This means that at this > point any local varobj seen by varobj_invalidate_iter either has > already been invalidated by varobj_invalidate_if_uses_objfile or only > contains valid references and there is no reason to invalidate it. > > This patch proposes to remove this un-necessary invalidation and adds a un-necessary - > unnecessary > testcase which exercises a scenario where a local varobj can legitimately > survive a call to varobj_invalidate_iter. > > At this point the varobj_invalidate and varobj_invalidate seem misnamed Missing _iter on the latter ? > since they deal with re-creating invalid objects and do not do > invalidation, but this will be fixed in a following patch. > > Tested on x86_64-linux. > --- > .../gdb.mi/mi-var-invalidate-shlib.exp | 40 +++++++++++++++++-- > gdb/varobj.c | 2 - > 2 files changed, 36 insertions(+), 6 deletions(-) > > diff --git a/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp b/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp > index 87d1d4a6a9d..9ba5af0c028 100644 > --- a/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp > +++ b/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp > @@ -40,10 +40,6 @@ if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable $opt > } > > proc do_test { separate_debuginfo } { > - if { $separate_debuginfo } { > - gdb_gnu_strip_debug $::shlib_path > - } > - > if { [mi_clean_restart] } { > unsupported "failed to start GDB" > return > @@ -132,6 +128,42 @@ proc do_test { separate_debuginfo } { > } > } > > +proc_with_prefix local_not_invalidated { separate_debuginfo } { > + if { [mi_clean_restart] } { > + unsupported "failed to start GDB" > + return > + } > + > + # Start the process once and create varobjs referencing the loaded objfiles. > + with_test_prefix "setup" { > + mi_load_shlibs $::shlib_path > + if { $separate_debuginfo } { > + mi_load_shlibs ${::shlib_path}.debug > + } > + > + mi_gdb_reinitialize_dir $::srcdir/$::subdir > + mi_gdb_load $::binfile > + > + mi_runto foo -pending > + mi_next "next" > + mi_create_varobj local_var local_var "create local varobj" > + } > + > + # A this point we are stoped in the shared library. If we reload symbols stoped -> stopped. Otherwise, LGTM. Thanks, - Tom > + # for the main binary, symbols for the shared library remain valid. > + # A varobj tracking variables in the scope of the shared library only > + # should not be invalidated. > + mi_gdb_load ${::binfile} > + mi_gdb_test "-var-update local_var" \ > + "\\^done,changelist=\\\[\\\]" \ > + "local_var preserved" > +} > + > foreach_with_prefix separate_debuginfo {0 1} { > + if { $separate_debuginfo } { > + gdb_gnu_strip_debug $::shlib_path > + } > + > do_test $separate_debuginfo > + local_not_invalidated $separate_debuginfo > } > diff --git a/gdb/varobj.c b/gdb/varobj.c > index 0683af1991e..a142bb02e03 100644 > --- a/gdb/varobj.c > +++ b/gdb/varobj.c > @@ -2387,8 +2387,6 @@ varobj_invalidate_iter (struct varobj *var) > var->root->is_valid = false; > } > } > - else /* locals must be invalidated. */ > - var->root->is_valid = false; > } > > /* Invalidate the varobjs that are tied to locals and re-create the ones that