[patch] Fix CLONE_VM vs. TLS [Re: Is CLONE_VM really needed in gdbserver?]

[Jan Kratochvil <jan.kratochvil@redhat.com>]

On Wed, 27 Jan 2010 01:08:26 +0100, Daniel Jacobowitz wrote:
> On Tue, Jan 26, 2010 at 03:51:38PM -0800, H.J. Lu wrote:
> > Hi,
> > 
> > There is a race condition between gdbserver and ld.so on Linux/x86-64:
> > 
> > http://www.sourceware.org/bugzilla/show_bug.cgi?id=11214
> > 
> > Is CLONE_VM really needed? In general, CLONE_VM is a very bad
> > idea if there is any symbol lookup in both parent and child processes.
> 
> It is necessary because gdbserver supports uClinux.  However, on Linux
> we might be able to get away with fork (see linux_tracefork_child in
> gdb/linux-nat.c).

Coded it as suggested.

No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu and on
x86_64-fedora12-linux-gnu via gdbserver.

Verified linux_supports_tracefork_flag gets still set with the patch.

Verified unsetting HAVE_FORK for gdb/gdbserver/ still works the same.

gdb/ already tests HAVE_FORK in config.in but it is brought in by other
macros, therefore rather added an explicit configure.ac test for it.

Have not found an easy enough uClinux disk image of some arch for qemu-*.


Regards,
Jan


gdb/
2010-01-27  Jan Kratochvil  <jan.kratochvil@redhat.com>

	PR libc/11214:
	* configure.ac (AC_CHECK_FUNCS): Check for fork.
	* configure: Regenerate.

gdb/gdbserver/
2010-01-27  Jan Kratochvil  <jan.kratochvil@redhat.com>

	PR libc/11214:
	* configure.ac (AC_CHECK_FUNCS): Check for fork.
	* configure: Regenerate.
	* config.in: Regenerate.
	* linux-low.c (linux_tracefork_child) [HAVE_FORK]: New.
	(linux_test_for_tracefork): Move `stack' in [!HAVE_FORK].
	(linux_test_for_tracefork) [HAVE_FORK]: New.

gdb/testsuite/
2010-01-27  Jan Kratochvil  <jan.kratochvil@redhat.com>

	PR libc/11214:
	* gdb.threads/current-lwp-dead.c (fn, main): Move CLONE_VM in [UCLINUX].

--- a/gdb/configure
+++ b/gdb/configure
@@ -11234,7 +11234,7 @@ for ac_func in canonicalize_file_name realpath getrusage getuid \
 		getgid pipe poll pread64 sbrk setpgid setpgrp setsid \
 		sigaction sigprocmask sigsetmask socketpair syscall \
 		ttrace wborder setlocale iconvlist libiconvlist btowc \
-		setrlimit getrlimit
+		setrlimit getrlimit fork
 do :
   as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
 ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -802,7 +802,7 @@ AC_CHECK_FUNCS([canonicalize_file_name realpath getrusage getuid \
 		getgid pipe poll pread64 sbrk setpgid setpgrp setsid \
 		sigaction sigprocmask sigsetmask socketpair syscall \
 		ttrace wborder setlocale iconvlist libiconvlist btowc \
-		setrlimit getrlimit])
+		setrlimit getrlimit fork])
 AM_LANGINFO_CODESET
 
 # Check the return and argument types of ptrace.  No canned test for
--- a/gdb/gdbserver/config.in
+++ b/gdb/gdbserver/config.in
@@ -30,6 +30,9 @@
 /* Define to 1 if you have the <fcntl.h> header file. */
 #undef HAVE_FCNTL_H
 
+/* Define to 1 if you have the `fork' function. */
+#undef HAVE_FORK
+
 /* Define to 1 if you have the <inttypes.h> header file. */
 #undef HAVE_INTTYPES_H
 
--- a/gdb/gdbserver/configure
+++ b/gdb/gdbserver/configure
@@ -3783,7 +3783,7 @@ fi
 
 done
 
-for ac_func in pread pwrite pread64
+for ac_func in pread pwrite pread64 fork
 do :
   as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
 ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
@@ -4153,6 +4153,8 @@ else
 /* end confdefs.h.  */
 
 #define _SYSCALL32
+/* Needed for new procfs interface on sparc-solaris.  */
+#define _STRUCTURED_PROC 1
 #include <sys/procfs.h>
 int
 main ()
@@ -4188,6 +4190,8 @@ else
 /* end confdefs.h.  */
 
 #define _SYSCALL32
+/* Needed for new procfs interface on sparc-solaris.  */
+#define _STRUCTURED_PROC 1
 #include <sys/procfs.h>
 int
 main ()
@@ -4223,6 +4227,8 @@ else
 /* end confdefs.h.  */
 
 #define _SYSCALL32
+/* Needed for new procfs interface on sparc-solaris.  */
+#define _STRUCTURED_PROC 1
 #include <sys/procfs.h>
 int
 main ()
@@ -4258,6 +4264,8 @@ else
 /* end confdefs.h.  */
 
 #define _SYSCALL32
+/* Needed for new procfs interface on sparc-solaris.  */
+#define _STRUCTURED_PROC 1
 #include <sys/procfs.h>
 int
 main ()
--- a/gdb/gdbserver/configure.ac
+++ b/gdb/gdbserver/configure.ac
@@ -42,7 +42,7 @@ AC_CHECK_HEADERS(sgtty.h termio.h termios.h sys/reg.h string.h dnl
 		 errno.h fcntl.h signal.h sys/file.h malloc.h dnl
 		 sys/ioctl.h netinet/in.h sys/socket.h netdb.h dnl
 		 netinet/tcp.h arpa/inet.h sys/wait.h)
-AC_CHECK_FUNCS(pread pwrite pread64)
+AC_CHECK_FUNCS(pread pwrite pread64 fork)
 AC_REPLACE_FUNCS(memmem)
 
 dnl dladdr is glibc-specific.  It is used by thread-db.c but only for
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -2586,6 +2586,14 @@ linux_tracefork_child (void *arg)
 {
   ptrace (PTRACE_TRACEME, 0, 0, 0);
   kill (getpid (), SIGSTOP);
+
+#ifdef HAVE_FORK
+
+  if (fork () == 0)
+    linux_tracefork_grandchild (NULL);
+
+#else /* !HAVE_FORK */
+
 #ifdef __ia64__
   __clone2 (linux_tracefork_grandchild, arg, STACK_SIZE,
 	    CLONE_VM | SIGCHLD, NULL);
@@ -2593,6 +2601,9 @@ linux_tracefork_child (void *arg)
   clone (linux_tracefork_grandchild, arg + STACK_SIZE,
 	 CLONE_VM | SIGCHLD, NULL);
 #endif
+
+#endif /* !HAVE_FORK */
+
   _exit (0);
 }
 
@@ -2605,18 +2616,31 @@ linux_test_for_tracefork (void)
 {
   int child_pid, ret, status;
   long second_pid;
+#ifndef HAVE_FORK
   char *stack = xmalloc (STACK_SIZE * 4);
+#endif /* !HAVE_FORK */
 
   linux_supports_tracefork_flag = 0;
 
+#ifdef HAVE_FORK
+
+  child_pid = fork ();
+  if (child_pid == 0)
+    linux_tracefork_child (NULL);
+
+#else /* !HAVE_FORK */
+
   /* Use CLONE_VM instead of fork, to support uClinux (no MMU).  */
 #ifdef __ia64__
   child_pid = __clone2 (linux_tracefork_child, stack, STACK_SIZE,
 			CLONE_VM | SIGCHLD, stack + STACK_SIZE * 2);
-#else
+#else /* !__ia64__ */
   child_pid = clone (linux_tracefork_child, stack + STACK_SIZE,
 		     CLONE_VM | SIGCHLD, stack + STACK_SIZE * 2);
-#endif
+#endif /* !__ia64__ */
+
+#endif /* !HAVE_FORK */
+
   if (child_pid == -1)
     perror_with_name ("clone");
 
@@ -2685,7 +2709,9 @@ linux_test_for_tracefork (void)
     }
   while (WIFSTOPPED (status));
 
+#ifndef HAVE_FORK
   free (stack);
+#endif /* !HAVE_FORK */
 }
 
 
--- a/gdb/testsuite/gdb.threads/current-lwp-dead.c
+++ b/gdb/testsuite/gdb.threads/current-lwp-dead.c
@@ -51,8 +51,11 @@ fn (void *unused)
   stack = malloc (STACK_SIZE);
   assert (stack != NULL);
 
-  new_pid = clone (fn_return, stack + STACK_SIZE, CLONE_FILES | CLONE_VM, NULL,
-		   NULL, NULL, NULL);
+  new_pid = clone (fn_return, stack + STACK_SIZE, CLONE_FILES
+#ifdef UCLINUX
+		   | CLONE_VM
+#endif /* UCLINUX */
+		   , NULL, NULL, NULL, NULL);
   assert (new_pid > 0);
 
   return 0;
@@ -67,8 +70,11 @@ main (int argc, char **argv)
   stack = malloc (STACK_SIZE);
   assert (stack != NULL);
 
-  new_pid = clone (fn, stack + STACK_SIZE, CLONE_FILES | CLONE_VM, NULL, NULL,
-		   NULL, NULL);
+  new_pid = clone (fn, stack + STACK_SIZE, CLONE_FILES
+#ifdef UCLINUX
+		   | CLONE_VM
+#endif /* UCLINUX */
+		   , NULL, NULL, NULL, NULL);
   assert (new_pid > 0);
 
   return 0;