__attribute__((packed)) vs. PCC_BITFIELD_TYPE_MATTERS (fwd)

__attribute__((packed)) vs. PCC_BITFIELD_TYPE_MATTERS (fwd)

[Joern Rennecke]

I hoped that Kenner would now at least get small bug fixes in a more timely
manner, so I could expect these go to egcs by the FSF merges -  but maybe
he don't think this patch is small, or whatever.  Well, here it goes:

Forwarded message:
From: Joern Rennecke <amylaar@cygnus.co.uk>
Message-Id: <199708131920.UAA03465@phal.cygnus.co.uk>
Subject: __attribute__((packed)) vs. PCC_BITFIELD_TYPE_MATTERS
To: kenner@vlsi1.ultra.nyu.edu (Richard Kenner)
Date: Wed, 13 Aug 1997 20:20:18 +0100 (BST)
X-Mailer: ELM [version 2.4 PL25]
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 8bit

Bitfield structure members do not work as expected when the tm.h file
defines PCC_BITFIELD_TYPE_MATTERS and the ((packed)) attribute has been
provided in the declaration of the field.
The gcc documentation states that this attribute causes a field to
have minimum alignment.  The alignment of the field itself is
indeed adjusted accordingly, but when PCC_BITFIELD_TYPE_MATTERS is
defined, the alignment of the structure is increased on its behalf to
that of the alignment of the base type of the field.
This is because some code in layout_record tests TYPE_PACKED instead of
DECL_PACKED.

Consider these structure definitions:

typedef struct
{
  unsigned int  i:24 __attribute__ ((packed));
}
BadStruct;

typedef struct
{
  char a,b,c;
}
GoodStruct;

int main()
{
  int i, j;

  printf("sizeof (BadStruct) = %d\n", sizeof (BadStruct));
  printf("sizeof (GoodStruct) = %d\n", sizeof (GoodStruct));
}

On a SunOs 4 host, this yields 4 for GoodStruct, but 3 for BadStruct.

There is one other place using TYPE_PACKED in similar code which should
also be fixed.  This is the PCC_BITFIELD_NBYTES_LIMITED code.  However,
this code is already unreachable, though it is not obivous.  Fixing
this problems makes the code obviously unreachable.

Appended is a patch from Jim Wilson that fixes these problems.

The PCC_BITFIELD_NBYTES_LIMITED code also has the problem that it doesn't
handle bitfields based on types that span multiple alignment units correctly.
Do you want me to make a patch to bring the PCC_BITFIELD_NBYTES_LIMITED
back into sync with the PCC_BITFIELD_TYPE_MATTERS code?

Or should it rather be removed and handled by the PCC_BITFIELD_TYPE_MATTERS
code just above?  That schould keep it in sync for good.

Or should PCC_BITFIELD_NBYTES_LIMITED support just be removed, since no port
uses it?


Thu Apr 10 16:34:52 1997  Jim Wilson  <wilson@cygnus.com>
 
        * stor-layout.c (layout_record): Test DECL_PACKED instead of
        TYPE_PACKED to determine alignment.

*** clean-ss-970404/stor-layout.c	Mon Mar 17 14:13:23 1997
--- ss-970404/stor-layout.c	Thu Apr 10 16:34:52 1997
*************** layout_record (rec)
*** 389,395 ****
  	      int type_align = TYPE_ALIGN (TREE_TYPE (field));
  	      if (maximum_field_alignment != 0)
  		type_align = MIN (type_align, maximum_field_alignment);
! 	      else if (TYPE_PACKED (rec))
  		type_align = MIN (type_align, BITS_PER_UNIT);
  
  	      record_align = MAX (record_align, type_align);
--- 389,395 ----
  	      int type_align = TYPE_ALIGN (TREE_TYPE (field));
  	      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);
  
  	      record_align = MAX (record_align, type_align);
*************** layout_record (rec)
*** 462,472 ****
  
  	  if (maximum_field_alignment != 0)
  	    type_align = MIN (type_align, maximum_field_alignment);
! 	  else if (TYPE_PACKED (rec))
  	    type_align = MIN (type_align, BITS_PER_UNIT);
  
  	  /* A bit field may not span the unit of alignment of its type.
  	     Advance to next boundary if necessary.  */
  	  if (const_size / type_align
  	      != (const_size + field_size - 1) / type_align)
  	    const_size = CEIL (const_size, type_align) * type_align;
--- 462,476 ----
  
  	  if (maximum_field_alignment != 0)
  	    type_align = MIN (type_align, maximum_field_alignment);
! 	  /* ??? This test is opposite the test in the containing if
! 	     statement, so this code is unreachable currently.  */
! 	  else if (DECL_PACKED (field))
  	    type_align = MIN (type_align, BITS_PER_UNIT);
  
  	  /* A bit field may not span the unit of alignment of its type.
  	     Advance to next boundary if necessary.  */
+ 	  /* ??? This code should match the code above for the
+ 	     PCC_BITFIELD_TYPE_MATTERS case.  */
  	  if (const_size / type_align
  	      != (const_size + field_size - 1) / type_align)
  	    const_size = CEIL (const_size, type_align) * type_align;