From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id vmvGBUA98mKAoyMAWB0awg (envelope-from ) for ; Tue, 09 Aug 2022 06:56:00 -0400 Received: by simark.ca (Postfix, from userid 112) id 057511E5EA; Tue, 9 Aug 2022 06:56:00 -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=wL2TWT3A; 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 70E7E1E222 for ; Tue, 9 Aug 2022 06:55:59 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 009CB385734F for ; Tue, 9 Aug 2022 10:55:58 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 009CB385734F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1660042558; bh=5nueknnuw/l/44qmsqIiWEk0ObRKPaaaAAHHylgB/do=; 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=wL2TWT3ApdjIlU+5Kz3K3yRiJeFe5oempKx6uKrQnARiVkgRM/6xDIkKF5FlNPmtg b/OM/2ZG6K5eJByObvICtY4Iz7tRMyKBoeZDGfAXgBMaL2061qu99iPG142dqBN7i7 AGkAk08qsJFJdSsGiJTvwuZCuseGyGQbqM3L/P3Y= Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 534F93858C00 for ; Tue, 9 Aug 2022 10:55:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 534F93858C00 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-out1.suse.de (Postfix) with ESMTPS id 8404434887; Tue, 9 Aug 2022 10:55:37 +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 691EF13AA1; Tue, 9 Aug 2022 10:55:37 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id s4aMGCk98mLXYAAAMHmgww (envelope-from ); Tue, 09 Aug 2022 10:55:37 +0000 Message-ID: <3956798c-7ab8-90f5-e0fb-cfc7d9d00834@suse.de> Date: Tue, 9 Aug 2022 12:55:37 +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 2/3] gdb/varobj: Reset varobj after relocations have been computed Content-Language: en-US To: Lancelot SIX , gdb-patches@sourceware.org References: <20220804130231.2126565-1-lancelot.six@amd.com> <20220804130231.2126565-3-lancelot.six@amd.com> In-Reply-To: <20220804130231.2126565-3-lancelot.six@amd.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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: > From: Tom de Vries > > [This patch is a followup to the discussion in > https://sourceware.org/pipermail/gdb-patches/2022-August/191188.html] > > PR/29426 shows failures when running the gdb.mi/mi-var-invalidate-shlib > test when using a compiler which does not produce a PIE executable by > default. > > In the testcase, a varobj is created to track a global variable, and > then the main binary is reloaded in GDB (using the file command). > > During the load of the new binary, GDB tries to recreate the varobj to > track the global in the new binary (varobj_invalidate_iter). At this > point, the old process is still in flight. So when we try to access to > the value of the global, in a PIE executable we only have access to the > unrelocated address (the objfile's text_section_offset () is 0). As a > consequence down the line read_value_memory fails to read the unrelated > address, so cannot evaluate the value of the global. Note that the > expression used to access to the global’s value is valid, so the varobj > can be created. When using a non PIE executable, the address of the > global GDB knows about at this point does not need relocation, so > read_value_memory can access the (old binary’s) value. > > So at this point, in the case of a non-PIE executable the value field is > set, while it is cleared in the case of PIE executable. Later when the > test issues a "-var-update global_var", the command sees no change in > the case of the non-PIE executable, while in the case of the PIE > executable install_new_value sees that value changes, leading to a > different output. > > This patch makes sure that, as we do for breakpoints, we wait until > relocation has happened before we try to recreate varobjs. This way we > have a consistent behavior between PIE and non-PIE binaries. > > Tested on x86_64-linux. > LGTM. Thanks, - Tom > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29426 > Co-authored-by: Lancelot SIX > --- > gdb/symfile.c | 6 ++--- > .../gdb.mi/mi-var-invalidate-shlib.exp | 2 +- > gdb/varobj.c | 22 +++++++------------ > gdb/varobj.h | 5 ++++- > 4 files changed, 16 insertions(+), 19 deletions(-) > > diff --git a/gdb/symfile.c b/gdb/symfile.c > index aea8c762ee6..f61235dd271 100644 > --- a/gdb/symfile.c > +++ b/gdb/symfile.c > @@ -1656,6 +1656,9 @@ symbol_file_command (const char *args, int from_tty) > > /* Now it's safe to re-add the breakpoints. */ > breakpoint_re_set (); > + > + /* Also, it's safe to re-add varobjs. */ > + varobj_re_set (); > } > } > > @@ -2870,9 +2873,6 @@ clear_symtab_users (symfile_add_flags add_flags) > clear_pc_function_cache (); > gdb::observers::new_objfile.notify (NULL); > > - /* Varobj may refer to old symbols, perform a cleanup. */ > - varobj_invalidate (); > - > /* Now that the various caches have been cleared, we can re_set > our breakpoints without risking it using stale data. */ > if ((add_flags & SYMFILE_DEFER_BP_RESET) == 0) > diff --git a/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp b/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp > index 9ba5af0c028..fdf80b79741 100644 > --- a/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp > +++ b/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp > @@ -99,7 +99,7 @@ proc do_test { separate_debuginfo } { > # When reloading the symbol file, only the var for the global in the main > # executable is re-created. > mi_gdb_test "-var-update global_var" \ > - "\\^done,changelist=\\\[{name=\"global_var\",in_scope=\"true\",type_changed=\"false\",has_more=\"0\"}\\\]" \ > + "\\^done,changelist=\\\[\\\]" \ > "global_var recreated" > mi_gdb_test "-var-update global_shlib_var" \ > "\\^done,changelist=\\\[{name=\"global_shlib_var\",in_scope=\"invalid\",has_more=\"0\"}\\\]" \ > diff --git a/gdb/varobj.c b/gdb/varobj.c > index a142bb02e03..55a7bd97f43 100644 > --- a/gdb/varobj.c > +++ b/gdb/varobj.c > @@ -2353,18 +2353,14 @@ all_root_varobjs (gdb::function_view func) > } > } > > -/* Invalidate varobj VAR if it is tied to locals and re-create it if it is > - defined on globals. It is a helper for varobj_invalidate. > - > - This function is called after changing the symbol file, in this case the > - pointers to "struct type" stored by the varobj are no longer valid. All > - varobj must be either re-evaluated, or marked as invalid here. */ > +/* Try to recreate the varobj VAR if it is a global or floating. This is a > + helper function for varobj_re_set. */ > > static void > -varobj_invalidate_iter (struct varobj *var) > +varobj_re_set_iter (struct varobj *var) > { > - /* global and floating var must be re-evaluated. */ > - if (var->root->floating || var->root->global) > + /* Invalidated globals and floating var must be re-evaluated. */ > + if (var->root->global || var->root->floating) > { > struct varobj *tmp_var; > > @@ -2389,14 +2385,12 @@ varobj_invalidate_iter (struct varobj *var) > } > } > > -/* Invalidate the varobjs that are tied to locals and re-create the ones that > - are defined on globals. > - Invalidated varobjs will be always printed in_scope="invalid". */ > +/* See varobj.h. */ > > void > -varobj_invalidate (void) > +varobj_re_set (void) > { > - all_root_varobjs (varobj_invalidate_iter); > + all_root_varobjs (varobj_re_set_iter); > } > > /* Ensure that no varobj keep references to OBJFILE. */ > diff --git a/gdb/varobj.h b/gdb/varobj.h > index 073da60d772..0c92d67e4f4 100644 > --- a/gdb/varobj.h > +++ b/gdb/varobj.h > @@ -314,7 +314,10 @@ extern void all_root_varobjs (gdb::function_view); > extern std::vector > varobj_update (struct varobj **varp, bool is_explicit); > > -extern void varobj_invalidate (void); > +/* Try to recreate any global or floating varobj. This is called after > + changing symbol files. */ > + > +extern void varobj_re_set (void); > > extern bool varobj_editable_p (const struct varobj *var); >