gcov: support GCC 12.1 and newer compilers

[PATCH] gcov: support GCC 12.1 and newer compilers

Posted by Martin Liška 3 years, 5 months ago

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 <mliska@suse.cz>
---
 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

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

Posted by Peter Oberparleiter 3 years, 5 months ago

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 <mliska@suse.cz>

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

Tested-by: Peter Oberparleiter <oberpar@linux.ibm.com>
Reviewed-by: Peter Oberparleiter <oberpar@linux.ibm.com>

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

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

Posted by Andrew Morton 3 years, 5 months ago

On Fri, 14 Oct 2022 11:30:51 +0200 Peter Oberparleiter <oberpar@linux.ibm.com> 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 <mliska@suse.cz>
> 
> Looks good, thanks! I successfully tested this patch on s390 using GCC
> 12.2 and 11.2.
> 
> Tested-by: Peter Oberparleiter <oberpar@linux.ibm.com>
> Reviewed-by: Peter Oberparleiter <oberpar@linux.ibm.com>
> 
> 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.

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

Posted by David Laight 3 years, 5 months ago

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)

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

Posted by Martin Liška 3 years, 5 months ago

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)

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

Posted by Martin Liška 3 years, 5 months ago

On 10/17/22 02:40, Andrew Morton wrote:
> On Fri, 14 Oct 2022 11:30:51 +0200 Peter Oberparleiter <oberpar@linux.ibm.com> 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 <mliska@suse.cz>
>>
>> Looks good, thanks! I successfully tested this patch on s390 using GCC
>> 12.2 and 11.2.
>>
>> Tested-by: Peter Oberparleiter <oberpar@linux.ibm.com>
>> Reviewed-by: Peter Oberparleiter <oberpar@linux.ibm.com>
>>
>> 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.