Hi Neil,
On 2 Oct 2016, at 23:32, NeilBrown [off-list ref] wrote:
quoted
On Thu, Sep 29 2016, James Clarke wrote:
Since the MBR layout only has partition records as 2-byte aligned, the 32-bit
fields in them are not aligned. Thus, they cannot be accessed on some
architectures (such as SPARC) by using a "struct MBR_part_record *" pointer,
as the compiler can assume that the pointer is properly aligned. Instead, the
records must be accessed by going through the MBR struct itself every time.
Weird....
Can you see if adding "__attribute__((packed))" to struct
MBR_part_record also fixes the problem?
That also works. When I wrote the patch initially, I wasn’t sure if it was a
"correct" fix, but having looked into it more I *believe* it is conformant. The
alignment of a packed struct is 1-byte, so, while the compiler may know that the
32-bit fields are 8-byte aligned within the struct, the pointer to the struct
need not be aligned, and so the correct conservative code is generated.
It seems strange that the compiler lets you take a pointer, but then
doesn't use it correctly. Maybe it is an inconsistency in the types.
Yes, the type doesn’t include the provenance of the pointer, so in general the
compiler can’t know it came from a packed struct (although in this case not much
static analysis would be needed). See [1] and [2].
I don't necessarily disagree with your fix, but I'd like to understand
why the current code is wrong.
Hopefully the links make it clearer.
Regards,
James
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628
[2] https://llvm.org/bugs/show_bug.cgi?id=22821