Fix buffer overflow when handling mismatched profiles

Jan Hubicka hubicka@ucw.cz
Mon Dec 17 13:25:00 GMT 2018


Hi,
building Firefox with FDO and trunk reproduces another problem where we
ICE in streamer on histogram counter being negative when it should not.
I will turn this ICE into profile verification error because it may
happen when profile collection was corrupted, but in this case it is
caused by fact that we expect more counters at profile-use time then we
produced at profile-generate time.  I am not sure why this happens yet,
but this leads to buffer overrun and we pick random garbage into the
statement histogram.

I am not sure who was writting get_coverage_counts but it was optimistic
person, since he/she dropped the test that numbers match because both
users already compute them for no use :)

Bootstrapped/regtested x86_64-linux, commited to trunk and I plan to
backport it to gcc8 after SPEC FDO tests passes.

Honza

	* coverage.c (struct conts_entry): Add n_counts.
	(remap_counts_file): Record number of ocunts.
	(get_coverage_counts): Verify that counts match.
	* coverage.h (get_coverage_counts): Update prototype.
	* profile.c (get_exec_counts. compute_value_histograms): Add
	n_counts parametrs.

Index: coverage.c
===================================================================
--- coverage.c	(revision 267185)
+++ coverage.c	(working copy)
@@ -74,6 +74,7 @@ struct counts_entry : pointer_hash <coun
   unsigned lineno_checksum;
   unsigned cfg_checksum;
   gcov_type *counts;
+  unsigned n_counts;
 
   /* hash_table support.  */
   static inline hashval_t hash (const counts_entry *);
@@ -260,6 +261,7 @@ read_counts_file (void)
 	      entry->lineno_checksum = lineno_checksum;
 	      entry->cfg_checksum = cfg_checksum;
 	      entry->counts = XCNEWVEC (gcov_type, n_counts);
+	      entry->n_counts = n_counts;
 	    }
 	  else if (entry->lineno_checksum != lineno_checksum
 		   || entry->cfg_checksum != cfg_checksum)
@@ -295,7 +297,7 @@ read_counts_file (void)
 
 gcov_type *
 get_coverage_counts (unsigned counter, unsigned cfg_checksum,
-		     unsigned lineno_checksum)
+		     unsigned lineno_checksum, unsigned int n_counts)
 {
   counts_entry *entry, elt;
 
@@ -344,17 +346,27 @@ get_coverage_counts (unsigned counter, u
       return NULL;
     }
   
-  if (entry->cfg_checksum != cfg_checksum)
+  if (entry->cfg_checksum != cfg_checksum || entry->n_counts != n_counts)
     {
       static int warned = 0;
       bool warning_printed = false;
 
-      warning_printed =
-	warning_at (DECL_SOURCE_LOCATION (current_function_decl),
-		    OPT_Wcoverage_mismatch,
-		    "the control flow of function %qD does not match "
-		    "its profile data (counter %qs)", current_function_decl,
-		    ctr_names[counter]);
+      if (entry->n_counts != n_counts)
+	warning_printed =
+	  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 %qs, expected %i and have %i)",
+		      current_function_decl,
+		      ctr_names[counter], entry->n_counts, n_counts);
+      else
+	warning_printed =
+	  warning_at (DECL_SOURCE_LOCATION (current_function_decl),
+		      OPT_Wcoverage_mismatch,
+		      "the control flow of function %qD does not match "
+		      "its profile data (counter %qs)", current_function_decl,
+		      ctr_names[counter]);
       if (warning_printed && dump_enabled_p ())
 	{
 	  dump_user_location_t loc
Index: coverage.h
===================================================================
--- coverage.h	(revision 267185)
+++ coverage.h	(working copy)
@@ -52,7 +52,8 @@ extern tree tree_coverage_counter_addr (
 /* Get all the counters for the current function.  */
 extern gcov_type *get_coverage_counts (unsigned /*counter*/,
 				       unsigned /*cfg_checksum*/,
-				       unsigned /*lineno_checksum*/);
+				       unsigned /*lineno_checksum*/,
+				       unsigned /*n_counts*/);
 
 extern tree get_gcov_type (void);
 extern bool coverage_node_map_initialized_p (void);
Index: profile.c
===================================================================
--- profile.c	(revision 267185)
+++ profile.c	(working copy)
@@ -218,7 +218,7 @@ get_exec_counts (unsigned cfg_checksum,
     }
 
   counts = get_coverage_counts (GCOV_COUNTER_ARCS, cfg_checksum,
-				lineno_checksum);
+				lineno_checksum, num_edges);
   if (!counts)
     return NULL;
 
@@ -780,7 +780,8 @@ compute_value_histograms (histogram_valu
 
       histogram_counts[t] = get_coverage_counts (COUNTER_FOR_HIST_TYPE (t),
 						 cfg_checksum,
-						 lineno_checksum);
+						 lineno_checksum,
+						 n_histogram_counters[t]);
       if (histogram_counts[t])
 	any = 1;
       act_count[t] = histogram_counts[t];



More information about the Gcc-patches mailing list