diff mbox series

gcov: fix gcov-tool merge for TOPN counters

Message ID 4b5b8222-c5d5-85cf-209e-4fb6b98cf10b@suse.cz
State New
Headers show
Series gcov: fix gcov-tool merge for TOPN counters | expand

Commit Message

Martin Liška June 16, 2020, 12:51 p.m. UTC 
Hello.

The patch is about corrupted gcov-tool merge command for a TOPN counter.
What was missing is transition of a on-disk representation to a memory
representation expected by __gcov_merge_topn function.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
I'm going to install it if there are no objections.

Martin

libgcc/ChangeLog:

	* libgcov-util.c (read_gcda_finalize): Remove const operator.
	(merge_wrapper): Add both counts and use them properly.
	(topn_to_memory_representation): New function.
	(gcov_merge): Covert on disk representation to in memory
	representation.
	* libgcov.h: Remove const operator.
---
  libgcc/libgcov-util.c | 70 ++++++++++++++++++++++++++++++++++++-------
  libgcc/libgcov.h      |  2 +-
  2 files changed, 61 insertions(+), 11 deletions(-)

Comments

Gerald Pfeifer July 2, 2020, 7:03 p.m. UTC | #1 
On Tue, 16 Jun 2020, Martin Liška wrote:
> libgcc/ChangeLog:
> 
> 	* libgcov-util.c (read_gcda_finalize): Remove const operator.
> 	(merge_wrapper): Add both counts and use them properly.
> 	(topn_to_memory_representation): New function.
> 	(gcov_merge): Covert on disk representation to in memory
> 	representation.

It may not have been this patch, but looking around the bootstrap
failure I reported earlier I noticed the following in stage 2:

  /scratch/tmp/gerald/GCC-HEAD/gcc/../libgcc/libgcov-util.c: In function 
  'int gcov_profile_merge(gcov_info*, gcov_info*, int, int)':
  /scratch/tmp/gerald/GCC-HEAD/gcc/../libgcc/libgcov-util.c:703:12: 
  warning: '*<unknown>' may be used uninitialized [-Wmaybe-uninitialized]
  703 |   tgt_tail = tgt_infos[tgt_cnt - 1];
      |   ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~

Do you also see this (or is this specific to my 32-bit tester)?

Gerald
Martin Liška July 3, 2020, 11:57 a.m. UTC | #2 
On 7/2/20 9:03 PM, Gerald Pfeifer wrote:
> On Tue, 16 Jun 2020, Martin Liška wrote:
>> libgcc/ChangeLog:
>>
>> 	* libgcov-util.c (read_gcda_finalize): Remove const operator.
>> 	(merge_wrapper): Add both counts and use them properly.
>> 	(topn_to_memory_representation): New function.
>> 	(gcov_merge): Covert on disk representation to in memory
>> 	representation.
> 
> It may not have been this patch, but looking around the bootstrap
> failure I reported earlier I noticed the following in stage 2:
> 
>    /scratch/tmp/gerald/GCC-HEAD/gcc/../libgcc/libgcov-util.c: In function
>    'int gcov_profile_merge(gcov_info*, gcov_info*, int, int)':
>    /scratch/tmp/gerald/GCC-HEAD/gcc/../libgcc/libgcov-util.c:703:12:
>    warning: '*<unknown>' may be used uninitialized [-Wmaybe-uninitialized]
>    703 |   tgt_tail = tgt_infos[tgt_cnt - 1];
>        |   ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
> 
> Do you also see this (or is this specific to my 32-bit tester)?

Hello.

Yes, I can also see it. But it's a false positive..

Thanks for heads up,
Martin

> 
> Gerald
>
diff mbox series

Patch

diff --git a/libgcc/libgcov-util.c b/libgcc/libgcov-util.c
index fff54c6a3f6..224c190ee63 100644
--- a/libgcc/libgcov-util.c
+++ b/libgcc/libgcov-util.c
@@ -243,7 +243,7 @@  read_gcda_finalize (struct gcov_info *obj_info)
    /* We set the following fields: merge, n_functions, functions
       and summary.  */
    obj_info->n_functions = num_fn_info;
-  obj_info->functions = (const struct gcov_fn_info**) obstack_finish (&fn_info);
+  obj_info->functions = (struct gcov_fn_info**) obstack_finish (&fn_info);
  
    /* wrap all the counter array.  */
    for (i=0; i< GCOV_COUNTERS; i++)
@@ -506,14 +506,58 @@  gcov_get_merge_weight (void)
     value buffer and weights and then calls the merge function.  */
  
  static void
-merge_wrapper (gcov_merge_fn f, gcov_type *v1, gcov_unsigned_t n,
-               gcov_type *v2, unsigned w)
+merge_wrapper (gcov_merge_fn f, gcov_type *v1, gcov_unsigned_t n1,
+	       gcov_type *v2, gcov_unsigned_t n2, unsigned w)
  {
    gcov_value_buf = v2;
    gcov_value_buf_pos = 0;
-  gcov_value_buf_size = n;
+  gcov_value_buf_size = n2;
    gcov_merge_weight = w;
-  (*f) (v1, n);
+  (*f) (v1, n1);
+}
+
+/* Convert on disk representation of a TOPN counter to in memory representation
+   that is expected from __gcov_merge_topn function.  */
+
+static void
+topn_to_memory_representation (struct gcov_ctr_info *info)
+{
+  auto_vec<gcov_type> output;
+  gcov_type *values = info->values;
+  int count = info->num;
+
+  while (count > 0)
+    {
+      output.safe_push (values[0]);
+      gcov_type n = values[1];
+      output.safe_push (n);
+      if (n > 0)
+	{
+	  struct gcov_kvp *tuples
+	    = (struct gcov_kvp *)xcalloc (sizeof (struct gcov_kvp), n);
+	  for (unsigned i = 0; i < n - 1; i++)
+	    tuples[i].next = &tuples[i + 1];
+	  for (unsigned i = 0; i < n; i++)
+	    {
+	      tuples[i].value = values[2 + 2 * i];
+	      tuples[i].count = values[2 + 2 * i + 1];
+	    }
+	  output.safe_push ((intptr_t)&tuples[0]);
+	}
+      else
+	output.safe_push (0);
+
+      unsigned len = 2 * n + 2;
+      values += len;
+      count -= len;
+    }
+  gcc_assert (count == 0);
+
+  /* Allocate new buffer and copy it there.  */
+  info->num = output.length ();
+  info->values = (gcov_type *)xmalloc (sizeof (gcov_type) * info->num);
+  for (unsigned i = 0; i < info->num; i++)
+    info->values[i] = output[i];
  }
  
  /* Offline tool to manipulate profile data.
@@ -543,9 +587,9 @@  gcov_merge (struct gcov_info *info1, struct gcov_info *info2, int w)
    for (f_ix = 0; f_ix < n_functions; f_ix++)
      {
        unsigned t_ix;
-      const struct gcov_fn_info *gfi_ptr1 = info1->functions[f_ix];
-      const struct gcov_fn_info *gfi_ptr2 = info2->functions[f_ix];
-      const struct gcov_ctr_info *ci_ptr1, *ci_ptr2;
+      struct gcov_fn_info *gfi_ptr1 = info1->functions[f_ix];
+      struct gcov_fn_info *gfi_ptr2 = info2->functions[f_ix];
+      struct gcov_ctr_info *ci_ptr1, *ci_ptr2;
  
        if (!gfi_ptr1 || gfi_ptr1->key != info1)
          continue;
@@ -569,8 +613,14 @@  gcov_merge (struct gcov_info *info1, struct gcov_info *info2, int w)
            gcc_assert (merge1 == merge2);
            if (!merge1)
              continue;
-          gcc_assert (ci_ptr1->num == ci_ptr2->num);
-          merge_wrapper (merge1, ci_ptr1->values, ci_ptr1->num, ci_ptr2->values, w);
+
+	  if (merge1 == __gcov_merge_topn)
+	    topn_to_memory_representation (ci_ptr1);
+	  else
+	    gcc_assert (ci_ptr1->num == ci_ptr2->num);
+
+	  merge_wrapper (merge1, ci_ptr1->values, ci_ptr1->num,
+			 ci_ptr2->values, ci_ptr2->num, w);
            ci_ptr1++;
            ci_ptr2++;
          }
diff --git a/libgcc/libgcov.h b/libgcc/libgcov.h
index 5d237a4c730..ffa9a690af4 100644
--- a/libgcc/libgcov.h
+++ b/libgcc/libgcov.h
@@ -216,7 +216,7 @@  struct gcov_info
    const struct gcov_fn_info *const *functions; /* pointer to pointers
                                                    to function information  */
  #else
-  const struct gcov_fn_info **functions;
+  struct gcov_fn_info **functions;
  #endif /* !IN_GCOV_TOOL */
  };