[PATCH] Fix bug with popen/pthread_create

[PATCH] Fix bug with popen/pthread_create

[Jakub Jelinek]

On Thu, Feb 06, 2003 at 05:16:00PM +0100, Thorsten Kukuk wrote:
> > > the appendend testprogram works fine with glibc 2.2.x, but it does
> > > not work with current glibc from CVS. The "printf("in main\n");
> > > is never reached. If I don't call popen(), it works fine. Any ideas,
> > > what could go wrong? This happens on all architectures.

The following patch fixes it on a few arches (tested IA-32 so far).
The remaining ones have to be changed too.

2003-02-06  Jakub Jelinek  <jakub@redhat.com>

	* tst-popen2.c: New test.
	* Makefile (tests): Add tst-popen2.
	* sysdeps/unix/sysv/linux/alpha/vfork.S (__vfork): Branch to __fork
	whenever libpthread.so is loaded.
	* sysdeps/unix/sysv/linux/i386/vfork.S (__vfork): Likewise.
	* sysdeps/unix/sysv/linux/ia64/vfork.S (__vfork): Likewise.
	* sysdeps/unix/sysv/linux/s390/s390-64/vfork.S (__vfork): Likewise.
	* sysdeps/unix/sysv/linux/sparc/sparc32/vfork.S (__vfork): Likewise.
	* sysdeps/unix/sysv/linux/sparc/sparc64/vfork.S (__vfork): Likewise.
	* sysdeps/unix/sysv/linux/x86_64/vfork.S (__vfork): Likewise.

--- libc/linuxthreads/sysdeps/unix/sysv/linux/alpha/vfork.S.jj	2003-01-17 14:17:36.000000000 -0500
+++ libc/linuxthreads/sysdeps/unix/sysv/linux/alpha/vfork.S	2003-02-06 14:46:18.000000000 -0500
@@ -27,10 +27,12 @@ __LABEL(__vfork)
 	.prologue 1
 	PSEUDO_PROF
 
-	SINGLE_THREAD_P(t0)
 #ifdef SHARED
+	ldq	t0, __libc_pthread_functions(gp) !gprel
 	bne	t0, HIDDEN_JUMPTARGET (__fork) !samegp
 #else
+	.weak	__pthread_fork
+	ldq	t0, __pthread_fork(gp) !literal
 	bne	t0, $do_fork
 #endif
 
--- libc/linuxthreads/sysdeps/unix/sysv/linux/i386/vfork.S.jj	2003-01-09 15:27:26.000000000 -0500
+++ libc/linuxthreads/sysdeps/unix/sysv/linux/i386/vfork.S	2003-02-06 13:31:43.000000000 -0500
@@ -31,7 +31,19 @@ ENTRY (__vfork)
 
 #ifdef __NR_vfork
 
-	SINGLE_THREAD_P
+# ifdef SHARED
+#  if !defined HAVE_HIDDEN || !USE___THREAD
+	SETUP_PIC_REG (cx)
+#  else
+	call	__i686.get_pc_thunk.cx
+#  endif
+	addl	$_GLOBAL_OFFSET_TABLE_, %ecx
+	cmpl	$0, __libc_pthread_functions@GOTOFF(%ecx)
+# else
+	.weak	__pthread_fork
+	movl	$__pthread_fork, %eax
+	testl	%eax, %eax
+# endif
 	jne	HIDDEN_JUMPTARGET (__fork)
 
 	/* Pop the return PC value into ECX.  */
--- libc/linuxthreads/sysdeps/unix/sysv/linux/ia64/vfork.S.jj	2003-01-13 20:23:24.000000000 -0500
+++ libc/linuxthreads/sysdeps/unix/sysv/linux/ia64/vfork.S	2003-02-06 14:58:10.000000000 -0500
@@ -30,7 +30,14 @@
 /* Implemented as __clone_syscall(CLONE_VFORK | CLONE_VM | SIGCHLD, 0)	*/
 
 ENTRY(__vfork)
-	SINGLE_THREAD_P
+#ifdef SHARED
+	addl r14 = @gprel(__libc_pthread_functions#), gp;;
+#else
+	.weak	__pthread_fork
+	addl r14 = @ltoff(@fptr(__pthread_fork#)), gp;;
+#endif
+	ld8 r14 = [r14];;
+	cmp.ne p6, p7 = 0, r14
 (p6)	br.cond.spnt.few HIDDEN_JUMPTARGET (__fork);;
 	alloc r2=ar.pfs,0,0,2,0
 	mov out0=CLONE_VM+CLONE_VFORK+SIGCHLD
--- libc/linuxthreads/sysdeps/unix/sysv/linux/s390/s390-64/vfork.S.jj	2003-01-28 05:30:30.000000000 -0500
+++ libc/linuxthreads/sysdeps/unix/sysv/linux/s390/s390-64/vfork.S	2003-02-06 14:51:50.000000000 -0500
@@ -27,7 +27,14 @@
    and the process ID of the new process to the old process.  */
 
 ENTRY (__vfork)
-	SINGLE_THREAD_P
+#ifdef SHARED
+	larl	%r1,__libc_pthread_functions
+	lg	%r1,0(%r1)
+#else
+	.weak	__pthread_fork
+	larl	%r1,__pthread_fork
+#endif
+	ltgr	%r1,%r1
 	jgne	HIDDEN_JUMPTARGET(__fork)
 
 	/* Do vfork system call.  */
--- libc/linuxthreads/sysdeps/unix/sysv/linux/sparc/sparc32/vfork.S.jj	2003-01-27 15:48:41.000000000 -0500
+++ libc/linuxthreads/sysdeps/unix/sysv/linux/sparc/sparc32/vfork.S	2003-02-06 14:32:57.000000000 -0500
@@ -20,12 +20,28 @@
 #include <sysdep-cancel.h>
 
 	.text
+#ifdef SHARED
+.LLGETPC0:
+	retl
+	 addl	%o7, %o0, %o0
+#endif
 ENTRY(__vfork)
-	ld	[%g7 + MULTIPLE_THREADS_OFFSET], %o0
-	cmp	%o0, 0
 #ifdef SHARED
+	mov	%o7, %o1
+	sethi	%hi(_GLOBAL_OFFSET_TABLE_-4), %o0
+	call	.LLGETPC0
+	 add	%o0, %lo(_GLOBAL_OFFSET_TABLE_+4), %o0
+	sethi	%hi(__libc_pthread_functions), %o2
+	mov	%o1, %o7
+	or	%o2, %lo(__libc_pthread_functions), %o2
+	ld	[%o0 + %o2], %o2
+	ld	[%o2], %o2
+	cmp	%o2, 0
 	bne	HIDDEN_JUMPTARGET(__fork)
 #else
+	.weak	__pthread_fork
+	sethi	%hi(__pthread_fork), %o0
+	orcc	%o0, %lo(__pthread_fork), %o0
 	bne	1f
 #endif
 	 mov	__NR_vfork, %g1
--- libc/linuxthreads/sysdeps/unix/sysv/linux/sparc/sparc64/vfork.S.jj	2003-01-27 15:48:41.000000000 -0500
+++ libc/linuxthreads/sysdeps/unix/sysv/linux/sparc/sparc64/vfork.S	2003-02-06 14:36:15.000000000 -0500
@@ -19,13 +19,28 @@
 
 #include <sysdep-cancel.h>
 
-	.text
+#ifdef SHARED
+.LLGETPC0:
+	retl
+	 addl	%o7, %o0, %o0
+#endif
 ENTRY(__vfork)
-	ld	[%g7 + MULTIPLE_THREADS_OFFSET], %o0
 #ifdef SHARED
-	cmp	%o0, 0
-	bne	HIDDEN_JUMPTARGET (__fork)
+	mov	%o7, %o1
+	sethi	%hi(_GLOBAL_OFFSET_TABLE_-4), %o0
+	call	.LLGETPC0
+	 add	%o0, %lo(_GLOBAL_OFFSET_TABLE_+4), %o0
+	sethi	%hi(__libc_pthread_functions), %o2
+	mov	%o1, %o7
+	or	%o2, %lo(__libc_pthread_functions), %o2
+	ldx	[%o0 + %o2], %o2
+	ldx	[%o2], %o2
+	cmp	%o2, 0
+	bne	HIDDEN_JUMPTARGET(__fork)
 #else
+	.weak	__pthread_fork
+	sethi	%hi(__pthread_fork), %o0
+	or	%o0, %lo(__pthread_fork), %o0
 	brnz,pn	%o0, 1f
 #endif
 	 mov	__NR_vfork, %g1
--- libc/linuxthreads/sysdeps/unix/sysv/linux/x86_64/vfork.S.jj	2003-01-09 15:28:21.000000000 -0500
+++ libc/linuxthreads/sysdeps/unix/sysv/linux/x86_64/vfork.S	2003-02-06 14:33:23.000000000 -0500
@@ -27,7 +27,13 @@
 
 ENTRY (__vfork)
 
-	SINGLE_THREAD_P
+#ifdef SHARED
+	cmpq	$0, __libc_pthread_functions(%rip)
+#else
+	.weak	__pthread_fork
+	movl	$__pthread_fork, %eax
+	testq	%rax, %rax
+#endif
 	jne	HIDDEN_JUMPTARGET (__fork)
 
 	/* Pop the return PC value into RDI.  We need a register that
--- libc/linuxthreads/tst-popen2.c.jj	2003-02-06 14:01:27.000000000 -0500
+++ libc/linuxthreads/tst-popen2.c	2003-02-06 14:05:55.000000000 -0500
@@ -0,0 +1,41 @@
+#include <errno.h>
+#include <error.h>
+#include <pthread.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+static void *
+dummy (void *x)
+{
+  return NULL;
+}
+
+static char buf[sizeof "something\n"];
+
+static int
+do_test (void)
+{
+  FILE *f;
+  pthread_t p;
+  int err;
+
+  f = popen ("echo something", "r");
+  if (f == NULL)
+    error (EXIT_FAILURE, errno, "popen failed");
+  if (fgets (buf, sizeof (buf), f) == NULL)
+    error (EXIT_FAILURE, 0, "fgets failed");
+  if (strcmp (buf, "something\n"))
+    error (EXIT_FAILURE, 0, "read wrong data");
+  if (pclose (f))
+    error (EXIT_FAILURE, errno, "pclose returned non-zero");
+  if ((err = pthread_create (&p, NULL, dummy, NULL)))
+    error (EXIT_FAILURE, err, "pthread_create failed");
+  if ((err = pthread_join (p, NULL)))
+    error (EXIT_FAILURE, err, "pthread_join failed");
+  exit (0);
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
--- libc/linuxthreads/Makefile.jj	2003-01-09 15:24:34.000000000 -0500
+++ libc/linuxthreads/Makefile	2003-02-06 14:02:19.000000000 -0500
@@ -88,7 +88,7 @@ tests = ex1 ex2 ex3 ex4 ex5 ex6 ex7 ex8 
 	tststack $(tests-nodelete-$(have-z-nodelete)) ecmutex ex14 ex15 ex16 \
 	ex17 ex18 tst-cancel tst-context bug-sleep \
 	tst-cancel1 tst-cancel2 tst-cancel3 tst-cancel4 tst-cancel5 \
-	tst-cancel6 tst-popen
+	tst-cancel6 tst-popen tst-popen2
 test-srcs = tst-signal
 # These tests are linked with libc before libpthread
 tests-reverse += tst-cancel5


	Jakub

Re: [PATCH] Fix bug with popen/pthread_create

[Richard Henderson]

On Thu, Feb 06, 2003 at 09:08:31PM +0100, Jakub Jelinek wrote:
> +	.weak	__pthread_fork
> +	movl	$__pthread_fork, %eax
> +	testq	%rax, %rax

Not movq?


r~

Re: [PATCH] Fix bug with popen/pthread_create

[Ulrich Drepper]

Richard Henderson wrote:
> On Thu, Feb 06, 2003 at 09:08:31PM +0100, Jakub Jelinek wrote:
> 
>>+	.weak	__pthread_fork
>>+	movl	$__pthread_fork, %eax
>>+	testq	%rax, %rax
> 
> 
> Not movq?

Yes, movq.  I adjusted the patch.

-- 
--------------.                        ,-.            444 Castro Street
Ulrich Drepper \    ,-----------------'   \ Mountain View, CA 94041 USA
Red Hat         `--' drepper at redhat.com `---------------------------

Re: [PATCH] Fix bug with popen/pthread_create

[Andreas Jaeger]

Richard Henderson <rth@twiddle.net> writes:

> On Thu, Feb 06, 2003 at 09:08:31PM +0100, Jakub Jelinek wrote:
>> +	.weak	__pthread_fork
>> +	movl	$__pthread_fork, %eax
>> +	testq	%rax, %rax
>
> Not movq?

I agree - and would then use also %rax:

movq $__pthread_fork, %rax

Andreas
-- 
 Andreas Jaeger
  SuSE Labs aj@suse.de
   private aj@arthur.inka.de
    http://www.suse.de/~aj

Re: [PATCH] Fix bug with popen/pthread_create

[Jakub Jelinek]

On Fri, Feb 07, 2003 at 07:24:55AM +0100, Andreas Jaeger wrote:
> Richard Henderson <rth@twiddle.net> writes:
> 
> > On Thu, Feb 06, 2003 at 09:08:31PM +0100, Jakub Jelinek wrote:
> >> +	.weak	__pthread_fork
> >> +	movl	$__pthread_fork, %eax
> >> +	testq	%rax, %rax
> >
> > Not movq?
> 
> I agree - and would then use also %rax:
> 
> movq $__pthread_fork, %rax

Isn't whole libc.a compiled in -mcmodel=small anyway?

	Jakub

Re: [PATCH] Fix bug with popen/pthread_create

[Andreas Jaeger]

Jakub Jelinek <jakub@redhat.com> writes:

> On Fri, Feb 07, 2003 at 07:24:55AM +0100, Andreas Jaeger wrote:
>> Richard Henderson <rth@twiddle.net> writes:
>> 
>> > On Thu, Feb 06, 2003 at 09:08:31PM +0100, Jakub Jelinek wrote:
>> >> +	.weak	__pthread_fork
>> >> +	movl	$__pthread_fork, %eax
>> >> +	testq	%rax, %rax
>> >
>> > Not movq?
>> 
>> I agree - and would then use also %rax:
>> 
>> movq $__pthread_fork, %rax
>
> Isn't whole libc.a compiled in -mcmodel=small anyway?

It is - but we should not restrain us there - but does this fragment
run only in static libs?

Andreas
-- 
 Andreas Jaeger
  SuSE Labs aj@suse.de
   private aj@arthur.inka.de
    http://www.suse.de/~aj

Re: [PATCH] Fix bug with popen/pthread_create

[Andreas Schwab]

Jakub Jelinek <jakub@redhat.com> writes:

|> --- libc/linuxthreads/sysdeps/unix/sysv/linux/i386/vfork.S.jj	2003-01-09 15:27:26.000000000 -0500
|> +++ libc/linuxthreads/sysdeps/unix/sysv/linux/i386/vfork.S	2003-02-06 13:31:43.000000000 -0500
|> @@ -31,7 +31,19 @@ ENTRY (__vfork)
|>  
|>  #ifdef __NR_vfork
|>  
|> -	SINGLE_THREAD_P
|> +# ifdef SHARED
|> +#  if !defined HAVE_HIDDEN || !USE___THREAD
|> +	SETUP_PIC_REG (cx)
|> +#  else
|> +	call	__i686.get_pc_thunk.cx
|> +#  endif
|> +	addl	$_GLOBAL_OFFSET_TABLE_, %ecx
|> +	cmpl	$0, __libc_pthread_functions@GOTOFF(%ecx)
|> +# else
|> +	.weak	__pthread_fork
|> +	movl	$__pthread_fork, %eax
|> +	testl	%eax, %eax
|> +# endif
|>  	jne	HIDDEN_JUMPTARGET (__fork)
|>  
|>  	/* Pop the return PC value into ECX.  */

Shouldn't that be moved outside of #ifdef __NR_vfork, so that __fork is
used even if compiling with kernel headers that do not define vfork?  The
minimum for i386 is still 2.0.10.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Fix bug with popen/pthread_create

[Jakub Jelinek]

On Fri, Feb 07, 2003 at 11:11:05AM +0100, Andreas Schwab wrote:
> |> --- libc/linuxthreads/sysdeps/unix/sysv/linux/i386/vfork.S.jj	2003-01-09 15:27:26.000000000 -0500
> |> +++ libc/linuxthreads/sysdeps/unix/sysv/linux/i386/vfork.S	2003-02-06 13:31:43.000000000 -0500
> |> @@ -31,7 +31,19 @@ ENTRY (__vfork)
> |>  
> |>  #ifdef __NR_vfork
> |>  
> |> -	SINGLE_THREAD_P
> |> +# ifdef SHARED
> |> +#  if !defined HAVE_HIDDEN || !USE___THREAD
> |> +	SETUP_PIC_REG (cx)
> |> +#  else
> |> +	call	__i686.get_pc_thunk.cx
> |> +#  endif
> |> +	addl	$_GLOBAL_OFFSET_TABLE_, %ecx
> |> +	cmpl	$0, __libc_pthread_functions@GOTOFF(%ecx)
> |> +# else
> |> +	.weak	__pthread_fork
> |> +	movl	$__pthread_fork, %eax
> |> +	testl	%eax, %eax
> |> +# endif
> |>  	jne	HIDDEN_JUMPTARGET (__fork)
> |>  
> |>  	/* Pop the return PC value into ECX.  */
> 
> Shouldn't that be moved outside of #ifdef __NR_vfork, so that __fork is
> used even if compiling with kernel headers that do not define vfork?

Yes.

	Jakub

Re: [PATCH] Fix bug with popen/pthread_create

[Ulrich Drepper]

Andreas Schwab wrote:

> Shouldn't that be moved outside of #ifdef __NR_vfork, so that __fork is
> used even if compiling with kernel headers that do not define vfork?  The
> minimum for i386 is still 2.0.10.

No.  If somebody calls vfork instead of fork the atfork handlers are not
called.  For the code this means we make the syscall directly (if
__NR_vfork is not defined you better don't have __ASSUME_VFORK_SYSCALL
defined).  I added some preprocessor code to check for this condition.

-- 
--------------.                        ,-.            444 Castro Street
Ulrich Drepper \    ,-----------------'   \ Mountain View, CA 94041 USA
Red Hat         `--' drepper at redhat.com `---------------------------

Re: [PATCH] Fix bug with popen/pthread_create

[Andreas Schwab]

Ulrich Drepper <drepper@redhat.com> writes:

|> Andreas Schwab wrote:
|> 
|> > Shouldn't that be moved outside of #ifdef __NR_vfork, so that __fork is
|> > used even if compiling with kernel headers that do not define vfork?  The
|> > minimum for i386 is still 2.0.10.
|> 
|> No.  If somebody calls vfork instead of fork the atfork handlers are not
|> called.

But when glibc knows about the real vfork then __fork is called even when
running on a kernel without vfork.  That makes it quite inconsistent.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."