From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28610 invoked by alias); 27 Jan 2010 22:12:49 -0000 Received: (qmail 28584 invoked by uid 22791); 27 Jan 2010 22:12:47 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 27 Jan 2010 22:12:41 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o0RMCeNC016189 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 27 Jan 2010 17:12:40 -0500 Received: from host0.dyn.jankratochvil.net (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o0RMCbo1013854 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 27 Jan 2010 17:12:39 -0500 Received: from host0.dyn.jankratochvil.net (localhost [127.0.0.1]) by host0.dyn.jankratochvil.net (8.14.3/8.14.3) with ESMTP id o0RMCb6J004913; Wed, 27 Jan 2010 23:12:37 +0100 Received: (from jkratoch@localhost) by host0.dyn.jankratochvil.net (8.14.3/8.14.3/Submit) id o0RMCaLX004910; Wed, 27 Jan 2010 23:12:36 +0100 Date: Wed, 27 Jan 2010 22:12:00 -0000 From: Jan Kratochvil To: gdb-patches@sourceware.org Cc: "H.J. Lu" , GDB Subject: [patch] Fix CLONE_VM vs. TLS [Re: Is CLONE_VM really needed in gdbserver?] Message-ID: <20100127221236.GA4746@host0.dyn.jankratochvil.net> References: <6dc9ffc81001261551j6221db6v88e96713d6dd9497@mail.gmail.com> <20100127000821.GA29862@caradoc.them.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100127000821.GA29862@caradoc.them.org> User-Agent: Mutt/1.5.20 (2009-08-17) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-01/txt/msg00599.txt.bz2 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 PR libc/11214: * configure.ac (AC_CHECK_FUNCS): Check for fork. * configure: Regenerate. gdb/gdbserver/ 2010-01-27 Jan Kratochvil 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 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 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 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 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 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 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 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;