public inbox for gdb-cvs@sourceware.org
help / color / mirror / Atom feed
* [binutils-gdb] gdb/testsuite: solve problems with compiler_info caching
@ 2022-06-09 13:41 Andrew Burgess
  0 siblings, 0 replies; only message in thread
From: Andrew Burgess @ 2022-06-09 13:41 UTC (permalink / raw)
  To: gdb-cvs

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=575a212a78c6f7e213933cf96d9a63642edc5069

commit 575a212a78c6f7e213933cf96d9a63642edc5069
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Wed Jun 8 14:04:19 2022 +0100

    gdb/testsuite: solve problems with compiler_info caching
    
    After this commit:
    
      commit 44d469c5f85a4243462b8966722dafa62b602bf5
      Date:   Tue May 31 16:43:44 2022 +0200
    
          gdb/testsuite: add Fortran compiler identification to GDB
    
    Some regressions were noticed:
    
      https://sourceware.org/pipermail/gdb-patches/2022-May/189673.html
    
    The problem is associated with how compiler_info variable is cached
    between calls to get_compiler_info.
    
    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.
    
    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.
    
    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.
    
    However, if the user starting playing with CC_FOR_TARGET or
    CXX_FOR_TARGET, then they might not get the behaviour they expect.
    
    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:
    
    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.
    
    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.
    
    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.
    
    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).
    
    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.
    
    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.
    
    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).
    
    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.
    
    Our override of load_lib then decides that this new global has to be
    preserved, and adds it to the gdb_persistent_globals array.
    
    From that point on compiler_info will never be recomputed!
    
    This commit addresses all the caching problems by doing the following:
    
    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.
    
    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.
    
    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.
    
    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.
    
    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
 
 proc get_compiler_info {{language "c"}} {
+
     # For compiler.c, compiler.cc and compiler.F90.
     global srcdir
 
@@ -4106,12 +4107,9 @@ proc get_compiler_info {{language "c"}} {
     global tool
 
     # 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
 
-    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"
     }
 
-    # 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 == "c" } {
+	# Legacy global data symbols.
+	gdb_persistent_global gcc_compiled
+
+	set gcc_compiled 0
+
+	regexp "^gcc-(\[0-9\]+)-" "$compiler_info" matchall gcc_compiled
+    }
 
     # Log what happened.
     verbose -log "get_compiler_info: $compiler_info"
@@ -4198,7 +4204,7 @@ proc get_compiler_info {{language "c"}} {
 # compiler_info.
 
 proc test_compiler_info { {compiler ""} {language "c"} } {
-    global compiler_info
+    gdb_persistent_global compiler_info_cache
 
     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"} } {
 
     # If no arg, return the compiler_info string.
     if [string match "" $compiler] {
-	return $compiler_info
+	return $compiler_info_cache($language)
     }
 
-    return [string match $compiler $compiler_info]
+    return [string match $compiler $compiler_info_cache($language)]
 }
 
 # Return the gcc major version, or -1.


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-06-09 13:41 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-09 13:41 [binutils-gdb] gdb/testsuite: solve problems with compiler_info caching Andrew Burgess

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).