ABI bug: PR middle-end/22275

Michael Matz matz@suse.de
Tue Feb 14 16:45:00 GMT 2006


Hi,

On Mon, 13 Feb 2006, Mark Mitchell wrote:

> (Would you please use "diff -p" in future to generate your patches?)

Oh, I did forget to update the scripts since the upgrade to SVN.  Thanks
for the reminder.

> I have stared at this patch for a while, trying to find bugs.  I can't
> find any, and you've done a lot of testing, so this patch is OK for
> mainline and for 4.1.

I've checked in the unchanged patch plus the doc change to the 4.1 branch
already.  Below is the patch I'll put to trunk when testing finishes.  
It's changed according to your comments.  The doc change is the same I put 
also to 4.1.  Thanks for the review.


Ciao,
Michael.
-- 
        PR middle-end/22275

        * stor-layout.c (layout_decl): Zero-width bitfields aren't
        influenced by maximum_field_alignment or DECL_PACKED.
        (update_alignment_for_field): Ditto.
        (place_field): Ditto.
        * doc/extend.texi (<#pragma pack>, <Type Attributes>): Document
        this behaviour.

Index: stor-layout.c
===================================================================
--- stor-layout.c	(revision 110982)
+++ stor-layout.c	(working copy)
@@ -339,17 +339,22 @@ layout_decl (tree decl, unsigned int kno
     /* For fields, it's a bit more complicated...  */
     {
       bool old_user_align = DECL_USER_ALIGN (decl);
+      bool zero_bitfield = false;
+      bool packed_p = DECL_PACKED (decl);
+      unsigned int mfa;
 
       if (DECL_BIT_FIELD (decl))
 	{
 	  DECL_BIT_FIELD_TYPE (decl) = type;
 
 	  /* A zero-length bit-field affects the alignment of the next
-	     field.  */
+	     field.  In essence such bit-fields are not influenced by
+	     any packing due to #pragma pack or attribute packed.  */
 	  if (integer_zerop (DECL_SIZE (decl))
-	      && ! DECL_PACKED (decl)
 	      && ! targetm.ms_bitfield_layout_p (DECL_FIELD_CONTEXT (decl)))
 	    {
+	      zero_bitfield = true;
+	      packed_p = false;
 #ifdef PCC_BITFIELD_TYPE_MATTERS
 	      if (PCC_BITFIELD_TYPE_MATTERS)
 		do_type_align (type, decl);
@@ -393,7 +398,7 @@ layout_decl (tree decl, unsigned int kno
 	      && DECL_ALIGN (decl) >= TYPE_ALIGN (type))
 	    DECL_BIT_FIELD (decl) = 0;
 	}
-      else if (DECL_PACKED (decl) && DECL_USER_ALIGN (decl))
+      else if (packed_p && DECL_USER_ALIGN (decl))
 	/* Don't touch DECL_ALIGN.  For other packed fields, go ahead and
 	   round up; we'll reduce it again below.  We want packing to
 	   supersede USER_ALIGN inherited from the type, but defer to
@@ -408,14 +413,14 @@ layout_decl (tree decl, unsigned int kno
 
 	 Note that do_type_align may set DECL_USER_ALIGN, so we need to
 	 check old_user_align instead.  */
-      if (DECL_PACKED (decl)
+      if (packed_p
 	  && !old_user_align
 	  && (DECL_NONADDRESSABLE_P (decl)
 	      || DECL_SIZE_UNIT (decl) == 0
 	      || TREE_CODE (DECL_SIZE_UNIT (decl)) == INTEGER_CST))
 	DECL_ALIGN (decl) = MIN (DECL_ALIGN (decl), BITS_PER_UNIT);
 
-      if (! DECL_USER_ALIGN (decl) && ! DECL_PACKED (decl))
+      if (! packed_p && ! DECL_USER_ALIGN (decl))
 	{
 	  /* Some targets (i.e. i386, VMS) limit struct field alignment
 	     to a lower boundary than alignment of variables unless
@@ -429,9 +434,13 @@ layout_decl (tree decl, unsigned int kno
 #endif
 	}
 
+      if (zero_bitfield)
+        mfa = initial_max_fld_align * BITS_PER_UNIT;
+      else
+	mfa = maximum_field_alignment;
       /* Should this be controlled by DECL_USER_ALIGN, too?  */
-      if (maximum_field_alignment != 0)
-	DECL_ALIGN (decl) = MIN (DECL_ALIGN (decl), maximum_field_alignment);
+      if (mfa != 0)
+	DECL_ALIGN (decl) = MIN (DECL_ALIGN (decl), mfa);
     }
 
   /* Evaluate nonconstant size only once, either now or as soon as safe.  */
@@ -715,7 +724,16 @@ update_alignment_for_field (record_layou
 	    type_align = ADJUST_FIELD_ALIGN (field, type_align);
 #endif
 
-	  if (maximum_field_alignment != 0)
+	  /* Targets might chose to handle unnamed and hence possibly
+	     zero-width bitfield.  Those are not influenced by #pragmas
+	     or packed attributes.  */
+	  if (integer_zerop (DECL_SIZE (field)))
+	    {
+	      if (initial_max_fld_align)
+	        type_align = MIN (type_align,
+				  initial_max_fld_align * BITS_PER_UNIT);
+	    }
+	  else if (maximum_field_alignment != 0)
 	    type_align = MIN (type_align, maximum_field_alignment);
 	  else if (DECL_PACKED (field))
 	    type_align = MIN (type_align, BITS_PER_UNIT);
@@ -1177,6 +1195,10 @@ place_field (record_layout_info rli, tre
   if (DECL_BIT_FIELD_TYPE (field))
     {
       unsigned int type_align = TYPE_ALIGN (type);
+      unsigned int mfa = maximum_field_alignment;
+
+      if (integer_zerop (DECL_SIZE (field)))
+        mfa = initial_max_fld_align * BITS_PER_UNIT;
 
       /* Only the MS bitfields use this.  We used to also put any kind of
 	 packed bit fields into prev_field, but that makes no sense, because
@@ -1185,8 +1207,8 @@ place_field (record_layout_info rli, tre
 	 are also not fulfilled.
 	 There is no sane value to set rli->remaining_in_alignment to when
 	 a packed bitfield in prev_field is unaligned.  */
-      if (maximum_field_alignment != 0)
-	type_align = MIN (type_align, maximum_field_alignment);
+      if (mfa != 0)
+	type_align = MIN (type_align, mfa);
       gcc_assert (rli->prev_field
 		  || actual_align >= type_align || DECL_PACKED (field)
 		  || integer_zerop (DECL_SIZE (field))
Index: doc/extend.texi
===================================================================
--- doc/extend.texi	(revision 110982)
+++ doc/extend.texi	(working copy)
@@ -3329,9 +3329,10 @@ alignment.  See your linker documentatio
 
 @item packed
 This attribute, attached to @code{struct} or @code{union} type
-definition, specifies that each member of the structure or union is
-placed to minimize the memory required.  When attached to an @code{enum}
-definition, it indicates that the smallest integral type should be used.
+definition, specifies that each member (other than zero-width bitfields)
+of the structure or union is placed to minimize the memory required.  When
+attached to an @code{enum} definition, it indicates that the smallest
+integral type should be used.
 
 @opindex fshort-enums
 Specifying this attribute for @code{struct} and @code{union} types is
@@ -9518,10 +9519,10 @@ way of knowing that that happened.)
 @subsection Structure-Packing Pragmas
 
 For compatibility with Win32, GCC supports a set of @code{#pragma}
-directives which change the maximum alignment of members of structures,
-unions, and classes subsequently defined.  The @var{n} value below always
-is required to be a small power of two and specifies the new alignment
-in bytes.
+directives which change the maximum alignment of members of structures
+(other than zero-width bitfields), unions, and classes subsequently
+defined.  The @var{n} value below always is required to be a small power
+of two and specifies the new alignment in bytes.
 
 @enumerate
 @item @code{#pragma pack(@var{n})} simply sets the new alignment.



More information about the Gcc-patches mailing list