All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] gcov: re-fix clang-11 support
@ 2021-04-07 18:54 Nick Desaulniers
  2021-04-07 18:54 ` [PATCH 1/2] gcov: re-fix clang-11+ support Nick Desaulniers
  2021-04-07 18:54 ` [PATCH 2/2] gcov: re-drop support for clang-10 Nick Desaulniers
  0 siblings, 2 replies; 10+ messages in thread
From: Nick Desaulniers @ 2021-04-07 18:54 UTC (permalink / raw)
  To: Peter Oberparleiter
  Cc: Nathan Chancellor, linux-kernel, clang-built-linux,
	Andrew Morton, Fangrui Song, Prasad Sodagudi, Nick Desaulniers

LLVM changed the expected function signature for
llvm_gcda_emit_function() in the clang-11 release.  Users of clang-11 or
newer may have noticed their kernels producing invalid coverage
information:

$ llvm-cov gcov -a -c -u -f -b <input>.gcda -- gcno=<input>.gcno
1 <func>: checksum mismatch, \
  (<lineno chksum A>, <cfg chksum B>) != (<lineno chksum A>, <cfg chksum C>)
2 Invalid .gcda File!
...

Similar to the last series, the patch is broken in two. The first is
tagged for inclusion in stable in order to continue supporting newer
versions of clang (clang-11+) for that tree, then the second drops the
older implementations to keep one and only support clang-11+. This same
pattern was done recently in:
https://lore.kernel.org/lkml/20210312224132.3413602-1-ndesaulniers@google.com/

We've since added CI coverage of CONFIG_GCOV
https://github.com/ClangBuiltLinux/continuous-integration2/pull/107
but need to find a better way to test validating the coverage info in
userspace.

Nick Desaulniers (2):
  gcov: re-fix clang-11+ support
  gcov: re-drop support for clang-10

 kernel/gcov/clang.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

-- 
2.31.1.295.g9ea45b61b8-goog


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] gcov: re-fix clang-11+ support
  2021-04-07 18:54 [PATCH 0/2] gcov: re-fix clang-11 support Nick Desaulniers
@ 2021-04-07 18:54 ` Nick Desaulniers
  2021-04-07 19:51   ` Nathan Chancellor
  2021-04-07 21:21   ` Andrew Morton
  2021-04-07 18:54 ` [PATCH 2/2] gcov: re-drop support for clang-10 Nick Desaulniers
  1 sibling, 2 replies; 10+ messages in thread
From: Nick Desaulniers @ 2021-04-07 18:54 UTC (permalink / raw)
  To: Peter Oberparleiter
  Cc: Nathan Chancellor, linux-kernel, clang-built-linux,
	Andrew Morton, Fangrui Song, Prasad Sodagudi, Nick Desaulniers,
	stable

LLVM changed the expected function signature for
llvm_gcda_emit_function() in the clang-11 release.  Users of clang-11 or
newer may have noticed their kernels producing invalid coverage
information:

$ llvm-cov gcov -a -c -u -f -b <input>.gcda -- gcno=<input>.gcno
1 <func>: checksum mismatch, \
  (<lineno chksum A>, <cfg chksum B>) != (<lineno chksum A>, <cfg chksum C>)
2 Invalid .gcda File!
...

Fix up the function signatures so calling this function interprets its
parameters correctly and computes the correct cfg checksum. In
particular, in clang-11, the additional checksum is no longer optional.

Link: https://reviews.llvm.org/rG25544ce2df0daa4304c07e64b9c8b0f7df60c11d
Cc: stable@vger.kernel.org #5.4+
Reported-by: Prasad Sodagudi <psodagud@quicinc.com>
Tested-by: Prasad Sodagudi <psodagud@quicinc.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 kernel/gcov/clang.c | 38 +++++++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/kernel/gcov/clang.c b/kernel/gcov/clang.c
index d41f5ecda9db..1747204541bf 100644
--- a/kernel/gcov/clang.c
+++ b/kernel/gcov/clang.c
@@ -69,7 +69,9 @@ struct gcov_fn_info {
 
 	u32 ident;
 	u32 checksum;
+#if CONFIG_CLANG_VERSION < 110000
 	u8 use_extra_checksum;
+#endif
 	u32 cfg_checksum;
 
 	u32 num_counters;
@@ -111,6 +113,7 @@ void llvm_gcda_start_file(const char *orig_filename, u32 version, u32 checksum)
 }
 EXPORT_SYMBOL(llvm_gcda_start_file);
 
+#if CONFIG_CLANG_VERSION < 110000
 void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
 		u8 use_extra_checksum, u32 cfg_checksum)
 {
@@ -126,6 +129,21 @@ void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
 	info->cfg_checksum = cfg_checksum;
 	list_add_tail(&info->head, &current_info->functions);
 }
+#else
+void llvm_gcda_emit_function(u32 ident, u32 func_checksum, u32 cfg_checksum)
+{
+	struct gcov_fn_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
+
+	if (!info)
+		return;
+
+	INIT_LIST_HEAD(&info->head);
+	info->ident = ident;
+	info->checksum = func_checksum;
+	info->cfg_checksum = cfg_checksum;
+	list_add_tail(&info->head, &current_info->functions);
+}
+#endif
 EXPORT_SYMBOL(llvm_gcda_emit_function);
 
 void llvm_gcda_emit_arcs(u32 num_counters, u64 *counters)
@@ -256,11 +274,16 @@ int gcov_info_is_compatible(struct gcov_info *info1, struct gcov_info *info2)
 		!list_is_last(&fn_ptr2->head, &info2->functions)) {
 		if (fn_ptr1->checksum != fn_ptr2->checksum)
 			return false;
+#if CONFIG_CLANG_VERSION < 110000
 		if (fn_ptr1->use_extra_checksum != fn_ptr2->use_extra_checksum)
 			return false;
 		if (fn_ptr1->use_extra_checksum &&
 			fn_ptr1->cfg_checksum != fn_ptr2->cfg_checksum)
 			return false;
+#else
+		if (fn_ptr1->cfg_checksum != fn_ptr2->cfg_checksum)
+			return false;
+#endif
 		fn_ptr1 = list_next_entry(fn_ptr1, head);
 		fn_ptr2 = list_next_entry(fn_ptr2, head);
 	}
@@ -378,17 +401,22 @@ size_t convert_to_gcda(char *buffer, struct gcov_info *info)
 
 	list_for_each_entry(fi_ptr, &info->functions, head) {
 		u32 i;
-		u32 len = 2;
-
-		if (fi_ptr->use_extra_checksum)
-			len++;
 
 		pos += store_gcov_u32(buffer, pos, GCOV_TAG_FUNCTION);
-		pos += store_gcov_u32(buffer, pos, len);
+#if CONFIG_CLANG_VERSION < 110000
+		pos += store_gcov_u32(buffer, pos,
+			fi_ptr->use_extra_checksum ? 3 : 2);
+#else
+		pos += store_gcov_u32(buffer, pos, 3);
+#endif
 		pos += store_gcov_u32(buffer, pos, fi_ptr->ident);
 		pos += store_gcov_u32(buffer, pos, fi_ptr->checksum);
+#if CONFIG_CLANG_VERSION < 110000
 		if (fi_ptr->use_extra_checksum)
 			pos += store_gcov_u32(buffer, pos, fi_ptr->cfg_checksum);
+#else
+		pos += store_gcov_u32(buffer, pos, fi_ptr->cfg_checksum);
+#endif
 
 		pos += store_gcov_u32(buffer, pos, GCOV_TAG_COUNTER_BASE);
 		pos += store_gcov_u32(buffer, pos, fi_ptr->num_counters * 2);
-- 
2.31.1.295.g9ea45b61b8-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] gcov: re-drop support for clang-10
  2021-04-07 18:54 [PATCH 0/2] gcov: re-fix clang-11 support Nick Desaulniers
  2021-04-07 18:54 ` [PATCH 1/2] gcov: re-fix clang-11+ support Nick Desaulniers
@ 2021-04-07 18:54 ` Nick Desaulniers
  2021-04-07 19:07   ` Fangrui Song
  2021-04-07 19:51   ` Nathan Chancellor
  1 sibling, 2 replies; 10+ messages in thread
From: Nick Desaulniers @ 2021-04-07 18:54 UTC (permalink / raw)
  To: Peter Oberparleiter
  Cc: Nathan Chancellor, linux-kernel, clang-built-linux,
	Andrew Morton, Fangrui Song, Prasad Sodagudi, Nick Desaulniers

LLVM changed the expected function signatures for
llvm_gcda_emit_function() in the clang-11 release.  Drop the older
implementations and require folks to upgrade their compiler if they're
interested in GCOV support.

Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 kernel/gcov/clang.c | 40 ----------------------------------------
 1 file changed, 40 deletions(-)

diff --git a/kernel/gcov/clang.c b/kernel/gcov/clang.c
index 1747204541bf..78c4dc751080 100644
--- a/kernel/gcov/clang.c
+++ b/kernel/gcov/clang.c
@@ -69,9 +69,6 @@ struct gcov_fn_info {
 
 	u32 ident;
 	u32 checksum;
-#if CONFIG_CLANG_VERSION < 110000
-	u8 use_extra_checksum;
-#endif
 	u32 cfg_checksum;
 
 	u32 num_counters;
@@ -113,23 +110,6 @@ void llvm_gcda_start_file(const char *orig_filename, u32 version, u32 checksum)
 }
 EXPORT_SYMBOL(llvm_gcda_start_file);
 
-#if CONFIG_CLANG_VERSION < 110000
-void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
-		u8 use_extra_checksum, u32 cfg_checksum)
-{
-	struct gcov_fn_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
-
-	if (!info)
-		return;
-
-	INIT_LIST_HEAD(&info->head);
-	info->ident = ident;
-	info->checksum = func_checksum;
-	info->use_extra_checksum = use_extra_checksum;
-	info->cfg_checksum = cfg_checksum;
-	list_add_tail(&info->head, &current_info->functions);
-}
-#else
 void llvm_gcda_emit_function(u32 ident, u32 func_checksum, u32 cfg_checksum)
 {
 	struct gcov_fn_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
@@ -143,7 +123,6 @@ void llvm_gcda_emit_function(u32 ident, u32 func_checksum, u32 cfg_checksum)
 	info->cfg_checksum = cfg_checksum;
 	list_add_tail(&info->head, &current_info->functions);
 }
-#endif
 EXPORT_SYMBOL(llvm_gcda_emit_function);
 
 void llvm_gcda_emit_arcs(u32 num_counters, u64 *counters)
@@ -274,16 +253,8 @@ int gcov_info_is_compatible(struct gcov_info *info1, struct gcov_info *info2)
 		!list_is_last(&fn_ptr2->head, &info2->functions)) {
 		if (fn_ptr1->checksum != fn_ptr2->checksum)
 			return false;
-#if CONFIG_CLANG_VERSION < 110000
-		if (fn_ptr1->use_extra_checksum != fn_ptr2->use_extra_checksum)
-			return false;
-		if (fn_ptr1->use_extra_checksum &&
-			fn_ptr1->cfg_checksum != fn_ptr2->cfg_checksum)
-			return false;
-#else
 		if (fn_ptr1->cfg_checksum != fn_ptr2->cfg_checksum)
 			return false;
-#endif
 		fn_ptr1 = list_next_entry(fn_ptr1, head);
 		fn_ptr2 = list_next_entry(fn_ptr2, head);
 	}
@@ -403,21 +374,10 @@ size_t convert_to_gcda(char *buffer, struct gcov_info *info)
 		u32 i;
 
 		pos += store_gcov_u32(buffer, pos, GCOV_TAG_FUNCTION);
-#if CONFIG_CLANG_VERSION < 110000
-		pos += store_gcov_u32(buffer, pos,
-			fi_ptr->use_extra_checksum ? 3 : 2);
-#else
 		pos += store_gcov_u32(buffer, pos, 3);
-#endif
 		pos += store_gcov_u32(buffer, pos, fi_ptr->ident);
 		pos += store_gcov_u32(buffer, pos, fi_ptr->checksum);
-#if CONFIG_CLANG_VERSION < 110000
-		if (fi_ptr->use_extra_checksum)
-			pos += store_gcov_u32(buffer, pos, fi_ptr->cfg_checksum);
-#else
 		pos += store_gcov_u32(buffer, pos, fi_ptr->cfg_checksum);
-#endif
-
 		pos += store_gcov_u32(buffer, pos, GCOV_TAG_COUNTER_BASE);
 		pos += store_gcov_u32(buffer, pos, fi_ptr->num_counters * 2);
 		for (i = 0; i < fi_ptr->num_counters; i++)
-- 
2.31.1.295.g9ea45b61b8-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] gcov: re-drop support for clang-10
  2021-04-07 18:54 ` [PATCH 2/2] gcov: re-drop support for clang-10 Nick Desaulniers
@ 2021-04-07 19:07   ` Fangrui Song
  2021-04-07 19:51   ` Nathan Chancellor
  1 sibling, 0 replies; 10+ messages in thread
From: Fangrui Song @ 2021-04-07 19:07 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Peter Oberparleiter, Nathan Chancellor, linux-kernel,
	clang-built-linux, Andrew Morton, Prasad Sodagudi


On 2021-04-07, Nick Desaulniers wrote:
>LLVM changed the expected function signatures for
>llvm_gcda_emit_function() in the clang-11 release.  Drop the older
>implementations and require folks to upgrade their compiler if they're
>interested in GCOV support.
>
>Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>---
> kernel/gcov/clang.c | 40 ----------------------------------------
> 1 file changed, 40 deletions(-)
>
>diff --git a/kernel/gcov/clang.c b/kernel/gcov/clang.c
>index 1747204541bf..78c4dc751080 100644
>--- a/kernel/gcov/clang.c
>+++ b/kernel/gcov/clang.c
>@@ -69,9 +69,6 @@ struct gcov_fn_info {
>
> 	u32 ident;
> 	u32 checksum;
>-#if CONFIG_CLANG_VERSION < 110000
>-	u8 use_extra_checksum;
>-#endif
> 	u32 cfg_checksum;
>
> 	u32 num_counters;
>@@ -113,23 +110,6 @@ void llvm_gcda_start_file(const char *orig_filename, u32 version, u32 checksum)
> }
> EXPORT_SYMBOL(llvm_gcda_start_file);
>
>-#if CONFIG_CLANG_VERSION < 110000
>-void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
>-		u8 use_extra_checksum, u32 cfg_checksum)
>-{
>-	struct gcov_fn_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
>-
>-	if (!info)
>-		return;
>-
>-	INIT_LIST_HEAD(&info->head);
>-	info->ident = ident;
>-	info->checksum = func_checksum;
>-	info->use_extra_checksum = use_extra_checksum;
>-	info->cfg_checksum = cfg_checksum;
>-	list_add_tail(&info->head, &current_info->functions);
>-}
>-#else
> void llvm_gcda_emit_function(u32 ident, u32 func_checksum, u32 cfg_checksum)
> {
> 	struct gcov_fn_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
>@@ -143,7 +123,6 @@ void llvm_gcda_emit_function(u32 ident, u32 func_checksum, u32 cfg_checksum)
> 	info->cfg_checksum = cfg_checksum;
> 	list_add_tail(&info->head, &current_info->functions);
> }
>-#endif
> EXPORT_SYMBOL(llvm_gcda_emit_function);
>
> void llvm_gcda_emit_arcs(u32 num_counters, u64 *counters)
>@@ -274,16 +253,8 @@ int gcov_info_is_compatible(struct gcov_info *info1, struct gcov_info *info2)
> 		!list_is_last(&fn_ptr2->head, &info2->functions)) {
> 		if (fn_ptr1->checksum != fn_ptr2->checksum)
> 			return false;
>-#if CONFIG_CLANG_VERSION < 110000
>-		if (fn_ptr1->use_extra_checksum != fn_ptr2->use_extra_checksum)
>-			return false;
>-		if (fn_ptr1->use_extra_checksum &&
>-			fn_ptr1->cfg_checksum != fn_ptr2->cfg_checksum)
>-			return false;
>-#else
> 		if (fn_ptr1->cfg_checksum != fn_ptr2->cfg_checksum)
> 			return false;
>-#endif
> 		fn_ptr1 = list_next_entry(fn_ptr1, head);
> 		fn_ptr2 = list_next_entry(fn_ptr2, head);
> 	}
>@@ -403,21 +374,10 @@ size_t convert_to_gcda(char *buffer, struct gcov_info *info)
> 		u32 i;
>
> 		pos += store_gcov_u32(buffer, pos, GCOV_TAG_FUNCTION);
>-#if CONFIG_CLANG_VERSION < 110000
>-		pos += store_gcov_u32(buffer, pos,
>-			fi_ptr->use_extra_checksum ? 3 : 2);
>-#else
> 		pos += store_gcov_u32(buffer, pos, 3);
>-#endif
> 		pos += store_gcov_u32(buffer, pos, fi_ptr->ident);
> 		pos += store_gcov_u32(buffer, pos, fi_ptr->checksum);
>-#if CONFIG_CLANG_VERSION < 110000
>-		if (fi_ptr->use_extra_checksum)
>-			pos += store_gcov_u32(buffer, pos, fi_ptr->cfg_checksum);
>-#else
> 		pos += store_gcov_u32(buffer, pos, fi_ptr->cfg_checksum);
>-#endif
>-
> 		pos += store_gcov_u32(buffer, pos, GCOV_TAG_COUNTER_BASE);
> 		pos += store_gcov_u32(buffer, pos, fi_ptr->num_counters * 2);
> 		for (i = 0; i < fi_ptr->num_counters; i++)
>-- 
>2.31.1.295.g9ea45b61b8-goog
>

Looks good for both. Thanks!

Reviewed-by: Fangrui Song <maskray@google.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] gcov: re-fix clang-11+ support
  2021-04-07 18:54 ` [PATCH 1/2] gcov: re-fix clang-11+ support Nick Desaulniers
@ 2021-04-07 19:51   ` Nathan Chancellor
  2021-04-07 21:21   ` Andrew Morton
  1 sibling, 0 replies; 10+ messages in thread
From: Nathan Chancellor @ 2021-04-07 19:51 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Peter Oberparleiter, linux-kernel, clang-built-linux,
	Andrew Morton, Fangrui Song, Prasad Sodagudi, stable

On Wed, Apr 07, 2021 at 11:54:55AM -0700, Nick Desaulniers wrote:
> LLVM changed the expected function signature for
> llvm_gcda_emit_function() in the clang-11 release.  Users of clang-11 or
> newer may have noticed their kernels producing invalid coverage
> information:
> 
> $ llvm-cov gcov -a -c -u -f -b <input>.gcda -- gcno=<input>.gcno
> 1 <func>: checksum mismatch, \
>   (<lineno chksum A>, <cfg chksum B>) != (<lineno chksum A>, <cfg chksum C>)
> 2 Invalid .gcda File!
> ...
> 
> Fix up the function signatures so calling this function interprets its
> parameters correctly and computes the correct cfg checksum. In
> particular, in clang-11, the additional checksum is no longer optional.
> 
> Link: https://reviews.llvm.org/rG25544ce2df0daa4304c07e64b9c8b0f7df60c11d
> Cc: stable@vger.kernel.org #5.4+
> Reported-by: Prasad Sodagudi <psodagud@quicinc.com>
> Tested-by: Prasad Sodagudi <psodagud@quicinc.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  kernel/gcov/clang.c | 38 +++++++++++++++++++++++++++++++++-----
>  1 file changed, 33 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/gcov/clang.c b/kernel/gcov/clang.c
> index d41f5ecda9db..1747204541bf 100644
> --- a/kernel/gcov/clang.c
> +++ b/kernel/gcov/clang.c
> @@ -69,7 +69,9 @@ struct gcov_fn_info {
>  
>  	u32 ident;
>  	u32 checksum;
> +#if CONFIG_CLANG_VERSION < 110000
>  	u8 use_extra_checksum;
> +#endif
>  	u32 cfg_checksum;
>  
>  	u32 num_counters;
> @@ -111,6 +113,7 @@ void llvm_gcda_start_file(const char *orig_filename, u32 version, u32 checksum)
>  }
>  EXPORT_SYMBOL(llvm_gcda_start_file);
>  
> +#if CONFIG_CLANG_VERSION < 110000
>  void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
>  		u8 use_extra_checksum, u32 cfg_checksum)
>  {
> @@ -126,6 +129,21 @@ void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
>  	info->cfg_checksum = cfg_checksum;
>  	list_add_tail(&info->head, &current_info->functions);
>  }
> +#else
> +void llvm_gcda_emit_function(u32 ident, u32 func_checksum, u32 cfg_checksum)
> +{
> +	struct gcov_fn_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
> +
> +	if (!info)
> +		return;
> +
> +	INIT_LIST_HEAD(&info->head);
> +	info->ident = ident;
> +	info->checksum = func_checksum;
> +	info->cfg_checksum = cfg_checksum;
> +	list_add_tail(&info->head, &current_info->functions);
> +}
> +#endif
>  EXPORT_SYMBOL(llvm_gcda_emit_function);
>  
>  void llvm_gcda_emit_arcs(u32 num_counters, u64 *counters)
> @@ -256,11 +274,16 @@ int gcov_info_is_compatible(struct gcov_info *info1, struct gcov_info *info2)
>  		!list_is_last(&fn_ptr2->head, &info2->functions)) {
>  		if (fn_ptr1->checksum != fn_ptr2->checksum)
>  			return false;
> +#if CONFIG_CLANG_VERSION < 110000
>  		if (fn_ptr1->use_extra_checksum != fn_ptr2->use_extra_checksum)
>  			return false;
>  		if (fn_ptr1->use_extra_checksum &&
>  			fn_ptr1->cfg_checksum != fn_ptr2->cfg_checksum)
>  			return false;
> +#else
> +		if (fn_ptr1->cfg_checksum != fn_ptr2->cfg_checksum)
> +			return false;
> +#endif
>  		fn_ptr1 = list_next_entry(fn_ptr1, head);
>  		fn_ptr2 = list_next_entry(fn_ptr2, head);
>  	}
> @@ -378,17 +401,22 @@ size_t convert_to_gcda(char *buffer, struct gcov_info *info)
>  
>  	list_for_each_entry(fi_ptr, &info->functions, head) {
>  		u32 i;
> -		u32 len = 2;
> -
> -		if (fi_ptr->use_extra_checksum)
> -			len++;
>  
>  		pos += store_gcov_u32(buffer, pos, GCOV_TAG_FUNCTION);
> -		pos += store_gcov_u32(buffer, pos, len);
> +#if CONFIG_CLANG_VERSION < 110000
> +		pos += store_gcov_u32(buffer, pos,
> +			fi_ptr->use_extra_checksum ? 3 : 2);
> +#else
> +		pos += store_gcov_u32(buffer, pos, 3);
> +#endif
>  		pos += store_gcov_u32(buffer, pos, fi_ptr->ident);
>  		pos += store_gcov_u32(buffer, pos, fi_ptr->checksum);
> +#if CONFIG_CLANG_VERSION < 110000
>  		if (fi_ptr->use_extra_checksum)
>  			pos += store_gcov_u32(buffer, pos, fi_ptr->cfg_checksum);
> +#else
> +		pos += store_gcov_u32(buffer, pos, fi_ptr->cfg_checksum);
> +#endif
>  
>  		pos += store_gcov_u32(buffer, pos, GCOV_TAG_COUNTER_BASE);
>  		pos += store_gcov_u32(buffer, pos, fi_ptr->num_counters * 2);
> -- 
> 2.31.1.295.g9ea45b61b8-goog
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] gcov: re-drop support for clang-10
  2021-04-07 18:54 ` [PATCH 2/2] gcov: re-drop support for clang-10 Nick Desaulniers
  2021-04-07 19:07   ` Fangrui Song
@ 2021-04-07 19:51   ` Nathan Chancellor
  1 sibling, 0 replies; 10+ messages in thread
From: Nathan Chancellor @ 2021-04-07 19:51 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Peter Oberparleiter, linux-kernel, clang-built-linux,
	Andrew Morton, Fangrui Song, Prasad Sodagudi

On Wed, Apr 07, 2021 at 11:54:56AM -0700, Nick Desaulniers wrote:
> LLVM changed the expected function signatures for
> llvm_gcda_emit_function() in the clang-11 release.  Drop the older
> implementations and require folks to upgrade their compiler if they're
> interested in GCOV support.
> 
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  kernel/gcov/clang.c | 40 ----------------------------------------
>  1 file changed, 40 deletions(-)
> 
> diff --git a/kernel/gcov/clang.c b/kernel/gcov/clang.c
> index 1747204541bf..78c4dc751080 100644
> --- a/kernel/gcov/clang.c
> +++ b/kernel/gcov/clang.c
> @@ -69,9 +69,6 @@ struct gcov_fn_info {
>  
>  	u32 ident;
>  	u32 checksum;
> -#if CONFIG_CLANG_VERSION < 110000
> -	u8 use_extra_checksum;
> -#endif
>  	u32 cfg_checksum;
>  
>  	u32 num_counters;
> @@ -113,23 +110,6 @@ void llvm_gcda_start_file(const char *orig_filename, u32 version, u32 checksum)
>  }
>  EXPORT_SYMBOL(llvm_gcda_start_file);
>  
> -#if CONFIG_CLANG_VERSION < 110000
> -void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
> -		u8 use_extra_checksum, u32 cfg_checksum)
> -{
> -	struct gcov_fn_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
> -
> -	if (!info)
> -		return;
> -
> -	INIT_LIST_HEAD(&info->head);
> -	info->ident = ident;
> -	info->checksum = func_checksum;
> -	info->use_extra_checksum = use_extra_checksum;
> -	info->cfg_checksum = cfg_checksum;
> -	list_add_tail(&info->head, &current_info->functions);
> -}
> -#else
>  void llvm_gcda_emit_function(u32 ident, u32 func_checksum, u32 cfg_checksum)
>  {
>  	struct gcov_fn_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
> @@ -143,7 +123,6 @@ void llvm_gcda_emit_function(u32 ident, u32 func_checksum, u32 cfg_checksum)
>  	info->cfg_checksum = cfg_checksum;
>  	list_add_tail(&info->head, &current_info->functions);
>  }
> -#endif
>  EXPORT_SYMBOL(llvm_gcda_emit_function);
>  
>  void llvm_gcda_emit_arcs(u32 num_counters, u64 *counters)
> @@ -274,16 +253,8 @@ int gcov_info_is_compatible(struct gcov_info *info1, struct gcov_info *info2)
>  		!list_is_last(&fn_ptr2->head, &info2->functions)) {
>  		if (fn_ptr1->checksum != fn_ptr2->checksum)
>  			return false;
> -#if CONFIG_CLANG_VERSION < 110000
> -		if (fn_ptr1->use_extra_checksum != fn_ptr2->use_extra_checksum)
> -			return false;
> -		if (fn_ptr1->use_extra_checksum &&
> -			fn_ptr1->cfg_checksum != fn_ptr2->cfg_checksum)
> -			return false;
> -#else
>  		if (fn_ptr1->cfg_checksum != fn_ptr2->cfg_checksum)
>  			return false;
> -#endif
>  		fn_ptr1 = list_next_entry(fn_ptr1, head);
>  		fn_ptr2 = list_next_entry(fn_ptr2, head);
>  	}
> @@ -403,21 +374,10 @@ size_t convert_to_gcda(char *buffer, struct gcov_info *info)
>  		u32 i;
>  
>  		pos += store_gcov_u32(buffer, pos, GCOV_TAG_FUNCTION);
> -#if CONFIG_CLANG_VERSION < 110000
> -		pos += store_gcov_u32(buffer, pos,
> -			fi_ptr->use_extra_checksum ? 3 : 2);
> -#else
>  		pos += store_gcov_u32(buffer, pos, 3);
> -#endif
>  		pos += store_gcov_u32(buffer, pos, fi_ptr->ident);
>  		pos += store_gcov_u32(buffer, pos, fi_ptr->checksum);
> -#if CONFIG_CLANG_VERSION < 110000
> -		if (fi_ptr->use_extra_checksum)
> -			pos += store_gcov_u32(buffer, pos, fi_ptr->cfg_checksum);
> -#else
>  		pos += store_gcov_u32(buffer, pos, fi_ptr->cfg_checksum);
> -#endif
> -
>  		pos += store_gcov_u32(buffer, pos, GCOV_TAG_COUNTER_BASE);
>  		pos += store_gcov_u32(buffer, pos, fi_ptr->num_counters * 2);
>  		for (i = 0; i < fi_ptr->num_counters; i++)
> -- 
> 2.31.1.295.g9ea45b61b8-goog
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] gcov: re-fix clang-11+ support
  2021-04-07 18:54 ` [PATCH 1/2] gcov: re-fix clang-11+ support Nick Desaulniers
  2021-04-07 19:51   ` Nathan Chancellor
@ 2021-04-07 21:21   ` Andrew Morton
  2021-04-07 21:28     ` Nick Desaulniers
  1 sibling, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2021-04-07 21:21 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Peter Oberparleiter, Nathan Chancellor, linux-kernel,
	clang-built-linux, Fangrui Song, Prasad Sodagudi, stable

On Wed,  7 Apr 2021 11:54:55 -0700 Nick Desaulniers <ndesaulniers@google.com> wrote:

> LLVM changed the expected function signature for
> llvm_gcda_emit_function() in the clang-11 release.  Users of clang-11 or
> newer may have noticed their kernels producing invalid coverage
> information:
> 
> $ llvm-cov gcov -a -c -u -f -b <input>.gcda -- gcno=<input>.gcno
> 1 <func>: checksum mismatch, \
>   (<lineno chksum A>, <cfg chksum B>) != (<lineno chksum A>, <cfg chksum C>)
> 2 Invalid .gcda File!
> ...
> 
> Fix up the function signatures so calling this function interprets its
> parameters correctly and computes the correct cfg checksum. In
> particular, in clang-11, the additional checksum is no longer optional.

Which tree is this against?  I'm seeing quite a lot of rejects against
Linus's current.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] gcov: re-fix clang-11+ support
  2021-04-07 21:21   ` Andrew Morton
@ 2021-04-07 21:28     ` Nick Desaulniers
  2021-04-07 22:26       ` Andrew Morton
  0 siblings, 1 reply; 10+ messages in thread
From: Nick Desaulniers @ 2021-04-07 21:28 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Peter Oberparleiter, Nathan Chancellor, LKML, clang-built-linux,
	Fangrui Song, Prasad Sodagudi, # 3.4.x

On Wed, Apr 7, 2021 at 2:21 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Wed,  7 Apr 2021 11:54:55 -0700 Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> > LLVM changed the expected function signature for
> > llvm_gcda_emit_function() in the clang-11 release.  Users of clang-11 or
> > newer may have noticed their kernels producing invalid coverage
> > information:
> >
> > $ llvm-cov gcov -a -c -u -f -b <input>.gcda -- gcno=<input>.gcno
> > 1 <func>: checksum mismatch, \
> >   (<lineno chksum A>, <cfg chksum B>) != (<lineno chksum A>, <cfg chksum C>)
> > 2 Invalid .gcda File!
> > ...
> >
> > Fix up the function signatures so calling this function interprets its
> > parameters correctly and computes the correct cfg checksum. In
> > particular, in clang-11, the additional checksum is no longer optional.
>
> Which tree is this against?  I'm seeing quite a lot of rejects against
> Linus's current.

Today's linux-next; the only recent changes to this single source file
since my last patches were:

commit b3c4e66c908b ("gcov: combine common code")
commit 17d0508a080d ("gcov: use kvmalloc()")

both have your sign off, so I assume those are in your tree?

-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] gcov: re-fix clang-11+ support
  2021-04-07 21:28     ` Nick Desaulniers
@ 2021-04-07 22:26       ` Andrew Morton
  2021-04-08 18:46         ` [PATCH v2] " Nick Desaulniers
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2021-04-07 22:26 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Peter Oberparleiter, Nathan Chancellor, LKML, clang-built-linux,
	Fangrui Song, Prasad Sodagudi, # 3.4.x

On Wed, 7 Apr 2021 14:28:21 -0700 Nick Desaulniers <ndesaulniers@google.com> wrote:

> On Wed, Apr 7, 2021 at 2:21 PM Andrew Morton <akpm@linux-foundation.org> wrote:
> >
> > On Wed,  7 Apr 2021 11:54:55 -0700 Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > > LLVM changed the expected function signature for
> > > llvm_gcda_emit_function() in the clang-11 release.  Users of clang-11 or
> > > newer may have noticed their kernels producing invalid coverage
> > > information:
> > >
> > > $ llvm-cov gcov -a -c -u -f -b <input>.gcda -- gcno=<input>.gcno
> > > 1 <func>: checksum mismatch, \
> > >   (<lineno chksum A>, <cfg chksum B>) != (<lineno chksum A>, <cfg chksum C>)
> > > 2 Invalid .gcda File!
> > > ...
> > >
> > > Fix up the function signatures so calling this function interprets its
> > > parameters correctly and computes the correct cfg checksum. In
> > > particular, in clang-11, the additional checksum is no longer optional.
> >
> > Which tree is this against?  I'm seeing quite a lot of rejects against
> > Linus's current.
> 
> Today's linux-next; the only recent changes to this single source file
> since my last patches were:
> 
> commit b3c4e66c908b ("gcov: combine common code")
> commit 17d0508a080d ("gcov: use kvmalloc()")
> 
> both have your sign off, so I assume those are in your tree?

Yes, I presently have

gcov-clang-drop-support-for-clang-10-and-older.patch
gcov-combine-common-code.patch
gcov-simplify-buffer-allocation.patch
gcov-use-kvmalloc.patch

But this patch ("gcov: re-fix clang-11+ support") has cc:stable, so it
should be against Linus's tree, to give the -stable trees something
more mergeable.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] gcov: re-fix clang-11+ support
  2021-04-07 22:26       ` Andrew Morton
@ 2021-04-08 18:46         ` Nick Desaulniers
  0 siblings, 0 replies; 10+ messages in thread
From: Nick Desaulniers @ 2021-04-08 18:46 UTC (permalink / raw)
  To: akpm
  Cc: clang-built-linux, linux-kernel, maskray, nathan, ndesaulniers,
	oberpar, psodagud, stable

LLVM changed the expected function signature for
llvm_gcda_emit_function() in the clang-11 release.  Users of clang-11 or
newer may have noticed their kernels producing invalid coverage
information:

$ llvm-cov gcov -a -c -u -f -b <input>.gcda -- gcno=<input>.gcno
1 <func>: checksum mismatch, \
  (<lineno chksum A>, <cfg chksum B>) != (<lineno chksum A>, <cfg chksum C>)
2 Invalid .gcda File!
...

Fix up the function signatures so calling this function interprets its
parameters correctly and computes the correct cfg checksum. In
particular, in clang-11, the additional checksum is no longer optional.

Link: https://reviews.llvm.org/rG25544ce2df0daa4304c07e64b9c8b0f7df60c11d
Cc: stable@vger.kernel.org #5.4+
Reported-by: Prasad Sodagudi <psodagud@quicinc.com>
Tested-by: Prasad Sodagudi <psodagud@quicinc.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
---
Changes V1 -> V2:
* Carried Nathan's reviewed-by tag.
* Rebased on mainline, as per Andrew.
* Left off patch 2/2 from the series
https://lore.kernel.org/lkml/20210407185456.41943-1-ndesaulniers@google.com/
  I assume that dropping support for clang-10+GCOV will be held
  separately for -next for 5.13, while this will be sent for 5.12?

 kernel/gcov/clang.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/kernel/gcov/clang.c b/kernel/gcov/clang.c
index 8743150db2ac..c466c7fbdece 100644
--- a/kernel/gcov/clang.c
+++ b/kernel/gcov/clang.c
@@ -70,7 +70,9 @@ struct gcov_fn_info {
 
 	u32 ident;
 	u32 checksum;
+#if CONFIG_CLANG_VERSION < 110000
 	u8 use_extra_checksum;
+#endif
 	u32 cfg_checksum;
 
 	u32 num_counters;
@@ -145,10 +147,8 @@ void llvm_gcda_emit_function(u32 ident, const char *function_name,
 
 	list_add_tail(&info->head, &current_info->functions);
 }
-EXPORT_SYMBOL(llvm_gcda_emit_function);
 #else
-void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
-		u8 use_extra_checksum, u32 cfg_checksum)
+void llvm_gcda_emit_function(u32 ident, u32 func_checksum, u32 cfg_checksum)
 {
 	struct gcov_fn_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
 
@@ -158,12 +158,11 @@ void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
 	INIT_LIST_HEAD(&info->head);
 	info->ident = ident;
 	info->checksum = func_checksum;
-	info->use_extra_checksum = use_extra_checksum;
 	info->cfg_checksum = cfg_checksum;
 	list_add_tail(&info->head, &current_info->functions);
 }
-EXPORT_SYMBOL(llvm_gcda_emit_function);
 #endif
+EXPORT_SYMBOL(llvm_gcda_emit_function);
 
 void llvm_gcda_emit_arcs(u32 num_counters, u64 *counters)
 {
@@ -293,11 +292,16 @@ int gcov_info_is_compatible(struct gcov_info *info1, struct gcov_info *info2)
 		!list_is_last(&fn_ptr2->head, &info2->functions)) {
 		if (fn_ptr1->checksum != fn_ptr2->checksum)
 			return false;
+#if CONFIG_CLANG_VERSION < 110000
 		if (fn_ptr1->use_extra_checksum != fn_ptr2->use_extra_checksum)
 			return false;
 		if (fn_ptr1->use_extra_checksum &&
 			fn_ptr1->cfg_checksum != fn_ptr2->cfg_checksum)
 			return false;
+#else
+		if (fn_ptr1->cfg_checksum != fn_ptr2->cfg_checksum)
+			return false;
+#endif
 		fn_ptr1 = list_next_entry(fn_ptr1, head);
 		fn_ptr2 = list_next_entry(fn_ptr2, head);
 	}
@@ -529,17 +533,22 @@ static size_t convert_to_gcda(char *buffer, struct gcov_info *info)
 
 	list_for_each_entry(fi_ptr, &info->functions, head) {
 		u32 i;
-		u32 len = 2;
-
-		if (fi_ptr->use_extra_checksum)
-			len++;
 
 		pos += store_gcov_u32(buffer, pos, GCOV_TAG_FUNCTION);
-		pos += store_gcov_u32(buffer, pos, len);
+#if CONFIG_CLANG_VERSION < 110000
+		pos += store_gcov_u32(buffer, pos,
+			fi_ptr->use_extra_checksum ? 3 : 2);
+#else
+		pos += store_gcov_u32(buffer, pos, 3);
+#endif
 		pos += store_gcov_u32(buffer, pos, fi_ptr->ident);
 		pos += store_gcov_u32(buffer, pos, fi_ptr->checksum);
+#if CONFIG_CLANG_VERSION < 110000
 		if (fi_ptr->use_extra_checksum)
 			pos += store_gcov_u32(buffer, pos, fi_ptr->cfg_checksum);
+#else
+		pos += store_gcov_u32(buffer, pos, fi_ptr->cfg_checksum);
+#endif
 
 		pos += store_gcov_u32(buffer, pos, GCOV_TAG_COUNTER_BASE);
 		pos += store_gcov_u32(buffer, pos, fi_ptr->num_counters * 2);

base-commit: 3fb4f979b4fa1f92a02b538ae86e725b73e703d0
-- 
2.31.1.295.g9ea45b61b8-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-04-08 18:46 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-07 18:54 [PATCH 0/2] gcov: re-fix clang-11 support Nick Desaulniers
2021-04-07 18:54 ` [PATCH 1/2] gcov: re-fix clang-11+ support Nick Desaulniers
2021-04-07 19:51   ` Nathan Chancellor
2021-04-07 21:21   ` Andrew Morton
2021-04-07 21:28     ` Nick Desaulniers
2021-04-07 22:26       ` Andrew Morton
2021-04-08 18:46         ` [PATCH v2] " Nick Desaulniers
2021-04-07 18:54 ` [PATCH 2/2] gcov: re-drop support for clang-10 Nick Desaulniers
2021-04-07 19:07   ` Fangrui Song
2021-04-07 19:51   ` Nathan Chancellor

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.