[PATCH 1/1] gdb, testsuite: Handle unused compiler option fdiagnostics-color=never.

[PATCH 1/1] gdb, testsuite: Handle unused compiler option fdiagnostics-color=never.

[Abdul Basit Ijaz]

From: "Ijaz, Abdul B" <abdul.b.ijaz@intel.com>

The 'univeral_compile_options' in gdb.exp file only verifies the support
of '-fdiagnostics-color=never' for the "C" source file.  So while running
tests with assembly source file (.s), many of them are not able to run
on icx/clang compilers because '-fdiagnostics-color=never' option is not
supported.  After this change, this function is split into multiple
functions to check the support for different type of sources individually.

Before this change, in the case of clang and ICX compiler, this error is
shown for assembly source files:

'''
icx -fdiagnostics-color=never -Wno-unknown-warning-option -fno-pie -c -O0 -o
amd64-entry-value0.o gdb/testsuite/gdb.arch/amd64-entry-value.s (timeout = 300)

icx: warning: argument unused during compilation: '-fdiagnostics-color=never'
[-Wunused-command-line-argument]

gdb compile failed, icx: warning: argument unused during compilation:
'-fdiagnostics-color=never' [-Wunused-command-line-argument]

UNTESTED: gdb.arch/amd64-entry-value.exp: failed to prepare
'''

Similarly this error is shown for the clang compiler:

'''
clang  -fdiagnostics-color=never -Wno-unknown-warning-option -fno-pie -c -O0
-o amd64-entry-value0.o gdb/testsuite/gdb.arch/amd64-entry-value.s

clang: warning: argument unused during compilation:
 '-fdiagnostics-color=never' [-Wunused-command-line-argument]
'''

2024-03-22 Ijaz, Abdul B <abdul.b.ijaz@intel.com>_
---
 gdb/testsuite/lib/gdb.exp | 48 +++++++++++++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 10 deletions(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index a0c4855ffc5..4f19162b3ce 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -5018,17 +5018,9 @@ proc gdb_wrapper_init { args } {
 }
 
 # Determine options that we always want to pass to the compiler.
-gdb_caching_proc universal_compile_options {} {
-    set me "universal_compile_options"
+proc universal_compile_options {src obj} {
     set options {}
 
-    set src [standard_temp_file ccopts.c]
-    set obj [standard_temp_file ccopts.o]
-
-    gdb_produce_source $src {
-	int foo(void) { return 0; }
-    }
-
     # Try an option for disabling colored diagnostics.  Some compilers
     # yield colored diagnostics by default (when run from a tty) unless
     # such an option is specified.
@@ -5038,6 +5030,23 @@ gdb_caching_proc universal_compile_options {} {
 	# Seems to have worked; use the option.
 	lappend options $opt
     }
+
+    return $options
+}
+
+# Determine options that we always want to pass to the C compiler.
+gdb_caching_proc universal_compile_options_c {} {
+    set me "universal_compile_options_c"
+
+    set src [standard_temp_file ccopts.c]
+    set obj [standard_temp_file ccopts.o]
+
+    gdb_produce_source $src {
+	int foo(void) { int arr[100]={0}; return 0; }
+    }
+
+    set options [universal_compile_options $src $obj]
+
     file delete $src
     file delete $obj
 
@@ -5045,6 +5054,21 @@ gdb_caching_proc universal_compile_options {} {
     return $options
 }
 
+# Determine options that we always want to pass to the compiler for
+# assembly source files with the extension ".s".
+gdb_caching_proc universal_compile_options_assembly {} {
+    set me "universal_compile_options_assembly"
+
+    set obj [standard_temp_file csymbol.o]
+    set src "$::srcdir/gdb.arch/amd64-entry-value.s"
+
+    set options [universal_compile_options $src $obj]
+    file delete $obj
+
+    verbose "$me:  returning $options" 2
+    return $options
+}
+
 # Compile the code in $code to a file based on $name, using the flags
 # $compile_flag as well as debug, nowarning and quiet  (unless otherwise
 # specified in default_compile_flags).
@@ -5224,7 +5248,11 @@ proc gdb_compile {source dest type options} {
     if {[lsearch -exact $options rust] != -1} {
 	# -fdiagnostics-color is not a rustcc option.
     } else {
-	set new_options [universal_compile_options]
+	if {[string match *.s $source] != 0} {
+	    set new_options [universal_compile_options_assembly]
+	} else {
+	    set new_options [universal_compile_options_c]
+	}
     }
 
     # C/C++ specific settings.
-- 
2.34.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

Re: [PATCH 1/1] gdb, testsuite: Handle unused compiler option fdiagnostics-color=never.

[Keith Seitz]

Hi,

Thanks for the patch! More clang compatibility is always good.

On 3/22/24 06:55, Abdul Basit Ijaz wrote:
> From: "Ijaz, Abdul B" <abdul.b.ijaz@intel.com>
> 
> The 'univeral_compile_options' in gdb.exp file only verifies the support
> of '-fdiagnostics-color=never' for the "C" source file.  So while running
> tests with assembly source file (.s), many of them are not able to run
> on icx/clang compilers because '-fdiagnostics-color=never' option is not
> supported.  After this change, this function is split into multiple
> functions to check the support for different type of sources individually.

I like this approach.

> Before this change, in the case of clang and ICX compiler, this error is
> shown for assembly source files:
> 
> '''
> icx -fdiagnostics-color=never -Wno-unknown-warning-option -fno-pie -c -O0 -o
> amd64-entry-value0.o gdb/testsuite/gdb.arch/amd64-entry-value.s (timeout = 300)
> 
> icx: warning: argument unused during compilation: '-fdiagnostics-color=never'
> [-Wunused-command-line-argument]
> 
> gdb compile failed, icx: warning: argument unused during compilation:
> '-fdiagnostics-color=never' [-Wunused-command-line-argument]
> 
> UNTESTED: gdb.arch/amd64-entry-value.exp: failed to prepare
> '''
> 
> Similarly this error is shown for the clang compiler:
> 
> '''
> clang  -fdiagnostics-color=never -Wno-unknown-warning-option -fno-pie -c -O0
> -o amd64-entry-value0.o gdb/testsuite/gdb.arch/amd64-entry-value.s
> 
> clang: warning: argument unused during compilation:
>   '-fdiagnostics-color=never' [-Wunused-command-line-argument]
> '''
> 
> 2024-03-22 Ijaz, Abdul B <abdul.b.ijaz@intel.com>_

Was this (apparently) stray line the start of a ChangeLog entry? [We
don't require those anymore.]

> ---
>   gdb/testsuite/lib/gdb.exp | 48 +++++++++++++++++++++++++++++++--------
>   1 file changed, 38 insertions(+), 10 deletions(-)
> 
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index a0c4855ffc5..4f19162b3ce 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -5018,17 +5018,9 @@ proc gdb_wrapper_init { args } {
>   }
>   
>   # Determine options that we always want to pass to the compiler.
> -gdb_caching_proc universal_compile_options {} {
> -    set me "universal_compile_options"
> +proc universal_compile_options {src obj} {
>       set options {}
>   
> -    set src [standard_temp_file ccopts.c]
> -    set obj [standard_temp_file ccopts.o]
> -
> -    gdb_produce_source $src {
> -	int foo(void) { return 0; }
> -    }
> -
>       # Try an option for disabling colored diagnostics.  Some compilers
>       # yield colored diagnostics by default (when run from a tty) unless
>       # such an option is specified.
> @@ -5038,6 +5030,23 @@ gdb_caching_proc universal_compile_options {} {
>   	# Seems to have worked; use the option.
>   	lappend options $opt
>       }
> +
> +    return $options
> +}
> +
> +# Determine options that we always want to pass to the C compiler.
> +gdb_caching_proc universal_compile_options_c {} {
> +    set me "universal_compile_options_c"
> +
> +    set src [standard_temp_file ccopts.c]
> +    set obj [standard_temp_file ccopts.o]
> +
> +    gdb_produce_source $src {
> +	int foo(void) { int arr[100]={0}; return 0; }

This patch changes this from "int foo(void) { return 0; }".
I don't see that documented why that was necessary?

> +    }
> +
> +    set options [universal_compile_options $src $obj]
> +
>       file delete $src
>       file delete $obj
>   
> @@ -5045,6 +5054,21 @@ gdb_caching_proc universal_compile_options {} {
>       return $options
>   }
>   
> +# Determine options that we always want to pass to the compiler for
> +# assembly source files with the extension ".s".
> +gdb_caching_proc universal_compile_options_assembly {} {
> +    set me "universal_compile_options_assembly"
> +
> +    set obj [standard_temp_file csymbol.o]
> +    set src "$::srcdir/gdb.arch/amd64-entry-value.s"

I don't think this is going to work anywhere except on x86.
In fact, I tried this on aarch64, and (with below issue
fixed), the log contains a bunch of errors (with "-v -v"
verbosity to DejaGNU):

Executing on host: clang    -fdiagnostics-color=never -c -o 
/root/test-fsf-master/gdb/build-aarch64-redhat-linux-gnu/gdb/testsuite/temp/69953/csymbol.o 
/root/test-fsf-master/gdb/build-aarch64-redhat-linux-gnu/gdb/testsuite/../../../gdb/testsuite/gdb.arch/amd64-entry-value.s 
    (timeout = 300)
builtin_spawn -ignore SIGHUP clang -fdiagnostics-color=never -c -o 
/root/test-fsf-master/gdb/build-aarch64-redhat-linux-gnu/gdb/testsuite/temp/69953/csymbol.o 
/root/test-fsf-master/gdb/build-aarch64-redhat-linux-gnu/gdb/testsuite/../../../gdb/testsuite/gdb.arch/amd64-entry-value.s
pid is 70084 -70084
clang: warning: argument unused during compilation: 
'-fdiagnostics-color=never'
[-Wunused-command-line-argument]
/root/test-fsf-master/gdb/build-aarch64-redhat-linux-gnu/gdb/testsuite/../../../gdb/testsuite/gdb.arch/amd64-entry-value.s:37:16: 
error: unexpected token in argument list
         movl    $0, _ZL1v(%rip)
                          ^
/root/test-fsf-master/gdb/build-aarch64-redhat-linux-gnu/gdb/testsuite/../../../gdb/testsuite/gdb.arch/amd64-entry-value.s:57:12: 
error: unexpected token in argument list
         addsd   .LC0(%rip), %xmm0
                     ^
[8000+ lines deleted]

[I note that the output contained a bunch of terminal escape codes
(which I've deleted for clarity).]

I think something more along the lines of using gdb_produce_source is
needed, where we attempt to output really generic asm (if possible?).
I wonder if we could do something along the lines of

    gdb_produce_source $src {
            .text
            .globl main
            .type main, @function
        main:
             nop
    }

Warning: untested. This does compile with both gcc and clang on aarch64
and x86_64 (with -z noexecstack), but that's all I've attempted to
verify.

Failing that, there is a relatively simple fallback for checking
for clang specifically: "test_compiler_info {clang-*}".
That might be safer than trying to write some generic asm code?

> +
> +    set options [universal_compile_options $src $obj]
> +    file delete $obj
> +
> +    verbose "$me:  returning $options" 2
> +    return $options

I really appreciate the verbose statements. Helped me understand
several of the questions I have!

> +}
> +
>   # Compile the code in $code to a file based on $name, using the flags
>   # $compile_flag as well as debug, nowarning and quiet  (unless otherwise
>   # specified in default_compile_flags).
> @@ -5224,7 +5248,11 @@ proc gdb_compile {source dest type options} {
>       if {[lsearch -exact $options rust] != -1} {
>   	# -fdiagnostics-color is not a rustcc option.
>       } else {
> -	set new_options [universal_compile_options]
> +	if {[string match *.s $source] != 0} {

I don't think this is sufficient. A bunch of test cases use ".S",
such as every aarch64 test using assembler. Perhaps add
"-nocase"?

> +	    set new_options [universal_compile_options_assembly]
> +	} else {
> +	    set new_options [universal_compile_options_c]
> +	}
>       }
>   
>       # C/C++ specific settings.

Thanks again for the patch!

Keith