[PATCH] gcov: fix TOPN streaming from shared libraries

[Martin Liška]

On 9/22/20 1:13 AM, Sergei Trofimovich wrote:
> 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.

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 <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
>              {
> 

The fix is fine, please install it.

Martin