[PATCH 0/2] Fix regressions introduced by Fortran compiler identification series

[PATCH 0/2] Fix regressions introduced by Fortran compiler identification series

[Nils-Christian Kempke]

After pushing a series that introduced a separate Fortran compiler
identification mechanism to the testsuite

https://sourceware.org/pipermail/gdb-patches/2022-May/189604.html

Regressions were found as noted here

https://sourceware.org/pipermail/gdb-patches/2022-May/189673.html

and here

https://sourceware.org/pipermail/gdb-patches/2022-June/189679.html

The first email noted, that running the whole testsuite would cause
regressions.  Likely due to the caching of the compiler info throughout
the testsuite.

When running the whole testsuite the fist call to get_compiler_info or
test_compiler_info would set the global compiler_info variable.  Every
subsequent call to get/test_compiler_info would simply check wether
compiler_info had already been set and, if so, omit running the actual
compiler identification and return compiler_info instead.

I changed this behavior to now have several cache variables, one for
each language that can be passed to get/_test_compiler_info (which
would be the default C, Cpp and Fortran).

I could only reporduce the regressions by running

  make check

without any parallelism in the testsuite folder.  Running with any amount
of thread via

  make ckeck -jN

where N could even be 1 in my tests (if I did not make some kind of
mistake here).  I still feel like running make check with any kindo of
parallelism run every single testcase individually (likely distributed
to the available threads).

Either way, after this patch the regressions I could reproduce with make
check are gone and testsuite performance should be ok again.

As to the second part of these regressions: I noticed that after running
`make check` with failing gdb.fortran test additional `.mod` files were
created in the gdb/testsuite directory.  I think these ended up there
for the same reason we saw the regressions.  In gdb.exp the `-J` option
is set for the executable whenever compiling with gfortran - it indicates
the target directory for the compiled module files gfortran generates (or
read in if they are already compiled).  As the compiler was firstly
identified as `gcc..` this parameter was not set when compiling the
Fortran test and, as a result, all mod files ended up in the current
working directory.  This did not seem to have any effect on the first
run.  However, any subsequent run of the testsuite would now show this
fail (even when running gdb.fortran/info-types.exp individually):

  gdb compile failed, /.../gdb//testsuite/gdb.fortran/info-types.f90:47:14:

     47 |   type (m1t1) :: var_b
        |              1
  Error: Derived type 'm1t1' at (1) is being used before it is defined
  /.../gdb/testsuite/gdb.fortran/info-types.f90:50:9:

     50 |   var_b%b = 2
        |         1
    Error: Symbol 'var_b' at (1) has no IMPLICIT type

additonal fails, as compiling the executables would attempt to use these
`.mod` already in the directory.  The m1t1 type is declared in a module
called `mod1`, which also is being used in the failed tests
nested-funcs.exp and module.exp.  For the last two, mod1 actually looks
the exact same - only the version for info-types.exp differs.  The mod1
file in the directory (for me at least) was created by the latter 2 - so
while the latter 2 tests would pass, the info-types.exp could not as
`mod1.mod` does not contain the necessary definitions.

This error should not happen anymore after this series as the original
problems with caching the compiler name should have been eliminated.
Another solution here would be to simply delete the `.mod` file in the
testsuite directory.

Cheers,

Nils


Nils-Christian Kempke (2):
  gdb/testsuite: use test_compiler_info in gcc_major_version
  gdb/testsuite: cache compiler_info on a per language basis

 gdb/testsuite/lib/compiler.F90 | 14 ++++----
 gdb/testsuite/lib/compiler.c   | 18 +++++-----
 gdb/testsuite/lib/compiler.cc  | 18 +++++-----
 gdb/testsuite/lib/gdb.exp      | 66 +++++++++++++++++++++++++++-------
 4 files changed, 78 insertions(+), 38 deletions(-)

-- 
2.25.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 1/2] gdb/testsuite: use test_compiler_info in gcc_major_version

[Nils-Christian Kempke]

The procedure gcc_major_version was earlier using the global variable
compiler_info to retrieve gcc's major version.  This is discouraged and
(as can be read in a comment in compiler.c) compiler_info should be
local to get_compiler_info and test_compiler_info.

The preferred way of getting the compiler string is via calling
test_compiler_info without arguments.  Gcc_major_version was changed to
do that.

gdb/testsuite/ChangeLog:
2022-06-01  Nils-Christian Kempke  <nils-christian.kempke@intel.com>

	* gdb.exp (gcc_major_version): Use test_compiler_info over
	global variable compiler_info.

Signed-off-by: Nils-Christian Kempke <nils-christian.kempke@intel.com>
---
 gdb/testsuite/lib/gdb.exp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 402450152a..4ee7c1fb0a 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -4218,12 +4218,11 @@ proc test_compiler_info { {compiler ""} {language ""} } {
 # For gcc 7.5.0, the major version 7.
 
 proc gcc_major_version { } {
-    global compiler_info
     global decimal
     if { ![test_compiler_info "gcc-*"] } {
 	return -1
     }
-    set res [regexp gcc-($decimal)-($decimal)- $compiler_info \
+    set res [regexp gcc-($decimal)-($decimal)- [test_compiler_info] \
 		 dummy_var major minor]
     if { $res != 1 } {
 	return -1
-- 
2.25.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 2/2] gdb/testsuite: cache compiler_info on a per language basis

[Nils-Christian Kempke]

An earlier patch series caused some regressions when running the
whole testsuite as noted here:

  https://sourceware.org/pipermail/gdb-patches/2022-May/189673.html

The reason for these regressions was the way get_compiler_info currenlty
caches the compiler identification string.  Every time get_compiler_info
is called, it checks wether the global variable compiler_info has been
set and, if not, the function actually attempts to identify the
requested compiler (C/Cpp/Fortran) and sets compiler_info. For every
subsequent call to get_compiler_info, it will find the compiler_info
variable as set, and it will, instead of identifying the compiler anew,
return the cached value in compiler_info.

This caching mechanism lead to troubles when running the whole testsuite
and entering the gdb.fortran part of the testsuite run.  As the first
call to get_compiler_info was done without any arguments (using the C
compiler identification) the compiler_info variable had been set
to whatever c compiler is being used, e.g. 'gcc-...'.  The fortran
testsuite now tried to match the string it got from test_compiler_info
against the Fortran compiler names (flang, ifx, gfortran ...), none of
which could be matched against the stored C compiler name.  Thus,
Fortran specific procedures like fortran_main, fortran_runto_main, and
the bunch of Fortran compiler dependent intrinsic typename procedures
(like fortran_int4 ...) failed to match against any known Fortran
compiler.  This lead to many test failing completely, some only
partially.  In addition, compiler dependent KFAILs and output checks in
the Fortran testsuite could not longer be matched.

This patch makes the compiler_info caching language specific.  Each
language that can be handled by get_/test_compiler_info gets its own
global variable c_/cpp_/f_compiler_info.  The respective variables are
checked whenever the get_/test_compiler_info are called for a specific
language (no language defaulting to C) and, if the language compiler has
been identified already, the cached value is returned.  Else, the
compiler identification for the requested language is triggered.

The three compiler identification files have been changed to no longer
set compiler_info, but instead set the language dependent compiler
information variable.

This eliminates the regressions introduced with the aforementioned
series.

gdb/testsuite/ChangeLog:
2022-06-01  Nils-Christian Kempke  <nils-christian.kempke@intel.com>

	* lib/compiler.F90: Rename compiler_info to f_compiler_info.
	* lib/compiler.c: Rename compiler_info to c_compiler_info.
	* lib/compiler.cc: Rename compiler_info to cc_compiler_info.
	* lib/gdb.exp (get_compiler_info): Make compiler info caching
	language dependent.

Signed-off-by: Nils-Christian Kempke <nils-christian.kempke@intel.com>
---
 gdb/testsuite/lib/compiler.F90 | 14 ++++----
 gdb/testsuite/lib/compiler.c   | 18 +++++-----
 gdb/testsuite/lib/compiler.cc  | 18 +++++-----
 gdb/testsuite/lib/gdb.exp      | 63 ++++++++++++++++++++++++++++------
 4 files changed, 77 insertions(+), 36 deletions(-)

diff --git a/gdb/testsuite/lib/compiler.F90 b/gdb/testsuite/lib/compiler.F90
index 71cf3d2c2b..314dbd5c02 100644
--- a/gdb/testsuite/lib/compiler.F90
+++ b/gdb/testsuite/lib/compiler.F90
@@ -13,15 +13,15 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-set compiler_info "unknown"
+set f_compiler_info "unknown"
 
 #if defined (__GFORTRAN__)
-set compiler_info [join {gfortran __GNUC__ __GNUC_MINOR__ __GNUC_PATCHLEVEL__} -]
+set f_compiler_info [join {gfortran __GNUC__ __GNUC_MINOR__ __GNUC_PATCHLEVEL__} -]
 #endif
 
 /* ARM seems to not define a patch version.  */
 #if defined (__ARM_LINUX_COMPILER__)
-set compiler_info [join {armflang __armclang_major__ __armclang_minor__ 0} -]
+set f_compiler_info [join {armflang __armclang_major__ __armclang_minor__ 0} -]
 #endif
 
 /* Classic flang and LLVM flang emit their respective macros differently.  */
@@ -31,12 +31,12 @@ set compiler_info [join {armflang __armclang_major__ __armclang_minor__ 0} -]
 set major __flang_major__
 set minor __flang_minor__
 set patch __flang_patchlevel__
-set compiler_info [join "flang-llvm $major $minor $patch" -]
+set f_compiler_info [join "flang-llvm $major $minor $patch" -]
 #endif
 
 /* Classic Flang.  */
 #if defined (__FLANG)
-set compiler_info [join {flang-classic __FLANG_MAJOR__ __FLANG_MINOR__ __FLANG_PATCHLEVEL__} -]
+set f_compiler_info [join {flang-classic __FLANG_MAJOR__ __FLANG_MINOR__ __FLANG_PATCHLEVEL__} -]
 #endif
 
 /* Intel LLVM emits a string like 20220100 with version 2021.2.0 and higher.  */
@@ -44,7 +44,7 @@ set compiler_info [join {flang-classic __FLANG_MAJOR__ __FLANG_MINOR__ __FLANG_P
 set major [string range __INTEL_LLVM_COMPILER 0 3]
 set minor [string range __INTEL_LLVM_COMPILER 4 5]
 set patch [string range __INTEL_LLVM_COMPILER 6 7]
-set compiler_info [join "ifx $major $minor $patch" -]
+set f_compiler_info [join "ifx $major $minor $patch" -]
 #elif defined (__INTEL_COMPILER)
 /* Starting with 2021 the ifort versioning scheme changed.  Before, Intel ifort
    would define its version as e.g. 19.0.0 or rather __INTEL_COMPILER would be
@@ -65,5 +65,5 @@ set major __INTEL_COMPILER
 set minor __INTEL_COMPILER_UPDATE
 set patch 0
 #endif
-set compiler_info [join "ifort $major $minor $patch" -]
+set f_compiler_info [join "ifort $major $minor $patch" -]
 #endif
diff --git a/gdb/testsuite/lib/compiler.c b/gdb/testsuite/lib/compiler.c
index 86140f8c0e..817da77914 100644
--- a/gdb/testsuite/lib/compiler.c
+++ b/gdb/testsuite/lib/compiler.c
@@ -32,41 +32,41 @@
 
    */
 
-set compiler_info "unknown"
+set c_compiler_info "unknown"
 
 #if defined (__GNUC__)
 #if defined (__GNUC_PATCHLEVEL__)
 /* Only GCC versions >= 3.0 define the __GNUC_PATCHLEVEL__ macro.  */
-set compiler_info [join {gcc __GNUC__ __GNUC_MINOR__ __GNUC_PATCHLEVEL__} -]
+set c_compiler_info [join {gcc __GNUC__ __GNUC_MINOR__ __GNUC_PATCHLEVEL__} -]
 #else
-set compiler_info [join {gcc __GNUC__ __GNUC_MINOR__ "unknown"} -]
+set c_compiler_info [join {gcc __GNUC__ __GNUC_MINOR__ "unknown"} -]
 #endif
 #endif
 
 #if defined (__xlc__)
 /* IBM'x xlc compiler. NOTE:  __xlc__ expands to a double quoted string of four
    numbers separated by '.'s: currently "7.0.0.0" */
-set need_a_set [regsub -all {\.} [join {xlc __xlc__} -] - compiler_info]
+set need_a_set [regsub -all {\.} [join {xlc __xlc__} -] - c_compiler_info]
 #endif
 
 #if defined (__ARMCC_VERSION)
-set compiler_info [join {armcc __ARMCC_VERSION} -]
+set c_compiler_info [join {armcc __ARMCC_VERSION} -]
 #endif
 
 #if defined (__clang__)
-set compiler_info [join {clang __clang_major__ __clang_minor__ __clang_patchlevel__} -]
+set c_compiler_info [join {clang __clang_major__ __clang_minor__ __clang_patchlevel__} -]
 #endif
 
 #if defined (__ICC)
 set icc_major [string range __ICC 0 1]
 set icc_minor [format "%d" [string range __ICC 2 [expr {[string length __ICC] -1}]]]
 set icc_update __INTEL_COMPILER_UPDATE
-set compiler_info [join "icc $icc_major $icc_minor $icc_update" -]
+set c_compiler_info [join "icc $icc_major $icc_minor $icc_update" -]
 #elif defined (__ICL)
 set icc_major [string range __ICL 0 1]
 set icc_minor [format "%d" [string range __ICL 2 [expr {[string length __ICL] -1}]]]
 set icc_update __INTEL_COMPILER_UPDATE
-set compiler_info [join "icc $icc_major $icc_minor $icc_update" -]
+set c_compiler_info [join "icc $icc_major $icc_minor $icc_update" -]
 #elif defined(__INTEL_LLVM_COMPILER) && defined(__clang_version__)
 /* Intel Next Gen compiler defines preprocessor __INTEL_LLVM_COMPILER and
    provides version info in __clang_version__ e.g. value:
@@ -75,5 +75,5 @@ set total_length [string length __clang_version__]
 set version_start_index [string last "(" __clang_version__]
 set version_string [string range __clang_version__ $version_start_index+5 $total_length-2]
 set version_updated_string [string map {. -} $version_string]
-set compiler_info "icx-$version_updated_string"
+set c_compiler_info "icx-$version_updated_string"
 #endif
diff --git a/gdb/testsuite/lib/compiler.cc b/gdb/testsuite/lib/compiler.cc
index a52e81c2e3..3036e887ed 100755
--- a/gdb/testsuite/lib/compiler.cc
+++ b/gdb/testsuite/lib/compiler.cc
@@ -20,41 +20,41 @@
 /* This file is exactly like compiler.c.  I could just use compiler.c if
    I could be sure that every C++ compiler accepted extensions of ".c".  */
 
-set compiler_info "unknown"
+set cc_compiler_info "unknown"
 
 #if defined (__GNUC__)
 #if defined (__GNUC_PATCHLEVEL__)
 /* Only GCC versions >= 3.0 define the __GNUC_PATCHLEVEL__ macro.  */
-set compiler_info [join {gcc __GNUC__ __GNUC_MINOR__ __GNUC_PATCHLEVEL__} -]
+set cc_compiler_info [join {gcc __GNUC__ __GNUC_MINOR__ __GNUC_PATCHLEVEL__} -]
 #else
-set compiler_info [join {gcc __GNUC__ __GNUC_MINOR__ "unknown"} -]
+set cc_compiler_info [join {gcc __GNUC__ __GNUC_MINOR__ "unknown"} -]
 #endif
 #endif
 
 #if defined (__xlc__)
 /* IBM'x xlc compiler. NOTE:  __xlc__ expands to a double quoted string of four
    numbers separated by '.'s: currently "7.0.0.0" */
-set need_a_set [regsub -all {\.} [join {xlc __xlc__} -] - compiler_info]
+set need_a_set [regsub -all {\.} [join {xlc __xlc__} -] - cc_compiler_info]
 #endif
 
 #if defined (__ARMCC_VERSION)
-set compiler_info [join {armcc __ARMCC_VERSION} -]
+set cc_compiler_info [join {armcc __ARMCC_VERSION} -]
 #endif
 
 #if defined (__clang__)
-set compiler_info [join {clang __clang_major__ __clang_minor__ __clang_patchlevel__} -]
+set cc_compiler_info [join {clang __clang_major__ __clang_minor__ __clang_patchlevel__} -]
 #endif
 
 #if defined (__ICC)
 set icc_major [string range __ICC 0 1]
 set icc_minor [format "%d" [string range __ICC 2 [expr {[string length __ICC] -1}]]]
 set icc_update __INTEL_COMPILER_UPDATE
-set compiler_info [join "icc $icc_major $icc_minor $icc_update" -]
+set cc_compiler_info [join "icc $icc_major $icc_minor $icc_update" -]
 #elif defined (__ICL)
 set icc_major [string range __ICL 0 1]
 set icc_minor [format "%d" [string range __ICL 2 [expr {[string length __ICL] -1}]]]
 set icc_update __INTEL_COMPILER_UPDATE
-set compiler_info [join "icc $icc_major $icc_minor $icc_update" -]
+set cc_compiler_info [join "icc $icc_major $icc_minor $icc_update" -]
 #elif defined(__INTEL_LLVM_COMPILER) && defined(__clang_version__)
 /* Intel Next Gen compiler defines preprocessor __INTEL_LLVM_COMPILER and
    provides version info in __clang_version__ e.g. value:
@@ -63,5 +63,5 @@ set total_length [string length __clang_version__]
 set version_start_index [string last "(" __clang_version__]
 set version_string [string range __clang_version__ $version_start_index+5 $total_length-2]
 set version_updated_string [string map {. -} $version_string]
-set compiler_info "icx-$version_updated_string"
+set cc_compiler_info "icx-$version_updated_string"
 #endif
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 4ee7c1fb0a..4bf93e0c75 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -4113,25 +4113,49 @@ proc get_compiler_info {{arg ""}} {
     global outdir
     global tool
 
-    # These come from compiler.c, compiler.cc or compiler.F90.
+    # These come from compiler.c, compiler.cc or compiler.F90.  They are local
+    # to get_compiler_info and should usually not be used outside.  They are
+    # used for caching the language dependent compiler info.
+    global c_compiler_info
+    global cpp_compiler_info
+    global f_compiler_info
+
+    # Local to get_compiler_info and test_compiler_info.  Used for
+    # communicating the compiler info from get_compiler_info to
+    # test_compiler_info (as well as to avoid some ifs over all supported
+    # languages).
     global compiler_info
 
     # Legacy global data symbols.
     global gcc_compiled
 
-    if [info exists compiler_info] {
-	# Already computed.
-	return 0
-    }
-
-    # Choose which file to preprocess.
-    set ifile "${srcdir}/lib/compiler.c"
+    # Check for each language whether we already cached the requested
+    # compiler info.  If not, choose which file to preprocess.
     if { $arg == "c++" } {
+	if { [info exists cc_compiler_info] } {
+	    set compiler_info $cc_compiler_info
+	    return 0
+	}
 	set ifile "${srcdir}/lib/compiler.cc"
     } elseif { $arg == "f90" } {
+	if { [info exists f_compiler_info] } {
+	    set compiler_info $f_compiler_info
+	    return 0
+	}
 	set ifile "${srcdir}/lib/compiler.F90"
+    } else {
+	# Default to C compiler identifiaction.
+	if { [info exists c_compiler_info] } {
+	    set compiler_info $c_compiler_info
+	    return 0
+	}
+	set ifile "${srcdir}/lib/compiler.c"
     }
 
+    # We'll attempt to identify a compiler for a language we have not yet
+    # processed.  For now the compiler_info is unknown.
+    set compiler_info "unknown"
+
     # Run $ifile through the right preprocessor.
     # Toggle gdb.log to keep the compiler output out of the log.
     set saved_log [log_file -info]
@@ -4171,10 +4195,27 @@ proc get_compiler_info {{arg ""}} {
 	}
     }
 
-    # Set to unknown if for some reason compiler_info didn't get defined.
-    if ![info exists compiler_info] {
+    # Compiler_info was set to unknown earlier, before starting the compiler
+    # identification.  If for some reason c/cpp/f_compiler_info didn't get
+    # set we'll leave it at that.  Else, update compiler_info to
+    # c/cpp/f_compiler_info.
+    if { $arg == "c++" } {
+	if { [info exists cpp_compiler_info] } {
+	    set compiler_info $cpp_compiler_info
+	}
+    } elseif { $arg == "f90" } {
+	if { [info exists f_compiler_info] } {
+	    set compiler_info $f_compiler_info
+	}
+    } else {
+	# Default to C compiler identifiaction.
+	if { [info exists cpp_compiler_info] } {
+	    set compiler_info $cpp_compiler_info
+	}
+    }
+
+    if { $compiler_info == "unknown" } {
 	verbose -log "get_compiler_info: compiler_info not provided"
-	set compiler_info "unknown"
     }
     # Also set to unknown compiler if any diagnostics happened.
     if { $unknown } {
-- 
2.25.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928