From patchwork Mon Apr 25 18:18:03 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Carlos O'Donell X-Patchwork-Id: 11878 Received: (qmail 60296 invoked by alias); 25 Apr 2016 18:18:19 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 60285 invoked by uid 89); 25 Apr 2016 18:18:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.5 required=5.0 tests=AWL, BAYES_50, RCVD_IN_DNSWL_LOW, TBC autolearn=no version=3.3.2 spammy=staying, UD:sched.h, schedh, sched.h X-HELO: mail-qg0-f52.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:from:organization :message-id:date:user-agent:mime-version:in-reply-to :content-transfer-encoding; bh=oY1BmihAogEkGdZukewf7Eb3S9oHAxJ6EzD1dZ5kO7g=; b=Kwde+FDH0InjQSH6j0Mfdzo5Vu3JAQQDTAipB9e3h9T7JDeCeNCokbKX44WR2kZDSt aTH9iVUA1lPVR/x/xrm1XeqqMyk/RflulxSc4s6AMQ5fbBYmQ5J2Kfft9mz8YqNPCoec AjyybbgX0WJG1cmnGTksGI0KyFphJkiHW6rZPSg0/Jj0z10QVDbLd8Qj8I+VukBv2llo qhJosa3XNJEQfZfc+t5/akqyb7lhPmLZGA1KwHb1NIuun+Rbi1sIkO3MQcfet+VQewoK vRjVYbrvFbPDjZ0Lc83WdLFIVuSlQBpuEbHo1p09fWZWACaRxEWogoMjxps5QwGtuJ+4 Tn0A== X-Gm-Message-State: AOPr4FVXUASQ7h510H0Lm32z8DO505J79NNRc8Mxhx3DYHwAbY5yvEcBTJ59sFD4Ih8ObYHc X-Received: by 10.140.86.112 with SMTP id o103mr36953094qgd.9.1461608286357; Mon, 25 Apr 2016 11:18:06 -0700 (PDT) Subject: Re: [PATCH v3] Fix clone (CLONE_VM) pid/tid reset (BZ#19957) To: Adhemerval Zanella , libc-alpha@sourceware.org References: <1461351245-22814-1-git-send-email-adhemerval.zanella@linaro.org> From: Carlos O'Donell Message-ID: <571E5F5B.6080903@redhat.com> Date: Mon, 25 Apr 2016 14:18:03 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1 MIME-Version: 1.0 In-Reply-To: <1461351245-22814-1-git-send-email-adhemerval.zanella@linaro.org> On 04/22/2016 02:54 PM, Adhemerval Zanella wrote: > Changes from previous version: > > * 'Fixed' the remaning ports: alpha, microblaze, sh, ia64, tile, > hppa, m68k, and nio2. I did actually tested the fix for these > architecture in specific, so any review would be appreciated. Looks good to me. Thanks for the new test! Two required changes: * hppa changes are wrong, fixed version provided below. * ppc32 comment needs fixup. Suggestions: * Mention bug number in test. * Suggest changes to test comments. > diff --git a/sysdeps/unix/sysv/linux/hppa/clone.S b/sysdeps/unix/sysv/linux/hppa/clone.S > index 0a18137..3716acd 100644 > --- a/sysdeps/unix/sysv/linux/hppa/clone.S > +++ b/sysdeps/unix/sysv/linux/hppa/clone.S > @@ -133,19 +133,11 @@ ENTRY(__clone) > > .LthreadStart: > # define CLONE_VM_BIT 23 /* 0x00000100 */ > -# define CLONE_THREAD_BIT 15 /* 0x00010000 */ > - /* Load original clone flags. > - If CLONE_THREAD was passed, don't reset the PID/TID. > - If CLONE_VM was passed, we need to store -1 to PID/TID. > - If CLONE_VM and CLONE_THREAD were not set store the result > - of getpid to PID/TID. */ > + /* Load original clone flags. If CLONE_VM was passed, don't reset the > + PID/TID. Otherwise store the result of getpid to PID/TID. */ > ldw -56(%sp), %r26 > - bb,<,n %r26, CLONE_THREAD_BIT, 1f > - bb,< %r26, CLONE_VM_BIT, 2f > - ldi -1, %ret0 > - ble 0x100(%sr2, %r0) > + bb,<,n %r26, CLONE_VM_BIT, 1f > ldi __NR_getpid, %r20 > -2: > mfctl %cr27, %r26 > stw %ret0, PID_THREAD_OFFSET(%r26) > stw %ret0, TID_THREAD_OFFSET(%r26) Should be: --- Your original v3 patch removes the 'ble' required for the syscall. > diff --git a/sysdeps/unix/sysv/linux/i386/clone.S b/sysdeps/unix/sysv/linux/i386/clone.S > index ef447d1..25f2a9c 100644 > --- a/sysdeps/unix/sysv/linux/i386/clone.S > +++ b/sysdeps/unix/sysv/linux/i386/clone.S > @@ -40,7 +40,6 @@ > #define SYS_clone 120 > > #define CLONE_VM 0x00000100 > -#define CLONE_THREAD 0x00010000 > > .text > ENTRY (__clone) > @@ -108,7 +107,7 @@ L(thread_start): > cfi_undefined (eip); > /* Note: %esi is zero. */ > movl %esi,%ebp /* terminate the stack frame */ > - testl $CLONE_THREAD, %edi > + testl $CLONE_VM, %edi > je L(newpid) > L(haspid): > call *%ebx > @@ -124,9 +123,6 @@ L(here): > > .subsection 2 > L(newpid): > - testl $CLONE_VM, %edi > - movl $-1, %eax > - jne L(nomoregetpid) > movl $SYS_ify(getpid), %eax > ENTER_KERNEL > L(nomoregetpid): > diff --git a/sysdeps/unix/sysv/linux/ia64/clone2.S b/sysdeps/unix/sysv/linux/ia64/clone2.S > index fc046eb..b4cfdfc 100644 > --- a/sysdeps/unix/sysv/linux/ia64/clone2.S > +++ b/sysdeps/unix/sysv/linux/ia64/clone2.S > @@ -67,12 +67,10 @@ ENTRY(__clone2) > (CHILD) mov loc0=gp > (PARENT) ret > ;; > - tbit.nz p6,p0=in3,16 /* CLONE_THREAD */ > - tbit.z p7,p10=in3,8 /* CLONE_VM */ > + tbit.nz p6,p0=in3,8 /* CLONE_VM */ > (p6) br.cond.dptk 1f > ;; > mov r15=SYS_ify (getpid) > -(p10) addl r8=-1,r0 > (p7) break __BREAK_SYSCALL > ;; > add r9=PID,r13 > diff --git a/sysdeps/unix/sysv/linux/m68k/clone.S b/sysdeps/unix/sysv/linux/m68k/clone.S > index 33474cc..84eb2b9 100644 > --- a/sysdeps/unix/sysv/linux/m68k/clone.S > +++ b/sysdeps/unix/sysv/linux/m68k/clone.S > @@ -25,7 +25,6 @@ > #include > > #define CLONE_VM 0x00000100 > -#define CLONE_THREAD 0x00010000 > > /* int clone(int (*fn)(void *arg), void *child_stack, int flags, void *arg, > void *parent_tidptr, void *tls, void *child_tidptr) */ > @@ -101,12 +100,9 @@ thread_start: > subl %fp, %fp /* terminate the stack frame */ > /* Check and see if we need to reset the PID. */ > movel %d1, %a1 > - andl #CLONE_THREAD, %d1 > + andl #CLONE_VM, %d1 > jne donepid > movel %a1, %d1 > - movel #-1, %d0 > - andl #CLONE_VM, %d1 > - jne gotpid > movel #SYS_ify (getpid), %d0 > trap #0 > gotpid: > diff --git a/sysdeps/unix/sysv/linux/mips/clone.S b/sysdeps/unix/sysv/linux/mips/clone.S > index feb8250..39634c5 100644 > --- a/sysdeps/unix/sysv/linux/mips/clone.S > +++ b/sysdeps/unix/sysv/linux/mips/clone.S > @@ -131,9 +131,8 @@ L(thread_start): > /* The stackframe has been created on entry of clone(). */ > > /* Check and see if we need to reset the PID. */ > - LONG_L a0,(PTRSIZE*2)(sp) > - and a1,a0,CLONE_THREAD > - beqz a1,L(restore_pid) > + and a1,a0,CLONE_VM > + beqz a1,L(restore_pid) > L(donepid): > > /* Restore the arg for user's function. */ > @@ -153,12 +152,8 @@ L(donepid): > #endif > > L(restore_pid): > - and a1,a0,CLONE_VM > - li v0,-1 > - bnez a1,L(gotpid) > li v0,__NR_getpid > syscall > -L(gotpid): > READ_THREAD_POINTER(v1) > INT_S v0,PID_OFFSET(v1) > INT_S v0,TID_OFFSET(v1) > diff --git a/sysdeps/unix/sysv/linux/nios2/clone.S b/sysdeps/unix/sysv/linux/nios2/clone.S > index e4d3970..30b6e4a 100644 > --- a/sysdeps/unix/sysv/linux/nios2/clone.S > +++ b/sysdeps/unix/sysv/linux/nios2/clone.S > @@ -26,7 +26,6 @@ > #include > > #define CLONE_VM 0x00000100 > -#define CLONE_THREAD 0x00010000 > > /* int clone(int (*fn)(void *arg), void *child_stack, int flags, void *arg, > void *parent_tidptr, void *tls, void *child_tidptr) */ > @@ -71,13 +70,9 @@ thread_start: > > /* We expect the argument registers to be preserved across system > calls and across task cloning, so flags should be in r4 here. */ > - andhi r2, r4, %hi(CLONE_THREAD) > + andi r2, r4, CLONE_VM > bne r2, zero, 2f > - andi r3, r4, CLONE_VM > - movi r2, -1 > - bne r3, zero, 3f > DO_CALL (getpid, 0) > -3: > stw r2, PID_OFFSET(r23) > stw r2, TID_OFFSET(r23) > 2: > diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S b/sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S > index eb973db..3761ded 100644 > --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S > +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S > @@ -76,13 +76,11 @@ ENTRY (__clone) > crandc cr1*4+eq,cr1*4+eq,cr0*4+so > bne- cr1,L(parent) /* The '-' is to minimise the race. */ > > - andis. r0,r28,CLONE_THREAD>>16 > - bne+ r0,L(oldpid) > - andi. r0,r28,CLONE_VM > - li r3,-1 > - bne- r0,L(nomoregetpid) > + /* If CLONE_VM is set does not update the pid/tid field. */ s/does not/do not/g > + andi. r0,r29,CLONE_VM > + bne+ cr0,L(oldpid) > + > DO_CALL(SYS_ify(getpid)) > -L(nomoregetpid): > stw r3,TID(r2) > stw r3,PID(r2) > L(oldpid): > diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S b/sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S > index 959fdb7..7c59b9b 100644 > --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S > +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S > @@ -78,13 +78,11 @@ ENTRY (__clone) > crandc cr1*4+eq,cr1*4+eq,cr0*4+so > bne- cr1,L(parent) /* The '-' is to minimise the race. */ > > - andis. r0,r29,CLONE_THREAD>>16 > + /* If CLONE_VM is set do not update the pid/tid field. */ > + rldicl. r0,r29,56,63 /* flags & CLONE_VM. */ > bne+ cr0,L(oldpid) > - andi. r0,r29,CLONE_VM > - li r3,-1 > - bne- cr0,L(nomoregetpid) > + > DO_CALL(SYS_ify(getpid)) > -L(nomoregetpid): > stw r3,TID(r13) > stw r3,PID(r13) > L(oldpid): > diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/clone.S b/sysdeps/unix/sysv/linux/s390/s390-32/clone.S > index f774e90..2f8fa0b 100644 > --- a/sysdeps/unix/sysv/linux/s390/s390-32/clone.S > +++ b/sysdeps/unix/sysv/linux/s390/s390-32/clone.S > @@ -54,13 +54,10 @@ error: > PSEUDO_END (__clone) > > thread_start: > - tmh %r3,1 /* CLONE_THREAD == 0x00010000 */ > - jne 1f > - lhi %r2,-1 > tml %r3,256 /* CLONE_VM == 0x00000100 */ > - jne 2f > + jne 1f > svc SYS_ify(getpid) > -2: ear %r3,%a0 > + ear %r3,%a0 > st %r2,PID(%r3) > st %r2,TID(%r3) > 1: > diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/clone.S b/sysdeps/unix/sysv/linux/s390/s390-64/clone.S > index 928a881..fb81692 100644 > --- a/sysdeps/unix/sysv/linux/s390/s390-64/clone.S > +++ b/sysdeps/unix/sysv/linux/s390/s390-64/clone.S > @@ -55,13 +55,10 @@ error: > PSEUDO_END (__clone) > > thread_start: > - tmh %r3,1 /* CLONE_THREAD == 0x00010000 */ > + tmll %r3,256 /* CLONE_VM == 0x00000100 */ > jne 1f > - lhi %r2,-1 > - tml %r3,256 /* CLONE_VM == 0x00000100 */ > - jne 2f > svc SYS_ify(getpid) > -2: ear %r3,%a0 > + ear %r3,%a0 > sllg %r3,%r3,32 > ear %r3,%a1 > st %r2,PID(%r3) > diff --git a/sysdeps/unix/sysv/linux/sh/clone.S b/sysdeps/unix/sysv/linux/sh/clone.S > index a73dd7d..4cd7df1 100644 > --- a/sysdeps/unix/sysv/linux/sh/clone.S > +++ b/sysdeps/unix/sysv/linux/sh/clone.S > @@ -67,15 +67,11 @@ ENTRY(__clone) > /* terminate the stack frame */ > mov #0, r14 > mov r4, r0 > - shlr16 r0 > - tst #1, r0 // CLONE_THREAD = (1 << 16) > + shlr8 r0 > + tst #1, r0 // CLONE_VM = (1 << 8) > bf/s 4f > mov r4, r0 > /* new pid */ > - shlr8 r0 > - tst #1, r0 // CLONE_VM = (1 << 8) > - bf/s 3f > - mov #-1, r0 > mov #+SYS_ify(getpid), r3 > trapa #0x15 > 3: > diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/clone.S b/sysdeps/unix/sysv/linux/sparc/sparc32/clone.S > index 68f8202..d6c92f6 100644 > --- a/sysdeps/unix/sysv/linux/sparc/sparc32/clone.S > +++ b/sysdeps/unix/sysv/linux/sparc/sparc32/clone.S > @@ -25,7 +25,6 @@ > #include > > #define CLONE_VM 0x00000100 > -#define CLONE_THREAD 0x00010000 > > /* int clone(int (*fn)(void *arg), void *child_stack, int flags, void *arg, > pid_t *ptid, void *tls, pid_t *ctid); */ > @@ -80,15 +79,10 @@ END(__clone) > > .type __thread_start,@function > __thread_start: > - sethi %hi(CLONE_THREAD), %l0 > - andcc %g4, %l0, %g0 > + andcc %g4, CLONE_VM, %g0 > bne 1f > - andcc %g4, CLONE_VM, %g0 > - bne,a 2f > - mov -1,%o0 > set __NR_getpid,%g1 > ta 0x10 > -2: > st %o0,[%g7 + PID] > st %o0,[%g7 + TID] > 1: > diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/clone.S b/sysdeps/unix/sysv/linux/sparc/sparc64/clone.S > index cecffa7..b0f6266 100644 > --- a/sysdeps/unix/sysv/linux/sparc/sparc64/clone.S > +++ b/sysdeps/unix/sysv/linux/sparc/sparc64/clone.S > @@ -25,7 +25,6 @@ > #include > > #define CLONE_VM 0x00000100 > -#define CLONE_THREAD 0x00010000 > > /* int clone(int (*fn)(void *arg), void *child_stack, int flags, void *arg, > pid_t *ptid, void *tls, pid_t *ctid); */ > @@ -77,15 +76,11 @@ END(__clone) > > .type __thread_start,@function > __thread_start: > - sethi %hi(CLONE_THREAD), %l0 > - andcc %g4, %l0, %g0 > + andcc %g4, CLONE_VM, %g0 > bne,pt %icc, 1f > - andcc %g4, CLONE_VM, %g0 > - bne,a,pn %icc, 2f > - mov -1,%o0 > set __NR_getpid,%g1 > ta 0x6d > -2: st %o0,[%g7 + PID] > + st %o0,[%g7 + PID] > st %o0,[%g7 + TID] > 1: > mov %g0, %fp /* terminate backtrace */ > diff --git a/sysdeps/unix/sysv/linux/tile/clone.S b/sysdeps/unix/sysv/linux/tile/clone.S > index a300eb5..47d0588 100644 > --- a/sysdeps/unix/sysv/linux/tile/clone.S > +++ b/sysdeps/unix/sysv/linux/tile/clone.S > @@ -164,43 +164,31 @@ ENTRY (__clone) > cfi_def_cfa_offset (FRAME_SIZE) > cfi_undefined (lr) > /* Check and see if we need to reset the PID, which we do if > - CLONE_THREAD isn't set, i.e. we're not staying in the thread group. > - If CLONE_VM is set, we're doing some kind of thread-like clone, > - so we set the tid/pid to -1 to disable using the cached values > - in getpid(). Otherwise (if CLONE_VM isn't set), it's a > + CLONE_VM isn't set, i.e. we're do not share the same pthread_t > + with parent. Otherwise (if CLONE_VM isn't set), it's a > fork-like clone, and we go ahead and write the cached values > from the true system pid (retrieved via __NR_getpid syscall). */ > #ifdef __tilegx__ > { > moveli r0, hw1_last(CLONE_VM) > - moveli r1, hw1_last(CLONE_THREAD) > } > { > shl16insli r0, r0, hw0(CLONE_VM) > - shl16insli r1, r1, hw0(CLONE_THREAD) > } > #else > { > moveli r0, lo16(CLONE_VM) > - moveli r1, lo16(CLONE_THREAD) > } > { > auli r0, r0, ha16(CLONE_VM) > - auli r1, r1, ha16(CLONE_THREAD) > } > #endif > { > and r0, r30, r0 > - and r1, r30, r1 > - } > - BNEZ r1, .Lno_reset_pid /* CLONE_THREAD is set */ > - { > - movei r0, -1 > - BNEZ r0, .Lgotpid /* CLONE_VM is set */ > } > + BNEZ r0, .Lno_reset_pid /* CLONE_VM is set */ > moveli TREG_SYSCALL_NR_NAME, __NR_getpid > swint1 > -.Lgotpid: > ADDLI_PTR r2, tp, PID_OFFSET > { > ST4 r2, r0 > diff --git a/sysdeps/unix/sysv/linux/tst-clone2.c b/sysdeps/unix/sysv/linux/tst-clone2.c > new file mode 100644 > index 0000000..7452a47 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/tst-clone2.c > @@ -0,0 +1,178 @@ > +/* Test if CLONE_VM does not change pthread pid/tid field. Mention bug #. > + Copyright (C) 2016 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + . */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include /* for THREAD_* macros. */ > + > +static int sig; > +static int pipefd[2]; > + > +static int > +f (void *a) > +{ > + close (pipefd[0]); > + > + pid_t pid = THREAD_GETMEM (THREAD_SELF, pid); > + pid_t tid = THREAD_GETMEM (THREAD_SELF, tid); > + > + while (write (pipefd[1], &pid, sizeof pid) < 0) > + continue; > + while (write (pipefd[1], &tid, sizeof tid) < 0) > + continue; > + > + return 0; > +} > + > + > +static int > +clone_test (int clone_flags) > +{ > + sig = SIGRTMIN; > + sigset_t ss; > + sigemptyset (&ss); > + sigaddset (&ss, sig); > + if (sigprocmask (SIG_BLOCK, &ss, NULL) != 0) > + { > + printf ("sigprocmask failed: %m\n"); > + return 1; > + } > + > + if (pipe2 (pipefd, O_CLOEXEC)) > + { > + printf ("sigprocmask failed: %m\n"); > + return 1; > + } > + > + pid_t ppid = getpid (); > + > +#ifdef __ia64__ > + extern int __clone2 (int (*__fn) (void *__arg), void *__child_stack_base, > + size_t __child_stack_size, int __flags, > + void *__arg, ...); > + char st[256 * 1024] __attribute__ ((aligned)); > + pid_t p = __clone2 (f, st, sizeof (st), clone_flags, 0); > +#else > + char st[128 * 1024] __attribute__ ((aligned)); > +#if _STACK_GROWS_DOWN > + pid_t p = clone (f, st + sizeof (st), clone_flags, 0); > +#elif _STACK_GROWS_UP > + pid_t p = clone (f, st, clone_flags, 0); > +#else > +#error "Define either _STACK_GROWS_DOWN or _STACK_GROWS_UP" > +#endif > +#endif > + close (pipefd[1]); > + > + if (p == -1) > + { > + printf ("clone failed: %m\n"); > + return 1; > + } > + > + pid_t pid, tid; > + if (read (pipefd[0], &pid, sizeof pid) != sizeof pid) > + { > + printf ("read pid failed: %m\n"); > + kill (p, SIGKILL); > + return 1; > + } > + if (read (pipefd[0], &tid, sizeof tid) != sizeof tid) > + { > + printf ("read pid failed: %m\n"); > + kill (p, SIGKILL); > + return 1; > + } > + > + close (pipefd[0]); > + > + int ret = 0; > + > + /* For CLONE_VM glibc clone implementation does not change the pthread > + pid/tid field. */ > + if ((clone_flags & CLONE_VM) == CLONE_VM) > + { > + if ((ppid != pid) || (ppid != tid)) > + { > + printf ("parent pid (%i) != received pid/tid (%i/%i)\n", > + (int)ppid, (int)pid, (int)tid); > + ret = 1; > + } > + } > + /* For any other flag clone updates the new pthread pid and tid with > + the clone return value. */ > + else > + { > + if ((p != pid) || (p != tid)) > + { > + printf ("child pid (%i) != received pid/tid (%i/%i)\n", > + (int)p, (int)pid, (int)tid); > + ret = 1; > + } > + } > + > + int e; > + if (waitpid (p, &e, __WCLONE) != p) > + { > + puts ("waitpid failed"); > + kill (p, SIGKILL); > + return 1; > + } > + if (!WIFEXITED (e)) > + { > + if (WIFSIGNALED (e)) > + printf ("died from signal %s\n", strsignal (WTERMSIG (e))); > + else > + puts ("did not terminate correctly"); > + return 1; > + } > + if (WEXITSTATUS (e) != 0) > + { > + printf ("exit code %d\n", WEXITSTATUS (e)); > + return 1; > + } > + > + return ret; > +} > + > +int > +do_test (void) > +{ > + /* It first checks the clone implementation without any flag, which will > + make the child new pid to be cached on its pthread structure. */ Suggest: /* First, check that the clone implementation, without any flag, updates the struct pthread to contain the new PID and TID. */ > + int ret = clone_test (0); > + /* Then check if clone with CLONE_VM avoid caching it since memory is > + shared between parent and it might overwrite parent's pthread > + structure. */ Suggest: /* Second, check that with CLONE_VM the struct pthread PID and TID fields remain unmodified after the clone. Any modifications would cause problem for the parent as described in bug 19957. */ > + ret += clone_test (CLONE_VM); > + return ret; > +} > + > +#define TEST_FUNCTION do_test () > +#include "../test-skeleton.c" > diff --git a/sysdeps/unix/sysv/linux/tst-getpid2.c b/sysdeps/unix/sysv/linux/tst-getpid2.c > deleted file mode 100644 > index fc98cb6..0000000 > --- a/sysdeps/unix/sysv/linux/tst-getpid2.c > +++ /dev/null > @@ -1,2 +0,0 @@ > -#define TEST_CLONE_FLAGS CLONE_VM > -#include "tst-getpid1.c" > diff --git a/sysdeps/unix/sysv/linux/x86_64/clone.S b/sysdeps/unix/sysv/linux/x86_64/clone.S > index 1a50957..66f4b11 100644 > --- a/sysdeps/unix/sysv/linux/x86_64/clone.S > +++ b/sysdeps/unix/sysv/linux/x86_64/clone.S > @@ -24,7 +24,6 @@ > #include > > #define CLONE_VM 0x00000100 > -#define CLONE_THREAD 0x00010000 > > /* The userland implementation is: > int clone (int (*fn)(void *arg), void *child_stack, int flags, void *arg), > @@ -92,14 +91,11 @@ L(thread_start): > the outermost frame obviously. */ > xorl %ebp, %ebp > > - testq $CLONE_THREAD, %rdi > + andq $CLONE_VM, %rdi > jne 1f > - testq $CLONE_VM, %rdi > - movl $-1, %eax > - jne 2f > movl $SYS_ify(getpid), %eax > syscall > -2: movl %eax, %fs:PID > + movl %eax, %fs:PID > movl %eax, %fs:TID > 1: > > diff --git a/sysdeps/unix/sysv/linux/hppa/clone.S b/sysdeps/unix/sysv/linux/hppa/clone.S index 0a18137..43f5a78 100644 --- a/sysdeps/unix/sysv/linux/hppa/clone.S +++ b/sysdeps/unix/sysv/linux/hppa/clone.S @@ -135,17 +135,12 @@ ENTRY(__clone) # define CLONE_VM_BIT 23 /* 0x00000100 */ # define CLONE_THREAD_BIT 15 /* 0x00010000 */ /* Load original clone flags. - If CLONE_THREAD was passed, don't reset the PID/TID. - If CLONE_VM was passed, we need to store -1 to PID/TID. - If CLONE_VM and CLONE_THREAD were not set store the result - of getpid to PID/TID. */ + If CLONE_VM was passed, don't modify PID/TID. + Otherwise store the result of getpid to PID/TID. */ ldw -56(%sp), %r26 - bb,<,n %r26, CLONE_THREAD_BIT, 1f - bb,< %r26, CLONE_VM_BIT, 2f - ldi -1, %ret0 + bb,<,n %r26, CLONE_VM_BIT, 1f ble 0x100(%sr2, %r0) ldi __NR_getpid, %r20 -2: mfctl %cr27, %r26 stw %ret0, PID_THREAD_OFFSET(%r26) stw %ret0, TID_THREAD_OFFSET(%r26)