ABI bug: PR middle-end/22275

[Michael Matz <matz at suse dot de>]

Hi,

this is an change in behaviour from GCC pre-3.4, and one which doesn't 
make sense, namely the zero-width bitfields had no special significance 
regarding aligning following fields anymore.  Mark already gave good 
reasoning for switching back in 
  http://gcc.gnu.org/ml/gcc/2005-12/msg00558.html
without reaction.  Interesting discussion, thoughts and justification also
is in the bugreport itself, especially
  http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275#c33

One problem remains for now with being completely doing the same as 3.3, 
as it was doing insane things leading to a packed struct being larger than 
an unpacked one, and differences between a struct with attribute packed 
and one under #pragma pack(1) influence.

The below patch is the one from the bugreport, and essentially makes 
zero-width bitfields an alignment marker only, so that it's aligned 
according to it's declared type, without influence of #pragma pack or 
attribute packed.

This patch as is was tested on trunk without Ada, without regressions on 
i686,x86_64,ppc,ppc64,ia64,s390x and produced one regression in s390, 
namely
 tmpdir-g++.dg-struct-layout-1/t016 cp_compat_x_tst.o-cp_compat_y_tst.o execute
which I'm going to look into now.

I'm thinking to change the test program from 
  http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275#c42
into a testcase.

This is one of two (I know about) showstoppers for 4.1.  It would be nice 
to have some people knowing the layout code to look over it.


Ciao,
Michael.
-- 
        * stor-layout.c (layout_decl): Zero-width bit-fields aren't influenced
        by maximum_field_alignment or DECL_PACKED.
        (update_alignment_for_field): Ditto.
        (place_field): Ditto.

Index: stor-layout.c
===================================================================
--- stor-layout.c	(revision 110394)
+++ stor-layout.c	(working copy)
@@ -338,6 +338,8 @@
     /* For fields, it's a bit more complicated...  */
     {
       bool old_user_align = DECL_USER_ALIGN (decl);
+      bool zero_bitfield = false;
+      unsigned int mfa;
 
       if (DECL_BIT_FIELD (decl))
 	{
@@ -346,9 +348,9 @@
 	  /* A zero-length bit-field affects the alignment of the next
 	     field.  */
 	  if (integer_zerop (DECL_SIZE (decl))
-	      && ! DECL_PACKED (decl)
 	      && ! targetm.ms_bitfield_layout_p (DECL_FIELD_CONTEXT (decl)))
 	    {
+	      zero_bitfield = true;
 #ifdef PCC_BITFIELD_TYPE_MATTERS
 	      if (PCC_BITFIELD_TYPE_MATTERS)
 		do_type_align (type, decl);
@@ -409,12 +411,13 @@
 	 check old_user_align instead.  */
       if (DECL_PACKED (decl)
 	  && !old_user_align
+	  && !zero_bitfield
 	  && (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 (! DECL_USER_ALIGN (decl) && (! DECL_PACKED (decl) || zero_bitfield))
 	{
 	  /* Some targets (i.e. i386, VMS) limit struct field alignment
 	     to a lower boundary than alignment of variables unless
@@ -428,9 +431,13 @@
 #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.  */
@@ -714,7 +721,16 @@
 	    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);
@@ -1176,7 +1192,11 @@
   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
 	 an 8 bit packed bit field shouldn't impose more restriction on
@@ -1184,8 +1204,8 @@
 	 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))