public inbox for libc-hacker@sourceware.org
 help / color / mirror / Atom feed
* getpid/vfork broken
@ 2004-03-07 23:18 Andreas Schwab
  2004-03-07 23:30 ` Ulrich Drepper
  0 siblings, 1 reply; 28+ messages in thread
From: Andreas Schwab @ 2004-03-07 23:18 UTC (permalink / raw)
  To: libc-hacker

getpid does not return the correct result after vfork:

$ cat getpid.c
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/wait.h>

int
main (void)
{
  if (vfork () == 0)
    {
      printf ("%d\n", getpid ());
      exit (0);
    }
  wait (0);
  if (vfork () == 0)
    {
      printf ("%d\n", getpid ());
      exit (0);
    }
  wait (0);
  printf ("%d\n", getpid ());
  return 0;
}
$ ./getpid
31302
31302
31302

Note that the number is actually the pid of the first child, not the
parent.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: getpid/vfork broken
  2004-03-07 23:18 getpid/vfork broken Andreas Schwab
@ 2004-03-07 23:30 ` Ulrich Drepper
  2004-03-08  2:22   ` Roland McGrath
  0 siblings, 1 reply; 28+ messages in thread
From: Ulrich Drepper @ 2004-03-07 23:30 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: libc-hacker

Andreas Schwab wrote:
> getpid does not return the correct result after vfork:

It doesn't have to.  You are not allowed to call anything but _exit or
one of the exec functions.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: getpid/vfork broken
  2004-03-07 23:30 ` Ulrich Drepper
@ 2004-03-08  2:22   ` Roland McGrath
  2004-03-08  2:28     ` Ulrich Drepper
  0 siblings, 1 reply; 28+ messages in thread
From: Roland McGrath @ 2004-03-08  2:22 UTC (permalink / raw)
  To: Ulrich Drepper; +Cc: Andreas Schwab, libc-hacker

> Andreas Schwab wrote:
> > getpid does not return the correct result after vfork:
> 
> It doesn't have to.  You are not allowed to call anything but _exit or
> one of the exec functions.

getpid is something that has always worked in all contexts.  I don't think
it's worth any other minor benefit to risk breaking applications that do
this one thing and have worked since the invention of vfork.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: getpid/vfork broken
  2004-03-08  2:22   ` Roland McGrath
@ 2004-03-08  2:28     ` Ulrich Drepper
  2004-03-08  2:37       ` Roland McGrath
  0 siblings, 1 reply; 28+ messages in thread
From: Ulrich Drepper @ 2004-03-08  2:28 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Andreas Schwab, libc-hacker

Roland McGrath wrote:

> getpid is something that has always worked in all contexts.

No exceptions.  If we start with one interface there will soon be a
second and a third and...  vfork is special.  And it can easily be
replaced with fork if somebody does something stupid.  You can even
preload code to fix a broken application.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: getpid/vfork broken
  2004-03-08  2:28     ` Ulrich Drepper
@ 2004-03-08  2:37       ` Roland McGrath
  2004-03-08 13:10         ` Jakub Jelinek
  0 siblings, 1 reply; 28+ messages in thread
From: Roland McGrath @ 2004-03-08  2:37 UTC (permalink / raw)
  To: Ulrich Drepper; +Cc: Andreas Schwab, libc-hacker

The new specification for vfork where you can't even call dup2 is very
close to useless.  The origin of vfork and true standard for its behavior
that applications have been written for is 4.2BSD, where simple system
calls were always safe, and getpid has always been the simplest system
call.  The modern BSD specification remains that the address space is
shared, with all that entails, but since getpid has never relied on data
contents in the address space before, citing that as license to make it do
the wrong thing is dubious at best.  Since the useless specification has
been enshrined in POSIX, where vfork in fact never belonged at all, it is
likely that every historical program will be broken and people will just
have to adapt to giving up vfork entirely.  Progress.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: getpid/vfork broken
  2004-03-08  2:37       ` Roland McGrath
@ 2004-03-08 13:10         ` Jakub Jelinek
  2004-03-09  1:28           ` Roland McGrath
  2004-03-09  7:48           ` Ulrich Drepper
  0 siblings, 2 replies; 28+ messages in thread
From: Jakub Jelinek @ 2004-03-08 13:10 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Ulrich Drepper, Andreas Schwab, libc-hacker

On Sun, Mar 07, 2004 at 06:37:33PM -0800, Roland McGrath wrote:
> The new specification for vfork where you can't even call dup2 is very
> close to useless.  The origin of vfork and true standard for its behavior
> that applications have been written for is 4.2BSD, where simple system
> calls were always safe, and getpid has always been the simplest system
> call.  The modern BSD specification remains that the address space is
> shared, with all that entails, but since getpid has never relied on data
> contents in the address space before, citing that as license to make it do
> the wrong thing is dubious at best.  Since the useless specification has
> been enshrined in POSIX, where vfork in fact never belonged at all, it is
> likely that every historical program will be broken and people will just
> have to adapt to giving up vfork entirely.  Progress.

Here is what would be needed to make vfork & getpid work together:

1) vfork needs to:
   struct pthread *pd = THREAD_SELF;
   int oldval = THREAD_GETMEM (pd, pid);
   int newval = oldval ? -oldval : INT_MIN;
   THREAD_SETMEM (pd, pid) = newval;
   actual vfork syscall (oldval must be saved in a register
			 preserved accross the syscall)
   THREAD_SETMEM (pd, pid) = oldval;
2) raise would need:
    /* raise is an async-safe function.  It could be called while the
       fork function temporarily invalidated the PID field.  Adjust for
       that.  */
    if (__builtin_expect (pid <= 0, 0))
-      pid = pid == 0 ? selftid : -pid;
+      pid = (pid & INT_MAX) == 0 ? selftid : -pid;

vfork is written in assembly for each architecture, so so that would need to
be coded for each architecture.

->pid field seems to be only accessed in raise, getpid and fork from libc.so
functions, when libpthread is loaded, THREAD_SELF->pid will always be
non-zero, getpid with THREAD_SELF->pid == INT_MIN will always do the real
syscall and while vfork is running, the same program certainly shouldn't
call fork() (neither in a signal handler nor in the vfork child).

	Jakub

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: getpid/vfork broken
  2004-03-08 13:10         ` Jakub Jelinek
@ 2004-03-09  1:28           ` Roland McGrath
  2004-03-09  2:04             ` Ulrich Drepper
  2004-03-09  7:48           ` Ulrich Drepper
  1 sibling, 1 reply; 28+ messages in thread
From: Roland McGrath @ 2004-03-09  1:28 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Ulrich Drepper, Andreas Schwab, libc-hacker

In NPTL-supporting Linux kernels (2.6), I think we can just use the clone
syscall with CLONE_VFORK|CLONE_CHILD_SETTID.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: getpid/vfork broken
  2004-03-09  1:28           ` Roland McGrath
@ 2004-03-09  2:04             ` Ulrich Drepper
  2004-03-09  3:50               ` Richard Henderson
  0 siblings, 1 reply; 28+ messages in thread
From: Ulrich Drepper @ 2004-03-09  2:04 UTC (permalink / raw)
  To: Roland McGrath; +Cc: libc-hacker

Roland McGrath wrote:
> In NPTL-supporting Linux kernels (2.6), I think we can just use the clone
> syscall with CLONE_VFORK|CLONE_CHILD_SETTID.

No.  There is a reason why a vfork syscall was introduced.  There aren't
enough registers to use clone and hold the return address.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: getpid/vfork broken
  2004-03-09  2:04             ` Ulrich Drepper
@ 2004-03-09  3:50               ` Richard Henderson
  0 siblings, 0 replies; 28+ messages in thread
From: Richard Henderson @ 2004-03-09  3:50 UTC (permalink / raw)
  To: Ulrich Drepper; +Cc: Roland McGrath, libc-hacker

On Mon, Mar 08, 2004 at 06:04:09PM -0800, Ulrich Drepper wrote:
> No.  There is a reason why a vfork syscall was introduced.  There aren't
> enough registers to use clone and hold the return address.

On x86, perhaps.  The rest can do so.


r~

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: getpid/vfork broken
  2004-03-08 13:10         ` Jakub Jelinek
  2004-03-09  1:28           ` Roland McGrath
@ 2004-03-09  7:48           ` Ulrich Drepper
  2004-03-09 13:01             ` Andreas Schwab
  1 sibling, 1 reply; 28+ messages in thread
From: Ulrich Drepper @ 2004-03-09  7:48 UTC (permalink / raw)
  To: libc-hacker

I've checked in some changes for x86 and x86-64.  But I still think it's
a terrible idea to even try to support it.

For instance, on x86 it's not possible to get the tid field corrected,
too.  So raise won't work (fortunately it's caught by the kernel).  It
is possible on other archs but only at the code of making each and every
user of the tid field more complicated.  That's simply not acceptable
and this is why vfork was crippled.  It interferes far too much with
many other parts of the implementation.

Anyway, for the getpid handling, the other archs need to be handled,
too.  The x86/x86-64 changes are kept small, I envision this is the
model for the other archs, too.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: getpid/vfork broken
  2004-03-09  7:48           ` Ulrich Drepper
@ 2004-03-09 13:01             ` Andreas Schwab
  2004-03-09 13:59               ` [PATCH] getpid/vfork/raise fix Jakub Jelinek
  0 siblings, 1 reply; 28+ messages in thread
From: Andreas Schwab @ 2004-03-09 13:01 UTC (permalink / raw)
  To: Ulrich Drepper; +Cc: libc-hacker

Ulrich Drepper <drepper@redhat.com> writes:

> Anyway, for the getpid handling, the other archs need to be handled,
> too.  The x86/x86-64 changes are kept small, I envision this is the
> model for the other archs, too.

Here's the same for ia64.

Andreas.

2004-03-09  Andreas Schwab  <schwab@suse.de>

	* sysdeps/ia64/tcb-offsets.sym: Add PID.
	* sysdeps/unix/sysv/linux/ia64/vfork.S: New file.

--- nptl/sysdeps/ia64/tcb-offsets.sym.~1.5.~	2003-12-11 11:35:12.000000000 +0100
+++ nptl/sysdeps/ia64/tcb-offsets.sym	2004-03-09 10:49:45.000000000 +0100
@@ -1,5 +1,6 @@
 #include <sysdep.h>
 #include <tls.h>
 
+PID			offsetof (struct pthread, pid) - sizeof (struct pthread)
 MULTIPLE_THREADS_OFFSET offsetof (struct pthread, header.multiple_threads) - sizeof (struct pthread)
 SYSINFO_OFFSET		offsetof (tcbhead_t, private)
--- /dev/null	2004-02-28 23:52:18.000000000 +0100
+++ nptl/sysdeps/unix/sysv/linux/ia64/vfork.S	2004-03-09 13:19:05.000000000 +0100
@@ -0,0 +1,54 @@
+/* Copyright (C) 2000, 2002, 2004 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, write to the Free
+   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307 USA.  */
+
+
+#include <sysdep.h>
+#define _SIGNAL_H
+#include <bits/signum.h>
+#include <tcb-offsets.h>
+
+/* The following are defined in linux/sched.h, which unfortunately	*/
+/* is not safe for inclusion in an assembly file.			*/
+#define CLONE_VM        0x00000100      /* set if VM shared between processes */
+#define CLONE_VFORK     0x00004000      /* set if the parent wants the child to wake it up on mm_release */
+
+/* pid_t vfork(void); */
+/* Implemented as __clone_syscall(CLONE_VFORK | CLONE_VM | SIGCHLD, 0)	*/
+
+ENTRY(__vfork)
+	alloc r2=ar.pfs,0,1,2,0
+	adds r14=PID,r13
+	mov r15=-1
+	;; 
+	ld4 loc0=[r14]
+	st4 [r14]=r15
+	mov out0=CLONE_VM+CLONE_VFORK+SIGCHLD
+	mov out1=0		/* Standard sp value.			*/
+	;;
+	DO_CALL_VIA_BREAK (SYS_ify (clone))
+	cmp.eq p0,p7=0,r8
+	cmp.eq p6,p0=-1,r10
+	adds r14=PID,r13
+	;; 
+(p7)	st4 [r14]=loc0
+(p6)	br.cond.spnt.few __syscall_error
+	ret
+PSEUDO_END(__vfork)
+libc_hidden_def (__vfork)
+
+weak_alias (__vfork, vfork)

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH] getpid/vfork/raise fix
  2004-03-09 13:01             ` Andreas Schwab
@ 2004-03-09 13:59               ` Jakub Jelinek
  2004-03-09 16:44                 ` Andreas Schwab
                                   ` (2 more replies)
  0 siblings, 3 replies; 28+ messages in thread
From: Jakub Jelinek @ 2004-03-09 13:59 UTC (permalink / raw)
  To: Ulrich Drepper, Andreas Schwab; +Cc: Glibc hackers

On Tue, Mar 09, 2004 at 02:01:21PM +0100, Andreas Schwab wrote:
> Ulrich Drepper <drepper@redhat.com> writes:
> 
> > Anyway, for the getpid handling, the other archs need to be handled,
> > too.  The x86/x86-64 changes are kept small, I envision this is the
> > model for the other archs, too.
> 
> Here's the same for ia64.
> 
> Andreas.
> 
> 2004-03-09  Andreas Schwab  <schwab@suse.de>
> 
> 	* sysdeps/ia64/tcb-offsets.sym: Add PID.
> 	* sysdeps/unix/sysv/linux/ia64/vfork.S: New file.

Please wait a little bit.

The following patch changes vfork/raise so that tst-vfork2 included below
succeeds.  raise is async-safe and thus it certainly should work any time,
even while executing a vfork.
The raise.c changes at least on i386/x86-64 don't create any additional
instructions, just change a testl into andl.
Also, it changes libpthread.so's vfork to handle getpid () as well
(there SAVE_PID can be simpler, since THREAD_SELF->pid certainly will
be non-zero).

Also, x86-64 did not build at all:
In file included from sysdeps/unix/sysv/linux/x86_64/vfork.S:35,
                 from sysdeps/unix/sysv/linux/x86_64/vfork.S:35,
...
                 from sysdeps/unix/sysv/linux/x86_64/vfork.S:35,
                 from ../nptl/sysdeps/unix/sysv/linux/x86_64/pt-vfork.S:1:
sysdeps/unix/sysv/linux/x86_64/vfork.S:19:10: #include nested too deeply
sysdeps/unix/sysv/linux/x86_64/vfork.S:35:10: #include nested too deeply
make[2]: *** [/usr/src/libc/obj/nptl/pt-vfork.o] Error 1
because <sysdeps/unix/sysv/linux/x86_64/vfork.S> include in
nptl/sysdeps/unix/sysv/linux/x86_64/pt-vfork.S included
nptl/sysdeps/unix/sysv/linux/x86_64/vfork.S and so did that include
in nptl/sysdeps/unix/sysv/linux/x86_64/vfork.S.

2004-02-09  Jakub Jelinek  <jakub@redhat.com>

	* posix/Makefile (tests): Add tst-vfork2.
	* posix/tst-vfork1.c (do_test): Fix comment.
	* posix/tst-vfork2.c: New test.
nptl/
	* sysdeps/unix/sysv/linux/i386/vfork.S (SAVE_PID): Negate PID
	if non-zero and set to INT_MIN if zero.
	* sysdeps/unix/sysv/linux/x86_64/vfork.S (SAVE_PID): Likewise.
	* sysdeps/unix/sysv/linux/i386/pt-vfork.S: Include tcb-offsets.h.
	(SAVE_PID, RESTORE_PID): Define.
	(__vfork): Use it.
	* sysdeps/unix/sysv/linux/x86_64/pt-vfork.S: Include tcb-offsets.h.
	Use relative path to avoid including NPTL i386/vfork.S.
	(SAVE_PID, RESTORE_PID): Define.
	* sysdeps/unix/sysv/linux/raise.c: Include limits.h.
	(raise): Handle THREAD_SELF->pid INT_MIN the same as 0.
	* Makefile (tests): Add tst-vfork1, tst-vfork2, tst-vfork1x and
	tst-vfork2x.
	(tests-reverse): Add tst-vfork1x and tst-vfork2x.
	* tst-vfork1.c: New test.
	* tst-vfork2.c: New test.
	* tst-vfork1x.c: New test.
	* tst-vfork2x.c: New test.

--- libc/nptl/sysdeps/unix/sysv/linux/i386/pt-vfork.S.jj	2004-01-12 11:06:56.000000000 +0100
+++ libc/nptl/sysdeps/unix/sysv/linux/i386/pt-vfork.S	2004-03-09 14:47:21.957260874 +0100
@@ -1,4 +1,4 @@
-/* Copyright (C) 1999, 2002 Free Software Foundation, Inc.
+/* Copyright (C) 1999, 2002, 2004 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Andreas Schwab <schwab@gnu.org>.
 
@@ -21,6 +21,21 @@
 #define _ERRNO_H	1
 #include <bits/errno.h>
 #include <kernel-features.h>
+#include <tcb-offsets.h>
+
+/* Save the PID value.  */
+#define SAVE_PID \
+	movl	%gs:PID, %edx; 						      \
+	movl	%edx, %eax;						      \
+	negl	%eax;							      \
+	movl	%eax, %gs:PID
+
+/* Restore the old PID value in the parent.  */
+#define RESTORE_PID \
+	testl	%eax, %eax;						      \
+	je	1f;							      \
+	movl	%edx, %gs:PID;						      \
+1:
 
 /* Clone the calling process, but without copying the whole address space.
    The calling process is suspended until the new process exits or is
@@ -31,10 +46,14 @@ ENTRY (__vfork)
 	/* Pop the return PC value into ECX.  */
 	popl	%ecx
 
+	SAVE_PID
+
 	/* Stuff the syscall number in EAX and enter into the kernel.  */
 	movl	$SYS_ify (vfork), %eax
 	int	$0x80
 
+	RESTORE_PID
+
 	/* Jump to the return PC.  Don't jump directly since this
 	   disturbs the branch target cache.  Instead push the return
 	   address back on the stack.  */
--- libc/nptl/sysdeps/unix/sysv/linux/i386/vfork.S.jj	2004-03-09 07:14:20.000000000 +0100
+++ libc/nptl/sysdeps/unix/sysv/linux/i386/vfork.S	2004-03-09 13:56:18.874763704 +0100
@@ -21,9 +21,13 @@
 /* Save the PID value.  */
 #define SAVE_PID \
 	movl	%gs:PID, %edx; 						      \
-	movl	$-1, %gs:PID
+	movl	%edx, %eax;						      \
+	negl	%eax;							      \
+	jne	1f;							      \
+	movl	$0x80000000, %eax;					      \
+1:	movl	%eax, %gs:PID
 
-/* Restore the old PID value in the parent.  In the child store 0.  */
+/* Restore the old PID value in the parent.  */
 #define RESTORE_PID \
 	testl	%eax, %eax;						      \
 	je	1f;							      \
--- libc/nptl/sysdeps/unix/sysv/linux/raise.c.jj	2003-12-29 15:01:27.000000000 +0100
+++ libc/nptl/sysdeps/unix/sysv/linux/raise.c	2004-03-09 11:36:05.351402845 +0100
@@ -1,4 +1,4 @@
-/* Copyright (C) 2002, 2003 Free Software Foundation, Inc.
+/* Copyright (C) 2002, 2003, 2004 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Ulrich Drepper <drepper@redhat.com>, 2002.
 
@@ -18,6 +18,7 @@
    02111-1307 USA.  */
 
 #include <errno.h>
+#include <limits.h>
 #include <signal.h>
 #include <sysdep.h>
 #include <nptl/pthreadP.h>
@@ -53,10 +54,10 @@ raise (sig)
 #if __ASSUME_TGKILL || defined __NR_tgkill
   else
     /* raise is an async-safe function.  It could be called while the
-       fork function temporarily invalidated the PID field.  Adjust for
+       fork/vfork function temporarily invalidated the PID field.  Adjust for
        that.  */
     if (__builtin_expect (pid <= 0, 0))
-      pid = pid == 0 ? selftid : -pid;
+      pid = (pid & INT_MAX) == 0 ? selftid : -pid;
 #endif
 
 #if __ASSUME_TGKILL
--- libc/nptl/sysdeps/unix/sysv/linux/x86_64/pt-vfork.S.jj	2002-11-28 09:34:22.000000000 +0100
+++ libc/nptl/sysdeps/unix/sysv/linux/x86_64/pt-vfork.S	2004-03-09 14:44:53.636925647 +0100
@@ -1 +1,33 @@
-#include <sysdeps/unix/sysv/linux/x86_64/vfork.S>
+/* Copyright (C) 2004 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, write to the Free
+   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307 USA.  */
+
+#include <tcb-offsets.h>
+
+#define SAVE_PID \
+	movl	%fs:PID, %esi;						      \
+	movl	%esi, %edx;						      \
+	negl	%edx;							      \
+	movl	%edx, %fs:PID
+
+#define RESTORE_PID \
+	testq	%rax, %rax;						      \
+	je	1f;							      \
+	movl	%esi, %fs:PID;						      \
+1:
+
+#include <../../../../../../sysdeps/unix/sysv/linux/x86_64/vfork.S>
--- libc/nptl/sysdeps/unix/sysv/linux/x86_64/vfork.S.jj	2004-03-09 07:28:23.000000000 +0100
+++ libc/nptl/sysdeps/unix/sysv/linux/x86_64/vfork.S	2004-03-09 13:40:28.713295993 +0100
@@ -20,7 +20,11 @@
 
 #define SAVE_PID \
 	movl	%fs:PID, %esi;						      \
-	movl	$-1, %fs:PID
+	movl	$0x80000000, %ecx;					      \
+	movl	%esi, %edx;						      \
+	negl	%edx;							      \
+	cmove	%ecx, %edx;						      \
+	movl	%edx, %fs:PID
 
 #define RESTORE_PID \
 	testq	%rax, %rax;						      \
--- libc/nptl/Makefile.jj	2004-03-03 18:28:59.000000000 +0100
+++ libc/nptl/Makefile	2004-03-09 14:36:30.367401555 +0100
@@ -237,7 +237,8 @@ tests = tst-attr1 tst-attr2 tst-attr3 \
 	tst-context1 \
 	tst-sched1 \
 	tst-backtrace1 \
-	tst-oddstacklimit
+	tst-oddstacklimit \
+	tst-vfork1 tst-vfork2 tst-vfork1x tst-vfork2x
 
 # Files which must not be linked with libpthread.
 tests-nolibpthread = tst-unload
@@ -337,7 +338,7 @@ ifeq ($(build-static),yes)
 tests-static += tst-locale1 tst-locale2
 endif
 # These tests are linked with libc before libpthread
-tests-reverse += tst-cancel5 tst-cancel23
+tests-reverse += tst-cancel5 tst-cancel23 tst-vfork1x tst-vfork2x
 
 include ../Rules
 
--- libc/nptl/tst-vfork1.c.jj	2004-03-09 14:34:04.945544474 +0100
+++ libc/nptl/tst-vfork1.c	2004-03-09 14:33:57.961799957 +0100
@@ -0,0 +1 @@
+#include <posix/tst-vfork1.c>
--- libc/nptl/tst-vfork2.c.jj	2004-03-09 14:34:12.492187796 +0100
+++ libc/nptl/tst-vfork2.c	2004-03-09 14:34:16.293504425 +0100
@@ -0,0 +1 @@
+#include <posix/tst-vfork2.c>
--- libc/nptl/tst-vfork1x.c.jj	2004-03-09 14:34:04.000000000 +0100
+++ libc/nptl/tst-vfork1x.c	2004-03-09 14:33:57.000000000 +0100
@@ -0,0 +1 @@
+#include <posix/tst-vfork1.c>
--- libc/nptl/tst-vfork2x.c.jj	2004-03-09 14:35:10.298795753 +0100
+++ libc/nptl/tst-vfork2x.c	2004-03-09 14:35:00.748512634 +0100
@@ -0,0 +1 @@
+#include <posix/tst-vfork2.c>
--- libc/posix/tst-vfork2.c.jj	2004-03-09 13:18:30.939786289 +0100
+++ libc/posix/tst-vfork2.c	2004-03-09 13:32:32.517759464 +0100
@@ -0,0 +1,199 @@
+/* Test for chmod functions.
+   Copyright (C) 2004 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Ulrich Drepper <drepper@redhat.com>, 2004.
+
+   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, write to the Free
+   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307 USA.  */
+
+#include <errno.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <time.h>
+#include <unistd.h>
+#include <sys/time.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+int raise_fail;
+
+static void
+alrm (int sig)
+{
+  if (raise (SIGUSR1) < 0)
+    raise_fail = 1;
+}
+
+/* This test relies on non-POSIX functionality since the child
+   processes call write, nanosleep and getpid.  */
+static int
+do_test (void)
+{
+  int result = 0;
+  int fd[2];
+
+  signal (SIGUSR1, SIG_IGN);
+
+  struct sigaction sa;
+  sa.sa_handler = alrm;
+  sigemptyset (&sa.sa_mask);
+  sa.sa_flags = 0;
+  if (sigaction (SIGALRM, &sa, NULL) < 0)
+    {
+      puts ("couldn't set up SIGALRM handler");
+      return 1;
+    }
+
+  if (pipe (fd) == -1)
+    {
+      puts ("pipe failed");
+      return 1;
+    }
+
+  struct itimerval it;
+  it.it_value.tv_sec = 0;
+  it.it_value.tv_usec = 200 * 1000;
+  it.it_interval = it.it_value;
+  if (setitimer (ITIMER_REAL, &it, NULL) < 0)
+    {
+      puts ("couldn't set up timer");
+      return 1;
+    }
+
+  /* First vfork() without previous getpid().  */
+  pid_t p1;
+  if ((p1 = vfork ()) == 0)
+    {
+      pid_t p = getpid ();
+
+      struct timespec ts;
+      ts.tv_sec = 1;
+      ts.tv_nsec = 0;
+      TEMP_FAILURE_RETRY (nanosleep (&ts, &ts));
+      _exit (TEMP_FAILURE_RETRY (write (fd[1], &p, sizeof (p))) != sizeof (p));
+    }
+  else if (p1 == -1)
+    {
+      puts ("1st vfork failed");
+      result = 1;
+    }
+
+  memset (&it, 0, sizeof (it));
+  setitimer (ITIMER_REAL, &it, NULL);
+
+  pid_t p2 = 0;
+  if (TEMP_FAILURE_RETRY (read (fd[0], &p2, sizeof (pid_t))) != sizeof (pid_t))
+    {
+      puts ("1st read failed");
+      result = 1;
+    }
+  int r;
+  if (TEMP_FAILURE_RETRY (waitpid (p1, &r, 0)) != p1)
+    {
+      puts ("1st waitpid failed");
+      result = 1;
+    }
+  else if (r != 0)
+    {
+      puts ("write in 1st child failed");
+      result = 1;
+    }
+
+  /* Main process' ID.  */
+  pid_t p0 = getpid ();
+
+  /* vfork() again, but after a getpid() in the main process.  */
+  pid_t p3;
+  if ((p3 = vfork ()) == 0)
+    {
+      pid_t p = getpid ();
+      _exit (TEMP_FAILURE_RETRY (write (fd[1], &p, sizeof (p))) != sizeof (p));
+    }
+  else if (p1 == -1)
+    {
+      puts ("2nd vfork failed");
+      result = 1;
+    }
+
+  pid_t p4;
+  if (TEMP_FAILURE_RETRY (read (fd[0], &p4, sizeof (pid_t))) != sizeof (pid_t))
+    {
+      puts ("2nd read failed");
+      result = 1;
+    }
+  if (TEMP_FAILURE_RETRY (waitpid (p3, &r, 0)) != p3)
+    {
+      puts ("2nd waitpid failed");
+      result = 1;
+    }
+  else if (r != 0)
+    {
+      puts ("write in 2nd child failed");
+      result = 1;
+    }
+
+  /* And getpid in the main process again.  */
+  pid_t p5 = getpid ();
+
+  /* Analysis of the results.  */
+  if (p0 != p5)
+    {
+      printf ("p0(%ld) != p5(%ld)\n", (long int) p0, (long int) p5);
+      result = 1;
+    }
+
+  if (p0 == p1)
+    {
+      printf ("p0(%ld) == p1(%ld)\n", (long int) p0, (long int) p1);
+      result = 1;
+    }
+
+  if (p1 != p2)
+    {
+      printf ("p1(%ld) != p2(%ld)\n", (long int) p1, (long int) p2);
+      result = 1;
+    }
+
+  if (p0 == p3)
+    {
+      printf ("p0(%ld) == p3(%ld)\n", (long int) p0, (long int) p3);
+      result = 1;
+    }
+
+  if (p3 != p4)
+    {
+      printf ("p3(%ld) != p4(%ld)\n", (long int) p3, (long int) p4);
+      result = 1;
+    }
+
+  close (fd[0]);
+  close (fd[1]);
+
+  if (raise_fail)
+    {
+      puts ("raise failed");
+      result = 1;
+    }
+
+  if (result == 0)
+    puts ("All OK");
+
+  return result;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
--- libc/posix/tst-vfork1.c.jj	2004-03-09 09:49:09.000000000 +0100
+++ libc/posix/tst-vfork1.c	2004-03-09 11:33:26.138172512 +0100
@@ -26,7 +26,7 @@
 #include <sys/wait.h>
 
 /* This test relies on non-POSIX functionality since the child
-   processes call write.  */
+   processes call write and getpid.  */
 static int
 do_test (void)
 {
--- libc/posix/Makefile.jj	2004-03-09 10:58:14.000000000 +0100
+++ libc/posix/Makefile	2004-03-09 13:35:55.967245760 +0100
@@ -81,7 +81,7 @@ tests		:= tstgetopt testfnm runtests run
 		   bug-regex17 bug-regex18 bug-regex19 bug-regex20 \
 		   bug-regex21 bug-regex22 bug-regex23 tst-nice tst-nanosleep \
 		   transbug tst-rxspencer tst-pcre tst-boost \
-		   bug-ga1 tst-vfork1
+		   bug-ga1 tst-vfork1 tst-vfork2
 xtests		:= bug-ga2
 ifeq (yes,$(build-shared))
 test-srcs	:= globtest


	Jakub

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] getpid/vfork/raise fix
  2004-03-09 13:59               ` [PATCH] getpid/vfork/raise fix Jakub Jelinek
@ 2004-03-09 16:44                 ` Andreas Schwab
  2004-03-09 19:38                   ` Jakub Jelinek
  2004-03-10  5:53                 ` Ulrich Drepper
  2004-03-10 10:37                 ` Andreas Schwab
  2 siblings, 1 reply; 28+ messages in thread
From: Andreas Schwab @ 2004-03-09 16:44 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Ulrich Drepper, Glibc hackers

Jakub Jelinek <jakub@redhat.com> writes:

> --- libc/posix/tst-vfork2.c.jj	2004-03-09 13:18:30.939786289 +0100
> +++ libc/posix/tst-vfork2.c	2004-03-09 13:32:32.517759464 +0100
> @@ -0,0 +1,199 @@
> +/* Test for chmod functions.

Typo.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] getpid/vfork/raise fix
  2004-03-09 16:44                 ` Andreas Schwab
@ 2004-03-09 19:38                   ` Jakub Jelinek
  0 siblings, 0 replies; 28+ messages in thread
From: Jakub Jelinek @ 2004-03-09 19:38 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Ulrich Drepper, Glibc hackers

On Tue, Mar 09, 2004 at 05:44:10PM +0100, Andreas Schwab wrote:
> Jakub Jelinek <jakub@redhat.com> writes:
> 
> > --- libc/posix/tst-vfork2.c.jj	2004-03-09 13:18:30.939786289 +0100
> > +++ libc/posix/tst-vfork2.c	2004-03-09 13:32:32.517759464 +0100
> > @@ -0,0 +1,199 @@
> > +/* Test for chmod functions.
> 
> Typo.

Pasto actually, tst-vfork1.c has the same comment.

	Jakub

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] getpid/vfork/raise fix
  2004-03-09 13:59               ` [PATCH] getpid/vfork/raise fix Jakub Jelinek
  2004-03-09 16:44                 ` Andreas Schwab
@ 2004-03-10  5:53                 ` Ulrich Drepper
  2004-03-10 10:37                 ` Andreas Schwab
  2 siblings, 0 replies; 28+ messages in thread
From: Ulrich Drepper @ 2004-03-10  5:53 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Glibc hackers

I've applied the patch.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] getpid/vfork/raise fix
  2004-03-09 13:59               ` [PATCH] getpid/vfork/raise fix Jakub Jelinek
  2004-03-09 16:44                 ` Andreas Schwab
  2004-03-10  5:53                 ` Ulrich Drepper
@ 2004-03-10 10:37                 ` Andreas Schwab
  2004-03-10 12:24                   ` Jakub Jelinek
  2004-03-10 18:44                   ` Ulrich Drepper
  2 siblings, 2 replies; 28+ messages in thread
From: Andreas Schwab @ 2004-03-10 10:37 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Ulrich Drepper, Glibc hackers

Here are the corresponding changes for ia64.

Andreas.

2004-03-10  Andreas Schwab  <schwab@suse.de>

	* sysdeps/ia64/tcb-offsets.sym: Add PID.
	* sysdeps/unix/sysv/linux/ia64/vfork.S: New file.
	* sysdeps/unix/sysv/linux/ia64/pt-vfork.S: Properly handle PID
	cache.

--- nptl/sysdeps/ia64/tcb-offsets.sym.~1.5.~	2003-12-11 11:35:12.000000000 +0100
+++ nptl/sysdeps/ia64/tcb-offsets.sym	2004-03-09 10:49:45.000000000 +0100
@@ -1,5 +1,6 @@
 #include <sysdep.h>
 #include <tls.h>
 
+PID			offsetof (struct pthread, pid) - sizeof (struct pthread)
 MULTIPLE_THREADS_OFFSET offsetof (struct pthread, header.multiple_threads) - sizeof (struct pthread)
 SYSINFO_OFFSET		offsetof (tcbhead_t, private)
--- nptl/sysdeps/unix/sysv/linux/ia64/pt-vfork.S.~1.2.~	2003-12-11 11:35:12.000000000 +0100
+++ nptl/sysdeps/unix/sysv/linux/ia64/pt-vfork.S	2004-03-10 11:28:34.057587179 +0100
@@ -1,4 +1,4 @@
-/* Copyright (C) 2000, 2002, 2003 Free Software Foundation, Inc.
+/* Copyright (C) 2000, 2002, 2003, 2004 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
@@ -32,13 +32,22 @@
 ENTRY(__vfork)
 	.prologue	// work around a GAS bug which triggers if
 	.body		// first .prologue is not at the beginning of proc.
-	alloc r2=ar.pfs,0,0,2,0
+	alloc r2=ar.pfs,0,1,2,0
+	adds r14=PID,r13
+	;; 
+	ld4 loc0=[r14]
+	;;
+	sub r15=0,loc0
 	mov out0=CLONE_VM+CLONE_VFORK+SIGCHLD
 	mov out1=0		/* Standard sp value.			*/
 	;;
+	st4 [r14]=r15
 	DO_CALL (SYS_ify (clone))
+	cmp.eq p0,p7=0,r8
 	cmp.eq p6,p0=-1,r10
+	adds r14=PID,r13
 	;;
+(p7)	st4 [r14]=loc0
 (p6)	br.cond.spnt.few __syscall_error
 	ret
 PSEUDO_END(__vfork)
--- /dev/null	2004-02-28 23:52:18.000000000 +0100
+++ nptl/sysdeps/unix/sysv/linux/ia64/vfork.S	2004-03-10 09:36:47.000000000 +0100
@@ -0,0 +1,58 @@
+/* Copyright (C) 2000, 2002, 2004 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, write to the Free
+   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307 USA.  */
+
+
+#include <sysdep.h>
+#define _SIGNAL_H
+#include <bits/signum.h>
+#include <tcb-offsets.h>
+
+/* The following are defined in linux/sched.h, which unfortunately	*/
+/* is not safe for inclusion in an assembly file.			*/
+#define CLONE_VM        0x00000100      /* set if VM shared between processes */
+#define CLONE_VFORK     0x00004000      /* set if the parent wants the child to wake it up on mm_release */
+
+/* pid_t vfork(void); */
+/* Implemented as __clone_syscall(CLONE_VFORK | CLONE_VM | SIGCHLD, 0)	*/
+
+ENTRY(__vfork)
+	alloc r2=ar.pfs,0,1,2,0
+	adds r14=PID,r13
+	;; 
+	ld4 loc0=[r14]
+	;;
+	sub r15=0,loc0
+	cmp.eq p0,p6=0,loc0
+	;;
+(p6)	movl r15=0x80000000
+	mov out0=CLONE_VM+CLONE_VFORK+SIGCHLD
+	mov out1=0		/* Standard sp value.			*/
+	st4 [r14]=r15
+	;;
+	DO_CALL_VIA_BREAK (SYS_ify (clone))
+	cmp.eq p0,p7=0,r8
+	cmp.eq p6,p0=-1,r10
+	adds r14=PID,r13
+	;; 
+(p7)	st4 [r14]=loc0
+(p6)	br.cond.spnt.few __syscall_error
+	ret
+PSEUDO_END(__vfork)
+libc_hidden_def (__vfork)
+
+weak_alias (__vfork, vfork)

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] getpid/vfork/raise fix
  2004-03-10 10:37                 ` Andreas Schwab
@ 2004-03-10 12:24                   ` Jakub Jelinek
  2004-03-10 14:33                     ` Andreas Schwab
  2004-03-10 18:44                   ` Ulrich Drepper
  1 sibling, 1 reply; 28+ messages in thread
From: Jakub Jelinek @ 2004-03-10 12:24 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Ulrich Drepper, Glibc hackers

On Wed, Mar 10, 2004 at 11:37:43AM +0100, Andreas Schwab wrote:
> --- nptl/sysdeps/unix/sysv/linux/ia64/pt-vfork.S.~1.2.~	2003-12-11 11:35:12.000000000 +0100
> +++ nptl/sysdeps/unix/sysv/linux/ia64/pt-vfork.S	2004-03-10 11:28:34.057587179 +0100
> @@ -1,4 +1,4 @@
> -/* Copyright (C) 2000, 2002, 2003 Free Software Foundation, Inc.
> +/* Copyright (C) 2000, 2002, 2003, 2004 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
> @@ -32,13 +32,22 @@
>  ENTRY(__vfork)
>  	.prologue	// work around a GAS bug which triggers if
>  	.body		// first .prologue is not at the beginning of proc.
> -	alloc r2=ar.pfs,0,0,2,0
> +	alloc r2=ar.pfs,0,1,2,0
> +	adds r14=PID,r13
> +	;; 
> +	ld4 loc0=[r14]
> +	;;
> +	sub r15=0,loc0
>  	mov out0=CLONE_VM+CLONE_VFORK+SIGCHLD
>  	mov out1=0		/* Standard sp value.			*/
>  	;;
> +	st4 [r14]=r15
>  	DO_CALL (SYS_ify (clone))
> +	cmp.eq p0,p7=0,r8
>  	cmp.eq p6,p0=-1,r10
> +	adds r14=PID,r13
>  	;;
> +(p7)	st4 [r14]=loc0

Can you use a local register for saving oldval accross the syscall though?
Registers in the register window can be saved to the backing store, if that
happens, vfork child restores it from the backing store, calls some routine
and something else is stored into that location in backing store, vfork
parent will afterwards restore a different value.
E.g. IA-32 vfork saves return address in a register and not on the stack for
a reason.

IMHO only a call clobbered register preserved accross system call can be
used, and is it is not available, vfork has to choose a slower way:

int oldval = self->pid;
int newval = oldval ? -oldval : 0x80000000;
self->pid = newval;
vfork syscall
newval = self->pid;
oldval = (newval & 0x7fffffff) ? -newval : 0;
self->pid = oldval;

(and with pt-vfork just self->pid = -self->pid; before and after).

	Jakub

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] getpid/vfork/raise fix
  2004-03-10 12:24                   ` Jakub Jelinek
@ 2004-03-10 14:33                     ` Andreas Schwab
  2004-03-10 15:01                       ` Jakub Jelinek
  2004-03-10 16:56                       ` David Mosberger
  0 siblings, 2 replies; 28+ messages in thread
From: Andreas Schwab @ 2004-03-10 14:33 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Ulrich Drepper, Glibc hackers

Jakub Jelinek <jakub@redhat.com> writes:

> Can you use a local register for saving oldval accross the syscall though?
> Registers in the register window can be saved to the backing store, if that
> happens, vfork child restores it from the backing store, calls some routine
> and something else is stored into that location in backing store, vfork
> parent will afterwards restore a different value.

I think you are right.

> IMHO only a call clobbered register preserved accross system call can be
> used, and is it is not available, vfork has to choose a slower way:
>
> int oldval = self->pid;
> int newval = oldval ? -oldval : 0x80000000;
> self->pid = newval;
> vfork syscall
> newval = self->pid;
> oldval = (newval & 0x7fffffff) ? -newval : 0;
> self->pid = oldval;

That doesn't seem to work, though.  I'm getting p3 != p4 in the posix
vfork tests.  Or do you see any error in my implementation?

ENTRY(__vfork)
	alloc r2=ar.pfs,0,0,2,0
	adds r14=PID,r13
	;; 
	ld4 r16=[r14]
	;;
	sub r15=0,r16
	cmp.eq p0,p6=0,r16
	;;
(p6)	movl r15=0x80000000
	mov out0=CLONE_VM+CLONE_VFORK+SIGCHLD
	mov out1=0		/* Standard sp value.			*/
	;;
	st4 [r14]=r15
	DO_CALL_VIA_BREAK (SYS_ify (clone))
	cmp.eq p6,p0=0,r8
	adds r14=PID,r13
(p6)	br.cond.dptk 1f
	;;
	ld4 r15=[r14]
	;;
	extr.u r16=r15,0,31
	;;
	cmp.eq p0,p6=0,r16
	;;
(p6)	sub r16=0,r15
	;;
	st4 [r14]=r16
1:
	cmp.eq p6,p0=-1,r10
(p6)	br.cond.spnt.few __syscall_error
	ret
PSEUDO_END(__vfork)

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] getpid/vfork/raise fix
  2004-03-10 14:33                     ` Andreas Schwab
@ 2004-03-10 15:01                       ` Jakub Jelinek
  2004-03-10 15:06                         ` Andreas Schwab
  2004-03-10 16:56                       ` David Mosberger
  1 sibling, 1 reply; 28+ messages in thread
From: Jakub Jelinek @ 2004-03-10 15:01 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Ulrich Drepper, Glibc hackers

On Wed, Mar 10, 2004 at 03:33:05PM +0100, Andreas Schwab wrote:
> That doesn't seem to work, though.  I'm getting p3 != p4 in the posix
> vfork tests.  Or do you see any error in my implementation?

Yes.

> ENTRY(__vfork)
> 	alloc r2=ar.pfs,0,0,2,0
> 	adds r14=PID,r13
> 	;; 
> 	ld4 r16=[r14]
> 	;;
> 	sub r15=0,r16
> 	cmp.eq p0,p6=0,r16

This line should read
	cmp.eq p6,p0=0,r16
instead.
You want to set ->pid to 0x80000000 if ->pid has been previously 0, not
non-zero.
With this change both tst-vfork1.c and tst-vfork2.c pass for me
(well, I was testing with a LD_PRELOAD'ed lib containing just this assembly,
did not want to build full ia64 glibc).

	Jakub

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] getpid/vfork/raise fix
  2004-03-10 15:01                       ` Jakub Jelinek
@ 2004-03-10 15:06                         ` Andreas Schwab
  0 siblings, 0 replies; 28+ messages in thread
From: Andreas Schwab @ 2004-03-10 15:06 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Ulrich Drepper, Glibc hackers

Jakub Jelinek <jakub@redhat.com> writes:

> On Wed, Mar 10, 2004 at 03:33:05PM +0100, Andreas Schwab wrote:
>> That doesn't seem to work, though.  I'm getting p3 != p4 in the posix
>> vfork tests.  Or do you see any error in my implementation?
>
> Yes.
>
>> ENTRY(__vfork)
>> 	alloc r2=ar.pfs,0,0,2,0
>> 	adds r14=PID,r13
>> 	;; 
>> 	ld4 r16=[r14]
>> 	;;
>> 	sub r15=0,r16
>> 	cmp.eq p0,p6=0,r16
>
> This line should read
> 	cmp.eq p6,p0=0,r16
> instead.
> You want to set ->pid to 0x80000000 if ->pid has been previously 0, not
> non-zero.

You're right, of course.  Strange it didn't fail with my old version.

> With this change both tst-vfork1.c and tst-vfork2.c pass for me

/me too.

Thanks, Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] getpid/vfork/raise fix
  2004-03-10 14:33                     ` Andreas Schwab
  2004-03-10 15:01                       ` Jakub Jelinek
@ 2004-03-10 16:56                       ` David Mosberger
  2004-03-10 17:02                         ` Jakub Jelinek
  1 sibling, 1 reply; 28+ messages in thread
From: David Mosberger @ 2004-03-10 16:56 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Jakub Jelinek, Ulrich Drepper, Glibc hackers

>>>>> On Wed, 10 Mar 2004 15:33:05 +0100, Andreas Schwab <schwab@suse.de> said:

  Andreas> That doesn't seem to work, though.  I'm getting p3 != p4 in
  Andreas> the posix vfork tests.  Or do you see any error in my
  Andreas> implementation?

Most scratch registers are _not_ preserved across system calls.  You
could spill the register onto the memory stack though.  If it uses at
most 16 bytes, you won't have to adjust the stack pointer even.

	--david

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] getpid/vfork/raise fix
  2004-03-10 16:56                       ` David Mosberger
@ 2004-03-10 17:02                         ` Jakub Jelinek
  2004-03-10 17:45                           ` David Mosberger
  0 siblings, 1 reply; 28+ messages in thread
From: Jakub Jelinek @ 2004-03-10 17:02 UTC (permalink / raw)
  To: davidm; +Cc: Andreas Schwab, Ulrich Drepper, Glibc hackers

On Wed, Mar 10, 2004 at 08:38:12AM -0800, David Mosberger wrote:
> >>>>> On Wed, 10 Mar 2004 15:33:05 +0100, Andreas Schwab <schwab@suse.de> said:
> 
>   Andreas> That doesn't seem to work, though.  I'm getting p3 != p4 in
>   Andreas> the posix vfork tests.  Or do you see any error in my
>   Andreas> implementation?
> 
> Most scratch registers are _not_ preserved across system calls.  You
> could spill the register onto the memory stack though.  If it uses at
> most 16 bytes, you won't have to adjust the stack pointer even.

But as soon as you spill something into memory the vfork child can clobber
it and you restore in the parent something different from what you saved.

	Jakub

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] getpid/vfork/raise fix
  2004-03-10 17:02                         ` Jakub Jelinek
@ 2004-03-10 17:45                           ` David Mosberger
  2004-03-10 18:19                             ` Jakub Jelinek
  0 siblings, 1 reply; 28+ messages in thread
From: David Mosberger @ 2004-03-10 17:45 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: davidm, Andreas Schwab, Ulrich Drepper, Glibc hackers

>>>>> On Wed, 10 Mar 2004 15:50:52 +0100, Jakub Jelinek <jakub@redhat.com> said:

  >>  Most scratch registers are _not_ preserved across system calls.
  >> You could spill the register onto the memory stack though.  If it
  >> uses at most 16 bytes, you won't have to adjust the stack pointer
  >> even.

  Jakub> But as soon as you spill something into memory the vfork
  Jakub> child can clobber it and you restore in the parent something
  Jakub> different from what you saved.

How many registers need to be preserved?  Just one?  (Sorry, I could
read the code but I'm a bit preoccupied with other stuff at the
moment.)  If it's just one, I think you could use r11 because that one
is required to be preserved for the light-weight syscall convention.

	--david

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] getpid/vfork/raise fix
  2004-03-10 17:45                           ` David Mosberger
@ 2004-03-10 18:19                             ` Jakub Jelinek
  2004-03-10 19:24                               ` David Mosberger
  0 siblings, 1 reply; 28+ messages in thread
From: Jakub Jelinek @ 2004-03-10 18:19 UTC (permalink / raw)
  To: davidm; +Cc: Andreas Schwab, Ulrich Drepper, Glibc hackers

On Wed, Mar 10, 2004 at 09:45:00AM -0800, David Mosberger wrote:
> >>>>> On Wed, 10 Mar 2004 15:50:52 +0100, Jakub Jelinek <jakub@redhat.com> said:
> 
>   >>  Most scratch registers are _not_ preserved across system calls.
>   >> You could spill the register onto the memory stack though.  If it
>   >> uses at most 16 bytes, you won't have to adjust the stack pointer
>   >> even.
> 
>   Jakub> But as soon as you spill something into memory the vfork
>   Jakub> child can clobber it and you restore in the parent something
>   Jakub> different from what you saved.
> 
> How many registers need to be preserved?  Just one?  (Sorry, I could
> read the code but I'm a bit preoccupied with other stuff at the
> moment.)  If it's just one, I think you could use r11 because that one
> is required to be preserved for the light-weight syscall convention.

One (plus b0) is needed.
But first of all I'd like to understand the differences between
NPTL pt-vfork.S and libc vfork.S on IA-64.  Why does one use
DO_CALL_VIA_BREAK and the other one DO_CALL, one has gas workarounds and one
does not?

	Jakub

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] getpid/vfork/raise fix
  2004-03-10 10:37                 ` Andreas Schwab
  2004-03-10 12:24                   ` Jakub Jelinek
@ 2004-03-10 18:44                   ` Ulrich Drepper
  2004-03-10 19:18                     ` [PATCH] IA-64 pt-vfork fix Jakub Jelinek
  1 sibling, 1 reply; 28+ messages in thread
From: Ulrich Drepper @ 2004-03-10 18:44 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Glibc hackers

I applied the hopefully correct patch.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH] IA-64 pt-vfork fix
  2004-03-10 18:44                   ` Ulrich Drepper
@ 2004-03-10 19:18                     ` Jakub Jelinek
  2004-03-10 23:10                       ` Ulrich Drepper
  0 siblings, 1 reply; 28+ messages in thread
From: Jakub Jelinek @ 2004-03-10 19:18 UTC (permalink / raw)
  To: Ulrich Drepper; +Cc: Andreas Schwab, Glibc hackers

On Wed, Mar 10, 2004 at 10:44:07AM -0800, Ulrich Drepper wrote:
> I applied the hopefully correct patch.

pt-vfork.S still needed similar changes.
The following patch works for me with LD_PRELOAD=pt-vfork.so:libpthread.so ./tst-vfork{1,2}

2004-03-10  Jakub Jelinek  <jakub@redhat.com>

	* sysdeps/unix/sysv/linux/ia64/pt-vfork.S (__vfork): Don't use
	a local register for saving old PID.  Negate PID in parent upon
	exit.

--- libc/nptl/sysdeps/unix/sysv/linux/ia64/pt-vfork.S.jj	2004-03-10 20:02:20.000000000 +0100
+++ libc/nptl/sysdeps/unix/sysv/linux/ia64/pt-vfork.S	2004-03-10 20:14:25.288330129 +0100
@@ -32,22 +32,26 @@
 ENTRY(__vfork)
 	.prologue	// work around a GAS bug which triggers if
 	.body		// first .prologue is not at the beginning of proc.
-	alloc r2=ar.pfs,0,1,2,0
+	alloc r2=ar.pfs,0,0,2,0
 	adds r14=PID,r13
 	;; 
-	ld4 loc0=[r14]
+	ld4 r16=[r14]
 	;;
-	sub r15=0,loc0
+	sub r15=0,r16
 	mov out0=CLONE_VM+CLONE_VFORK+SIGCHLD
 	mov out1=0		/* Standard sp value.			*/
 	;;
 	st4 [r14]=r15
 	DO_CALL (SYS_ify (clone))
 	cmp.eq p0,p7=0,r8
-	cmp.eq p6,p0=-1,r10
 	adds r14=PID,r13
 	;;
-(p7)	st4 [r14]=loc0
+(p7)	ld4 r16=[r14]
+	cmp.eq p6,p0=-1,r10
+	;;
+(p7)	sub r15=0,r16
+	;;
+(p7)	st4 [r14]=r15
 (p6)	br.cond.spnt.few __syscall_error
 	ret
 PSEUDO_END(__vfork)


	Jakub

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] getpid/vfork/raise fix
  2004-03-10 18:19                             ` Jakub Jelinek
@ 2004-03-10 19:24                               ` David Mosberger
  0 siblings, 0 replies; 28+ messages in thread
From: David Mosberger @ 2004-03-10 19:24 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: davidm, Andreas Schwab, Ulrich Drepper, Glibc hackers

>>>>> On Wed, 10 Mar 2004 17:07:58 +0100, Jakub Jelinek <jakub@redhat.com> said:

  Jakub> One (plus b0) is needed.  But first of all I'd like to
  Jakub> understand the differences between NPTL pt-vfork.S and libc
  Jakub> vfork.S on IA-64.  Why does one use DO_CALL_VIA_BREAK and the
  Jakub> other one DO_CALL, one has gas workarounds and one does not?

My memory is a bit fuzzy (isn't it always... ;-), but as I remember
it, new-style syscall stub support is tied to NPTL, so vfork.S is
never actually compiled when new-style syscalls stubs are in effect.
Because of that, DO_CALL_VIA_BREAK isn't _necessary_ for vfork.S.
Furthermore, the gas workarounds are needed only for the new-style
syscall stubs.  If you preferred them to be in sync, I _think_ it
would be OK to make vfork.S more similar (perhaps even identical) to
pt-vfork.S.

	--david

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] IA-64 pt-vfork fix
  2004-03-10 19:18                     ` [PATCH] IA-64 pt-vfork fix Jakub Jelinek
@ 2004-03-10 23:10                       ` Ulrich Drepper
  0 siblings, 0 replies; 28+ messages in thread
From: Ulrich Drepper @ 2004-03-10 23:10 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Glibc hackers

Applied.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2004-03-10 23:10 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-03-07 23:18 getpid/vfork broken Andreas Schwab
2004-03-07 23:30 ` Ulrich Drepper
2004-03-08  2:22   ` Roland McGrath
2004-03-08  2:28     ` Ulrich Drepper
2004-03-08  2:37       ` Roland McGrath
2004-03-08 13:10         ` Jakub Jelinek
2004-03-09  1:28           ` Roland McGrath
2004-03-09  2:04             ` Ulrich Drepper
2004-03-09  3:50               ` Richard Henderson
2004-03-09  7:48           ` Ulrich Drepper
2004-03-09 13:01             ` Andreas Schwab
2004-03-09 13:59               ` [PATCH] getpid/vfork/raise fix Jakub Jelinek
2004-03-09 16:44                 ` Andreas Schwab
2004-03-09 19:38                   ` Jakub Jelinek
2004-03-10  5:53                 ` Ulrich Drepper
2004-03-10 10:37                 ` Andreas Schwab
2004-03-10 12:24                   ` Jakub Jelinek
2004-03-10 14:33                     ` Andreas Schwab
2004-03-10 15:01                       ` Jakub Jelinek
2004-03-10 15:06                         ` Andreas Schwab
2004-03-10 16:56                       ` David Mosberger
2004-03-10 17:02                         ` Jakub Jelinek
2004-03-10 17:45                           ` David Mosberger
2004-03-10 18:19                             ` Jakub Jelinek
2004-03-10 19:24                               ` David Mosberger
2004-03-10 18:44                   ` Ulrich Drepper
2004-03-10 19:18                     ` [PATCH] IA-64 pt-vfork fix Jakub Jelinek
2004-03-10 23:10                       ` Ulrich Drepper

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).