[PATCH] Fix type field walking in gimplifier unsharing

[Richard Biener]

Gimplification is done in-place and thus relies on all processed
trees being unshared.  This is achieved by unshare_body which
in the end uses walk_tree to get at all interesting trees that possibly
need unsharing.

Unfortunately it doesn't really work because walk_tree only walks
types and type-related fields (TYPE_SIZE, TYPE_MIN_VALUE, etc.) in
very narrow circumstances.

The symptom of failing to unshare trees used in those fields and
in the IL of a function body that is gimplified is that those
trees get modified by the gimplification process which basically
leaks temporary decls into them.

Note that only type sizes that are actually needed for the IL
of a function are gimplified and thus there are referenced
types with still save-expr TYPE_SIZE after gimplification.

Eventually dwarf2out knows to emit debug info for those by
generating dwarf expressions but will be surely not able to do
so if the expressions contain random local decls that may no
longer be there.

With my patch to make the gimplifier use SSA names for temporaries
those may now leak into the non-gimplified TYPE_SIZEs and if they
are later released SSA names with NULL_TREE type (or even garbage
memory if the SSA name is ggc freed) is in there.  This crashes
in various ways when those trees are accessed (in dwarf2out, in the
LTO streamer, in other places looking at pointed-to types).

Thus the following patch which makes the gimplifier unsharing
visit all types.

Alternative patches include unsharing trees when we build a
save_expr around them, doing that only when stor-layout does
this to TYPE_SIZE fields (and friends) or try to have an
extra pass over the GENERIC IL to just mark the expressions
in the type fields not walked by the current walking (might
turn out tricky but it would result in the "least" unsharing
thus only unshare the parts we eventually gimplify).

It might be that we just need to declare tree sharing that runs
into the above issue as invalid (they involve Fortran testcases
or ubsan testcases only as far as I can see - interestingly
no Ada testcases are affected).

So - any opinion on the "correct" way to fix this?  Clearly
the gimplifier running into shared trees is a bug.

Thanks,
Richard.

2016-04-27  Richard Biener  <rguenther@suse.de>

	* gimplify.c (copy_if_shared_r): Walk types, type sizes
	and bounds.
	(unmark_visited_r): Unmark them.

Index: gcc/gimplify.c
===================================================================
--- gcc/gimplify.c.orig	2016-04-27 10:29:54.784677194 +0200
+++ gcc/gimplify.c	2016-04-27 10:29:38.708496357 +0200
@@ -832,31 +832,61 @@ copy_if_shared_r (tree *tp, int *walk_su
   tree t = *tp;
   enum tree_code code = TREE_CODE (t);
 
-  /* Skip types, decls, and constants.  But we do want to look at their
-     types and the bounds of types.  Mark them as visited so we properly
-     unmark their subtrees on the unmark pass.  If we've already seen them,
-     don't look down further.  */
-  if (TREE_CODE_CLASS (code) == tcc_type
-      || TREE_CODE_CLASS (code) == tcc_declaration
+  bool was_visited = TREE_VISITED (t);
+
+  /* If the node wasn't visited already mark it so and recurse on its type.  */
+  if (! was_visited)
+    {
+      TREE_VISITED (t) = 1;
+
+      /* walk_tree does not descend into TREE_TYPE unless this is a
+	 DECL_EXPR of a TYPE_DECL.  */
+      if (CODE_CONTAINS_STRUCT (code, TS_TYPED))
+	walk_tree (&TREE_TYPE (t), copy_if_shared_r, data, NULL);
+    }
+
+  /* Skip types, decls, and constants for copying.  But we do want to look
+     at their types and the bounds of types.  Mark them as visited so we
+     properly unmark their subtrees on the unmark pass.  If we've already
+     seen them, don't look down further.  */
+  if (TREE_CODE_CLASS (code) == tcc_declaration
       || TREE_CODE_CLASS (code) == tcc_constant)
     {
-      if (TREE_VISITED (t))
+      if (was_visited)
+	*walk_subtrees = 0;
+    }
+  else if (TREE_CODE_CLASS (code) == tcc_type)
+    {
+      if (was_visited)
 	*walk_subtrees = 0;
       else
-	TREE_VISITED (t) = 1;
+	{
+	  /* walk_type_fields does not walk type sizes or bounds if not
+	     coming directly from a DECL_EXPR context.  */
+	  /* ???  ideally we'd mark all exprs reached via types as visited
+	     before copy_if_shared so duplicates in types that do not matter
+	     for gimplification are never unshared.  */
+	  walk_tree (&TYPE_SIZE (t), copy_if_shared_r, data, NULL);
+	  walk_tree (&TYPE_SIZE_UNIT (t), copy_if_shared_r, data, NULL);
+	  if (INTEGRAL_TYPE_P (t)
+	      || TREE_CODE (t) == FIXED_POINT_TYPE
+	      || TREE_CODE (t) == REAL_TYPE)
+	    {
+	      walk_tree (&TYPE_MAX_VALUE (t), copy_if_shared_r, data, NULL);
+	      walk_tree (&TYPE_MIN_VALUE (t), copy_if_shared_r, data, NULL);
+	    }
+	}
     }
 
   /* If this node has been visited already, unshare it and don't look
      any deeper.  */
-  else if (TREE_VISITED (t))
+  else if (was_visited)
     {
       walk_tree (tp, mostly_copy_tree_r, data, NULL);
       *walk_subtrees = 0;
     }
 
-  /* Otherwise, mark the node as visited and keep looking.  */
-  else
-    TREE_VISITED (t) = 1;
+  /* Otherwise keep looking.  */
 
   return NULL_TREE;
 }
@@ -903,7 +933,25 @@ unmark_visited_r (tree *tp, int *walk_su
 
   /* If this node has been visited, unmark it and keep looking.  */
   if (TREE_VISITED (t))
-    TREE_VISITED (t) = 0;
+    {
+      TREE_VISITED (t) = 0;
+
+      if (TYPE_P (t))
+	{
+	  walk_tree (&TYPE_SIZE (t), unmark_visited_r, NULL, NULL);
+	  walk_tree (&TYPE_SIZE_UNIT (t), unmark_visited_r, NULL, NULL);
+	  if (INTEGRAL_TYPE_P (t)
+	      || TREE_CODE (t) == FIXED_POINT_TYPE
+	      || TREE_CODE (t) == REAL_TYPE)
+	    {
+	      walk_tree (&TYPE_MAX_VALUE (t), unmark_visited_r, NULL, NULL);
+	      walk_tree (&TYPE_MIN_VALUE (t), unmark_visited_r, NULL, NULL);
+	    }
+	}
+
+      if (CODE_CONTAINS_STRUCT (TREE_CODE (t), TS_TYPED))
+	walk_tree (&TREE_TYPE (t), unmark_visited_r, NULL, NULL);
+    }
 
   /* Otherwise, don't look any deeper.  */
   else