On 2/17/21 9:36 AM, Martin Liška wrote:
I'll write it even more robust...
This is more elegant approach I've just tested on the instrumented clang.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Thanks,
Martin
>From a8a879ba8a8ce3e90a712ff4c436b5d993faad7d Mon Sep 17 00:00:00 2001
From: Martin Liska <mli...@suse.cz>
Date: Tue, 16 Feb 2021 16:28:06 +0100
Subject: [PATCH] profiling: fix streaming of TOPN counters
libgcc/ChangeLog:
PR gcov-profile/99105
* libgcov-driver.c (write_top_counters): Rename to ...
(write_topn_counters): ... this.
(write_one_data): Pre-allocate buffer for number of items
in the corresponding linked lists.
* libgcov-merge.c (__gcov_merge_topn): Use renamed function.
gcc/testsuite/ChangeLog:
PR gcov-profile/99105
* gcc.dg/tree-prof/indir-call-prof-malloc.c: Use profile
correction as the wrapped malloc is called one more time
from libgcov.
---
.../gcc.dg/tree-prof/indir-call-prof-malloc.c | 2 +-
libgcc/libgcov-driver.c | 37 +++++++++++++++----
libgcc/libgcov-merge.c | 1 +
3 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/gcc/testsuite/gcc.dg/tree-prof/indir-call-prof-malloc.c b/gcc/testsuite/gcc.dg/tree-prof/indir-call-prof-malloc.c
index 454e224c95f..7bda4aedfc8 100644
--- a/gcc/testsuite/gcc.dg/tree-prof/indir-call-prof-malloc.c
+++ b/gcc/testsuite/gcc.dg/tree-prof/indir-call-prof-malloc.c
@@ -1,4 +1,4 @@
-/* { dg-options "-O2 -ldl" } */
+/* { dg-options "-O2 -ldl -fprofile-correction" } */
#define _GNU_SOURCE
#include <stdio.h>
diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c
index e474e032b54..e6efa97254e 100644
--- a/libgcc/libgcov-driver.c
+++ b/libgcc/libgcov-driver.c
@@ -337,32 +337,53 @@ read_error:
/* Store all TOP N counters where each has a dynamic length. */
static void
-write_top_counters (const struct gcov_ctr_info *ci_ptr,
- unsigned t_ix,
- gcov_unsigned_t n_counts)
+write_topn_counters (const struct gcov_ctr_info *ci_ptr,
+ unsigned t_ix,
+ gcov_unsigned_t n_counts)
{
unsigned counters = n_counts / GCOV_TOPN_MEM_COUNTERS;
gcc_assert (n_counts % GCOV_TOPN_MEM_COUNTERS == 0);
+
+ /* It can happen in a multi-threaded environment that number of counters is
+ different from the size of the corresponding linked lists. */
+ unsigned *list_sizes = (unsigned *) xcalloc (counters, sizeof (unsigned));
unsigned pair_total = 0;
+
for (unsigned i = 0; i < counters; i++)
- pair_total += ci_ptr->values[GCOV_TOPN_MEM_COUNTERS * i + 1];
+ {
+ gcov_type start = ci_ptr->values[GCOV_TOPN_MEM_COUNTERS * i + 2];
+ for (struct gcov_kvp *node = (struct gcov_kvp *)(intptr_t)start;
+ node != NULL; node = node->next)
+ {
+ ++pair_total;
+ ++list_sizes[i];
+ }
+ }
+
unsigned disk_size = GCOV_TOPN_DISK_COUNTERS * counters + 2 * pair_total;
gcov_write_tag_length (GCOV_TAG_FOR_COUNTER (t_ix),
GCOV_TAG_COUNTER_LENGTH (disk_size));
for (unsigned i = 0; i < counters; i++)
{
- gcov_type pair_count = ci_ptr->values[GCOV_TOPN_MEM_COUNTERS * i + 1];
gcov_write_counter (ci_ptr->values[GCOV_TOPN_MEM_COUNTERS * i]);
- gcov_write_counter (pair_count);
+ gcov_write_counter (list_sizes[i]);
gcov_type start = ci_ptr->values[GCOV_TOPN_MEM_COUNTERS * i + 2];
+
+ unsigned j = 0;
for (struct gcov_kvp *node = (struct gcov_kvp *)(intptr_t)start;
- node != NULL; node = node->next)
+ node != NULL; node = node->next, j++)
{
gcov_write_counter (node->value);
gcov_write_counter (node->count);
+
+ /* Stop when we reach expected number of items. */
+ if (j + 1 == list_sizes[i])
+ break;
}
}
+
+ free (list_sizes);
}
/* Write counters in GI_PTR and the summary in PRG to a gcda file. In
@@ -425,7 +446,7 @@ write_one_data (const struct gcov_info *gi_ptr,
n_counts = ci_ptr->num;
if (t_ix == GCOV_COUNTER_V_TOPN || t_ix == GCOV_COUNTER_V_INDIR)
- write_top_counters (ci_ptr, t_ix, n_counts);
+ write_topn_counters (ci_ptr, t_ix, n_counts);
else
{
/* Do not stream when all counters are zero. */
diff --git a/libgcc/libgcov-merge.c b/libgcc/libgcov-merge.c
index 7db188a4f4c..3379b688128 100644
--- a/libgcc/libgcov-merge.c
+++ b/libgcc/libgcov-merge.c
@@ -109,6 +109,7 @@ __gcov_merge_topn (gcov_type *counters, unsigned n_counters)
/* First value is number of total executions of the profiler. */
gcov_type all = gcov_get_counter_ignore_scaling (-1);
gcov_type n = gcov_get_counter_ignore_scaling (-1);
+ gcc_assert (n <= GCOV_TOPN_MAXIMUM_TRACKED_VALUES);
unsigned full = all < 0;
gcov_type *total = &counters[GCOV_TOPN_MEM_COUNTERS * i];
--
2.30.0
