public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 0/1] gdb, testsuite: Handle unused compiler option fdiagnostics-color=never.
@ 2024-05-14 17:16 Abdul Basit Ijaz
  2024-05-14 17:16 ` [PATCH v2 1/1] " Abdul Basit Ijaz
  0 siblings, 1 reply; 4+ messages in thread
From: Abdul Basit Ijaz @ 2024-05-14 17:16 UTC (permalink / raw)
  To: gdb-patches; +Cc: abdul.b.ijaz, christina.schimpe, felix.willgerodt, keiths

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

Hi All,

This patch updates the handling of '-fdiagnostics-color=never' in the
function 'univeral_compile_options' of gdb.exp file.  Before this change
it 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.

Changes since V1:

* Fix the feedback on V1 patch i.e remove sign off line in commit message
  and remove the changes not needed for the fix.
* Use generic source file for checking the compile option support in .s
  assembly file.

V1 patch:
https://sourceware.org/pipermail/gdb-patches/2024-March/207477.html

Thanks & Best Regards
Abdul Basit


Ijaz, Abdul B (1):
  gdb, testsuite: Handle unused compiler option
    fdiagnostics-color=never.

 gdb/testsuite/lib/gdb.exp | 52 +++++++++++++++++++++++++++++++--------
 1 file changed, 42 insertions(+), 10 deletions(-)

-- 
2.34.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 1/1] gdb, testsuite: Handle unused compiler option fdiagnostics-color=never.
  2024-05-14 17:16 [PATCH v2 0/1] gdb, testsuite: Handle unused compiler option fdiagnostics-color=never Abdul Basit Ijaz
@ 2024-05-14 17:16 ` Abdul Basit Ijaz
  2024-05-15 16:17   ` Tom Tromey
  0 siblings, 1 reply; 4+ messages in thread
From: Abdul Basit Ijaz @ 2024-05-14 17:16 UTC (permalink / raw)
  To: gdb-patches; +Cc: abdul.b.ijaz, christina.schimpe, felix.willgerodt, keiths

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]
'''
---
 gdb/testsuite/lib/gdb.exp | 52 +++++++++++++++++++++++++++++++--------
 1 file changed, 42 insertions(+), 10 deletions(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 0d78691c381..54c61c95d79 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -5046,17 +5046,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.
@@ -5066,6 +5058,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) { return 0; }
+    }
+
+    set options [universal_compile_options $src $obj]
+
     file delete $src
     file delete $obj
 
@@ -5073,6 +5082,25 @@ 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 src [standard_temp_file ccopts.s]
+    set obj [standard_temp_file csymbol.o]
+
+    gdb_produce_source $src {
+	main:
+    }
+
+    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).
@@ -5252,7 +5280,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
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 1/1] gdb, testsuite: Handle unused compiler option fdiagnostics-color=never.
  2024-05-14 17:16 ` [PATCH v2 1/1] " Abdul Basit Ijaz
@ 2024-05-15 16:17   ` Tom Tromey
  2024-05-15 19:06     ` Ijaz, Abdul B
  0 siblings, 1 reply; 4+ messages in thread
From: Tom Tromey @ 2024-05-15 16:17 UTC (permalink / raw)
  To: Abdul Basit Ijaz; +Cc: gdb-patches, christina.schimpe, felix.willgerodt, keiths

>>>>> Abdul Basit Ijaz <abdul.b.ijaz@intel.com> writes:

> 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.

Thanks for the patch.

> -	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]

Should this also check for *.S files?  If so it needs a change, and if
not it should probably have a comment explaining why not.

Tom

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH v2 1/1] gdb, testsuite: Handle unused compiler option fdiagnostics-color=never.
  2024-05-15 16:17   ` Tom Tromey
@ 2024-05-15 19:06     ` Ijaz, Abdul B
  0 siblings, 0 replies; 4+ messages in thread
From: Ijaz, Abdul B @ 2024-05-15 19:06 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches, Schimpe, Christina, Willgerodt, Felix, keiths

Hi Tom,

Thanks for the feedback.

>> +	if {[string match *.s $source] != 0} {
>> +	    set new_options [universal_compile_options_assembly]

> Should this also check for *.S files?  If so it needs a change, and if not it should probably have a comment explaining why not.

Icx/icpx/ifx/clang compilers does not support this compile option only for ".s" files while .S files does not have this issue. Sure I will add the comment here.

Best Regards
Abdul Basit

-----Original Message-----
From: Tom Tromey <tom@tromey.com> 
Sent: Wednesday, May 15, 2024 6:18 PM
To: Ijaz, Abdul B <abdul.b.ijaz@intel.com>
Cc: gdb-patches@sourceware.org; Schimpe, Christina <christina.schimpe@intel.com>; Willgerodt, Felix <felix.willgerodt@intel.com>; keiths@redhat.com
Subject: Re: [PATCH v2 1/1] gdb, testsuite: Handle unused compiler option fdiagnostics-color=never.

>>>>> Abdul Basit Ijaz <abdul.b.ijaz@intel.com> writes:

> 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.

Thanks for the patch.

> -	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]

Should this also check for *.S files?  If so it needs a change, and if not it should probably have a comment explaining why not.

Tom
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-05-15 19:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-14 17:16 [PATCH v2 0/1] gdb, testsuite: Handle unused compiler option fdiagnostics-color=never Abdul Basit Ijaz
2024-05-14 17:16 ` [PATCH v2 1/1] " Abdul Basit Ijaz
2024-05-15 16:17   ` Tom Tromey
2024-05-15 19:06     ` Ijaz, Abdul B

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).