[patch] Fix GC issue triggered by arithmetic overflow checking

[Eric Botcazou <ebotcazou@adacore.com>]

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

Hi,

adding patterns for unsigned arithmetic overflow checking in a back-end can 
have unexpected fallout because of a latent GC issue: when they are present, 
GIMPLE optimization passes can create complex (math. sense) types at will by 
invoking build_complex_type.  Now build_complex_type goes through the type 
caonicalization hashtable, which is GC-ed, so its behavior depends on the 
actual collection points.

The other type-building functions present in tree.c do the same so no big deal 
but build_complex_type is special because it also does:

  /* We need to create a name, since complex is a fundamental type.  */
  if (! TYPE_NAME (t))
    {
      const char *name;
      if (component_type == char_type_node)
	name = "complex char";
      else if (component_type == signed_char_type_node)
	name = "complex signed char";
      else if (component_type == unsigned_char_type_node)
	name = "complex unsigned char";
      else if (component_type == short_integer_type_node)
	name = "complex short int";
      else if (component_type == short_unsigned_type_node)
	name = "complex short unsigned int";
      else if (component_type == integer_type_node)
	name = "complex int";
      else if (component_type == unsigned_type_node)
	name = "complex unsigned int";
      else if (component_type == long_integer_type_node)
	name = "complex long int";
      else if (component_type == long_unsigned_type_node)
	name = "complex long unsigned int";
      else if (component_type == long_long_integer_type_node)
	name = "complex long long int";
      else if (component_type == long_long_unsigned_type_node)
	name = "complex long long unsigned int";
      else
	name = 0;

      if (name != 0)
	TYPE_NAME (t) = build_decl (UNKNOWN_LOCATION, TYPE_DECL,
	    			    get_identifier (name), t);
    }

so it creates a DECL node every time a new canonical complex type is created, 
bumping the DECL_UID counter in the process.  Which means that the DECL_UID 
counter is sensitive to the collection points, which in turn means that the 
result of algorithms depending on the DECL_UID counter also is.

This for example resulted in a bootstrap comparison failure on a SPARC/Solaris 
machine doing a strict stage2/stage3 comparison because the contents of the 
.debug_loc section were different: location lists computed by var-tracking 
were slightly different because of a different hashing.

I'm not sure whether the hashing done by var-tracking should be sensitive to 
the DECL_UID of nodes or not, but I think that having the DECL_UID counter 
depend on the collection points is highly undesirable, so the attached patch 
attempts to prevent it; it at least fixed the bootstrap comparison failure.

Tested on x86_64-suse-linux, OK for the mainline?


2016-10-08  Eric Botcazou  <ebotcazou@adacore.com>

	* tree.h (build_complex_type): Add second parameter with default.
	* builtins.c (expand_builtin_cexpi): Pass false in call to above.
	(fold_builtin_sincos): Likewise.
	(fold_builtin_arith_overflow): Likewise.
	* gimple-fold.c (fold_builtin_atomic_compare_exchange): Likewise.
	(gimple_fold_call): Likewise.
	* stor-layout.c (bitwise_type_for_mode): Likewise.
	* tree-ssa-dce.c (maybe_optimize_arith_overflow): Likewise.
	* tree-ssa-math-opts.c (match_uaddsub_overflow): Likewise.
	* tree.c (build_complex): Likewise.
	(build_complex_type): Add NAMED second parameter and adjust recursive
	call.  Create a TYPE_DECL only if NAMED is true.

-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 6426 bytes --]

Index: builtins.c
===================================================================
--- builtins.c	(revision 240888)
+++ builtins.c	(working copy)
@@ -2356,7 +2356,7 @@ expand_builtin_cexpi (tree exp, rtx targ
   else
     {
       tree call, fn = NULL_TREE, narg;
-      tree ctype = build_complex_type (type);
+      tree ctype = build_complex_type (type, false);
 
       if (DECL_FUNCTION_CODE (fndecl) == BUILT_IN_CEXPIF)
 	fn = builtin_decl_explicit (BUILT_IN_CEXPF);
@@ -2396,7 +2396,7 @@ expand_builtin_cexpi (tree exp, rtx targ
     }
 
   /* Now build the proper return type.  */
-  return expand_expr (build2 (COMPLEX_EXPR, build_complex_type (type),
+  return expand_expr (build2 (COMPLEX_EXPR, build_complex_type (type, false),
 			      make_tree (TREE_TYPE (arg), op2),
 			      make_tree (TREE_TYPE (arg), op1)),
 		      target, VOIDmode, EXPAND_NORMAL);
@@ -7226,7 +7226,7 @@ fold_builtin_sincos (location_t loc,
   /* Canonicalize sincos to cexpi.  */
   if (TREE_CODE (arg0) == REAL_CST)
     {
-      tree complex_type = build_complex_type (type);
+      tree complex_type = build_complex_type (type, false);
       call = fold_const_call (as_combined_fn (fn), complex_type, arg0);
     }
   if (!call)
@@ -8137,7 +8137,7 @@ fold_builtin_arith_overflow (location_t
 				 ? boolean_true_node : boolean_false_node,
 				 arg2);
 
-  tree ctype = build_complex_type (type);
+  tree ctype = build_complex_type (type, false);
   tree call = build_call_expr_internal_loc (loc, ifn, ctype,
 					    2, arg0, arg1);
   tree tgt = save_expr (call);
Index: gimple-fold.c
===================================================================
--- gimple-fold.c	(revision 240888)
+++ gimple-fold.c	(working copy)
@@ -3210,7 +3210,7 @@ fold_builtin_atomic_compare_exchange (gi
   tree fndecl = gimple_call_fndecl (stmt);
   tree parmt = TYPE_ARG_TYPES (TREE_TYPE (fndecl));
   tree itype = TREE_VALUE (TREE_CHAIN (TREE_CHAIN (parmt)));
-  tree ctype = build_complex_type (itype);
+  tree ctype = build_complex_type (itype, false);
   tree expected = TREE_OPERAND (gimple_call_arg (stmt, 1), 0);
   gimple *g = gimple_build_assign (make_ssa_name (TREE_TYPE (expected)),
 				   expected);
@@ -3582,7 +3582,7 @@ gimple_fold_call (gimple_stmt_iterator *
 	    {
 	      if (overflow == NULL_TREE)
 		overflow = build_zero_cst (TREE_TYPE (result));
-	      tree ctype = build_complex_type (TREE_TYPE (result));
+	      tree ctype = build_complex_type (TREE_TYPE (result), false);
 	      if (TREE_CODE (result) == INTEGER_CST
 		  && TREE_CODE (overflow) == INTEGER_CST)
 		result = build_complex (ctype, result, overflow);
Index: stor-layout.c
===================================================================
--- stor-layout.c	(revision 240888)
+++ stor-layout.c	(working copy)
@@ -469,7 +469,7 @@ bitwise_type_for_mode (machine_mode mode
     return build_vector_type_for_mode (inner_type, mode);
 
   if (COMPLEX_MODE_P (mode))
-    return build_complex_type (inner_type);
+    return build_complex_type (inner_type, false);
 
   gcc_checking_assert (GET_MODE_INNER (mode) == mode);
   return inner_type;
Index: tree-ssa-dce.c
===================================================================
--- tree-ssa-dce.c	(revision 240888)
+++ tree-ssa-dce.c	(working copy)
@@ -1194,7 +1194,7 @@ maybe_optimize_arith_overflow (gimple_st
   if (TREE_CODE (result) == INTEGER_CST && TREE_OVERFLOW (result))
     result = drop_tree_overflow (result);
   tree overflow = build_zero_cst (type);
-  tree ctype = build_complex_type (type);
+  tree ctype = build_complex_type (type, false);
   if (TREE_CODE (result) == INTEGER_CST)
     result = build_complex (ctype, result, overflow);
   else
Index: tree-ssa-math-opts.c
===================================================================
--- tree-ssa-math-opts.c	(revision 240888)
+++ tree-ssa-math-opts.c	(working copy)
@@ -3736,7 +3736,7 @@ match_uaddsub_overflow (gimple_stmt_iter
   if (!ovf_use_seen || !use_seen)
     return false;
 
-  tree ctype = build_complex_type (type);
+  tree ctype = build_complex_type (type, false);
   tree rhs1 = gimple_assign_rhs1 (stmt);
   tree rhs2 = gimple_assign_rhs2 (stmt);
   gcall *g = gimple_build_call_internal (code == PLUS_EXPR
Index: tree.c
===================================================================
--- tree.c	(revision 240888)
+++ tree.c	(working copy)
@@ -2021,7 +2021,7 @@ build_complex (tree type, tree real, tre
 
   TREE_REALPART (t) = real;
   TREE_IMAGPART (t) = imag;
-  TREE_TYPE (t) = type ? type : build_complex_type (TREE_TYPE (real));
+  TREE_TYPE (t) = type ? type : build_complex_type (TREE_TYPE (real), false);
   TREE_OVERFLOW (t) = TREE_OVERFLOW (real) | TREE_OVERFLOW (imag);
   return t;
 }
@@ -8758,10 +8758,15 @@ build_offset_type (tree basetype, tree t
   return t;
 }
 
-/* Create a complex type whose components are COMPONENT_TYPE.  */
+/* Create a complex type whose components are COMPONENT_TYPE.
+
+   If NAMED is true, the type is given a TYPE_NAME.  We do not always
+   do so because this creates a DECL node and thus make the DECL_UIDs
+   dependent on the type canonicalization hashtable, which is GC-ed,
+   so the DECL_UIDs would not be stable wrt garbage collection.  */
 
 tree
-build_complex_type (tree component_type)
+build_complex_type (tree component_type, bool named)
 {
   tree t;
   inchash::hash hstate;
@@ -8788,11 +8793,11 @@ build_complex_type (tree component_type)
 	SET_TYPE_STRUCTURAL_EQUALITY (t);
       else if (TYPE_CANONICAL (component_type) != component_type)
 	TYPE_CANONICAL (t)
-	  = build_complex_type (TYPE_CANONICAL (component_type));
+	  = build_complex_type (TYPE_CANONICAL (component_type), named);
     }
 
   /* We need to create a name, since complex is a fundamental type.  */
-  if (! TYPE_NAME (t))
+  if (!TYPE_NAME (t) && named)
     {
       const char *name;
       if (component_type == char_type_node)
Index: tree.h
===================================================================
--- tree.h	(revision 240888)
+++ tree.h	(working copy)
@@ -4042,7 +4042,7 @@ extern tree build_varargs_function_type_
 extern tree build_method_type_directly (tree, tree, tree);
 extern tree build_method_type (tree, tree);
 extern tree build_offset_type (tree, tree);
-extern tree build_complex_type (tree);
+extern tree build_complex_type (tree, bool named = true);
 extern tree array_type_nelts (const_tree);
 
 extern tree value_member (tree, tree);