From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9747 invoked by alias); 24 Mar 2008 17:49:27 -0000 Received: (qmail 9736 invoked by uid 22791); 24 Mar 2008 17:49:25 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 24 Mar 2008 17:49:07 +0000 Received: (qmail 21105 invoked from network); 24 Mar 2008 17:49:03 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 24 Mar 2008 17:49:03 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: linux native async fix Date: Mon, 24 Mar 2008 17:49:00 -0000 User-Agent: KMail/1.9.6 (enterprise 0.20070907.709405) MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_Tm+5HW0SRkPSRSI" Message-Id: <200803241749.07975.pedro@codesourcery.com> 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: 2008-03/txt/msg00359.txt.bz2 --Boundary-00=_Tm+5HW0SRkPSRSI Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 958 The current code caches pending events in lp->status in some situations. This is useful so linux_nat_wait notices an event we have already waited for, it is handled immediatelly without waitpid'ing. It works nicelly in non-async mode, because target_wait is always called after proceeding the target. This pending event caching bypasses the event loop, so we can have the case where an event in one thread goes unnoticed until some other event happens. There's code currently that tried to detected this in linux_nat_resume, and puts an special wake event token in the event pipe. This logic is racy and can brake in several corner case situations, especially with non-stop mode. Since in async mode we already have a place to cache events, so we don't really need another. With this patch, in async mode, no event is left pending in lp->status. Tested in sync mode, and async mode all-stop, non-stop in x86_64-unknown-linux-gnu. OK? -- Pedro Alves --Boundary-00=_Tm+5HW0SRkPSRSI Content-Type: text/x-diff; charset="us-ascii"; name="linux_async_fixes.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="linux_async_fixes.diff" Content-length: 10169 2008-03-24 Pedro Alves * linux-nat.c (drain_queued_events): Fix comment typo. (linux_nat_attach): In async mode, don't rely on storing a pending status. Instead place the wait status on the pipe. (linux_nat_resume): Remove unreacheable shortcut code in async mode. (stop_wait_callback): In async mode, don't store pending status. Instead, cancel breakpoints or resend the signal appropriatelly. (cancel_breakpoint): New, refactored from cancel_breakpoints_callback. (cancel_breakpoints_callback): Call cancel_breakpoint. (pipe_to_local_event_queue): Remove special token processing. (linux_nat_wait): Issue an internal error if a pending status is found in async mode. --- gdb/linux-nat.c | 162 +++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 103 insertions(+), 59 deletions(-) Index: src/gdb/linux-nat.c =================================================================== --- src.orig/gdb/linux-nat.c 2008-03-24 17:28:16.000000000 +0000 +++ src/gdb/linux-nat.c 2008-03-24 17:28:20.000000000 +0000 @@ -288,7 +288,7 @@ push_waitpid (int pid, int status, int o waitpid_queue = new_event; } -/* Drain all queued event of PID. If PID is -1, the effect is of +/* Drain all queued events of PID. If PID is -1, the effect is of draining all events. */ static void drain_queued_events (int pid) @@ -799,6 +799,8 @@ static struct sigaction async_sigchld_ac static int stop_wait_callback (struct lwp_info *lp, void *data); static int linux_nat_thread_alive (ptid_t ptid); static char *linux_child_pid_to_exec_file (int pid); +static int cancel_breakpoint (struct lwp_info *lp); + /* Convert wait status STATUS to a string. Used for printing debug messages only. */ @@ -1134,6 +1136,7 @@ linux_nat_attach (char *args, int from_t pid_t pid; int status; int cloned = 0; + int options = 0; /* FIXME: We should probably accept a list of process id's, and attach all of them. */ @@ -1151,13 +1154,14 @@ 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 = my_waitpid (GET_PID (inferior_ptid), &status, 0); + pid = my_waitpid (GET_PID (inferior_ptid), &status, options); 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 = my_waitpid (GET_PID (inferior_ptid), &status, __WCLONE); + options = __WCLONE; + pid = my_waitpid (GET_PID (inferior_ptid), &status, options); cloned = 1; } @@ -1170,17 +1174,22 @@ linux_nat_attach (char *args, int from_t lp->cloned = cloned; lp->stopped = 1; - - /* Fake the SIGSTOP that core GDB expects. */ - lp->status = W_STOPCODE (SIGSTOP); lp->resumed = 1; - if (debug_linux_nat) - fprintf_unfiltered (gdb_stdlog, - "LNA: waitpid %ld, faking SIGSTOP\n", (long) pid); - if (target_can_async_p ()) + + if (!target_can_async_p ()) { - /* Wake event loop with special token, to get to WFI. */ - linux_nat_event_pipe_push (-1, -1, -1); + /* Fake the SIGSTOP that core GDB expects. */ + lp->status = W_STOPCODE (SIGSTOP); + if (debug_linux_nat) + fprintf_unfiltered (gdb_stdlog, + "LNA: waitpid %ld, faking SIGSTOP\n", (long) pid); + } + else + { + /* We already waited for this LWP, so put the wait result on the + pipe. The event loop will wake up and gets us to handling + this event. */ + linux_nat_event_pipe_push (pid, status, options); /* Register in the event loop. */ target_async (inferior_event_handler, 0); } @@ -1358,6 +1367,10 @@ linux_nat_resume (ptid_t ptid, int step, other threads. This bit of code needs to be synchronized with linux_nat_wait. */ + /* In async mode, we never have pending wait status. */ + if (target_can_async_p () && lp->status) + internal_error (__FILE__, __LINE__, "Pending status in async mode"); + if (lp->status && WIFSTOPPED (lp->status)) { int saved_signo = target_signal_from_host (WSTOPSIG (lp->status)); @@ -1390,13 +1403,6 @@ linux_nat_resume (ptid_t ptid, int step, "LLR: Short circuiting for status 0x%x\n", lp->status); - if (target_can_async_p ()) - { - /* Wake event loop with special token, to get to WFI. */ - linux_nat_event_pipe_push (-1, -1, -1); - - target_async (inferior_event_handler, 0); - } return; } @@ -1752,22 +1758,44 @@ stop_wait_callback (struct lwp_info *lp, "SWC: Candidate SIGTRAP event in %s\n", target_pid_to_str (lp->ptid)); } - /* Hold the SIGTRAP for handling by linux_nat_wait. */ + /* Hold this event/waitstatus while we check to see if + there are any more (we still want to get that SIGSTOP). */ stop_wait_callback (lp, data); - /* If there's another event, throw it back into the queue. */ - if (lp->status) + + if (target_can_async_p ()) { - if (debug_linux_nat) + /* Don't leave a pending wait status in async mode. + Retrigger the breakpoint. */ + if (!cancel_breakpoint (lp)) { - fprintf_unfiltered (gdb_stdlog, - "SWC: kill %s, %s\n", - target_pid_to_str (lp->ptid), - status_to_str ((int) status)); + /* There was no gdb breakpoint set at pc. Put + the event back in the queue. */ + if (debug_linux_nat) + fprintf_unfiltered (gdb_stdlog, + "SWC: kill %s, %s\n", + target_pid_to_str (lp->ptid), + status_to_str ((int) status)); + kill_lwp (GET_LWP (lp->ptid), WSTOPSIG (status)); } - kill_lwp (GET_LWP (lp->ptid), WSTOPSIG (lp->status)); } - /* Save the sigtrap event. */ - lp->status = status; + else + { + /* Hold the SIGTRAP for handling by + linux_nat_wait. */ + /* If there's another event, throw it back into the + queue. */ + if (lp->status) + { + if (debug_linux_nat) + fprintf_unfiltered (gdb_stdlog, + "SWC: kill %s, %s\n", + target_pid_to_str (lp->ptid), + status_to_str ((int) status)); + kill_lwp (GET_LWP (lp->ptid), WSTOPSIG (lp->status)); + } + /* Save the sigtrap event. */ + lp->status = status; + } return 0; } else @@ -1794,12 +1822,11 @@ stop_wait_callback (struct lwp_info *lp, /* Hold this event/waitstatus while we check to see if there are any more (we still want to get that SIGSTOP). */ stop_wait_callback (lp, data); - /* If the lp->status field is still empty, use it to hold - this event. If not, then this event must be returned - to the event queue of the LWP. */ - if (lp->status == 0) - lp->status = status; - else + + /* If the lp->status field is still empty, use it to + hold this event. If not, then this event must be + returned to the event queue of the LWP. */ + if (lp->status || target_can_async_p ()) { if (debug_linux_nat) { @@ -1810,6 +1837,8 @@ stop_wait_callback (struct lwp_info *lp, } kill_lwp (GET_LWP (lp->ptid), WSTOPSIG (status)); } + else + lp->status = status; return 0; } } @@ -1980,6 +2009,37 @@ select_event_lwp_callback (struct lwp_in } static int +cancel_breakpoint (struct lwp_info *lp) +{ + /* Arrange for a breakpoint to be hit again later. We don't keep + the SIGTRAP status and don't forward the SIGTRAP signal to the + LWP. We will handle the current event, eventually we will resume + this LWP, and this breakpoint will trap again. + + If we do not do this, then we run the risk that the user will + delete or disable the breakpoint, but the LWP will have already + tripped on it. */ + + if (breakpoint_inserted_here_p (read_pc_pid (lp->ptid) - + gdbarch_decr_pc_after_break + (current_gdbarch))) + { + if (debug_linux_nat) + fprintf_unfiltered (gdb_stdlog, + "CB: Push back breakpoint for %s\n", + target_pid_to_str (lp->ptid)); + + /* Back up the PC if necessary. */ + if (gdbarch_decr_pc_after_break (current_gdbarch)) + write_pc_pid (read_pc_pid (lp->ptid) - gdbarch_decr_pc_after_break + (current_gdbarch), + lp->ptid); + return 1; + } + return 0; +} + +static int cancel_breakpoints_callback (struct lwp_info *lp, void *data) { struct lwp_info *event_lp = data; @@ -2001,24 +2061,9 @@ cancel_breakpoints_callback (struct lwp_ if (lp->status != 0 && WIFSTOPPED (lp->status) && WSTOPSIG (lp->status) == SIGTRAP - && breakpoint_inserted_here_p (read_pc_pid (lp->ptid) - - gdbarch_decr_pc_after_break - (current_gdbarch))) - { - if (debug_linux_nat) - fprintf_unfiltered (gdb_stdlog, - "CBC: Push back breakpoint for %s\n", - target_pid_to_str (lp->ptid)); - - /* Back up the PC if necessary. */ - if (gdbarch_decr_pc_after_break (current_gdbarch)) - write_pc_pid (read_pc_pid (lp->ptid) - gdbarch_decr_pc_after_break - (current_gdbarch), - lp->ptid); - - /* Throw away the SIGTRAP. */ - lp->status = 0; - } + && cancel_breakpoint (lp)) + /* Throw away the SIGTRAP. */ + lp->status = 0; return 0; } @@ -2288,12 +2333,7 @@ pipe_to_local_event_queue (void) while (linux_nat_num_queued_events) { int lwpid, status, options; - lwpid = linux_nat_event_pipe_pop (&status, &options); - if (lwpid == -1 && status == -1 && options == -1) - /* Special wake up event loop token. */ - continue; - gdb_assert (lwpid > 0); push_waitpid (lwpid, status, options); } @@ -2367,6 +2407,10 @@ retry: lp = iterate_over_lwps (status_callback, NULL); if (lp) { + if (target_can_async_p ()) + internal_error (__FILE__, __LINE__, + "Found an LWP with a pending status in async mode."); + status = lp->status; lp->status = 0; --Boundary-00=_Tm+5HW0SRkPSRSI--