[Bug gcov-profile/103652] New: Producing profile with -O2 -flto and trying to consume it with -O3 -flto leads to ICEs on indirect call profiling

[Bug gcov-profile/103652] New: Producing profile with -O2 -flto and trying to consume it with -O3 -flto leads to ICEs on indirect call profiling

[hubicka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103652

            Bug ID: 103652
           Summary: Producing profile with -O2 -flto and trying to consume
                    it with -O3 -flto leads to ICEs on indirect call
                    profiling
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: gcov-profile
          Assignee: unassigned at gcc dot gnu.org
          Reporter: hubicka at gcc dot gnu.org
                CC: marxin at gcc dot gnu.org
  Target Milestone: ---

Building clang in the funny way (training with -O2 -flto -fprofile-generate)
and use with -O3 -flto -fprofile-generate I get ICE here:

Program received signal SIGSEGV, Segmentation fault.
compute_value_histograms (lineno_checksum=2372477078, cfg_checksum=4074682759,
values=...) at ../../gcc/profile.c:931
931                   hist->hvalue.counters[j] = act_count[t][j];
(gdb) l
926               gimple_add_histogram_value (cfun, stmt, hist);
927               hist->n_counters = total_size;
928               hist->hvalue.counters = XNEWVEC (gcov_type,
hist->n_counters);
929               for (j = 0; j < hist->n_counters; j++)
930                 if (act_count[t])
931                   hist->hvalue.counters[j] = act_count[t][j];
932                 else
933                   hist->hvalue.counters[j] = 0;
934               act_count[t] += hist->n_counters;
935               sort_hist_values (hist);
(gdb) p hist
$1 = (histogram_value) 0x21e9d40
(gdb) p *hist
$2 = {hvalue = {value = 0x7fffea4b9e10, stmt = 0x7fffea4a4ab0, counters =
0x7ffdf13db010, next = 0x0}, type = HIST_TYPE_INDIR_CALL, n_counters =
1059049550, fun = 0x7fffed96ce40, hdata = {intvl = {int_start = 0, steps = 0}}}
(gdb) p hist->n_counters
$6 = 1059049550

and I also get ICE:

/home/jh/llvm-project/clang/lib/AST/ASTContext.cpp: At top level:
/home/jh/llvm-project/clang/lib/AST/ASTContext.cpp:11856:1: internal compiler
error: in stream_out_histogram_value, at value-prof.c:340


I think it is mismatched profile but I do not know - certainly the streaming
needs more sanity checks here....

[Bug gcov-profile/103652] Producing profile with -O2 -flto and trying to consume it with -O3 -flto leads to ICEs on indirect call profiling

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103652

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to Jan Hubicka from comment #0)
> Building clang in the funny way (training with -O2 -flto -fprofile-generate)
> and use with -O3 -flto -fprofile-generate I get ICE here:

Do you mean, -O3 -flto -fprofile-use, right?

I would have expected some -Werror=coverage-mismatch errors. Can you please
create a smaller reproducer where you see the error?

[Bug gcov-profile/103652] Producing profile with -O2 -flto and trying to consume it with -O3 -flto leads to ICEs on indirect call profiling

[hubicka at kam dot mff.cuni.cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103652

--- Comment #2 from hubicka at kam dot mff.cuni.cz ---
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103652
> 
> --- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
> (In reply to Jan Hubicka from comment #0)
> > Building clang in the funny way (training with -O2 -flto -fprofile-generate)
> > and use with -O3 -flto -fprofile-generate I get ICE here:
> 
> Do you mean, -O3 -flto -fprofile-use, right?
> 
> I would have expected some -Werror=coverage-mismatch errors. Can you please
> create a smaller reproducer where you see the error?
You need to disable those to build multithreaded programs like clang.  I
think you can produce testcase easily by making a function with one
indirect call for train run and many indirect calls in profile-use run.

I have patch to avoid the buffer overflow - can send it after getting to
office.

[Bug gcov-profile/103652] Producing profile with -O2 -flto and trying to consume it with -O3 -flto leads to ICEs on indirect call profiling

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103652

--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
> You need to disable those to build multithreaded programs like clang.

Well, I'm specifically speaking about:
error: the control flow of function ‘BZ2_compressBlock’ does not match its
profile data (counter ‘arcs’) 

this type of errors should not happen even in a multi-threaded programs.

> I think you can produce testcase easily by making a function with one
> indirect call for train run and many indirect calls in profile-use run.
> 
> I have patch to avoid the buffer overflow - can send it after getting to
> office.

Sure, please send it.

Re: [Bug gcov-profile/103652] Producing profile with -O2 -flto and trying to consume it with -O3 -flto leads to ICEs on indirect call profiling

[Jan Hubicka]

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

> 
> Well, I'm specifically speaking about:
> error: the control flow of function ‘BZ2_compressBlock’ does not match its
> profile data (counter ‘arcs’) 
> 
> this type of errors should not happen even in a multi-threaded programs.

There are some cases where I see even those on clang build - I am not
sure how that happens (if it is configury difference or generated code
or gcc bug) It is on my TODO to analyse...

In any case we should never ICE on malformed gcda files. Especially not
by buffer overflow :)
> 
> > I think you can produce testcase easily by making a function with one
> > indirect call for train run and many indirect calls in profile-use run.
> > 
> > I have patch to avoid the buffer overflow - can send it after getting to
> > office.
> 
> Sure, please send it.
Attached.

Honza

[-- Attachment #2: buffer --]
[-- Type: text/plain, Size: 4796 bytes --]

diff --git a/gcc/coverage.c b/gcc/coverage.c
index 7f8b532cb52..49c370cb8c8 100644
--- a/gcc/coverage.c
+++ b/gcc/coverage.c
@@ -296,7 +296,7 @@ read_counts_file (void)
 
 gcov_type *
 get_coverage_counts (unsigned counter, unsigned cfg_checksum,
-		     unsigned lineno_checksum, unsigned int n_counts)
+		     unsigned lineno_checksum, unsigned int *n_counts)
 {
   counts_entry *entry, elt;
 
@@ -348,12 +348,12 @@ get_coverage_counts (unsigned counter, unsigned cfg_checksum,
   if (entry->cfg_checksum != cfg_checksum
       || (counter != GCOV_COUNTER_V_INDIR
 	  && counter != GCOV_COUNTER_V_TOPN
-	  && entry->n_counts != n_counts))
+	  && entry->n_counts != *n_counts))
     {
       static int warned = 0;
       bool warning_printed = false;
 
-      if (entry->n_counts != n_counts)
+      if (entry->n_counts != *n_counts)
 	warning_printed =
 	  warning_at (DECL_SOURCE_LOCATION (current_function_decl),
 		      OPT_Wcoverage_mismatch,
@@ -361,7 +361,7 @@ get_coverage_counts (unsigned counter, unsigned cfg_checksum,
 		      "does not match "
 		      "its profile data (counter %qs, expected %i and have %i)",
 		      current_function_decl,
-		      ctr_names[counter], entry->n_counts, n_counts);
+		      ctr_names[counter], entry->n_counts, *n_counts);
       else
 	warning_printed =
 	  warning_at (DECL_SOURCE_LOCATION (current_function_decl),
@@ -404,9 +404,25 @@ get_coverage_counts (unsigned counter, unsigned cfg_checksum,
 		  current_function_decl);
     }
 
+  *n_counts = entry->n_counts;
   return entry->counts;
 }
 
+/* Returns the counters for a particular tag and verifies that counts matches
+   the expectation.  */
+
+gcov_type *
+get_coverage_counts (unsigned counter, unsigned cfg_checksum,
+		     unsigned lineno_checksum, unsigned int n_counts)
+{
+  unsigned int n_counts2 = n_counts;
+  gcov_type *ret
+	  = get_coverage_counts (counter, cfg_checksum,
+				 lineno_checksum, &n_counts2);
+  gcc_assert (!ret || n_counts2 == n_counts);
+  return ret;
+}
+
 /* Allocate NUM counters of type COUNTER. Returns nonzero if the
    allocation succeeded.  */
 
diff --git a/gcc/coverage.h b/gcc/coverage.h
index 22646d439fc..7f488811a4e 100644
--- a/gcc/coverage.h
+++ b/gcc/coverage.h
@@ -54,6 +54,10 @@ extern gcov_type *get_coverage_counts (unsigned /*counter*/,
 				       unsigned /*cfg_checksum*/,
 				       unsigned /*lineno_checksum*/,
 				       unsigned /*n_counts*/);
+extern gcov_type *get_coverage_counts (unsigned /*counter*/,
+				       unsigned /*cfg_checksum*/,
+				       unsigned /*lineno_checksum*/,
+				       unsigned */*n_counts*/);
 
 extern tree get_gcov_type (void);
 extern bool coverage_node_map_initialized_p (void);
diff --git a/gcc/profile.c b/gcc/profile.c
index d4103058fcd..0fe0910c296 100644
--- a/gcc/profile.c
+++ b/gcc/profile.c
@@ -898,7 +898,7 @@ compute_value_histograms (histogram_values values, unsigned cfg_checksum,
       histogram_counts[t] = get_coverage_counts (COUNTER_FOR_HIST_TYPE (t),
 						 cfg_checksum,
 						 lineno_checksum,
-						 n_histogram_counters[t]);
+						 &n_histogram_counters[t]);
       if (histogram_counts[t])
 	any = 1;
       act_count[t] = histogram_counts[t];
@@ -918,20 +918,47 @@ compute_value_histograms (histogram_values values, unsigned cfg_checksum,
       /* TOP N counter uses variable number of counters.  */
       if (topn_p)
 	{
-	  unsigned total_size;
+	  gcov_type total_size;
+	  bool ignore = false;
 	  if (act_count[t])
-	    total_size = 2 + 2 * act_count[t][1];
+	    {
+	      total_size = 2 + 2 * act_count[t][1];
+	      /* Watch for counter corruption
+		 and possible memory overflows.  */
+	      if (total_size < 2
+		  || total_size > n_histogram_counters [t])
+		{
+		  warning_at (DECL_SOURCE_LOCATION (current_function_decl),
+			      OPT_Wcoverage_mismatch,
+			      "number of counters in profile data for function %qD "
+			      "does not match "
+			      "its profile data (counter %s)",
+			      current_function_decl,
+			      hist->type == HIST_TYPE_TOPN_VALUES
+			      ? "topn" : "indir_call");
+		  total_size = 2;
+		  ignore = true;
+		  act_count[t] = NULL;
+		}
+	    }
 	  else
-	    total_size = 2;
+	    {
+	      total_size = 2;
+	      ignore = true;
+	    }
 	  gimple_add_histogram_value (cfun, stmt, hist);
 	  hist->n_counters = total_size;
 	  hist->hvalue.counters = XNEWVEC (gcov_type, hist->n_counters);
 	  for (j = 0; j < hist->n_counters; j++)
-	    if (act_count[t])
+	    if (!ignore)
 	      hist->hvalue.counters[j] = act_count[t][j];
 	    else
 	      hist->hvalue.counters[j] = 0;
-	  act_count[t] += hist->n_counters;
+	  if (!ignore)
+	    {
+	      act_count[t] += hist->n_counters;
+	      n_histogram_counters [t] -= hist->n_counters;
+	    }
 	  sort_hist_values (hist);
 	}
       else

[Bug gcov-profile/103652] Producing profile with -O2 -flto and trying to consume it with -O3 -flto leads to ICEs on indirect call profiling

[hubicka at kam dot mff.cuni.cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103652

--- Comment #4 from hubicka at kam dot mff.cuni.cz ---
> 
> Well, I'm specifically speaking about:
> error: the control flow of function ‘BZ2_compressBlock’ does not match its
> profile data (counter ‘arcs’) 
> 
> this type of errors should not happen even in a multi-threaded programs.

There are some cases where I see even those on clang build - I am not
sure how that happens (if it is configury difference or generated code
or gcc bug) It is on my TODO to analyse...

In any case we should never ICE on malformed gcda files. Especially not
by buffer overflow :)
> 
> > I think you can produce testcase easily by making a function with one
> > indirect call for train run and many indirect calls in profile-use run.
> > 
> > I have patch to avoid the buffer overflow - can send it after getting to
> > office.
> 
> Sure, please send it.
Attached.

Honza

[Bug gcov-profile/103652] Producing profile with -O2 -flto and trying to consume it with -O3 -flto leads to ICEs on indirect call profiling

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103652

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2021-12-13
     Ever confirmed|0                           |1

--- Comment #5 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to hubicka from comment #4)
> Created attachment 51988 [details]
> buffer
> 

The patch is fine, please test it and install it.