Fix WPA corruption seen in LTO bootstrap

Fix WPA corruption seen in LTO bootstrap

[Jan Hubicka]

Hi,
the problems with LTO build reported by Toon is caused by a hack disabling
jump function in ipa-prop on wpa.  The hack is no longer needed and it is wrong,
since jump function makes no sense when they are not updated. Consequentely
inline cost metrics get lost.

I am testing the following patch that also fortifies the datastructures
for array overruns (this was how the problem manifested itself).

Bootstrapping/regtesting x86_64-linux, will commit it if passes.

Honza

	* ipa-prop.c (ipa_propagate_indirect_call_infos): Remove obsolette
	WPA hack.
	* ipa-prop.h (ipa_get_param, ipa_is_param_used, ipa_param_cannot_devirtualize_p,
	ipa_param_types_vec_empty, ipa_get_ith_jump_func, ipa_get_lattice):
	Fortify array bounds.
	* ipa-inline-analysis.c (add_clause): Fix clause ordering.
	(and_predicates, or_predicates, predicates_equal_p, evaulate_predicate):
	Sanity check predicate length.
	(remap_predicate): Likewise; sanity check jump functions.
	(inline_read_section, inline_write_summary): Sanity check
	predicate length.
Index: ipa-prop.c
===================================================================
--- ipa-prop.c	(revision 172898)
+++ ipa-prop.c	(working copy)
@@ -1890,10 +1890,6 @@ bool
 ipa_propagate_indirect_call_infos (struct cgraph_edge *cs,
 				   VEC (cgraph_edge_p, heap) **new_edges)
 {
-  /* FIXME lto: We do not stream out indirect call information.  */
-  if (flag_wpa)
-    return false;
-
   /* Do nothing if the preparation phase has not been carried out yet
      (i.e. during early inlining).  */
   if (!ipa_node_params_vector)
Index: ipa-prop.h
===================================================================
--- ipa-prop.h	(revision 172898)
+++ ipa-prop.h	(working copy)
@@ -228,6 +228,7 @@ ipa_get_param_count (struct ipa_node_par
 static inline tree
 ipa_get_param (struct ipa_node_params *info, int i)
 {
+  gcc_assert (i >= 0 && i <= info->param_count);
   return info->params[i].decl;
 }
 
@@ -237,6 +238,7 @@ ipa_get_param (struct ipa_node_params *i
 static inline bool
 ipa_is_param_used (struct ipa_node_params *info, int i)
 {
+  gcc_assert (i >= 0 && i <= info->param_count);
   return info->params[i].used;
 }
 
@@ -247,6 +249,7 @@ ipa_is_param_used (struct ipa_node_param
 static inline bool
 ipa_param_cannot_devirtualize_p (struct ipa_node_params *info, int i)
 {
+  gcc_assert (i >= 0 && i <= info->param_count);
   return info->params[i].cannot_devirtualize;
 }
 
@@ -256,6 +259,7 @@ ipa_param_cannot_devirtualize_p (struct
 static inline bool
 ipa_param_types_vec_empty (struct ipa_node_params *info, int i)
 {
+  gcc_assert (i >= 0 && i <= info->param_count);
   return info->params[i].types == NULL;
 }
 
@@ -315,6 +319,7 @@ ipa_get_cs_argument_count (struct ipa_ed
 static inline struct ipa_jump_func *
 ipa_get_ith_jump_func (struct ipa_edge_args *args, int i)
 {
+  gcc_assert (i >= 0 && i <= args->argument_count);
   return &args->jump_functions[i];
 }
 
@@ -528,6 +533,7 @@ tree build_ref_for_offset (location_t, t
 static inline struct ipcp_lattice *
 ipa_get_lattice (struct ipa_node_params *info, int i)
 {
+  gcc_assert (i >= 0 && i <= info->param_count);
   return &(info->params[i].ipcp_lattice);
 }
 
Index: ipa-inline-analysis.c
===================================================================
--- ipa-inline-analysis.c	(revision 172898)
+++ ipa-inline-analysis.c	(working copy)
@@ -200,7 +200,7 @@ static inline void
 add_clause (struct predicate *p, clause_t clause)
 {
   int i;
-  int insert_here = 0;
+  int insert_here = -1;
   /* True clause.  */
   if (!clause)
     return;
@@ -211,7 +211,7 @@ add_clause (struct predicate *p, clause_
       p->clause[0] = (1 << predicate_false_condition);
       p->clause[1] = 0;
     }
-  for (i = 0; i < MAX_CLAUSES; i++)
+  for (i = 0; i < MAX_CLAUSES - 1; i++)
     {
       if (p->clause[i] == clause)
         return;
@@ -225,8 +225,11 @@ add_clause (struct predicate *p, clause_
     return;
   /* Keep clauses ordered by index, so equivalence testing is easy.  */
   p->clause[i + 1] = 0;
-  for (;i > insert_here; i--)
-    p->clause[i] = p->clause[i - 1];
+  if (insert_here >= 0)
+    for (;i > insert_here; i--)
+      p->clause[i] = p->clause[i - 1];
+  else
+    insert_here = i;
   p->clause[insert_here] = clause;
 }
 
@@ -239,7 +242,10 @@ and_predicates (struct predicate *p, str
   struct predicate out = *p;
   int i;
   for (i = 0; p2->clause[i]; i++)
-    add_clause (&out, p2->clause[i]);
+    {
+      gcc_checking_assert (i < MAX_CLAUSES);
+      add_clause (&out, p2->clause[i]);
+    }
   return out;
 }
 
@@ -264,7 +270,10 @@ or_predicates (struct predicate *p, stru
     }
   for (i = 0; p->clause[i]; i++)
     for (j = 0; p2->clause[j]; j++)
-      add_clause (&out, p->clause[i] | p2->clause[j]);
+      {
+        gcc_checking_assert (i < MAX_CLAUSES && j < MAX_CLAUSES);
+        add_clause (&out, p->clause[i] | p2->clause[j]);
+      }
   return out;
 }
 
@@ -276,8 +285,11 @@ predicates_equal_p (struct predicate *p,
 {
   int i;
   for (i = 0; p->clause[i]; i++)
-    if (p->clause[i] != p2->clause[i])
-      return false;
+    {
+      gcc_checking_assert (i < MAX_CLAUSES);
+      if (p->clause[i] != p2->clause[i])
+        return false;
+    }
   return !p2->clause[i];
 }
 
@@ -296,8 +308,11 @@ evaulate_predicate (struct predicate *p,
 
   /* See if we can find clause we can disprove.  */
   for (i = 0; p->clause[i]; i++)
-    if (!(p->clause[i] & possible_truths))
-      return false;
+    {
+      gcc_checking_assert (i < MAX_CLAUSES);
+      if (!(p->clause[i] & possible_truths))
+        return false;
+    }
   return true;
 }
 
@@ -1166,6 +1181,8 @@ remap_predicate (struct inline_summary *
       int cond;
       struct predicate clause_predicate = false_predicate ();
 
+      gcc_assert (i < MAX_CLAUSES);
+
       for (cond = 0; cond < NUM_CONDITIONS; cond ++)
 	/* Do we have condition we can't disprove?   */
 	if (clause & possible_truths & (1 << cond))
@@ -1240,6 +1257,7 @@ inline_merge_summary (struct cgraph_edge
 	      && jfunc->value.pass_through.operation == NOP_EXPR)
 	    map = jfunc->value.pass_through.formal_id;
 	  VEC_replace (int, operand_map, i, map);
+	  gcc_assert (map < ipa_get_param_count (IPA_NODE_REF (to)));
 	}
     }
   for (i = 0; VEC_iterate (size_time_entry, callee_info->entry, i, e); i++)
@@ -1544,6 +1562,7 @@ inline_read_section (struct lto_file_dec
 	  do 
 	    {
 	      clause = e.predicate.clause[k++] = lto_input_uleb128 (&ib);
+	      gcc_assert (k < MAX_CLAUSES);
 	    }
 	  while (clause);
 
@@ -1658,8 +1677,11 @@ inline_write_summary (cgraph_node_set se
 	      lto_output_uleb128_stream (ob->main_stream,
 					 e->time);
 	      for (j = 0; e->predicate.clause[j]; j++)
-		lto_output_uleb128_stream (ob->main_stream,
-					   e->predicate.clause[j]);
+		{
+		   gcc_assert (j < MAX_CLAUSES);
+		   lto_output_uleb128_stream (ob->main_stream,
+					      e->predicate.clause[j]);
+		}
 	      lto_output_uleb128_stream (ob->main_stream, 0);
 	    }
 	}

Re: Fix WPA corruption seen in LTO bootstrap

[Jan Hubicka]

> Hi,
> the problems with LTO build reported by Toon is caused by a hack disabling
> jump function in ipa-prop on wpa.  The hack is no longer needed and it is wrong,
> since jump function makes no sense when they are not updated. Consequentely
> inline cost metrics get lost.
> 
> I am testing the following patch that also fortifies the datastructures
> for array overruns (this was how the problem manifested itself).
> 
> Bootstrapping/regtesting x86_64-linux, will commit it if passes.
> 
> Honza
> 
> 	* ipa-prop.c (ipa_propagate_indirect_call_infos): Remove obsolette
> 	WPA hack.

Martin,
since indirect inlining at WPA time is enabled for 4.6, I think it is just matter
of testcase showing that the fix is needed for 4.6, too (i.e. arrange function A
to be inlined to B that passes through callback but use different formal ID to do so.)

Honza

Re: Fix WPA corruption seen in LTO bootstrap

[H.J. Lu]

On Sat, Apr 23, 2011 at 6:33 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> the problems with LTO build reported by Toon is caused by a hack disabling
> jump function in ipa-prop on wpa.  The hack is no longer needed and it is wrong,
> since jump function makes no sense when they are not updated. Consequentely
> inline cost metrics get lost.
>
> I am testing the following patch that also fortifies the datastructures
> for array overruns (this was how the problem manifested itself).
>
> Bootstrapping/regtesting x86_64-linux, will commit it if passes.
>
> Honza
>
>        * ipa-prop.c (ipa_propagate_indirect_call_infos): Remove obsolette
>        WPA hack.
>        * ipa-prop.h (ipa_get_param, ipa_is_param_used, ipa_param_cannot_devirtualize_p,
>        ipa_param_types_vec_empty, ipa_get_ith_jump_func, ipa_get_lattice):
>        Fortify array bounds.
>        * ipa-inline-analysis.c (add_clause): Fix clause ordering.
>        (and_predicates, or_predicates, predicates_equal_p, evaulate_predicate):
>        Sanity check predicate length.
>        (remap_predicate): Likewise; sanity check jump functions.
>        (inline_read_section, inline_write_summary): Sanity check
>        predicate length.

This caused:

http://gcc.gnu.org/ml/gcc-cvs/2011-04/msg01110.html

-- 
H.J.

Re: Fix WPA corruption seen in LTO bootstrap

[H.J. Lu]

On Tue, May 10, 2011 at 8:12 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sat, Apr 23, 2011 at 6:33 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> Hi,
>> the problems with LTO build reported by Toon is caused by a hack disabling
>> jump function in ipa-prop on wpa.  The hack is no longer needed and it is wrong,
>> since jump function makes no sense when they are not updated. Consequentely
>> inline cost metrics get lost.
>>
>> I am testing the following patch that also fortifies the datastructures
>> for array overruns (this was how the problem manifested itself).
>>
>> Bootstrapping/regtesting x86_64-linux, will commit it if passes.
>>
>> Honza
>>
>>        * ipa-prop.c (ipa_propagate_indirect_call_infos): Remove obsolette
>>        WPA hack.
>>        * ipa-prop.h (ipa_get_param, ipa_is_param_used, ipa_param_cannot_devirtualize_p,
>>        ipa_param_types_vec_empty, ipa_get_ith_jump_func, ipa_get_lattice):
>>        Fortify array bounds.
>>        * ipa-inline-analysis.c (add_clause): Fix clause ordering.
>>        (and_predicates, or_predicates, predicates_equal_p, evaulate_predicate):
>>        Sanity check predicate length.
>>        (remap_predicate): Likewise; sanity check jump functions.
>>        (inline_read_section, inline_write_summary): Sanity check
>>        predicate length.
>
> This caused:
>
> http://gcc.gnu.org/ml/gcc-cvs/2011-04/msg01110.html
>

I meant:


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48954



-- 
H.J.