From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x22e.google.com (mail-lj1-x22e.google.com [IPv6:2a00:1450:4864:20::22e]) by sourceware.org (Postfix) with ESMTPS id 3A2B0395BC51 for ; Wed, 16 Nov 2022 15:38:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3A2B0395BC51 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lj1-x22e.google.com with SMTP id x21so22277998ljg.10 for ; Wed, 16 Nov 2022 07:38:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=LrLsGo493pf4W1mOPOLZPtC9MI63x5hVZKvQT5wTid0=; b=Fao8bj7sevw0XPP8LbW/j+0pVHx8VOJnUvKARi00oiLZmF1NbaSliE9OuzQBDeGzNM TweZ4ZPIYBd6eSteaWpAWErVXJN+0/zBeimrter84PEB/7UAm38ukOdKNKVTUXedsW7A Ir5OR1QBnETPEt4BnD0J2Ch/sGNN88fmfu3TmSNpp9Zctg1m22U5ZfWwZPUN6yHtKJYC KA5TICXLEhAKm3Z/1ADo40exEyOUH/aLkrTY1iiOS+2RufV9dxb6/p9Ch7KNJi0w3WiX aeYukXzsljS/hBtZtsK2mHwK79nRQHw9Q5Sd1LuGg8u1bnjJFuEMDRTs7sb3nR47GnDd TO5w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=LrLsGo493pf4W1mOPOLZPtC9MI63x5hVZKvQT5wTid0=; b=NZbAMKqHBUY8gOsF2F6t6yErS1NY5RNq8Vi9Z1BTwN22U5x7t/X1SpfMFnkXgrdJ+2 1BPqjVFzyg617OI8sW6RE/uIOB5ZHHuRenE1XibfTHqt4bxNII/lkUXmMjiTZNHJbbP2 vbQQfVr8AvGB2LLn79qPn+ql7n6oIluqv/vhRraafeRdIOgi0wAQ4nEOJiX0QoZyhMAY MD43S5a462NFaFDBq7ndCRQ+UneXSxq6STSzZX0jYwnKFv7Aml2S7qaTGZ/SzEVlH5UL ybyZX6njUjyIn9xURwO1tgLprNS7WC4XTP+9eor575GEPz2zGhQ79aNqAYci/H+O9ua8 IwaA== X-Gm-Message-State: ANoB5pkYl1K8Iy9W/N3AVrv52R33IiwcbyyzflbKs16eYr8Qaxr95JfM zoQ4U6TvjxzJslIXEXvsI88s+vZJw8C8Pt67EKk= X-Google-Smtp-Source: AA0mqf6npTEZ9QQCUvMRi9CqOlJaIX7Sq7tv22Zp8lZVUypzDoNklIB8hdOQrBxerpI7jFirHDAnATuouuQz76Tmles= X-Received: by 2002:a05:651c:1247:b0:26d:e656:d853 with SMTP id h7-20020a05651c124700b0026de656d853mr7531881ljh.177.1668613116679; Wed, 16 Nov 2022 07:38:36 -0800 (PST) MIME-Version: 1.0 References: <20220909125624.2561867-1-torbjorn.svensson@foss.st.com> <662abb45-dad9-f0f7-4af6-940829d648a6@foss.st.com> <54513c03-1421-b480-fc6b-2229e811396b@foss.st.com> <20221116021154.4AE372042F@pchp3.se.axis.com> <48c23331-643f-c747-9c6b-09a36938a4aa@foss.st.com> In-Reply-To: <48c23331-643f-c747-9c6b-09a36938a4aa@foss.st.com> From: Richard Biener Date: Wed, 16 Nov 2022 16:38:23 +0100 Message-ID: Subject: Re: [PATCH] testsuite: Fix mistransformed gcov To: Torbjorn SVENSSON Cc: Hans-Peter Nilsson , gcc-patches@gcc.gnu.org, mikestump@comcast.net Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-7.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Wed, Nov 16, 2022 at 9:00 AM Torbjorn SVENSSON via Gcc-patches wrote: > > Hi, > > On 2022-11-16 03:11, Hans-Peter Nilsson wrote: > > How was r13-2619-g34b9a03353d3fd "gcov: Respect triplet when looking > > for gcov" tested? I'm having a hard time believing it was tested with > > a *cross-compiler* *in-build-tree*. I think it was only tested for > > the special-case of an installed cross-compiler; not even with a > > native build exercising the false branch (see patch), considering that > > a breaking typo ("}" vs "]") snuck by in the first revision, before > > commit. > > I was testing this in out-of-tree test with a cross-compiler (built for > arm-none-eabi) running on Windows and Linux and just noticed the failure > and (wrongly) assumed that all of the cases needed the transformation > call as that's how other tools were handled. > The test systems used is are hosts that does not have any internet > connection, so that's why I got a typo in the first patch when moving > the fix to a system that did have an internet connection. > Sorry for the mess I caused! > > Regarding the patch you propose; it looks good to me, but I'm not able > to approve it. The patch is OK. Richard. > Kind regards, > Torbj=C3=B6rn > > > I guess reviewers forgot to ask that, but it's really on the > > submitter; it's a general requirement for patches to say how it was > > tested. Usually no more is needed than "tested with a cross-compiler > > for ..., no regressions" (implying running twice and comparing results > > before and after the patch). > > > > In this case, when adjusting test-framework bits, a little more care > > and mentioned details about how it was tested, would have been in > > order. It's likely it'd have shown that an uninstalled in-tree cross > > (an IMHO more expected case) wasn't tested, which that patch broke for > > the one gcov invocation that the testsuite does for cross-builds > > (IIUC). > > > > It can be said like this: I tested *this* patch as follows (all > > directory names below manually redacted), showing no regressions and > > fixing the regression for the in-tree cross build; > > > > For a native build (x86_64, Debian 11): > > - In the gcc build directory: > > make check RUNTESTFLAGS=3Dgcov.exp > > > > - In an empty new directory after native installation in /pre. > > /gccsrc/top/contrib/test_installed --prefix=3D/pre gcov.exp > > > > - Also, for good measure (mentioned somewhere in the installation > > documentation) native: > > for tool in gcc g++ ; do env PATH=3D/pre/bin:$PATH runtest \ > > --tool $tool --srcdir=3D/gccsrc/top/gcc/testsuite gcov.exp; done > > > > For a cris-elf cross: > > - In the gcc build directory: > > make check 'RUNTESTFLAGS=3D--target_board=3Dcris-sim gcov.exp' > > > > - In an empty new directory after installation in /pre. > > /gccsrc/top/contrib/test_installed --with-gcc=3D/pre/bin/cris-elf-gcc \ > > --with-g++=3D/pre/bin/cris-elf-g++ --with-gfortran=3D/pre/bin/cris-el= f-gfortran \ > > --target=3Dcris-elf --target_board=3Dcris-sim gcov.exp > > > > Ok to commit? > > > > brgds, H-P > > PS. Beware that the proc name may be up for bikeshedding. > > > > ---- 8< -------- 8< -------- 8< -------- 8< ---- > > > > In commit r13-2619-g34b9a03353d3fd, [transform] was applied to all > > invocations of gcov, for both out-of-tree and in-tree testing. > > For in-tree cross builds, this means gcov was called as > > "/path/to/gccobj/gcc/target-tuple-gcov" gcov-pr94029.c which is > > incorrect, as it's there "/path/to/gccobj/gcc/gcov" until it's > > installed. This caused a testsuite failure, like: > > > > Running /x/gcc/gcc/testsuite/gcc.misc-tests/gcov.exp ... > > FAIL: gcc.misc-tests/gcov-pr94029.c gcov failed: spawn failed > > > > To avoid cumbersome conditionals, use a dedicated new helper function. > > > > gcc/testsuite: > > * lib/gcc-dg.exp (gcc-transform-out-of-tree): New proc. > > * g++.dg/gcov/gcov.exp, gcc.misc-tests/gcov.exp: Call > > gcc-transform-out-of-tree instead of transform. > > --- > > gcc/testsuite/g++.dg/gcov/gcov.exp | 4 ++-- > > gcc/testsuite/gcc.misc-tests/gcov.exp | 4 ++-- > > gcc/testsuite/lib/gcc-dg.exp | 13 +++++++++++++ > > 3 files changed, 17 insertions(+), 4 deletions(-) > > > > diff --git a/gcc/testsuite/g++.dg/gcov/gcov.exp b/gcc/testsuite/g++.dg/= gcov/gcov.exp > > index 04e7a0164865..c9f20958836b 100644 > > --- a/gcc/testsuite/g++.dg/gcov/gcov.exp > > +++ b/gcc/testsuite/g++.dg/gcov/gcov.exp > > @@ -24,9 +24,9 @@ global GXX_UNDER_TEST > > > > # Find gcov in the same directory as $GXX_UNDER_TEST. > > if { ![is_remote host] && [string match "*/*" [lindex $GXX_UNDER_TEST= 0]] } { > > - set GCOV [file dirname [lindex $GXX_UNDER_TEST 0]]/[transform gcov= ] > > + set GCOV [file dirname [lindex $GXX_UNDER_TEST 0]]/[gcc-transform-= out-of-tree gcov] > > } else { > > - set GCOV [transform gcov] > > + set GCOV [gcc-transform-out-of-tree gcov] > > } > > > > # Initialize harness. > > diff --git a/gcc/testsuite/gcc.misc-tests/gcov.exp b/gcc/testsuite/gcc.= misc-tests/gcov.exp > > index b8e9661aa537..90ceec46e0f0 100644 > > --- a/gcc/testsuite/gcc.misc-tests/gcov.exp > > +++ b/gcc/testsuite/gcc.misc-tests/gcov.exp > > @@ -24,9 +24,9 @@ global GCC_UNDER_TEST > > > > # For now find gcov in the same directory as $GCC_UNDER_TEST. > > if { ![is_remote host] && [string match "*/*" [lindex $GCC_UNDER_TEST= 0]] } { > > - set GCOV [file dirname [lindex $GCC_UNDER_TEST 0]]/[transform gcov= ] > > + set GCOV [file dirname [lindex $GCC_UNDER_TEST 0]]/[gcc-transform-= out-of-tree gcov] > > } else { > > - set GCOV [transform gcov] > > + set GCOV [gcc-transform-out-of-tree gcov] > > } > > > > # Initialize harness. > > diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.ex= p > > index 23ec038f41eb..2910c9ce0998 100644 > > --- a/gcc/testsuite/lib/gcc-dg.exp > > +++ b/gcc/testsuite/lib/gcc-dg.exp > > @@ -1429,5 +1429,18 @@ proc scan-symbol-not { args } { > > } > > } > > > > +# Transform a tool-name to its canonical-target-name by "transform" > > +# (which may return the original name for native targets) but only if > > +# testing out-of-tree. When in-tree, the tool is expected to be found > > +# by its original name, typically with some build-directory prefix > > +# prepended by the caller. > > +proc gcc-transform-out-of-tree { args } { > > + global TESTING_IN_BUILD_TREE > > + if { [info exists TESTING_IN_BUILD_TREE] } { > > + return $args; > > + } > > + return [transform $args] > > +} > > + > > set additional_prunes "" > > set dg_runtest_extra_prunes ""