[2/6] gdb/testsuite: fix gdb.trace/unavailable-dwarf-piece.exp with Clang

Message ID e79df44660fd3936e699d76f50607bfca6a65495.1668184173.git.aburgess@redhat.com
State Committed
Commit 33c1395cf5e9deec7733691ba32c450e5c27f757
Headers
Series The DWARF assembler and Clang |

Commit Message

Andrew Burgess Nov. 11, 2022, 4:36 p.m. UTC
  I noticed that the test gdb.trace/unavailable-dwarf-piece.exp was
failing when run with Clang.  Or rather, the test was not running as
the test executable failed to compile.

The problem is that Clang was emitting this warning:

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

This warning is emitted when compiling the assembler file generated
by the DWARF assembler.

Most DWARF assembler tests generate the assembler file into a file
with the '.S' extension.  However, this particular test uses a '.s'
extension.

Now a .S file will be passed through the preprocessor, while a .s will
be sent straight to the assembler.  My guess is that Clang doesn't
support the -fdiagnostics-color=never option for the assembler, but
does for the preprocessor.

That's a little annoying, but easily worked around.  We don't care if
our assembler file is passed through the preprocessor, so, in this
commit, I just change the file extension from .s to .S, and the
problem is fixed.

Currently, the unavailable-dwarf-piece.exp script names the assembler
file using standard_output_file, in this commit I've switched to make
use of standard_testfile, as that seems to be the more common way of
doing this sort of thing.

With these changes the test now passes with Clang 9.0.1 and 15.0.2,
and also still passes with gcc.
---
 gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

Comments

Guinevere Larsen Nov. 16, 2022, 2:41 p.m. UTC | #1
On 11/11/2022 17:36, Andrew Burgess via Gdb-patches wrote:
> I noticed that the test gdb.trace/unavailable-dwarf-piece.exp was
> failing when run with Clang.  Or rather, the test was not running as
> the test executable failed to compile.
>
> The problem is that Clang was emitting this warning:
>
>    warning: argument unused during compilation: '-fdiagnostics-color=never' [-Wunused-command-line-argument]
>
> This warning is emitted when compiling the assembler file generated
> by the DWARF assembler.
>
> Most DWARF assembler tests generate the assembler file into a file
> with the '.S' extension.  However, this particular test uses a '.s'
> extension.
>
> Now a .S file will be passed through the preprocessor, while a .s will
> be sent straight to the assembler.  My guess is that Clang doesn't
> support the -fdiagnostics-color=never option for the assembler, but
> does for the preprocessor.
>
> That's a little annoying, but easily worked around.  We don't care if
> our assembler file is passed through the preprocessor, so, in this
> commit, I just change the file extension from .s to .S, and the
> problem is fixed.
>
> Currently, the unavailable-dwarf-piece.exp script names the assembler
> file using standard_output_file, in this commit I've switched to make
> use of standard_testfile, as that seems to be the more common way of
> doing this sort of thing.
>
> With these changes the test now passes with Clang 9.0.1 and 15.0.2,
> and also still passes with gcc.
> ---
>   gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp b/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
> index f80f8005fcf..13c6f38737c 100644
> --- a/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
> +++ b/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
> @@ -20,9 +20,9 @@ if {![dwarf2_support]} {
>       return 0
>   }
>   
> -standard_testfile .c
> +standard_testfile .c -dbg.S
>   
> -set asm_file [standard_output_file "${testfile}-dbg.s"]
> +set asm_file $srcfile2
>   set opts {}
>   
>   if  { [gdb_compile ${srcdir}/${subdir}/${srcfile} ${binfile}1.o \

I tried running this test with these changes using clang 14.0.5 and 
clang 16.0.0, and both times I got the following output:

builtin_spawn -ignore SIGHUP clang-14 -fdiagnostics-color=never 
-Wno-unknown-warning-option -w -c -g -o 
/home/blarsen/Documents/fsf_build/gdb/testsuite/temp/1163842/is_ilp32_target-1163842.o 
/home/blarsen/Documents/fsf_build/gdb/testsuite/temp/1163842/is_ilp32_target-1163842.c
/home/blarsen/Documents/fsf_build/gdb/testsuite/temp/1163842/is_ilp32_target-1163842.c:2:12: 
error: 'dummy' declared as an array with a negative size
         int dummy[sizeof (int) == 4
                   ^~~~~~~~~~~~~~~~~
1 error generated.


And I get an "unsupported" test. The error message says that the target 
doesn't support trace, but I'm not sure if it isn't related to the 
compilation failure. Do you see anything similar?
  
Andrew Burgess Nov. 17, 2022, 11:06 a.m. UTC | #2
Bruno Larsen <blarsen@redhat.com> writes:

> On 11/11/2022 17:36, Andrew Burgess via Gdb-patches wrote:
>> I noticed that the test gdb.trace/unavailable-dwarf-piece.exp was
>> failing when run with Clang.  Or rather, the test was not running as
>> the test executable failed to compile.
>>
>> The problem is that Clang was emitting this warning:
>>
>>    warning: argument unused during compilation: '-fdiagnostics-color=never' [-Wunused-command-line-argument]
>>
>> This warning is emitted when compiling the assembler file generated
>> by the DWARF assembler.
>>
>> Most DWARF assembler tests generate the assembler file into a file
>> with the '.S' extension.  However, this particular test uses a '.s'
>> extension.
>>
>> Now a .S file will be passed through the preprocessor, while a .s will
>> be sent straight to the assembler.  My guess is that Clang doesn't
>> support the -fdiagnostics-color=never option for the assembler, but
>> does for the preprocessor.
>>
>> That's a little annoying, but easily worked around.  We don't care if
>> our assembler file is passed through the preprocessor, so, in this
>> commit, I just change the file extension from .s to .S, and the
>> problem is fixed.
>>
>> Currently, the unavailable-dwarf-piece.exp script names the assembler
>> file using standard_output_file, in this commit I've switched to make
>> use of standard_testfile, as that seems to be the more common way of
>> doing this sort of thing.
>>
>> With these changes the test now passes with Clang 9.0.1 and 15.0.2,
>> and also still passes with gcc.
>> ---
>>   gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp b/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
>> index f80f8005fcf..13c6f38737c 100644
>> --- a/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
>> +++ b/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
>> @@ -20,9 +20,9 @@ if {![dwarf2_support]} {
>>       return 0
>>   }
>>   
>> -standard_testfile .c
>> +standard_testfile .c -dbg.S
>>   
>> -set asm_file [standard_output_file "${testfile}-dbg.s"]
>> +set asm_file $srcfile2
>>   set opts {}
>>   
>>   if  { [gdb_compile ${srcdir}/${subdir}/${srcfile} ${binfile}1.o \
>
> I tried running this test with these changes using clang 14.0.5 and 
> clang 16.0.0, and both times I got the following output:
>
> builtin_spawn -ignore SIGHUP clang-14 -fdiagnostics-color=never 
> -Wno-unknown-warning-option -w -c -g -o 
> /home/blarsen/Documents/fsf_build/gdb/testsuite/temp/1163842/is_ilp32_target-1163842.o 
> /home/blarsen/Documents/fsf_build/gdb/testsuite/temp/1163842/is_ilp32_target-1163842.c
> /home/blarsen/Documents/fsf_build/gdb/testsuite/temp/1163842/is_ilp32_target-1163842.c:2:12: 
> error: 'dummy' declared as an array with a negative size
>          int dummy[sizeof (int) == 4
>                    ^~~~~~~~~~~~~~~~~
> 1 error generated.

That error is part of the is_ilp32_target proc, and is a result of this
C source code:

	int dummy[sizeof (int) == 4
		  && sizeof (void *) == 4
		  && sizeof (long) == 4 ? 1 : -1];

So on a non ilp32 target (e.g. x86-64) the array length will be -1, and
the code, by design, will fail to compile.  We use the failure to
compile as an indication that the target is non-ilp32.

> And I get an "unsupported" test. The error message says that the target 
> doesn't support trace, but I'm not sure if it isn't related to the 
> compilation failure. Do you see anything similar?

The unsupported is not related to the error described above.

I also see the "unsupported".

But the critical thing is that previously, when compiling with Clang I
didn't get to the unsupported, I instead saw a failure to compile the
test program (error in the original commit message).  With this fix the
Clang program now compiles.

There is a small chance that for [reasons] the test might fail on a
target that does support tracing when compiled with Clang.  But I'm
going to leave that for others to figure out.  Any fix for that problem
would be a completely unrelated fix to the one I'm proposing here
anyway, so it would always be a separate patch.

Thanks,
Andrew
  
Tom Tromey Dec. 12, 2022, 5:55 p.m. UTC | #3
>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> Currently, the unavailable-dwarf-piece.exp script names the assembler
Andrew> file using standard_output_file, in this commit I've switched to make
Andrew> use of standard_testfile, as that seems to be the more common way of
Andrew> doing this sort of thing.

I think this caused the .S file to be created outside of the test's
directory.  Using standard_output_file would fix this.  I think it's
more normal to do this as well; searching for "set asm_file" yields a
lot of hits of the form:

    set asm_file [standard_output_file $srcfile2]

So if this change isn't needed to make the test work with clang, I'd
suggest reverting this part.

Tom
  
Andrew Burgess Dec. 13, 2022, 3:39 p.m. UTC | #4
Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Andrew> Currently, the unavailable-dwarf-piece.exp script names the assembler
> Andrew> file using standard_output_file, in this commit I've switched to make
> Andrew> use of standard_testfile, as that seems to be the more common way of
> Andrew> doing this sort of thing.
>
> I think this caused the .S file to be created outside of the test's
> directory.  Using standard_output_file would fix this.  I think it's
> more normal to do this as well; searching for "set asm_file" yields a
> lot of hits of the form:
>
>     set asm_file [standard_output_file $srcfile2]
>
> So if this change isn't needed to make the test work with clang, I'd
> suggest reverting this part.

Sorry for this.  I pushed the patch below to fix this issue.

Thanks,
Andrew

---

commit dc3fb44540b586c02ec2f841e106a8d2cc2a422d
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Tue Dec 13 15:37:17 2022 +0000

    gdb/testsuite: avoid creating temp file in gdb/testsuite/ directory
    
    After this commit:
    
      commit 33c1395cf5e9deec7733691ba32c450e5c27f757
      Date:   Fri Nov 11 15:26:46 2022 +0000
    
          gdb/testsuite: fix gdb.trace/unavailable-dwarf-piece.exp with Clang
    
    The gdb.trace/unavailable-dwarf-piece.exp test script was creating a
    temporary file in the build/gdb/testsuite/ directory, instead of in
    the expected place in the outputs directory.
    
    Fix this by adding a call to standard_output_file.

diff --git a/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp b/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
index 13c6f38737c..d73b9f1e33f 100644
--- a/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
+++ b/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
@@ -22,7 +22,7 @@ if {![dwarf2_support]} {
 
 standard_testfile .c -dbg.S
 
-set asm_file $srcfile2
+set asm_file [standard_output_file $srcfile2]
 set opts {}
 
 if  { [gdb_compile ${srcdir}/${subdir}/${srcfile} ${binfile}1.o \
  

Patch

diff --git a/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp b/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
index f80f8005fcf..13c6f38737c 100644
--- a/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
+++ b/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
@@ -20,9 +20,9 @@  if {![dwarf2_support]} {
     return 0
 }
 
-standard_testfile .c
+standard_testfile .c -dbg.S
 
-set asm_file [standard_output_file "${testfile}-dbg.s"]
+set asm_file $srcfile2
 set opts {}
 
 if  { [gdb_compile ${srcdir}/${subdir}/${srcfile} ${binfile}1.o \