Re: [PATCH] gcov: fix TOPN streaming from shared libraries

[Sergei Trofimovich <slyfox@gentoo.org>]

[-- Attachment #1: Type: text/plain, Size: 3255 bytes --]

On Mon, 21 Sep 2020 20:38:07 +0300 (MSK)
Alexander Monakov <amonakov@ispras.ru> wrote:

> On Mon, 21 Sep 2020, Martin Liška wrote:
> 
> > On 9/6/20 1:24 PM, Sergei Trofimovich wrote:  
> > > From: Sergei Trofimovich <siarheit@google.com>
> > > 
> > > Before the change gcc did not stream correctly TOPN counters
> > > if counters belonged to a non-local shared object.
> > > 
> > > As a result zero-section optimization generated TOPN sections
> > > in a form not recognizable by '__gcov_merge_topn'.
> > > 
> > > The problem happens because in a case of multiple shared objects
> > > '__gcov_merge_topn' function is present in address space multiple
> > > times (once per each object).
> > > 
> > > The fix is to never rely on function address and predicate on TOPN
> > > counter types.  
> > 
> > Hello.
> > 
> > Thank you for the analysis! I think it's the correct fix and it's probably
> > similar to what we used to see for indirect_call_tuple.
> > 
> > @Alexander: Am I right?  
> 
> Yes, analysis presented by Sergei in Bugzilla looks correct. Pedantically I
> wouldn't say the indirect call issue was similar: it's a different gotcha
> arising from mixing static and dynamic linking. There we had some symbols
> preempted by the main executable (but not all symbols), here we have lack
> of preemption/unification as relevant libgcov symbol is hidden.
> 
> I cannot judge if the fix is correct (don't know the code that well) but it
> looks reasonable. If you could come up with a clearer wording for the new
> comment it would be nice, I struggled to understand it.

Yeah, I agree the comment is very misleading. The code is already very clear
about special casing of TOPN counters. How about dropping the comment?

v2:

From 300585164f0a719a3a283c8da3a4061615f6da3a Mon Sep 17 00:00:00 2001
From: Sergei Trofimovich <siarheit@google.com>
Date: Sun, 6 Sep 2020 12:13:54 +0100
Subject: [PATCH v2] gcov: fix TOPN streaming from shared libraries

Before the change gcc did not stream correctly TOPN counters
if counters belonged to a non-local shared object.

As a result zero-section optimization generated TOPN sections
in a form not recognizable by '__gcov_merge_topn'.

The problem happens because in a case of multiple shared objects
'__gcov_merge_topn' function is present in address space multiple
times (once per each object).

The fix is to never rely on function address and predicate on TOPN
counter types.

libgcc/ChangeLog:

        PR gcov-profile/96913
        * libgcov-driver.c (write_one_data): Avoid function pointer
        comparison in TOP streaming decision.
---
 libgcc/libgcov-driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c
index 58914268d4e..e53e4dc392a 100644
--- a/libgcc/libgcov-driver.c
+++ b/libgcc/libgcov-driver.c
@@ -424,7 +424,7 @@ write_one_data (const struct gcov_info *gi_ptr,

          n_counts = ci_ptr->num;

-         if (gi_ptr->merge[t_ix] == __gcov_merge_topn)
+         if (t_ix == GCOV_COUNTER_V_TOPN || t_ix == GCOV_COUNTER_V_INDIR)
            write_top_counters (ci_ptr, t_ix, n_counts);
          else
            {
-- 

  Sergei

[-- Attachment #2: v2-0001-gcov-fix-TOPN-streaming-from-shared-libraries.patch --]
[-- Type: text/x-patch, Size: 1381 bytes --]

From 300585164f0a719a3a283c8da3a4061615f6da3a Mon Sep 17 00:00:00 2001
From: Sergei Trofimovich <siarheit@google.com>
Date: Sun, 6 Sep 2020 12:13:54 +0100
Subject: [PATCH v2] gcov: fix TOPN streaming from shared libraries

Before the change gcc did not stream correctly TOPN counters
if counters belonged to a non-local shared object.

As a result zero-section optimization generated TOPN sections
in a form not recognizable by '__gcov_merge_topn'.

The problem happens because in a case of multiple shared objects
'__gcov_merge_topn' function is present in address space multiple
times (once per each object).

The fix is to never rely on function address and predicate on TOPN
counter types.

libgcc/ChangeLog:

	PR gcov-profile/96913
	* libgcov-driver.c (write_one_data): Avoid function pointer
	comparison in TOP streaming decision.
---
 libgcc/libgcov-driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c
index 58914268d4e..e53e4dc392a 100644
--- a/libgcc/libgcov-driver.c
+++ b/libgcc/libgcov-driver.c
@@ -424,7 +424,7 @@ write_one_data (const struct gcov_info *gi_ptr,
 
 	  n_counts = ci_ptr->num;
 
-	  if (gi_ptr->merge[t_ix] == __gcov_merge_topn)
+	  if (t_ix == GCOV_COUNTER_V_TOPN || t_ix == GCOV_COUNTER_V_INDIR)
 	    write_top_counters (ci_ptr, t_ix, n_counts);
 	  else
 	    {
-- 
2.28.0