Re: [tip:mm/pkeys] mm/pkeys: Fix siginfo ABI breakage caused by new u64 field

[Ingo Molnar]

* Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:

> On Thu, Mar 3, 2016 at 8:53 AM, tip-bot for Dave Hansen
> <tipbot@xxxxxxxxx> wrote:
> >
> > If u64 has a natural alignment of 8 bytes (this is rare, most 32-bit
> > platforms align it to 4 bytes), then the leadup to the _sifields union
> > matters:
> 
> Side note: I'm not sure that "this is rare" comment is necessarily correct.
> 
> I think natural alignment is pretty common, even for 32-bit targets.
> x86-32 is I think the exception rather than the rule.
> 
> There is some real odd case iirc - embedded m68k, which has some
> ridiculous alignment rules. I think it only ever aligns to 16-bit
> boundaries.

So I got curious about this, but couldn't find any good online documentation about 
the alignment defaults of various architectures that GCC supports. So I reverted 
the fix and added the new check from linux-next:

    Revert "mm/pkeys: Fix siginfo ABI breakage caused by new u64 field"
    kernel/signal.c: add compile-time check for __ARCH_SI_PREAMBLE_SIZE

... which does:

 void __init signals_init(void)
 {
+       /* If this check fails, the __ARCH_SI_PREAMBLE_SIZE value is wrong! */
+       BUILD_BUG_ON(__ARCH_SI_PREAMBLE_SIZE
+               != offsetof(struct siginfo, _sifields._pad));
+

and tested it on the -tip cross-arch build testing suite, which gave the following 
result (only 32-bit architectures listed):

                                 (warns)               (warns)
testing     x86-32:  -git:  pass (    0),  -tip:  pass (    0)
testing        arm:  -git:  pass (    1),  -tip:  FAIL  .....
testing   blackfin:  -git:  pass (    0),  -tip:  pass (    0)
testing       cris:  -git:  pass (   32),  -tip:  pass (   32)
testing        frv:  -git:  pass (    1),  -tip:  FAIL  .....
testing       m32r:  -git:  pass (    6),  -tip:  pass (    6)
testing       m68k:  -git:  pass (    1),  -tip:  pass (    1)
testing microblaze:  -git:  pass (    0),  -tip:  pass (    0)
testing       mips:  -git:  pass (    1),  -tip:  FAIL  .....
testing   openrisc:  -git:  pass (    2),  -tip:  pass (    2)
testing     parisc:  -git:  pass (    0),  -tip:  FAIL  .....
testing         sh:  -git:  pass (   36),  -tip:  pass (   36)
testing      sparc:  -git:  pass (    0),  -tip:  FAIL  .....
testing       tile:  -git:  pass (    5),  -tip:  pass (    5)
testing     xtensa:  -git:  pass (    0),  -tip:  FAIL  .....
testing  powerpc32:  -git:  pass (    0),  -tip:  FAIL  .....

so if my test is correct then it's 9 architectures that align u64 to 4 bytes, vs. 
7 that align it to 8 bytes.

So naturally aligned u64 is definitely not 'rare' (so the characterisation in my 
changelog is wrong), but it's not dominant either.

FWIIW: if we only list 'major' architectures then x86-32 is indeed the odd one 
out...

> I do keep coming back to the fact that we should *probably* just do
> something like
> 
>     typedef unsigned long long __attribute__((aligned(8))) __u64;
> 
> and then introduce a separate "u64_unaligned" type for all the legacy
> cases that depended on 32-bit alignment.
> 
> It's horrendously nasty to test, though.

So in theory we could test most of it by comparing the disassembly of allyesconfig 
builds, but comparing disassemblies is a pretty hard to use method in practice.

A more workable method would be to have a test .c file that includes all UAPI 
structures in existence and defines a variable out of every single one, and then 
generates a list of sizeof() values or so. But even that isn't perfect: a 
structure might shift some fields forward, into a pre-existing hole, without 
changing the sizeof? We'd need a list of all field offsets in all structures to be 
really sure, and that's nasty.

Thanks,

	Ingo