From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-sender-0.a4lg.com (mail-sender-0.a4lg.com [IPv6:2401:2500:203:30b:4000:6bfe:4757:0]) by sourceware.org (Postfix) with ESMTPS id BEA343858D3C for ; Sun, 16 Oct 2022 13:01:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BEA343858D3C Received: from [127.0.0.1] (localhost [127.0.0.1]) by mail-sender-0.a4lg.com (Postfix) with ESMTPSA id 75323300089; Sun, 16 Oct 2022 13:01:11 +0000 (UTC) Message-ID: Date: Sun, 16 Oct 2022 22:01:08 +0900 Mime-Version: 1.0 Subject: Re: [PING^4 PATCH 0/1] sim/testsuite: PR29596, Fix broken "make check-sim" by trimming excess path from arch To: Andrew Burgess , Mike Frysinger Cc: gdb-patches@sourceware.org References: <87wn96d3g2.fsf@redhat.com> Content-Language: en-US From: Tsukasa OI In-Reply-To: <87wn96d3g2.fsf@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-5.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, KAM_ASCII_DIVIDERS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 16 Oct 2022 13:01:17 -0000 Ping! On 2022/10/11 20:06, Andrew Burgess wrote: > > Hi! > > Thanks for looking at this issue. > > > Tsukasa OI 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 >> >> . >> Tracker on GitHub: >> >> >> 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 >