From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 14C8E3853807 for ; Tue, 11 Oct 2022 11:06:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 14C8E3853807 Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-204-0jXDcLnCMuiNgiX2x__ztA-1; Tue, 11 Oct 2022 07:06:56 -0400 X-MC-Unique: 0jXDcLnCMuiNgiX2x__ztA-1 Received: by mail-wm1-f70.google.com with SMTP id bg21-20020a05600c3c9500b003c2acbff422so693976wmb.0 for ; Tue, 11 Oct 2022 04:06:56 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=y5PzNRfUChKGFrnPwsyG1PZQXaKzRSDA+ymPOys5+wM=; b=494qvAz69jKMOM+ax5jy25UHrVRldg6k8ese2YzGcXJr870ihuoVx2hNuFRvRazs2V voeIqcIN+B6qBP32Ac5lfu+k9ce9xgTjZum1h77hXHeWgUQnND3rRpepAT6MI1BNSPyh vhp/SsLrx8TNiRjx/c/hwABY8Eka3Z1sEyiQiai2icaVqVOiVqm+3Fjo94PiuRKpcmQg T46rRBp9WJNplSMzaI5TFWM9P5yv3h7ub/9DZXRPXLbtf7PCHCp+o6qKt/VYZvpi56WI lrgjChNuDrQdBaGHZqeq8hl1c2SuLoAKuegyNSR2NItizKnwyYFPEq98vl2TK504v+bI J21A== X-Gm-Message-State: ACrzQf2lSbgp6WcGbBQyBoJ+UgUdQokh7z7sx4I419jqMwPO6mxD/x64 Xp3BQQAIY+9XbALuHzQfSwhIIlYPlJWANq2cGQdZ1bmWdOPY4QIBQLacqWizU6tnZ3Kiv8Uz19b gyRsXRuCvYP2/9X1ZG0pH9g== X-Received: by 2002:a5d:5f03:0:b0:22f:8ad4:bd46 with SMTP id cl3-20020a5d5f03000000b0022f8ad4bd46mr9378278wrb.120.1665486415232; Tue, 11 Oct 2022 04:06:55 -0700 (PDT) X-Google-Smtp-Source: AMsMyM4r2qBj8FSD5zdP2fq+iVMJk/QPvCorhj4lFn/BpTmCwGJJ6pVrZdhJIqC62xRvRYuVr1f1jA== X-Received: by 2002:a5d:5f03:0:b0:22f:8ad4:bd46 with SMTP id cl3-20020a5d5f03000000b0022f8ad4bd46mr9378264wrb.120.1665486414888; Tue, 11 Oct 2022 04:06:54 -0700 (PDT) Received: from localhost (52.72.115.87.dyn.plus.net. [87.115.72.52]) by smtp.gmail.com with ESMTPSA id g6-20020a05600c4ec600b003b477532e66sm2524201wmq.2.2022.10.11.04.06.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 11 Oct 2022 04:06:54 -0700 (PDT) From: Andrew Burgess To: Tsukasa OI , Tsukasa OI , Mike Frysinger Cc: gdb-patches@sourceware.org Subject: Re: [PING^4 PATCH 0/1] sim/testsuite: PR29596, Fix broken "make check-sim" by trimming excess path from arch In-Reply-To: References: Date: Tue, 11 Oct 2022 12:06:53 +0100 Message-ID: <87wn96d3g2.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-2.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_ASCII_DIVIDERS, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, 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: Tue, 11 Oct 2022 11:07:00 -0000 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... > > > > [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