2022-10-13 08:01:16

by Martin Liška

[permalink] [raw]
Subject: [PATCH] gcov: support GCC 12.1 and newer compilers

Starting with GCC 12.1, there are 2 significant changes to the .gcda
file format that needs to be supported:

a) [gcov: Use system IO buffering] (23eb66d1d46a34cb28c4acbdf8a1deb80a7c5a05) changed
that all sizes in the format are in bytes and not in words (4B)
b) [gcov: make profile merging smarter] (72e0c742bd01f8e7e6dcca64042b9ad7e75979de)
add a new checksum to the file header.

Tested with GCC 7.5, 10.4, 12.2 and the current master.

Signed-off-by: Martin Liska <[email protected]>
---
kernel/gcov/gcc_4_7.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/kernel/gcov/gcc_4_7.c b/kernel/gcov/gcc_4_7.c
index 460c12b7dfea..7971e989e425 100644
--- a/kernel/gcov/gcc_4_7.c
+++ b/kernel/gcov/gcc_4_7.c
@@ -30,6 +30,13 @@

#define GCOV_TAG_FUNCTION_LENGTH 3

+/* Since GCC 12.1 sizes are in BYTES and not in WORDS (4B). */
+#if (__GNUC__ >= 12)
+#define GCOV_UNIT_SIZE 4
+#else
+#define GCOV_UNIT_SIZE 1
+#endif
+
static struct gcov_info *gcov_info_head;

/**
@@ -383,12 +390,18 @@ size_t convert_to_gcda(char *buffer, struct gcov_info *info)
pos += store_gcov_u32(buffer, pos, info->version);
pos += store_gcov_u32(buffer, pos, info->stamp);

+#if (__GNUC__ >= 12)
+ /* Use zero as checksum of the compilation unit. */
+ pos += store_gcov_u32(buffer, pos, 0);
+#endif
+
for (fi_idx = 0; fi_idx < info->n_functions; fi_idx++) {
fi_ptr = info->functions[fi_idx];

/* Function record. */
pos += store_gcov_u32(buffer, pos, GCOV_TAG_FUNCTION);
- pos += store_gcov_u32(buffer, pos, GCOV_TAG_FUNCTION_LENGTH);
+ pos += store_gcov_u32(buffer, pos,
+ GCOV_TAG_FUNCTION_LENGTH * GCOV_UNIT_SIZE);
pos += store_gcov_u32(buffer, pos, fi_ptr->ident);
pos += store_gcov_u32(buffer, pos, fi_ptr->lineno_checksum);
pos += store_gcov_u32(buffer, pos, fi_ptr->cfg_checksum);
@@ -402,7 +415,8 @@ size_t convert_to_gcda(char *buffer, struct gcov_info *info)
/* Counter record. */
pos += store_gcov_u32(buffer, pos,
GCOV_TAG_FOR_COUNTER(ct_idx));
- pos += store_gcov_u32(buffer, pos, ci_ptr->num * 2);
+ pos += store_gcov_u32(buffer, pos,
+ ci_ptr->num * 2 * GCOV_UNIT_SIZE);

for (cv_idx = 0; cv_idx < ci_ptr->num; cv_idx++) {
pos += store_gcov_u64(buffer, pos,
--
2.37.3


2022-10-14 09:36:46

by Peter Oberparleiter

[permalink] [raw]
Subject: Re: [PATCH] gcov: support GCC 12.1 and newer compilers

On 13.10.2022 09:40, Martin Liška wrote:
> Starting with GCC 12.1, there are 2 significant changes to the .gcda
> file format that needs to be supported:
>
> a) [gcov: Use system IO buffering] (23eb66d1d46a34cb28c4acbdf8a1deb80a7c5a05) changed
> that all sizes in the format are in bytes and not in words (4B)
> b) [gcov: make profile merging smarter] (72e0c742bd01f8e7e6dcca64042b9ad7e75979de)
> add a new checksum to the file header.
>
> Tested with GCC 7.5, 10.4, 12.2 and the current master.
>
> Signed-off-by: Martin Liska <[email protected]>

Looks good, thanks! I successfully tested this patch on s390 using GCC
12.2 and 11.2.

Tested-by: Peter Oberparleiter <[email protected]>
Reviewed-by: Peter Oberparleiter <[email protected]>

Andrew, could you add this patch via your tree?


> ---
> kernel/gcov/gcc_4_7.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/gcov/gcc_4_7.c b/kernel/gcov/gcc_4_7.c
> index 460c12b7dfea..7971e989e425 100644
> --- a/kernel/gcov/gcc_4_7.c
> +++ b/kernel/gcov/gcc_4_7.c
> @@ -30,6 +30,13 @@
>
> #define GCOV_TAG_FUNCTION_LENGTH 3
>
> +/* Since GCC 12.1 sizes are in BYTES and not in WORDS (4B). */
> +#if (__GNUC__ >= 12)
> +#define GCOV_UNIT_SIZE 4
> +#else
> +#define GCOV_UNIT_SIZE 1
> +#endif
> +
> static struct gcov_info *gcov_info_head;
>
> /**
> @@ -383,12 +390,18 @@ size_t convert_to_gcda(char *buffer, struct gcov_info *info)
> pos += store_gcov_u32(buffer, pos, info->version);
> pos += store_gcov_u32(buffer, pos, info->stamp);
>
> +#if (__GNUC__ >= 12)
> + /* Use zero as checksum of the compilation unit. */
> + pos += store_gcov_u32(buffer, pos, 0);
> +#endif
> +
> for (fi_idx = 0; fi_idx < info->n_functions; fi_idx++) {
> fi_ptr = info->functions[fi_idx];
>
> /* Function record. */
> pos += store_gcov_u32(buffer, pos, GCOV_TAG_FUNCTION);
> - pos += store_gcov_u32(buffer, pos, GCOV_TAG_FUNCTION_LENGTH);
> + pos += store_gcov_u32(buffer, pos,
> + GCOV_TAG_FUNCTION_LENGTH * GCOV_UNIT_SIZE);
> pos += store_gcov_u32(buffer, pos, fi_ptr->ident);
> pos += store_gcov_u32(buffer, pos, fi_ptr->lineno_checksum);
> pos += store_gcov_u32(buffer, pos, fi_ptr->cfg_checksum);
> @@ -402,7 +415,8 @@ size_t convert_to_gcda(char *buffer, struct gcov_info *info)
> /* Counter record. */
> pos += store_gcov_u32(buffer, pos,
> GCOV_TAG_FOR_COUNTER(ct_idx));
> - pos += store_gcov_u32(buffer, pos, ci_ptr->num * 2);
> + pos += store_gcov_u32(buffer, pos,
> + ci_ptr->num * 2 * GCOV_UNIT_SIZE);
>
> for (cv_idx = 0; cv_idx < ci_ptr->num; cv_idx++) {
> pos += store_gcov_u64(buffer, pos,

--
Peter Oberparleiter
Linux on IBM Z Development - IBM Germany R&D

2022-10-17 01:24:30

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] gcov: support GCC 12.1 and newer compilers

On Fri, 14 Oct 2022 11:30:51 +0200 Peter Oberparleiter <[email protected]> wrote:

> On 13.10.2022 09:40, Martin Liška wrote:
> > Starting with GCC 12.1, there are 2 significant changes to the .gcda
> > file format that needs to be supported:
> >
> > a) [gcov: Use system IO buffering] (23eb66d1d46a34cb28c4acbdf8a1deb80a7c5a05) changed
> > that all sizes in the format are in bytes and not in words (4B)
> > b) [gcov: make profile merging smarter] (72e0c742bd01f8e7e6dcca64042b9ad7e75979de)
> > add a new checksum to the file header.
> >
> > Tested with GCC 7.5, 10.4, 12.2 and the current master.
> >
> > Signed-off-by: Martin Liska <[email protected]>
>
> Looks good, thanks! I successfully tested this patch on s390 using GCC
> 12.2 and 11.2.
>
> Tested-by: Peter Oberparleiter <[email protected]>
> Reviewed-by: Peter Oberparleiter <[email protected]>
>
> Andrew, could you add this patch via your tree?

Sure.

The changelog doesn't tell us what the user-visible effects of this are
(please, it should do so), but it sounds to me like those effects are
"gcov is utterly busted".

So I'll add a cc:stable to this, so that people can use new gcc
versions to build older kernels.

2022-10-17 08:14:10

by Martin Liška

[permalink] [raw]
Subject: Re: [PATCH] gcov: support GCC 12.1 and newer compilers

On 10/17/22 02:40, Andrew Morton wrote:
> On Fri, 14 Oct 2022 11:30:51 +0200 Peter Oberparleiter <[email protected]> wrote:
>
>> On 13.10.2022 09:40, Martin Liška wrote:
>>> Starting with GCC 12.1, there are 2 significant changes to the .gcda
>>> file format that needs to be supported:
>>>
>>> a) [gcov: Use system IO buffering] (23eb66d1d46a34cb28c4acbdf8a1deb80a7c5a05) changed
>>> that all sizes in the format are in bytes and not in words (4B)
>>> b) [gcov: make profile merging smarter] (72e0c742bd01f8e7e6dcca64042b9ad7e75979de)
>>> add a new checksum to the file header.
>>>
>>> Tested with GCC 7.5, 10.4, 12.2 and the current master.
>>>
>>> Signed-off-by: Martin Liska <[email protected]>
>>
>> Looks good, thanks! I successfully tested this patch on s390 using GCC
>> 12.2 and 11.2.
>>
>> Tested-by: Peter Oberparleiter <[email protected]>
>> Reviewed-by: Peter Oberparleiter <[email protected]>
>>
>> Andrew, could you add this patch via your tree?
>
> Sure.
>
> The changelog doesn't tell us what the user-visible effects of this are
> (please, it should do so), but it sounds to me like those effects are
> "gcov is utterly busted".

Hi.

Sorry for the missing user-visible effect. You are correct, the created .gcda
format can't be read by gcov tool since GCC 12.1 release.

Thanks,
Martin

>
> So I'll add a cc:stable to this, so that people can use new gcc
> versions to build older kernels.

2022-10-17 12:01:06

by David Laight

[permalink] [raw]
Subject: RE: [PATCH] gcov: support GCC 12.1 and newer compilers

From: Andrew Morton
> Sent: 17 October 2022 01:41
..
> The changelog doesn't tell us what the user-visible effects of this are
> (please, it should do so), but it sounds to me like those effects are
> "gcov is utterly busted".
>
> So I'll add a cc:stable to this, so that people can use new gcc
> versions to build older kernels.

I can't help wondering what happens if you link a binary
compiled with an old gcc with one build with a new gcc?

This could easily happen with out of tree loadable modules.
Or just linking an old .a file into a userspace binary.

Now maybe the gcov data isn't used (I've not looked up what
it is for) but is sounds like something whose format should
be set in stone?

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

2022-10-17 12:36:34

by Martin Liška

[permalink] [raw]
Subject: Re: [PATCH] gcov: support GCC 12.1 and newer compilers

On 10/17/22 13:38, David Laight wrote:
> From: Andrew Morton
>> Sent: 17 October 2022 01:41
> ..
>> The changelog doesn't tell us what the user-visible effects of this are
>> (please, it should do so), but it sounds to me like those effects are
>> "gcov is utterly busted".
>>
>> So I'll add a cc:stable to this, so that people can use new gcc
>> versions to build older kernels.
>
> I can't help wondering what happens if you link a binary
> compiled with an old gcc with one build with a new gcc?

It should be fine as the code emits each .gcda file with version that
comes from the compilation phase:

pos += store_gcov_u32(buffer, pos, info->version);

>
> This could easily happen with out of tree loadable modules.

Sure.

> Or just linking an old .a file into a userspace binary.
>
> Now maybe the gcov data isn't used (I've not looked up what
> it is for) but is sounds like something whose format should
> be set in stone?

No, I've doing some changes to the format based on new challenges we face.

Cheers,
Martin

>
> David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)