This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v3] Fix clone (CLONE_VM) pid/tid reset (BZ#19957)
- From: Carlos O'Donell <carlos at redhat dot com>
- To: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>, libc-alpha at sourceware dot org
- Date: Mon, 25 Apr 2016 14:18:03 -0400
- Subject: Re: [PATCH v3] Fix clone (CLONE_VM) pid/tid reset (BZ#19957)
- Authentication-results: sourceware.org; auth=none
- References: <1461351245-22814-1-git-send-email-adhemerval dot zanella at linaro dot 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:
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)
---
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 <tls.h>
>
> #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 <tcb-offsets.h>
>
> #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 <sysdep.h>
>
> #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 <sysdep.h>
>
> #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
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <sched.h>
> +#include <signal.h>
> +#include <string.h>
> +#include <stdio.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <stddef.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <stdlib.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
> +
> +#include <tls.h> /* 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 <asm-syntax.h>
>
> #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:
>
>
--
Cheers,
Carlos.