gdb/testsuite: solve problems with compiler_info caching
authorAndrew Burgess <aburgess@redhat.com>
Wed, 8 Jun 2022 13:04:19 +0000 (14:04 +0100)
committerAndrew Burgess <aburgess@redhat.com>
Thu, 9 Jun 2022 13:40:48 +0000 (14:40 +0100)
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.

gdb/testsuite/lib/gdb.exp

index e28c33e8455b5fda65b3d358f311c9fba01b8e59..67e5838a7a760659ee37bb0418d92a25a8dd203d 100644 (file)
@@ -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.