From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32159 invoked by alias); 15 May 2005 17:03:07 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 31453 invoked from network); 15 May 2005 17:02:53 -0000 Received: from unknown (HELO nevyn.them.org) (66.93.172.17) by sourceware.org with SMTP; 15 May 2005 17:02:53 -0000 Received: from drow by nevyn.them.org with local (Exim 4.50) id 1DXMVt-0000Ad-Ly; Sun, 15 May 2005 13:02:49 -0400 Date: Sun, 15 May 2005 17:12:00 -0000 From: Daniel Jacobowitz To: Ulrich Weigand Cc: gdb-patches@sources.redhat.com Subject: Re: [RFA] Fix internal error in wait_lwp (interrupted system call) Message-ID: <20050515170248.GA11855@nevyn.them.org> Mail-Followup-To: Ulrich Weigand , gdb-patches@sources.redhat.com References: <20050512211439.GA14174@nevyn.them.org> <200505131236.j4DCatE5014770@53v30g15.boeblingen.de.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200505131236.j4DCatE5014770@53v30g15.boeblingen.de.ibm.com> User-Agent: Mutt/1.5.8i X-SW-Source: 2005-05/txt/msg00372.txt.bz2 On Fri, May 13, 2005 at 02:36:55PM +0200, Ulrich Weigand wrote: > As verifying correct EINTR handling everywhere appears to be a major > effort, and the simple SA_RESTART patch both fixes a serious problem, > and appears to be correct in any case, I'd argue for applying that > patch now, and maybe later on adding EINTR handling as required. I'll do both. I didn't audit every single restartable syscall, just the other uses of waitpid in this file; that will do for now. I know that most of these are not robust against EINTR and should not have to be. There was also another call to sigaction; only relevant on LinuxThreads with older kernels. Committed the below after testing on i686-pc-linux-gnu. Thank you for discovering this problem. -- Daniel Jacobowitz CodeSourcery, LLC 2005-05-15 Daniel Jacobowitz * linux-nat.c (child_follow_fork, linux_handle_extended_wait) (lin_lwp_attach_lwp, linux_nat_attach, wait_lwp, child_wait) (linux_nat_wait, kill_wait_callback): Use my_waitpid. (_initialize_linux_nat, lin_thread_get_thread_signals): Use SA_RESTART. Index: linux-nat.c =================================================================== RCS file: /cvs/src/src/gdb/linux-nat.c,v retrieving revision 1.27 diff -u -p -r1.27 linux-nat.c --- linux-nat.c 6 Mar 2005 16:42:20 -0000 1.27 +++ linux-nat.c 15 May 2005 14:11:22 -0000 @@ -377,7 +377,7 @@ child_follow_fork (int follow_child) int status; ptrace (PTRACE_CONT, parent_pid, 0, 0); - waitpid (parent_pid, &status, __WALL); + my_waitpid (parent_pid, &status, __WALL); if ((status >> 16) != PTRACE_EVENT_VFORK_DONE) warning (_("Unexpected waitpid result %06x when waiting for " "vfork-done"), status); @@ -494,10 +494,8 @@ linux_handle_extended_wait (int pid, int { /* The new child has a pending SIGSTOP. We can't affect it until it hits the SIGSTOP, but we're already attached. */ - do { - ret = waitpid (new_pid, &status, - (event == PTRACE_EVENT_CLONE) ? __WCLONE : 0); - } while (ret == -1 && errno == EINTR); + ret = my_waitpid (new_pid, &status, + (event == PTRACE_EVENT_CLONE) ? __WCLONE : 0); if (ret == -1) perror_with_name (_("waiting for new child")); else if (ret != new_pid) @@ -868,11 +866,11 @@ lin_lwp_attach_lwp (ptid_t ptid, int ver "LLAL: PTRACE_ATTACH %s, 0, 0 (OK)\n", target_pid_to_str (ptid)); - pid = waitpid (GET_LWP (ptid), &status, 0); + pid = my_waitpid (GET_LWP (ptid), &status, 0); if (pid == -1 && errno == ECHILD) { /* Try again with __WCLONE to check cloned processes. */ - pid = waitpid (GET_LWP (ptid), &status, __WCLONE); + pid = my_waitpid (GET_LWP (ptid), &status, __WCLONE); lp->cloned = 1; } @@ -920,13 +918,13 @@ linux_nat_attach (char *args, int from_t /* Make sure the initial process is stopped. The user-level threads layer might want to poke around in the inferior, and that won't work if things haven't stabilized yet. */ - pid = waitpid (GET_PID (inferior_ptid), &status, 0); + pid = my_waitpid (GET_PID (inferior_ptid), &status, 0); if (pid == -1 && errno == ECHILD) { warning (_("%s is a cloned process"), target_pid_to_str (inferior_ptid)); /* Try again with __WCLONE to check cloned processes. */ - pid = waitpid (GET_PID (inferior_ptid), &status, __WCLONE); + pid = my_waitpid (GET_PID (inferior_ptid), &status, __WCLONE); lp->cloned = 1; } @@ -1191,10 +1189,10 @@ wait_lwp (struct lwp_info *lp) gdb_assert (!lp->stopped); gdb_assert (lp->status == 0); - pid = waitpid (GET_LWP (lp->ptid), &status, 0); + pid = my_waitpid (GET_LWP (lp->ptid), &status, 0); if (pid == -1 && errno == ECHILD) { - pid = waitpid (GET_LWP (lp->ptid), &status, __WCLONE); + pid = my_waitpid (GET_LWP (lp->ptid), &status, __WCLONE); if (pid == -1 && errno == ECHILD) { /* The thread has previously exited. We need to delete it @@ -1706,10 +1704,10 @@ child_wait (ptid_t ptid, struct target_w attached process. */ set_sigio_trap (); - pid = waitpid (GET_PID (ptid), &status, 0); + pid = my_waitpid (GET_PID (ptid), &status, 0); if (pid == -1 && errno == ECHILD) /* Try again with __WCLONE to check cloned processes. */ - pid = waitpid (GET_PID (ptid), &status, __WCLONE); + pid = my_waitpid (GET_PID (ptid), &status, __WCLONE); if (debug_linux_nat) { @@ -1920,7 +1918,7 @@ retry: { pid_t lwpid; - lwpid = waitpid (pid, &status, options); + lwpid = my_waitpid (pid, &status, options); if (lwpid > 0) { gdb_assert (pid == -1 || lwpid == pid); @@ -2264,7 +2262,7 @@ kill_wait_callback (struct lwp_info *lp, { do { - pid = waitpid (GET_LWP (lp->ptid), NULL, __WCLONE); + pid = my_waitpid (GET_LWP (lp->ptid), NULL, __WCLONE); if (pid != (pid_t) -1 && debug_linux_nat) { fprintf_unfiltered (gdb_stdlog, @@ -2279,7 +2277,7 @@ kill_wait_callback (struct lwp_info *lp, do { - pid = waitpid (GET_LWP (lp->ptid), NULL, 0); + pid = my_waitpid (GET_LWP (lp->ptid), NULL, 0); if (pid != (pid_t) -1 && debug_linux_nat) { fprintf_unfiltered (gdb_stdlog, @@ -3095,7 +3093,7 @@ Specify any of the following keywords fo action.sa_handler = sigchld_handler; sigemptyset (&action.sa_mask); - action.sa_flags = 0; + action.sa_flags = SA_RESTART; sigaction (SIGCHLD, &action, NULL); /* Make sure we don't block SIGCHLD during a sigsuspend. */ @@ -3168,7 +3166,7 @@ lin_thread_get_thread_signals (sigset_t action.sa_handler = sigchld_handler; sigemptyset (&action.sa_mask); - action.sa_flags = 0; + action.sa_flags = SA_RESTART; sigaction (cancel, &action, NULL); /* We block the "cancel" signal throughout this code ... */