This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH]: Fix structure aliasing regressions


This should fix most of the do_structure_copy reported bugs (I know of
one or two it will not, mostly dealing with almost-empty class copying).

I can't bootstrap with Ada, so i can't verify that this is really the
case.

Diego, it also fixes a bug that will probably hit you if you are trying
to replace the old aliaser, which is that dereferences of anything were
not properly getting set to anything, due to an optimization i forgot
about in process_constraint.  This bug was introduced during a cleanup i
made after your first review.

IE the constraint ANYTHING = &ANYTHING (thus, *ANYTHING = ANYTHING) was
getting ignored.

The tree-ssa-alias.c portion is necessary because we might not see the
portion that is used once the address is taken, and it may get casted
back, but the structure alias analyzer will come up with the right
answer, and will abort when it can't find a field that should be there.

This is the pta-1.c testcase.

Diego, if you'd like to approve the tree-ssa-alias.c part, i can commit
the rest (the c-typeck.c change is the problem referred to in an earlier
email that should get a seperate PR)


Bootstrapped and regtested on i686-pc-linux-gnu
Okay for mainline?

--Dan
2005-06-20  Daniel Berlin  <dberlin@dberlin.org>

	* c-typeck.c (build_function_call): Set fundecl = function again.
	* tree-ssa-alias.c (find_used_portions): Address taking causes the
	entire variable to be used.
	* tree-ssa-structalias.c (do_structure_copy): Fix handling of
	unknown size variables, and structure copies from addressof
	operations.  Simplify how we do *a = *b type structure copies.
	(init_base_vars): Add ANYTHING = &ANYTHING constraint the right
	way.  READONLY's address is not taken by default.
	INTEGER dereference should point to anything.
	(create_variable_info_for): It's okay for the first field to not start
	at 0.

Index: c-typeck.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-typeck.c,v
retrieving revision 1.456
diff -u -p -r1.456 c-typeck.c
--- c-typeck.c	19 Jun 2005 16:55:22 -0000	1.456
+++ c-typeck.c	20 Jun 2005 18:49:21 -0000
@@ -1990,6 +1990,7 @@ build_function_call (tree function, tree
 	return tem;
 
       name = DECL_NAME (function);
+      fundecl = function;
     }
   function = default_function_array_conversion (function);
 
Index: tree-ssa-alias.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-alias.c,v
retrieving revision 2.96
diff -u -p -r2.96 tree-ssa-alias.c
--- tree-ssa-alias.c	10 Jun 2005 17:44:22 -0000	2.96
+++ tree-ssa-alias.c	20 Jun 2005 18:49:21 -0000
@@ -3119,6 +3119,35 @@ find_used_portions (tree *tp, int *walk_
 	  }
       }
       break;
+      /* This is here to make sure we mark the entire base variable as used
+	 when you take its address.  Because our used portion analysis is
+	 simple, we aren't looking at casts or pointer arithmetic to see what
+	 happens when you take the address.  */
+    case ADDR_EXPR:
+      {
+	tree var = get_base_address (TREE_OPERAND (*tp, 0));
+
+	if (var 
+	    && DECL_P (var)
+	    && DECL_SIZE (var)
+	    && var_can_have_subvars (var)
+	    && TREE_CODE (DECL_SIZE (var)) == INTEGER_CST)
+	  {
+	    used_part_t up;
+	    size_t uid = var_ann (var)->uid;	    
+	    
+	    up = get_or_create_used_part_for (uid);
+ 
+	    up->minused = 0;
+	    up->maxused = TREE_INT_CST_LOW (DECL_SIZE (var));
+	    up->implicit_uses = true;
+
+	    used_portions[uid] = up;
+	    *walk_subtrees = 0;
+	    return NULL_TREE;
+	  }
+      }
+      break;
     case VAR_DECL:
     case PARM_DECL:
       {
Index: tree-ssa-structalias.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-structalias.c,v
retrieving revision 2.4
diff -u -p -r2.4 tree-ssa-structalias.c
--- tree-ssa-structalias.c	15 Jun 2005 17:37:45 -0000	2.4
+++ tree-ssa-structalias.c	20 Jun 2005 18:49:21 -0000
@@ -2321,8 +2320,6 @@ do_structure_copy (tree lhsop, tree rhso
   unsigned HOST_WIDE_INT lhssize;
   unsigned HOST_WIDE_INT rhssize;
 
-  lhssize = TREE_INT_CST_LOW (TYPE_SIZE (TREE_TYPE (lhsop)));
-  rhssize = TREE_INT_CST_LOW (TYPE_SIZE (TREE_TYPE (rhsop)));
   lhs = get_constraint_for (lhsop);  
   rhs = get_constraint_for (rhsop);
   
@@ -2334,8 +2331,18 @@ do_structure_copy (tree lhsop, tree rhso
       rhs = tmp;
     }
   
-  /* If the RHS is a special var, set all the LHS fields to that
-     special var.  */
+  /*  This is fairly conservative for the RHS == ADDRESSOF case, in that it's
+      possible it's something we could handle.  However, most cases falling
+      into this are dealing with transparent unions, which are slightly
+      weird. */
+  if (rhs.type == ADDRESSOF && rhs.var > integer_id)
+    {
+      rhs.type = ADDRESSOF;
+      rhs.var = anything_id;
+    }
+
+  /* If the RHS is a special var, or an addressof, set all the LHS fields to
+     that special var.  */
   if (rhs.var <= integer_id)
     {
       for (p = get_varinfo (lhs.var); p; p = p->next)
@@ -2351,6 +2358,20 @@ do_structure_copy (tree lhsop, tree rhso
     }
   else
     {
+      /* The size only really matters insofar as we don't set more or less of
+	 the variable.  If we hit an unknown size var, the size should be the
+	 whole darn thing.  */
+      if (get_varinfo (rhs.var)->is_unknown_size_var)
+	rhssize = ~0;
+      else
+	rhssize = TREE_INT_CST_LOW (TYPE_SIZE (TREE_TYPE (rhsop)));
+
+      if (get_varinfo (lhs.var)->is_unknown_size_var)
+	lhssize = ~0;
+      else
+	lhssize = TREE_INT_CST_LOW (TYPE_SIZE (TREE_TYPE (lhsop)));
+
+  
       if (rhs.type == SCALAR && lhs.type == SCALAR)  
 	do_simple_structure_copy (lhs, rhs, MIN (lhssize, rhssize));
       else if (lhs.type != DEREF && rhs.type == DEREF)
@@ -2362,14 +2383,12 @@ do_structure_copy (tree lhsop, tree rhso
 	  tree rhsdecl = get_varinfo (rhs.var)->decl;
 	  tree pointertype = TREE_TYPE (rhsdecl);
 	  tree pointedtotype = TREE_TYPE (pointertype);
-	  tree tmpvar;
+	  tree tmpvar;  
+
 	  gcc_assert (rhs.type == DEREF && lhs.type == DEREF);
 	  tmpvar = create_tmp_var_raw (pointedtotype, "structcopydereftmp");
-	  lhs = get_constraint_for (tmpvar);
-	  do_rhs_deref_structure_copy (lhs, rhs, MIN (lhssize, rhssize));
-	  rhs = lhs;
-	  lhs = get_constraint_for (lhsop);
-	  do_lhs_deref_structure_copy (lhs, rhs, MIN (lhssize, rhssize));
+	  do_structure_copy (tmpvar, rhsop);
+	  do_structure_copy (lhsop, tmpvar);
 	}
     }
 }
@@ -2786,7 +2805,6 @@ create_variable_info_for (tree decl, con
 	}
       
       field = fo->field;
-      gcc_assert (bitpos_of_field (field) == 0);
       vi->size = TREE_INT_CST_LOW (DECL_SIZE (field));
       for (i = 1; VEC_iterate (fieldoff_s, fieldstack, i, fo); i++)
 	{
@@ -3049,8 +3067,10 @@ init_base_vars (void)
   rhs.var = anything_id;
   rhs.offset = 0;
   var_anything->address_taken = true;
-  process_constraint (new_constraint (lhs, rhs));
-
+  /* This specifically does not use process_constraint because
+     process_constraint ignores all anything = anything constraints, since all
+     but this one are redundant.  */
+  VEC_safe_push (constraint_t, gc, constraints, new_constraint (lhs, rhs));
   
   /* Create the READONLY variable, used to represent that a variable
      points to readonly memory.  */
@@ -3075,7 +3095,6 @@ init_base_vars (void)
   rhs.type = ADDRESSOF;
   rhs.var = anything_id;
   rhs.offset = 0;
-  var_readonly->address_taken = true;
   
   process_constraint (new_constraint (lhs, rhs));
   
@@ -3091,6 +3110,16 @@ init_base_vars (void)
   var_integer->next = NULL;
   integer_id = 3;
   VEC_safe_push (varinfo_t, gc, varmap, var_integer);
+
+  /* *INTEGER = ANYTHING, because we don't know where a dereference of a random
+     integer will point to.  */
+  lhs.type = SCALAR;
+  lhs.var = integer_id;
+  lhs.offset = 0;
+  rhs.type = ADDRESSOF;
+  rhs.var = anything_id;
+  rhs.offset = 0;
+  process_constraint (new_constraint (lhs, rhs));
 }  
 
 
Index: testsuite/gcc.c-torture/compile/pta-1.c
===================================================================
RCS file: testsuite/gcc.c-torture/compile/pta-1.c
diff -N testsuite/gcc.c-torture/compile/pta-1.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gcc.c-torture/compile/pta-1.c	20 Jun 2005 18:49:22 -0000
@@ -0,0 +1,31 @@
+typedef struct JSObject JSObject;
+typedef struct JSObjectMap *(*JSNewObjectMapOp) (JSObject *obj);
+typedef JSObject *(*JSGetMethodOp) (JSObject *obj);
+struct JSObjectOps {
+    JSNewObjectMapOp newObjectMap;
+};
+struct JSXMLObjectOps {
+    struct JSObjectOps base;
+    JSGetMethodOp getMethod;
+};
+struct JSObjectMap {
+    struct JSObjectOps *ops;
+};
+struct JSObject {
+    struct JSObjectMap *map;
+};
+
+struct JSXMLObjectOps js_XMLObjectOps;
+
+
+/* We need to create SFT's for the entire structure when this address is taken, 
+   not just the part in the component reference itself.  */
+JSObject *JS_GetMethod(JSObject *obj)
+{
+    if (obj->map->ops == &js_XMLObjectOps.base) {
+        struct JSXMLObjectOps *ops;
+        ops = (struct JSXMLObjectOps *) obj->map->ops;
+        obj = ops->getMethod(obj);
+    }
+    return obj;
+}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]