This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH]: Fix structure aliasing regressions
- From: Daniel Berlin <dberlin at dberlin dot org>
- To: Diego Novillo <dnovillo at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 20 Jun 2005 14:52:14 -0400
- Subject: [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;
+}