Re: [PATCH] hppa: Fix stack alignment passed to clone

[John David Anglin <dave dot anglin at bell dot net>]

On 2019-10-19 3:17 p.m., Florian Weimer wrote:
> * John David Anglin:
>
>> The hppa architecture requires strict alignment for loads and
>> stores.  As a result, the minimum stack alignment that will work is
>> 8 bytes.  This patch adjust __clone() to align the stack argument
>> passed to it.  It also adjusts slightly some formatting.
>>
>> This fixes the nptl/tst-tls1 test.
> I wonder if we should change this in nptl/tst-tls1
>
>   xpthread_attr_setstack (&a, thr_stack + 1, STACK_SIZE);
>
> to:
>
>   xpthread_attr_setstack (&a, thr_stack + _Alignof (max_align_t), STACK_SIZE);
>
> This should fix the test on hppa.  Since the alignment of the test TLS
> variables is much larger than that of max_align_t, I don't think it
> interferes with the objective of the test.
Yes, that should fix the test.  However, I still think the hppa fix should be applied because it
avoids misaligned accesses and then a segmentation fault in ld.so.1.  We have this code in
__clone():

        /* Ensure stack argument is 8-byte aligned.  */
        ldo             7(%r25),%r25
        depi            0,31,3,%r25

        /* Save the function pointer, arg, and flags on the new stack.  */
        stwm    %r26, 64(%r25)
        stw     %r23, -60(%r25)
        stw     %r24, -56(%r25)

The stwm instruction adds 64 to %r25.  The preceding two instructions round the stack argument
to an 8-byte aligned value.  So, we are just adding an additional 0 to 7.  Not a big deal.

If %r25 is not aligned, the stores for %r23 and %r24 generated unaligned traps.  Then, the kernel mucks
with the stack argument because the PSW W bit is stored in the bottom of sp:

        /* for now we can *always* set the W bit on entry to the syscall
         * since we don't support wide userland processes.  We could
         * also save the current SM other than in r0 and restore it on
         * exit from the syscall, and also use that value to know
         * whether to do narrow or wide syscalls. -PB
         */
        ssm     PSW_SM_W, %r1
        extrd,u %r1,PSW_W_BIT,1,%r1
        /* sp must be aligned on 4, so deposit the W bit setting into
         * the bottom of sp temporarily */
        or,ev   %r1,%r30,%r30

The child crashes when clone returns.  With the alignment, the code runs correctly.  So, I think we
shouldn't worry about the extra two instructions.

The other alternative is to generate an error when passed a misaligned stack argument.  I looked at
the opengroup spec but it didn't clarify the situations where arguments are invalid.

Dave

-- 
John David Anglin  dave.anglin@bell.net