This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: {make,set,swap}context broken on powerpc32
- From: Steven Munroe <munroesj at us dot ibm dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: libc-alpha at sources dot redhat dot com, Paul Mackerras <paulus at samba dot org>, Peter Bergner <bergner at us dot ibm dot com>
- Date: Tue, 12 Dec 2006 16:16:22 -0600
- Subject: Re: {make,set,swap}context broken on powerpc32
- References: <20061212130526.GC29911@devserv.devel.redhat.com>
Jakub Jelinek wrote:
>Hi!
>
>The following testcase crashes on powerpc32 with CVS glibc (but glibc 2.5 as
>well) and 2.6.18 kernel, when glibc is configured for 2.6.0+ kernel
>(i.e. __ASSUME_SWAPCONTEXT_SYSCALL). The problem is in clobberring r2 (== TLS)
>register.
>
The kernel should not restore r2 from the ucontext and should keep the
r2 for the thread. The problem state implementation (#undef
__ASSUME_SWAPCONTEXT_SYSCAL) leaves r2 untouched.
Seems that __ASSUME_SWAPCONTEXT_SYSCALL does not get set unless
--enable-kernel=2.6.0+ is configured so my default testing does not
hit. Also powerpc64 is using the older __ASSUME_NEW_RT_SIGRETURN_SYSCALL
and does not hit this.
>One thing is that swapcontext syscall restores r2 (for 32-bit processes)
>from the saved registers. As it is used by swapcontext and setcontext
>syscalls, that in itself is a problem - can't you getcontext in one
>thread and setcontext in another one?
>
The getcontext/setcontext context is more like a coroutine call and can
not (should not) change the thread pointer (or the thread struct or the
kernels internal thread). So it is more like a N to M thread model.
>Additionally, it seems makecontext call doesn't preserve any registers
>that getcontext saved, as it overwrites uc_mcontext.uc_regs pointer
>(on this testcase getcontext created context has
>uc_mcontext.uc_regs pointing to uc_reg_space + 12 while makecontext
>resets that to uc_mcontext.uc_regs).
>The combination of those two problems causes the failure of this testcase,
>but I think for both the problems we can construct a testcase separately.
>
Its more complicated then that. When we added Altivec we had to force
quadword aligment of the uc_reg_space even when the ucontext struct
itself was not and some kernels did not get the set uc_regs_ptr at all.
So the sequence:
addi r11,r3,_UC_REG_SPACE+12 // round up if not already quadword
clrrwi r11,r11,4 // round down to quadword
stw r11,_UC_REGS_PTR(r3)
Forces a round up to quadword and makes sure the uc_regs_ptr is set. If
you look at a getcontext you will see the same sequence. So for a
properly created ucontext makecontext is not changing the uc_regs_ptr at
all.
So this can only be a problem if the kernel is following different
rules. For example if the kernel was not forcing quadword alignment for
uc_reg_space or was only force quadword for PPC_FEATURE_HAS_ALTIVEC.
makecontext does overwrite the parameter registers the PC and LR to set
the call to the target function. Otherwise non-volitile registes are not
changed.
>For makecontext, I wonder if we just shouldn't memmove the register values
>around if we change the pointer. For the swapcontext/setcontext, there
>
>is a bigger problem, either we change the kernel not to restore r2,
>or we could e.g. compare the saved r2 with the current r2, if equal to what
>we do ATM, otherwise copy the whole ucontext_t into a temporary, change
>r2 in it and only then do the swapcontext syscall.
>
>
We should not have to memmove the registers unless they are unaligned,
which should never happen.
Perhaps we should use __ASSUME_SWAPCONTEXT_SYSCALL to tell makecontext
to assume us_regs is valid (set and aligned) and use it as is to set up
the parameter for the target function?
But it is a problem that the kernel is modifying r2. The should be fixed
in the kernel.
>#define _XOPEN_SOURCE 600
>#include <stdlib.h>
>#include <stdio.h>
>#include <ucontext.h>
>
>ucontext_t oucp, ucp;
>char st1[8192];
>__thread int thr;
>
>void
>cf (int i)
>{
> if (i != 78 || thr != 94)
> {
> printf ("i %d thr %d\n", i, thr);
> exit (1);
> }
> exit (0);
>}
>
>int
>main (void)
>{
> if (getcontext (&oucp) != 0 || getcontext (&ucp) != 0)
> {
> puts ("getcontext failed");
> return 1;
> }
> thr = 94;
> ucp.uc_link = &oucp;
> ucp.uc_stack.ss_sp = st1;
> ucp.uc_stack.ss_size = sizeof st1;
> makecontext (&ucp, (void (*) ()) cf, 1, 78);
> if (setcontext (&ucp) != 0)
> {
> puts ("setcontext failed");
> return 1;
> }
> return 0;
>}
>
> Jakub
>