This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Don't ignore packed on char bitfields (4.0 regression)
- From: Ian Lance Taylor <iant at google dot com>
- To: Adam Nemet <anemet at caviumnetworks dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Wed, 21 Jan 2009 08:00:31 -0800
- Subject: Re: [PATCH] Don't ignore packed on char bitfields (4.0 regression)
- References: <18806.13045.677719.384942@foo.home>
Adam Nemet <anemet@caviumnetworks.com> writes:
> * c-decl.c (finish_struct): Move code to set DECL_PACKED after
> DECL_BIT_FIELD is alreay known. Also inherit packed for fields
> that are bitfield regardless of their type.
> * c-common.c (handle_packed_attribute): Don't ignore packed on
> bitfields.
> * c.opt (Wpacked-bitfield-compat): New warning option.
> * stor-layout.c (place_field): Warn if offset of a field changed.
> * doc/extend.texi (packed): Mention the ABI change.
> * doc/invoke.texi (-Wpacked-bitfield-compat): Document.
> (Warning Options): Add it to the list.
>
> testsuite/
> * gcc.dg/bitfld-15.c, gcc.dg/bitfld-16.c,
> gcc.dg/bitfld-17.c,gcc.dg/bitfld-18.c: New tests.
Please add some C++ test cases along the lines of your C test cases.
You may need to make a change to check_field_decls in cp/class.c
similar to your change to finish_struct in c-decl.c.
> Index: stor-layout.c
> ===================================================================
> --- stor-layout.c (revision 143450)
> +++ stor-layout.c (working copy)
> @@ -937,7 +937,9 @@ place_field (record_layout_info rli, tre
> && TREE_CODE (field) == FIELD_DECL
> && type != error_mark_node
> && DECL_BIT_FIELD (field)
> - && ! DECL_PACKED (field)
> + && (! DECL_PACKED (field)
> + /* Enter for these packed fields only to issue a warning. */
> + || TYPE_ALIGN (type) <= BITS_PER_UNIT)
> && maximum_field_alignment == 0
> && ! integer_zerop (DECL_SIZE (field))
> && host_integerp (DECL_SIZE (field), 1)
> @@ -958,12 +960,25 @@ place_field (record_layout_info rli, tre
> /* A bit field may not span more units of alignment of its type
> than its type itself. Advance to next boundary if necessary. */
> if (excess_unit_span (offset, bit_offset, field_size, type_align, type))
> - rli->bitpos = round_up (rli->bitpos, type_align);
> + {
> + if (DECL_PACKED (field))
> + {
> + if (warn_packed_bitfield_compat)
> + inform
> + (input_location,
> + "Offset of packed bit-field %qD has changed in GCC 4.4",
> + field);
> + goto error_done;
> + }
> +
> + rli->bitpos = round_up (rli->bitpos, type_align);
> + }
>
> TYPE_USER_ALIGN (rli->t) |= TYPE_USER_ALIGN (type);
> }
> #endif
>
> + error_done:
> #ifdef BITFIELD_NBYTES_LIMITED
> if (BITFIELD_NBYTES_LIMITED
> && ! targetm.ms_bitfield_layout_p (rli->t)
I don't see any need for a goto here, and it is problematic because it
will lead to an unused label warning in some cases. Just set a flag
variable to avoid setting TYPE_USER_ALIGN.
This is OK with those changes.
Thanks.
Ian