From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lndn.lancelotsix.com (vps-42846194.vps.ovh.net [IPv6:2001:41d0:801:2000::2400]) by sourceware.org (Postfix) with ESMTPS id 732FE385AC30 for ; Thu, 4 Aug 2022 13:02:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 732FE385AC30 Received: from octopus.. (unknown [IPv6:2a01:e0a:535:2750:508f:a0d5:5691:a9a3]) by lndn.lancelotsix.com (Postfix) with ESMTPSA id 1680B89876; Thu, 4 Aug 2022 13:02:39 +0000 (UTC) From: Lancelot SIX To: gdb-patches@sourceware.org Cc: tdevries@suse.de, lsix@lancelotsix.com, Lancelot SIX Subject: [PATCH 2/3] gdb/varobj: Reset varobj after relocations have been computed Date: Thu, 4 Aug 2022 14:02:30 +0100 Message-Id: <20220804130231.2126565-3-lancelot.six@amd.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20220804130231.2126565-1-lancelot.six@amd.com> References: <20220804130231.2126565-1-lancelot.six@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.11 (lndn.lancelotsix.com [0.0.0.0]); Thu, 04 Aug 2022 13:02:40 +0000 (UTC) X-Spam-Status: No, score=-9.7 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_QUARANTINE, KAM_DMARC_STATUS, KHOP_HELO_FCRDNS, RCVD_IN_SBL_CSS, SPF_HELO_NONE, SPF_SOFTFAIL, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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, 04 Aug 2022 13:02:44 -0000 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. 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); -- 2.34.1