From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 7833) id E57523857363; Thu, 11 Aug 2022 14:17:24 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E57523857363 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Lancelot SIX To: gdb-cvs@sourceware.org Subject: [binutils-gdb] gdb/varobj: Reset varobj after relocations have been computed X-Act-Checkin: binutils-gdb X-Git-Author: Tom de Vries X-Git-Refname: refs/heads/master X-Git-Oldrev: 739be95178196df4babdcb47de856a12ba06253f X-Git-Newrev: ccb5e559ef13f1c7a32312199f7887b463c56216 Message-Id: <20220811141724.E57523857363@sourceware.org> Date: Thu, 11 Aug 2022 14:17:24 +0000 (GMT) X-BeenThere: gdb-cvs@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Aug 2022 14:17:25 -0000 https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3Dccb5e559ef13= f1c7a32312199f7887b463c56216 commit ccb5e559ef13f1c7a32312199f7887b463c56216 Author: Tom de Vries Date: Wed Aug 10 22:23:28 2022 +0100 gdb/varobj: Reset varobj after relocations have been computed =20 [This patch is a followup to the discussion in https://sourceware.org/pipermail/gdb-patches/2022-August/191188.html] =20 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. =20 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). =20 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=E2=80=99s value is valid, so th= e 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=E2=80=99s) value. =20 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. =20 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. =20 Tested on x86_64-linux. =20 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=3D29426 Co-authored-by: Lancelot SIX Diff: --- gdb/symfile.c | 6 +++--- gdb/testsuite/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 361274eea45..eb27668f9d3 100644 --- a/gdb/symfile.c +++ b/gdb/symfile.c @@ -1656,6 +1656,9 @@ symbol_file_command (const char *args, int from_tty) =20 /* Now it's safe to re-add the breakpoints. */ breakpoint_re_set (); + + /* Also, it's safe to re-add varobjs. */ + varobj_re_set (); } } =20 @@ -2870,9 +2873,6 @@ clear_symtab_users (symfile_add_flags add_flags) clear_pc_function_cache (); gdb::observers::new_objfile.notify (NULL); =20 - /* 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) =3D=3D 0) diff --git a/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp b/gdb/testsui= te/gdb.mi/mi-var-invalidate-shlib.exp index c8b9b3e039a..7d1938880f9 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=3D\\\[{name=3D\"global_var\",in_scope=3D\"true\",= type_changed=3D\"false\",has_more=3D\"0\"}\\\]" \ + "\\^done,changelist=3D\\\[\\\]" \ "global_var recreated" mi_gdb_test "-var-update global_shlib_var" \ "\\^done,changelist=3D\\\[{name=3D\"global_shlib_var\",in_scope=3D\"i= nvalid\",has_more=3D\"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) } } =20 -/* 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. */ =20 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; =20 @@ -2389,14 +2385,12 @@ varobj_invalidate_iter (struct varobj *var) } } =20 -/* Invalidate the varobjs that are tied to locals and re-create the ones t= hat - are defined on globals. - Invalidated varobjs will be always printed in_scope=3D"invalid". */ +/* See varobj.h. */ =20 void=20 -varobj_invalidate (void) +varobj_re_set (void) { - all_root_varobjs (varobj_invalidate_iter); + all_root_varobjs (varobj_re_set_iter); } =20 /* 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); =20 -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); =20 extern bool varobj_editable_p (const struct varobj *var);