RE: [PATCH] Fix another wrong-code bug with -fstrict-volatile-bitfields

[Bernd Edlinger <bernd dot edlinger at hotmail dot de>]

Hi,


when looking at the m68k I realized the following, which is
a general problem...

If the alignment of the structure is less than sizeof(field), the
strict volatile bitfields code may read beyond the end of the
structure!

Consider this example:

struct s
{
  char x : 8;
  volatile unsigned int y : 31;
  volatile unsigned int z : 1;
} __attribute__((packed));

struct s global;


Here we have sizeof(struct s) = 5, alignment(global) == 1,
However when we access global.z we read a 32-bit word
at offset 4, which touches 3 bytes that are not safe to use. 

Something like that does never happen with -fno-strict-volatile-bitfields,
because IIRC, with the only exception of the simple_mem_bitfield_p code path,
there is never an access mode used which is larger than MEM_ALIGN(x).

In this example, if I want to use the packed attribute,
I also have to use the aligned(4) attribute, this satisfies the
check "MEM_ALIGN (op0) < modesize", which is IMO always necessary
for strict volatile bitfields, not only on STRICT_ALIGNMENT targets.

On a target, that has BIGGEST_ALIGNMENT < BITS_PER_WORD,
to use the strict volatile bitfields, you have to add the __attribute__((aligned(4)))
to the structure.

I had to do that on the pr23623.c test case, to have it passed on m68k for instance.


I have attached the updated patch.  As explained before, the check
MEM_ALIGN (op0) < modesize should always be done in strict_volatile_bitfield_p.

For the targets, that usually enable -fstrict-volatile-bitfields, nothing changes,
Except when we use "packed" on the structure, we need to add also an aligned(4)
attribute. For m68k where the natural alignment of any structure is <=2 we need to
force aligned(4) if we want to ensure the access is in SImode.

Boot-strapped and reg-tested on x86_64-linux-gnu.
OK for trunk?


Thanks
Bernd.

Attachment: changelog-volatile-bitfields-1.txt
Description: Text document

Attachment: patch-volatile-bitfields-1.diff
Description: Binary data