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

Message ID 20210908112058.GA15665@delia
State New
Headers show
Series
  • [gdb/testsuite] Handle unrecognized command line option in gdb_compile_test
Related show

Commit Message

Simon Marchi via Gdb-patches Sept. 8, 2021, 11:20 a.m.
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(-)

Comments

Joel Brobecker Sept. 8, 2021, 12:22 p.m. | #1
> 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
Simon Marchi via Gdb-patches Sept. 8, 2021, 1:45 p.m. | #2
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

>
Joel Brobecker Sept. 8, 2021, 2:12 p.m. | #3
> [ 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
Simon Marchi via Gdb-patches Sept. 10, 2021, 4:38 p.m. | #4
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

Patch

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