* [PATCH v2] gcov: Respect triplet when looking for gcov
@ 2022-09-09 12:56 Torbjörn SVENSSON
2022-09-11 14:34 ` Mikael Morin
0 siblings, 1 reply; 12+ messages in thread
From: Torbjörn SVENSSON @ 2022-09-09 12:56 UTC (permalink / raw)
To: gcc-patches
Cc: hubicka, mliska, nathan, ro, mikestump, Torbjörn SVENSSON
When testing a cross toolchain outside the build tree, the binary name
for gcov is prefixed with the triplet.
gcc/testsuite/ChangeLog:
* g++.dg/gcov/gcov.exp: Respect triplet when looking for gcov.
* gcc.misc-tests/gcov.exp: Likewise.
Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
---
gcc/testsuite/g++.dg/gcov/gcov.exp | 4 ++--
gcc/testsuite/gcc.misc-tests/gcov.exp | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/gcc/testsuite/g++.dg/gcov/gcov.exp b/gcc/testsuite/g++.dg/gcov/gcov.exp
index 88acd95c361..04e7a016486 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]]/gcov
+ set GCOV [file dirname [lindex $GXX_UNDER_TEST 0]]/[transform gcov]
} else {
- set GCOV gcov
+ set GCOV [transform gcov]
}
# Initialize harness.
diff --git a/gcc/testsuite/gcc.misc-tests/gcov.exp b/gcc/testsuite/gcc.misc-tests/gcov.exp
index 82376d90ac2..a55ce234f6e 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]]/gcov
+ set GCOV [file dirname [lindex $GCC_UNDER_TEST 0]]/[transform gcov]
} else {
- set GCOV gcov
+ set GCOV {transform gcov]
}
# Initialize harness.
--
2.25.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] gcov: Respect triplet when looking for gcov
2022-09-09 12:56 [PATCH v2] gcov: Respect triplet when looking for gcov Torbjörn SVENSSON
@ 2022-09-11 14:34 ` Mikael Morin
2022-09-11 16:04 ` Torbjorn SVENSSON
0 siblings, 1 reply; 12+ messages in thread
From: Mikael Morin @ 2022-09-11 14:34 UTC (permalink / raw)
To: Torbjörn SVENSSON, gcc-patches; +Cc: nathan, hubicka
Hello,
> diff --git a/gcc/testsuite/gcc.misc-tests/gcov.exp b/gcc/testsuite/gcc.misc-tests/gcov.exp
> index 82376d90ac2..a55ce234f6e 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
(...)
> } else {
> - set GCOV gcov
> + set GCOV {transform gcov]
Typo: I guess the opening curly bracket '{' should be a square one '['?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] gcov: Respect triplet when looking for gcov
2022-09-11 14:34 ` Mikael Morin
@ 2022-09-11 16:04 ` Torbjorn SVENSSON
2022-09-11 19:38 ` Mikael Morin
0 siblings, 1 reply; 12+ messages in thread
From: Torbjorn SVENSSON @ 2022-09-11 16:04 UTC (permalink / raw)
To: Mikael Morin, gcc-patches; +Cc: nathan, hubicka
Hi,
On 2022-09-11 16:34, Mikael Morin wrote:
> Hello,
>
>> diff --git a/gcc/testsuite/gcc.misc-tests/gcov.exp
>> b/gcc/testsuite/gcc.misc-tests/gcov.exp
>> index 82376d90ac2..a55ce234f6e 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
> (...)
>> } else {
>> - set GCOV gcov
>> + set GCOV {transform gcov]
>
> Typo: I guess the opening curly bracket '{' should be a square one '['?
Yes. Apparently I was too stressed when preparing this patch. Can you
fix it for me and submit it or do you want me to send a v3?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] gcov: Respect triplet when looking for gcov
2022-09-11 16:04 ` Torbjorn SVENSSON
@ 2022-09-11 19:38 ` Mikael Morin
2022-09-12 7:06 ` Torbjorn SVENSSON
0 siblings, 1 reply; 12+ messages in thread
From: Mikael Morin @ 2022-09-11 19:38 UTC (permalink / raw)
To: Torbjorn SVENSSON, gcc-patches; +Cc: nathan, hubicka
Le 11/09/2022 à 18:04, Torbjorn SVENSSON a écrit :
> Can you fix it for me and submit it or do you want me to send a v3?
For trivial things like this, there is no need for a v3 (nor was there
for a v2).
Do you miss a git write account and need someone to push for you?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] gcov: Respect triplet when looking for gcov
2022-09-11 19:38 ` Mikael Morin
@ 2022-09-12 7:06 ` Torbjorn SVENSSON
2022-09-12 7:40 ` Mikael Morin
2022-09-12 8:40 ` Martin Liška
0 siblings, 2 replies; 12+ messages in thread
From: Torbjorn SVENSSON @ 2022-09-12 7:06 UTC (permalink / raw)
To: Mikael Morin, gcc-patches; +Cc: nathan, hubicka
On 2022-09-11 21:38, Mikael Morin wrote:
> Le 11/09/2022 à 18:04, Torbjorn SVENSSON a écrit :
>> Can you fix it for me and submit it or do you want me to send a v3?
>
> For trivial things like this, there is no need for a v3 (nor was there
> for a v2).
> Do you miss a git write account and need someone to push for you?
Ok!
I do not have any write access, so yes, please push it for me!
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] gcov: Respect triplet when looking for gcov
2022-09-12 7:06 ` Torbjorn SVENSSON
@ 2022-09-12 7:40 ` Mikael Morin
2022-09-12 8:40 ` Martin Liška
1 sibling, 0 replies; 12+ messages in thread
From: Mikael Morin @ 2022-09-12 7:40 UTC (permalink / raw)
To: Torbjorn SVENSSON, gcc-patches; +Cc: nathan, hubicka
Le 12/09/2022 à 09:06, Torbjorn SVENSSON a écrit :
>
>
> On 2022-09-11 21:38, Mikael Morin wrote:
>> Le 11/09/2022 à 18:04, Torbjorn SVENSSON a écrit :
>>> Can you fix it for me and submit it or do you want me to send a v3?
>>
>> For trivial things like this, there is no need for a v3 (nor was there
>> for a v2).
>> Do you miss a git write account and need someone to push for you?
>
> Ok!
>
> I do not have any write access, so yes, please push it for me!
No problem, I will do it the next time I synchronize my work tree, but
it will have to wait a few days until that time.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] gcov: Respect triplet when looking for gcov
2022-09-12 7:06 ` Torbjorn SVENSSON
2022-09-12 7:40 ` Mikael Morin
@ 2022-09-12 8:40 ` Martin Liška
2022-09-12 9:39 ` Torbjorn SVENSSON
1 sibling, 1 reply; 12+ messages in thread
From: Martin Liška @ 2022-09-12 8:40 UTC (permalink / raw)
To: Torbjorn SVENSSON, Mikael Morin, gcc-patches; +Cc: nathan, hubicka
On 9/12/22 09:06, Torbjorn SVENSSON via Gcc-patches wrote:
>
>
> On 2022-09-11 21:38, Mikael Morin wrote:
>> Le 11/09/2022 à 18:04, Torbjorn SVENSSON a écrit :
>>> Can you fix it for me and submit it or do you want me to send a v3?
>>
>> For trivial things like this, there is no need for a v3 (nor was there for a v2).
>> Do you miss a git write account and need someone to push for you?
>
> Ok!
>
> I do not have any write access, so yes, please push it for me!
Please attach a patch with git format-patch and I'm going to push it now.
Cheers,
Martin
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] gcov: Respect triplet when looking for gcov
2022-09-12 8:40 ` Martin Liška
@ 2022-09-12 9:39 ` Torbjorn SVENSSON
2022-09-12 9:42 ` Martin Liška
2022-11-16 2:11 ` [PATCH] testsuite: Fix mistransformed gcov Hans-Peter Nilsson
0 siblings, 2 replies; 12+ messages in thread
From: Torbjorn SVENSSON @ 2022-09-12 9:39 UTC (permalink / raw)
To: Martin Liška, Mikael Morin, gcc-patches; +Cc: nathan, hubicka
[-- Attachment #1: Type: text/plain, Size: 653 bytes --]
On 2022-09-12 10:40, Martin Liška wrote:
> On 9/12/22 09:06, Torbjorn SVENSSON via Gcc-patches wrote:
>>
>>
>> On 2022-09-11 21:38, Mikael Morin wrote:
>>> Le 11/09/2022 à 18:04, Torbjorn SVENSSON a écrit :
>>>> Can you fix it for me and submit it or do you want me to send a v3?
>>>
>>> For trivial things like this, there is no need for a v3 (nor was there for a v2).
>>> Do you miss a git write account and need someone to push for you?
>>
>> Ok!
>>
>> I do not have any write access, so yes, please push it for me!
>
> Please attach a patch with git format-patch and I'm going to push it now.
>
> Cheers,
> Martin
Patch attached as requested.
[-- Attachment #2: 0001-gcov-Respect-triplet-when-looking-for-gcov.patch --]
[-- Type: text/plain, Size: 2044 bytes --]
From 0400f4b34a62ad5a54e87c537b4a41bc630fd105 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Torbj=C3=B6rn=20SVENSSON?= <torbjorn.svensson@foss.st.com>
Date: Fri, 9 Sep 2022 12:19:27 +0200
Subject: [PATCH] gcov: Respect triplet when looking for gcov
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
When testing a cross toolchain outside the build tree, the binary name
for gcov is prefixed with the triplet.
gcc/testsuite/ChangeLog:
* g++.dg/gcov/gcov.exp: Respect triplet when looking for gcov.
* gcc.misc-tests/gcov.exp: Likewise.
Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
---
gcc/testsuite/g++.dg/gcov/gcov.exp | 4 ++--
gcc/testsuite/gcc.misc-tests/gcov.exp | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/gcc/testsuite/g++.dg/gcov/gcov.exp b/gcc/testsuite/g++.dg/gcov/gcov.exp
index 88acd95c361..04e7a016486 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]]/gcov
+ set GCOV [file dirname [lindex $GXX_UNDER_TEST 0]]/[transform gcov]
} else {
- set GCOV gcov
+ set GCOV [transform gcov]
}
# Initialize harness.
diff --git a/gcc/testsuite/gcc.misc-tests/gcov.exp b/gcc/testsuite/gcc.misc-tests/gcov.exp
index 82376d90ac2..b8e9661aa53 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]]/gcov
+ set GCOV [file dirname [lindex $GCC_UNDER_TEST 0]]/[transform gcov]
} else {
- set GCOV gcov
+ set GCOV [transform gcov]
}
# Initialize harness.
--
2.25.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] gcov: Respect triplet when looking for gcov
2022-09-12 9:39 ` Torbjorn SVENSSON
@ 2022-09-12 9:42 ` Martin Liška
2022-11-16 2:11 ` [PATCH] testsuite: Fix mistransformed gcov Hans-Peter Nilsson
1 sibling, 0 replies; 12+ messages in thread
From: Martin Liška @ 2022-09-12 9:42 UTC (permalink / raw)
To: Torbjorn SVENSSON, Mikael Morin, gcc-patches; +Cc: nathan, hubicka
On 9/12/22 11:39, Torbjorn SVENSSON wrote:
> On 2022-09-12 10:40, Martin Liška wrote:
>> On 9/12/22 09:06, Torbjorn SVENSSON via Gcc-patches wrote:
>>>
>>>
>>> On 2022-09-11 21:38, Mikael Morin wrote:
>>>> Le 11/09/2022 à 18:04, Torbjorn SVENSSON a écrit :
>>>>> Can you fix it for me and submit it or do you want me to send a v3?
>>>>
>>>> For trivial things like this, there is no need for a v3 (nor was there for a v2).
>>>> Do you miss a git write account and need someone to push for you?
>>>
>>> Ok!
>>>
>>> I do not have any write access, so yes, please push it for me!
>>
>> Please attach a patch with git format-patch and I'm going to push it now.
>>
>> Cheers,
>> Martin
>
> Patch attached as requested.
Thanks.
Pushed with a small correction of the commit message:
remote: *** ChangeLog format failed:
remote: *** ERR: line should start with a tab: " * g++.dg/gcov/gcov.exp: Respect triplet when looking for gcov."
remote: *** ERR: line should start with a tab: " * gcc.misc-tests/gcov.exp: Likewise."
as 34b9a03353d3fdc5c57f2708469d0df78c6d6508.
Cheers,
Martin
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] testsuite: Fix mistransformed gcov
2022-09-12 9:39 ` Torbjorn SVENSSON
2022-09-12 9:42 ` Martin Liška
@ 2022-11-16 2:11 ` Hans-Peter Nilsson
2022-11-16 7:59 ` Torbjorn SVENSSON
1 sibling, 1 reply; 12+ messages in thread
From: Hans-Peter Nilsson @ 2022-11-16 2:11 UTC (permalink / raw)
To: gcc-patches, Torbjorn SVENSSON; +Cc: mikestump
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] testsuite: Fix mistransformed gcov
2022-11-16 2:11 ` [PATCH] testsuite: Fix mistransformed gcov Hans-Peter Nilsson
@ 2022-11-16 7:59 ` Torbjorn SVENSSON
2022-11-16 15:38 ` Richard Biener
0 siblings, 1 reply; 12+ messages in thread
From: Torbjorn SVENSSON @ 2022-11-16 7:59 UTC (permalink / raw)
To: Hans-Peter Nilsson, gcc-patches; +Cc: mikestump
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.
Kind regards,
Torbjörn
> 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 ""
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] testsuite: Fix mistransformed gcov
2022-11-16 7:59 ` Torbjorn SVENSSON
@ 2022-11-16 15:38 ` Richard Biener
0 siblings, 0 replies; 12+ messages in thread
From: Richard Biener @ 2022-11-16 15:38 UTC (permalink / raw)
To: Torbjorn SVENSSON; +Cc: Hans-Peter Nilsson, gcc-patches, mikestump
On Wed, Nov 16, 2022 at 9:00 AM Torbjorn SVENSSON via Gcc-patches
<gcc-patches@gcc.gnu.org> 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örn
>
> > 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 ""
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-11-16 15:38 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-09 12:56 [PATCH v2] gcov: Respect triplet when looking for gcov Torbjörn SVENSSON
2022-09-11 14:34 ` Mikael Morin
2022-09-11 16:04 ` Torbjorn SVENSSON
2022-09-11 19:38 ` Mikael Morin
2022-09-12 7:06 ` Torbjorn SVENSSON
2022-09-12 7:40 ` Mikael Morin
2022-09-12 8:40 ` Martin Liška
2022-09-12 9:39 ` Torbjorn SVENSSON
2022-09-12 9:42 ` Martin Liška
2022-11-16 2:11 ` [PATCH] testsuite: Fix mistransformed gcov Hans-Peter Nilsson
2022-11-16 7:59 ` Torbjorn SVENSSON
2022-11-16 15:38 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).