Hi! The following patch fixes the reported ms_struct/-mms-bitfields structure layout issues from PR52991.
There are multiple issues, two of them introduced by the https://gcc.gnu.org/ml/gcc-patches/2006-04/msg01064.html -mms-bitfields revamp from Eric and follow-up fix r114552, the rest has been introduced later when the known_align < desired_align case has been enabled for the ms bitfield layout. The first 2 hunks fix alignment of packed non-bitfield fields, we can't ignore all the alignment updates for them, just should use only desired_align which takes DECL_PACKED into account, rather than MAX (type_align, desired_align). Similarly, the last hunk in stor-layout.c makes sure that for DECL_PACKED fields we use BITS_PER_UNIT alignment rather than the type alignment. The rest attempts to unbreak r184409 which enabled known_align < desired_align case; doing that if rli->prev_field and ms layout is wrong, we first need to deal with the bitfield packing and if we are within a bitfield word, we shouldn't do any realignment, only in between them. The patch reverts changes to bf-ms-layout{,-2}.c tests done in 2012, which were done just to match the r184409 changes, and adds 2 new tests. All of these 4 I've tested (slightly tweaked, so that it compiles with VC) with the online VC compiler http://rextester.com/l/c_online_compiler_visual . Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2018-02-27 Jakub Jelinek <ja...@redhat.com> PR target/52991 * stor-layout.c (update_alignment_for_field): For targetm.ms_bitfield_layout_p (rli->t), if !is_bitfield && !DECL_PACKED (field), do the alignment update, just use only desired_align instead of MAX (type_align, desired_align) as the alignment. (place_field): Don't do known_align < desired_align handling early if targetm.ms_bitfield_layout_p (rli->t) and rli->prev_field is non-NULL, instead do it after rli->prev_field handling and only if not within a bitfield word. For DECL_PACKED (field) use type_align of BITS_PER_UNIT. * gcc.dg/bf-ms-layout.c: Revert 2012-04-26 changes. * gcc.dg/bf-ms-layout-2.c: Revert 2012-02-23 changes. * gcc.dg/bf-ms-layout-4.c: New test. * gcc.dg/bf-ms-layout-5.c: New test. --- gcc/stor-layout.c.jj 2018-02-22 14:35:33.135216198 +0100 +++ gcc/stor-layout.c 2018-02-27 18:56:26.906494801 +0100 @@ -1038,7 +1038,7 @@ update_alignment_for_field (record_layou the type, except that for zero-size bitfields this only applies if there was an immediately prior, nonzero-size bitfield. (That's the way it is, experimentally.) */ - if ((!is_bitfield && !DECL_PACKED (field)) + if (!is_bitfield || ((DECL_SIZE (field) == NULL_TREE || !integer_zerop (DECL_SIZE (field))) ? !DECL_PACKED (field) @@ -1047,7 +1047,10 @@ update_alignment_for_field (record_layou && ! integer_zerop (DECL_SIZE (rli->prev_field))))) { unsigned int type_align = TYPE_ALIGN (type); - type_align = MAX (type_align, desired_align); + if (!is_bitfield && DECL_PACKED (field)) + type_align = desired_align; + else + type_align = MAX (type_align, desired_align); if (maximum_field_alignment != 0) type_align = MIN (type_align, maximum_field_alignment); rli->record_align = MAX (rli->record_align, type_align); @@ -1303,7 +1306,9 @@ 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? */ - if (known_align < desired_align) + if (known_align < desired_align + && (! targetm.ms_bitfield_layout_p (rli->t) + || rli->prev_field == NULL)) { /* No, we need to skip space before this field. Bump the cumulative size to multiple of field alignment. */ @@ -1331,8 +1336,6 @@ 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 @@ -1448,6 +1451,8 @@ place_field (record_layout_info rli, tre /* This is a bitfield if it exists. */ if (rli->prev_field) { + bool realign_p = known_align < desired_align; + /* If both are bitfields, nonzero, and the same size, this is the middle of a run. Zero declared size fields are special and handled as "end of run". (Note: it's nonzero declared @@ -1481,7 +1486,10 @@ place_field (record_layout_info rli, tre rli->remaining_in_alignment = typesize - bitsize; } else - rli->remaining_in_alignment -= bitsize; + { + rli->remaining_in_alignment -= bitsize; + realign_p = false; + } } else { @@ -1512,6 +1520,31 @@ place_field (record_layout_info rli, tre rli->prev_field = NULL; } + /* Does this field automatically have alignment it needs by virtue + of the fields that precede it and the record's own alignment? */ + if (realign_p) + { + /* If the alignment is still within offset_align, just align + the bit position. */ + if (desired_align < rli->offset_align) + rli->bitpos = round_up (rli->bitpos, desired_align); + else + { + /* First adjust OFFSET by the partial bits, then align. */ + tree d = size_binop (CEIL_DIV_EXPR, rli->bitpos, + bitsize_unit_node); + rli->offset = size_binop (PLUS_EXPR, rli->offset, + fold_convert (sizetype, d)); + rli->bitpos = bitsize_zero_node; + + rli->offset = round_up (rli->offset, + desired_align / BITS_PER_UNIT); + } + + if (! TREE_CONSTANT (rli->offset)) + rli->offset_align = desired_align; + } + normalize_rli (rli); } @@ -1530,7 +1563,7 @@ place_field (record_layout_info rli, tre if (!DECL_BIT_FIELD_TYPE (field) || (prev_saved != NULL ? !simple_cst_equal (TYPE_SIZE (type), TYPE_SIZE (prev_type)) - : !integer_zerop (DECL_SIZE (field)) )) + : !integer_zerop (DECL_SIZE (field)))) { /* Never smaller than a byte for compatibility. */ unsigned int type_align = BITS_PER_UNIT; @@ -1555,7 +1588,8 @@ place_field (record_layout_info rli, tre } /* Now align (conventionally) for the new type. */ - type_align = TYPE_ALIGN (TREE_TYPE (field)); + if (! DECL_PACKED (field)) + type_align = TYPE_ALIGN (TREE_TYPE (field)); if (maximum_field_alignment != 0) type_align = MIN (type_align, maximum_field_alignment); --- gcc/testsuite/gcc.dg/bf-ms-layout.c.jj 2017-06-20 21:38:01.634906200 +0200 +++ gcc/testsuite/gcc.dg/bf-ms-layout.c 2018-02-27 18:04:06.923851839 +0100 @@ -153,27 +153,27 @@ int main(){ struct ten test_ten; #if defined (_TEST_MS_LAYOUT) || defined (_MSC_VER) - size_t exp_sizeof_one = 8; - size_t exp_sizeof_two = 12; + size_t exp_sizeof_one = 12; + size_t exp_sizeof_two = 16; size_t exp_sizeof_three =6; size_t exp_sizeof_four = 8; size_t exp_sizeof_five = 3; size_t exp_sizeof_six = 8; size_t exp_sizeof_seven = 3; - size_t exp_sizeof_eight = 2; + size_t exp_sizeof_eight = 4; size_t exp_sizeof_nine = 8; - size_t exp_sizeof_ten = 8; + size_t exp_sizeof_ten = 16; - unsigned char exp_one_c = 7; - unsigned char exp_two_c = 9; + unsigned char exp_one_c = 8; + unsigned char exp_two_c = 12; unsigned char exp_three_c = 4; unsigned char exp_four_c = 4; char exp_five_c = 2; char exp_six_c = 5; char exp_seven_c = 2; - char exp_eight_c = 1; + char exp_eight_c = 2; char exp_nine_c = 0; - char exp_ten_c = 1; + char exp_ten_c = 8; #else /* testing -mno-ms-bitfields */ --- gcc/testsuite/gcc.dg/bf-ms-layout-2.c.jj 2017-06-20 21:38:02.112900704 +0200 +++ gcc/testsuite/gcc.dg/bf-ms-layout-2.c 2018-02-27 18:04:06.923851839 +0100 @@ -158,27 +158,27 @@ int main(){ struct ten test_ten; #if defined (_TEST_MS_LAYOUT) || defined (_MSC_VER) - size_t exp_sizeof_one = 8; - size_t exp_sizeof_two = 12; + size_t exp_sizeof_one = 12; + size_t exp_sizeof_two = 16; size_t exp_sizeof_three =6; size_t exp_sizeof_four = 8; size_t exp_sizeof_five = 3; size_t exp_sizeof_six = 8; size_t exp_sizeof_seven = 3; - size_t exp_sizeof_eight = 2; + size_t exp_sizeof_eight = 4; size_t exp_sizeof_nine = 8; - size_t exp_sizeof_ten = 8; + size_t exp_sizeof_ten = 16; - unsigned char exp_one_c = 7; - unsigned char exp_two_c = 9; + unsigned char exp_one_c = 8; + unsigned char exp_two_c = 12; unsigned char exp_three_c = 4; unsigned char exp_four_c = 4; char exp_five_c = 2; char exp_six_c = 5; char exp_seven_c = 2; - char exp_eight_c = 1; + char exp_eight_c = 2; char exp_nine_c = 0; - char exp_ten_c = 1; + char exp_ten_c = 8; #else /* testing -mno-ms-bitfields */ --- gcc/testsuite/gcc.dg/bf-ms-layout-4.c.jj 2018-02-27 18:09:09.544421580 +0100 +++ gcc/testsuite/gcc.dg/bf-ms-layout-4.c 2018-02-27 18:18:00.845039925 +0100 @@ -0,0 +1,43 @@ +/* PR target/52991 */ +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */ + +#define CHECK(expr) extern char c[(expr) ? 1 : -1] +#define offsetof(x, y) __builtin_offsetof (x, y) + +struct test_sp1 { + int a; + short b; + int c; + char d; +} __attribute__((packed,ms_struct)); + +CHECK (sizeof (struct test_sp1) == 11); +CHECK (offsetof (struct test_sp1, a) == 0); +CHECK (offsetof (struct test_sp1, b) == 4); +CHECK (offsetof (struct test_sp1, c) == 6); +CHECK (offsetof (struct test_sp1, d) == 10); + +struct test_sp3 { + int a; + short b __attribute__((aligned(8))); + int c; + char d; +} __attribute__((packed,ms_struct)); + +CHECK (sizeof (struct test_sp3) == 16); +CHECK (offsetof (struct test_sp3, a) == 0); +CHECK (offsetof (struct test_sp3, b) == 8); +CHECK (offsetof (struct test_sp3, c) == 10); +CHECK (offsetof (struct test_sp3, d) == 14); + +struct test_s4 { + int a; + short b; + int c:15; + char d; +} __attribute__((ms_struct)); + +CHECK (sizeof (struct test_s4) == 16); +CHECK (offsetof (struct test_s4, a) == 0); +CHECK (offsetof (struct test_s4, b) == 4); +CHECK (offsetof (struct test_s4, d) == 12); --- gcc/testsuite/gcc.dg/bf-ms-layout-5.c.jj 2018-02-27 18:31:24.043753173 +0100 +++ gcc/testsuite/gcc.dg/bf-ms-layout-5.c 2018-02-27 18:35:29.825676223 +0100 @@ -0,0 +1,45 @@ +/* PR target/52991 */ +/* { dg-do run { target i?86-*-* x86_64-*-* } } */ + +struct S { + int a : 2; + __attribute__((aligned (8))) int b : 2; + int c : 28; + __attribute__((aligned (16))) int d : 2; + int e : 30; +} __attribute__((ms_struct)); + +struct S s; + +int +main () +{ + int i; + if (sizeof (s) != 32) + __builtin_abort (); + s.a = -1; + for (i = 0; i < 32; ++i) + if (((char *) &s)[i] != (i ? 0 : 3)) + __builtin_abort (); + s.a = 0; + s.b = -1; + for (i = 0; i < 32; ++i) + if (((char *) &s)[i] != (i ? 0 : 12)) + __builtin_abort (); + s.b = 0; + s.c = -1; + for (i = 0; i < 32; ++i) + if (((signed char *) &s)[i] != (i > 3 ? 0 : (i ? -1 : -16))) + __builtin_abort (); + s.c = 0; + s.d = -1; + for (i = 0; i < 32; ++i) + if (((signed char *) &s)[i] != (i == 16 ? 3 : 0)) + __builtin_abort (); + s.d = 0; + s.e = -1; + for (i = 0; i < 32; ++i) + if (((signed char *) &s)[i] != ((i < 16 || i > 19) ? 0 : (i == 16 ? -4 : -1))) + __builtin_abort (); + return 0; +} Jakub
