Message ID | d4bc72e1-7b0c-2e85-a8a0-5ff9ef357a7d@linaro.org |
---|---|
State | New |
Headers | show |
On Fri, 7 Jul 2017 11:36:17 -0300 Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > On 25/06/2017 19:07, Sergei Trofimovich wrote: > > Minimal reproducer: > > > > #include <pthread.h> > > > > static void * f (void * p) { return NULL; } > > > > int main (int argc, const char ** argv) { > > pthread_t t; > > pthread_create (&t, NULL, &f, NULL); > > > > pthread_join (t, NULL); > > return 0; > > } > > > > $ gcc -O0 -ggdb3 -o r bug.c -pthread && ./r > > > > Program terminated with signal SIGSEGV, Segmentation fault. > > #0 0x2000000000077da0 in start_thread (arg=0x0) at pthread_create.c:432 > > 432 __madvise (pd->stackblock, freesize - PTHREAD_STACK_MIN, MADV_DONTNEED); > > > > Here crash happens right after attempt to free unused part of > > thread's stack. > > > > On most architectures stack grows only down or grows only up. > > And there glibc decides which of unused ends of stack blocks can be freed. > > > > ia64 maintans two stacks. Both of them grow from the opposite directions: > > - normal "sp" stack (stack for local variables) grows down > > - register stack "bsp" grows up from the opposite end of stack block > > > > In this failure case we have prematurely freed "rsp" stack. > > > > The change leaves a few pages from both sides of stack block. > > > > Bug: https://sourceware.org/PR21672 > > Bug: https://bugs.gentoo.org/622694 > > I am trying to validate this change using the same emulator you referenced > in the bug reports (ski) using first master then 2.25 to check if it is > a recent change (since Mike Frysinger reported that on actual hardware that > nptl tests are actually working [1]). However, even your simple > testcase seems to fail either or without the proposed fix. So I am not sure > if it is something wrong with the kernel I build (a 4.12 from linux-stable), > the base environment I set (I basically use the sysroot create from > build-many-glibc.py plus a busybox to have some workable tools). > > In any case, I would like to know why exactly this started to happen > since on 2.25 mostly nptl cases shows no issue. I am more inclined to > take this is something related to my changes for BZ#18988 [2] and I think > we need to take these separate stack in consideration on 'setup_stack_prot' > which is not what we currently doing. > > If I understood correctly, for both IA64 and HPPA (the only arch with > _STACK_GROWN_UP) the mmap area for thread stack will have two disjoin areas > and on ia64 it will be the two stack areas divided by the guard page. If > it is the case I think we need to use the same logic of _STACK_GROWS_UP > on 'setup_stack_prot'. Could you check if the patch below helps? Oh, I didn't mention which glibc works and which does not. On my system glibc-2.23 works and glibc-2.24 does not. I didn't try 2.25 yet. AFAIU it does not yet contain stack changes. I planned to try 2.25/git after I get 2.24 back working. I think ia64 worked only by chance as [handwave] register stack does not necessarily gets spilled to memory if CPU has enough cache. I'll try glibc-2.25 with your patch only and report back. > diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c > index ec7d42e..d07b410 100644 > --- a/nptl/allocatestack.c > +++ b/nptl/allocatestack.c > @@ -356,7 +356,7 @@ setup_stack_prot (char *mem, size_t size, char *guard, size_t guardsize, > const int prot) > { > char *guardend = guard + guardsize; > -#if _STACK_GROWS_DOWN > +#if _STACK_GROWS_DOWN && !defined(NEED_SEPARATE_REGISTER_STACK) > /* As defined at guard_position, for architectures with downward stack > the guard page is always at start of the allocated area. */ > if (__mprotect (guardend, size - guardsize, prot) != 0) > > [1] https://sourceware.org/glibc/wiki/Release/2.25 > [2] https://sourceware.org/git/?p=glibc.git;a=commit;h=0edbf1230131dfeb03d843d2859e2104456fad80 > > > Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org> > > --- > > nptl/pthread_create.c | 18 ++++++++++++++++-- > > 1 file changed, 16 insertions(+), 2 deletions(-) > > > > diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c > > index 7a970ffc5b..6e3f6db5b1 100644 > > --- a/nptl/pthread_create.c > > +++ b/nptl/pthread_create.c > > @@ -555,10 +555,24 @@ START_THREAD_DEFN > > size_t pagesize_m1 = __getpagesize () - 1; > > #ifdef _STACK_GROWS_DOWN > > char *sp = CURRENT_STACK_FRAME; > > - size_t freesize = (sp - (char *) pd->stackblock) & ~pagesize_m1; > > + char *freeblock = (char *) pd->stackblock; > > + size_t freesize = (sp - freeblock) & ~pagesize_m1; > > assert (freesize < pd->stackblock_size); > > +# ifdef __ia64__ > > if (freesize > PTHREAD_STACK_MIN) > > - __madvise (pd->stackblock, freesize - PTHREAD_STACK_MIN, MADV_DONTNEED); > > + { > > + /* On ia64 stack grows both ways! > > + - normal "sp" stack (stack for local variables) grows down > > + - register stack "bsp" grows up from the opposite end of stack block > > + > > + Thus we leave PTHREAD_STACK_MIN bytes from stack block top > > + and leave same PTHREAD_STACK_MIN at stack block bottom. */ > > + freeblock += PTHREAD_STACK_MIN; > > + freesize -= PTHREAD_STACK_MIN; > > + } > > +# endif > > + if (freesize > PTHREAD_STACK_MIN) > > + __madvise (freeblock, freesize - PTHREAD_STACK_MIN, MADV_DONTNEED); > > #else > > /* Page aligned start of memory to free (higher than or equal > > to current sp plus the minimum stack size). */ > > >
diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index ec7d42e..d07b410 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -356,7 +356,7 @@ setup_stack_prot (char *mem, size_t size, char *guard, size_t guardsize, const int prot) { char *guardend = guard + guardsize; -#if _STACK_GROWS_DOWN +#if _STACK_GROWS_DOWN && !defined(NEED_SEPARATE_REGISTER_STACK) /* As defined at guard_position, for architectures with downward stack the guard page is always at start of the allocated area. */ if (__mprotect (guardend, size - guardsize, prot) != 0)