[PING^3 PATCH 0/1] sim/testsuite: Fix broken "make check-sim" by trimming excess path from arch

[Tsukasa OI <research_trasio@irq.a4lg.com>]

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