Wolfgang Denk wd@denx.de writes:
Dear Tom,
In message 20140210222819.GE7049@bill-the-cat you wrote:
I agree we shouldn't use __packed without a good reason. And I don't think we've got many no-reason uses right now but I'm all in favor of dropping them and keeping an eye on new users. The problem is that unless we do something, any of the valid and we aren't going to / can't change them __packed structs will be the next place things blow up when gcc optimizes things in a new (and valid based on what we've told it!) way.
If I look around where __packed (and variants) is being used I could just cry. Just have a loog for example at include/ec_commands.h - it appears they always add __packed to all structs, including such where all entries are uint8_t - see for example struct ec_lpc_host_args :-(
This is obviously insane and should be fixed. Deliberately miscompiling other code isn't going to help with that.
Takes valid code? But would this original code run on an architecture where any unaligned access causes a machine check? My understanding is it doesn't?
It would run because being __packed gcc would know to do the right thing on that architecture.
We are not discussing application code here.
What does that have to do with anything. The compiler works exactly the same whatever the code is for.
We are dealing with low- level hardware related stuff. When I try to read from a 32 bit device register I must be absolutley sure that this will be a single 32 bit transfer. It is totally unacceptable if I have to fear that the compiler might break this up into 4 byte accesses behind my back.
So because you're afraid of __packed abuse, you want to make _other_ completely valid code crash? Would it not be preferable to make the actually broken code fail? Then someone might notice and fix it. Furthermore, the scenario you seem worried about will still happen on ARMv5 and other architectures which do not support unaligned memory accesses.