This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: {make,set,swap}context broken on powerpc32


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
>  


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]