[PATCH][gdb/testsuite] Handle unrecognized command line option in gdb_compile_test

[PATCH][gdb/testsuite] Handle unrecognized command line option in gdb_compile_test

[Tom de Vries]

Hi,

When running the gdb testsuite with gnatmake-4.8, I get many fails of the
following form:
...
gcc: error: unrecognized command line option '-fgnat-encodings=all'^M
gnatmake: "gdb.ada/O2_float_param/foo.adb" compilation error^M
compiler exited with status 1
compilation failed: gcc ... gdb.ada/O2_float_param/foo.adb
gcc: error: unrecognized command line option '-fgnat-encodings=all'
gnatmake: "gdb.ada/O2_float_param/foo.adb" compilation error
FAIL: gdb.ada/O2_float_param.exp: scenario=all: compilation foo.adb
...

Fix this by marking the test unsupported instead, such that we have:
...
UNSUPPORTED: gdb.ada/O2_float_param.exp: scenario=all: compilation foo.adb \
  (unsupported option '-fgnat-encodings=all')
...

Also stop gdb_compile_test from doing pass/fail.  The gdb testsuite is meant
to test gdb, and a fail is meant to indicate a problem with gdb.  A compiler
failure means that the test is unsupported.

Tested on x86_64-linux.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28318

Any comments?

Thanks,
- Tom

[gdb/testsuite] Handle unrecognized command line option in gdb_compile_test

---
 gdb/testsuite/lib/gdb.exp | 35 +++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index ff19760bac4..460b44c2160 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2201,22 +2201,33 @@ proc gdb_interact { } {
 
 # Examine the output of compilation to determine whether compilation
 # failed or not.  If it failed determine whether it is due to missing
-# compiler or due to compiler error.  Report pass, fail or unsupported
-# as appropriate
+# compiler or due to compiler error.  Report unsupported as appropriate.
 
 proc gdb_compile_test {src output} {
+    set msg "compilation [file tail $src]"
+
     if { $output == "" } {
-	pass "compilation [file tail $src]"
-    } elseif { [regexp {^[a-zA-Z_0-9]+: Can't find [^ ]+\.$} $output] } {
-	unsupported "compilation [file tail $src]"
-    } elseif { [regexp {.*: command not found[\r|\n]*$} $output] } {
-	unsupported "compilation [file tail $src]"
-    } elseif { [regexp {.*: [^\r\n]*compiler not installed[^\r\n]*[\r|\n]*$} $output] } {
-	unsupported "compilation [file tail $src]"
-    } else {
-	verbose -log "compilation failed: $output" 2
-	fail "compilation [file tail $src]"
+	return
+    }
+
+    if { [regexp {^[a-zA-Z_0-9]+: Can't find [^ ]+\.$} $output]
+	 || [regexp {.*: command not found[\r|\n]*$} $output]
+	 || [regexp {.*: [^\r\n]*compiler not installed[^\r\n]*[\r|\n]*$} $output] } {
+	unsupported "$msg (missing compiler)"
+	return
     }
+
+    set gcc_re ".*: error: unrecognized command line option "
+    set clang_re ".*: error: unsupported option "
+    if { [regexp "(?:$gcc_re|$clang_re)(\[^ \t;\r\n\]*)" $output dummy option]
+	 && $option != "" } {
+	unsupported "$msg (unsupported option $option)"
+	return
+    }
+
+    # Unclassified compilation failure, be more verbose.
+    verbose -log "compilation failed: $output" 2
+    unsupported "$msg"
 }
 
 # Return a 1 for configurations for which we don't even want to try to

Re: [PATCH][gdb/testsuite] Handle unrecognized command line option in gdb_compile_test

[Joel Brobecker]

> When running the gdb testsuite with gnatmake-4.8, I get many fails of the
> following form:
> ...
> gcc: error: unrecognized command line option '-fgnat-encodings=all'^M
> gnatmake: "gdb.ada/O2_float_param/foo.adb" compilation error^M
> compiler exited with status 1
> compilation failed: gcc ... gdb.ada/O2_float_param/foo.adb
> gcc: error: unrecognized command line option '-fgnat-encodings=all'
> gnatmake: "gdb.ada/O2_float_param/foo.adb" compilation error
> FAIL: gdb.ada/O2_float_param.exp: scenario=all: compilation foo.adb
> ...
> 
> Fix this by marking the test unsupported instead, such that we have:
> ...
> UNSUPPORTED: gdb.ada/O2_float_param.exp: scenario=all: compilation foo.adb \
>   (unsupported option '-fgnat-encodings=all')
> ...
> 
> Also stop gdb_compile_test from doing pass/fail.  The gdb testsuite is meant
> to test gdb, and a fail is meant to indicate a problem with gdb.  A compiler
> failure means that the test is unsupported.

For the last part, I suggest we remain consistent with what we do when
building programs in other languages. Looking at gdb_compile_test,
we have ...

    } else {
        verbose -log "compilation failed: $output" 2
        fail "compilation [file tail $src]"

In my opinion, it's better to generate a FAIL here, because a failure
to compile could come from an error in the testcase, rather than
an unsupported build.

> Tested on x86_64-linux.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28318
> 
> Any comments?
> 
> Thanks,
> - Tom
> 
> [gdb/testsuite] Handle unrecognized command line option in gdb_compile_test
> 
> ---
>  gdb/testsuite/lib/gdb.exp | 35 +++++++++++++++++++++++------------
>  1 file changed, 23 insertions(+), 12 deletions(-)
> 
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index ff19760bac4..460b44c2160 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -2201,22 +2201,33 @@ proc gdb_interact { } {
>  
>  # Examine the output of compilation to determine whether compilation
>  # failed or not.  If it failed determine whether it is due to missing
> -# compiler or due to compiler error.  Report pass, fail or unsupported
> -# as appropriate
> +# compiler or due to compiler error.  Report unsupported as appropriate.
>  
>  proc gdb_compile_test {src output} {
> +    set msg "compilation [file tail $src]"
> +
>      if { $output == "" } {
> -	pass "compilation [file tail $src]"
> -    } elseif { [regexp {^[a-zA-Z_0-9]+: Can't find [^ ]+\.$} $output] } {
> -	unsupported "compilation [file tail $src]"
> -    } elseif { [regexp {.*: command not found[\r|\n]*$} $output] } {
> -	unsupported "compilation [file tail $src]"
> -    } elseif { [regexp {.*: [^\r\n]*compiler not installed[^\r\n]*[\r|\n]*$} $output] } {
> -	unsupported "compilation [file tail $src]"
> -    } else {
> -	verbose -log "compilation failed: $output" 2
> -	fail "compilation [file tail $src]"
> +	return
> +    }
> +
> +    if { [regexp {^[a-zA-Z_0-9]+: Can't find [^ ]+\.$} $output]
> +	 || [regexp {.*: command not found[\r|\n]*$} $output]
> +	 || [regexp {.*: [^\r\n]*compiler not installed[^\r\n]*[\r|\n]*$} $output] } {
> +	unsupported "$msg (missing compiler)"
> +	return
>      }
> +
> +    set gcc_re ".*: error: unrecognized command line option "
> +    set clang_re ".*: error: unsupported option "
> +    if { [regexp "(?:$gcc_re|$clang_re)(\[^ \t;\r\n\]*)" $output dummy option]
> +	 && $option != "" } {
> +	unsupported "$msg (unsupported option $option)"
> +	return
> +    }
> +
> +    # Unclassified compilation failure, be more verbose.
> +    verbose -log "compilation failed: $output" 2
> +    unsupported "$msg"
>  }
>  
>  # Return a 1 for configurations for which we don't even want to try to

-- 
Joel

Re: [PATCH][gdb/testsuite] Handle unrecognized command line option in gdb_compile_test

[Tom de Vries]

On 9/8/21 2:22 PM, Joel Brobecker wrote:
>> When running the gdb testsuite with gnatmake-4.8, I get many fails of the
>> following form:
>> ...
>> gcc: error: unrecognized command line option '-fgnat-encodings=all'^M
>> gnatmake: "gdb.ada/O2_float_param/foo.adb" compilation error^M
>> compiler exited with status 1
>> compilation failed: gcc ... gdb.ada/O2_float_param/foo.adb
>> gcc: error: unrecognized command line option '-fgnat-encodings=all'
>> gnatmake: "gdb.ada/O2_float_param/foo.adb" compilation error
>> FAIL: gdb.ada/O2_float_param.exp: scenario=all: compilation foo.adb
>> ...
>>
>> Fix this by marking the test unsupported instead, such that we have:
>> ...
>> UNSUPPORTED: gdb.ada/O2_float_param.exp: scenario=all: compilation foo.adb \
>>   (unsupported option '-fgnat-encodings=all')
>> ...
>>
>> Also stop gdb_compile_test from doing pass/fail.  The gdb testsuite is meant
>> to test gdb, and a fail is meant to indicate a problem with gdb.  A compiler
>> failure means that the test is unsupported.
> 
> For the last part, I suggest we remain consistent with what we do when
> building programs in other languages. Looking at gdb_compile_test,
> we have ...
> 
>     } else {
>         verbose -log "compilation failed: $output" 2
>         fail "compilation [file tail $src]"
> 

Right, that's what it says in gdb_compile_test. Using that proc however,
is the exception, used only for ada and f77.

Becoming consistent with other languages means to change that fail in
gdb_compile_test to unsupported.   Which is what this patch does.

> In my opinion, it's better to generate a FAIL here, because a failure
> to compile could come from an error in the testcase, rather than
> an unsupported build.
> 

[ To reiterate my earlier point, it's counter-pattern to generate a FAIL
for something that's not a problem in the tool-under-test.  So much
counter-pattern that I had to resist the reflex to commit this as obvious. ]

Indeed, there could be an error in the testcase.  Also, there could be
no error in the test-case.

In the former case, having a FAIL instead of UNSUPPORTED could increase
the change of noticing this, agreed.  In the latter case, having a FAIL
instead of UNSUPPORTED when testing an older compiler, is an annoyance
which drowns out FAILs that do need attention.

So, I'm not saying that noticing test-case errors causing compilation
problem is not worthwhile.  But I'm saying that turning the compilation
errors into FAILs is the wrong solution.

Thanks,
- Tom

>> Tested on x86_64-linux.
>>
>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28318
>>
>> Any comments?
>>
>> Thanks,
>> - Tom
>>
>> [gdb/testsuite] Handle unrecognized command line option in gdb_compile_test
>>
>> ---
>>  gdb/testsuite/lib/gdb.exp | 35 +++++++++++++++++++++++------------
>>  1 file changed, 23 insertions(+), 12 deletions(-)
>>
>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>> index ff19760bac4..460b44c2160 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -2201,22 +2201,33 @@ proc gdb_interact { } {
>>  
>>  # Examine the output of compilation to determine whether compilation
>>  # failed or not.  If it failed determine whether it is due to missing
>> -# compiler or due to compiler error.  Report pass, fail or unsupported
>> -# as appropriate
>> +# compiler or due to compiler error.  Report unsupported as appropriate.
>>  
>>  proc gdb_compile_test {src output} {
>> +    set msg "compilation [file tail $src]"
>> +
>>      if { $output == "" } {
>> -	pass "compilation [file tail $src]"
>> -    } elseif { [regexp {^[a-zA-Z_0-9]+: Can't find [^ ]+\.$} $output] } {
>> -	unsupported "compilation [file tail $src]"
>> -    } elseif { [regexp {.*: command not found[\r|\n]*$} $output] } {
>> -	unsupported "compilation [file tail $src]"
>> -    } elseif { [regexp {.*: [^\r\n]*compiler not installed[^\r\n]*[\r|\n]*$} $output] } {
>> -	unsupported "compilation [file tail $src]"
>> -    } else {
>> -	verbose -log "compilation failed: $output" 2
>> -	fail "compilation [file tail $src]"
>> +	return
>> +    }
>> +
>> +    if { [regexp {^[a-zA-Z_0-9]+: Can't find [^ ]+\.$} $output]
>> +	 || [regexp {.*: command not found[\r|\n]*$} $output]
>> +	 || [regexp {.*: [^\r\n]*compiler not installed[^\r\n]*[\r|\n]*$} $output] } {
>> +	unsupported "$msg (missing compiler)"
>> +	return
>>      }
>> +
>> +    set gcc_re ".*: error: unrecognized command line option "
>> +    set clang_re ".*: error: unsupported option "
>> +    if { [regexp "(?:$gcc_re|$clang_re)(\[^ \t;\r\n\]*)" $output dummy option]
>> +	 && $option != "" } {
>> +	unsupported "$msg (unsupported option $option)"
>> +	return
>> +    }
>> +
>> +    # Unclassified compilation failure, be more verbose.
>> +    verbose -log "compilation failed: $output" 2
>> +    unsupported "$msg"
>>  }
>>  
>>  # Return a 1 for configurations for which we don't even want to try to
> 

Re: [PATCH][gdb/testsuite] Handle unrecognized command line option in gdb_compile_test

[Joel Brobecker]

> [ To reiterate my earlier point, it's counter-pattern to generate a FAIL
> for something that's not a problem in the tool-under-test.  So much
> counter-pattern that I had to resist the reflex to commit this as obvious. ]
> 
> Indeed, there could be an error in the testcase.  Also, there could be
> no error in the test-case.
> 
> In the former case, having a FAIL instead of UNSUPPORTED could increase
> the change of noticing this, agreed.  In the latter case, having a FAIL
> instead of UNSUPPORTED when testing an older compiler, is an annoyance
> which drowns out FAILs that do need attention.
> 
> So, I'm not saying that noticing test-case errors causing compilation
> problem is not worthwhile.  But I'm saying that turning the compilation
> errors into FAILs is the wrong solution.

I see what you mean. I don't really have a strong opinion on this.
Speaking for myself, I tend to blindly accept UNSUPPORTED results,
so I am unlikely to notice incorrect ones, which suggests that, for me
at least, using UNSUPPORTED is no better. But then again, the way
I have being doing testing in the past is by "diff-ing" the testsuite
report before and after my change -- so it's not like I would notice
a FAIL better ;-). So it's also be no worse (again, just speaking for
myself, as YMMV in this case).

-- 
Joel

Re: [PATCH][gdb/testsuite] Handle unrecognized command line option in gdb_compile_test

[Tom de Vries]

On 9/8/21 1:20 PM, Tom de Vries wrote:
> Hi,
> 
> When running the gdb testsuite with gnatmake-4.8, I get many fails of the
> following form:
> ...
> gcc: error: unrecognized command line option '-fgnat-encodings=all'^M
> gnatmake: "gdb.ada/O2_float_param/foo.adb" compilation error^M
> compiler exited with status 1
> compilation failed: gcc ... gdb.ada/O2_float_param/foo.adb
> gcc: error: unrecognized command line option '-fgnat-encodings=all'
> gnatmake: "gdb.ada/O2_float_param/foo.adb" compilation error
> FAIL: gdb.ada/O2_float_param.exp: scenario=all: compilation foo.adb
> ...
> 
> Fix this by marking the test unsupported instead, such that we have:
> ...
> UNSUPPORTED: gdb.ada/O2_float_param.exp: scenario=all: compilation foo.adb \
>   (unsupported option '-fgnat-encodings=all')
> ...

I've committed this non-controversial part and backported to gdb-11-branch.

Thanks,
- Tom