Hi Neil, > On 2 Oct 2016, at 23:32, NeilBrown <neilb@xxxxxxxx> wrote: > >> 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-- To unsubscribe from this list: send the line "unsubscribe linux-raid" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
