[Bug target/67246] MIPS: lw (load word) is generated for byte bitfield, leading to unaligned access

["pinskia at gcc dot gnu.org" <gcc-bugzilla at gcc dot gnu dot org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67246

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Thomas Martitz from comment #2)
> Thanks for your immediate reply.
> 
> In trying to provide a better test case I think I've found what the culprit
> is.
> 
> The full iphdr defintion is:
> 
> struct iphdr {
> #if defined(__LITTLE_ENDIAN_BITFIELD)
> 	__u8	ihl:4,
> 		version:4;
> #elif defined (__BIG_ENDIAN_BITFIELD)
> 	__u8	version:4,
>   		ihl:4;
> #else
> #error	"Please fix <asm/byteorder.h>"
> #endif
> 	__u8	tos;
> 	__be16	tot_len;
> 	__be16	id;
> 	__be16	frag_off;
> 	__u8	ttl;
> 	__u8	protocol;
> 	__sum16	check;
> 	__be32	saddr;
> 	__be32	daddr;
> 	/*The options start here. */
> };
> 
> Note that it ends with 32bit saddr and daddr fields.
> 
> If I change these to __be16, then the following code is generated:
>  248:   96220000        lhu     v0,0(s1)
>  24c:   8fa40044        lw      a0,68(sp)
>  250:   7c421a00        ext     v0,v0,0x8,0x4
>  254:   00021080        sll     v0,v0,0x2
>  258:   00821021        addu    v0,a0,v0
> 
> So I guess that means the 32bit fields change the alignment of the whole
> struct to 4 byte. And then the compiler assumes iph must be 4-byte aligned
> (in a conformant program).

yes that is correct.  The struct has alignment of 4 byte when there is a field
of 4 bytes long (unless you mark the struct to have a different alignment).

> 
> The linux code carefully avoids accessing saddr and daddr fields near this
> code (not shown above) so I guess it's aware of the potentially unaligned
> access.
> 
> So I'm not sure anymore who's on fault now

The linux kernel fault.  Does not matter what about the code around it or not.