[PATCH 0/8] GCOV improvements

[PATCH 0/8] GCOV improvements

[marxin]

Hi.

Following patch series improves the infrastructure, fixes couple of issues
and hopefully improves also documentation of the functionality.
I decided to split the changes into various patches to make it easier
for reviewers.

Bootstraps and survives regression tests on ppc64le-linux-gnu and can
generate LCOV HTML page for GCC (running test-suite).

Martin

marxin (8):
  gcno file: do not stream block flags (PR gcov-profile/80031).
  Remove .gcno file when compilation does not success (PR driver/56469).
  Simplify representation of locations of a block.
  Introduce new option -w which shows verbose informations.
  Make gcno more precise about BBs really belonging to a line (PR
    gcov-profile/79891).
  Fix format_gcov to not print misleading values (PR gcov-profile/53915)
  Sort options of gcov, gcov-dump and gcov-tool both in --help and
    documentation
  Enhance documentation of gcov.

 gcc/coverage.c                         |  12 +
 gcc/coverage.h                         |   1 +
 gcc/doc/gcov-dump.texi                 |  10 +-
 gcc/doc/gcov-tool.texi                 |  50 ++---
 gcc/doc/gcov.texi                      | 143 ++++++------
 gcc/gcov-dump.c                        |  24 +-
 gcc/gcov-io.h                          |   1 -
 gcc/gcov-tool.c                        |  10 +-
 gcc/gcov.c                             | 395 ++++++++++++++++-----------------
 gcc/profile.c                          |  42 ++--
 gcc/testsuite/gcc.misc-tests/gcov-17.c |  30 +++
 gcc/testsuite/gcc.misc-tests/gcov-18.c |  27 +++
 gcc/toplev.c                           |   3 +
 13 files changed, 391 insertions(+), 357 deletions(-)
 create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-17.c
 create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-18.c

-- 
2.12.2

[PATCH 5/8] Make gcno more precise about BBs really belonging to a line (PR gcov-profile/79891).

[marxin]

gcc/ChangeLog:

2017-04-26  Martin Liska  <mliska@suse.cz>

	PR gcov-profile/79891
	* gcov.c (add_line_counts): Assign BBs to lines just if the BB
	is marked by compiler as living on a line.
	(get_cycles_count): Remove usage of the union.
	(output_intermediate_file): Likewise.
	(find_source): Fix GNU coding style.
	(accumulate_line_counts): Remove old non-all block mode.
	(output_lines): Remove usage of the union.
	* profile.c (output_location): Include all BBs, even if
	belonging to a same line (and file) as a previous BB.

gcc/testsuite/ChangeLog:

2017-04-26  Martin Liska  <mliska@suse.cz>

	PR gcov-profile/79891
	* gcc.misc-tests/gcov-17.c: New test.
	* gcc.misc-tests/gcov-18.c: New test.
---
 gcc/gcov.c                             | 92 +++++++++++++---------------------
 gcc/profile.c                          | 39 +++++++-------
 gcc/testsuite/gcc.misc-tests/gcov-17.c | 30 +++++++++++
 gcc/testsuite/gcc.misc-tests/gcov-18.c | 27 ++++++++++
 4 files changed, 110 insertions(+), 78 deletions(-)
 create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-17.c
 create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-18.c

diff --git a/gcc/gcov.c b/gcc/gcov.c
index 6163d7d6dee..0adb4466f70 100644
--- a/gcc/gcov.c
+++ b/gcc/gcov.c
@@ -235,14 +235,9 @@ typedef struct line_info
   bool has_block (block_t *needle);
 
   gcov_type count;	   /* execution count */
-  union
-  {
-    arc_t *branches;	   /* branches from blocks that end on this
-			      line. Used for branch-counts when not
-			      all-blocks mode.  */
-    block_t *blocks;       /* blocks which start on this line.  Used
-			      in all-blocks mode.  */
-  } u;
+  arc_t *branches;	   /* branches from blocks that end on this line.  */
+  block_t *blocks;	   /* blocks which start on this line.
+			      Used in all-blocks mode.  */
   unsigned exists : 1;
   unsigned unexceptional : 1;
 } line_t;
@@ -250,7 +245,7 @@ typedef struct line_info
 bool
 line_t::has_block (block_t *needle)
 {
-  for (block_t *n = u.blocks; n; n = n->chain)
+  for (block_t *n = blocks; n; n = n->chain)
     if (n == needle)
       return true;
 
@@ -604,7 +599,7 @@ get_cycles_count (line_t &linfo, bool handle_negative_cycles = true)
 
   loop_type result = NO_LOOP;
   gcov_type count = 0;
-  for (block_t *block = linfo.u.blocks; block; block = block->chain)
+  for (block_t *block = linfo.blocks; block; block = block->chain)
     {
       arc_vector_t path;
       block_vector_t blocked;
@@ -882,7 +877,7 @@ output_intermediate_file (FILE *gcov_file, source_t *src)
 	fprintf (gcov_file, "lcount:%u,%s\n", line_num,
 		 format_gcov (line->count, 0, -1));
       if (flag_branches)
-        for (arc = line->u.branches; arc; arc = arc->line_next)
+	for (arc = line->branches; arc; arc = arc->line_next)
           {
             if (!arc->is_unconditional && !arc->is_call_non_return)
               {
@@ -1275,7 +1270,7 @@ find_source (const char *file_name)
 
   /* Resort the name map.  */
   qsort (names, n_names, sizeof (*names), name_sort);
-  
+
  check_date:
   if (sources[idx].file_time > bbg_file_time)
     {
@@ -2235,13 +2230,11 @@ mangle_name (char const *base, char *ptr)
 static void
 add_line_counts (coverage_t *coverage, function_t *fn)
 {
-  unsigned ix;
-  line_t *line = NULL; /* This is propagated from one iteration to the
-			  next.  */
-
+  bool has_any_line = false;
   /* Scan each basic block.  */
-  for (ix = 0; ix != fn->blocks.size (); ix++)
+  for (unsigned ix = 0; ix != fn->blocks.size (); ix++)
     {
+      line_t *line = NULL;
       block_t *block = &fn->blocks[ix];
       if (block->count && ix && ix + 1 != fn->blocks.size ())
 	fn->blocks_executed++;
@@ -2269,32 +2262,32 @@ add_line_counts (coverage_t *coverage, function_t *fn)
       block->cycle.arc = NULL;
       block->cycle.ident = ~0U;
 
+      if (!has_any_line)
+	has_any_line = true;
+
       if (!ix || ix + 1 == fn->blocks.size ())
 	/* Entry or exit block */;
-      else if (flag_all_blocks)
+      else if (line != NULL)
 	{
-	  line_t *block_line = line;
-
-	  if (!block_line)
-	    block_line = &sources[fn->src].lines[fn->line];
-
-	  block->chain = block_line->u.blocks;
-	  block_line->u.blocks = block;
-	}
-      else if (flag_branches)
-	{
-	  arc_t *arc;
+	  block->chain = line->blocks;
+	  line->blocks = block;
 
-	  for (arc = block->succ; arc; arc = arc->succ_next)
+	  if (flag_branches)
 	    {
-	      arc->line_next = line->u.branches;
-	      line->u.branches = arc;
-	      if (coverage && !arc->is_unconditional)
-		add_branch_counts (coverage, arc);
+	      arc_t *arc;
+
+	      for (arc = block->succ; arc; arc = arc->succ_next)
+		{
+		  arc->line_next = line->branches;
+		  line->branches = arc;
+		  if (coverage && !arc->is_unconditional)
+		    add_branch_counts (coverage, arc);
+		}
 	    }
 	}
     }
-  if (!line)
+
+  if (!has_any_line)
     fnotice (stderr, "%s:no lines for '%s'\n", bbg_file_name, fn->name);
 }
 
@@ -2317,22 +2310,7 @@ accumulate_line_counts (source_t *src)
 
   for (ix = src->num_lines, line = src->lines; ix--; line++)
     {
-      if (!flag_all_blocks)
-	{
-	  arc_t *arc, *arc_p, *arc_n;
-
-	  /* Total and reverse the branch information.  */
-	  for (arc = line->u.branches, arc_p = NULL; arc;
-	       arc_p = arc, arc = arc_n)
-	    {
-	      arc_n = arc->line_next;
-	      arc->line_next = arc_p;
-
-	      add_branch_counts (&src->coverage, arc);
-	    }
-	  line->u.branches = arc_p;
-	}
-      else if (line->u.blocks)
+      if (line->blocks)
 	{
 	  /* The user expects the line count to be the number of times
 	     a line has been executed. Simply summing the block count
@@ -2344,17 +2322,17 @@ accumulate_line_counts (source_t *src)
 	  gcov_type count = 0;
 
 	  /* Reverse the block information.  */
-	  for (block = line->u.blocks, block_p = NULL; block;
+	  for (block = line->blocks, block_p = NULL; block;
 	       block_p = block, block = block_n)
 	    {
 	      block_n = block->chain;
 	      block->chain = block_p;
 	      block->cycle.ident = ix;
 	    }
-	  line->u.blocks = block_p;
+	  line->blocks = block_p;
 
 	  /* Sum the entry arcs.  */
-	  for (block = line->u.blocks; block; block = block->chain)
+	  for (block = line->blocks; block; block = block->chain)
 	    {
 	      arc_t *arc;
 
@@ -2364,7 +2342,7 @@ accumulate_line_counts (source_t *src)
 	    }
 
 	  /* Cycle detection.  */
-	  for (block = line->u.blocks; block; block = block->chain)
+	  for (block = line->blocks; block; block = block->chain)
 	    {
 	      for (arc_t *arc = block->pred; arc; arc = arc->pred_next)
 		if (!line->has_block (arc->src))
@@ -2550,7 +2528,7 @@ output_lines (FILE *gcov_file, const source_t *src)
 	  arc_t *arc;
 	  int ix, jx;
 
-	  for (ix = jx = 0, block = line->u.blocks; block;
+	  for (ix = jx = 0, block = line->blocks; block;
 	       block = block->chain)
 	    {
 	      if (!block->is_call_return)
@@ -2574,7 +2552,7 @@ output_lines (FILE *gcov_file, const source_t *src)
 	  int ix;
 	  arc_t *arc;
 
-	  for (ix = 0, arc = line->u.branches; arc; arc = arc->line_next)
+	  for (ix = 0, arc = line->branches; arc; arc = arc->line_next)
 	    ix += output_branch_count (gcov_file, ix, arc);
 	}
     }
diff --git a/gcc/profile.c b/gcc/profile.c
index c7eed0e3dfd..3346af4e961 100644
--- a/gcc/profile.c
+++ b/gcc/profile.c
@@ -941,29 +941,26 @@ output_location (char const *file_name, int line,
   name_differs = !prev_file_name || filename_cmp (file_name, prev_file_name);
   line_differs = prev_line != line;
 
-  if (name_differs || line_differs)
+  if (!*offset)
     {
-      if (!*offset)
-	{
-	  *offset = gcov_write_tag (GCOV_TAG_LINES);
-	  gcov_write_unsigned (bb->index);
-	  name_differs = line_differs=true;
-	}
+      *offset = gcov_write_tag (GCOV_TAG_LINES);
+      gcov_write_unsigned (bb->index);
+      name_differs = line_differs = true;
+    }
 
-      /* If this is a new source file, then output the
-	 file's name to the .bb file.  */
-      if (name_differs)
-	{
-	  prev_file_name = file_name;
-	  gcov_write_unsigned (0);
-	  gcov_write_string (prev_file_name);
-	}
-      if (line_differs)
-	{
-	  gcov_write_unsigned (line);
-	  prev_line = line;
-	}
-     }
+  /* If this is a new source file, then output the
+     file's name to the .bb file.  */
+  if (name_differs)
+    {
+      prev_file_name = file_name;
+      gcov_write_unsigned (0);
+      gcov_write_string (prev_file_name);
+    }
+  if (line_differs)
+    {
+      gcov_write_unsigned (line);
+      prev_line = line;
+    }
 }
 
 /* Instrument and/or analyze program behavior based on program the CFG.
diff --git a/gcc/testsuite/gcc.misc-tests/gcov-17.c b/gcc/testsuite/gcc.misc-tests/gcov-17.c
new file mode 100644
index 00000000000..1cff708be9b
--- /dev/null
+++ b/gcc/testsuite/gcc.misc-tests/gcov-17.c
@@ -0,0 +1,30 @@
+/* Test gcov block mode.  */
+
+/* { dg-options "-fprofile-arcs -ftest-coverage" } */
+/* { dg-do run { target native } } */
+
+unsigned int
+UuT (void)
+{
+  unsigned int true_var = 1;
+  unsigned int false_var = 0;
+  unsigned int ret = 0;
+
+  if (true_var) /* count(1) */
+    {
+      if (false_var) /* count(1) */
+	ret = 111; /* count(#####) */
+    }
+  else
+    ret = 999; /* count(#####) */
+  return ret;
+}
+
+int
+main (int argc, char **argv)
+{
+  UuT ();
+  return 0;
+}
+
+/* { dg-final { run-gcov { -a gcov-17.c } } } */
diff --git a/gcc/testsuite/gcc.misc-tests/gcov-18.c b/gcc/testsuite/gcc.misc-tests/gcov-18.c
new file mode 100644
index 00000000000..4625c23087c
--- /dev/null
+++ b/gcc/testsuite/gcc.misc-tests/gcov-18.c
@@ -0,0 +1,27 @@
+/* Test gcov block mode.  */
+
+/* { dg-options "-fprofile-arcs -ftest-coverage" } */
+/* { dg-do run { target native } } */
+
+int a = 0;
+
+void foo() /* count(1) */
+{
+  a = 1;
+}
+
+void bar() /* count(1) */
+{
+  a++;
+}
+
+int main() /* count(1) */
+{
+  foo (); goto baz; lab: bar (); /* count(2) */
+
+  baz:
+    if (a == 1) /* count(2) */
+      goto lab; /* count(1) */
+}
+
+/* { dg-final { run-gcov { gcov-18.c } } } */
-- 
2.12.2

Re: [PATCH 5/8] Make gcno more precise about BBs really belonging to a line (PR gcov-profile/79891).

[Nathan Sidwell]

On 04/25/2017 09:08 AM, marxin wrote:
> gcc/ChangeLog:
> 
> 2017-04-26  Martin Liska  <mliska@suse.cz>
> 
> 	PR gcov-profile/79891
> 	* gcov.c (add_line_counts): Assign BBs to lines just if the BB
> 	is marked by compiler as living on a line.
> 	(get_cycles_count): Remove usage of the union.
> 	(output_intermediate_file): Likewise.
> 	(find_source): Fix GNU coding style.
> 	(accumulate_line_counts): Remove old non-all block mode.
> 	(output_lines): Remove usage of the union.
> 	* profile.c (output_location): Include all BBs, even if
> 	belonging to a same line (and file) as a previous BB.
> 


> @@ -2269,32 +2262,32 @@ add_line_counts (coverage_t *coverage, function_t *fn)
>         block->cycle.arc = NULL;
>         block->cycle.ident = ~0U;
>   
> +      if (!has_any_line)
> +	has_any_line = true;

You know, you're allowed to set a true bool to true :)

Could you annotate the testcase with what the original problem was?
I think that'll help a future regression investigator.

otherwise ok.

nathan

-- 
Nathan Sidwell

Re: [PATCH 5/8] Make gcno more precise about BBs really belonging to a line (PR gcov-profile/79891).

[Martin Liška]

On 04/28/2017 01:41 PM, Nathan Sidwell wrote:
> On 04/25/2017 09:08 AM, marxin wrote:
>> gcc/ChangeLog:
>>
>> 2017-04-26  Martin Liska  <mliska@suse.cz>
>>
>>     PR gcov-profile/79891
>>     * gcov.c (add_line_counts): Assign BBs to lines just if the BB
>>     is marked by compiler as living on a line.
>>     (get_cycles_count): Remove usage of the union.
>>     (output_intermediate_file): Likewise.
>>     (find_source): Fix GNU coding style.
>>     (accumulate_line_counts): Remove old non-all block mode.
>>     (output_lines): Remove usage of the union.
>>     * profile.c (output_location): Include all BBs, even if
>>     belonging to a same line (and file) as a previous BB.
>>
> 
> 
>> @@ -2269,32 +2262,32 @@ add_line_counts (coverage_t *coverage, function_t *fn)
>>         block->cycle.arc = NULL;
>>         block->cycle.ident = ~0U;
>>   +      if (!has_any_line)
>> +    has_any_line = true;
> 
> You know, you're allowed to set a true bool to true :)

Heh, stupid mistake. Actually, can be funny warning candidate.
I guess I'm not first who did it.

> 
> Could you annotate the testcase with what the original problem was?

Sure, however I do not expect any PRs related to this commit. No, I'm kidding,
the patch make a significant changes :)

Martin

> I think that'll help a future regression investigator.
> 
> otherwise ok.
> 
> nathan
> 

[PATCH 2/8] Remove .gcno file when compilation does not success (PR driver/56469).

[marxin]

gcc/ChangeLog:

2017-04-19  Martin Liska  <mliska@suse.cz>

	PR driver/56469
	* coverage.c (coverage_remove_note_file): New function.
	* coverage.h: Declare the function.
	* toplev.c (finalize): Clean if an error has been seen.
---
 gcc/coverage.c | 12 ++++++++++++
 gcc/coverage.h |  1 +
 gcc/toplev.c   |  3 +++
 3 files changed, 16 insertions(+)

diff --git a/gcc/coverage.c b/gcc/coverage.c
index 0a949c3a9f7..53e379b3295 100644
--- a/gcc/coverage.c
+++ b/gcc/coverage.c
@@ -731,6 +731,18 @@ coverage_end_function (unsigned lineno_checksum, unsigned cfg_checksum)
     }
 }

 
+/* Remove coverage file if opened.  */
+
+void
+coverage_remove_note_file (void)
+{
+  if (bbg_file_name)
+    {
+      gcov_close ();
+      unlink (bbg_file_name);
+    }
+}
+
 /* Build a coverage variable of TYPE for function FN_DECL.  If COUNTER
    >= 0 it is a counter array, otherwise it is the function structure.  */
 
diff --git a/gcc/coverage.h b/gcc/coverage.h
index cde6aef1d76..90454c0ecba 100644
--- a/gcc/coverage.h
+++ b/gcc/coverage.h
@@ -24,6 +24,7 @@ along with GCC; see the file COPYING3.  If not see
 
 extern void coverage_init (const char *);
 extern void coverage_finish (void);
+extern void coverage_remove_note_file (void);
 
 /* Start outputting coverage information for the current
    function.  */
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 54a4f05c9a1..f1384fc2fda 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1915,6 +1915,9 @@ finalize (bool no_backend)
       stack_usage_file = NULL;
     }
 
+  if (seen_error ())
+    coverage_remove_note_file ();
+
   if (!no_backend)
     {
       statistics_fini ();
-- 
2.12.2

Re: [PATCH 2/8] Remove .gcno file when compilation does not success (PR driver/56469).

[Nathan Sidwell]

On 04/19/2017 03:56 AM, marxin wrote:
> gcc/ChangeLog:
> 
> 2017-04-19  Martin Liska  <mliska@suse.cz>
> 
> 	PR driver/56469
> 	* coverage.c (coverage_remove_note_file): New function.
> 	* coverage.h: Declare the function.
> 	* toplev.c (finalize): Clean if an error has been seen.

ok

nathan
-- 
Nathan Sidwell

[PATCH 6/8] Fix format_gcov to not print misleading values (PR gcov-profile/53915)

[marxin]

gcc/ChangeLog:

2017-04-27  Martin Liska  <mliska@suse.cz>

	PR gcov-profile/53915
	* gcov.c (format_gcov): Print 'NAN %' when top > bottom.
---
 gcc/gcov.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/gcc/gcov.c b/gcc/gcov.c
index 0adb4466f70..22378583c5c 100644
--- a/gcc/gcov.c
+++ b/gcc/gcov.c
@@ -1942,6 +1942,13 @@ format_gcov (gcov_type top, gcov_type bottom, int dp)
 {
   static char buffer[20];
 
+  /* Handle invalid values that would result in a misleading value.  */
+  if (bottom != 0 && top > bottom && dp >= 0)
+    {
+      sprintf (buffer, "NAN %%");
+      return buffer;
+    }
+
   if (dp >= 0)
     {
       float ratio = bottom ? (float)top / bottom : 0;
-- 
2.12.2

Re: [PATCH 6/8] Fix format_gcov to not print misleading values (PR gcov-profile/53915)

[Nathan Sidwell]

On 04/27/2017 06:04 AM, marxin wrote:
> gcc/ChangeLog:
> 
> 2017-04-27  Martin Liska  <mliska@suse.cz>
> 
> 	PR gcov-profile/53915
> 	* gcov.c (format_gcov): Print 'NAN %' when top > bottom.

ok


-- 
Nathan Sidwell

[PATCH 1/8] gcno file: do not stream block flags (PR gcov-profile/80031).

[marxin]

gcc/ChangeLog:

2017-03-13  Martin Liska  <mliska@suse.cz>

	PR gcov-profile/80031
	* gcov-dump.c (tag_blocks): Just print number of basic blocks.
	* gcov-io.h (GCOV_TAG_BLOCKS_NUM): Remove unused macro.
	* gcov.c (read_graph_file): Read just number of blocks.
	* profile.c (branch_prob): Do not stream 0 flags per a basic
	block.
---
 gcc/gcov-dump.c | 22 ++--------------------
 gcc/gcov-io.h   |  1 -
 gcc/gcov.c      |  7 +------
 gcc/profile.c   |  3 +--
 4 files changed, 4 insertions(+), 29 deletions(-)

diff --git a/gcc/gcov-dump.c b/gcc/gcov-dump.c
index 47db1795313..f2522577e9d 100644
--- a/gcc/gcov-dump.c
+++ b/gcc/gcov-dump.c
@@ -318,27 +318,9 @@ tag_function (const char *filename ATTRIBUTE_UNUSED,
 static void
 tag_blocks (const char *filename ATTRIBUTE_UNUSED,
 	    unsigned tag ATTRIBUTE_UNUSED, unsigned length ATTRIBUTE_UNUSED,
-	    unsigned depth)
+	    unsigned depth ATTRIBUTE_UNUSED)
 {
-  unsigned n_blocks = GCOV_TAG_BLOCKS_NUM (length);
-
-  printf (" %u blocks", n_blocks);
-
-  if (flag_dump_contents)
-    {
-      unsigned ix;
-
-      for (ix = 0; ix != n_blocks; ix++)
-	{
-	  if (!(ix & 7))
-	    {
-	      printf ("\n");
-	      print_prefix (filename, depth, gcov_position ());
-	      printf (VALUE_PADDING_PREFIX VALUE_PREFIX, ix);
-	    }
-	  printf ("%04x ", gcov_read_unsigned ());
-	}
-    }
+  printf (" %u blocks", gcov_read_unsigned ());
 }
 
 static void
diff --git a/gcc/gcov-io.h b/gcc/gcov-io.h
index 1fb58dd918e..1c8ee8f9a2a 100644
--- a/gcc/gcov-io.h
+++ b/gcc/gcov-io.h
@@ -230,7 +230,6 @@ typedef uint64_t gcov_type_unsigned;
 #define GCOV_TAG_FUNCTION_LENGTH (3)
 #define GCOV_TAG_BLOCKS		 ((gcov_unsigned_t)0x01410000)
 #define GCOV_TAG_BLOCKS_LENGTH(NUM) (NUM)
-#define GCOV_TAG_BLOCKS_NUM(LENGTH) (LENGTH)
 #define GCOV_TAG_ARCS		 ((gcov_unsigned_t)0x01430000)
 #define GCOV_TAG_ARCS_LENGTH(NUM)  (1 + (NUM) * 2)
 #define GCOV_TAG_ARCS_NUM(LENGTH)  (((LENGTH) - 1) / 2)
diff --git a/gcc/gcov.c b/gcc/gcov.c
index bb26a1a9787..63f6a75f1af 100644
--- a/gcc/gcov.c
+++ b/gcc/gcov.c
@@ -129,7 +129,6 @@ typedef struct block_info
 
   /* Block execution count.  */
   gcov_type count;
-  unsigned flags : 12;
   unsigned count_valid : 1;
   unsigned valid_chain : 1;
   unsigned invalid_chain : 1;
@@ -1374,12 +1373,8 @@ read_graph_file (void)
 		     bbg_file_name, fn->name);
 	  else
 	    {
-	      unsigned ix, num_blocks = GCOV_TAG_BLOCKS_NUM (length);
-	      fn->num_blocks = num_blocks;
-
+	      fn->num_blocks = gcov_read_unsigned ();
 	      fn->blocks = XCNEWVEC (block_t, fn->num_blocks);
-	      for (ix = 0; ix != num_blocks; ix++)
-		fn->blocks[ix].flags = gcov_read_unsigned ();
 	    }
 	}
       else if (fn && tag == GCOV_TAG_ARCS)
diff --git a/gcc/profile.c b/gcc/profile.c
index c6f462d2f7a..c7eed0e3dfd 100644
--- a/gcc/profile.c
+++ b/gcc/profile.c
@@ -1195,8 +1195,7 @@ branch_prob (void)
 
       /* Basic block flags */
       offset = gcov_write_tag (GCOV_TAG_BLOCKS);
-      for (i = 0; i != (unsigned) (n_basic_blocks_for_fn (cfun)); i++)
-	gcov_write_unsigned (0);
+      gcov_write_unsigned (n_basic_blocks_for_fn (cfun));
       gcov_write_length (offset);
 
       /* Arcs */
-- 
2.12.2

Re: [PATCH 1/8] gcno file: do not stream block flags (PR gcov-profile/80031).

[Nathan Sidwell]

On 04/21/2017 08:40 AM, marxin wrote:
> gcc/ChangeLog:
> 
> 2017-03-13  Martin Liska  <mliska@suse.cz>
> 
> 	PR gcov-profile/80031
> 	* gcov-dump.c (tag_blocks): Just print number of basic blocks.
> 	* gcov-io.h (GCOV_TAG_BLOCKS_NUM): Remove unused macro.
> 	* gcov.c (read_graph_file): Read just number of blocks.
> 	* profile.c (branch_prob): Do not stream 0 flags per a basic
> 	block.

ok


-- 
Nathan Sidwell

[PATCH 4/8] Introduce new option -w which shows verbose informations.

[marxin]

gcc/ChangeLog:

2017-04-26  Martin Liska  <mliska@suse.cz>

	* gcov.c (process_args): Handle new argument 'w'.
	(read_graph_file): Assign ID to BBs.
	(output_branch_count): Display BB # if verbose flag is set.
	(output_lines): Likewise for arcs.
	(print_usage): Add '--verbose' option help.
	* doc/gcov.texi: Document --verbose (-w) option.
---
 gcc/doc/gcov.texi |  4 ++++
 gcc/gcov.c        | 38 ++++++++++++++++++++++++++++++--------
 2 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/gcc/doc/gcov.texi b/gcc/doc/gcov.texi
index d728444e1e7..2b4101018a2 100644
--- a/gcc/doc/gcov.texi
+++ b/gcc/doc/gcov.texi
@@ -289,6 +289,10 @@ where the @var{source-file} component is the final filename part and
 the @var{md5} component is calculated from the full mangled name that
 would have been used otherwise.
 
+@item -w
+@itemx --verbose
+Print verbose informations related to basic blocks and arcs.
+
 @end table
 
 @command{gcov} should be run with the current directory the same as that
diff --git a/gcc/gcov.c b/gcc/gcov.c
index 7400cdee110..6163d7d6dee 100644
--- a/gcc/gcov.c
+++ b/gcc/gcov.c
@@ -137,6 +137,8 @@ typedef struct block_info
   gcov_type num_succ;
   gcov_type num_pred;
 
+  unsigned id;
+
   /* Block execution count.  */
   gcov_type count;
   unsigned count_valid : 1;
@@ -366,6 +368,10 @@ static int flag_long_names = 0;
 
 static int flag_hash_filenames = 0;
 
+/* Print verbose informations.  */
+
+static int flag_verbose = 0;
+
 /* Output count information for every basic block, not merely those
    that contain line number information.  */
 
@@ -696,6 +702,7 @@ print_usage (int error_p)
   fnotice (file, "  -s, --source-prefix DIR         Source prefix to elide\n");
   fnotice (file, "  -u, --unconditional-branches    Show unconditional branch counts too\n");
   fnotice (file, "  -v, --version                   Print version number, then exit\n");
+  fnotice (file, "  -w, --verbose                   Print verbose informations\n");
   fnotice (file, "  -x, --hash-filenames            Hash long pathnames\n");
   fnotice (file, "\nFor bug reporting instructions, please see:\n%s.\n",
 	   bug_report_url);
@@ -721,6 +728,7 @@ static const struct option options[] =
 {
   { "help",                 no_argument,       NULL, 'h' },
   { "version",              no_argument,       NULL, 'v' },
+  { "verbose",              no_argument,       NULL, 'w' },
   { "all-blocks",           no_argument,       NULL, 'a' },
   { "branch-probabilities", no_argument,       NULL, 'b' },
   { "branch-counts",        no_argument,       NULL, 'c' },
@@ -747,7 +755,7 @@ process_args (int argc, char **argv)
 {
   int opt;
 
-  const char *opts = "abcdfhilmno:prs:uvx";
+  const char *opts = "abcdfhilmno:prs:uvwx";
   while ((opt = getopt_long (argc, argv, opts, options, NULL)) != -1)
     {
       switch (opt)
@@ -802,6 +810,9 @@ process_args (int argc, char **argv)
 	case 'x':
 	  flag_hash_filenames = 1;
 	  break;
+	case 'w':
+	  flag_verbose = 1;
+	  break;
 	case 'v':
 	  print_version ();
 	  /* print_version will exit.  */
@@ -1371,6 +1382,7 @@ read_graph_file (void)
       else if (fn && tag == GCOV_TAG_ARCS)
 	{
 	  unsigned src = gcov_read_unsigned ();
+	  fn->blocks[src].id = src;
 	  unsigned num_dests = GCOV_TAG_ARCS_NUM (length);
 	  block_t *src_blk = &fn->blocks[src];
 	  unsigned mark_catches = 0;
@@ -2395,12 +2407,17 @@ output_branch_count (FILE *gcov_file, int ix, const arc_t *arc)
   else if (!arc->is_unconditional)
     {
       if (arc->src->count)
-	fnotice (gcov_file, "branch %2d taken %s%s\n", ix,
+	fnotice (gcov_file, "branch %2d taken %s%s", ix,
 		 format_gcov (arc->count, arc->src->count, -flag_counts),
 		 arc->fall_through ? " (fallthrough)"
 		 : arc->is_throw ? " (throw)" : "");
       else
-	fnotice (gcov_file, "branch %2d never executed\n", ix);
+	fnotice (gcov_file, "branch %2d never executed", ix);
+
+      if (flag_verbose)
+	fnotice (gcov_file, " (BB %d)", arc->dst->id);
+
+      fnotice (gcov_file, "\n");
     }
   else if (flag_unconditional && !arc->dst->is_call_return)
     {
@@ -2537,11 +2554,16 @@ output_lines (FILE *gcov_file, const source_t *src)
 	       block = block->chain)
 	    {
 	      if (!block->is_call_return)
-		fprintf (gcov_file, "%9s:%5u-block %2d\n",
-			 !line->exists ? "-" : block->count
-			 ? format_gcov (block->count, 0, -1)
-			 : block->exceptional ? "%%%%%" : "$$$$$",
-			 line_num, ix++);
+		{
+		  fprintf (gcov_file, "%9s:%5u-block %2d",
+			   !line->exists ? "-" : block->count
+			   ? format_gcov (block->count, 0, -1)
+			   : block->exceptional ? "%%%%%" : "$$$$$",
+			   line_num, ix++);
+		  if (flag_verbose)
+		    fprintf (gcov_file, " (BB %u)", block->id);
+		  fprintf (gcov_file, "\n");
+		}
 	      if (flag_branches)
 		for (arc = block->succ; arc; arc = arc->succ_next)
 		  jx += output_branch_count (gcov_file, jx, arc);
-- 
2.12.2

Re: [PATCH 4/8] Introduce new option -w which shows verbose informations.

[Nathan Sidwell]

On 04/25/2017 08:05 AM, marxin wrote:
> gcc/ChangeLog:
> 
> 2017-04-26  Martin Liska  <mliska@suse.cz>
> 
> 	* gcov.c (process_args): Handle new argument 'w'.
> 	(read_graph_file): Assign ID to BBs.
> 	(output_branch_count): Display BB # if verbose flag is set.
> 	(output_lines): Likewise for arcs.
> 	(print_usage): Add '--verbose' option help.
> 	* doc/gcov.texi: Document --verbose (-w) option.

ok


-- 
Nathan Sidwell

[PATCH 7/8] Sort options of gcov, gcov-dump and gcov-tool both in --help and documentation

[marxin]

gcc/ChangeLog:

2017-04-27  Martin Liska  <mliska@suse.cz>

	* doc/gcov.texi: Sort options in alphabetic order.
	* doc/gcov-dump.texi: Likewise.
	* doc/gcov-tool.texi: Likewise.
	* gcov.c (print_usage): Likewise.
	* gcov-dump.c (print_usage): Likewise.
	* gcov-tool.c (print_merge_usage_message): Likewise.
	(print_rewrite_usage_message): Likewise.
	(print_overlap_usage_message): Likewise.
---
 gcc/doc/gcov-dump.texi |  10 ++--
 gcc/doc/gcov-tool.texi |  50 +++++++++---------
 gcc/doc/gcov.texi      | 141 +++++++++++++++++++++++++------------------------
 gcc/gcov-dump.c        |   2 +-
 gcc/gcov-tool.c        |  10 ++--
 gcc/gcov.c             |   2 +-
 6 files changed, 108 insertions(+), 107 deletions(-)

diff --git a/gcc/doc/gcov-dump.texi b/gcc/doc/gcov-dump.texi
index d7931fd3a19..26653d28def 100644
--- a/gcc/doc/gcov-dump.texi
+++ b/gcc/doc/gcov-dump.texi
@@ -72,11 +72,6 @@ gcov-dump [@option{-v}|@option{--version}]
 Display help about using @command{gcov-dump} (on the standard output), and
 exit without doing any further processing.
 
-@item -v
-@itemx --version
-Display the @command{gcov-dump} version number (on the standard output),
-and exit without doing any further processing.
-
 @item -l
 @itemx --long
 Dump content of records.
@@ -85,6 +80,11 @@ Dump content of records.
 @itemx --positions
 Dump positions of records.
 
+@item -v
+@itemx --version
+Display the @command{gcov-dump} version number (on the standard output),
+and exit without doing any further processing.
+
 @item -w
 @itemx --working-sets
 Dump working set computed from summary.
diff --git a/gcc/doc/gcov-tool.texi b/gcc/doc/gcov-tool.texi
index 86044fc1c0b..79f6d96f5ce 100644
--- a/gcc/doc/gcov-tool.texi
+++ b/gcc/doc/gcov-tool.texi
@@ -113,23 +113,23 @@ gcov-tool @r{[}@var{global-options}@r{]} SUB_COMMAND @r{[}@var{sub_command-optio
 gcov-tool [@option{-v}|@option{--version}] [@option{-h}|@option{--help}]
 
 gcov-tool merge [merge-options] @var{directory1} @var{directory2}
+     [@option{-o}|@option{--output} @var{directory}]
      [@option{-v}|@option{--verbose}]
-     [@option{-o}|@option{ --output} @var{directory}]
      [@option{-w}|@option{--weight} @var{w1,w2}]
 
 gcov-tool rewrite [rewrite-options] @var{directory}
-     [@option{-v}|@option{--verbose}]
+     [@option{-n}|@option{--normalize} @var{long_long_value}]
      [@option{-o}|@option{--output} @var{directory}]
      [@option{-s}|@option{--scale} @var{float_or_simple-frac_value}]
-     [@option{-n}|@option{--normalize} @var{long_long_value}]
+     [@option{-v}|@option{--verbose}]
 
 gcov-tool overlap [overlap-options] @var{directory1} @var{directory2}
-     [@option{-v}|@option{--verbose}]
-     [@option{-h}|@option{--hotonly}]
      [@option{-f}|@option{--function}]
      [@option{-F}|@option{--fullname}]
+     [@option{-h}|@option{--hotonly}]
      [@option{-o}|@option{--object}]
      [@option{-t}|@option{--hot_threshold}] @var{float}
+     [@option{-v}|@option{--verbose}]
 
 @c man end
 @c man begin SEEALSO
@@ -152,17 +152,17 @@ and exit without doing any further processing.
 
 @item merge
 Merge two profile directories.
-
 @table @gcctabopt
-@item -v
-@itemx --verbose
-Set the verbose mode.
 
 @item -o @var{directory}
 @itemx --output @var{directory}
 Set the output profile directory. Default output directory name is
 @var{merged_profile}.
 
+@item -v
+@itemx --verbose
+Set the verbose mode.
+
 @item -w @var{w1},@var{w2}
 @itemx --weight @var{w1},@var{w2}
 Set the merge weights of the @var{directory1} and @var{directory2},
@@ -171,11 +171,12 @@ respectively. The default weights are 1 for both.
 
 @item rewrite
 Read the specified profile directory and rewrite to a new directory.
-
 @table @gcctabopt
-@item -v
-@itemx --verbose
-Set the verbose mode.
+
+@item -n @var{long_long_value}
+@itemx --normalize <long_long_value>
+Normalize the profile. The specified value is the max counter value
+in the new profile.
 
 @item -o @var{directory}
 @itemx --output @var{directory}
@@ -186,10 +187,9 @@ Set the output profile directory. Default output name is @var{rewrite_profile}.
 Scale the profile counters. The specified value can be in floating point value,
 or simple fraction value form, such 1, 2, 2/3, and 5/3.
 
-@item -n @var{long_long_value}
-@itemx --normalize <long_long_value>
-Normalize the profile. The specified value is the max counter value
-in the new profile.
+@item -v
+@itemx --verbose
+Set the verbose mode.
 @end table
 
 @item overlap
@@ -201,14 +201,6 @@ matched counters and p1_sum_all and p2_sum_all are the sum of counter
 values in profile 1 and profile 2, respectively.
 
 @table @gcctabopt
-@item -v
-@itemx --verbose
-Set the verbose mode.
-
-@item -h
-@itemx --hotonly
-Only print info for hot objects/functions.
-
 @item -f
 @itemx --function
 Print function level overlap score.
@@ -217,6 +209,10 @@ Print function level overlap score.
 @itemx --fullname
 Print full gcda filename.
 
+@item -h
+@itemx --hotonly
+Only print info for hot objects/functions.
+
 @item -o
 @itemx --object
 Print object level overlap score.
@@ -224,6 +220,10 @@ Print object level overlap score.
 @item -t @var{float}
 @itemx --hot_threshold <float>
 Set the threshold for hot counter value.
+
+@item -v
+@itemx --verbose
+Set the verbose mode.
 @end table
 
 @end table
diff --git a/gcc/doc/gcov.texi b/gcc/doc/gcov.texi
index 2b4101018a2..1befb5a3e08 100644
--- a/gcc/doc/gcov.texi
+++ b/gcc/doc/gcov.texi
@@ -143,15 +143,6 @@ gpl(7), gfdl(7), fsf-funding(7), gcc(1) and the Info entry for @file{gcc}.
 
 @c man begin OPTIONS
 @table @gcctabopt
-@item -h
-@itemx --help
-Display help about using @command{gcov} (on the standard output), and
-exit without doing any further processing.
-
-@item -v
-@itemx --version
-Display the @command{gcov} version number (on the standard output),
-and exit without doing any further processing.
 
 @item -a
 @itemx --all-blocks
@@ -172,68 +163,18 @@ be shown, unless the @option{-u} option is given.
 Write branch frequencies as the number of branches taken, rather than
 the percentage of branches taken.
 
-@item -n
-@itemx --no-output
-Do not create the @command{gcov} output file.
-
-@item -l
-@itemx --long-file-names
-Create long file names for included source files.  For example, if the
-header file @file{x.h} contains code, and was included in the file
-@file{a.c}, then running @command{gcov} on the file @file{a.c} will
-produce an output file called @file{a.c##x.h.gcov} instead of
-@file{x.h.gcov}.  This can be useful if @file{x.h} is included in
-multiple source files and you want to see the individual
-contributions.  If you use the @samp{-p} option, both the including
-and included file names will be complete path names.
-
-@item -p
-@itemx --preserve-paths
-Preserve complete path information in the names of generated
-@file{.gcov} files.  Without this option, just the filename component is
-used.  With this option, all directories are used, with @samp{/} characters
-translated to @samp{#} characters, @file{.} directory components
-removed and unremoveable @file{..}
-components renamed to @samp{^}.  This is useful if sourcefiles are in several
-different directories.
-
-@item -r
-@itemx --relative-only
-Only output information about source files with a relative pathname
-(after source prefix elision).  Absolute paths are usually system
-header files and coverage of any inline functions therein is normally
-uninteresting.
+@item -d
+@itemx --display-progress
+Display the progress on the standard output.
 
 @item -f
 @itemx --function-summaries
 Output summaries for each function in addition to the file level summary.
 
-@item -o @var{directory|file}
-@itemx --object-directory @var{directory}
-@itemx --object-file @var{file}
-Specify either the directory containing the gcov data files, or the
-object path name.  The @file{.gcno}, and
-@file{.gcda} data files are searched for using this option.  If a directory
-is specified, the data files are in that directory and named after the
-input file name, without its extension.  If a file is specified here,
-the data files are named after that file, without its extension.
-
-@item -s @var{directory}
-@itemx --source-prefix @var{directory}
-A prefix for source file names to remove when generating the output
-coverage files.  This option is useful when building in a separate
-directory, and the pathname to the source directory is not wanted when
-determining the output file names.  Note that this prefix detection is
-applied before determining whether the source file is absolute.
-
-@item -u
-@itemx --unconditional-branches
-When branch probabilities are given, include those of unconditional branches.
-Unconditional branches are normally not interesting.
-
-@item -d
-@itemx --display-progress
-Display the progress on the standard output.
+@item -h
+@itemx --help
+Display help about using @command{gcov} (on the standard output), and
+exit without doing any further processing.
 
 @item -i
 @itemx --intermediate-format
@@ -274,11 +215,75 @@ lcount:26,1
 branch:28,nottaken
 @end smallexample
 
+@item -l
+@itemx --long-file-names
+Create long file names for included source files.  For example, if the
+header file @file{x.h} contains code, and was included in the file
+@file{a.c}, then running @command{gcov} on the file @file{a.c} will
+produce an output file called @file{a.c##x.h.gcov} instead of
+@file{x.h.gcov}.  This can be useful if @file{x.h} is included in
+multiple source files and you want to see the individual
+contributions.  If you use the @samp{-p} option, both the including
+and included file names will be complete path names.
+
 @item -m
 @itemx --demangled-names
 Display demangled function names in output. The default is to show
 mangled function names.
 
+@item -n
+@itemx --no-output
+Do not create the @command{gcov} output file.
+
+@item -o @var{directory|file}
+@itemx --object-directory @var{directory}
+@itemx --object-file @var{file}
+Specify either the directory containing the gcov data files, or the
+object path name.  The @file{.gcno}, and
+@file{.gcda} data files are searched for using this option.  If a directory
+is specified, the data files are in that directory and named after the
+input file name, without its extension.  If a file is specified here,
+the data files are named after that file, without its extension.
+
+@item -p
+@itemx --preserve-paths
+Preserve complete path information in the names of generated
+@file{.gcov} files.  Without this option, just the filename component is
+used.  With this option, all directories are used, with @samp{/} characters
+translated to @samp{#} characters, @file{.} directory components
+removed and unremoveable @file{..}
+components renamed to @samp{^}.  This is useful if sourcefiles are in several
+different directories.
+
+@item -r
+@itemx --relative-only
+Only output information about source files with a relative pathname
+(after source prefix elision).  Absolute paths are usually system
+header files and coverage of any inline functions therein is normally
+uninteresting.
+
+@item -s @var{directory}
+@itemx --source-prefix @var{directory}
+A prefix for source file names to remove when generating the output
+coverage files.  This option is useful when building in a separate
+directory, and the pathname to the source directory is not wanted when
+determining the output file names.  Note that this prefix detection is
+applied before determining whether the source file is absolute.
+
+@item -u
+@itemx --unconditional-branches
+When branch probabilities are given, include those of unconditional branches.
+Unconditional branches are normally not interesting.
+
+@item -v
+@itemx --version
+Display the @command{gcov} version number (on the standard output),
+and exit without doing any further processing.
+
+@item -w
+@itemx --verbose
+Print verbose informations related to basic blocks and arcs.
+
 @item -x
 @itemx --hash-filenames
 By default, gcov uses the full pathname of the source files to to create
@@ -289,10 +294,6 @@ where the @var{source-file} component is the final filename part and
 the @var{md5} component is calculated from the full mangled name that
 would have been used otherwise.
 
-@item -w
-@itemx --verbose
-Print verbose informations related to basic blocks and arcs.
-
 @end table
 
 @command{gcov} should be run with the current directory the same as that
diff --git a/gcc/gcov-dump.c b/gcc/gcov-dump.c
index f2522577e9d..d24e72ac4a1 100644
--- a/gcc/gcov-dump.c
+++ b/gcc/gcov-dump.c
@@ -136,9 +136,9 @@ print_usage (void)
   printf ("Usage: gcov-dump [OPTION] ... gcovfiles\n");
   printf ("Print coverage file contents\n");
   printf ("  -h, --help           Print this help\n");
-  printf ("  -v, --version        Print version number\n");
   printf ("  -l, --long           Dump record contents too\n");
   printf ("  -p, --positions      Dump record positions\n");
+  printf ("  -v, --version        Print version number\n");
   printf ("  -w, --working-sets   Dump working set computed from summary\n");
   printf ("\nFor bug reporting instructions, please see:\n%s.\n",
 	   bug_report_url);
diff --git a/gcc/gcov-tool.c b/gcc/gcov-tool.c
index 80c08354857..74e77b90d72 100644
--- a/gcc/gcov-tool.c
+++ b/gcc/gcov-tool.c
@@ -173,8 +173,8 @@ print_merge_usage_message (int error_p)
   FILE *file = error_p ? stderr : stdout;
 
   fnotice (file, "  merge [options] <dir1> <dir2>         Merge coverage file contents\n");
-  fnotice (file, "    -v, --verbose                       Verbose mode\n");
   fnotice (file, "    -o, --output <dir>                  Output directory\n");
+  fnotice (file, "    -v, --verbose                       Verbose mode\n");
   fnotice (file, "    -w, --weight <w1,w2>                Set weights (float point values)\n");
 }
 
@@ -267,10 +267,10 @@ print_rewrite_usage_message (int error_p)
   FILE *file = error_p ? stderr : stdout;
 
   fnotice (file, "  rewrite [options] <dir>               Rewrite coverage file contents\n");
-  fnotice (file, "    -v, --verbose                       Verbose mode\n");
+  fnotice (file, "    -n, --normalize <int64_t>           Normalize the profile\n");
   fnotice (file, "    -o, --output <dir>                  Output directory\n");
   fnotice (file, "    -s, --scale <float or simple-frac>  Scale the profile counters\n");
-  fnotice (file, "    -n, --normalize <int64_t>           Normalize the profile\n");
+  fnotice (file, "    -v, --verbose                       Verbose mode\n");
 }
 
 static const struct option rewrite_options[] =
@@ -417,12 +417,12 @@ print_overlap_usage_message (int error_p)
   FILE *file = error_p ? stderr : stdout;
 
   fnotice (file, "  overlap [options] <dir1> <dir2>       Compute the overlap of two profiles\n");
-  fnotice (file, "    -v, --verbose                       Verbose mode\n");
-  fnotice (file, "    -h, --hotonly                       Only print info for hot objects/functions\n");
   fnotice (file, "    -f, --function                      Print function level info\n");
   fnotice (file, "    -F, --fullname                      Print full filename\n");
+  fnotice (file, "    -h, --hotonly                       Only print info for hot objects/functions\n");
   fnotice (file, "    -o, --object                        Print object level info\n");
   fnotice (file, "    -t <float>, --hot_threshold <float> Set the threshold for hotness\n");
+  fnotice (file, "    -v, --verbose                       Verbose mode\n");
 
 }
 
diff --git a/gcc/gcov.c b/gcc/gcov.c
index 22378583c5c..2798009e713 100644
--- a/gcc/gcov.c
+++ b/gcc/gcov.c
@@ -679,13 +679,13 @@ print_usage (int error_p)
 
   fnotice (file, "Usage: gcov [OPTION...] SOURCE|OBJ...\n\n");
   fnotice (file, "Print code coverage information.\n\n");
-  fnotice (file, "  -h, --help                      Print this help, then exit\n");
   fnotice (file, "  -a, --all-blocks                Show information for every basic block\n");
   fnotice (file, "  -b, --branch-probabilities      Include branch probabilities in output\n");
   fnotice (file, "  -c, --branch-counts             Output counts of branches taken\n\
                                     rather than percentages\n");
   fnotice (file, "  -d, --display-progress          Display progress information\n");
   fnotice (file, "  -f, --function-summaries        Output summaries for each function\n");
+  fnotice (file, "  -h, --help                      Print this help, then exit\n");
   fnotice (file, "  -i, --intermediate-format       Output .gcov file in intermediate text format\n");
   fnotice (file, "  -l, --long-file-names           Use long output file names for included\n\
                                     source files\n");
-- 
2.12.2

Re: [PATCH 7/8] Sort options of gcov, gcov-dump and gcov-tool both in --help and documentation

[Nathan Sidwell]

On 04/27/2017 05:02 AM, marxin wrote:
> gcc/ChangeLog:
> 
> 2017-04-27  Martin Liska  <mliska@suse.cz>
> 
> 	* doc/gcov.texi: Sort options in alphabetic order.
> 	* doc/gcov-dump.texi: Likewise.
> 	* doc/gcov-tool.texi: Likewise.
> 	* gcov.c (print_usage): Likewise.
> 	* gcov-dump.c (print_usage): Likewise.
> 	* gcov-tool.c (print_merge_usage_message): Likewise.
> 	(print_rewrite_usage_message): Likewise.
> 	(print_overlap_usage_message): Likewise.

ok


-- 
Nathan Sidwell

[PATCH 8/8] Enhance documentation of gcov.

[marxin]

gcc/ChangeLog:

2017-04-27  Martin Liska  <mliska@suse.cz>

	* doc/gcov.texi: Enhance documentation of gcov.
---
 gcc/doc/gcov.texi | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gcc/doc/gcov.texi b/gcc/doc/gcov.texi
index 1befb5a3e08..c96f86df830 100644
--- a/gcc/doc/gcov.texi
+++ b/gcc/doc/gcov.texi
@@ -324,7 +324,9 @@ command line option.  The @var{execution_count} is @samp{-} for lines
 containing no code.  Unexecuted lines are marked @samp{#####} or
 @samp{====}, depending on whether they are reachable by
 non-exceptional paths or only exceptional paths such as C++ exception
-handlers, respectively.
+handlers, respectively. Given @samp{-a} option, unexecuted blocks are
+marked @samp{$$$$$} or @samp{%%%%%}, depending whether a basic block
+is reachable via non-exceptional or exceptional paths.
 
 Some lines of information at the start have @var{line_number} of zero.
 These preamble lines are of the form
@@ -675,5 +677,5 @@ it.  This can be overcome by, for example, setting the environment as
 setting will name the data file @file{/target/run/build/foo.gcda}.
 
 You must move the data files to the expected directory tree in order to
-use them for profile directed optimizations (@option{--use-profile}), or to
+use them for profile directed optimizations (@option{-fprofile-use}), or to
 use the @command{gcov} tool.
-- 
2.12.2

Re: [PATCH 8/8] Enhance documentation of gcov.

[Nathan Sidwell]

On 04/27/2017 05:24 AM, marxin wrote:
> gcc/ChangeLog:
> 
> 2017-04-27  Martin Liska  <mliska@suse.cz>
> 
> 	* doc/gcov.texi: Enhance documentation of gcov.


ok


-- 
Nathan Sidwell

Re: [PATCH 8/8] Enhance documentation of gcov.

[Martin Sebor]

On 04/27/2017 03:24 AM, marxin wrote:
> gcc/ChangeLog:
>
> 2017-04-27  Martin Liska  <mliska@suse.cz>
>
> 	* doc/gcov.texi: Enhance documentation of gcov.
> ---
>  gcc/doc/gcov.texi | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/doc/gcov.texi b/gcc/doc/gcov.texi
> index 1befb5a3e08..c96f86df830 100644
> --- a/gcc/doc/gcov.texi
> +++ b/gcc/doc/gcov.texi
> @@ -324,7 +324,9 @@ command line option.  The @var{execution_count} is @samp{-} for lines
>  containing no code.  Unexecuted lines are marked @samp{#####} or
>  @samp{====}, depending on whether they are reachable by
>  non-exceptional paths or only exceptional paths such as C++ exception
> -handlers, respectively.
> +handlers, respectively. Given @samp{-a} option, unexecuted blocks are
> +marked @samp{$$$$$} or @samp{%%%%%}, depending whether a basic block
> +is reachable via non-exceptional or exceptional paths.

Since I started picking on this change set I might as well keep
at it ;)   Just a tiny nit: the sentence is missing a preposition:

   depending <ins>on </ins> whether a basic block is reachable.

Martin

Re: [PATCH 8/8] Enhance documentation of gcov.

[Gerald Pfeifer]

On Fri, 28 Apr 2017, Martin Sebor wrote:
>> 2017-04-27  Martin Liska  <mliska@suse.cz>
>>
>> 	* doc/gcov.texi: Enhance documentation of gcov.
> Since I started picking on this change set I might as well keep
> at it ;)   Just a tiny nit: the sentence is missing a preposition:
> 
>   depending <ins>on </ins> whether a basic block is reachable.

This adds a bit more.  Applied.

(This section still needs a bit more love, ideally by a native
speaker.)

Gerald


2018-07-22  Gerald Pfeifer  <gerald@pfeifer.com>

	* doc/gcov.texi (Invoking Gcov): Editorial changes.

Index: doc/gcov.texi
===================================================================
--- doc/gcov.texi	(revision 262161)
+++ doc/gcov.texi	(working copy)
@@ -378,12 +378,12 @@
 containing no code.  Unexecuted lines are marked @samp{#####} or
 @samp{=====}, depending on whether they are reachable by
 non-exceptional paths or only exceptional paths such as C++ exception
-handlers, respectively. Given @samp{-a} option, unexecuted blocks are
+handlers, respectively. Given the @samp{-a} option, unexecuted blocks are
 marked @samp{$$$$$} or @samp{%%%%%}, depending on whether a basic block
 is reachable via non-exceptional or exceptional paths.
 Executed basic blocks having a statement with zero @var{execution_count}
-end with @samp{*} character and are colored with magenta color with @option{-k}
-option.  The functionality is not supported in Ada.
+end with @samp{*} character and are colored with magenta color with
+the @option{-k} option.  This functionality is not supported in Ada.
 
 Note that GCC can completely remove the bodies of functions that are
 not needed -- for instance if they are inlined everywhere.  Such functions

[PATCH 3/8] Simplify representation of locations of a block.

[marxin]

gcc/ChangeLog:

2017-04-26  Martin Liska  <mliska@suse.cz>

	* gcov.c (struct block_location_info): New struct.
	(process_file): Fill up the new structure.
	(read_graph_file): Replace usage of encoding by the newly added
	struct.
	(add_line_counts): Likewise.
	(accumulate_line_counts): Remove usage of the union.
	(function_info::function_info): New function.
	(function_info::~function_info): Likewise.
	(process_file): Call delete instead of release_function.
	(release_function): Release the function.
	(release_structures): Call delete instead of release_function.
	(solve_flow_graph): Replace usage of num_blocks.
	(find_exception_blocks): Likewise.
	(output_lines): Fix GNU coding style.
---
 gcc/gcov.c | 253 ++++++++++++++++++++++++++++---------------------------------
 1 file changed, 114 insertions(+), 139 deletions(-)

diff --git a/gcc/gcov.c b/gcc/gcov.c
index 63f6a75f1af..7400cdee110 100644
--- a/gcc/gcov.c
+++ b/gcc/gcov.c
@@ -114,6 +114,16 @@ typedef struct arc_info
   struct arc_info *pred_next;
 } arc_t;
 
+struct block_location_info
+{
+  block_location_info (unsigned _source_file_idx):
+    source_file_idx (_source_file_idx)
+  {}
+
+  unsigned source_file_idx;
+  vector<unsigned> lines;
+};
+
 /* Describes a basic block. Contains lists of arcs to successor and
    predecessor blocks.  */
 
@@ -141,26 +151,16 @@ typedef struct block_info
   /* Block is a landing pad for longjmp or throw.  */
   unsigned is_nonlocal_return : 1;
 
-  union
+  vector<block_location_info> locations;
+
+  struct
   {
-    struct
-    {
-     /* Array of line numbers and source files. source files are
-        introduced by a linenumber of zero, the next 'line number' is
-        the number of the source file.  Always starts with a source
-        file.  */
-      unsigned *encoding;
-      unsigned num;
-    } line; /* Valid until blocks are linked onto lines */
-    struct
-    {
-      /* Single line graph cycle workspace.  Used for all-blocks
-	 mode.  */
-      arc_t *arc;
-      unsigned ident;
-    } cycle; /* Used in all-blocks mode, after blocks are linked onto
-	       lines.  */
-  } u;
+    /* Single line graph cycle workspace.  Used for all-blocks
+       mode.  */
+    arc_t *arc;
+    unsigned ident;
+  } cycle; /* Used in all-blocks mode, after blocks are linked onto
+	     lines.  */
 
   /* Temporary chain for solving graph, and for chaining blocks on one
      line.  */
@@ -172,6 +172,9 @@ typedef struct block_info
 
 typedef struct function_info
 {
+  function_info ();
+  ~function_info ();
+
   /* Name of function.  */
   char *name;
   char *demangled_name;
@@ -186,8 +189,7 @@ typedef struct function_info
      at blocks[0] and the exit block is at blocks[1].  */
 #define ENTRY_BLOCK (0)
 #define EXIT_BLOCK (1)
-  block_t *blocks;
-  unsigned num_blocks;
+  vector<block_t> blocks;
   unsigned blocks_executed;
 
   /* Raw arc coverage counts.  */
@@ -427,9 +429,31 @@ static void output_lines (FILE *, const source_t *);
 static char *make_gcov_file_name (const char *, const char *);
 static char *mangle_name (const char *, char *);
 static void release_structures (void);
-static void release_function (function_t *);
 extern int main (int, char **);
 
+function_info::function_info ()
+{
+  memset (this, 0, sizeof (*this));
+}
+
+function_info::~function_info ()
+{
+  for (int i = blocks.size () - 1; i >= 0; i--)
+    {
+      arc_t *arc, *arc_n;
+
+      for (arc = blocks[i].succ; arc; arc = arc_n)
+	{
+	  arc_n = arc->succ_next;
+	  free (arc);
+	}
+    }
+  free (counts);
+  if (flag_demangled_names && demangled_name != name)
+    free (demangled_name);
+  free (name);
+}
+
 /* Cycle detection!
    There are a bajillion algorithms that do this.  Boost's function is named
    hawick_cycles, so I used the algorithm by K. A. Hawick and H. A. James in
@@ -906,29 +930,26 @@ process_file (const char *file_name)
 	  *prev = fn;
 
 	  /* Mark last line in files touched by function.  */
-	  for (block_no = 0; block_no != fn->num_blocks; block_no++)
+	  for (block_no = 0; block_no != fn->blocks.size (); block_no++)
 	    {
-	      unsigned *enc = fn->blocks[block_no].u.line.encoding;
-	      unsigned num = fn->blocks[block_no].u.line.num;
+	      block_t *block = &fn->blocks[block_no];
+	      for (unsigned i = 0; i < block->locations.size (); i++)
+		{
+		  unsigned s = block->locations[i].source_file_idx;
 
-	      for (; num--; enc++)
-		if (!*enc)
-		  {
-		    if (enc[1] != src)
-		      {
-			if (line >= sources[src].num_lines)
-			  sources[src].num_lines = line + 1;
-			line = 0;
-			src = enc[1];
-		      }
-		    enc++;
-		    num--;
-		  }
-		else if (*enc > line)
-		  line = *enc;
+		  /* Sort lines of locations.  */
+		  sort (block->locations[i].lines.begin (),
+			block->locations[i].lines.end ());
+
+		  if (!block->locations[i].lines.empty ())
+		    {
+		      unsigned last_line
+			= block->locations[i].lines.back () + 1;
+		      if (last_line > sources[s].num_lines)
+			sources[s].num_lines = last_line;
+		    }
+		}
 	    }
-	  if (line >= sources[src].num_lines)
-	    sources[src].num_lines = line + 1;
 
 	  solve_flow_graph (fn);
 	  if (fn->has_catch)
@@ -939,7 +960,7 @@ process_file (const char *file_name)
       else
 	/* The function was not in the executable -- some other
 	   instance must have been selected.  */
-	release_function (fn);
+	delete fn;
     }
 }
 
@@ -1040,31 +1061,6 @@ generate_results (const char *file_name)
     executed_summary (total_lines, total_executed);
 }
 
-/* Release a function structure */
-
-static void
-release_function (function_t *fn)
-{
-  unsigned ix;
-  block_t *block;
-
-  for (ix = fn->num_blocks, block = fn->blocks; ix--; block++)
-    {
-      arc_t *arc, *arc_n;
-
-      for (arc = block->succ; arc; arc = arc_n)
-	{
-	  arc_n = arc->succ_next;
-	  free (arc);
-	}
-    }
-  free (fn->blocks);
-  free (fn->counts);
-  if (flag_demangled_names && fn->demangled_name != fn->name)
-    free (fn->demangled_name);
-  free (fn->name);
-}
-
 /* Release all memory used.  */
 
 static void
@@ -1084,7 +1080,7 @@ release_structures (void)
   while ((fn = functions))
     {
       functions = fn->next;
-      release_function (fn);
+      delete fn;
     }
 }
 
@@ -1298,8 +1294,6 @@ read_graph_file (void)
   function_t *fn = NULL;
   function_t *fns = NULL;
   function_t **fns_end = &fns;
-  unsigned src_idx = 0;
-  unsigned ix;
   unsigned tag;
 
   if (!gcov_open (bbg_file_name, 1))
@@ -1343,10 +1337,10 @@ read_graph_file (void)
 	  lineno_checksum = gcov_read_unsigned ();
 	  cfg_checksum = gcov_read_unsigned ();
 	  function_name = xstrdup (gcov_read_string ());
-	  src_idx = find_source (gcov_read_string ());
+	  unsigned src_idx = find_source (gcov_read_string ());
 	  lineno = gcov_read_unsigned ();
 
-	  fn = XCNEW (function_t);
+	  fn = new function_t;
 	  fn->name = function_name;
 	  if (flag_demangled_names)
 	    {
@@ -1368,14 +1362,11 @@ read_graph_file (void)
 	}
       else if (fn && tag == GCOV_TAG_BLOCKS)
 	{
-	  if (fn->blocks)
+	  if (!fn->blocks.empty ())
 	    fnotice (stderr, "%s:already seen blocks for '%s'\n",
 		     bbg_file_name, fn->name);
 	  else
-	    {
-	      fn->num_blocks = gcov_read_unsigned ();
-	      fn->blocks = XCNEWVEC (block_t, fn->num_blocks);
-	    }
+	    fn->blocks.resize (gcov_read_unsigned ());
 	}
       else if (fn && tag == GCOV_TAG_ARCS)
 	{
@@ -1385,7 +1376,7 @@ read_graph_file (void)
 	  unsigned mark_catches = 0;
 	  struct arc_info *arc;
 
-	  if (src >= fn->num_blocks || fn->blocks[src].succ)
+	  if (src >= fn->blocks.size () || fn->blocks[src].succ)
 	    goto corrupt;
 
 	  while (num_dests--)
@@ -1393,7 +1384,7 @@ read_graph_file (void)
 	      unsigned dest = gcov_read_unsigned ();
 	      unsigned flags = gcov_read_unsigned ();
 
-	      if (dest >= fn->num_blocks)
+	      if (dest >= fn->blocks.size ())
 		goto corrupt;
 	      arc = XCNEW (arc_t);
 
@@ -1454,38 +1445,27 @@ read_graph_file (void)
       else if (fn && tag == GCOV_TAG_LINES)
 	{
 	  unsigned blockno = gcov_read_unsigned ();
-	  unsigned *line_nos = XCNEWVEC (unsigned, length - 1);
+	  block_t *block = &fn->blocks[blockno];
 
-	  if (blockno >= fn->num_blocks || fn->blocks[blockno].u.line.encoding)
+	  if (blockno >= fn->blocks.size ())
 	    goto corrupt;
 
-	  for (ix = 0; ;  )
+	  while (true)
 	    {
 	      unsigned lineno = gcov_read_unsigned ();
 
 	      if (lineno)
-		{
-		  if (!ix)
-		    {
-		      line_nos[ix++] = 0;
-		      line_nos[ix++] = src_idx;
-		    }
-		  line_nos[ix++] = lineno;
-		}
+		block->locations.back ().lines.push_back (lineno);
 	      else
 		{
 		  const char *file_name = gcov_read_string ();
 
 		  if (!file_name)
 		    break;
-		  src_idx = find_source (file_name);
-		  line_nos[ix++] = 0;
-		  line_nos[ix++] = src_idx;
+		  block->locations.push_back (block_location_info
+					      (find_source (file_name)));
 		}
 	    }
-
-	  fn->blocks[blockno].u.line.encoding = line_nos;
-	  fn->blocks[blockno].u.line.num = ix;
 	}
       else if (current_tag && !GCOV_TAG_IS_SUBTAG (current_tag, tag))
 	{
@@ -1643,7 +1623,7 @@ solve_flow_graph (function_t *fn)
   block_t *invalid_blocks = NULL;  /* invalid, but inferable blocks.  */
 
   /* The arcs were built in reverse order.  Fix that now.  */
-  for (ix = fn->num_blocks; ix--;)
+  for (ix = fn->blocks.size (); ix--;)
     {
       arc_t *arc_p, *arc_n;
 
@@ -1664,7 +1644,7 @@ solve_flow_graph (function_t *fn)
       fn->blocks[ix].pred = arc_p;
     }
 
-  if (fn->num_blocks < 2)
+  if (fn->blocks.size () < 2)
     fnotice (stderr, "%s:'%s' lacks entry and/or exit blocks\n",
 	     bbg_file_name, fn->name);
   else
@@ -1688,8 +1668,9 @@ solve_flow_graph (function_t *fn)
 
   /* Propagate the measured counts, this must be done in the same
      order as the code in profile.c  */
-  for (ix = 0, blk = fn->blocks; ix != fn->num_blocks; ix++, blk++)
+  for (unsigned i = 0; i < fn->blocks.size (); i++)
     {
+      blk = &fn->blocks[i];
       block_t const *prev_dst = NULL;
       int out_of_order = 0;
       int non_fake_succ = 0;
@@ -1883,8 +1864,8 @@ solve_flow_graph (function_t *fn)
 
   /* If the graph has been correctly solved, every block will have a
      valid count.  */
-  for (ix = 0; ix < fn->num_blocks; ix++)
-    if (!fn->blocks[ix].count_valid)
+  for (unsigned i = 0; ix < fn->blocks.size (); i++)
+    if (!fn->blocks[i].count_valid)
       {
 	fnotice (stderr, "%s:graph is unsolvable for '%s'\n",
 		 bbg_file_name, fn->name);
@@ -1898,14 +1879,14 @@ static void
 find_exception_blocks (function_t *fn)
 {
   unsigned ix;
-  block_t **queue = XALLOCAVEC (block_t *, fn->num_blocks);
+  block_t **queue = XALLOCAVEC (block_t *, fn->blocks.size ());
 
   /* First mark all blocks as exceptional.  */
-  for (ix = fn->num_blocks; ix--;)
+  for (ix = fn->blocks.size (); ix--;)
     fn->blocks[ix].exceptional = 1;
 
   /* Now mark all the blocks reachable via non-fake edges */
-  queue[0] = fn->blocks;
+  queue[0] = &fn->blocks[0];
   queue[0]->exceptional = 0;
   for (ix = 1; ix;)
     {
@@ -2247,43 +2228,36 @@ add_line_counts (coverage_t *coverage, function_t *fn)
 			  next.  */
 
   /* Scan each basic block.  */
-  for (ix = 0; ix != fn->num_blocks; ix++)
+  for (ix = 0; ix != fn->blocks.size (); ix++)
     {
       block_t *block = &fn->blocks[ix];
-      unsigned *encoding;
-      const source_t *src = NULL;
-      unsigned jx;
-
-      if (block->count && ix && ix + 1 != fn->num_blocks)
+      if (block->count && ix && ix + 1 != fn->blocks.size ())
 	fn->blocks_executed++;
-      for (jx = 0, encoding = block->u.line.encoding;
-	   jx != block->u.line.num; jx++, encoding++)
-	if (!*encoding)
-	  {
-	    src = &sources[*++encoding];
-	    jx++;
-	  }
-	else
-	  {
-	    line = &src->lines[*encoding];
+      for (unsigned i = 0; i < block->locations.size (); i++)
+	{
+	  const source_t *src = &sources[block->locations[i].source_file_idx];
 
-	    if (coverage)
-	      {
-		if (!line->exists)
-		  coverage->lines++;
-		if (!line->count && block->count)
-		  coverage->lines_executed++;
-	      }
-	    line->exists = 1;
-	    if (!block->exceptional)
-	      line->unexceptional = 1;
-	    line->count += block->count;
-	  }
-      free (block->u.line.encoding);
-      block->u.cycle.arc = NULL;
-      block->u.cycle.ident = ~0U;
+	  vector<unsigned> &lines = block->locations[i].lines;
+	  for (unsigned j = 0; j < lines.size (); j++)
+	    {
+	      line = &src->lines[lines[j]];
+	      if (coverage)
+		{
+		  if (!line->exists)
+		    coverage->lines++;
+		  if (!line->count && block->count)
+		    coverage->lines_executed++;
+		}
+	      line->exists = 1;
+	      if (!block->exceptional)
+		line->unexceptional = 1;
+	      line->count += block->count;
+	    }
+	}
+      block->cycle.arc = NULL;
+      block->cycle.ident = ~0U;
 
-      if (!ix || ix + 1 == fn->num_blocks)
+      if (!ix || ix + 1 == fn->blocks.size ())
 	/* Entry or exit block */;
       else if (flag_all_blocks)
 	{
@@ -2363,7 +2337,7 @@ accumulate_line_counts (source_t *src)
 	    {
 	      block_n = block->chain;
 	      block->chain = block_p;
-	      block->u.cycle.ident = ix;
+	      block->cycle.ident = ix;
 	    }
 	  line->u.blocks = block_p;
 
@@ -2533,7 +2507,8 @@ output_lines (FILE *gcov_file, const source_t *src)
 	  fprintf (gcov_file, " returned %s",
 		   format_gcov (return_count, called_count, 0));
 	  fprintf (gcov_file, " blocks executed %s",
-		   format_gcov (fn->blocks_executed, fn->num_blocks - 2, 0));
+		   format_gcov (fn->blocks_executed, fn->blocks.size () - 2,
+				0));
 	  fprintf (gcov_file, "\n");
 	}
 
-- 
2.12.2

Re: [PATCH 3/8] Simplify representation of locations of a block.

[Nathan Sidwell]

On 04/21/2017 10:02 AM, marxin wrote:
> gcc/ChangeLog:
> 
> 2017-04-26  Martin Liska  <mliska@suse.cz>
> 
> 	* gcov.c (struct block_location_info): New struct.
> 	(process_file): Fill up the new structure.
> 	(read_graph_file): Replace usage of encoding by the newly added
> 	struct.
> 	(add_line_counts): Likewise.
> 	(accumulate_line_counts): Remove usage of the union.
> 	(function_info::function_info): New function.
> 	(function_info::~function_info): Likewise.
> 	(process_file): Call delete instead of release_function.
> 	(release_function): Release the function.
> 	(release_structures): Call delete instead of release_function.
> 	(solve_flow_graph): Replace usage of num_blocks.
> 	(find_exception_blocks): Likewise.
> 	(output_lines): Fix GNU coding style.

> diff --git a/gcc/gcov.c b/gcc/gcov.c
> index 63f6a75f1af..7400cdee110 100644
> --- a/gcc/gcov.c
> +++ b/gcc/gcov.c
> @@ -114,6 +114,16 @@ typedef struct arc_info
>     struct arc_info *pred_next;
>   } arc_t;
>   
> +struct block_location_info

/* needs comment.  */



> @@ -427,9 +429,31 @@ static void output_lines (FILE *, const source_t *);
>   static char *make_gcov_file_name (const char *, const char *);
>   static char *mangle_name (const char *, char *);
>   static void release_structures (void);
> -static void release_function (function_t *);
>   extern int main (int, char **);
>   
> +function_info::function_info ()
> +{
> +  memset (this, 0, sizeof (*this));

EW.  ok with a comment about function_info's c++11's PoDness.

ok with those fixed.

nathan
-- 
Nathan Sidwell

Re: [PATCH 3/8] Simplify representation of locations of a block.

[Martin Sebor]

On 04/28/2017 05:47 AM, Nathan Sidwell wrote:
> On 04/21/2017 10:02 AM, marxin wrote:
>> gcc/ChangeLog:
>>
>> 2017-04-26  Martin Liska  <mliska@suse.cz>
>>
>>     * gcov.c (struct block_location_info): New struct.
>>     (process_file): Fill up the new structure.
>>     (read_graph_file): Replace usage of encoding by the newly added
>>     struct.
>>     (add_line_counts): Likewise.
>>     (accumulate_line_counts): Remove usage of the union.
>>     (function_info::function_info): New function.
>>     (function_info::~function_info): Likewise.
>>     (process_file): Call delete instead of release_function.
>>     (release_function): Release the function.
>>     (release_structures): Call delete instead of release_function.
>>     (solve_flow_graph): Replace usage of num_blocks.
>>     (find_exception_blocks): Likewise.
>>     (output_lines): Fix GNU coding style.
>
>> diff --git a/gcc/gcov.c b/gcc/gcov.c
>> index 63f6a75f1af..7400cdee110 100644
>> --- a/gcc/gcov.c
>> +++ b/gcc/gcov.c
>> @@ -114,6 +114,16 @@ typedef struct arc_info
>>     struct arc_info *pred_next;
>>   } arc_t;
>>   +struct block_location_info
>
> /* needs comment.  */
>
>
>
>> @@ -427,9 +429,31 @@ static void output_lines (FILE *, const source_t *);
>>   static char *make_gcov_file_name (const char *, const char *);
>>   static char *mangle_name (const char *, char *);
>>   static void release_structures (void);
>> -static void release_function (function_t *);
>>   extern int main (int, char **);
>>   +function_info::function_info ()
>> +{
>> +  memset (this, 0, sizeof (*this));
>
> EW.  ok with a comment about function_info's c++11's PoDness.

Unless it's some other kind of vector, the patch adds a vector
member to the class, which makes it not a PoD.(*)

In addition, it would make the class ever so slightly safer to
use if it were made non-copyable (by declaring its copy ctor
private).  Otherwise, accidentally creating a copy of an object
of the type could lead to a double free in the newly added dtor.

Martin

[*]  Strictly speaking using memset to initialize pointers to
null isn't guaranteed to work on targets where a null pointer
isn't all bits clear.  I don't know if GCC is meant to build
on such targets but the code would be cleaner (albeit more
verbose) if it defeault-initialized each member in the ctor
initializer list.

Re: [PATCH 3/8] Simplify representation of locations of a block.

[Nathan Sidwell]

On 04/28/2017 12:28 PM, Martin Sebor wrote:

> Unless it's some other kind of vector, the patch adds a vector
> member to the class, which makes it not a PoD.(*)

oh, well double ew.  Your code is bad and you should feel bad.

Write proper member initializers please.

> [*]  Strictly speaking using memset to initialize pointers to
> null isn't guaranteed to work on targets where a null pointer
> isn't all bits clear.  I don't know if GCC is meant to build

Such targets are unuseable.  I know of one that started like that 
(xputers), but eventually had to change so that all-bits zero meant 
NULL.  Too much user code exposes that assumption.

nathan

-- 
Nathan Sidwell

[PATCH] Fix documentation and a ctor in gcov.c

[Martin Liška]

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

On 04/28/2017 07:23 PM, Nathan Sidwell wrote:
> Write proper member initializers please.

Hi.

Done that, patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
I consider the patch as pre-approved.

Martin

[-- Attachment #2: 0001-Fix-documentation-and-a-ctor-in-gcov.c.patch --]
[-- Type: text/x-patch, Size: 1878 bytes --]

From e694ed03b29882bbaaa02747acb188e16d459514 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 2 May 2017 13:38:57 +0200
Subject: [PATCH] Fix documentation and a ctor in gcov.c

gcc/ChangeLog:

2017-05-02  Martin Liska  <mliska@suse.cz>

	* doc/gcov.texi: Add missing preposition.
	* gcov.c (function_info::function_info): Properly fill up
	all member variables.
---
 gcc/doc/gcov.texi | 2 +-
 gcc/gcov.c        | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/gcc/doc/gcov.texi b/gcc/doc/gcov.texi
index c96f86df830..706aa6cf0b0 100644
--- a/gcc/doc/gcov.texi
+++ b/gcc/doc/gcov.texi
@@ -325,7 +325,7 @@ containing no code.  Unexecuted lines are marked @samp{#####} or
 @samp{====}, depending on whether they are reachable by
 non-exceptional paths or only exceptional paths such as C++ exception
 handlers, respectively. Given @samp{-a} option, unexecuted blocks are
-marked @samp{$$$$$} or @samp{%%%%%}, depending whether a basic block
+marked @samp{$$$$$} or @samp{%%%%%}, depending on whether a basic block
 is reachable via non-exceptional or exceptional paths.
 
 Some lines of information at the start have @var{line_number} of zero.
diff --git a/gcc/gcov.c b/gcc/gcov.c
index 4e6771e79d0..a5aa4aadcac 100644
--- a/gcc/gcov.c
+++ b/gcc/gcov.c
@@ -435,10 +435,11 @@ static char *mangle_name (const char *, char *);
 static void release_structures (void);
 extern int main (int, char **);
 
-function_info::function_info ()
+function_info::function_info (): name (NULL), demangled_name (NULL),
+  ident (0), lineno_checksum (0), cfg_checksum (0), has_catch (0),
+  blocks (), blocks_executed (0), counts (NULL), num_counts (0),
+  line (0), src (0), next_file_fn (NULL), next (NULL)
 {
-  /* The type is POD, so that we can use memset.  */
-  memset (this, 0, sizeof (*this));
 }
 
 function_info::~function_info ()
-- 
2.12.2

Re: [PATCH] Fix documentation and a ctor in gcov.c

[Nathan Sidwell]

On 05/02/2017 11:37 AM, Martin Liška wrote:
> On 04/28/2017 07:23 PM, Nathan Sidwell wrote:
>> Write proper member initializers please.
> 
> Hi.
> 
> Done that, patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> I consider the patch as pre-approved.

yes, thanks!


-- 
Nathan Sidwell

Re: [PATCH 3/8] Simplify representation of locations of a block.

[Pedro Alves]

On 04/28/2017 05:28 PM, Martin Sebor wrote:
> On 04/28/2017 05:47 AM, Nathan Sidwell wrote:

>>> @@ -427,9 +429,31 @@ static void output_lines (FILE *, const source_t
>>> *);
>>>   static char *make_gcov_file_name (const char *, const char *);
>>>   static char *mangle_name (const char *, char *);
>>>   static void release_structures (void);
>>> -static void release_function (function_t *);
>>>   extern int main (int, char **);
>>>   +function_info::function_info ()
>>> +{
>>> +  memset (this, 0, sizeof (*this));
>>
>> EW.  ok with a comment about function_info's c++11's PoDness.
> 
> Unless it's some other kind of vector, the patch adds a vector
> member to the class, which makes it not a PoD.(*)

Funny, just this week we added this to gdb to catch such misuses
at compile time:
 https://sourceware.org/ml/gdb-patches/2017-04/msg00378.html
 https://sourceware.org/ml/gdb-patches/2017-04/msg00381.html

Thanks,
Pedro Alves

Re: [PATCH 3/8] Simplify representation of locations of a block.

[Martin Sebor]

On 04/28/2017 01:01 PM, Pedro Alves wrote:
> On 04/28/2017 05:28 PM, Martin Sebor wrote:
>> On 04/28/2017 05:47 AM, Nathan Sidwell wrote:
>
>>>> @@ -427,9 +429,31 @@ static void output_lines (FILE *, const source_t
>>>> *);
>>>>   static char *make_gcov_file_name (const char *, const char *);
>>>>   static char *mangle_name (const char *, char *);
>>>>   static void release_structures (void);
>>>> -static void release_function (function_t *);
>>>>   extern int main (int, char **);
>>>>   +function_info::function_info ()
>>>> +{
>>>> +  memset (this, 0, sizeof (*this));
>>>
>>> EW.  ok with a comment about function_info's c++11's PoDness.
>>
>> Unless it's some other kind of vector, the patch adds a vector
>> member to the class, which makes it not a PoD.(*)
>
> Funny, just this week we added this to gdb to catch such misuses
> at compile time:
>  https://sourceware.org/ml/gdb-patches/2017-04/msg00378.html
>  https://sourceware.org/ml/gdb-patches/2017-04/msg00381.html

It's all too easy to turn a POD into a class and miss that its
objects are still being treated as PODs, especially in a C code
base that's (slowly) transitioning to C++.  It's also easy form
GCC to detect them.  I'm playing with a simple enhancement that
makes GCC warn on these kinds of misuses.

Martin

Re: [PATCH 3/8] Simplify representation of locations of a block.

[Pedro Alves]

On 04/28/2017 08:01 PM, Pedro Alves wrote:
> On 04/28/2017 05:28 PM, Martin Sebor wrote:
>> On 04/28/2017 05:47 AM, Nathan Sidwell wrote:
> 
>>>> @@ -427,9 +429,31 @@ static void output_lines (FILE *, const source_t
>>>> *);
>>>>   static char *make_gcov_file_name (const char *, const char *);
>>>>   static char *mangle_name (const char *, char *);
>>>>   static void release_structures (void);
>>>> -static void release_function (function_t *);
>>>>   extern int main (int, char **);
>>>>   +function_info::function_info ()
>>>> +{
>>>> +  memset (this, 0, sizeof (*this));
>>>
>>> EW.  ok with a comment about function_info's c++11's PoDness.
>>
>> Unless it's some other kind of vector, the patch adds a vector
>> member to the class, which makes it not a PoD.(*)
> 
> Funny, just this week we added this to gdb to catch such misuses
> at compile time:
>  https://sourceware.org/ml/gdb-patches/2017-04/msg00378.html
>  https://sourceware.org/ml/gdb-patches/2017-04/msg00381.html

I think that teaching g++ new warnings for these issues would
be neat, BTW.

Thanks,
Pedro Alves

Re: [PATCH 0/8] GCOV improvements

[Nathan Sidwell]

On 04/28/2017 05:26 AM, marxin wrote:
> Hi.
> 
> Following patch series improves the infrastructure, fixes couple of issues
> and hopefully improves also documentation of the functionality.
> I decided to split the changes into various patches to make it easier
> for reviewers.

thanks for doing this.  I think I've reviewed all of them, but they 
turned up in my inbox this morning with funky dates from the past few days.

nathan

-- 
Nathan Sidwell

Re: [PATCH 0/8] GCOV improvements

[Martin Liška]

On 04/28/2017 01:49 PM, Nathan Sidwell wrote:
> On 04/28/2017 05:26 AM, marxin wrote:
>> Hi.
>>
>> Following patch series improves the infrastructure, fixes couple of issues
>> and hopefully improves also documentation of the functionality.
>> I decided to split the changes into various patches to make it easier
>> for reviewers.
> 
> thanks for doing this.  I think I've reviewed all of them, but they turned up in my inbox this morning with funky dates from the past few days.

Hi.

I thank you for very fast review. Yep, you replied all of them. It's caused by fact that commiter date != author date in git.
Next time, I'll be more patient and eventually run rebase that will modify that ([1]).

I'm planning to include the simple ones (not requesting infrastructure changes) to backport.
What's your opinion about doc changes, should I include that as well?

[1] http://stackoverflow.com/questions/1579643/change-timestamps-while-rebasing-git-branch/7352870#7352870

Martin

> 
> nathan
> 

Re: [PATCH 0/8] GCOV improvements

[Gerald Pfeifer]

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

On Fri, 28 Apr 2017, Martin Liška wrote:
> I'm planning to include the simple ones (not requesting infrastructure 
> changes) to backport. What's your opinion about doc changes, should I 
> include that as well?

Given that GCC 7 is going to be the one used in various environments
for years (and will be the most current release for the next year) I
think that would be good.

Gerald