* [PATCH][gdb/testsuite] Handle unrecognized command line option in gdb_compile_test
@ 2021-09-08 11:20 Tom de Vries
2021-09-08 12:22 ` Joel Brobecker
2021-09-10 16:38 ` Tom de Vries
0 siblings, 2 replies; 5+ messages in thread
From: Tom de Vries @ 2021-09-08 11:20 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey, Joel Brobecker
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][gdb/testsuite] Handle unrecognized command line option in gdb_compile_test
2021-09-08 11:20 [PATCH][gdb/testsuite] Handle unrecognized command line option in gdb_compile_test Tom de Vries
@ 2021-09-08 12:22 ` Joel Brobecker
2021-09-08 13:45 ` Tom de Vries
2021-09-10 16:38 ` Tom de Vries
1 sibling, 1 reply; 5+ messages in thread
From: Joel Brobecker @ 2021-09-08 12:22 UTC (permalink / raw)
To: Tom de Vries; +Cc: gdb-patches, Tom Tromey, Joel Brobecker, 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][gdb/testsuite] Handle unrecognized command line option in gdb_compile_test
2021-09-08 12:22 ` Joel Brobecker
@ 2021-09-08 13:45 ` Tom de Vries
2021-09-08 14:12 ` Joel Brobecker
0 siblings, 1 reply; 5+ messages in thread
From: Tom de Vries @ 2021-09-08 13:45 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches, Tom Tromey, Joel Brobecker
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][gdb/testsuite] Handle unrecognized command line option in gdb_compile_test
2021-09-08 13:45 ` Tom de Vries
@ 2021-09-08 14:12 ` Joel Brobecker
0 siblings, 0 replies; 5+ messages in thread
From: Joel Brobecker @ 2021-09-08 14:12 UTC (permalink / raw)
To: Tom de Vries; +Cc: Joel Brobecker, gdb-patches, Tom Tromey, 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][gdb/testsuite] Handle unrecognized command line option in gdb_compile_test
2021-09-08 11:20 [PATCH][gdb/testsuite] Handle unrecognized command line option in gdb_compile_test Tom de Vries
2021-09-08 12:22 ` Joel Brobecker
@ 2021-09-10 16:38 ` Tom de Vries
1 sibling, 0 replies; 5+ messages in thread
From: Tom de Vries @ 2021-09-10 16:38 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey, Joel Brobecker
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-09-10 16:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-08 11:20 [PATCH][gdb/testsuite] Handle unrecognized command line option in gdb_compile_test Tom de Vries
2021-09-08 12:22 ` Joel Brobecker
2021-09-08 13:45 ` Tom de Vries
2021-09-08 14:12 ` Joel Brobecker
2021-09-10 16:38 ` Tom de Vries
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).