Patchwork [stor-layout] : Fix PR 52238 - -mms-bitfields: __attribute__ ((aligned (n))) ignored for struct members

login
register
mail settings
Submitter Kai Tietz
Date Feb. 17, 2012, 12:15 p.m.
Message ID <CAEwic4aGVwySgbTVtcEJGh6j67z4DZtYL_0fQbDkqQYy9A7MpQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/141813/
State New
Headers show

Comments

Kai Tietz - Feb. 17, 2012, 12:15 p.m.
Hi,

this patch fixes old regression about ms-bitfields structure layout.
This patch was
verified by VLC community and libAV people (as to see in bug-report).
The result of
the structure-layout was also compared to VC's 32-bit and 64-bit
result by VLC people.

ChangeLog

2012-02-17  Kai Tietz  <ktietz@redhat.com>

	PR target/52238
	* stor-layout.c (place_field): Handle desired_align for
	ms-bitfields, too.

2012-02-17  Kai Tietz  <ktietz@redhat.com>

	* gcc.dg/bf-ms-layout-3.c: New testcase.

Tested for i686-w64-mingw32, x86_64-w64-mingw32, i686-pc-cygwin.  Ok for apply?

Regards,
Kai

+
Richard Henderson - Feb. 20, 2012, 9:49 p.m.
On 02/17/12 04:15, Kai Tietz wrote:
> 2012-02-17  Kai Tietz  <ktietz@redhat.com>
> 
> 	PR target/52238
> 	* stor-layout.c (place_field): Handle desired_align for
> 	ms-bitfields, too.
> 
> 2012-02-17  Kai Tietz  <ktietz@redhat.com>
> 
> 	* gcc.dg/bf-ms-layout-3.c: New testcase.

As I mentioned on IRC, please use the attribute instead of -mms-bitfields.

Otherwise ok.


r~
Kai Tietz - Feb. 20, 2012, 10:11 p.m.
2012/2/20 Richard Henderson <rth@redhat.com>:
> On 02/17/12 04:15, Kai Tietz wrote:
>> 2012-02-17  Kai Tietz  <ktietz@redhat.com>
>>
>>       PR target/52238
>>       * stor-layout.c (place_field): Handle desired_align for
>>       ms-bitfields, too.
>>
>> 2012-02-17  Kai Tietz  <ktietz@redhat.com>
>>
>>       * gcc.dg/bf-ms-layout-3.c: New testcase.
>
> As I mentioned on IRC, please use the attribute instead of -mms-bitfields.
>
> Otherwise ok.
>
>
> r~

Ok applied with adjusted testcase at rev. 184409 to trunk, and
back-merged change to 4.6.x branch at rev 184410.

This bug reaches back at least to 4.2, but I think a back-merge to 4.6
is enough here.

Thanks,
Kai

Patch

Index: gcc/gcc/stor-layout.c
===================================================================
--- gcc.orig/gcc/stor-layout.c
+++ gcc/gcc/stor-layout.c
@@ -1141,15 +1141,14 @@  place_field (record_layout_info rli, tre
     }

   /* Does this field automatically have alignment it needs by virtue
-     of the fields that precede it and the record's own alignment?
-     We already align ms_struct fields, so don't re-align them.  */
-  if (known_align < desired_align
-      && !targetm.ms_bitfield_layout_p (rli->t))
+     of the fields that precede it and the record's own alignment?  */
+  if (known_align < desired_align)
     {
       /* No, we need to skip space before this field.
 	 Bump the cumulative size to multiple of field alignment.  */

-      if (DECL_SOURCE_LOCATION (field) != BUILTINS_LOCATION)
+      if (!targetm.ms_bitfield_layout_p (rli->t)
+          && DECL_SOURCE_LOCATION (field) != BUILTINS_LOCATION)
 	warning (OPT_Wpadded, "padding struct to align %q+D", field);

       /* If the alignment is still within offset_align, just align
@@ -1171,7 +1170,8 @@  place_field (record_layout_info rli, tre

       if (! TREE_CONSTANT (rli->offset))
 	rli->offset_align = desired_align;
-
+      if (targetm.ms_bitfield_layout_p (rli->t))
+	rli->prev_field = NULL;
     }

   /* Handle compatibility with PCC.  Note that if the record has any
Index: gcc/gcc/testsuite/gcc.dg/bf-ms-layout-3.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.dg/bf-ms-layout-3.c
@@ -0,0 +1,47 @@ 
+/* Test for MS bitfield layout */
+/* { dg-do run { target *-*-interix* *-*-mingw* *-*-cygwin*
i?86-*-darwin* } } */
+/* { dg-options "-mms-bitfields" } */
+
+extern void abort();
+
+struct {
+    char a;
+    char b __attribute__ ((aligned (16)));
+} s1;
+
+struct {
+  char a;
+  char b;
+} s2;
+
+struct {
+  char a : 6;
+  char b __attribute__ ((aligned (16)));
+} s3;
+
+struct {
+  char a : 6;
+  char b __attribute__ ((aligned (2)));
+} s4;
+
+struct {
+  char a : 6;
+  char b __attribute__ ((aligned (1)));
+} s5;
+
+__PTRDIFF_TYPE__ offs (const void *a, const void *b)
+{
+  return (__PTRDIFF_TYPE__) ((const char*)a  - (const char*)b);
+}
+
+int main()
+{
+  if (offs (&s1.b, &s1) != 16
+      || offs (&s2.b, &s2) != 1
+      || offs (&s3.b, &s3) != 16
+      || offs (&s4.b, &s4) != 2
+      || offs (&s5.b, &s5) != 1)
+    abort ();
+  return 0;
+}