Re: another wrong-code problem with -fstrict-volatile-bitfields

[Richard Guenther <richard dot guenther at gmail dot com>]

On Sat, Aug 25, 2012 at 10:15 PM, Sandra Loosemore
<sandra@codesourcery.com> wrote:
> While I was grovelling around trying to swap in more state on the bitfield
> store/extract code for the patch rewrite being discussed here:
>
> http://gcc.gnu.org/ml/gcc-patches/2012-08/msg01546.html
>
> I found a reference to PR23623 and found that it is broken again, but in a
> different way.  On ARM EABI, the attached test case correctly does 32-bit
> reads for the volatile int bit-field with -fstrict-volatile-bitfields, but
> incorrectly does 8-bit writes.  I thought I should try to track this down
> and fix it first, as part of making the bit-field read/extract code more
> consistent with each other, before trying to figure out a new place to hook
> in the packedp attribute stuff.  The patch I previously submitted does not
> fix the behavior of this test case for writing, nor does reverting the older
> patch that added the packedp attribute for reading break that case.
>
> After I tweaked a couple other places in store_bit_field_1 to handle
> -fstrict-volatile-bitfields consistently with extract_bit_field_1, I've
> gotten it into store_fixed_bit_field, to parallel the read case where it is
> getting into extract_fixed_bit_field.  But there it's failing to reach the
> special case for using the declared mode of the field with
> -fstrict-volatile-bitfields because it's been passed bitregion_start = 0 and
> bitregion_end = 7 so it thinks it must not write more than 8 bits no matter
> what.  Those values are coming in from expand_assignment, which is in turn
> getting them from get_bit_range.
>
> I'm really confused -- where is the right place to reconcile the new C++
> memory model with -fstrict-volatile-bitfields?

Appearantly they conflict.  The proper place to fix this is in struct-layout.c
where we compute DECL_BIT_FIELD_REPRESENTATIVE.

Your testcase is

extern struct
{
   unsigned int b : 1;
} bf1;

void writeb(void)
{
   bf1.b = 1;

what is sizeof (bf1) / alignof (bf1)?  I suppose with
-fstrict-volatile-bitfields
it is 4.  Thus make the DECL_BIT_FIELD_REPRESENTATIVE cover
tail padding in the structure for -fstrict-volatile-bitfields.  If the
size is _not_ 4
then

extern struct
{
   unsigned int b : 1;
} bf1;
char c;

may be miscompiling

c = 1;
bf1.b = 1;
return c;

if bf1 and c are adjacent in .data.

Richard.

> -Sandra
>