[PATCH] gcov-profile: Fix -fcompare-debug with -fprofile-generate [PR100520]

[PATCH] gcov-profile: Fix -fcompare-debug with -fprofile-generate [PR100520]

[Martin Liška]

Hello.

This strips .gk from aux_base_name in coverage.c.
Do you like the implementation of endswith, or do we have the functionality somewhere?

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

	PR gcov-profile/100520

gcc/ChangeLog:

	* coverage.c (coverage_compute_profile_id): Strip .gk when
	compare debug is used.
	* system.h (endswith): New function.

gcc/testsuite/ChangeLog:

	* gcc.dg/pr100520.c: New test.
---
  gcc/coverage.c                  |  7 +++++--
  gcc/system.h                    | 13 +++++++++++++
  gcc/testsuite/gcc.dg/pr100520.c |  5 +++++
  3 files changed, 23 insertions(+), 2 deletions(-)
  create mode 100644 gcc/testsuite/gcc.dg/pr100520.c

diff --git a/gcc/coverage.c b/gcc/coverage.c
index 4467f1eaa5c..4daa3f9fc30 100644
--- a/gcc/coverage.c
+++ b/gcc/coverage.c
@@ -571,8 +571,11 @@ coverage_compute_profile_id (struct cgraph_node *n)
        if (!use_name_only && first_global_object_name)
  	chksum = coverage_checksum_string
  	  (chksum, first_global_object_name);
-      chksum = coverage_checksum_string
-	(chksum, aux_base_name);
+      char *base_name = xstrdup (aux_base_name);
+      if (endswith (base_name, ".gk"))
+	base_name[strlen (base_name) - 3] = '\0';
+      chksum = coverage_checksum_string (chksum, base_name);
+      free (base_name);
      }
  
    /* Non-negative integers are hopefully small enough to fit in all targets.
diff --git a/gcc/system.h b/gcc/system.h
index adde3e264b6..4ac656c9c3c 100644
--- a/gcc/system.h
+++ b/gcc/system.h
@@ -1305,4 +1305,17 @@ startswith (const char *str, const char *prefix)
    return strncmp (str, prefix, strlen (prefix)) == 0;
  }
  
+/* Return true if STR string ends with SUFFIX.  */
+
+static inline bool
+endswith (const char *str, const char *suffix)
+{
+  size_t str_len = strlen (str);
+  size_t suffix_len = strlen (suffix);
+  if (str_len < suffix_len)
+    return false;
+
+  return memcmp (str + str_len - suffix_len, suffix, suffix_len) == 0;
+}
+
  #endif /* ! GCC_SYSTEM_H */
diff --git a/gcc/testsuite/gcc.dg/pr100520.c b/gcc/testsuite/gcc.dg/pr100520.c
new file mode 100644
index 00000000000..60f79c2b888
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr100520.c
@@ -0,0 +1,5 @@
+/* PR gcov-profile/100520 */
+/* { dg-do compile } */
+/* { dg-options "-fcompare-debug -fprofile-generate" } */
+
+static int f() {}
-- 
2.33.1

Re: [PATCH] gcov-profile: Fix -fcompare-debug with -fprofile-generate [PR100520]

[Jan Hubicka]

> Hello.
> 
> This strips .gk from aux_base_name in coverage.c.
> Do you like the implementation of endswith, or do we have the functionality somewhere?
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Ready to be installed?
> Thanks,
> Martin
> 
> 	PR gcov-profile/100520
> 
> gcc/ChangeLog:
> 
> 	* coverage.c (coverage_compute_profile_id): Strip .gk when
> 	compare debug is used.
> 	* system.h (endswith): New function.

Droping .kg in coverage.c seems OK, but having endswith included into
every gcc source looks like bit of overkill given that is can be open
coded in 3 statements?

Honza
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/pr100520.c: New test.
> ---
>  gcc/coverage.c                  |  7 +++++--
>  gcc/system.h                    | 13 +++++++++++++
>  gcc/testsuite/gcc.dg/pr100520.c |  5 +++++
>  3 files changed, 23 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/pr100520.c
> 
> diff --git a/gcc/coverage.c b/gcc/coverage.c
> index 4467f1eaa5c..4daa3f9fc30 100644
> --- a/gcc/coverage.c
> +++ b/gcc/coverage.c
> @@ -571,8 +571,11 @@ coverage_compute_profile_id (struct cgraph_node *n)
>        if (!use_name_only && first_global_object_name)
>  	chksum = coverage_checksum_string
>  	  (chksum, first_global_object_name);
> -      chksum = coverage_checksum_string
> -	(chksum, aux_base_name);
> +      char *base_name = xstrdup (aux_base_name);
> +      if (endswith (base_name, ".gk"))
> +	base_name[strlen (base_name) - 3] = '\0';
> +      chksum = coverage_checksum_string (chksum, base_name);
> +      free (base_name);
>      }
>    /* Non-negative integers are hopefully small enough to fit in all targets.
> diff --git a/gcc/system.h b/gcc/system.h
> index adde3e264b6..4ac656c9c3c 100644
> --- a/gcc/system.h
> +++ b/gcc/system.h
> @@ -1305,4 +1305,17 @@ startswith (const char *str, const char *prefix)
>    return strncmp (str, prefix, strlen (prefix)) == 0;
>  }
> +/* Return true if STR string ends with SUFFIX.  */
> +
> +static inline bool
> +endswith (const char *str, const char *suffix)
> +{
> +  size_t str_len = strlen (str);
> +  size_t suffix_len = strlen (suffix);
> +  if (str_len < suffix_len)
> +    return false;
> +
> +  return memcmp (str + str_len - suffix_len, suffix, suffix_len) == 0;
> +}
> +
>  #endif /* ! GCC_SYSTEM_H */
> diff --git a/gcc/testsuite/gcc.dg/pr100520.c b/gcc/testsuite/gcc.dg/pr100520.c
> new file mode 100644
> index 00000000000..60f79c2b888
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr100520.c
> @@ -0,0 +1,5 @@
> +/* PR gcov-profile/100520 */
> +/* { dg-do compile } */
> +/* { dg-options "-fcompare-debug -fprofile-generate" } */
> +
> +static int f() {}
> -- 
> 2.33.1
> 

Re: [PATCH] gcov-profile: Fix -fcompare-debug with -fprofile-generate [PR100520]

[Martin Liška]

On 11/5/21 18:30, Jan Hubicka wrote:
> every gcc source looks like bit of overkill given that is can be open
> coded in 3 statements?

Why? It's a static inline function with few statements. I don't want to copy&paste
the same code at every location. I bet there must quite some open-coded implementations
of endswith in the GCC source code.

Martin

Re: [PATCH] gcov-profile: Fix -fcompare-debug with -fprofile-generate [PR100520]

[Jan Hubicka]

> On 11/5/21 18:30, Jan Hubicka wrote:
> > every gcc source looks like bit of overkill given that is can be open
> > coded in 3 statements?
> 
> Why? It's a static inline function with few statements. I don't want to copy&paste
> the same code at every location. I bet there must quite some open-coded implementations
> of endswith in the GCC source code.

I guess it is a matter of taste, but to me system.h should not be
universal include bringing a lot of unrelated things because in long
term it is how precompiled headers came to be.  In theory such random
generally useful things probably would belong to libiberty, but that
also seems but of overkill to me.

Anyway I guess we need someone with approval right to system.h to OK
that. I see there is already startswith so having endwith is probably
not too bad.

Honza
> 
> Martin

Re: [PATCH] gcov-profile: Fix -fcompare-debug with -fprofile-generate [PR100520]

[Richard Biener]

On Mon, Nov 8, 2021 at 12:01 PM Jan Hubicka via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> > On 11/5/21 18:30, Jan Hubicka wrote:
> > > every gcc source looks like bit of overkill given that is can be open
> > > coded in 3 statements?
> >
> > Why? It's a static inline function with few statements. I don't want to copy&paste
> > the same code at every location. I bet there must quite some open-coded implementations
> > of endswith in the GCC source code.
>
> I guess it is a matter of taste, but to me system.h should not be
> universal include bringing a lot of unrelated things because in long
> term it is how precompiled headers came to be.  In theory such random
> generally useful things probably would belong to libiberty, but that
> also seems but of overkill to me.
>
> Anyway I guess we need someone with approval right to system.h to OK
> that. I see there is already startswith so having endwith is probably
> not too bad.

OK.

Richard.

> Honza
> >
> > Martin