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]

Different fix for 21166


This is a different patch for 21166, a problem with packed char fields. My original patch dealt with this in the middle end by unsetting DECL_PACKED on fields that turned out to fall on a natural alignment boundary. I now think this is incorrect. The layout of a particular field is an accident of where it is in the structure, we shouldn't use that to change its type. However, I do think it nonsensical to have a packed char field -- which was the original problem.

Therefore this patch reverts my original patch, and changes the C and C++ FE's to not set DECL_PACKED on fields whose type already has char alignment. You won't get a warning on this case.

If you explicitly try and pack a char field, you'll get a warning.

booted & tested on i686-pc-linux-gnu, ok? I'd like this change in 4.1 too, as my original patch is already there.

nathan
--
Nathan Sidwell    ::   http://www.codesourcery.com   ::     CodeSourcery LLC
nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk

2005-11-25  Nathan Sidwell  <nathan@codesourcery.com>

	PR c++/21166
	* c-decl.c (finish_struct):  Only set DECL_PACKED on a field
	when its natural alignment is > BITS_PER_UNIT.
	* stor-layout.c (finalize_type_size): Revert my patch of 2005-08-08.
	* c-common.c (handle_packed_attribute): Ignore packing on a field
	whose type is naturally char aligned.

	PR c++/21166
	* cp/class.c (check_field_decls): Only set DECL_PACKED on a field
	when its natural alignment is > BITS_PER_UNIT.

Index: cp/class.c
===================================================================
--- cp/class.c	(revision 107432)
+++ cp/class.c	(working copy)
@@ -2801,7 +2801,7 @@ check_field_decls (tree t, tree *access_
 		  (0,
 		   "ignoring packed attribute on unpacked non-POD field %q+#D",
 		   x);
-	      else
+	      else if (TYPE_ALIGN (TREE_TYPE (x)) > BITS_PER_UNIT)
 		DECL_PACKED (x) = 1;
 	    }
 
Index: c-decl.c
===================================================================
--- c-decl.c	(revision 107432)
+++ c-decl.c	(working copy)
@@ -5326,7 +5326,9 @@ finish_struct (tree t, tree fieldlist, t
   for (x = fieldlist; x; x = TREE_CHAIN (x))
     {
       DECL_CONTEXT (x) = t;
-      DECL_PACKED (x) |= TYPE_PACKED (t);
+
+      if (TYPE_PACKED (t) && TYPE_ALIGN (TREE_TYPE (x)) > BITS_PER_UNIT)
+	DECL_PACKED (x) = 1;
 
       /* If any field is const, the structure type is pseudo-const.  */
       if (TREE_READONLY (x))
Index: stor-layout.c
===================================================================
--- stor-layout.c	(revision 107432)
+++ stor-layout.c	(working copy)
@@ -1495,8 +1495,6 @@ finalize_type_size (tree type)
 void
 finish_record_layout (record_layout_info rli, int free_p)
 {
-  tree field;
-  
   /* Compute the final size.  */
   finalize_record_size (rli);
 
@@ -1506,15 +1504,6 @@ finish_record_layout (record_layout_info
   /* Perform any last tweaks to the TYPE_SIZE, etc.  */
   finalize_type_size (rli->t);
 
-  /* We might be able to clear DECL_PACKED on any members that happen
-     to be suitably aligned (not forgetting the alignment of the type
-     itself).  */
-  for (field = TYPE_FIELDS (rli->t); field; field = TREE_CHAIN (field))
-    if (TREE_CODE (field) == FIELD_DECL && DECL_PACKED (field)
-	&& DECL_OFFSET_ALIGN (field) >= TYPE_ALIGN (TREE_TYPE (field))
-	&& TYPE_ALIGN (rli->t) >= TYPE_ALIGN (TREE_TYPE (field)))
-      DECL_PACKED (field) = 0;
-  
   /* Lay out any static members.  This is done now because their type
      may use the record's type.  */
   while (rli->pending_statics)
Index: c-common.c
===================================================================
--- c-common.c	(revision 107432)
+++ c-common.c	(working copy)
@@ -4029,17 +4029,23 @@ handle_packed_attribute (tree *node, tre
 
 	     struct Foo {
 	       struct Foo const *ptr; // creates a variant w/o packed flag
-	       } __ attribute__((packed)); // packs it now.
-	  */
+	     } __ attribute__((packed)); // packs it now.
+	   */
 	  tree probe;
 
 	  for (probe = *node; probe; probe = TYPE_NEXT_VARIANT (probe))
 	    TYPE_PACKED (probe) = 1;
 	}
-
     }
   else if (TREE_CODE (*node) == FIELD_DECL)
-    DECL_PACKED (*node) = 1;
+    {
+      if (TYPE_ALIGN (TREE_TYPE (*node)) <= BITS_PER_UNIT)
+	warning (OPT_Wattributes,
+		 "%qE attribute ignored for field of type %qT",
+		 name, TREE_TYPE (*node));
+      else
+	DECL_PACKED (*node) = 1;
+    }
   /* We can't set DECL_PACKED for a VAR_DECL, because the bit is
      used for DECL_REGISTER.  It wouldn't mean anything anyway.
      We can't set DECL_PACKED on the type of a TYPE_DECL, because

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