public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RESEND PATCH 0/1] sim/testsuite: Trim extra path from arch
       [not found] <28df7073ec5a8f601ba47c9d2f7d4a7a0ce08753.1657795051.git.research_trasio@irq.a4lg.com>
@ 2022-08-27  1:53 ` Tsukasa OI
  2022-08-27  1:53   ` [RESEND PATCH 1/1] " Tsukasa OI
                     ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Tsukasa OI @ 2022-08-27  1:53 UTC (permalink / raw)
  To: Tsukasa OI, Andrew Burgess, Mike Frysinger, Nelson Chu, Kito Cheng
  Cc: gdb-patches

Hello,

The patch is a RESEND of
<https://sourceware.org/pipermail/binutils/2022-July/121814.html>
but to the right mailing list and completely new cover letter.

Because my copyright assignment is complete and I know the right place to
talk, let me describe the details.

I first reproduced this bug while I'm adding simulator tests to RISC-V
but talking with AArch64 (with some real tests) would be better.


[How to reproduce]

1.  Configure Binutils with aarch64-unknown-elf and build it
    /src/binutils/configure \
        --target=aarch64-unknown-elf \
        --prefix=/opt/cross/aarch64-unknown-elf \
        --enable-multilib
    && make
2.  Run `make check-sim' and confirmed that the simulator tests "pass"
3.  Intentionally try to fail the test by modifying
    `sim/testsuite/aarch64/pass.s'
    (replace the last line from "pass" to "fail")
4.  Run `make check-sim' and "confirmed" that the simulator tests "pass"


[Analysis]

... Yes, something is going wrong.  After the test, you can see the test
log in `sim/testsuite/aarch64/allinsn/testrun.log':

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Test run by builduser on Sat Aug 27 01:04:31 2022
Target is aarch64-unknown-elf
Host   is x86_64-pc-linux-gnu

                ===  tests ===

Schedule of variations:
    unix

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /src/binutils/sim/testsuite/config/default.exp as tool-and-target-specific interface file.
WARNING: Assuming target board is the local machine (which is probably wrong).
You may need to set your DEJAGNU environment variable.
Running /src/binutils/sim/testsuite/./aarch64/allinsn.exp ...
Can't find a compatible assembler
Executing on host: false /src/binutils/sim/testsuite/lib/compilercheck.c     -E -o testsuite/./aarch64/allinsn/compilercheck.x    (timeout = 300)
spawn -ignore SIGHUP false /src/binutils/sim/testsuite/lib/compilercheck.c -E -o testsuite/./aarch64/allinsn/compilercheck.x
compiler exited with status 1
Can't find a compatible C compiler
Testing adds.s on machine aarch64.
UNTESTED: ./aarch64/adds.s
[...]
Testing pass.s on machine aarch64.
UNTESTED: ./aarch64/pass.s
[...]
Testing xtn.s on machine aarch64.
UNTESTED: ./aarch64/xtn.s
testcase /src/binutils/sim/testsuite/./aarch64/allinsn.exp completed in 0 seconds

                ===  Summary ===

# of untested testcases         27
runtest completed at Sat Aug 27 01:04:31 2022
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<


We clearly find that the testcase didn't find recently built assembler.
That assembler (and the linker) is supposed to be used because the
simulator itself (sim/Makefile) sets its configuration.

Quoting `sim/site.exp' and `sim/site-sim-config.exp':
    set AS_FOR_TARGET_AARCH64 "/build/binutils/./gas/as-new"
    set LD_FOR_TARGET_AARCH64 "/build/binutils/./ld/ld-new"


At last, we find `sim/testsuite/lib/sim-defs.exp'.

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
proc sim_init_toolchain {} {
[...]
    set arch [sim_arch]
    set ARCH [string map {- _} [string toupper $arch]]
    foreach var {AS LD CC} {
    set var_for_target "${var}_FOR_TARGET"
    global $var_for_target
    set var_for_target_arch "${var_for_target}_${ARCH}"
    global $var_for_target_arch

    if [info exists $var_for_target_arch] {
        set $var_for_target [set $var_for_target_arch]
    } else {
        set $var_for_target ""
    }
[...]
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<


In here, it extracts {AS,LD,CC}_FOR_TARGET_AARCH64 and sets proper
{AS,LD,CC}_FOR_TARGET.  At least, it is supposed to do so.

However, this block doesn't work because the `arch' variable returned by
the `sim_arch' function is "./aarch64".  That is supposed to be "aarch64".

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
proc sim_arch {} {
    global subdir
    set arch "$subdir"
    while { [file dirname $arch] != "." } {
    set arch [file dirname $arch]
    }
    return "$arch"
}
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<


`sim_arch' function is simple.  Until the dirname part of the "arch" is not
"." (current directory), it trims the filename part.

For instance, if subdir is "A/B/C/D", this function returns "A".  However,
if subdir is "./A/B/C/D", this function returns "./A", not "A".
In fact, actual subdir value here is "./aarch64" and we will get
"./aarch64" for [sim_arch].  As a result, it fails to extract proper
assembler/linker/compiler for given target.


[Fix: Patch Details]

After this patch, `sim/testsuite/aarch64/allinsn/testrun.log' will
look like this:

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Test run by builduser on Sat Aug 27 01:34:29 2022
Target is aarch64-unknown-elf
Host   is x86_64-pc-linux-gnu

                ===  tests ===

Schedule of variations:
    unix

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /src/binutils/sim/testsuite/config/default.exp as tool-and-target-specific interface file.
WARNING: Assuming target board is the local machine (which is probably wrong).
You may need to set your DEJAGNU environment variable.
Running /src/binutils/sim/testsuite/./aarch64/allinsn.exp ...
Found a compatible assembler
Can't execute C compiler
Testing adds.s on machine aarch64.
Executing on host: /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/adds.s  -I/src/binutils/sim/testsuite/./aarch64  -o testsuite/./aarch64/allinsn/adds.s.o    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/adds.s -I/src/binutils/sim/testsuite/./aarch64 -o testsuite/./aarch64/allinsn/adds.s.o
Executing on host: /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/adds.s.o   -o testsuite/./aarch64/allinsn/adds.s.x    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/adds.s.o -o testsuite/./aarch64/allinsn/adds.s.x
./aarch64/run    testsuite/./aarch64/allinsn/adds.s.x
spawn ./aarch64/run testsuite/./aarch64/allinsn/adds.s.x
pass
PASS: aarch64 adds.s
[...]
Testing pass.s on machine aarch64.
Executing on host: /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/pass.s  -I/src/binutils/sim/testsuite/./aarch64  -o testsuite/./aarch64/allinsn/pass.s.o    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/pass.s -I/src/binutils/sim/testsuite/./aarch64 -o testsuite/./aarch64/allinsn/pass.s.o
Executing on host: /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/pass.s.o   -o testsuite/./aarch64/allinsn/pass.s.x    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/pass.s.o -o testsuite/./aarch64/allinsn/pass.s.x
./aarch64/run    testsuite/./aarch64/allinsn/pass.s.x
spawn ./aarch64/run testsuite/./aarch64/allinsn/pass.s.x
fail
status:  0
output:  fail

pattern: pass

FAIL: aarch64 pass.s (execution)
[...]
Testing xtn.s on machine aarch64.
Executing on host: /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/xtn.s  -I/src/binutils/sim/testsuite/./aarch64  -o testsuite/./aarch64/allinsn/xtn.s.o    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/xtn.s -I/src/binutils/sim/testsuite/./aarch64 -o testsuite/./aarch64/allinsn/xtn.s.o
Executing on host: /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/xtn.s.o   -o testsuite/./aarch64/allinsn/xtn.s.x    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/xtn.s.o -o testsuite/./aarch64/allinsn/xtn.s.x
./aarch64/run    testsuite/./aarch64/allinsn/xtn.s.x
spawn ./aarch64/run testsuite/./aarch64/allinsn/xtn.s.x
pass
PASS: aarch64 xtn.s
testcase /src/binutils/sim/testsuite/./aarch64/allinsn.exp completed in 0 seconds

                ===  Summary ===

# of expected passes            26
# of unexpected failures        1
runtest completed at Sat Aug 27 01:34:29 2022
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<


OK, we successfully failed.  If we restore `sim/testsuite/aarch64/pass.s',
it completely succeeds.

This patch trims "./" part of "./arch" using the `file tail' function.
Even if the subdir input is "aarch64", it returns "aarch64".
With this patch, we can now successfully test the simulator.


This is effectively a prerequisite of my Zmmul patchset on RISC-V because
I (and another person who also submitted Zmmul patchset) accidentally broke
the simulator while adding Zmmul and my Zmmul patchset contains a test case
to make sure that the simulator works as expected.

Thanks,
Tsukasa




Tsukasa OI (1):
  sim/testsuite: Trim extra path from arch

 sim/testsuite/lib/sim-defs.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: 46e59b72f21029f2a863e3828cec76a03283b49d
-- 
2.34.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [RESEND PATCH 1/1] sim/testsuite: Trim extra path from arch
  2022-08-27  1:53 ` [RESEND PATCH 0/1] sim/testsuite: Trim extra path from arch Tsukasa OI
@ 2022-08-27  1:53   ` Tsukasa OI
  2022-09-14 10:56   ` [PING^2 PATCH 0/1] " Tsukasa OI
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 16+ messages in thread
From: Tsukasa OI @ 2022-08-27  1:53 UTC (permalink / raw)
  To: Tsukasa OI, Andrew Burgess, Mike Frysinger, Nelson Chu, Kito Cheng
  Cc: gdb-patches

If subdir begins with "./", sim_arch returned "./ARCH" instead of
"ARCH" and target-specific check-sim is prevented.  This commit fixes
the issue to make `make check-sim' work.

sim/ChangeLog:

	* testsuite/lib/sim-defs.exp: Fix return value of sim_arch.
---
 sim/testsuite/lib/sim-defs.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sim/testsuite/lib/sim-defs.exp b/sim/testsuite/lib/sim-defs.exp
index 5528d64684b..6d8f0134089 100644
--- a/sim/testsuite/lib/sim-defs.exp
+++ b/sim/testsuite/lib/sim-defs.exp
@@ -14,7 +14,7 @@ proc sim_arch {} {
     while { [file dirname $arch] != "." } {
 	set arch [file dirname $arch]
     }
-    return "$arch"
+    return [file tail $arch]
 }
 
 # Initialize the testrun.
-- 
2.34.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PING^2 PATCH 0/1] sim/testsuite: Trim extra path from arch
  2022-08-27  1:53 ` [RESEND PATCH 0/1] sim/testsuite: Trim extra path from arch Tsukasa OI
  2022-08-27  1:53   ` [RESEND PATCH 1/1] " Tsukasa OI
@ 2022-09-14 10:56   ` Tsukasa OI
  2022-09-14 10:56     ` [PING^2 PATCH 1/1] " Tsukasa OI
  2022-09-21 16:02     ` [PING^3 PATCH 0/1] sim/testsuite: Fix broken "make check-sim" by trimming excess " Tsukasa OI
  2022-10-23 19:16   ` [RESEND PATCH 0/1] sim/testsuite: Trim extra " Mike Frysinger
  2022-10-23 19:16   ` [PATCH] sim: testsuite: tweak parallel find invocation [PR sim/29596] Mike Frysinger
  3 siblings, 2 replies; 16+ messages in thread
From: Tsukasa OI @ 2022-09-14 10:56 UTC (permalink / raw)
  To: Tsukasa OI; +Cc: gdb-patches

NOTE: the cover letter may change each time I ping.

Hello,

This patch fixes the problem which make check-sim (testing the simulator)
doesn't work on many targets.

The patch is a PING (2) of
<https://sourceware.org/pipermail/binutils/2022-July/121814.html>
<https://sourceware.org/pipermail/gdb-patches/2022-August/191564.html>.
Tracker on GitHub:
<https://github.com/a4lg/binutils-gdb/wiki/sim_trim_path_from_arch>

I first reproduced this bug while I'm adding simulator tests to RISC-V
but talking with AArch64 (with some upstream tests) would be better.



[How to reproduce]

1.  Configure Binutils with aarch64-unknown-elf and build it
    /src/binutils/configure --target=aarch64-unknown-elf && make
2.  Run `make check-sim' and confirmed that the simulator tests "pass"
3.  Intentionally try to fail the test by modifying
    `sim/testsuite/aarch64/pass.s'
    (replace the last line from "pass" to "fail")
4.  Run `make check-sim' and "confirmed" that the simulator tests "pass"
    (it should fail!)



[Analysis]

... Yes, something is going wrong.  After the test, you can see the test
log in `sim/testsuite/aarch64/allinsn/testrun.log':

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Test run by builduser on Sat Aug 27 01:04:31 2022
Target is aarch64-unknown-elf
Host   is x86_64-pc-linux-gnu

                ===  tests ===

Schedule of variations:
    unix

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /src/binutils/sim/testsuite/config/default.exp as tool-and-target-specific interface file.
WARNING: Assuming target board is the local machine (which is probably wrong).
You may need to set your DEJAGNU environment variable.
Running /src/binutils/sim/testsuite/./aarch64/allinsn.exp ...
Can't find a compatible assembler
Executing on host: false /src/binutils/sim/testsuite/lib/compilercheck.c     -E -o testsuite/./aarch64/allinsn/compilercheck.x    (timeout = 300)
spawn -ignore SIGHUP false /src/binutils/sim/testsuite/lib/compilercheck.c -E -o testsuite/./aarch64/allinsn/compilercheck.x
compiler exited with status 1
Can't find a compatible C compiler
Testing adds.s on machine aarch64.
UNTESTED: ./aarch64/adds.s
Testing addv.s on machine aarch64.
UNTESTED: ./aarch64/addv.s
[...]
Testing pass.s on machine aarch64.
UNTESTED: ./aarch64/pass.s
[...]
Testing xtn.s on machine aarch64.
UNTESTED: ./aarch64/xtn.s
testcase /src/binutils/sim/testsuite/./aarch64/allinsn.exp completed in 0 seconds

                ===  Summary ===

# of untested testcases         27
runtest completed at Sat Aug 27 01:04:31 2022
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<


We clearly find that the test runner didn't find recently built assembler
(gas/as-new).  That assembler (and the linker) is supposed to be used
because the simulator itself (sim/Makefile) sets its configuration.

Quoting `sim/site.exp' and `sim/site-sim-config.exp':
    set AS_FOR_TARGET_AARCH64 "/build/binutils/./gas/as-new"
    set LD_FOR_TARGET_AARCH64 "/build/binutils/./ld/ld-new"


At last, we find `sim/testsuite/lib/sim-defs.exp'.

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
proc sim_init_toolchain {} {
[...]
    set arch [sim_arch]
    set ARCH [string map {- _} [string toupper $arch]]
    foreach var {AS LD CC} {
    set var_for_target "${var}_FOR_TARGET"
    global $var_for_target
    set var_for_target_arch "${var_for_target}_${ARCH}"
    global $var_for_target_arch

    if [info exists $var_for_target_arch] {
        set $var_for_target [set $var_for_target_arch]
    } else {
        set $var_for_target ""
    }
[...]
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<


In here, it extracts {AS,LD,CC}_FOR_TARGET_AARCH64 and sets proper
{AS,LD,CC}_FOR_TARGET.  At least, it is supposed to do so.

However, this block doesn't work because the `arch' variable returned by
the `sim_arch' function is "./aarch64".  That is supposed to be "aarch64".

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
proc sim_arch {} {
    global subdir
    set arch "$subdir"
    while { [file dirname $arch] != "." } {
    set arch [file dirname $arch]
    }
    return "$arch"
}
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<



[Fix: Cause and Patch Details]

`sim_arch' function is simple.  Until the dirname part of the "arch" is not
"." (current directory), it trims the filename part.

For instance, if subdir is "A/B/C/D", this function returns "A".  However,
if subdir is "./A/B/C/D", this function returns "./A", not "A".
In fact, actual subdir value here is "./aarch64" and we will get
"./aarch64" for [sim_arch].  As a result, it fails to extract proper
assembler/linker/compiler for given target.

To deal with it, I added another "file tail" function call before returning.



[Analysis: After the Patch]

After this patch, `sim/testsuite/aarch64/allinsn/testrun.log' will
look like this:

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Test run by builduser on Sat Aug 27 01:34:29 2022
Target is aarch64-unknown-elf
Host   is x86_64-pc-linux-gnu

                ===  tests ===

Schedule of variations:
    unix

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /src/binutils/sim/testsuite/config/default.exp as tool-and-target-specific interface file.
WARNING: Assuming target board is the local machine (which is probably wrong).
You may need to set your DEJAGNU environment variable.
Running /src/binutils/sim/testsuite/./aarch64/allinsn.exp ...
Found a compatible assembler
Can't execute C compiler
Testing adds.s on machine aarch64.
Executing on host: /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/adds.s  -I/src/binutils/sim/testsuite/./aarch64  -o testsuite/./aarch64/allinsn/adds.s.o    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/adds.s -I/src/binutils/sim/testsuite/./aarch64 -o testsuite/./aarch64/allinsn/adds.s.o
Executing on host: /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/adds.s.o   -o testsuite/./aarch64/allinsn/adds.s.x    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/adds.s.o -o testsuite/./aarch64/allinsn/adds.s.x
./aarch64/run    testsuite/./aarch64/allinsn/adds.s.x
spawn ./aarch64/run testsuite/./aarch64/allinsn/adds.s.x
pass
PASS: aarch64 adds.s
[...]
Testing pass.s on machine aarch64.
Executing on host: /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/pass.s  -I/src/binutils/sim/testsuite/./aarch64  -o testsuite/./aarch64/allinsn/pass.s.o    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/pass.s -I/src/binutils/sim/testsuite/./aarch64 -o testsuite/./aarch64/allinsn/pass.s.o
Executing on host: /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/pass.s.o   -o testsuite/./aarch64/allinsn/pass.s.x    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/pass.s.o -o testsuite/./aarch64/allinsn/pass.s.x
./aarch64/run    testsuite/./aarch64/allinsn/pass.s.x
spawn ./aarch64/run testsuite/./aarch64/allinsn/pass.s.x
fail
status:  0
output:  fail

pattern: pass

FAIL: aarch64 pass.s (execution)
[...]
Testing xtn.s on machine aarch64.
Executing on host: /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/xtn.s  -I/src/binutils/sim/testsuite/./aarch64  -o testsuite/./aarch64/allinsn/xtn.s.o    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/xtn.s -I/src/binutils/sim/testsuite/./aarch64 -o testsuite/./aarch64/allinsn/xtn.s.o
Executing on host: /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/xtn.s.o   -o testsuite/./aarch64/allinsn/xtn.s.x    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/xtn.s.o -o testsuite/./aarch64/allinsn/xtn.s.x
./aarch64/run    testsuite/./aarch64/allinsn/xtn.s.x
spawn ./aarch64/run testsuite/./aarch64/allinsn/xtn.s.x
pass
PASS: aarch64 xtn.s
testcase /src/binutils/sim/testsuite/./aarch64/allinsn.exp completed in 0 seconds

                ===  Summary ===

# of expected passes            26
# of unexpected failures        1
runtest completed at Sat Aug 27 01:34:29 2022
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<


OK, we "successfully" failed.  If we restore `sim/testsuite/aarch64/pass.s',
it completely succeeds.

This patch trims "./" part of "./arch" using the `file tail' function.
Even if the subdir input is "aarch64", it returns "aarch64".
With this patch, we can now successfully test the simulator.


Regards,
Tsukasa




Tsukasa OI (1):
  sim/testsuite: Trim extra path from arch

 sim/testsuite/lib/sim-defs.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: e959744eca88a4d145f39d5fbf4ab095af0f16b4
-- 
2.34.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PING^2 PATCH 1/1] sim/testsuite: Trim extra path from arch
  2022-09-14 10:56   ` [PING^2 PATCH 0/1] " Tsukasa OI
@ 2022-09-14 10:56     ` Tsukasa OI
  2022-09-21 16:02     ` [PING^3 PATCH 0/1] sim/testsuite: Fix broken "make check-sim" by trimming excess " Tsukasa OI
  1 sibling, 0 replies; 16+ messages in thread
From: Tsukasa OI @ 2022-09-14 10:56 UTC (permalink / raw)
  To: Tsukasa OI; +Cc: gdb-patches

If subdir begins with "./", sim_arch returned "./ARCH" instead of
"ARCH" and target-specific check-sim is prevented.  This commit fixes
the issue to make `make check-sim' work.

sim/ChangeLog:

	* testsuite/lib/sim-defs.exp: Fix return value of sim_arch.
---
 sim/testsuite/lib/sim-defs.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sim/testsuite/lib/sim-defs.exp b/sim/testsuite/lib/sim-defs.exp
index 5528d64684b..6d8f0134089 100644
--- a/sim/testsuite/lib/sim-defs.exp
+++ b/sim/testsuite/lib/sim-defs.exp
@@ -14,7 +14,7 @@ proc sim_arch {} {
     while { [file dirname $arch] != "." } {
 	set arch [file dirname $arch]
     }
-    return "$arch"
+    return [file tail $arch]
 }
 
 # Initialize the testrun.
-- 
2.34.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PING^3 PATCH 0/1] sim/testsuite: Fix broken "make check-sim" by trimming excess path from arch
  2022-09-14 10:56   ` [PING^2 PATCH 0/1] " Tsukasa OI
  2022-09-14 10:56     ` [PING^2 PATCH 1/1] " Tsukasa OI
@ 2022-09-21 16:02     ` Tsukasa OI
  2022-09-21 16:02       ` [PING^3 PATCH 1/1] sim/testsuite: Trim extra " Tsukasa OI
  2022-10-09  3:55       ` [PING^4 PATCH 0/1] sim/testsuite: PR29596, Fix broken "make check-sim" by trimming excess " Tsukasa OI
  1 sibling, 2 replies; 16+ messages in thread
From: Tsukasa OI @ 2022-09-21 16:02 UTC (permalink / raw)
  To: Tsukasa OI; +Cc: gdb-patches

TODO
NOTE: the cover letter and subject may change each time I ping.

Hello,

This patch fixes the problem which make check-sim (testing the simulator)
doesn't work on many targets.

The patch is a PING (2) of
<https://sourceware.org/pipermail/binutils/2022-July/121814.html>
<https://sourceware.org/pipermail/gdb-patches/2022-August/191564.html>.
Tracker on GitHub:
<https://github.com/a4lg/binutils-gdb/wiki/sim_trim_path_from_arch>

I first reproduced this bug while I'm adding simulator tests to RISC-V
but talking with AArch64 (with some upstream tests) would be better.



[How to reproduce]

1.  Configure Binutils with aarch64-unknown-elf and build it
    /src/binutils/configure --target=aarch64-unknown-elf && make
2.  Run `make check-sim' and confirmed that the simulator tests "pass"
3.  Intentionally try to fail the test by modifying
    `sim/testsuite/aarch64/pass.s'
    (replace the last line from "pass" to "fail")
4.  Run `make check-sim' and "confirmed" that the simulator tests "pass"
    (it should fail!)



[Analysis]

... Yes, something is going wrong.  After the test, you can see the test
log in `sim/testsuite/aarch64/allinsn/testrun.log':

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Test run by builduser on Sat Aug 27 01:04:31 2022
Target is aarch64-unknown-elf
Host   is x86_64-pc-linux-gnu

                ===  tests ===

Schedule of variations:
    unix

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /src/binutils/sim/testsuite/config/default.exp as tool-and-target-specific interface file.
WARNING: Assuming target board is the local machine (which is probably wrong).
You may need to set your DEJAGNU environment variable.
Running /src/binutils/sim/testsuite/./aarch64/allinsn.exp ...
Can't find a compatible assembler
Executing on host: false /src/binutils/sim/testsuite/lib/compilercheck.c     -E -o testsuite/./aarch64/allinsn/compilercheck.x    (timeout = 300)
spawn -ignore SIGHUP false /src/binutils/sim/testsuite/lib/compilercheck.c -E -o testsuite/./aarch64/allinsn/compilercheck.x
compiler exited with status 1
Can't find a compatible C compiler
Testing adds.s on machine aarch64.
UNTESTED: ./aarch64/adds.s
Testing addv.s on machine aarch64.
UNTESTED: ./aarch64/addv.s
[...]
Testing pass.s on machine aarch64.
UNTESTED: ./aarch64/pass.s
[...]
Testing xtn.s on machine aarch64.
UNTESTED: ./aarch64/xtn.s
testcase /src/binutils/sim/testsuite/./aarch64/allinsn.exp completed in 0 seconds

                ===  Summary ===

# of untested testcases         27
runtest completed at Sat Aug 27 01:04:31 2022
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<


We clearly find that the test runner didn't find recently built assembler
(gas/as-new).  That assembler (and the linker) is supposed to be used
because the simulator itself (sim/Makefile) sets its configuration.

Quoting `sim/site.exp' and `sim/site-sim-config.exp':
    set AS_FOR_TARGET_AARCH64 "/build/binutils/./gas/as-new"
    set LD_FOR_TARGET_AARCH64 "/build/binutils/./ld/ld-new"


At last, we find `sim/testsuite/lib/sim-defs.exp'.

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
proc sim_init_toolchain {} {
[...]
    set arch [sim_arch]
    set ARCH [string map {- _} [string toupper $arch]]
    foreach var {AS LD CC} {
    set var_for_target "${var}_FOR_TARGET"
    global $var_for_target
    set var_for_target_arch "${var_for_target}_${ARCH}"
    global $var_for_target_arch

    if [info exists $var_for_target_arch] {
        set $var_for_target [set $var_for_target_arch]
    } else {
        set $var_for_target ""
    }
[...]
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<


In here, it extracts {AS,LD,CC}_FOR_TARGET_AARCH64 and sets proper
{AS,LD,CC}_FOR_TARGET.  At least, it is supposed to do so.

However, this block doesn't work because the `arch' variable returned by
the `sim_arch' function is "./aarch64".  That is supposed to be "aarch64".

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
proc sim_arch {} {
    global subdir
    set arch "$subdir"
    while { [file dirname $arch] != "." } {
    set arch [file dirname $arch]
    }
    return "$arch"
}
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<



[Fix: Cause and Patch Details]

`sim_arch' function is simple.  Until the dirname part of the "arch" is not
"." (current directory), it trims the filename part.

For instance, if subdir is "A/B/C/D", this function returns "A".  However,
if subdir is "./A/B/C/D", this function returns "./A", not "A".
In fact, actual subdir value here is "./aarch64" and we will get
"./aarch64" for [sim_arch].  As a result, it fails to extract proper
assembler/linker/compiler for given target.

To deal with it, I added another "file tail" function call before returning.



[Analysis: After the Patch]

After this patch, `sim/testsuite/aarch64/allinsn/testrun.log' will
look like this:

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Test run by builduser on Sat Aug 27 01:34:29 2022
Target is aarch64-unknown-elf
Host   is x86_64-pc-linux-gnu

                ===  tests ===

Schedule of variations:
    unix

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /src/binutils/sim/testsuite/config/default.exp as tool-and-target-specific interface file.
WARNING: Assuming target board is the local machine (which is probably wrong).
You may need to set your DEJAGNU environment variable.
Running /src/binutils/sim/testsuite/./aarch64/allinsn.exp ...
Found a compatible assembler
Can't execute C compiler
Testing adds.s on machine aarch64.
Executing on host: /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/adds.s  -I/src/binutils/sim/testsuite/./aarch64  -o testsuite/./aarch64/allinsn/adds.s.o    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/adds.s -I/src/binutils/sim/testsuite/./aarch64 -o testsuite/./aarch64/allinsn/adds.s.o
Executing on host: /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/adds.s.o   -o testsuite/./aarch64/allinsn/adds.s.x    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/adds.s.o -o testsuite/./aarch64/allinsn/adds.s.x
./aarch64/run    testsuite/./aarch64/allinsn/adds.s.x
spawn ./aarch64/run testsuite/./aarch64/allinsn/adds.s.x
pass
PASS: aarch64 adds.s
[...]
Testing pass.s on machine aarch64.
Executing on host: /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/pass.s  -I/src/binutils/sim/testsuite/./aarch64  -o testsuite/./aarch64/allinsn/pass.s.o    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/pass.s -I/src/binutils/sim/testsuite/./aarch64 -o testsuite/./aarch64/allinsn/pass.s.o
Executing on host: /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/pass.s.o   -o testsuite/./aarch64/allinsn/pass.s.x    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/pass.s.o -o testsuite/./aarch64/allinsn/pass.s.x
./aarch64/run    testsuite/./aarch64/allinsn/pass.s.x
spawn ./aarch64/run testsuite/./aarch64/allinsn/pass.s.x
fail
status:  0
output:  fail

pattern: pass

FAIL: aarch64 pass.s (execution)
[...]
Testing xtn.s on machine aarch64.
Executing on host: /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/xtn.s  -I/src/binutils/sim/testsuite/./aarch64  -o testsuite/./aarch64/allinsn/xtn.s.o    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/xtn.s -I/src/binutils/sim/testsuite/./aarch64 -o testsuite/./aarch64/allinsn/xtn.s.o
Executing on host: /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/xtn.s.o   -o testsuite/./aarch64/allinsn/xtn.s.x    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/xtn.s.o -o testsuite/./aarch64/allinsn/xtn.s.x
./aarch64/run    testsuite/./aarch64/allinsn/xtn.s.x
spawn ./aarch64/run testsuite/./aarch64/allinsn/xtn.s.x
pass
PASS: aarch64 xtn.s
testcase /src/binutils/sim/testsuite/./aarch64/allinsn.exp completed in 0 seconds

                ===  Summary ===

# of expected passes            26
# of unexpected failures        1
runtest completed at Sat Aug 27 01:34:29 2022
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<


OK, we "successfully" failed.  If we restore `sim/testsuite/aarch64/pass.s',
it completely succeeds.

This patch trims "./" part of "./arch" using the `file tail' function.
Even if the subdir input is "aarch64", it returns "aarch64".
With this patch, we can now successfully test the simulator.


Regards,
Tsukasa




Tsukasa OI (1):
  sim/testsuite: Trim extra path from arch

 sim/testsuite/lib/sim-defs.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: 0242db993f8ad0a0de16dafc4ebd35bb6190c833
-- 
2.34.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PING^3 PATCH 1/1] sim/testsuite: Trim extra path from arch
  2022-09-21 16:02     ` [PING^3 PATCH 0/1] sim/testsuite: Fix broken "make check-sim" by trimming excess " Tsukasa OI
@ 2022-09-21 16:02       ` Tsukasa OI
  2022-10-09  3:55       ` [PING^4 PATCH 0/1] sim/testsuite: PR29596, Fix broken "make check-sim" by trimming excess " Tsukasa OI
  1 sibling, 0 replies; 16+ messages in thread
From: Tsukasa OI @ 2022-09-21 16:02 UTC (permalink / raw)
  To: Tsukasa OI; +Cc: gdb-patches

If subdir begins with "./", sim_arch returned "./ARCH" instead of
"ARCH" and target-specific check-sim is prevented.  This commit fixes
the issue to make "make check-sim" work.

sim/ChangeLog:

	* testsuite/lib/sim-defs.exp: Fix return value of sim_arch
	by trimming leading "./".
---
 sim/testsuite/lib/sim-defs.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sim/testsuite/lib/sim-defs.exp b/sim/testsuite/lib/sim-defs.exp
index 5528d64684b..6d8f0134089 100644
--- a/sim/testsuite/lib/sim-defs.exp
+++ b/sim/testsuite/lib/sim-defs.exp
@@ -14,7 +14,7 @@ proc sim_arch {} {
     while { [file dirname $arch] != "." } {
 	set arch [file dirname $arch]
     }
-    return "$arch"
+    return [file tail $arch]
 }
 
 # Initialize the testrun.
-- 
2.34.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PING^4 PATCH 0/1] sim/testsuite: PR29596, Fix broken "make check-sim" by trimming excess path from arch
  2022-09-21 16:02     ` [PING^3 PATCH 0/1] sim/testsuite: Fix broken "make check-sim" by trimming excess " Tsukasa OI
  2022-09-21 16:02       ` [PING^3 PATCH 1/1] sim/testsuite: Trim extra " Tsukasa OI
@ 2022-10-09  3:55       ` Tsukasa OI
  2022-10-09  3:55         ` [PING^4 PATCH 1/1] sim/testsuite: PR29596, Trim extra " Tsukasa OI
  2022-10-11 11:06         ` [PING^4 PATCH 0/1] sim/testsuite: PR29596, Fix broken "make check-sim" by trimming excess " Andrew Burgess
  1 sibling, 2 replies; 16+ messages in thread
From: Tsukasa OI @ 2022-10-09  3:55 UTC (permalink / raw)
  To: Tsukasa OI, Andrew Burgess, Mike Frysinger; +Cc: gdb-patches

TODO
NOTE: the cover letter and subject may change each time I ping.

Hello,

This patch fixes the problem which make check-sim (testing the simulator)
doesn't work on many targets.

The patch is a PING (4) of
<https://sourceware.org/pipermail/binutils/2022-July/121814.html>
<https://sourceware.org/pipermail/gdb-patches/2022-August/191564.html>.
Tracker on GitHub:
<https://github.com/a4lg/binutils-gdb/wiki/sim_trim_path_from_arch>

In this ping, I removed ChangeLog and won't change anymore (unless I miss
something).



I first reproduced this bug while I'm adding simulator tests to RISC-V
but talking with AArch64 (with some upstream tests) would be better.



[How to reproduce]

1.  Configure Binutils with aarch64-unknown-elf and build it
    /src/binutils/configure --target=aarch64-unknown-elf && make
2.  Run `make check-sim' and confirmed that the simulator tests "pass"
3.  Intentionally try to fail the test by modifying
    `sim/testsuite/aarch64/pass.s'
    (replace the last line from "pass" to "fail")
4.  Run `make check-sim' and "confirmed" that the simulator tests "pass"
    (it should fail!)



[Analysis]

... Yes, something is going wrong.  After the test, you can see the test
log in `sim/testsuite/aarch64/allinsn/testrun.log':

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Test run by builduser on Sat Aug 27 01:04:31 2022
Target is aarch64-unknown-elf
Host   is x86_64-pc-linux-gnu

                ===  tests ===

Schedule of variations:
    unix

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /src/binutils/sim/testsuite/config/default.exp as tool-and-target-specific interface file.
WARNING: Assuming target board is the local machine (which is probably wrong).
You may need to set your DEJAGNU environment variable.
Running /src/binutils/sim/testsuite/./aarch64/allinsn.exp ...
Can't find a compatible assembler
Executing on host: false /src/binutils/sim/testsuite/lib/compilercheck.c     -E -o testsuite/./aarch64/allinsn/compilercheck.x    (timeout = 300)
spawn -ignore SIGHUP false /src/binutils/sim/testsuite/lib/compilercheck.c -E -o testsuite/./aarch64/allinsn/compilercheck.x
compiler exited with status 1
Can't find a compatible C compiler
Testing adds.s on machine aarch64.
UNTESTED: ./aarch64/adds.s
Testing addv.s on machine aarch64.
UNTESTED: ./aarch64/addv.s
[...]
Testing pass.s on machine aarch64.
UNTESTED: ./aarch64/pass.s
[...]
Testing xtn.s on machine aarch64.
UNTESTED: ./aarch64/xtn.s
testcase /src/binutils/sim/testsuite/./aarch64/allinsn.exp completed in 0 seconds

                ===  Summary ===

# of untested testcases         27
runtest completed at Sat Aug 27 01:04:31 2022
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<


We clearly find that the test runner didn't find recently built assembler
(gas/as-new).  That assembler (and the linker) is supposed to be used
because the simulator itself (sim/Makefile) sets its configuration.

Quoting `sim/site.exp' and `sim/site-sim-config.exp':
    set AS_FOR_TARGET_AARCH64 "/build/binutils/./gas/as-new"
    set LD_FOR_TARGET_AARCH64 "/build/binutils/./ld/ld-new"


At last, we find `sim/testsuite/lib/sim-defs.exp'.

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
proc sim_init_toolchain {} {
[...]
    set arch [sim_arch]
    set ARCH [string map {- _} [string toupper $arch]]
    foreach var {AS LD CC} {
    set var_for_target "${var}_FOR_TARGET"
    global $var_for_target
    set var_for_target_arch "${var_for_target}_${ARCH}"
    global $var_for_target_arch

    if [info exists $var_for_target_arch] {
        set $var_for_target [set $var_for_target_arch]
    } else {
        set $var_for_target ""
    }
[...]
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<


In here, it extracts {AS,LD,CC}_FOR_TARGET_AARCH64 and sets proper
{AS,LD,CC}_FOR_TARGET.  At least, it is supposed to do so.

However, this block doesn't work because the `arch' variable returned by
the `sim_arch' function is "./aarch64".  That is supposed to be "aarch64".

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
proc sim_arch {} {
    global subdir
    set arch "$subdir"
    while { [file dirname $arch] != "." } {
    set arch [file dirname $arch]
    }
    return "$arch"
}
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<



[Fix: Cause and Patch Details]

`sim_arch' function is simple.  Until the dirname part of the "arch" is not
"." (current directory), it trims the filename part.

For instance, if subdir is "A/B/C/D", this function returns "A".  However,
if subdir is "./A/B/C/D", this function returns "./A", not "A".
In fact, actual subdir value here is "./aarch64" and we will get
"./aarch64" for [sim_arch].  As a result, it fails to extract proper
assembler/linker/compiler for given target.

To deal with it, I added another "file tail" function call before returning.



[Analysis: After the Patch]

After this patch, `sim/testsuite/aarch64/allinsn/testrun.log' will
look like this:

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Test run by builduser on Sat Aug 27 01:34:29 2022
Target is aarch64-unknown-elf
Host   is x86_64-pc-linux-gnu

                ===  tests ===

Schedule of variations:
    unix

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /src/binutils/sim/testsuite/config/default.exp as tool-and-target-specific interface file.
WARNING: Assuming target board is the local machine (which is probably wrong).
You may need to set your DEJAGNU environment variable.
Running /src/binutils/sim/testsuite/./aarch64/allinsn.exp ...
Found a compatible assembler
Can't execute C compiler
Testing adds.s on machine aarch64.
Executing on host: /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/adds.s  -I/src/binutils/sim/testsuite/./aarch64  -o testsuite/./aarch64/allinsn/adds.s.o    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/adds.s -I/src/binutils/sim/testsuite/./aarch64 -o testsuite/./aarch64/allinsn/adds.s.o
Executing on host: /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/adds.s.o   -o testsuite/./aarch64/allinsn/adds.s.x    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/adds.s.o -o testsuite/./aarch64/allinsn/adds.s.x
./aarch64/run    testsuite/./aarch64/allinsn/adds.s.x
spawn ./aarch64/run testsuite/./aarch64/allinsn/adds.s.x
pass
PASS: aarch64 adds.s
[...]
Testing pass.s on machine aarch64.
Executing on host: /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/pass.s  -I/src/binutils/sim/testsuite/./aarch64  -o testsuite/./aarch64/allinsn/pass.s.o    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/pass.s -I/src/binutils/sim/testsuite/./aarch64 -o testsuite/./aarch64/allinsn/pass.s.o
Executing on host: /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/pass.s.o   -o testsuite/./aarch64/allinsn/pass.s.x    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/pass.s.o -o testsuite/./aarch64/allinsn/pass.s.x
./aarch64/run    testsuite/./aarch64/allinsn/pass.s.x
spawn ./aarch64/run testsuite/./aarch64/allinsn/pass.s.x
fail
status:  0
output:  fail

pattern: pass

FAIL: aarch64 pass.s (execution)
[...]
Testing xtn.s on machine aarch64.
Executing on host: /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/xtn.s  -I/src/binutils/sim/testsuite/./aarch64  -o testsuite/./aarch64/allinsn/xtn.s.o    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/xtn.s -I/src/binutils/sim/testsuite/./aarch64 -o testsuite/./aarch64/allinsn/xtn.s.o
Executing on host: /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/xtn.s.o   -o testsuite/./aarch64/allinsn/xtn.s.x    (timeout = 300)
spawn -ignore SIGHUP /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/xtn.s.o -o testsuite/./aarch64/allinsn/xtn.s.x
./aarch64/run    testsuite/./aarch64/allinsn/xtn.s.x
spawn ./aarch64/run testsuite/./aarch64/allinsn/xtn.s.x
pass
PASS: aarch64 xtn.s
testcase /src/binutils/sim/testsuite/./aarch64/allinsn.exp completed in 0 seconds

                ===  Summary ===

# of expected passes            26
# of unexpected failures        1
runtest completed at Sat Aug 27 01:34:29 2022
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<


OK, we "successfully" failed.  If we restore `sim/testsuite/aarch64/pass.s',
it completely succeeds.

This patch trims "./" part of "./arch" using the `file tail' function.
Even if the subdir input is "aarch64", it returns "aarch64".
With this patch, we can now successfully test the simulator.


Regards,
Tsukasa




Tsukasa OI (1):
  sim/testsuite: PR29596, Trim extra path from arch

 sim/testsuite/lib/sim-defs.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: c10a862f17847bc9c50d680c87b87dc51ae4b95e
-- 
2.34.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PING^4 PATCH 1/1] sim/testsuite: PR29596, Trim extra path from arch
  2022-10-09  3:55       ` [PING^4 PATCH 0/1] sim/testsuite: PR29596, Fix broken "make check-sim" by trimming excess " Tsukasa OI
@ 2022-10-09  3:55         ` Tsukasa OI
  2022-10-11 11:06         ` [PING^4 PATCH 0/1] sim/testsuite: PR29596, Fix broken "make check-sim" by trimming excess " Andrew Burgess
  1 sibling, 0 replies; 16+ messages in thread
From: Tsukasa OI @ 2022-10-09  3:55 UTC (permalink / raw)
  To: Tsukasa OI, Andrew Burgess, Mike Frysinger; +Cc: gdb-patches

If subdir begins with "./", sim_arch returned "./ARCH" instead of
"ARCH" and target-specific check-sim is prevented.  This commit fixes
the issue (by trimming extra path from arch) to make "make check-sim" work.
---
 sim/testsuite/lib/sim-defs.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sim/testsuite/lib/sim-defs.exp b/sim/testsuite/lib/sim-defs.exp
index 5528d64684b..6d8f0134089 100644
--- a/sim/testsuite/lib/sim-defs.exp
+++ b/sim/testsuite/lib/sim-defs.exp
@@ -14,7 +14,7 @@ proc sim_arch {} {
     while { [file dirname $arch] != "." } {
 	set arch [file dirname $arch]
     }
-    return "$arch"
+    return [file tail $arch]
 }
 
 # Initialize the testrun.
-- 
2.34.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PING^4 PATCH 0/1] sim/testsuite: PR29596, Fix broken "make check-sim" by trimming excess path from arch
  2022-10-09  3:55       ` [PING^4 PATCH 0/1] sim/testsuite: PR29596, Fix broken "make check-sim" by trimming excess " Tsukasa OI
  2022-10-09  3:55         ` [PING^4 PATCH 1/1] sim/testsuite: PR29596, Trim extra " Tsukasa OI
@ 2022-10-11 11:06         ` Andrew Burgess
  2022-10-12  5:39           ` Tsukasa OI
  2022-10-16 13:01           ` Tsukasa OI
  1 sibling, 2 replies; 16+ messages in thread
From: Andrew Burgess @ 2022-10-11 11:06 UTC (permalink / raw)
  To: Tsukasa OI, Tsukasa OI, Mike Frysinger; +Cc: gdb-patches


Hi!

Thanks for looking at this issue.


Tsukasa OI <research_trasio@irq.a4lg.com> writes:

> TODO
> NOTE: the cover letter and subject may change each time I ping.

For a single patch series like this a cover letter really isn't needed.
All the information you've written in this cover letter would be better
included in the commit message for the patch.  It's all really useful
info that somebody looking at this patch in the future might want to
know.

>
> Hello,
>
> This patch fixes the problem which make check-sim (testing the simulator)
> doesn't work on many targets.
>
> The patch is a PING (4) of
> <https://sourceware.org/pipermail/binutils/2022-July/121814.html>
> <https://sourceware.org/pipermail/gdb-patches/2022-August/191564.html>.
> Tracker on GitHub:
> <https://github.com/a4lg/binutils-gdb/wiki/sim_trim_path_from_arch>
>
> In this ping, I removed ChangeLog and won't change anymore (unless I miss
> something).
>
>
>
> I first reproduced this bug while I'm adding simulator tests to RISC-V
> but talking with AArch64 (with some upstream tests) would be better.
>
>
>
> [How to reproduce]
>
> 1.  Configure Binutils with aarch64-unknown-elf and build it
>     /src/binutils/configure --target=aarch64-unknown-elf && make
> 2.  Run `make check-sim' and confirmed that the simulator tests "pass"
> 3.  Intentionally try to fail the test by modifying
>     `sim/testsuite/aarch64/pass.s'
>     (replace the last line from "pass" to "fail")
> 4.  Run `make check-sim' and "confirmed" that the simulator tests "pass"
>     (it should fail!)

I followed these instructions but I couldn't reproduce the problem you
are seeing.  More below...

>
>
>
> [Analysis]
>
> ... Yes, something is going wrong.  After the test, you can see the test
> log in `sim/testsuite/aarch64/allinsn/testrun.log':
>
> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
> Test run by builduser on Sat Aug 27 01:04:31 2022
> Target is aarch64-unknown-elf
> Host   is x86_64-pc-linux-gnu
>
>                 ===  tests ===
>
> Schedule of variations:
>     unix
>
> Running target unix
> Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
> Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
> Using /src/binutils/sim/testsuite/config/default.exp as tool-and-target-specific interface file.
> WARNING: Assuming target board is the local machine (which is probably wrong).
> You may need to set your DEJAGNU environment variable.
> Running /src/binutils/sim/testsuite/./aarch64/allinsn.exp ...

I don't see this, I see:

  Running ../../src.dev-1/sim/testsuite/aarch64/allinsn.exp ...

> Can't find a compatible assembler
> Executing on host: false /src/binutils/sim/testsuite/lib/compilercheck.c     -E -o testsuite/./aarch64/allinsn/compilercheck.x    (timeout = 300)
> spawn -ignore SIGHUP false /src/binutils/sim/testsuite/lib/compilercheck.c -E -o testsuite/./aarch64/allinsn/compilercheck.x
> compiler exited with status 1
> Can't find a compatible C compiler
> Testing adds.s on machine aarch64.
> UNTESTED: ./aarch64/adds.s
> Testing addv.s on machine aarch64.
> UNTESTED: ./aarch64/addv.s
> [...]
> Testing pass.s on machine aarch64.
> UNTESTED: ./aarch64/pass.s

And here I see:

  Testing pass.s on machine aarch64.
  Executing on host: /tmp/build/./gas/as-new ../../src/sim/testsuite/aarch64/pass.s  -I../../src/sim/testsuite/aarch64  -o /tmp/build/sim/pass.s.o    (timeout = 300)
  spawn -ignore SIGHUP /tmp/build/./gas/as-new ../../src/sim/testsuite/aarch64/pass.s -I../../src/sim/testsuite/aarch64 -o /tmp/build/sim/pass.s.o
  Executing on host: /tmp/build/./ld/ld-new /tmp/build/sim/pass.s.o   -o /tmp/build/sim/pass.s.x    (timeout = 300)
  spawn -ignore SIGHUP /tmp/build/./ld/ld-new /tmp/build/sim/pass.s.o -o /tmp/build/sim/pass.s.x
  ./aarch64/run    /tmp/build/sim/pass.s.x 
  spawn ./aarch64/run /tmp/build/sim/pass.s.x
  fail

which is what we'd expect.

I'm using commit d2d69057a2c, which is current HEAD of master.

Any idea why I might be seeing different results to you?

Thanks,
Andrew

> [...]
> Testing xtn.s on machine aarch64.
> UNTESTED: ./aarch64/xtn.s
> testcase /src/binutils/sim/testsuite/./aarch64/allinsn.exp completed in 0 seconds
>
>                 ===  Summary ===
>
> # of untested testcases         27
> runtest completed at Sat Aug 27 01:04:31 2022
> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>
>
> We clearly find that the test runner didn't find recently built assembler
> (gas/as-new).  That assembler (and the linker) is supposed to be used
> because the simulator itself (sim/Makefile) sets its configuration.
>
> Quoting `sim/site.exp' and `sim/site-sim-config.exp':
>     set AS_FOR_TARGET_AARCH64 "/build/binutils/./gas/as-new"
>     set LD_FOR_TARGET_AARCH64 "/build/binutils/./ld/ld-new"
>
>
> At last, we find `sim/testsuite/lib/sim-defs.exp'.
>
> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
> proc sim_init_toolchain {} {
> [...]
>     set arch [sim_arch]
>     set ARCH [string map {- _} [string toupper $arch]]
>     foreach var {AS LD CC} {
>     set var_for_target "${var}_FOR_TARGET"
>     global $var_for_target
>     set var_for_target_arch "${var_for_target}_${ARCH}"
>     global $var_for_target_arch
>
>     if [info exists $var_for_target_arch] {
>         set $var_for_target [set $var_for_target_arch]
>     } else {
>         set $var_for_target ""
>     }
> [...]
> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>
>
> In here, it extracts {AS,LD,CC}_FOR_TARGET_AARCH64 and sets proper
> {AS,LD,CC}_FOR_TARGET.  At least, it is supposed to do so.
>
> However, this block doesn't work because the `arch' variable returned by
> the `sim_arch' function is "./aarch64".  That is supposed to be "aarch64".
>
> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
> proc sim_arch {} {
>     global subdir
>     set arch "$subdir"
>     while { [file dirname $arch] != "." } {
>     set arch [file dirname $arch]
>     }
>     return "$arch"
> }
> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>
>
>
> [Fix: Cause and Patch Details]
>
> `sim_arch' function is simple.  Until the dirname part of the "arch" is not
> "." (current directory), it trims the filename part.
>
> For instance, if subdir is "A/B/C/D", this function returns "A".  However,
> if subdir is "./A/B/C/D", this function returns "./A", not "A".
> In fact, actual subdir value here is "./aarch64" and we will get
> "./aarch64" for [sim_arch].  As a result, it fails to extract proper
> assembler/linker/compiler for given target.
>
> To deal with it, I added another "file tail" function call before returning.
>
>
>
> [Analysis: After the Patch]
>
> After this patch, `sim/testsuite/aarch64/allinsn/testrun.log' will
> look like this:
>
> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
> Test run by builduser on Sat Aug 27 01:34:29 2022
> Target is aarch64-unknown-elf
> Host   is x86_64-pc-linux-gnu
>
>                 ===  tests ===
>
> Schedule of variations:
>     unix
>
> Running target unix
> Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
> Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
> Using /src/binutils/sim/testsuite/config/default.exp as tool-and-target-specific interface file.
> WARNING: Assuming target board is the local machine (which is probably wrong).
> You may need to set your DEJAGNU environment variable.
> Running /src/binutils/sim/testsuite/./aarch64/allinsn.exp ...
> Found a compatible assembler
> Can't execute C compiler
> Testing adds.s on machine aarch64.
> Executing on host: /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/adds.s  -I/src/binutils/sim/testsuite/./aarch64  -o testsuite/./aarch64/allinsn/adds.s.o    (timeout = 300)
> spawn -ignore SIGHUP /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/adds.s -I/src/binutils/sim/testsuite/./aarch64 -o testsuite/./aarch64/allinsn/adds.s.o
> Executing on host: /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/adds.s.o   -o testsuite/./aarch64/allinsn/adds.s.x    (timeout = 300)
> spawn -ignore SIGHUP /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/adds.s.o -o testsuite/./aarch64/allinsn/adds.s.x
> ./aarch64/run    testsuite/./aarch64/allinsn/adds.s.x
> spawn ./aarch64/run testsuite/./aarch64/allinsn/adds.s.x
> pass
> PASS: aarch64 adds.s
> [...]
> Testing pass.s on machine aarch64.
> Executing on host: /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/pass.s  -I/src/binutils/sim/testsuite/./aarch64  -o testsuite/./aarch64/allinsn/pass.s.o    (timeout = 300)
> spawn -ignore SIGHUP /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/pass.s -I/src/binutils/sim/testsuite/./aarch64 -o testsuite/./aarch64/allinsn/pass.s.o
> Executing on host: /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/pass.s.o   -o testsuite/./aarch64/allinsn/pass.s.x    (timeout = 300)
> spawn -ignore SIGHUP /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/pass.s.o -o testsuite/./aarch64/allinsn/pass.s.x
> ./aarch64/run    testsuite/./aarch64/allinsn/pass.s.x
> spawn ./aarch64/run testsuite/./aarch64/allinsn/pass.s.x
> fail
> status:  0
> output:  fail
>
> pattern: pass
>
> FAIL: aarch64 pass.s (execution)
> [...]
> Testing xtn.s on machine aarch64.
> Executing on host: /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/xtn.s  -I/src/binutils/sim/testsuite/./aarch64  -o testsuite/./aarch64/allinsn/xtn.s.o    (timeout = 300)
> spawn -ignore SIGHUP /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/xtn.s -I/src/binutils/sim/testsuite/./aarch64 -o testsuite/./aarch64/allinsn/xtn.s.o
> Executing on host: /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/xtn.s.o   -o testsuite/./aarch64/allinsn/xtn.s.x    (timeout = 300)
> spawn -ignore SIGHUP /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/xtn.s.o -o testsuite/./aarch64/allinsn/xtn.s.x
> ./aarch64/run    testsuite/./aarch64/allinsn/xtn.s.x
> spawn ./aarch64/run testsuite/./aarch64/allinsn/xtn.s.x
> pass
> PASS: aarch64 xtn.s
> testcase /src/binutils/sim/testsuite/./aarch64/allinsn.exp completed in 0 seconds
>
>                 ===  Summary ===
>
> # of expected passes            26
> # of unexpected failures        1
> runtest completed at Sat Aug 27 01:34:29 2022
> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>
>
> OK, we "successfully" failed.  If we restore `sim/testsuite/aarch64/pass.s',
> it completely succeeds.
>
> This patch trims "./" part of "./arch" using the `file tail' function.
> Even if the subdir input is "aarch64", it returns "aarch64".
> With this patch, we can now successfully test the simulator.
>
>
> Regards,
> Tsukasa
>
>
>
>
> Tsukasa OI (1):
>   sim/testsuite: PR29596, Trim extra path from arch
>
>  sim/testsuite/lib/sim-defs.exp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
>
> base-commit: c10a862f17847bc9c50d680c87b87dc51ae4b95e
> -- 
> 2.34.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PING^4 PATCH 0/1] sim/testsuite: PR29596, Fix broken "make check-sim" by trimming excess path from arch
  2022-10-11 11:06         ` [PING^4 PATCH 0/1] sim/testsuite: PR29596, Fix broken "make check-sim" by trimming excess " Andrew Burgess
@ 2022-10-12  5:39           ` Tsukasa OI
  2022-10-16 13:01           ` Tsukasa OI
  1 sibling, 0 replies; 16+ messages in thread
From: Tsukasa OI @ 2022-10-12  5:39 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

On 2022/10/11 20:06, Andrew Burgess wrote:
> 
> Hi!
> 
> Thanks for looking at this issue.
> 
> 
> Tsukasa OI <research_trasio@irq.a4lg.com> writes:
> 
>> TODO
>> NOTE: the cover letter and subject may change each time I ping.
> 
> For a single patch series like this a cover letter really isn't needed.
> All the information you've written in this cover letter would be better
> included in the commit message for the patch.  It's all really useful
> info that somebody looking at this patch in the future might want to
> know.

I once had an accident because the commit message missed important
background and made some confusion.  But you are right.  I might not
stop submitting one patch with cover letter if there's something to
describe (but not suited to commit message) but otherwise, I will follow
that.

> 
>>
>> Hello,
>>
>> This patch fixes the problem which make check-sim (testing the simulator)
>> doesn't work on many targets.
>>
>> The patch is a PING (4) of
>> <https://sourceware.org/pipermail/binutils/2022-July/121814.html>
>> <https://sourceware.org/pipermail/gdb-patches/2022-August/191564.html>.
>> Tracker on GitHub:
>> <https://github.com/a4lg/binutils-gdb/wiki/sim_trim_path_from_arch>
>>
>> In this ping, I removed ChangeLog and won't change anymore (unless I miss
>> something).
>>
>>
>>
>> I first reproduced this bug while I'm adding simulator tests to RISC-V
>> but talking with AArch64 (with some upstream tests) would be better.
>>
>>
>>
>> [How to reproduce]
>>
>> 1.  Configure Binutils with aarch64-unknown-elf and build it
>>     /src/binutils/configure --target=aarch64-unknown-elf && make
>> 2.  Run `make check-sim' and confirmed that the simulator tests "pass"
>> 3.  Intentionally try to fail the test by modifying
>>     `sim/testsuite/aarch64/pass.s'
>>     (replace the last line from "pass" to "fail")
>> 4.  Run `make check-sim' and "confirmed" that the simulator tests "pass"
>>     (it should fail!)
> 
> I followed these instructions but I couldn't reproduce the problem you
> are seeing.  More below...
> 
>>
>>
>>
>> [Analysis]
>>
>> ... Yes, something is going wrong.  After the test, you can see the test
>> log in `sim/testsuite/aarch64/allinsn/testrun.log':
>>
>> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>> Test run by builduser on Sat Aug 27 01:04:31 2022
>> Target is aarch64-unknown-elf
>> Host   is x86_64-pc-linux-gnu
>>
>>                 ===  tests ===
>>
>> Schedule of variations:
>>     unix
>>
>> Running target unix
>> Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
>> Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
>> Using /src/binutils/sim/testsuite/config/default.exp as tool-and-target-specific interface file.
>> WARNING: Assuming target board is the local machine (which is probably wrong).
>> You may need to set your DEJAGNU environment variable.
>> Running /src/binutils/sim/testsuite/./aarch64/allinsn.exp ...
> 
> I don't see this, I see:
> 
>   Running ../../src.dev-1/sim/testsuite/aarch64/allinsn.exp ...
> 
>> Can't find a compatible assembler
>> Executing on host: false /src/binutils/sim/testsuite/lib/compilercheck.c     -E -o testsuite/./aarch64/allinsn/compilercheck.x    (timeout = 300)
>> spawn -ignore SIGHUP false /src/binutils/sim/testsuite/lib/compilercheck.c -E -o testsuite/./aarch64/allinsn/compilercheck.x
>> compiler exited with status 1
>> Can't find a compatible C compiler
>> Testing adds.s on machine aarch64.
>> UNTESTED: ./aarch64/adds.s
>> Testing addv.s on machine aarch64.
>> UNTESTED: ./aarch64/addv.s
>> [...]
>> Testing pass.s on machine aarch64.
>> UNTESTED: ./aarch64/pass.s
> 
> And here I see:
> 
>   Testing pass.s on machine aarch64.
>   Executing on host: /tmp/build/./gas/as-new ../../src/sim/testsuite/aarch64/pass.s  -I../../src/sim/testsuite/aarch64  -o /tmp/build/sim/pass.s.o    (timeout = 300)
>   spawn -ignore SIGHUP /tmp/build/./gas/as-new ../../src/sim/testsuite/aarch64/pass.s -I../../src/sim/testsuite/aarch64 -o /tmp/build/sim/pass.s.o
>   Executing on host: /tmp/build/./ld/ld-new /tmp/build/sim/pass.s.o   -o /tmp/build/sim/pass.s.x    (timeout = 300)
>   spawn -ignore SIGHUP /tmp/build/./ld/ld-new /tmp/build/sim/pass.s.o -o /tmp/build/sim/pass.s.x
>   ./aarch64/run    /tmp/build/sim/pass.s.x 
>   spawn ./aarch64/run /tmp/build/sim/pass.s.x
>   fail
> 
> which is what we'd expect.
> 
> I'm using commit d2d69057a2c, which is current HEAD of master.
> 
> Any idea why I might be seeing different results to you?
> 
> Thanks,
> Andrew

Wow, interesting.  This is 100% reproducible on my environment (even
with commit d2d69057a2c) so... it feels like a kind of version-related
issue (most likely Tcl version).  Yes, it seems you are running "make
check-sim" correctly and... I would like to see your...

-   Output of "runtest --version"
-   $(builddir)/sim/Makefile
-   $(builddir)/sim/site.exp

My "runtest --version" result (Ubuntu 22.04.1 LTS x86_64) is:

> DejaGnu version 1.6.2
> Expect version  5.45.4
> Tcl version     8.6

Thanks,
Tsukasa

> 
>> [...]
>> Testing xtn.s on machine aarch64.
>> UNTESTED: ./aarch64/xtn.s
>> testcase /src/binutils/sim/testsuite/./aarch64/allinsn.exp completed in 0 seconds
>>
>>                 ===  Summary ===
>>
>> # of untested testcases         27
>> runtest completed at Sat Aug 27 01:04:31 2022
>> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>>
>>
>> We clearly find that the test runner didn't find recently built assembler
>> (gas/as-new).  That assembler (and the linker) is supposed to be used
>> because the simulator itself (sim/Makefile) sets its configuration.
>>
>> Quoting `sim/site.exp' and `sim/site-sim-config.exp':
>>     set AS_FOR_TARGET_AARCH64 "/build/binutils/./gas/as-new"
>>     set LD_FOR_TARGET_AARCH64 "/build/binutils/./ld/ld-new"
>>
>>
>> At last, we find `sim/testsuite/lib/sim-defs.exp'.
>>
>> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>> proc sim_init_toolchain {} {
>> [...]
>>     set arch [sim_arch]
>>     set ARCH [string map {- _} [string toupper $arch]]
>>     foreach var {AS LD CC} {
>>     set var_for_target "${var}_FOR_TARGET"
>>     global $var_for_target
>>     set var_for_target_arch "${var_for_target}_${ARCH}"
>>     global $var_for_target_arch
>>
>>     if [info exists $var_for_target_arch] {
>>         set $var_for_target [set $var_for_target_arch]
>>     } else {
>>         set $var_for_target ""
>>     }
>> [...]
>> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>>
>>
>> In here, it extracts {AS,LD,CC}_FOR_TARGET_AARCH64 and sets proper
>> {AS,LD,CC}_FOR_TARGET.  At least, it is supposed to do so.
>>
>> However, this block doesn't work because the `arch' variable returned by
>> the `sim_arch' function is "./aarch64".  That is supposed to be "aarch64".
>>
>> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>> proc sim_arch {} {
>>     global subdir
>>     set arch "$subdir"
>>     while { [file dirname $arch] != "." } {
>>     set arch [file dirname $arch]
>>     }
>>     return "$arch"
>> }
>> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>>
>>
>>
>> [Fix: Cause and Patch Details]
>>
>> `sim_arch' function is simple.  Until the dirname part of the "arch" is not
>> "." (current directory), it trims the filename part.
>>
>> For instance, if subdir is "A/B/C/D", this function returns "A".  However,
>> if subdir is "./A/B/C/D", this function returns "./A", not "A".
>> In fact, actual subdir value here is "./aarch64" and we will get
>> "./aarch64" for [sim_arch].  As a result, it fails to extract proper
>> assembler/linker/compiler for given target.
>>
>> To deal with it, I added another "file tail" function call before returning.
>>
>>
>>
>> [Analysis: After the Patch]
>>
>> After this patch, `sim/testsuite/aarch64/allinsn/testrun.log' will
>> look like this:
>>
>> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>> Test run by builduser on Sat Aug 27 01:34:29 2022
>> Target is aarch64-unknown-elf
>> Host   is x86_64-pc-linux-gnu
>>
>>                 ===  tests ===
>>
>> Schedule of variations:
>>     unix
>>
>> Running target unix
>> Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
>> Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
>> Using /src/binutils/sim/testsuite/config/default.exp as tool-and-target-specific interface file.
>> WARNING: Assuming target board is the local machine (which is probably wrong).
>> You may need to set your DEJAGNU environment variable.
>> Running /src/binutils/sim/testsuite/./aarch64/allinsn.exp ...
>> Found a compatible assembler
>> Can't execute C compiler
>> Testing adds.s on machine aarch64.
>> Executing on host: /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/adds.s  -I/src/binutils/sim/testsuite/./aarch64  -o testsuite/./aarch64/allinsn/adds.s.o    (timeout = 300)
>> spawn -ignore SIGHUP /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/adds.s -I/src/binutils/sim/testsuite/./aarch64 -o testsuite/./aarch64/allinsn/adds.s.o
>> Executing on host: /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/adds.s.o   -o testsuite/./aarch64/allinsn/adds.s.x    (timeout = 300)
>> spawn -ignore SIGHUP /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/adds.s.o -o testsuite/./aarch64/allinsn/adds.s.x
>> ./aarch64/run    testsuite/./aarch64/allinsn/adds.s.x
>> spawn ./aarch64/run testsuite/./aarch64/allinsn/adds.s.x
>> pass
>> PASS: aarch64 adds.s
>> [...]
>> Testing pass.s on machine aarch64.
>> Executing on host: /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/pass.s  -I/src/binutils/sim/testsuite/./aarch64  -o testsuite/./aarch64/allinsn/pass.s.o    (timeout = 300)
>> spawn -ignore SIGHUP /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/pass.s -I/src/binutils/sim/testsuite/./aarch64 -o testsuite/./aarch64/allinsn/pass.s.o
>> Executing on host: /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/pass.s.o   -o testsuite/./aarch64/allinsn/pass.s.x    (timeout = 300)
>> spawn -ignore SIGHUP /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/pass.s.o -o testsuite/./aarch64/allinsn/pass.s.x
>> ./aarch64/run    testsuite/./aarch64/allinsn/pass.s.x
>> spawn ./aarch64/run testsuite/./aarch64/allinsn/pass.s.x
>> fail
>> status:  0
>> output:  fail
>>
>> pattern: pass
>>
>> FAIL: aarch64 pass.s (execution)
>> [...]
>> Testing xtn.s on machine aarch64.
>> Executing on host: /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/xtn.s  -I/src/binutils/sim/testsuite/./aarch64  -o testsuite/./aarch64/allinsn/xtn.s.o    (timeout = 300)
>> spawn -ignore SIGHUP /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/xtn.s -I/src/binutils/sim/testsuite/./aarch64 -o testsuite/./aarch64/allinsn/xtn.s.o
>> Executing on host: /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/xtn.s.o   -o testsuite/./aarch64/allinsn/xtn.s.x    (timeout = 300)
>> spawn -ignore SIGHUP /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/xtn.s.o -o testsuite/./aarch64/allinsn/xtn.s.x
>> ./aarch64/run    testsuite/./aarch64/allinsn/xtn.s.x
>> spawn ./aarch64/run testsuite/./aarch64/allinsn/xtn.s.x
>> pass
>> PASS: aarch64 xtn.s
>> testcase /src/binutils/sim/testsuite/./aarch64/allinsn.exp completed in 0 seconds
>>
>>                 ===  Summary ===
>>
>> # of expected passes            26
>> # of unexpected failures        1
>> runtest completed at Sat Aug 27 01:34:29 2022
>> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>>
>>
>> OK, we "successfully" failed.  If we restore `sim/testsuite/aarch64/pass.s',
>> it completely succeeds.
>>
>> This patch trims "./" part of "./arch" using the `file tail' function.
>> Even if the subdir input is "aarch64", it returns "aarch64".
>> With this patch, we can now successfully test the simulator.
>>
>>
>> Regards,
>> Tsukasa
>>
>>
>>
>>
>> Tsukasa OI (1):
>>   sim/testsuite: PR29596, Trim extra path from arch
>>
>>  sim/testsuite/lib/sim-defs.exp | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>>
>> base-commit: c10a862f17847bc9c50d680c87b87dc51ae4b95e
>> -- 
>> 2.34.1
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PING^4 PATCH 0/1] sim/testsuite: PR29596, Fix broken "make check-sim" by trimming excess path from arch
  2022-10-11 11:06         ` [PING^4 PATCH 0/1] sim/testsuite: PR29596, Fix broken "make check-sim" by trimming excess " Andrew Burgess
  2022-10-12  5:39           ` Tsukasa OI
@ 2022-10-16 13:01           ` Tsukasa OI
  1 sibling, 0 replies; 16+ messages in thread
From: Tsukasa OI @ 2022-10-16 13:01 UTC (permalink / raw)
  To: Andrew Burgess, Mike Frysinger; +Cc: gdb-patches

Ping!

On 2022/10/11 20:06, Andrew Burgess wrote:
> 
> Hi!
> 
> Thanks for looking at this issue.
> 
> 
> Tsukasa OI <research_trasio@irq.a4lg.com> writes:
> 
>> TODO
>> NOTE: the cover letter and subject may change each time I ping.
> 
> For a single patch series like this a cover letter really isn't needed.
> All the information you've written in this cover letter would be better
> included in the commit message for the patch.  It's all really useful
> info that somebody looking at this patch in the future might want to
> know.
> 
>>
>> Hello,
>>
>> This patch fixes the problem which make check-sim (testing the simulator)
>> doesn't work on many targets.
>>
>> The patch is a PING (4) of
>> <https://sourceware.org/pipermail/binutils/2022-July/121814.html>
>> <https://sourceware.org/pipermail/gdb-patches/2022-August/191564.html>.
>> Tracker on GitHub:
>> <https://github.com/a4lg/binutils-gdb/wiki/sim_trim_path_from_arch>
>>
>> In this ping, I removed ChangeLog and won't change anymore (unless I miss
>> something).
>>
>>
>>
>> I first reproduced this bug while I'm adding simulator tests to RISC-V
>> but talking with AArch64 (with some upstream tests) would be better.
>>
>>
>>
>> [How to reproduce]
>>
>> 1.  Configure Binutils with aarch64-unknown-elf and build it
>>     /src/binutils/configure --target=aarch64-unknown-elf && make
>> 2.  Run `make check-sim' and confirmed that the simulator tests "pass"
>> 3.  Intentionally try to fail the test by modifying
>>     `sim/testsuite/aarch64/pass.s'
>>     (replace the last line from "pass" to "fail")
>> 4.  Run `make check-sim' and "confirmed" that the simulator tests "pass"
>>     (it should fail!)
> 
> I followed these instructions but I couldn't reproduce the problem you
> are seeing.  More below...

I found possible cause of non-reproduction.  While a single-threaded run
with "make -C sim check-DEJAGNU-single" resulted in the failure (as you
tested), multi-threaded run with "make -C sim check-DEJAGNU-parallel"
resulted in the unexpected success (as I tested).

Try forcing parallel simulator test with "make -C sim
check-DEJAGNU-parallel".

I fixed PR29596 and I hope that you can reproduce this issue.

Thanks,
Tsukasa

> 
>>
>>
>>
>> [Analysis]
>>
>> ... Yes, something is going wrong.  After the test, you can see the test
>> log in `sim/testsuite/aarch64/allinsn/testrun.log':
>>
>> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>> Test run by builduser on Sat Aug 27 01:04:31 2022
>> Target is aarch64-unknown-elf
>> Host   is x86_64-pc-linux-gnu
>>
>>                 ===  tests ===
>>
>> Schedule of variations:
>>     unix
>>
>> Running target unix
>> Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
>> Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
>> Using /src/binutils/sim/testsuite/config/default.exp as tool-and-target-specific interface file.
>> WARNING: Assuming target board is the local machine (which is probably wrong).
>> You may need to set your DEJAGNU environment variable.
>> Running /src/binutils/sim/testsuite/./aarch64/allinsn.exp ...
> 
> I don't see this, I see:
> 
>   Running ../../src.dev-1/sim/testsuite/aarch64/allinsn.exp ...
> 
>> Can't find a compatible assembler
>> Executing on host: false /src/binutils/sim/testsuite/lib/compilercheck.c     -E -o testsuite/./aarch64/allinsn/compilercheck.x    (timeout = 300)
>> spawn -ignore SIGHUP false /src/binutils/sim/testsuite/lib/compilercheck.c -E -o testsuite/./aarch64/allinsn/compilercheck.x
>> compiler exited with status 1
>> Can't find a compatible C compiler
>> Testing adds.s on machine aarch64.
>> UNTESTED: ./aarch64/adds.s
>> Testing addv.s on machine aarch64.
>> UNTESTED: ./aarch64/addv.s
>> [...]
>> Testing pass.s on machine aarch64.
>> UNTESTED: ./aarch64/pass.s
> 
> And here I see:
> 
>   Testing pass.s on machine aarch64.
>   Executing on host: /tmp/build/./gas/as-new ../../src/sim/testsuite/aarch64/pass.s  -I../../src/sim/testsuite/aarch64  -o /tmp/build/sim/pass.s.o    (timeout = 300)
>   spawn -ignore SIGHUP /tmp/build/./gas/as-new ../../src/sim/testsuite/aarch64/pass.s -I../../src/sim/testsuite/aarch64 -o /tmp/build/sim/pass.s.o
>   Executing on host: /tmp/build/./ld/ld-new /tmp/build/sim/pass.s.o   -o /tmp/build/sim/pass.s.x    (timeout = 300)
>   spawn -ignore SIGHUP /tmp/build/./ld/ld-new /tmp/build/sim/pass.s.o -o /tmp/build/sim/pass.s.x
>   ./aarch64/run    /tmp/build/sim/pass.s.x 
>   spawn ./aarch64/run /tmp/build/sim/pass.s.x
>   fail
> 
> which is what we'd expect.
> 
> I'm using commit d2d69057a2c, which is current HEAD of master.
> 
> Any idea why I might be seeing different results to you?
> 
> Thanks,
> Andrew
> 
>> [...]
>> Testing xtn.s on machine aarch64.
>> UNTESTED: ./aarch64/xtn.s
>> testcase /src/binutils/sim/testsuite/./aarch64/allinsn.exp completed in 0 seconds
>>
>>                 ===  Summary ===
>>
>> # of untested testcases         27
>> runtest completed at Sat Aug 27 01:04:31 2022
>> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>>
>>
>> We clearly find that the test runner didn't find recently built assembler
>> (gas/as-new).  That assembler (and the linker) is supposed to be used
>> because the simulator itself (sim/Makefile) sets its configuration.
>>
>> Quoting `sim/site.exp' and `sim/site-sim-config.exp':
>>     set AS_FOR_TARGET_AARCH64 "/build/binutils/./gas/as-new"
>>     set LD_FOR_TARGET_AARCH64 "/build/binutils/./ld/ld-new"
>>
>>
>> At last, we find `sim/testsuite/lib/sim-defs.exp'.
>>
>> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>> proc sim_init_toolchain {} {
>> [...]
>>     set arch [sim_arch]
>>     set ARCH [string map {- _} [string toupper $arch]]
>>     foreach var {AS LD CC} {
>>     set var_for_target "${var}_FOR_TARGET"
>>     global $var_for_target
>>     set var_for_target_arch "${var_for_target}_${ARCH}"
>>     global $var_for_target_arch
>>
>>     if [info exists $var_for_target_arch] {
>>         set $var_for_target [set $var_for_target_arch]
>>     } else {
>>         set $var_for_target ""
>>     }
>> [...]
>> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>>
>>
>> In here, it extracts {AS,LD,CC}_FOR_TARGET_AARCH64 and sets proper
>> {AS,LD,CC}_FOR_TARGET.  At least, it is supposed to do so.
>>
>> However, this block doesn't work because the `arch' variable returned by
>> the `sim_arch' function is "./aarch64".  That is supposed to be "aarch64".
>>
>> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>> proc sim_arch {} {
>>     global subdir
>>     set arch "$subdir"
>>     while { [file dirname $arch] != "." } {
>>     set arch [file dirname $arch]
>>     }
>>     return "$arch"
>> }
>> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>>
>>
>>
>> [Fix: Cause and Patch Details]
>>
>> `sim_arch' function is simple.  Until the dirname part of the "arch" is not
>> "." (current directory), it trims the filename part.
>>
>> For instance, if subdir is "A/B/C/D", this function returns "A".  However,
>> if subdir is "./A/B/C/D", this function returns "./A", not "A".
>> In fact, actual subdir value here is "./aarch64" and we will get
>> "./aarch64" for [sim_arch].  As a result, it fails to extract proper
>> assembler/linker/compiler for given target.
>>
>> To deal with it, I added another "file tail" function call before returning.
>>
>>
>>
>> [Analysis: After the Patch]
>>
>> After this patch, `sim/testsuite/aarch64/allinsn/testrun.log' will
>> look like this:
>>
>> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>> Test run by builduser on Sat Aug 27 01:34:29 2022
>> Target is aarch64-unknown-elf
>> Host   is x86_64-pc-linux-gnu
>>
>>                 ===  tests ===
>>
>> Schedule of variations:
>>     unix
>>
>> Running target unix
>> Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
>> Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
>> Using /src/binutils/sim/testsuite/config/default.exp as tool-and-target-specific interface file.
>> WARNING: Assuming target board is the local machine (which is probably wrong).
>> You may need to set your DEJAGNU environment variable.
>> Running /src/binutils/sim/testsuite/./aarch64/allinsn.exp ...
>> Found a compatible assembler
>> Can't execute C compiler
>> Testing adds.s on machine aarch64.
>> Executing on host: /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/adds.s  -I/src/binutils/sim/testsuite/./aarch64  -o testsuite/./aarch64/allinsn/adds.s.o    (timeout = 300)
>> spawn -ignore SIGHUP /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/adds.s -I/src/binutils/sim/testsuite/./aarch64 -o testsuite/./aarch64/allinsn/adds.s.o
>> Executing on host: /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/adds.s.o   -o testsuite/./aarch64/allinsn/adds.s.x    (timeout = 300)
>> spawn -ignore SIGHUP /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/adds.s.o -o testsuite/./aarch64/allinsn/adds.s.x
>> ./aarch64/run    testsuite/./aarch64/allinsn/adds.s.x
>> spawn ./aarch64/run testsuite/./aarch64/allinsn/adds.s.x
>> pass
>> PASS: aarch64 adds.s
>> [...]
>> Testing pass.s on machine aarch64.
>> Executing on host: /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/pass.s  -I/src/binutils/sim/testsuite/./aarch64  -o testsuite/./aarch64/allinsn/pass.s.o    (timeout = 300)
>> spawn -ignore SIGHUP /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/pass.s -I/src/binutils/sim/testsuite/./aarch64 -o testsuite/./aarch64/allinsn/pass.s.o
>> Executing on host: /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/pass.s.o   -o testsuite/./aarch64/allinsn/pass.s.x    (timeout = 300)
>> spawn -ignore SIGHUP /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/pass.s.o -o testsuite/./aarch64/allinsn/pass.s.x
>> ./aarch64/run    testsuite/./aarch64/allinsn/pass.s.x
>> spawn ./aarch64/run testsuite/./aarch64/allinsn/pass.s.x
>> fail
>> status:  0
>> output:  fail
>>
>> pattern: pass
>>
>> FAIL: aarch64 pass.s (execution)
>> [...]
>> Testing xtn.s on machine aarch64.
>> Executing on host: /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/xtn.s  -I/src/binutils/sim/testsuite/./aarch64  -o testsuite/./aarch64/allinsn/xtn.s.o    (timeout = 300)
>> spawn -ignore SIGHUP /build/binutils/./gas/as-new /src/binutils/sim/testsuite/./aarch64/xtn.s -I/src/binutils/sim/testsuite/./aarch64 -o testsuite/./aarch64/allinsn/xtn.s.o
>> Executing on host: /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/xtn.s.o   -o testsuite/./aarch64/allinsn/xtn.s.x    (timeout = 300)
>> spawn -ignore SIGHUP /build/binutils/./ld/ld-new testsuite/./aarch64/allinsn/xtn.s.o -o testsuite/./aarch64/allinsn/xtn.s.x
>> ./aarch64/run    testsuite/./aarch64/allinsn/xtn.s.x
>> spawn ./aarch64/run testsuite/./aarch64/allinsn/xtn.s.x
>> pass
>> PASS: aarch64 xtn.s
>> testcase /src/binutils/sim/testsuite/./aarch64/allinsn.exp completed in 0 seconds
>>
>>                 ===  Summary ===
>>
>> # of expected passes            26
>> # of unexpected failures        1
>> runtest completed at Sat Aug 27 01:34:29 2022
>> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>>
>>
>> OK, we "successfully" failed.  If we restore `sim/testsuite/aarch64/pass.s',
>> it completely succeeds.
>>
>> This patch trims "./" part of "./arch" using the `file tail' function.
>> Even if the subdir input is "aarch64", it returns "aarch64".
>> With this patch, we can now successfully test the simulator.
>>
>>
>> Regards,
>> Tsukasa
>>
>>
>>
>>
>> Tsukasa OI (1):
>>   sim/testsuite: PR29596, Trim extra path from arch
>>
>>  sim/testsuite/lib/sim-defs.exp | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>>
>> base-commit: c10a862f17847bc9c50d680c87b87dc51ae4b95e
>> -- 
>> 2.34.1
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RESEND PATCH 0/1] sim/testsuite: Trim extra path from arch
  2022-08-27  1:53 ` [RESEND PATCH 0/1] sim/testsuite: Trim extra path from arch Tsukasa OI
  2022-08-27  1:53   ` [RESEND PATCH 1/1] " Tsukasa OI
  2022-09-14 10:56   ` [PING^2 PATCH 0/1] " Tsukasa OI
@ 2022-10-23 19:16   ` Mike Frysinger
  2022-10-24  6:31     ` Tsukasa OI
  2022-10-23 19:16   ` [PATCH] sim: testsuite: tweak parallel find invocation [PR sim/29596] Mike Frysinger
  3 siblings, 1 reply; 16+ messages in thread
From: Mike Frysinger @ 2022-10-23 19:16 UTC (permalink / raw)
  To: Tsukasa OI; +Cc: Andrew Burgess, Nelson Chu, Kito Cheng, gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1606 bytes --]

On 27 Aug 2022 01:53, Tsukasa OI wrote:
> The patch is a RESEND of
> <https://sourceware.org/pipermail/binutils/2022-July/121814.html>
> but to the right mailing list and completely new cover letter.

please clean up the patch and send it out standalone -- no need for a cover
letter, and the relevant details should all be in the patch itself.

> 1.  Configure Binutils with aarch64-unknown-elf and build it
>     /src/binutils/configure \
>         --target=aarch64-unknown-elf \
>         --prefix=/opt/cross/aarch64-unknown-elf \
>         --enable-multilib
>     && make
> 2.  Run `make check-sim' and confirmed that the simulator tests "pass"
> 3.  Intentionally try to fail the test by modifying
>     `sim/testsuite/aarch64/pass.s'
>     (replace the last line from "pass" to "fail")
> 4.  Run `make check-sim' and "confirmed" that the simulator tests "pass"

since i don't have the same paths as you, i didn't run the same configure.
but using --target=aarch64-elf and running `make check-sim` with -j1 vs -j4
doesn't show any behavior difference.  all the tests are found & run.  if i
add an error to one of the tests like you did, it fails in both modes.

$ runtest --version
DejaGnu version 1.6.3
Expect version  5.45.4
Tcl version     8.6

> However, this block doesn't work because the `arch' variable returned by
> the `sim_arch' function is "./aarch64".  That is supposed to be "aarch64".

i don't mind fixing sim_arch, but this really should be fixed too.  we
shouldn't be in a situation where "./aarch64" is present as an input.

seems easy enough to do so i pushed a patch for it.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] sim: testsuite: tweak parallel find invocation [PR sim/29596]
  2022-08-27  1:53 ` [RESEND PATCH 0/1] sim/testsuite: Trim extra path from arch Tsukasa OI
                     ` (2 preceding siblings ...)
  2022-10-23 19:16   ` [RESEND PATCH 0/1] sim/testsuite: Trim extra " Mike Frysinger
@ 2022-10-23 19:16   ` Mike Frysinger
  2022-10-23 19:44     ` [PATCH] sim: testsuite: update ignored .exp files " Mike Frysinger
  3 siblings, 1 reply; 16+ messages in thread
From: Mike Frysinger @ 2022-10-23 19:16 UTC (permalink / raw)
  To: gdb-patches; +Cc: research_trasio

Make sure we invoke runtest with the same exp filenames when running in
parallel as it will find when run single threaded.  When `runtest` finds
files itself, it will use paths like "aarch64/allinsn.exp".  When we run
`find .` with the %p option, it produces "./aarch64/allinsn.exp".  Switch
to %P to get "aarch64/allinsn.exp".

Bug: https://sourceware.org/PR29596
---
 sim/Makefile.in        | 2 +-
 sim/testsuite/local.mk | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sim/testsuite/local.mk b/sim/testsuite/local.mk
index c1799c728ae7..e55f8c3c8b22 100644
--- a/sim/testsuite/local.mk
+++ b/sim/testsuite/local.mk
@@ -49,7 +49,7 @@ check/%.exp:
 check-DEJAGNU-parallel:
 	$(AM_V_at)( \
 	$(MAKE) -k \
-	  `cd $(srcdir)/testsuite && find . -name '*.exp' -printf 'check/%p '`; \
+	  `cd $(srcdir)/testsuite && find . -name '*.exp' -printf 'check/%P '`; \
 	ret=$$?; \
 	$(SHELL) $(srcroot)/contrib/dg-extract-results.sh \
 	  `find testsuite/ -maxdepth 4 -name testrun.sum | sort` > testrun.sum; \
-- 
2.37.3


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] sim: testsuite: update ignored .exp files [PR sim/29596]
  2022-10-23 19:16   ` [PATCH] sim: testsuite: tweak parallel find invocation [PR sim/29596] Mike Frysinger
@ 2022-10-23 19:44     ` Mike Frysinger
  0 siblings, 0 replies; 16+ messages in thread
From: Mike Frysinger @ 2022-10-23 19:44 UTC (permalink / raw)
  To: gdb-patches

Now that we run `check/foo.exp` instead of `check/./foo.exp`,
update the config/ & lib/ exceptions to cover both paths.

Bug: https://sourceware.org/PR29596
---
 sim/Makefile.in        | 2 ++
 sim/testsuite/local.mk | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/sim/testsuite/local.mk b/sim/testsuite/local.mk
index e55f8c3c8b22..b8114a565997 100644
--- a/sim/testsuite/local.mk
+++ b/sim/testsuite/local.mk
@@ -40,7 +40,9 @@ DO_RUNTEST = \
 
 # Ignore dirs that only contain configuration settings.
 check/./config/%.exp: ; @true
+check/config/%.exp: ; @true
 check/./lib/%.exp: ; @true
+check/lib/%.exp: ; @true
 
 check/%.exp:
 	$(AM_V_at)mkdir -p testsuite/$*
-- 
2.37.3


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RESEND PATCH 0/1] sim/testsuite: Trim extra path from arch
  2022-10-23 19:16   ` [RESEND PATCH 0/1] sim/testsuite: Trim extra " Mike Frysinger
@ 2022-10-24  6:31     ` Tsukasa OI
  2022-10-24  7:33       ` Mike Frysinger
  0 siblings, 1 reply; 16+ messages in thread
From: Tsukasa OI @ 2022-10-24  6:31 UTC (permalink / raw)
  To: Mike Frysinger, gdb-patches

On 2022/10/24 4:16, Mike Frysinger wrote:
> On 27 Aug 2022 01:53, Tsukasa OI wrote:
>> The patch is a RESEND of
>> <https://sourceware.org/pipermail/binutils/2022-July/121814.html>
>> but to the right mailing list and completely new cover letter.
> 
> please clean up the patch and send it out standalone -- no need for a cover
> letter, and the relevant details should all be in the patch itself.

Sorry, I should have at least enhanced the commit message (I would have
attached a separate cover letter for information not relevant to the
change itself).

> 
>> 1.  Configure Binutils with aarch64-unknown-elf and build it
>>     /src/binutils/configure \
>>         --target=aarch64-unknown-elf \
>>         --prefix=/opt/cross/aarch64-unknown-elf \
>>         --enable-multilib
>>     && make
>> 2.  Run `make check-sim' and confirmed that the simulator tests "pass"
>> 3.  Intentionally try to fail the test by modifying
>>     `sim/testsuite/aarch64/pass.s'
>>     (replace the last line from "pass" to "fail")
>> 4.  Run `make check-sim' and "confirmed" that the simulator tests "pass"
> 
> since i don't have the same paths as you, i didn't run the same configure.
> but using --target=aarch64-elf and running `make check-sim` with -j1 vs -j4
> doesn't show any behavior difference.  all the tests are found & run.  if i
> add an error to one of the tests like you did, it fails in both modes.
> 
> $ runtest --version
> DejaGnu version 1.6.3
> Expect version  5.45.4
> Tcl version     8.6
> 
>> However, this block doesn't work because the `arch' variable returned by
>> the `sim_arch' function is "./aarch64".  That is supposed to be "aarch64".
> 
> i don't mind fixing sim_arch, but this really should be fixed too.  we
> shouldn't be in a situation where "./aarch64" is present as an input.
> 
> seems easy enough to do so i pushed a patch for it.
> -mike

That worked perfectly!

I'll withdraw this patchset and close the bug report.

Thanks,
Tsukasa

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RESEND PATCH 0/1] sim/testsuite: Trim extra path from arch
  2022-10-24  6:31     ` Tsukasa OI
@ 2022-10-24  7:33       ` Mike Frysinger
  0 siblings, 0 replies; 16+ messages in thread
From: Mike Frysinger @ 2022-10-24  7:33 UTC (permalink / raw)
  To: Tsukasa OI; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 672 bytes --]

On 24 Oct 2022 15:31, Tsukasa OI wrote:
> On 2022/10/24 4:16, Mike Frysinger wrote:
> > On 27 Aug 2022 01:53, Tsukasa OI wrote:
> >> However, this block doesn't work because the `arch' variable returned by
> >> the `sim_arch' function is "./aarch64".  That is supposed to be "aarch64".
> > 
> > i don't mind fixing sim_arch, but this really should be fixed too.  we
> > shouldn't be in a situation where "./aarch64" is present as an input.
> > 
> > seems easy enough to do so i pushed a patch for it.
> 
> That worked perfectly!
> 
> I'll withdraw this patchset and close the bug report.

i don't mind merging your patch too with a fixed up message
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2022-10-24  8:48 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <28df7073ec5a8f601ba47c9d2f7d4a7a0ce08753.1657795051.git.research_trasio@irq.a4lg.com>
2022-08-27  1:53 ` [RESEND PATCH 0/1] sim/testsuite: Trim extra path from arch Tsukasa OI
2022-08-27  1:53   ` [RESEND PATCH 1/1] " Tsukasa OI
2022-09-14 10:56   ` [PING^2 PATCH 0/1] " Tsukasa OI
2022-09-14 10:56     ` [PING^2 PATCH 1/1] " Tsukasa OI
2022-09-21 16:02     ` [PING^3 PATCH 0/1] sim/testsuite: Fix broken "make check-sim" by trimming excess " Tsukasa OI
2022-09-21 16:02       ` [PING^3 PATCH 1/1] sim/testsuite: Trim extra " Tsukasa OI
2022-10-09  3:55       ` [PING^4 PATCH 0/1] sim/testsuite: PR29596, Fix broken "make check-sim" by trimming excess " Tsukasa OI
2022-10-09  3:55         ` [PING^4 PATCH 1/1] sim/testsuite: PR29596, Trim extra " Tsukasa OI
2022-10-11 11:06         ` [PING^4 PATCH 0/1] sim/testsuite: PR29596, Fix broken "make check-sim" by trimming excess " Andrew Burgess
2022-10-12  5:39           ` Tsukasa OI
2022-10-16 13:01           ` Tsukasa OI
2022-10-23 19:16   ` [RESEND PATCH 0/1] sim/testsuite: Trim extra " Mike Frysinger
2022-10-24  6:31     ` Tsukasa OI
2022-10-24  7:33       ` Mike Frysinger
2022-10-23 19:16   ` [PATCH] sim: testsuite: tweak parallel find invocation [PR sim/29596] Mike Frysinger
2022-10-23 19:44     ` [PATCH] sim: testsuite: update ignored .exp files " Mike Frysinger

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).