On 9/22/20 1:13 AM, Sergei Trofimovich wrote:
On Mon, 21 Sep 2020 20:38:07 +0300 (MSK)
Alexander Monakov <amona...@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 <siarh...@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.

Thank you Alexander.


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 <siarh...@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
             {


The fix is fine, please install it.

Martin

Reply via email to