From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1726) id 03C533839C5F; Thu, 9 Jun 2022 13:41:36 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 03C533839C5F Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Andrew Burgess To: gdb-cvs@sourceware.org Subject: [binutils-gdb] gdb/testsuite: solve problems with compiler_info caching X-Act-Checkin: binutils-gdb X-Git-Author: Andrew Burgess X-Git-Refname: refs/heads/master X-Git-Oldrev: 0e471fde0742bbbc8e34f0d082d1b35c19aee96f X-Git-Newrev: 575a212a78c6f7e213933cf96d9a63642edc5069 Message-Id: <20220609134136.03C533839C5F@sourceware.org> Date: Thu, 9 Jun 2022 13:41:36 +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, 09 Jun 2022 13:41:36 -0000 https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D575a212a78c6= f7e213933cf96d9a63642edc5069 commit 575a212a78c6f7e213933cf96d9a63642edc5069 Author: Andrew Burgess Date: Wed Jun 8 14:04:19 2022 +0100 gdb/testsuite: solve problems with compiler_info caching =20 After this commit: =20 commit 44d469c5f85a4243462b8966722dafa62b602bf5 Date: Tue May 31 16:43:44 2022 +0200 =20 gdb/testsuite: add Fortran compiler identification to GDB =20 Some regressions were noticed: =20 https://sourceware.org/pipermail/gdb-patches/2022-May/189673.html =20 The problem is associated with how compiler_info variable is cached between calls to get_compiler_info. =20 Even before the above commit, get_compiler_info supported two language, C and C++. Calling get_compiler_info would set the global compiler_info based on the language passed as an argument to get_compiler_info, and, in theory, compiler_info would not be updated for the rest of the dejagnu run. =20 This obviously is slightly broken behaviour. If the first call to get_compiler_info was for the C++ language then compiler_info would be set based on the C++ compiler in use, while if the first call to get_compiler_info was for the C language then compiler_info would be set based on the C compiler. =20 This probably wasn't very noticable, assuming a GCC based test environment then in most cases the C and C++ compiler would be the same version. =20 However, if the user starting playing with CC_FOR_TARGET or CXX_FOR_TARGET, then they might not get the behaviour they expect. =20 Except, to make matters worse, most of the time, the user probably would get the behaviour they expected .... except when they didn't! I'll explain: =20 In gdb.exp we try to avoid global variables leaking between test scripts, this is done with the help of the two procs gdb_setup_known_globals and gdb_cleanup_globals. All known globals are recorded before a test script starts, and then, when the test script ends, any new globals are deleted. =20 Normally, compiler_info is only set as a result of a test script calling get_compiler_info or test_compiler_info. This means that the compiler_info global will not exist when the test script starts, but will exist when the test script end, and so, the compiler_info variable is deleted at the end of each test. =20 This means that, in reality, the compiler_info is recalculated once for each test script, hence, if a test script just checks on the C compiler, or just checks on the C++ compiler, then compiler_info will be correct and the user will get the behaviour they expect. =20 However, if a single test script tries to check both the C and C++ compiler versions then this will not work (even before the above commit). =20 The situation is made worse be the behaviour or the load_lib proc. This proc (provided by dejagnu) will only load each library once. This means that if a library defines a global, then this global would normally be deleted at the end of the first test script that includes the library. =20 As future attempts to load the library will not actually reload it, then the global will not be redefined and would be missing for later test scripts that also tried to load that library. =20 To work around this issue we override load_lib in gdb.exp, this new version adds all globals from the newly loaded library to the list of globals that should be preserved (not deleted). =20 And this is where things get interesting for us. The library trace-support.exp includes calls, at the file scope, to things like is_amd64_regs_target, which cause get_compiler_info to be called. This means that after loading the library the compiler_info global is defined. =20 Our override of load_lib then decides that this new global has to be preserved, and adds it to the gdb_persistent_globals array. =20 From that point on compiler_info will never be recomputed! =20 This commit addresses all the caching problems by doing the following: =20 Change the compiler_info global into compiler_info_cache global. This new global is an array, the keys of this array will be each of the supported languages, and the values will be the compiler version for that language. =20 Now, when we call get_compiler_info, if the compiler information for the specific language has not been computed, then we do that, and add it to the cache. =20 Next, compiler_info_cache is defined by calling gdb_persistent_global. This automatically adds the global to the list of persistent globals. Now the cache will not be deleted at the end of each test script. =20 This means that, for a single test run, we will compute the compiler version just once for each language, this result will then be cached between test scripts. =20 Finally, the legacy 'gcc_compiled' flag is now only set when we call get_compiler_info with the language 'c'. Without making this change the value of 'gcc_compiled' would change each time a new language is passed to get_compiler_info. If the last language was e.g. Fortran, then gcc_compiled might be left false. Diff: --- gdb/testsuite/lib/gdb.exp | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index e28c33e8455..67e5838a7a7 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -4098,6 +4098,7 @@ set gcc_compiled 0 # -- chastain 2004-01-06 =20 proc get_compiler_info {{language "c"}} { + # For compiler.c, compiler.cc and compiler.F90. global srcdir =20 @@ -4106,12 +4107,9 @@ proc get_compiler_info {{language "c"}} { global tool =20 # These come from compiler.c, compiler.cc or compiler.F90. - global compiler_info - - # Legacy global data symbols. - global gcc_compiled + gdb_persistent_global compiler_info_cache =20 - if [info exists compiler_info] { + if [info exists compiler_info_cache($language)] { # Already computed. return 0 } @@ -4178,9 +4176,17 @@ proc get_compiler_info {{language "c"}} { set compiler_info "unknown" } =20 - # Set the legacy symbols. - set gcc_compiled 0 - regexp "^gcc-(\[0-9\]+)-" "$compiler_info" matchall gcc_compiled + set compiler_info_cache($language) $compiler_info + + # Set the legacy symbol gcc_compiled. + if { $language =3D=3D "c" } { + # Legacy global data symbols. + gdb_persistent_global gcc_compiled + + set gcc_compiled 0 + + regexp "^gcc-(\[0-9\]+)-" "$compiler_info" matchall gcc_compiled + } =20 # Log what happened. verbose -log "get_compiler_info: $compiler_info" @@ -4198,7 +4204,7 @@ proc get_compiler_info {{language "c"}} { # compiler_info. =20 proc test_compiler_info { {compiler ""} {language "c"} } { - global compiler_info + gdb_persistent_global compiler_info_cache =20 if [get_compiler_info $language] { # An error will already have been printed in this case. Just @@ -4213,10 +4219,10 @@ proc test_compiler_info { {compiler ""} {language "= c"} } { =20 # If no arg, return the compiler_info string. if [string match "" $compiler] { - return $compiler_info + return $compiler_info_cache($language) } =20 - return [string match $compiler $compiler_info] + return [string match $compiler $compiler_info_cache($language)] } =20 # Return the gcc major version, or -1.