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]

ms-bitfield patch



As mentioned before, the MS bitfield code can give random results.
I have observed this for the tmpdir-gcc.dg-struct-layout-1/t011 and
tmpdir-gcc.dg-struct-layout-1/t024 tests.  In t024, test2154 / check2154
sometimes fails on sh64-elf for -m5-compact/-ml, depending on the
pathname of the compiler, particularily if no debugging information
is generated, and no whitespace is added to the source.

The difference comes down to the layout of struct S2154.

struct S2154 { _Complex signed char a;__attribute__((aligned (4))) _Bool b;unsigned long int c;__attribute__((aligned (4))) unsigned short int d;signed char __attribute__((packed)) e:((((8) - 1) & 7) + 1);signed char f:((((2) - 1) & 7) + 1);long long int __attribute__((aligned (2))) g;int * h; } ; struct S2154 s2154; extern struct S2154 a2154[5]; 

The correct offset for g is 16, but sometimes we get 24 instead.
We get 24 when prev_field is set to f when we process g.
prev_field should be set to f when we process f, but
rli->remaining_in_alignment is oninitialized, and if it contains a value
of 2 or more, the ms bitfield logic thinks it is putting f in the same byte
as e.

When fixing just this problem, we get a a consistently wrong result.
g has an alignment attribute, and place_field accordingly adjusts
rli->offset, and zeroes rli->bitpos.  The ms bitfield code is unaware
that this adjustment has taken place, and changes bitpos to the end
of the ms bitfield allocation - within the word the bitfield had
been allocated.

On the one hand, by fixing these bugs, we loose a bit of backwards
compatibility with previous gcc versions, but on the other hand,
this code is supposed to give data layout compatibility with other
compilers.  If we don't fix this in a gcc release with a major version
change, when can we?

I haven't tested this patch yet except as to confirm that it gives
the expected result for S2154.

2004-12-03  J"orn Rennecke <joern.rennecke@st.com>

	* stor-layout.c (place_field): Don't adjust rli->bitpos at the end
	of an MS bitfield run if we already adjusted bitpos/offset for an
	alignment as large or larger than the bitfield type size.
	Also set rli->remaining_in_alignment when we reset rli->prev_field.

Index: stor-layout.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/stor-layout.c,v
retrieving revision 1.217
diff -p -r1.217 stor-layout.c
*** stor-layout.c	20 Oct 2004 13:14:01 -0000	1.217
--- stor-layout.c	3 Dec 2004 22:41:36 -0000
*************** place_field (record_layout_info rli, tre
*** 1032,1040 ****
  		{
  		  tree type_size = TYPE_SIZE (TREE_TYPE (rli->prev_field));
  
! 		  rli->bitpos
! 		    = size_binop (PLUS_EXPR, type_size,
! 				  DECL_FIELD_BIT_OFFSET (rli->prev_field));
  		}
  	      else
  		/* We "use up" size zero fields; the code below should behave
--- 1032,1043 ----
  		{
  		  tree type_size = TYPE_SIZE (TREE_TYPE (rli->prev_field));
  
! 		  /* If the desired alignment greater or equal to TYPE_SIZE, we
! 		     have already adjusted rli->bitpos / rli->offset above.  */
! 		  if (tree_low_cst (type_size, 0) > desired_align)
! 		    rli->bitpos
! 		      = size_binop (PLUS_EXPR, type_size,
! 				    DECL_FIELD_BIT_OFFSET (rli->prev_field));
  		}
  	      else
  		/* We "use up" size zero fields; the code below should behave
*************** place_field (record_layout_info rli, tre
*** 1131,1139 ****
    if (known_align != actual_align)
      layout_decl (field, actual_align);
  
!   /* Only the MS bitfields use this.  */
!   if (rli->prev_field == NULL && DECL_BIT_FIELD_TYPE(field))
        rli->prev_field = field;
  
    /* Now add size of this field to the size of the record.  If the size is
       not constant, treat the field as being a multiple of bytes and just
--- 1134,1148 ----
    if (known_align != actual_align)
      layout_decl (field, actual_align);
  
!   /* Only the MS bitfields use this.  In particular, this is needed when
!      FIELD is a packed bitfield which is followed by a non-packed field.  */
!   if (rli->prev_field == NULL && DECL_BIT_FIELD_TYPE (field))
!     {
        rli->prev_field = field;
+       rli->remaining_in_alignment
+ 	= tree_low_cst (TYPE_SIZE (TREE_TYPE (field)), 0)
+ 			- tree_low_cst (DECL_SIZE (field), 0);
+     }
  
    /* Now add size of this field to the size of the record.  If the size is
       not constant, treat the field as being a multiple of bytes and just

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