From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp1.axis.com (smtp1.axis.com [195.60.68.17]) by sourceware.org (Postfix) with ESMTPS id 1B9023896C20 for ; Wed, 16 Nov 2022 02:11:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1B9023896C20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=axis.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=axis.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=axis.com; q=dns/txt; s=axis-central1; t=1668564717; x=1700100717; h=from:to:cc:in-reply-to:subject:mime-version: content-transfer-encoding:references:message-id:date; bh=mjLmq9dp6oh/S7s2m2Vl5oRraF+bSt4DzPpq5P2rEnU=; b=j7CJgyN6mKVuaZBAA+U5YeoV0a6GH4II+pyFfBZXJAEqzy6zvYDCSXbB Jt67MEd0ZN/pvG+p+J4gQsgoUQuV+uIp+ZFqUGw6ZQ0oLHiUh1tyocmYI ofTPWSVDB3IwwxbRhPLciQp/Co4Xmlah664WmJ/C2zkeXKD/BB2EH/MPw EBA5R8B9FpiC3mCcc0ROReFBMb9tp5LIrloYUv6CmrYQ95on4T/u5ATYG qypVUE9qMR2TNQhLBZF5Yk4M/4bg8Ykon0Z94+0UFY1lGwWBCP8P1aKvj AmiBk7376nIssUjxLHBIupuYWHcQG+JlGbWSxDJJfNRtLSuybIU3nMxcB A==; From: Hans-Peter Nilsson To: , Torbjorn SVENSSON CC: In-Reply-To: (message from Torbjorn SVENSSON via Gcc-patches on Mon, 12 Sep 2022 11:39:44 +0200) Subject: [PATCH] testsuite: Fix mistransformed gcov MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 8BIT 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> Message-ID: <20221116021154.4AE372042F@pchp3.se.axis.com> Date: Wed, 16 Nov 2022 03:11:54 +0100 X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,SPF_HELO_PASS,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: 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 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=gcov.exp - In an empty new directory after native installation in /pre. /gccsrc/top/contrib/test_installed --prefix=/pre gcov.exp - Also, for good measure (mentioned somewhere in the installation documentation) native: for tool in gcc g++ ; do env PATH=/pre/bin:$PATH runtest \ --tool $tool --srcdir=/gccsrc/top/gcc/testsuite gcov.exp; done For a cris-elf cross: - In the gcc build directory: make check 'RUNTESTFLAGS=--target_board=cris-sim gcov.exp' - In an empty new directory after installation in /pre. /gccsrc/top/contrib/test_installed --with-gcc=/pre/bin/cris-elf-gcc \ --with-g++=/pre/bin/cris-elf-g++ --with-gfortran=/pre/bin/cris-elf-gfortran \ --target=cris-elf --target_board=cris-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.exp 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 "" -- 2.30.2