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 2 --combine bugs (PR c/28706 and PR c/28712)


Hi!

There seem to be several issues lurking with --combine and aggregates
with attributes.  The problem is that within the same translation unit
the C FE requires the aggregate types to be identical (pointer comparison),
while in different TUs the requirements are much weaker.  If we see
say two RECORD_TYPEs in different TUs, one or both have some attributes
and comptypes says they are the same, during some duplicate_decls we may
generate a completely new RECORD_TYPE (to contain the merged attributes),
but then suddenly we have 2 types that are supposed to be the same, but
are in one TU and so next time we comptypes them, we get bad result.

The following patch does a few things to cure this:
1) for any attributes that has arguments the various tree.c attribute
   list comparison and merging routines were failing, as simple_cst_equal
   (intentionally?) doesn't handle TREE_LIST, which is used for the
   attribute arguments
2) the C FE now considers aggregate types incompatible if they are in
   different TUs and have incompatible attributes.  Say
foo.c:
struct A { char c; int i; } __attribute__((packed, aligned (8)));
void foo (struct A *x) { return __alignof__ (*x); }
bar.c:
struct A { char c; int i; } __attribute__((aligned (16)));
void foo (struct A *);
   I don't think it is a good idea to pretend these are compatible types,
   when merged the attributes would be combined from both TUs and the
   behaviour would varry depending on if --combine was used or not
   (and often even depending on the exact order of .c files).
   Unfortunately, because of incomplete types we can't just request
   the attributes are identical, so the patch just requests that
   one of the attribute sets is a subset of the other attribute set.
3) composite_type attempts harder not to create a new aggregate type
   (i.e. if say t2 has more complete set of attributes that is a superset
   of t1 attributes, t2 is returned, rather than creating a new RECORD_TYPE
   that will be also in the current TU)

Ok for trunk?

2006-08-14  Jakub Jelinek  <jakub@redhat.com>

	PR c/28706
	* tree.c (merge_attributes, attribute_list_contained): If both
	TREE_VALUEs are TREE_LISTs, use simple_cst_list_equal instead of
	simple_cst_equal.
	* c-typeck.c (comptypes_internal): Don't consider aggregates
	in different TUs as compatible if there one set of attributes is
	not a subset of the other type's attributes.
	(composite_type): Try harder not to create a new aggregate type.

	* gcc.dg/pr28706.c: New test.

--- gcc/testsuite/gcc.dg/pr28706.c.jj	2006-08-14 17:03:51.000000000 +0200
+++ gcc/testsuite/gcc.dg/pr28706.c	2006-08-14 17:04:59.000000000 +0200
@@ -0,0 +1,12 @@
+/* PR c/28706 */
+/* { dg-do compile } */
+/* { dg-options "--combine" } */
+/* { dg-additional-sources "pr28706.c" } */
+
+struct A
+{
+  int i;
+} __attribute__((aligned (sizeof (long int))));
+
+extern void foo (struct A *);
+extern void foo (struct A *);
--- gcc/testsuite/gcc.dg/pr28712.c.jj	2006-08-14 19:12:26.000000000 +0200
+++ gcc/testsuite/gcc.dg/pr28712.c	2006-08-14 19:12:18.000000000 +0200
@@ -0,0 +1,16 @@
+/* PR c/28712 */
+/* { dg-do compile } */
+/* { dg-options "--combine" } */
+/* { dg-additional-sources "pr28712.c pr28712.c" } */
+
+struct A;
+
+extern struct A *a;
+
+struct A { } __attribute__((packed));
+
+struct B __attribute__((aligned (sizeof (int))));
+
+extern struct B *b;
+
+struct B { int i; } __attribute__((packed));
--- gcc/tree.c.jj	2006-08-14 13:58:27.000000000 +0200
+++ gcc/tree.c	2006-08-14 17:42:24.000000000 +0200
@@ -3555,7 +3555,17 @@ merge_attributes (tree a1, tree a2)
 		   a = lookup_attribute (IDENTIFIER_POINTER (TREE_PURPOSE (a2)),
 					 TREE_CHAIN (a)))
 		{
-		  if (simple_cst_equal (TREE_VALUE (a), TREE_VALUE (a2)) == 1)
+		  if (TREE_VALUE (a) != NULL
+		      && TREE_CODE (TREE_VALUE (a)) == TREE_LIST
+		      && TREE_VALUE (a2) != NULL
+		      && TREE_CODE (TREE_VALUE (a2)) == TREE_LIST)
+		    {
+		      if (simple_cst_list_equal (TREE_VALUE (a),
+						 TREE_VALUE (a2)) == 1)
+			break;
+		    }
+		  else if (simple_cst_equal (TREE_VALUE (a),
+					     TREE_VALUE (a2)) == 1)
 		    break;
 		}
 	      if (a == NULL_TREE)
@@ -4366,15 +4376,21 @@ attribute_list_contained (tree l1, tree 
 	   attr = lookup_attribute (IDENTIFIER_POINTER (TREE_PURPOSE (t2)),
 				    TREE_CHAIN (attr)))
 	{
-	  if (simple_cst_equal (TREE_VALUE (t2), TREE_VALUE (attr)) == 1)
+	  if (TREE_VALUE (t2) != NULL
+	      && TREE_CODE (TREE_VALUE (t2)) == TREE_LIST
+	      && TREE_VALUE (attr) != NULL
+	      && TREE_CODE (TREE_VALUE (attr)) == TREE_LIST)
+	    {
+	      if (simple_cst_list_equal (TREE_VALUE (t2),
+					 TREE_VALUE (attr)) == 1)
+		break;
+	    }
+	  else if (simple_cst_equal (TREE_VALUE (t2), TREE_VALUE (attr)) == 1)
 	    break;
 	}
 
       if (attr == 0)
 	return 0;
-
-      if (simple_cst_equal (TREE_VALUE (t2), TREE_VALUE (attr)) != 1)
-	return 0;
     }
 
   return 1;
--- gcc/c-typeck.c.jj	2006-08-14 13:58:27.000000000 +0200
+++ gcc/c-typeck.c	2006-08-14 19:08:03.000000000 +0200
@@ -375,6 +375,19 @@ composite_type (tree t1, tree t2)
 	return build_type_attribute_variant (t1, attributes);
       }
 
+    case ENUMERAL_TYPE:
+    case RECORD_TYPE:
+    case UNION_TYPE:
+      if (attributes != NULL)
+	{
+	  /* Try harder not to create a new aggregate type.  */
+	  if (attribute_list_equal (TYPE_ATTRIBUTES (t1), attributes))
+	    return t1;
+	  if (attribute_list_equal (TYPE_ATTRIBUTES (t2), attributes))
+	    return t2;
+	}
+      return build_type_attribute_variant (t1, attributes);
+
     case FUNCTION_TYPE:
       /* Function types: prefer the one that specified arg types.
 	 If both do, merge the arg types.  Also merge the return types.  */
@@ -891,6 +904,13 @@ comptypes_internal (tree type1, tree typ
     case UNION_TYPE:
       if (val != 1 && !same_translation_unit_p (t1, t2))
 	{
+	  tree a1 = TYPE_ATTRIBUTES (t1);
+	  tree a2 = TYPE_ATTRIBUTES (t2);
+
+	  if (! attribute_list_contained (a1, a2)
+	      && ! attribute_list_contained (a2, a1))
+	    break;
+
 	  if (attrval != 2)
 	    return tagged_types_tu_compatible_p (t1, t2);
 	  val = tagged_types_tu_compatible_p (t1, t2);

	Jakub


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