[patch] gdb/testsuite: Fix more bugs in gdb testglue wrapper handling

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[patch] gdb/testsuite: Fix more bugs in gdb testglue wrapper handling

Sandra Loosemore
This patch fixes another batch of testsuite bugs I found, this time
related to handling of the gdb testglue wrapper that is needed on some
targets where the simulator (or whatever) does not properly report
program exit status to the test harness.  More details in the commit
message.

I tested this on nios2-elf.  OK to commit?

-Sandra


gdb-wrapper.patch (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[ping] Re: [patch] gdb/testsuite: Fix more bugs in gdb testglue wrapper handling

Sandra Loosemore
Ping?

https://sourceware.org/pipermail/gdb-patches/2020-July/170224.html

On 7/6/20 6:33 PM, Sandra Loosemore wrote:

> This patch fixes another batch of testsuite bugs I found, this time
> related to handling of the gdb testglue wrapper that is needed on some
> targets where the simulator (or whatever) does not properly report
> program exit status to the test harness.  More details in the commit
> message.
>
> I tested this on nios2-elf.  OK to commit?
>
> -Sandra
>

Reply | Threaded
Open this post in threaded view
|

Re: [patch] gdb/testsuite: Fix more bugs in gdb testglue wrapper handling

Tom Tromey-2
In reply to this post by Sandra Loosemore
>>>>> "Sandra" == Sandra Loosemore <[hidden email]> writes:

Sandra>      if {[target_info exists needs_status_wrapper] && \
Sandra>      [target_info needs_status_wrapper] != "0"} {
Sandra> - set result [build_wrapper [standard_output_file "testglue.o"]]
Sandra> + set result [build_wrapper "testglue.o"]
Sandra>   if { $result != "" } {
Sandra>      set gdb_wrapper_file [lindex $result 0]
Sandra> +    if ![is_remote host] {
Sandra> + set gdb_wrapper_file [file join [pwd] $gdb_wrapper_file]

If "make -j check" is used, isn't it possible that multiple runs will
use the same file name and then clash?

Tom
Reply | Threaded
Open this post in threaded view
|

Re: [patch] gdb/testsuite: Fix more bugs in gdb testglue wrapper handling

Sandra Loosemore
On 7/21/20 12:13 PM, Tom Tromey wrote:

>>>>>> "Sandra" == Sandra Loosemore <[hidden email]> writes:
>
> Sandra>      if {[target_info exists needs_status_wrapper] && \
> Sandra>      [target_info needs_status_wrapper] != "0"} {
> Sandra> - set result [build_wrapper [standard_output_file "testglue.o"]]
> Sandra> + set result [build_wrapper "testglue.o"]
> Sandra>   if { $result != "" } {
> Sandra>      set gdb_wrapper_file [lindex $result 0]
> Sandra> +    if ![is_remote host] {
> Sandra> + set gdb_wrapper_file [file join [pwd] $gdb_wrapper_file]
>
> If "make -j check" is used, isn't it possible that multiple runs will
> use the same file name and then clash?

Well, the first part of this patch hunk is undoing an incorrect change
from commit 24ac169ac5a918cd82b7485935f0c40a094c625e -- incorrect
because standard_output_file returns a pathname relative to build, which
cannot generally work on remote host.  Things were apparently working
fine for years up until February.  :-S

The second part is just the usual idiom to convert a relative pathname
to absolute in TCL, so that we can refer to it correctly when [pwd] is
something else.  That was actually the problem the previous commit was
trying to fix (incorrectly, by trying to rebuild the wrapper in [pwd],
and leaving the .o files polluting the source directory in some cases).

-Sandra
Reply | Threaded
Open this post in threaded view
|

Re: [patch] gdb/testsuite: Fix more bugs in gdb testglue wrapper handling

Tom Tromey-2
>>>>> "Sandra" == Sandra Loosemore <[hidden email]> writes:

Sandra> Well, the first part of this patch hunk is undoing an incorrect change
Sandra> from commit 24ac169ac5a918cd82b7485935f0c40a094c625e -- incorrect
Sandra> because standard_output_file returns a pathname relative to build,
Sandra> which cannot generally work on remote host.  Things were apparently
Sandra> working fine for years up until February.  :-S

Maybe nobody ever tried parallel tests in this scenario?

On the one hand, I'd hate to reintroduce a latent bug.
On the other hand, few people seem to use this code and I'd rather not
stand in the way of progress for some scenario that's not relevant.

So, the patch is OK with me.  Thanks.

Tom