On Tue, Dec 11, 2012 at 11:04 PM, Kai Tietz <[email protected]> wrote:
> Hello,
>
> This fixes an old regression about ms-structure-layout in combination
> with packed-attribute.
>
> ChangeLog
>
> 2012-12-11 Kai Tietz
>
> PR c/52991
> * stor-layout.c (start_record_layout): Handle
> packed-attribute for ms-structure-layout.
> (update_alignment_for_field): Likewise.
> (place_field): Likewise.
>
>
> Tested for i686-w64-mingw32, x86_64-w64-mingw32, and for
> x86_64-unknown-linux-gnu. Ok for apply?
Please add a testcase or two.
> Regards,
> Kai
>
> Index: stor-layout.c
> ===================================================================
> --- stor-layout.c (Revision 194386)
> +++ stor-layout.c (Arbeitskopie)
> @@ -756,7 +756,10 @@ start_record_layout (tree t)
> /* If the type has a minimum specified alignment (via an attribute
> declaration, for example) use it -- otherwise, start with a
> one-byte alignment. */
> - rli->record_align = MAX (BITS_PER_UNIT, TYPE_ALIGN (t));
> + if (TYPE_PACKED (t))
> + rli->record_align = BITS_PER_UNIT;
> + else
> + rli->record_align = MAX (BITS_PER_UNIT, TYPE_ALIGN (t));
That looks wrong. Having both TYPE_PACKED and TYPE_ALIGN != BITS_PER_UNIT
is inconsistent, so this part of the patch should not be necessary.
> rli->unpacked_align = rli->record_align;
> rli->offset_align = MAX (rli->record_align, BIGGEST_ALIGNMENT);
>
> @@ -952,15 +955,20 @@ update_alignment_for_field (record_layout_info rli
> meaningless. */
> if (targetm.ms_bitfield_layout_p (rli->t))
> {
> + if (rli->t && TYPE_PACKED (rli->t)
> + && (is_bitfield || !DECL_PACKED (field)
> + || DECL_SIZE (field) == NULL_TREE
> + || !integer_zerop (DECL_SIZE (field))))
> + desired_align = BITS_PER_UNIT;
See above - this looks redundant.
> /* Here, the alignment of the underlying type of a bitfield can
> affect the alignment of a record; even a zero-sized field
> can do this. The alignment should be to the alignment of
> 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))
> - || ((DECL_SIZE (field) == NULL_TREE
> - || !integer_zerop (DECL_SIZE (field)))
> + else if ((!is_bitfield && !DECL_PACKED (field))
> + || ((DECL_SIZE (field) == NULL_TREE
> + || !integer_zerop (DECL_SIZE (field)))
> ? !DECL_PACKED (field)
> : (rli->prev_field
> && DECL_BIT_FIELD_TYPE (rli->prev_field)
> @@ -1414,7 +1422,13 @@ place_field (record_layout_info rli, tree field)
> }
>
> /* Now align (conventionally) for the new type. */
> - type_align = TYPE_ALIGN (TREE_TYPE (field));
> + if (!TYPE_PACKED (rli->t))
> + {
> + type_align = TYPE_ALIGN (TREE_TYPE (field));
> + if (DECL_PACKED (field))
> + type_align = MIN (type_align, BITS_PER_UNIT);
> +
> + }
> if (maximum_field_alignment != 0)
> type_align = MIN (type_align, maximum_field_alignment);
