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

[Tom de Vries via Gdb-patches <gdb-patches@sourceware.org>]

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
>