[PATCH] Fix PR88936

Richard Biener rguenther@suse.de
Fri Apr 12 10:19:00 GMT 2019


The following fixes a IPA PTA miscompilation involving recursion
which means two instances of the same local variables can become
live.  For both testcases we confuse one for the other and cause
DSE to remove a store to the non-local one.

The fix involves adding a "shadow" variable representing all
"other" instances of locals to the IPA points-to solution.
We restrict this treatment to variables reaching their containing
function via parameters and those escaping to globals.

Bootstrap & regtest running on x86_64-unknown-linux-gnu.

Richard.

2019-04-12  Richard Biener  <rguenther@suse.de>

	PR ipa/88936
	* tree-ssa-structalias.c (struct variable_info): Add shadow_var_uid
	member.
	(new_var_info): Initialize it.
	(auto_var_p): New helper.
	(set_uids_in_ptset): Also set the shadow variable uid if required.
	(ipa_pta_execute): Postprocess points-to solutions assigning
	shadow variable uids for locals that may reach their containing
	function recursively.

	* gcc.dg/torture/pr88936-1.c: New testcase.
	* gcc.dg/torture/pr88936-2.c: New testcase.

Index: gcc/tree-ssa-structalias.c
===================================================================
--- gcc/tree-ssa-structalias.c	(revision 270306)
+++ gcc/tree-ssa-structalias.c	(working copy)
@@ -299,6 +299,11 @@ struct variable_info
   /* Full size of the base variable, in bits.  */
   unsigned HOST_WIDE_INT fullsize;
 
+  /* In IPA mode the shadow UID in case the variable needs to be duplicated in
+     the final points-to solution because it reaches its containing
+     function recursively.  Zero if none is needed.  */
+  unsigned int shadow_var_uid;
+
   /* Name of this variable */
   const char *name;
 
@@ -332,6 +337,24 @@ struct obstack final_solutions_obstack;
    Indexed directly by variable info id.  */
 static vec<varinfo_t> varmap;
 
+/* Return whether VAR is an automatic variable.  */
+
+static bool
+auto_var_p (const_tree var)
+{
+  if (VAR_P (var))
+    {
+      tree context = DECL_CONTEXT (var);
+      if (context
+	  && TREE_CODE (context) == FUNCTION_DECL
+	  && ! DECL_EXTERNAL (var)
+	  && ! TREE_STATIC (var))
+	return true;
+    }
+  return false;
+}
+
+
 /* Return the varmap element N */
 
 static inline varinfo_t
@@ -397,6 +420,7 @@ new_var_info (tree t, const char *name,
   ret->solution = BITMAP_ALLOC (&pta_obstack);
   ret->oldsolution = NULL;
   ret->next = 0;
+  ret->shadow_var_uid = 0;
   ret->head = ret->id;
 
   stats.total_vars++;
@@ -6452,6 +6476,16 @@ set_uids_in_ptset (bitmap into, bitmap f
 	      && (TREE_STATIC (vi->decl) || DECL_EXTERNAL (vi->decl))
 	      && ! decl_binds_to_current_def_p (vi->decl))
 	    pt->vars_contains_interposable = true;
+
+	  /* If this is a local variable we can have overlapping lifetime
+	     of different function invocations through recursion duplicate
+	     it with its shadow variable.  */
+	  if (in_ipa_mode
+	      && vi->shadow_var_uid != 0)
+	    {
+	      bitmap_set_bit (into, vi->shadow_var_uid);
+	      pt->vars_contains_nonlocal = true;
+	    }
 	}
 
       else if (TREE_CODE (vi->decl) == FUNCTION_DECL
@@ -8076,6 +8113,62 @@ ipa_pta_execute (void)
   /* From the constraints compute the points-to sets.  */
   solve_constraints ();
 
+  /* Now post-process solutions to handle locals from different
+     runtime instantiations coming in through recursive invocations.  */
+  unsigned shadow_var_cnt = 0;
+  for (unsigned i = 1; i < varmap.length (); ++i)
+    {
+      varinfo_t fi = get_varinfo (i);
+      if (fi->is_fn_info
+	  && fi->decl)
+	/* Automatic variables pointed to by their containing functions
+	   parameters need this treatment.  */
+	for (varinfo_t ai = first_vi_for_offset (fi, fi_parm_base);
+	     ai; ai = vi_next (ai))
+	  {
+	    varinfo_t vi = get_varinfo (find (ai->id));
+	    bitmap_iterator bi;
+	    unsigned j;
+	    EXECUTE_IF_SET_IN_BITMAP (vi->solution, 0, j, bi)
+	      {
+		varinfo_t pt = get_varinfo (j);
+		if (pt->shadow_var_uid == 0
+		    && pt->decl
+		    && auto_var_in_fn_p (pt->decl, fi->decl))
+		  {
+		    pt->shadow_var_uid = allocate_decl_uid ();
+		    shadow_var_cnt++;
+		  }
+	      }
+	  }
+      /* As well as global variables which are another way of passing
+         arguments to recursive invocations.  */
+      else if (fi->is_global_var)
+	{
+	  for (varinfo_t ai = fi; ai; ai = vi_next (ai))
+	    {
+	      varinfo_t vi = get_varinfo (find (ai->id));
+	      bitmap_iterator bi;
+	      unsigned j;
+	      EXECUTE_IF_SET_IN_BITMAP (vi->solution, 0, j, bi)
+		{
+		  varinfo_t pt = get_varinfo (j);
+		  if (pt->shadow_var_uid == 0
+		      && pt->decl
+		      && auto_var_p (pt->decl))
+		    {
+		      pt->shadow_var_uid = allocate_decl_uid ();
+		      shadow_var_cnt++;
+		    }
+		}
+	    }
+	}
+    }
+  if (shadow_var_cnt && dump_file && (dump_flags & TDF_DETAILS))
+    fprintf (dump_file, "Allocated %u shadow variables for locals "
+	     "maybe leaking into recursive invocations of their containing "
+	     "functions\n", shadow_var_cnt);
+
   /* Compute the global points-to sets for ESCAPED.
      ???  Note that the computed escape set is not correct
      for the whole unit as we fail to consider graph edges to
Index: gcc/testsuite/gcc.dg/torture/pr88936-1.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr88936-1.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/torture/pr88936-1.c	(working copy)
@@ -0,0 +1,27 @@
+/* { dg-do run } */
+/* { dg-additional-options "-fipa-pta" } */
+
+static long bug (long depth, long * v)
+{
+  if (depth == 0)
+    {
+      *v = 0;
+      return 1;
+    }
+
+  long r = 1;
+  long val = bug(depth - 1, &r);
+  return 2 * r + val;
+}
+
+static long ff (long depth)
+{
+  return bug(depth, (long*)0);
+}
+
+int main()
+{
+  if (ff(1) != 1)
+    __builtin_abort ();
+  return 0;
+}
Index: gcc/testsuite/gcc.dg/torture/pr88936-2.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr88936-2.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/torture/pr88936-2.c	(working copy)
@@ -0,0 +1,22 @@
+/* { dg-do run } */
+/* { dg-additional-options "-fipa-pta" } */
+
+static int *p;
+void bar(int cnt)
+{
+  int i = 0;
+  if (cnt == 0)
+    {
+      p = &i;
+      bar (1);
+      if (i != 1)
+	__builtin_abort ();
+    }
+  else if (cnt == 1)
+    *p = 1;
+}
+int main()
+{
+  bar (0);
+  return 0;
+}



More information about the Gcc-patches mailing list