* [PATCH] Fix PR 12702 - gdb can hang waiting for thread group leader (gdbserver)
@ 2013-07-20 10:00 Hui Zhu
2013-07-24 18:25 ` Pedro Alves
0 siblings, 1 reply; 12+ messages in thread
From: Hui Zhu @ 2013-07-20 10:00 UTC (permalink / raw)
To: gdb-patches ml
Hi,
I got a issue with leader-exit.exp will hang with target_board=native-gdbserver.
I found the reason is gdbserver still have this issue. So I post a
patch for gdbserver.
Thanks,
Hui
2013-07-20 Hui Zhu <hui@codesourcery.com>
PR gdb/12702
* linux-low.c (my_waitpid): Check if pid is exiting.
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -423,6 +423,15 @@ my_waitpid (int pid, int *status, int fl
if (wnohang)
break;
+ /* If just wait PID and it is exiting, sigsuspend will
+ hang. So check before call it. */
+ if (pid > 0 && linux_proc_pid_is_zombie (pid))
+ {
+ *status = 0;
+ ret = pid;
+ break;
+ }
+
if (debug_threads)
fprintf (stderr, "blocking\n");
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] Fix PR 12702 - gdb can hang waiting for thread group leader (gdbserver) 2013-07-20 10:00 [PATCH] Fix PR 12702 - gdb can hang waiting for thread group leader (gdbserver) Hui Zhu @ 2013-07-24 18:25 ` Pedro Alves 2013-11-19 6:19 ` Hui Zhu 0 siblings, 1 reply; 12+ messages in thread From: Pedro Alves @ 2013-07-24 18:25 UTC (permalink / raw) To: Hui Zhu; +Cc: gdb-patches ml On 07/20/2013 10:59 AM, Hui Zhu wrote: > Hi, > > I got a issue with leader-exit.exp will hang with target_board=native-gdbserver. > I found the reason is gdbserver still have this issue. So I post a > patch for gdbserver. leader-exit.exp is part of a series of tests for a set of related problems. See: http://www.sourceware.org/ml/gdb-patches/2011-10/msg00704.html Several are currently masked because gdbserver doesn't support fork/exec yet. We should fix this by implementing TARGET_WAITKIND_NO_RESUMED on gdbserver, like the patch above did for native. -- Pedro Alves ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Fix PR 12702 - gdb can hang waiting for thread group leader (gdbserver) 2013-07-24 18:25 ` Pedro Alves @ 2013-11-19 6:19 ` Hui Zhu 2014-01-23 14:51 ` Pedro Alves 0 siblings, 1 reply; 12+ messages in thread From: Hui Zhu @ 2013-11-19 6:19 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches ml, Gustavo, Luis [-- Attachment #1: Type: text/plain, Size: 1648 bytes --] On Thu, Jul 25, 2013 at 2:25 AM, Pedro Alves <palves@redhat.com> wrote: > On 07/20/2013 10:59 AM, Hui Zhu wrote: >> Hi, >> >> I got a issue with leader-exit.exp will hang with target_board=native-gdbserver. >> I found the reason is gdbserver still have this issue. So I post a >> patch for gdbserver. > > leader-exit.exp is part of a series of tests for a set of related > problems. See: > > http://www.sourceware.org/ml/gdb-patches/2011-10/msg00704.html > > Several are currently masked because gdbserver doesn't support > fork/exec yet. > > We should fix this by implementing TARGET_WAITKIND_NO_RESUMED > on gdbserver, like the patch above did for native. > > -- > Pedro Alves > Hi Pedro, According to your comments, I make a new patch to add TARGET_WAITKIND_NO_RESUMED to gdbserver. It can pass the leader-exit.exp and pass the regression test. Please help me review it. Thanks, Hui 2013-11-19 Hui Zhu <hui@codesourcery.com> * common/ptid.c (ptid_match): New. * common/ptid.h (ptid_match): New extern. * inferior.h (ptid_match): Removed. * infrun.c (ptid_match): Removed. 2013-11-19 Hui Zhu <hui@codesourcery.com> * linux-low.c (sigchld_mask): New. (handle_extended_wait): Add debug output. (num_lwps): New. (check_zombie_leaders_callback): New. (check_zombie_leaders): New. (not_suspended_callback): New. (check_pending_stop): New. (linux_wait_for_lwp): Call my_waitpid with WNOHANG. (linux_wait_for_event): Change the return check for linux_wait_for_lwp. (wait_lwp): New. (wait_for_sigstop): Call wait_lwp. * linux-low.h (lwp_info): Add pending_stop. * server.c (handle_target_event): Add check for TARGET_WAITKIND_NO_RESUMED. [-- Attachment #2: 2-fix-12702.txt --] [-- Type: text/plain, Size: 17473 bytes --] --- a/gdb/common/ptid.c +++ b/gdb/common/ptid.c @@ -114,3 +114,17 @@ ptid_tid_p (ptid_t ptid) return (ptid_get_tid (ptid) != 0); } + +int +ptid_match (ptid_t ptid, ptid_t filter) +{ + if (ptid_equal (filter, minus_one_ptid)) + return 1; + if (ptid_is_pid (filter) + && ptid_get_pid (ptid) == ptid_get_pid (filter)) + return 1; + else if (ptid_equal (ptid, filter)) + return 1; + + return 0; +} --- a/gdb/common/ptid.h +++ b/gdb/common/ptid.h @@ -78,4 +78,14 @@ int ptid_lwp_p (ptid_t ptid); /* Return true if PTID's tid member is non-zero. */ int ptid_tid_p (ptid_t ptid); +/* Returns true if PTID matches filter FILTER. FILTER can be the wild + card MINUS_ONE_PTID (all ptid match it); can be a ptid representing + a process (ptid_is_pid returns true), in which case, all lwps and + threads of that given process match, lwps and threads of other + processes do not; or, it can represent a specific thread, in which + case, only that thread will match true. PTID must represent a + specific LWP or THREAD, it can never be a wild card. */ + +extern int ptid_match (ptid_t ptid, ptid_t filter); + #endif --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -220,6 +220,9 @@ int using_threads = 1; jump pads). */ static int stabilizing_threads; +/* Signals to block SIGCHLD. */ +static sigset_t sigchld_mask; + static void linux_resume_one_lwp (struct lwp_info *lwp, int step, int signal, siginfo_t *info); static void linux_resume (struct thread_resume *resume_info, size_t n); @@ -407,6 +410,11 @@ handle_extended_wait (struct lwp_info *e warning ("wait returned unexpected status 0x%x", status); } + if (debug_threads) + fprintf (stderr, "HEW: Got clone event " + "from LWP %ld, new child is LWP %ld\n", + lwpid_of (event_child), new_pid); + ptid = ptid_build (pid_of (event_child), new_pid, 0); new_lwp = (struct lwp_info *) add_lwp (ptid); add_thread (ptid, new_lwp); @@ -1268,30 +1276,148 @@ find_lwp_pid (ptid_t ptid) return (struct lwp_info*) find_inferior (&all_lwps, same_lwp, &ptid); } +/* Return the number of known LWPs in the tgid given by PID. */ + +static int +num_lwps (int pid) +{ + int count = 0; + struct inferior_list_entry *inf = all_lwps.head; + + while (inf != NULL) + { + struct inferior_list_entry *next; + + next = inf->next; + if (ptid_get_pid(inf->id) == pid) + count++; + inf = next; + } + + return count; +} + +static void +check_zombie_leaders_callback (struct inferior_list_entry *inf) +{ + pid_t leader_pid = ptid_get_pid (inf->id); + struct lwp_info *leader_lp; + + leader_lp = find_lwp_pid (inf->id); + if (leader_lp != NULL + /* Check if there are other threads in the group, as we may + have raced with the inferior simply exiting. */ + && num_lwps (leader_pid) > 1 + && linux_proc_pid_is_zombie (leader_pid)) + delete_lwp (leader_lp); +} + +/* Detect zombie thread group leaders, and "exit" them. We can't reap + their exits until all other threads in the group have exited. */ + +static void +check_zombie_leaders (void) +{ + for_each_inferior (&all_processes, check_zombie_leaders_callback); +} + +static int +not_suspended_callback (struct inferior_list_entry *inf, void *arg) +{ + ptid_t filter = *(ptid_t *)arg; + struct lwp_info *lwp; + + if (!ptid_match (inf->id, filter)) + return 0; + + lwp = find_lwp_pid (inf->id); + if (lwp == NULL) + return 0; + + if (lwp->suspended) + return 0; + + return 1; +} + +static int +check_pending_stop (struct inferior_list_entry *inf, void *data) +{ + ptid_t ptid = *(ptid_t *) data; + struct lwp_info *lwp; + + if (ptid_equal (ptid, minus_one_ptid) + || ptid_get_pid(inf->id) == ptid_get_pid(ptid)) + { + lwp = find_lwp_pid (inf->id); + if (lwp->pending_stop) + return 1; + } + + return 0; +} + static struct lwp_info * linux_wait_for_lwp (ptid_t ptid, int *wstatp, int options) { - int ret; - int to_wait_for = -1; + int ret = 0; struct lwp_info *child = NULL; + sigset_t prev_mask; if (debug_threads) fprintf (stderr, "linux_wait_for_lwp: %s\n", target_pid_to_str (ptid)); - if (ptid_equal (ptid, minus_one_ptid)) - to_wait_for = -1; /* any child */ - else - to_wait_for = ptid_get_lwp (ptid); /* this lwp only */ + /* Make sure SIGCHLD is blocked. */ + sigprocmask (SIG_BLOCK, &sigchld_mask, &prev_mask); - options |= __WALL; + /* First check if there is a LWP with a wait status pending. */ + if (ptid_equal (ptid, minus_one_ptid) || ptid_get_lwp (ptid) == 0) + child = (struct lwp_info*) find_inferior (&all_lwps, check_pending_stop, + &ptid); + else + child = find_lwp_pid (ptid); + if (child && child->pending_stop) + { + if (debug_threads) + fprintf (stderr, + "linux_wait_for_lwp: %s has a wait status pending.\n", + target_pid_to_str (ptid_of (child))); + *wstatp = child->last_status; + child->pending_stop = 0; + child->stopped = 1; + goto got_child; + } retry: + ret = my_waitpid (-1, wstatp, __WCLONE | WNOHANG); + if (ret == 0 || (ret == -1 && errno == ECHILD)) + ret = my_waitpid (-1, wstatp, WNOHANG); - ret = my_waitpid (to_wait_for, wstatp, options); - if (ret == 0 || (ret == -1 && errno == ECHILD && (options & WNOHANG))) - return NULL; - else if (ret == -1) - perror_with_name ("waitpid"); + if (ret <= 0) + { + sigset_t suspend_mask; + + /* Check for zombie thread group leaders. Those can't be reaped + until all other threads in the thread group are. */ + check_zombie_leaders (); + + /* If there are no resumed children left, bail. We'd be stuck + forever in the sigsuspend call below otherwise. + OR no interesting event to report to the core.*/ + if ((find_inferior (&all_lwps, not_suspended_callback, &ptid) == NULL) + || (options & WNOHANG)) + { + sigprocmask (SIG_SETMASK, &prev_mask, NULL); + return NULL; + } + + /* Block until we get an event reported with SIGCHLD. */ + sigprocmask (SIG_SETMASK, NULL, &suspend_mask); + sigdelset (&suspend_mask, SIGCHLD); + sigsuspend (&suspend_mask); + + goto retry; + } if (debug_threads && (!WIFSTOPPED (*wstatp) @@ -1408,6 +1534,24 @@ retry: current_inferior = saved_inferior; } + if (!ptid_match (ptid_of (child), ptid)) + { + if (debug_threads) + { + char child_ptid_buf[80]; + + strcpy (child_ptid_buf, target_pid_to_str (ptid_of (child))); + fprintf (stderr, "linux_wait_for_lwp: %s is not same with %s.\n", + child_ptid_buf, target_pid_to_str (ptid)); + } + child->pending_stop = 1; + child->stopped = 0; + goto retry; + } + +got_child: + sigprocmask (SIG_SETMASK, &prev_mask, NULL); + return child; } @@ -1844,16 +1988,13 @@ linux_wait_for_event (ptid_t ptid, int * { event_child = linux_wait_for_lwp (wait_ptid, wstat, options); - if ((options & WNOHANG) && event_child == NULL) + if (event_child == NULL) { if (debug_threads) - fprintf (stderr, "WNOHANG set, no event found\n"); + fprintf (stderr, "No event found\n"); return 0; } - if (event_child == NULL) - error ("event from unknown child"); - if (ptid_is_pid (ptid) && ptid_get_pid (ptid) != ptid_get_pid (ptid_of (event_child))) { @@ -2314,8 +2455,25 @@ retry: pid = linux_wait_for_event (step_over_bkpt, &w, options & ~WNOHANG); } - if (pid == 0) /* only if TARGET_WNOHANG */ - return null_ptid; + if (pid == 0) + { + /* If there are no resumed children left, bail. We'd be stuck + forever in the sigsuspend call below otherwise. */ + if (find_inferior (&all_lwps, not_suspended_callback, &ptid) == NULL) + { + ourstatus->kind = TARGET_WAITKIND_NO_RESUMED; + return minus_one_ptid; + } + + /* No interesting event to report to the core. */ + if (target_options & TARGET_WNOHANG) + { + ourstatus->kind = TARGET_WAITKIND_IGNORE; + return minus_one_ptid; + } + + return null_ptid; + } event_child = get_thread_lwp (current_inferior); @@ -2899,6 +3057,175 @@ mark_lwp_dead (struct lwp_info *lwp, int lwp->stop_expected = 0; } +/* Wait for LP to stop. Returns the wait status, or 0 if the LWP has + exited. */ + +static int +wait_lwp (struct lwp_info *lwp) +{ + pid_t pid; + int status = 0; + int thread_dead = 0; + sigset_t prev_mask; + + /* Make sure SIGCHLD is blocked for sigsuspend avoiding a race below. */ + sigprocmask (SIG_BLOCK, &sigchld_mask, &prev_mask); + + for (;;) + { + sigset_t suspend_mask; + + /* If my_waitpid returns 0 it means the __WCLONE vs. non-__WCLONE kind + was right and we should just call sigsuspend. */ + + pid = my_waitpid (ptid_get_lwp (lwp->head.id), &status, WNOHANG); + if (pid == -1 && errno == ECHILD) + pid = my_waitpid (ptid_get_lwp (lwp->head.id), &status, __WCLONE | WNOHANG); + if (pid == -1 && errno == ECHILD) + { + /* The thread has previously exited. We need to delete it + now because, for some vendor 2.4 kernels with NPTL + support backported, there won't be an exit event unless + it is the main thread. 2.6 kernels will report an exit + event for each thread that exits, as expected. */ + thread_dead = 1; + if (debug_threads) + fprintf (stderr, "WL: %s vanished.\n", + target_pid_to_str (lwp->head.id)); + } + if (pid != 0) + break; + + /* Bugs 10970, 12702. + Thread group leader may have exited in which case we'll lock up in + waitpid if there are other threads, even if they are all zombies too. + Basically, we're not supposed to use waitpid this way. + __WCLONE is not applicable for the leader so we can't use that. + LINUX_NAT_THREAD_ALIVE cannot be used here as it requires a STOPPED + process; it gets ESRCH both for the zombie and for running processes. + + As a workaround, check if we're waiting for the thread group leader and + if it's a zombie, and avoid calling waitpid if it is. + + This is racy, what if the tgl becomes a zombie right after we check? + Therefore always use WNOHANG with sigsuspend - it is equivalent to + waiting waitpid but linux_proc_pid_is_zombie is safe this way. */ + + if (ptid_get_pid (lwp->head.id) == ptid_get_lwp (lwp->head.id) + && linux_proc_pid_is_zombie (ptid_get_lwp (lwp->head.id))) + { + thread_dead = 1; + if (debug_threads) + fprintf (stderr, "WL: Thread group leader %s vanished.\n", + target_pid_to_str (lwp->head.id)); + break; + } + + /* Wait for next SIGCHLD and try again. This may let SIGCHLD handlers + get invoked despite our caller had them intentionally blocked by + block_child_signals. This is sensitive only to the loop of + linux_nat_wait_1 and there if we get called my_waitpid gets called + again before it gets to sigsuspend so we can safely let the handlers + get executed here. */ + + /* Block until we get an event reported with SIGCHLD. */ + sigprocmask (SIG_SETMASK, NULL, &suspend_mask); + sigdelset (&suspend_mask, SIGCHLD); + sigsuspend (&suspend_mask); + } + + sigprocmask (SIG_SETMASK, &prev_mask, NULL); + + if (!thread_dead) + { + gdb_assert (pid == ptid_get_lwp (lwp->head.id)); + + if (debug_threads) + fprintf (stderr, "WL: waitpid %s received %x\n", + target_pid_to_str (lwp->head.id), status); + + /* Check if the thread has exited. */ + if (WIFEXITED (status) || WIFSIGNALED (status)) + { + thread_dead = 1; + if (debug_threads) + fprintf (stderr, "WL: %s exited.\n", + target_pid_to_str (lwp->head.id)); + } + } + + if (thread_dead) + { + delete_lwp (lwp); + return 0; + } + + gdb_assert (WIFSTOPPED (status)); + + lwp->stopped = 1; + lwp->last_status = status; + if (WIFSTOPPED (status)) + { + lwp->stop_pc = get_stop_pc (lwp); + if (WSTOPSIG (status) == SIGSTOP && lwp->stop_expected) + lwp->stop_expected = 0; + } + + if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP) + { + if (the_low_target.stopped_by_watchpoint == NULL) + { + lwp->stopped_by_watchpoint = 0; + } + else + { + struct thread_info *saved_inferior; + + saved_inferior = current_inferior; + current_inferior = get_lwp_thread (lwp); + + lwp->stopped_by_watchpoint + = the_low_target.stopped_by_watchpoint (); + + if (lwp->stopped_by_watchpoint) + { + if (the_low_target.stopped_data_address != NULL) + lwp->stopped_data_address + = the_low_target.stopped_data_address (); + else + lwp->stopped_data_address = 0; + } + + current_inferior = saved_inferior; + } + } + + /* Handle GNU/Linux's syscall SIGTRAPs. */ + if (WIFSTOPPED (status) && WSTOPSIG (status) == SYSCALL_SIGTRAP) + { + if (debug_threads) + fprintf (stderr, "WL: Handling syscall SIGTRAPs\n"); + /* No longer need the sysgood bit. The ptrace event ends up + recorded in lp->waitstatus if we care for it. We can carry + on handling the event like a regular SIGTRAP from here + on. */ + status = W_STOPCODE (SIGTRAP); + ptrace (PTRACE_CONT, ptid_get_lwp (lwp->head.id), 0, 0); + return wait_lwp (lwp); + } + + /* Handle GNU/Linux's extended waitstatus for trace events. */ + if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP && status >> 16 != 0) + { + if (debug_threads) + fprintf (stderr, "WL: Handling extended status 0x%06x\n", status); + handle_extended_wait (lwp, status); + return wait_lwp (lwp); + } + + return status; +} + static void wait_for_sigstop (struct inferior_list_entry *entry) { @@ -2906,8 +3233,6 @@ wait_for_sigstop (struct inferior_list_e struct thread_info *saved_inferior; int wstat; ptid_t saved_tid; - ptid_t ptid; - int pid; if (lwp->stopped) { @@ -2923,12 +3248,12 @@ wait_for_sigstop (struct inferior_list_e else saved_tid = null_ptid; /* avoid bogus unused warning */ - ptid = lwp->head.id; - if (debug_threads) fprintf (stderr, "wait_for_sigstop: pulling one event\n"); - pid = linux_wait_for_event (ptid, &wstat, __WALL); + wstat = wait_lwp (lwp); + if (wstat == 0) + return; /* If we stopped with a non-SIGSTOP signal, save it for later and record the pending SIGSTOP. If the process exited, just @@ -2952,9 +3277,10 @@ wait_for_sigstop (struct inferior_list_e else { if (debug_threads) - fprintf (stderr, "Process %d exited while stopping LWPs\n", pid); + fprintf (stderr, "Process %ld exited while stopping LWPs\n", + ptid_get_lwp (lwp->head.id)); - lwp = find_lwp_pid (pid_to_ptid (pid)); + lwp = find_lwp_pid (pid_to_ptid (ptid_get_lwp (lwp->head.id))); if (lwp) { /* Leave this status pending for the next time we're able to @@ -5864,5 +6190,8 @@ initialize_low (void) sigchld_action.sa_flags = SA_RESTART; sigaction (SIGCHLD, &sigchld_action, NULL); + sigemptyset (&sigchld_mask); + sigaddset(&sigchld_mask, SIGCHLD); + initialize_low_arch (); } --- a/gdb/gdbserver/linux-low.h +++ b/gdb/gdbserver/linux-low.h @@ -253,6 +253,9 @@ struct lwp_info event already received in a wait()). */ int stopped; + /* If this flag is set, the lwp has a wait status pending. */ + int pending_stop; + /* If this flag is set, the lwp is known to be dead already (exit event already received in a wait(), and is cached in status_pending). */ --- a/gdb/gdbserver/server.c +++ b/gdb/gdbserver/server.c @@ -3595,7 +3595,8 @@ handle_target_event (int err, gdb_client last_ptid = mywait (minus_one_ptid, &last_status, TARGET_WNOHANG, 1); - if (last_status.kind != TARGET_WAITKIND_IGNORE) + if (last_status.kind != TARGET_WAITKIND_IGNORE + && last_status.kind != TARGET_WAITKIND_NO_RESUMED) { int pid = ptid_get_pid (last_ptid); struct process_info *process = find_process_pid (pid); --- a/gdb/inferior.h +++ b/gdb/inferior.h @@ -65,16 +65,6 @@ extern void discard_infcall_control_stat extern struct regcache * get_infcall_suspend_state_regcache (struct infcall_suspend_state *); -/* Returns true if PTID matches filter FILTER. FILTER can be the wild - card MINUS_ONE_PTID (all ptid match it); can be a ptid representing - a process (ptid_is_pid returns true), in which case, all lwps and - threads of that given process match, lwps and threads of other - processes do not; or, it can represent a specific thread, in which - case, only that thread will match true. PTID must represent a - specific LWP or THREAD, it can never be a wild card. */ - -extern int ptid_match (ptid_t ptid, ptid_t filter); - /* Save value of inferior_ptid so that it may be restored by a later call to do_cleanups(). Returns the struct cleanup pointer needed for later doing the cleanup. */ --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -7059,20 +7059,6 @@ discard_infcall_control_state (struct in xfree (inf_status); } \f -int -ptid_match (ptid_t ptid, ptid_t filter) -{ - if (ptid_equal (filter, minus_one_ptid)) - return 1; - if (ptid_is_pid (filter) - && ptid_get_pid (ptid) == ptid_get_pid (filter)) - return 1; - else if (ptid_equal (ptid, filter)) - return 1; - - return 0; -} - /* restore_inferior_ptid() will be used by the cleanup machinery to restore the inferior_ptid value saved in a call to save_inferior_ptid(). */ ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Fix PR 12702 - gdb can hang waiting for thread group leader (gdbserver) 2013-11-19 6:19 ` Hui Zhu @ 2014-01-23 14:51 ` Pedro Alves 2014-01-23 14:10 ` [PATCH 1/5] Move ptid_match to common/ptid.c Pedro Alves 2014-02-27 14:38 ` [PATCH] Fix PR 12702 - gdb can hang waiting for thread group leader (gdbserver) Pedro Alves 0 siblings, 2 replies; 12+ messages in thread From: Pedro Alves @ 2014-01-23 14:51 UTC (permalink / raw) To: Hui Zhu; +Cc: gdb-patches ml, Gustavo, Luis On 11/19/2013 06:15 AM, Hui Zhu wrote: > On Thu, Jul 25, 2013 at 2:25 AM, Pedro Alves <palves@redhat.com> wrote: >> > On 07/20/2013 10:59 AM, Hui Zhu wrote: >>> >> Hi, >>> >> >>> >> I got a issue with leader-exit.exp will hang with target_board=native-gdbserver. >>> >> I found the reason is gdbserver still have this issue. So I post a >>> >> patch for gdbserver. >> > >> > leader-exit.exp is part of a series of tests for a set of related >> > problems. See: >> > >> > http://www.sourceware.org/ml/gdb-patches/2011-10/msg00704.html >> > >> > Several are currently masked because gdbserver doesn't support >> > fork/exec yet. >> > >> > We should fix this by implementing TARGET_WAITKIND_NO_RESUMED >> > on gdbserver, like the patch above did for native. >> > >> > -- >> > Pedro Alves >> > > Hi Pedro, > > According to your comments, I make a new patch to add > TARGET_WAITKIND_NO_RESUMED to gdbserver. > It can pass the leader-exit.exp and pass the regression test. > > Please help me review it. Hi Hui. Sorry for the time it takes to get through patches like this. This is hardly trivial code... > --- a/gdb/gdbserver/linux-low.h > +++ b/gdb/gdbserver/linux-low.h > @@ -253,6 +253,9 @@ struct lwp_info > event already received in a wait()). */ > int stopped; > > + /* If this flag is set, the lwp has a wait status pending. */ > + int pending_stop; > + You didn't explain it, so it took me a bit to grok this, but I figured out that essentially, you're adding yet another layer of pending events (a lower one). Hmm. I'd really like to avoid that. > + /* If there are no resumed children left, bail. We'd be stuck > + forever in the sigsuspend call below otherwise. > + OR no interesting event to report to the core.*/ > + if ((find_inferior (&all_lwps, not_suspended_callback, &ptid) == NULL) > + || (options & WNOHANG)) > + { Not clear to me why checking the suspended flag would be the right check when determining whether there are no resumed LWPs left. > +/* Wait for LP to stop. Returns the wait status, or 0 if the LWP has > + exited. */ > + > +static int > +wait_lwp (struct lwp_info *lwp) > +{ > + pid_t pid; > + int status = 0; > + int thread_dead = 0; > + sigset_t prev_mask; > + > + /* Make sure SIGCHLD is blocked for sigsuspend avoiding a race below. */ > + sigprocmask (SIG_BLOCK, &sigchld_mask, &prev_mask); > + > + for (;;) > + { > + sigset_t suspend_mask; > + > + /* If my_waitpid returns 0 it means the __WCLONE vs. non-__WCLONE kind > + was right and we should just call sigsuspend. */ > + > + pid = my_waitpid (ptid_get_lwp (lwp->head.id), &status, WNOHANG); > + if (pid == -1 && errno == ECHILD) > + pid = my_waitpid (ptid_get_lwp (lwp->head.id), &status, __WCLONE | WNOHANG); > + if (pid == -1 && errno == ECHILD) > + { > + /* The thread has previously exited. We need to delete it > + now because, for some vendor 2.4 kernels with NPTL > + support backported, there won't be an exit event unless > + it is the main thread. 2.6 kernels will report an exit > + event for each thread that exits, as expected. */ > + thread_dead = 1; > + if (debug_threads) > + fprintf (stderr, "WL: %s vanished.\n", > + target_pid_to_str (lwp->head.id)); > + } > + if (pid != 0) > + break; > + > + /* Bugs 10970, 12702. > + Thread group leader may have exited in which case we'll lock up in > + waitpid if there are other threads, even if they are all zombies too. > + Basically, we're not supposed to use waitpid this way. > + __WCLONE is not applicable for the leader so we can't use that. > + LINUX_NAT_THREAD_ALIVE cannot be used here as it requires a STOPPED > + process; it gets ESRCH both for the zombie and for running processes. > + > + As a workaround, check if we're waiting for the thread group leader and > + if it's a zombie, and avoid calling waitpid if it is. > + > + This is racy, what if the tgl becomes a zombie right after we check? > + Therefore always use WNOHANG with sigsuspend - it is equivalent to > + waiting waitpid but linux_proc_pid_is_zombie is safe this way. */ > + > + if (ptid_get_pid (lwp->head.id) == ptid_get_lwp (lwp->head.id) > + && linux_proc_pid_is_zombie (ptid_get_lwp (lwp->head.id))) > + { > + thread_dead = 1; > + if (debug_threads) > + fprintf (stderr, "WL: Thread group leader %s vanished.\n", > + target_pid_to_str (lwp->head.id)); > + break; > + } > + > + /* Wait for next SIGCHLD and try again. This may let SIGCHLD handlers > + get invoked despite our caller had them intentionally blocked by > + block_child_signals. This is sensitive only to the loop of > + linux_nat_wait_1 and there if we get called my_waitpid gets called > + again before it gets to sigsuspend so we can safely let the handlers > + get executed here. */ > + > + /* Block until we get an event reported with SIGCHLD. */ > + sigprocmask (SIG_SETMASK, NULL, &suspend_mask); > + sigdelset (&suspend_mask, SIGCHLD); > + sigsuspend (&suspend_mask); > + } > + > + sigprocmask (SIG_SETMASK, &prev_mask, NULL); > + > + if (!thread_dead) > + { > + gdb_assert (pid == ptid_get_lwp (lwp->head.id)); > + > + if (debug_threads) > + fprintf (stderr, "WL: waitpid %s received %x\n", > + target_pid_to_str (lwp->head.id), status); > + > + /* Check if the thread has exited. */ > + if (WIFEXITED (status) || WIFSIGNALED (status)) > + { > + thread_dead = 1; > + if (debug_threads) > + fprintf (stderr, "WL: %s exited.\n", > + target_pid_to_str (lwp->head.id)); > + } > + } > + > + if (thread_dead) > + { > + delete_lwp (lwp); > + return 0; > + } > + > + gdb_assert (WIFSTOPPED (status)); > + > + lwp->stopped = 1; > + lwp->last_status = status; > + if (WIFSTOPPED (status)) > + { > + lwp->stop_pc = get_stop_pc (lwp); > + if (WSTOPSIG (status) == SIGSTOP && lwp->stop_expected) > + lwp->stop_expected = 0; > + } > + > + if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP) > + { > + if (the_low_target.stopped_by_watchpoint == NULL) > + { > + lwp->stopped_by_watchpoint = 0; > + } > + else > + { > + struct thread_info *saved_inferior; > + > + saved_inferior = current_inferior; > + current_inferior = get_lwp_thread (lwp); > + > + lwp->stopped_by_watchpoint > + = the_low_target.stopped_by_watchpoint (); > + > + if (lwp->stopped_by_watchpoint) > + { > + if (the_low_target.stopped_data_address != NULL) > + lwp->stopped_data_address > + = the_low_target.stopped_data_address (); > + else > + lwp->stopped_data_address = 0; > + } > + > + current_inferior = saved_inferior; > + } > + } > + > + /* Handle GNU/Linux's syscall SIGTRAPs. */ > + if (WIFSTOPPED (status) && WSTOPSIG (status) == SYSCALL_SIGTRAP) > + { > + if (debug_threads) > + fprintf (stderr, "WL: Handling syscall SIGTRAPs\n"); > + /* No longer need the sysgood bit. The ptrace event ends up > + recorded in lp->waitstatus if we care for it. We can carry > + on handling the event like a regular SIGTRAP from here > + on. */ > + status = W_STOPCODE (SIGTRAP); > + ptrace (PTRACE_CONT, ptid_get_lwp (lwp->head.id), 0, 0); > + return wait_lwp (lwp); > + } > + > + /* Handle GNU/Linux's extended waitstatus for trace events. */ > + if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP && status >> 16 != 0) > + { > + if (debug_threads) > + fprintf (stderr, "WL: Handling extended status 0x%06x\n", status); > + handle_extended_wait (lwp, status); > + return wait_lwp (lwp); > + } > + > + return status; > +} This is main gripe with this patch. I'd really like to avoid bringing in more of this broken use of waitpid(PID) into gdbserver (I realize most of this is copied from GDB), and this duplication of low level wait status handling. I think we can do better in gdbserver. There's really no need for wait_for_sigstop to wait for each LWP in turn. I think that with some refactoring, we can make it reuse linux_wait_for_event, and only end up with having to handle pending statuses in one place. So I've been working on this most of this week, and I got this almost done but I'm afraid I'll need to give attention to other work, so I'm not sure I'll be able to be back at this before February. Anyway, I'll post a WIP series in response to this email. In addition, it goes a step forward and starts adding support for TARGET_WAITKIND_NO_RESUMED to the RSP as well, which is needed for fully fixing no-unwaited-for-left.exp. I've put the series in the my github as well: git@github.com:palves/gdb.git TARGET_WAITKIND_NO_RESUMED_gdbserver -- Pedro Alves ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/5] Move ptid_match to common/ptid.c. 2014-01-23 14:51 ` Pedro Alves @ 2014-01-23 14:10 ` Pedro Alves 2014-01-23 14:10 ` [PATCH 4/5] Teach gdbserver's linux backend about no unwaited-for children (TARGET_WAITDKIND_NO_RESUMED) Pedro Alves ` (3 more replies) 2014-02-27 14:38 ` [PATCH] Fix PR 12702 - gdb can hang waiting for thread group leader (gdbserver) Pedro Alves 1 sibling, 4 replies; 12+ messages in thread From: Pedro Alves @ 2014-01-23 14:10 UTC (permalink / raw) To: gdb-patches; +Cc: teawater, luis_gustavo So that gdbserver can use it too. --- gdb/common/ptid.c | 14 ++++++++++++++ gdb/common/ptid.h | 10 ++++++++++ gdb/inferior.h | 10 ---------- gdb/infrun.c | 14 -------------- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/gdb/common/ptid.c b/gdb/common/ptid.c index 49354ad..f614669 100644 --- a/gdb/common/ptid.c +++ b/gdb/common/ptid.c @@ -114,3 +114,17 @@ ptid_tid_p (ptid_t ptid) return (ptid_get_tid (ptid) != 0); } + +int +ptid_match (ptid_t ptid, ptid_t filter) +{ + if (ptid_equal (filter, minus_one_ptid)) + return 1; + if (ptid_is_pid (filter) + && ptid_get_pid (ptid) == ptid_get_pid (filter)) + return 1; + else if (ptid_equal (ptid, filter)) + return 1; + + return 0; +} diff --git a/gdb/common/ptid.h b/gdb/common/ptid.h index 362882d..21b46d9 100644 --- a/gdb/common/ptid.h +++ b/gdb/common/ptid.h @@ -78,4 +78,14 @@ int ptid_lwp_p (ptid_t ptid); /* Return true if PTID's tid member is non-zero. */ int ptid_tid_p (ptid_t ptid); +/* Returns true if PTID matches filter FILTER. FILTER can be the wild + card MINUS_ONE_PTID (all ptid match it); can be a ptid representing + a process (ptid_is_pid returns true), in which case, all lwps and + threads of that given process match, lwps and threads of other + processes do not; or, it can represent a specific thread, in which + case, only that thread will match true. PTID must represent a + specific LWP or THREAD, it can never be a wild card. */ + +extern int ptid_match (ptid_t ptid, ptid_t filter); + #endif diff --git a/gdb/inferior.h b/gdb/inferior.h index 290980a8..fe0d5f7 100644 --- a/gdb/inferior.h +++ b/gdb/inferior.h @@ -65,16 +65,6 @@ extern void discard_infcall_control_state (struct infcall_control_state *); extern struct regcache * get_infcall_suspend_state_regcache (struct infcall_suspend_state *); -/* Returns true if PTID matches filter FILTER. FILTER can be the wild - card MINUS_ONE_PTID (all ptid match it); can be a ptid representing - a process (ptid_is_pid returns true), in which case, all lwps and - threads of that given process match, lwps and threads of other - processes do not; or, it can represent a specific thread, in which - case, only that thread will match true. PTID must represent a - specific LWP or THREAD, it can never be a wild card. */ - -extern int ptid_match (ptid_t ptid, ptid_t filter); - /* Save value of inferior_ptid so that it may be restored by a later call to do_cleanups(). Returns the struct cleanup pointer needed for later doing the cleanup. */ diff --git a/gdb/infrun.c b/gdb/infrun.c index 71d9615..8982f73 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -7084,20 +7084,6 @@ discard_infcall_control_state (struct infcall_control_state *inf_status) xfree (inf_status); } \f -int -ptid_match (ptid_t ptid, ptid_t filter) -{ - if (ptid_equal (filter, minus_one_ptid)) - return 1; - if (ptid_is_pid (filter) - && ptid_get_pid (ptid) == ptid_get_pid (filter)) - return 1; - else if (ptid_equal (ptid, filter)) - return 1; - - return 0; -} - /* restore_inferior_ptid() will be used by the cleanup machinery to restore the inferior_ptid value saved in a call to save_inferior_ptid(). */ -- 1.7.11.7 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/5] Teach gdbserver's linux backend about no unwaited-for children (TARGET_WAITDKIND_NO_RESUMED). 2014-01-23 14:10 ` [PATCH 1/5] Move ptid_match to common/ptid.c Pedro Alves @ 2014-01-23 14:10 ` Pedro Alves 2014-01-23 14:10 ` [PATCH 5/5] Add TARGET_WAITKIND_NO_RESUMED support to the RSP Pedro Alves ` (2 subsequent siblings) 3 siblings, 0 replies; 12+ messages in thread From: Pedro Alves @ 2014-01-23 14:10 UTC (permalink / raw) To: gdb-patches; +Cc: teawater, luis_gustavo WIP. gdb.threads/leader-exit.exp now passes. gdb.threads/no-unwaited-for-left.exp now at least errors out instead of hanging: continue Continuing. warning: Remote failure reply: E.No unwaited-for children left. [Thread 15454] #1 stopped. 0x00000034cf408e60 in pthread_join (threadid=140737353922368, thread_return=0x0) at pthread_join.c:93 93 lll_wait_tid (pd->tid); (gdb) FAIL: gdb.threads/no-unwaited-for-left.exp: continue stops when the main thread exits (BTW, in case of error from vCont, it would be better to query the target for the current thread, or re-select one, instead of assuming current inferior_ptid.) --- gdb/gdbserver/inferiors.h | 22 ++ gdb/gdbserver/linux-low.c | 844 +++++++++++++++++++++++++++------------------- gdb/gdbserver/server.c | 18 +- 3 files changed, 537 insertions(+), 347 deletions(-) diff --git a/gdb/gdbserver/inferiors.h b/gdb/gdbserver/inferiors.h index 5f99fbc..6ba40b1 100644 --- a/gdb/gdbserver/inferiors.h +++ b/gdb/gdbserver/inferiors.h @@ -84,6 +84,28 @@ void add_inferior_to_list (struct inferior_list *list, void for_each_inferior (struct inferior_list *list, void (*action) (struct inferior_list_entry *)); +/* Helper for ALL_INFERIORS_TYPE. Gets the next element starting at + CUR, if CUR is not NULL. */ +#define A_I_NEXT(type, list, cur) \ + ((cur) != NULL \ + ? (type *) ((struct inferior_list_entry *) cur)->next \ + : NULL) + +/* Iterate over all inferiors of type TYPE in LIST, open loop + style. */ +#define ALL_INFERIORS_TYPE(type, list, cur, tmp) \ + for ((cur) = (type *) (list)->head, (tmp) = A_I_NEXT (type, list, cur); \ + (cur) != NULL; \ + (cur) = (tmp), (tmp) = A_I_NEXT (type, list, cur)) + +/* Iterate over all inferiors in LIST, open loop style. */ +#define ALL_INFERIORS(list, cur, tmp) \ + ALL_INFERIORS_TYPE (struct inferior_list_entry, list, cur, tmp) + +/* Iterate over all processes, open loop style. */ +#define ALL_PROCESSES(cur, tmp) \ + ALL_INFERIORS_TYPE (struct process_info, &all_processes, cur, tmp) + extern struct thread_info *current_inferior; void remove_inferior (struct inferior_list *list, struct inferior_list_entry *entry); diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index bac6134..3593dbc 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -225,6 +225,8 @@ static void linux_resume_one_lwp (struct lwp_info *lwp, static void linux_resume (struct thread_resume *resume_info, size_t n); static void stop_all_lwps (int suspend, struct lwp_info *except); static void unstop_all_lwps (int unsuspend, struct lwp_info *except); +static int linux_wait_for_event_filtered (ptid_t wait_ptid, ptid_t filter_ptid, + int *wstat, int options); static int linux_wait_for_event (ptid_t ptid, int *wstat, int options); static void *add_lwp (ptid_t ptid); static int linux_stopped_by_watchpoint (void); @@ -287,7 +289,7 @@ static int linux_event_pipe[2] = { -1, -1 }; #define target_is_async_p() (linux_event_pipe[0] != -1) static void send_sigstop (struct lwp_info *lwp); -static void wait_for_sigstop (struct inferior_list_entry *entry); +static void wait_for_sigstop (void); /* Return non-zero if HEADER is a 64-bit ELF file. */ @@ -346,6 +348,9 @@ linux_pid_exe_is_elf_64_file (int pid, unsigned int *machine) static void delete_lwp (struct lwp_info *lwp) { + if (debug_threads) + debug_printf ("deleting %ld\n", lwpid_of (lwp)); + remove_thread (get_lwp_thread (lwp)); remove_inferior (&all_lwps, &lwp->head); free (lwp->arch_private); @@ -407,6 +412,11 @@ handle_extended_wait (struct lwp_info *event_child, int wstat) warning ("wait returned unexpected status 0x%x", status); } + if (debug_threads) + debug_printf ("HEW: Got clone event " + "from LWP %ld, new child is LWP %ld\n", + lwpid_of (event_child), new_pid); + ptid = ptid_build (pid_of (event_child), new_pid, 0); new_lwp = (struct lwp_info *) add_lwp (ptid); add_thread (ptid, new_lwp); @@ -863,10 +873,8 @@ second_thread_of_pid_p (struct inferior_list_entry *entry, void *args) } static int -last_thread_of_process_p (struct thread_info *thread) +last_thread_of_process_p (int pid) { - ptid_t ptid = ((struct inferior_list_entry *)thread)->id; - int pid = ptid_get_pid (ptid); struct counter counter = { pid , 0 }; return (find_inferior (&all_threads, @@ -941,7 +949,7 @@ kill_one_lwp_callback (struct inferior_list_entry *entry, void *args) linux_kill_one_lwp (lwp); /* Make sure it died. The loop is most likely unnecessary. */ - pid = linux_wait_for_event (lwp->head.id, &wstat, __WALL); + pid = linux_wait_for_event (ptid_of (lwp), &wstat, __WALL); } while (pid > 0 && WIFSTOPPED (wstat)); return 0; @@ -986,7 +994,7 @@ linux_kill (int pid) linux_kill_one_lwp (lwp); /* Make sure it died. The loop is most likely unnecessary. */ - lwpid = linux_wait_for_event (lwp->head.id, &wstat, __WALL); + lwpid = linux_wait_for_event (ptid_of (lwp), &wstat, __WALL); } while (lwpid > 0 && WIFSTOPPED (wstat)); } @@ -1260,147 +1268,105 @@ find_lwp_pid (ptid_t ptid) return (struct lwp_info*) find_inferior (&all_lwps, same_lwp, &ptid); } -static struct lwp_info * -linux_wait_for_lwp (ptid_t ptid, int *wstatp, int options) -{ - int ret; - int to_wait_for = -1; - struct lwp_info *child = NULL; - - if (debug_threads) - debug_printf ("linux_wait_for_lwp: %s\n", target_pid_to_str (ptid)); - - if (ptid_equal (ptid, minus_one_ptid)) - to_wait_for = -1; /* any child */ - else - to_wait_for = ptid_get_lwp (ptid); /* this lwp only */ - - options |= __WALL; +/* Return the number of known LWPs in the tgid given by PID. */ -retry: - - ret = my_waitpid (to_wait_for, wstatp, options); - if (ret == 0 || (ret == -1 && errno == ECHILD && (options & WNOHANG))) - return NULL; - else if (ret == -1) - perror_with_name ("waitpid"); - - if (debug_threads - && (!WIFSTOPPED (*wstatp) - || (WSTOPSIG (*wstatp) != 32 - && WSTOPSIG (*wstatp) != 33))) - debug_printf ("Got an event from %d (%x)\n", ret, *wstatp); - - child = find_lwp_pid (pid_to_ptid (ret)); +static int +num_lwps (int pid) +{ + struct inferior_list_entry *inf, *tmp; + int count = 0; - /* If we didn't find a process, one of two things presumably happened: - - A process we started and then detached from has exited. Ignore it. - - A process we are controlling has forked and the new child's stop - was reported to us by the kernel. Save its PID. */ - if (child == NULL && WIFSTOPPED (*wstatp)) + ALL_INFERIORS (&all_lwps, inf, tmp) { - add_to_pid_list (&stopped_pids, ret, *wstatp); - goto retry; + if (ptid_get_pid (inf->id) == pid) + count++; } - else if (child == NULL) - goto retry; - - child->stopped = 1; - - child->last_status = *wstatp; - - if (WIFSTOPPED (*wstatp)) - { - struct process_info *proc; - - /* Architecture-specific setup after inferior is running. This - needs to happen after we have attached to the inferior and it - is stopped for the first time, but before we access any - inferior registers. */ - proc = find_process_pid (pid_of (child)); - if (proc->private->new_inferior) - { - struct thread_info *saved_inferior; - - saved_inferior = current_inferior; - current_inferior = get_lwp_thread (child); - - the_low_target.arch_setup (); - - current_inferior = saved_inferior; - proc->private->new_inferior = 0; - } - } + return count; +} - /* Fetch the possibly triggered data watchpoint info and store it in - CHILD. +/* Detect zombie thread group leaders, and "exit" them. We can't reap + their exits until all other threads in the group have exited. */ - On some archs, like x86, that use debug registers to set - watchpoints, it's possible that the way to know which watched - address trapped, is to check the register that is used to select - which address to watch. Problem is, between setting the - watchpoint and reading back which data address trapped, the user - may change the set of watchpoints, and, as a consequence, GDB - changes the debug registers in the inferior. To avoid reading - back a stale stopped-data-address when that happens, we cache in - LP the fact that a watchpoint trapped, and the corresponding data - address, as soon as we see CHILD stop with a SIGTRAP. If GDB - changes the debug registers meanwhile, we have the cached data we - can rely on. */ +static void +check_zombie_leaders (void) +{ + struct process_info *proc, *tmp; - if (WIFSTOPPED (*wstatp) && WSTOPSIG (*wstatp) == SIGTRAP) + ALL_PROCESSES (proc, tmp) { - if (the_low_target.stopped_by_watchpoint == NULL) - { - child->stopped_by_watchpoint = 0; - } - else - { - struct thread_info *saved_inferior; + pid_t leader_pid = pid_of (proc); + struct lwp_info *leader_lp; - saved_inferior = current_inferior; - current_inferior = get_lwp_thread (child); + leader_lp = find_lwp_pid (pid_to_ptid (leader_pid)); - child->stopped_by_watchpoint - = the_low_target.stopped_by_watchpoint (); + if (debug_threads) + debug_printf ("leader_pid=%d, leader_lp!=NULL=%d, " + "num_lwps=%d, zombie=%d\n", + leader_pid, leader_lp!= NULL, num_lwps (leader_pid), + linux_proc_pid_is_zombie (leader_pid)); + + if (leader_lp != NULL + /* Check if there are other threads in the group, as we may + have raced with the inferior simply exiting. */ + && !last_thread_of_process_p (leader_pid) + && linux_proc_pid_is_zombie (leader_pid)) + { + /* A leader zombie can mean one of two things: + + - It exited, and there's an exit status pending + available, or only the leader exited (not the whole + program). In the latter case, we can't waitpid the + leader's exit status until all other threads are gone. + + - There are 3 or more threads in the group, and a thread + other than the leader exec'd. On an exec, the Linux + kernel destroys all other threads (except the execing + one) in the thread group, and resets the execing thread's + tid to the tgid. No exit notification is sent for the + execing thread -- from the ptracer's perspective, it + appears as though the execing thread just vanishes. + Until we reap all other threads except the leader and the + execing thread, the leader will be zombie, and the + execing thread will be in `D (disc sleep)'. As soon as + all other threads are reaped, the execing thread changes + it's tid to the tgid, and the previous (zombie) leader + vanishes, giving place to the "new" leader. We could try + distinguishing the exit and exec cases, by waiting once + more, and seeing if something comes out, but it doesn't + sound useful. The previous leader _does_ go away, and + we'll re-add the new one once we see the exec event + (which is just the same as what would happen if the + previous leader did exit voluntarily before some other + thread execs). */ - if (child->stopped_by_watchpoint) - { - if (the_low_target.stopped_data_address != NULL) - child->stopped_data_address - = the_low_target.stopped_data_address (); - else - child->stopped_data_address = 0; - } + if (debug_threads) + fprintf (stderr, + "CZL: Thread group leader %d zombie " + "(it exited, or another thread execd).\n", + leader_pid); - current_inferior = saved_inferior; + delete_lwp (leader_lp); } } +} - /* Store the STOP_PC, with adjustment applied. This depends on the - architecture being defined already (so that CHILD has a valid - regcache), and on LAST_STATUS being set (to check for SIGTRAP or - not). */ - if (WIFSTOPPED (*wstatp)) - child->stop_pc = get_stop_pc (child); +/* Callback for `find_inferior'. Returns the first LWP that is not + stopped. ARG is a PTID filter. */ - if (debug_threads - && WIFSTOPPED (*wstatp) - && the_low_target.get_pc != NULL) - { - struct thread_info *saved_inferior = current_inferior; - struct regcache *regcache; - CORE_ADDR pc; +static int +not_stopped_callback (struct inferior_list_entry *entry, void *arg) +{ + struct lwp_info *lp = (struct lwp_info *) entry; + ptid_t filter = *(ptid_t *) arg; - current_inferior = get_lwp_thread (child); - regcache = get_thread_regcache (current_inferior, 1); - pc = (*the_low_target.get_pc) (regcache); - debug_printf ("linux_wait_for_lwp: pc is 0x%lx\n", (long) pc); - current_inferior = saved_inferior; - } + if (!ptid_match (ptid_of (lp), filter)) + return 0; - return child; + if (!lp->stopped) + return 1; + + return 0; } /* This function should only be called if the LWP got a SIGTRAP. @@ -1748,37 +1714,275 @@ cancel_breakpoint (struct lwp_info *lwp) return 0; } +/* Do low-level handling of the event, and check if we should go on + and pass it to caller code. Return the affected lwp if we are, or + NULL otherwise. */ + +static struct lwp_info * +linux_low_filter_event (ptid_t filter_ptid, int lwpid, int wstat) +{ + struct lwp_info *child; + + child = find_lwp_pid (pid_to_ptid (lwpid)); + + /* If we didn't find a process, one of two things presumably happened: + - A process we started and then detached from has exited. Ignore it. + - A process we are controlling has forked and the new child's stop + was reported to us by the kernel. Save its PID. */ + if (child == NULL && WIFSTOPPED (wstat)) + { + add_to_pid_list (&stopped_pids, lwpid, wstat); + return NULL; + } + else if (child == NULL) + return NULL; + + child->stopped = 1; + + child->last_status = wstat; + + if (WIFSTOPPED (wstat)) + { + struct process_info *proc; + + /* Architecture-specific setup after inferior is running. This + needs to happen after we have attached to the inferior and it + is stopped for the first time, but before we access any + inferior registers. */ + proc = find_process_pid (pid_of (child)); + if (proc->private->new_inferior) + { + struct thread_info *saved_inferior; + + saved_inferior = current_inferior; + current_inferior = get_lwp_thread (child); + + the_low_target.arch_setup (); + + current_inferior = saved_inferior; + + proc->private->new_inferior = 0; + } + } + + /* Store the STOP_PC, with adjustment applied. This depends on the + architecture being defined already (so that CHILD has a valid + regcache), and on LAST_STATUS being set (to check for SIGTRAP or + not). */ + if (WIFSTOPPED (wstat)) + { + if (debug_threads + && the_low_target.get_pc != NULL) + { + struct thread_info *saved_inferior = current_inferior; + struct regcache *regcache; + CORE_ADDR pc; + + current_inferior = get_lwp_thread (child); + regcache = get_thread_regcache (current_inferior, 1); + pc = (*the_low_target.get_pc) (regcache); + debug_printf ("linux_low_filter_event: pc is 0x%lx\n", (long) pc); + current_inferior = saved_inferior; + } + + child->stop_pc = get_stop_pc (child); + } + + /* Fetch the possibly triggered data watchpoint info and store it in + CHILD. + + On some archs, like x86, that use debug registers to set + watchpoints, it's possible that the way to know which watched + address trapped, is to check the register that is used to select + which address to watch. Problem is, between setting the + watchpoint and reading back which data address trapped, the user + may change the set of watchpoints, and, as a consequence, GDB + changes the debug registers in the inferior. To avoid reading + back a stale stopped-data-address when that happens, we cache in + LP the fact that a watchpoint trapped, and the corresponding data + address, as soon as we see CHILD stop with a SIGTRAP. If GDB + changes the debug registers meanwhile, we have the cached data we + can rely on. */ + + if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SIGTRAP) + { + if (the_low_target.stopped_by_watchpoint == NULL) + { + child->stopped_by_watchpoint = 0; + } + else + { + struct thread_info *saved_inferior; + + saved_inferior = current_inferior; + current_inferior = get_lwp_thread (child); + + child->stopped_by_watchpoint + = the_low_target.stopped_by_watchpoint (); + + if (child->stopped_by_watchpoint) + { + if (the_low_target.stopped_data_address != NULL) + child->stopped_data_address + = the_low_target.stopped_data_address (); + else + child->stopped_data_address = 0; + } + + current_inferior = saved_inferior; + } + } + + if (WIFSTOPPED (wstat) && child->must_set_ptrace_flags) + { + linux_enable_event_reporting (lwpid_of (child)); + child->must_set_ptrace_flags = 0; + } + + if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SIGTRAP + && wstat >> 16 != 0) + { + handle_extended_wait (child, wstat); + return NULL; + } + + if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SIGSTOP + && child->stop_expected) + { + struct thread_info *thread; + + if (debug_threads) + debug_printf ("Expected stop.\n"); + child->stop_expected = 0; + + thread = get_lwp_thread (child); + + if (thread->last_resume_kind == resume_stop) + { + /* We want to report the stop to the core. Treat the + SIGSTOP as a normal event. */ + } + else if (stopping_threads != NOT_STOPPING_THREADS) + { + /* Stopping threads. We don't want this SIGSTOP to end up + pending in the FILTER_PTID handling below. */ + return NULL; + } + else + { + /* Filter out the event. */ + linux_resume_one_lwp (child, child->stepping, 0, NULL); + return NULL; + } + } + + /* Check if the thread has exited. */ + if ((WIFEXITED (wstat) || WIFSIGNALED (wstat)) + && num_lwps (pid_of (child)) > 1) + { + if (debug_threads) + debug_printf ("LLW: %ld exited.\n", lwpid_of (child)); + + /* If there is at least one more LWP, then the exit signal + was not the end of the debugged application and should be + ignored. */ + delete_lwp (child); + return NULL; + } + + if (!ptid_match (ptid_of (child), filter_ptid)) + { + if (debug_threads) + debug_printf ("LWP %ld got an event %06x, leaving pending.\n", + lwpid_of (child), wstat); + + if (WIFSTOPPED (wstat)) + { + child->status_pending_p = 1; + child->status_pending = wstat; + + if (WSTOPSIG (wstat) != SIGSTOP) + { + /* Cancel breakpoint hits. The breakpoint may be + removed before we fetch events from this process to + report to the core. It is best not to assume the + moribund breakpoints heuristic always handles these + cases --- it could be too many events go through to + the core before this one is handled. All-stop always + cancels breakpoint hits in all threads. */ + if (non_stop + && WSTOPSIG (wstat) == SIGTRAP + && cancel_breakpoint (child)) + { + /* Throw away the SIGTRAP. */ + child->status_pending_p = 0; + + if (debug_threads) + debug_printf ("LLW: LWP %ld hit a breakpoint while" + " waiting for another process;" + " cancelled it\n", + lwpid_of (child)); + } + } + } + else if (WIFEXITED (wstat) || WIFSIGNALED (wstat)) + { + if (debug_threads) + debug_printf ("LLWE: process %ld exited while fetching " + "event from another LWP\n", + lwpid_of (child)); + + /* This was the last lwp in the process. Since events are + serialized to GDB core, and we can't report this one + right now, but GDB core and the other target layers will + want to be notified about the exit code/signal, leave the + status pending for the next time we're able to report + it. */ + mark_lwp_dead (child, wstat); + } + + return NULL; + } + + return child; +} + /* When the event-loop is doing a step-over, this points at the thread being stepped. */ ptid_t step_over_bkpt; -/* Wait for an event from child PID. If PID is -1, wait for any - child. Store the stop status through the status pointer WSTAT. - OPTIONS is passed to the waitpid call. Return 0 if no child stop - event was found and OPTIONS contains WNOHANG. Return the PID of - the stopped child otherwise. */ +/* Wait for an event from child(ren) WAIT_PTID, and return any that + match FILTER_PTID (leaving others pending). The PTIDs can be: + minus_one_ptid, to specify any child; a pid PTID, specifying all + lwps of a thread group; or a PTID representing a single lwp. Store + the stop status through the status pointer WSTAT. OPTIONS is + passed to the waitpid call. Return 0 if no event was found and + OPTIONS contains WNOHANG. Return -1 if no unwaited-for children + was found. Return the PID of the stopped child otherwise. */ static int -linux_wait_for_event (ptid_t ptid, int *wstat, int options) +linux_wait_for_event_filtered (ptid_t wait_ptid, ptid_t filter_ptid, + int *wstatp, int options) { struct lwp_info *event_child, *requested_child; - ptid_t wait_ptid; + sigset_t block_mask, prev_mask; + retry: event_child = NULL; requested_child = NULL; /* Check for a lwp with a pending status. */ - if (ptid_equal (ptid, minus_one_ptid) || ptid_is_pid (ptid)) + if (ptid_equal (filter_ptid, minus_one_ptid) || ptid_is_pid (filter_ptid)) { event_child = (struct lwp_info *) - find_inferior (&all_lwps, status_pending_p_callback, &ptid); + find_inferior (&all_lwps, status_pending_p_callback, &filter_ptid); if (debug_threads && event_child) debug_printf ("Got a pending child %ld\n", lwpid_of (event_child)); } - else + else if (!ptid_equal (filter_ptid, null_ptid)) { - requested_child = find_lwp_pid (ptid); + requested_child = find_lwp_pid (filter_ptid); if (stopping_threads == NOT_STOPPING_THREADS && requested_child->status_pending_p @@ -1803,146 +2007,140 @@ linux_wait_for_event (ptid_t ptid, int *wstat, int options) { if (debug_threads) debug_printf ("Got an event from pending child %ld (%04x)\n", - lwpid_of (event_child), event_child->status_pending); - *wstat = event_child->status_pending; + lwpid_of (event_child), event_child->status_pending); + *wstatp = event_child->status_pending; event_child->status_pending_p = 0; event_child->status_pending = 0; current_inferior = get_lwp_thread (event_child); return lwpid_of (event_child); } - if (ptid_is_pid (ptid)) - { - /* A request to wait for a specific tgid. This is not possible - with waitpid, so instead, we wait for any child, and leave - children we're not interested in right now with a pending - status to report later. */ - wait_ptid = minus_one_ptid; - } - else - wait_ptid = ptid; + /* But if we don't find a pending event, we'll have to wait. + + We only enter this loop if no process has a pending wait status. + Thus any action taken in response to a wait status inside this + loop is responding as soon as we detect the status, not after any + pending events. */ + + /* Make sure SIGCHLD is blocked until the sigsuspend below. Block + all signals while here. */ + sigfillset (&block_mask); + sigprocmask (SIG_BLOCK, &block_mask, &prev_mask); - /* We only enter this loop if no process has a pending wait status. Thus - any action taken in response to a wait status inside this loop is - responding as soon as we detect the status, not after any pending - events. */ - while (1) + while (event_child == NULL) { - event_child = linux_wait_for_lwp (wait_ptid, wstat, options); + pid_t ret = 0; - if ((options & WNOHANG) && event_child == NULL) - { - if (debug_threads) - debug_printf ("WNOHANG set, no event found\n"); - return 0; - } + /* Always use -1 and WNOHANG, due to couple of a kernel/ptrace + quirks: - if (event_child == NULL) - error ("event from unknown child"); + - If the thread group leader exits while other threads in the + thread group still exist, waitpid(TGID, ...) hangs. That + waitpid won't return an exit status until the other threads + in the group are reaped. - if (ptid_is_pid (ptid) - && ptid_get_pid (ptid) != ptid_get_pid (ptid_of (event_child))) - { - if (! WIFSTOPPED (*wstat)) - mark_lwp_dead (event_child, *wstat); - else - { - event_child->status_pending_p = 1; - event_child->status_pending = *wstat; - } - continue; - } + - When a non-leader thread execs, that thread just vanishes + without reporting an exit (so we'd hang if we waited for + it explicitly in that case). The exec event is reported + to the TGID pid (although we don't currently enable exec + events). */ + errno = 0; + ret = my_waitpid (-1, wstatp, options | WNOHANG); - current_inferior = get_lwp_thread (event_child); + if (debug_threads) + debug_printf ("LWFE: waitpid(-1, ...) returned %d, %s\n", + ret, errno ? strerror (errno) : "ERRNO-OK"); - /* Check for thread exit. */ - if (! WIFSTOPPED (*wstat)) + if (ret > 0) { if (debug_threads) - debug_printf ("LWP %ld exiting\n", lwpid_of (event_child)); - - /* If the last thread is exiting, just return. */ - if (last_thread_of_process_p (current_inferior)) { - if (debug_threads) - debug_printf ("LWP %ld is last lwp of process\n", - lwpid_of (event_child)); - return lwpid_of (event_child); + debug_printf ("LLW: waitpid %ld received %s\n", + (long) ret, status_to_str (*wstatp)); } - if (!non_stop) + event_child = linux_low_filter_event (filter_ptid, + ret, *wstatp); + if (event_child != NULL) { - current_inferior = (struct thread_info *) all_threads.head; - if (debug_threads) - debug_printf ("Current inferior is now %ld\n", - lwpid_of (get_thread_lwp (current_inferior))); - } - else - { - current_inferior = NULL; - if (debug_threads) - debug_printf ("Current inferior is now <NULL>\n"); - } - - /* If we were waiting for this particular child to do something... - well, it did something. */ - if (requested_child != NULL) - { - int lwpid = lwpid_of (event_child); - - /* Cancel the step-over operation --- the thread that - started it is gone. */ - if (finish_step_over (event_child)) - unstop_all_lwps (1, event_child); - delete_lwp (event_child); - return lwpid; + /* We got an event to report to the core. */ + break; } - delete_lwp (event_child); - - /* Wait for a more interesting event. */ + /* Retry until nothing comes out of waitpid. A single + SIGCHLD can indicate more than one child stopped. */ continue; } - if (event_child->must_set_ptrace_flags) + /* Check for zombie thread group leaders. Those can't be reaped + until all other threads in the thread group are. */ + check_zombie_leaders (); + + /* If there are no resumed children left in the set of LWPs we + want to wait for, bail. We can't just block in + waitpid/sigsuspend, because lwps might have been left stopped + in trace-stop state, and we'd be stuck forever waiting for + their status to change (which would only happen if we resumed + them). Even if WNOHANG is set, this return code is preferred + over 0 (below), as it is more detailed. */ + if ((find_inferior (&all_lwps, not_stopped_callback, &wait_ptid) == NULL)) { - linux_enable_event_reporting (lwpid_of (event_child)); - event_child->must_set_ptrace_flags = 0; + if (debug_threads) + debug_printf ("LLW: exit (no unwaited-for LWP)\n"); + sigprocmask (SIG_SETMASK, &prev_mask, NULL); + return -1; } - if (WIFSTOPPED (*wstat) && WSTOPSIG (*wstat) == SIGTRAP - && *wstat >> 16 != 0) + /* No interesting event to report to the caller. */ + if ((options & WNOHANG)) { - handle_extended_wait (event_child, *wstat); - continue; + if (debug_threads) + debug_printf ("WNOHANG set, no event found\n"); + + sigprocmask (SIG_SETMASK, &prev_mask, NULL); + return 0; } - if (WIFSTOPPED (*wstat) - && WSTOPSIG (*wstat) == SIGSTOP - && event_child->stop_expected) - { - int should_stop; + /* Block until we get an event reported with SIGCHLD. */ + if (debug_threads) + debug_printf ("sigsuspend'ing\n"); - if (debug_threads) - debug_printf ("Expected stop.\n"); - event_child->stop_expected = 0; + sigsuspend (&prev_mask); + sigprocmask (SIG_SETMASK, &prev_mask, NULL); + goto retry; + } - should_stop = (current_inferior->last_resume_kind == resume_stop - || stopping_threads != NOT_STOPPING_THREADS); + sigprocmask (SIG_SETMASK, &prev_mask, NULL); - if (!should_stop) - { - linux_resume_one_lwp (event_child, - event_child->stepping, 0, NULL); - continue; - } - } + current_inferior = get_lwp_thread (event_child); + /* Check for thread exit. */ + if (! WIFSTOPPED (*wstatp)) + { + gdb_assert (last_thread_of_process_p (pid_of (event_child))); + + if (debug_threads) + debug_printf ("LWP %d is the last lwp of process. " + "Process %ld exiting.\n", + pid_of (event_child), lwpid_of (event_child)); return lwpid_of (event_child); } - /* NOTREACHED */ - return 0; + return lwpid_of (event_child); +} + +/* Wait for an event from child(ren) PTID. PTIDs can be: + minus_one_ptid, to specify any child; a pid PTID, specifying all + lwps of a thread group; or a PTID representing a single lwp. Store + the stop status through the status pointer WSTAT. OPTIONS is + passed to the waitpid call. Return 0 if no event was found and + OPTIONS contains WNOHANG. Return -1 if no unwaited-for children + was found. Return the PID of the stopped child otherwise. */ + +static int +linux_wait_for_event (ptid_t ptid, int *wstatp, int options) +{ + return linux_wait_for_event_filtered (ptid, ptid, wstatp, options); } /* Count the LWP's that have had events. */ @@ -2304,70 +2502,69 @@ retry: pid = linux_wait_for_event (step_over_bkpt, &w, options & ~WNOHANG); } - if (pid == 0) /* only if TARGET_WNOHANG */ + if (pid == 0) { + gdb_assert (target_options & TARGET_WNOHANG); + if (debug_threads) { - debug_printf ("linux_wait_1 ret = null_ptid\n"); + debug_printf ("linux_wait_1 ret = null_ptid, " + "TARGET_WAITKIND_IGNORE\n"); debug_exit (); } + + ourstatus->kind = TARGET_WAITKIND_IGNORE; return null_ptid; } + else if (pid == -1) + { + if (debug_threads) + { + debug_printf ("linux_wait_1 ret = null_ptid, " + "TARGET_WAITKIND_NO_RESUMED\n"); + debug_exit (); + } - event_child = get_thread_lwp (current_inferior); - - /* If we are waiting for a particular child, and it exited, - linux_wait_for_event will return its exit status. Similarly if - the last child exited. If this is not the last child, however, - do not report it as exited until there is a 'thread exited' response - available in the remote protocol. Instead, just wait for another event. - This should be safe, because if the thread crashed we will already - have reported the termination signal to GDB; that should stop any - in-progress stepping operations, etc. + ourstatus->kind = TARGET_WAITKIND_NO_RESUMED; + return null_ptid; + } - Report the exit status of the last thread to exit. This matches - LinuxThreads' behavior. */ + event_child = get_thread_lwp (current_inferior); - if (last_thread_of_process_p (current_inferior)) + /* linux_wait_for_event only returns an exit status for the last + child of a process. Report it. */ + if (WIFEXITED (w) || WIFSIGNALED (w)) { - if (WIFEXITED (w) || WIFSIGNALED (w)) + if (WIFEXITED (w)) { - if (WIFEXITED (w)) - { - ourstatus->kind = TARGET_WAITKIND_EXITED; - ourstatus->value.integer = WEXITSTATUS (w); + ourstatus->kind = TARGET_WAITKIND_EXITED; + ourstatus->value.integer = WEXITSTATUS (w); - if (debug_threads) - { - debug_printf ("linux_wait_1 ret = %s, exited with " - "retcode %d\n", - target_pid_to_str (ptid_of (event_child)), - WEXITSTATUS (w)); - debug_exit (); - } - } - else + if (debug_threads) { - ourstatus->kind = TARGET_WAITKIND_SIGNALLED; - ourstatus->value.sig = gdb_signal_from_host (WTERMSIG (w)); - - if (debug_threads) - { - debug_printf ("linux_wait_1 ret = %s, terminated with " - "signal %d\n", - target_pid_to_str (ptid_of (event_child)), - WTERMSIG (w)); - debug_exit (); - } + debug_printf ("linux_wait_1 ret = %s, exited with " + "retcode %d\n", + target_pid_to_str (ptid_of (event_child)), + WEXITSTATUS (w)); + debug_exit (); } + } + else + { + ourstatus->kind = TARGET_WAITKIND_SIGNALLED; + ourstatus->value.sig = gdb_signal_from_host (WTERMSIG (w)); - return ptid_of (event_child); + if (debug_threads) + { + debug_printf ("linux_wait_1 ret = %s, terminated with " + "signal %d\n", + target_pid_to_str (ptid_of (event_child)), + WTERMSIG (w)); + debug_exit (); + } } - } - else - { - if (!WIFSTOPPED (w)) - goto retry; + + return ptid_of (event_child); } /* If this event was not handled before, and is not a SIGTRAP, we @@ -2906,23 +3103,15 @@ mark_lwp_dead (struct lwp_info *lwp, int wstat) lwp->stop_expected = 0; } +/* Wait for all children to stop for the SIGSTOPs we just queued. */ + static void -wait_for_sigstop (struct inferior_list_entry *entry) +wait_for_sigstop (void) { - struct lwp_info *lwp = (struct lwp_info *) entry; struct thread_info *saved_inferior; - int wstat; ptid_t saved_tid; - ptid_t ptid; - int pid; - - if (lwp->stopped) - { - if (debug_threads) - debug_printf ("wait_for_sigstop: LWP %ld already stopped\n", - lwpid_of (lwp)); - return; - } + int wstat; + int ret; saved_inferior = current_inferior; if (saved_inferior != NULL) @@ -2930,50 +3119,15 @@ wait_for_sigstop (struct inferior_list_entry *entry) else saved_tid = null_ptid; /* avoid bogus unused warning */ - ptid = lwp->head.id; - if (debug_threads) - debug_printf ("wait_for_sigstop: pulling one event\n"); + debug_printf ("wait_for_sigstop: pulling events\n"); - pid = linux_wait_for_event (ptid, &wstat, __WALL); - - /* If we stopped with a non-SIGSTOP signal, save it for later - and record the pending SIGSTOP. If the process exited, just - return. */ - if (WIFSTOPPED (wstat)) - { - if (debug_threads) - debug_printf ("LWP %ld stopped with signal %d\n", - lwpid_of (lwp), WSTOPSIG (wstat)); - - if (WSTOPSIG (wstat) != SIGSTOP) - { - if (debug_threads) - debug_printf ("LWP %ld stopped with non-sigstop status %06x\n", - lwpid_of (lwp), wstat); - - lwp->status_pending_p = 1; - lwp->status_pending = wstat; - } - } - else - { - if (debug_threads) - debug_printf ("Process %d exited while stopping LWPs\n", pid); - - lwp = find_lwp_pid (pid_to_ptid (pid)); - if (lwp) - { - /* Leave this status pending for the next time we're able to - report it. In the mean time, we'll report this lwp as - dead to GDB, so GDB doesn't try to read registers and - memory from it. This can only happen if this was the - last thread of the process; otherwise, PID is removed - from the thread tables before linux_wait_for_event - returns. */ - mark_lwp_dead (lwp, wstat); - } - } + /* Passing NULL_PTID as filter indicates we want all events to be + left pending. Eventually this returns when there are no + unwaited-for children left. */ + ret = linux_wait_for_event_filtered (minus_one_ptid, null_ptid, + &wstat, __WALL); + gdb_assert (ret == -1); if (saved_inferior == NULL || linux_thread_alive (saved_tid)) current_inferior = saved_inferior; @@ -3099,7 +3253,7 @@ stop_all_lwps (int suspend, struct lwp_info *except) find_inferior (&all_lwps, suspend_and_send_sigstop_callback, except); else find_inferior (&all_lwps, send_sigstop_callback, except); - for_each_inferior (&all_lwps, wait_for_sigstop); + wait_for_sigstop (); stopping_threads = NOT_STOPPING_THREADS; if (debug_threads) diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c index 88354be..0f50afe 100644 --- a/gdb/gdbserver/server.c +++ b/gdb/gdbserver/server.c @@ -2321,8 +2321,18 @@ resume (struct thread_resume *actions, size_t num_actions) { last_ptid = mywait (minus_one_ptid, &last_status, 0, 1); + if (last_status.kind == TARGET_WAITKIND_NO_RESUMED) + { + /* No proper RSP support for this yet. At least return + error. */ + sprintf (own_buf, "E.No unwaited-for children left."); + disable_async_io (); + return; + } + if (last_status.kind != TARGET_WAITKIND_EXITED - && last_status.kind != TARGET_WAITKIND_SIGNALLED) + && last_status.kind != TARGET_WAITKIND_SIGNALLED + && last_status.kind != TARGET_WAITKIND_NO_RESUMED) current_inferior->last_status = last_status; /* From the client's perspective, all-stop mode always stops all @@ -3848,7 +3858,11 @@ handle_target_event (int err, gdb_client_data client_data) last_ptid = mywait (minus_one_ptid, &last_status, TARGET_WNOHANG, 1); - if (last_status.kind != TARGET_WAITKIND_IGNORE) + if (last_status.kind == TARGET_WAITKIND_NO_RESUMED) + { + /* No RSP support for this yet. */ + } + else if (last_status.kind != TARGET_WAITKIND_IGNORE) { int pid = ptid_get_pid (last_ptid); struct process_info *process = find_process_pid (pid); -- 1.7.11.7 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 5/5] Add TARGET_WAITKIND_NO_RESUMED support to the RSP. 2014-01-23 14:10 ` [PATCH 1/5] Move ptid_match to common/ptid.c Pedro Alves 2014-01-23 14:10 ` [PATCH 4/5] Teach gdbserver's linux backend about no unwaited-for children (TARGET_WAITDKIND_NO_RESUMED) Pedro Alves @ 2014-01-23 14:10 ` Pedro Alves 2014-01-23 14:10 ` [PATCH 3/5] Linux waitpid/__WALL emulation wrapper: If WNOHANG is set, don't touch sigprocmask Pedro Alves 2014-01-23 14:10 ` [PATCH 2/5] Move status_to_str to nat/linux-waitpid.c Pedro Alves 3 siblings, 0 replies; 12+ messages in thread From: Pedro Alves @ 2014-01-23 14:10 UTC (permalink / raw) To: gdb-patches; +Cc: teawater, luis_gustavo Makes gdb.threads/no-unwaited-for-left.exp pass. WIP. --- gdb/gdbserver/remote-utils.c | 3 +++ gdb/gdbserver/server.c | 30 ++++++++++++++++++------------ gdb/remote.c | 13 ++++++++++--- 3 files changed, 31 insertions(+), 15 deletions(-) diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c index 3b88995..f5acd12 100644 --- a/gdb/gdbserver/remote-utils.c +++ b/gdb/gdbserver/remote-utils.c @@ -1429,6 +1429,9 @@ prepare_resume_reply (char *buf, ptid_t ptid, else sprintf (buf, "X%02x", status->value.sig); break; + case TARGET_WAITKIND_NO_RESUMED: + sprintf (buf, "N"); + break; default: error ("unhandled waitkind"); break; diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c index 0f50afe..e779793 100644 --- a/gdb/gdbserver/server.c +++ b/gdb/gdbserver/server.c @@ -2321,7 +2321,8 @@ resume (struct thread_resume *actions, size_t num_actions) { last_ptid = mywait (minus_one_ptid, &last_status, 0, 1); - if (last_status.kind == TARGET_WAITKIND_NO_RESUMED) + if (last_status.kind == TARGET_WAITKIND_NO_RESUMED + && 0 /* XXX Check if GDB supports this. */) { /* No proper RSP support for this yet. At least return error. */ @@ -3847,6 +3848,18 @@ handle_serial_event (int err, gdb_client_data client_data) return 0; } +static void +push_stop_notification (ptid_t ptid, struct target_waitstatus *status) +{ + struct vstop_notif *vstop_notif + = xmalloc (sizeof (struct vstop_notif)); + + vstop_notif->status = *status; + vstop_notif->ptid = ptid; + /* Push Stop notification. */ + notif_push (¬if_stop, (struct notif_event *) vstop_notif); +} + /* Event-loop callback for target events. */ int @@ -3860,7 +3873,9 @@ handle_target_event (int err, gdb_client_data client_data) if (last_status.kind == TARGET_WAITKIND_NO_RESUMED) { - /* No RSP support for this yet. */ + if (gdb_connected () + && 1 /* XXX Check here if GDB supports for this stop reply. */) + push_stop_notification (null_ptid, &last_status); } else if (last_status.kind != TARGET_WAITKIND_IGNORE) { @@ -3915,16 +3930,7 @@ handle_target_event (int err, gdb_client_data client_data) target_pid_to_str (last_ptid)); } else - { - struct vstop_notif *vstop_notif - = xmalloc (sizeof (struct vstop_notif)); - - vstop_notif->status = last_status; - vstop_notif->ptid = last_ptid; - /* Push Stop notification. */ - notif_push (¬if_stop, - (struct notif_event *) vstop_notif); - } + push_stop_notification (last_ptid, &last_status); } /* Be sure to not change the selected inferior behind GDB's back. diff --git a/gdb/remote.c b/gdb/remote.c index d886929..94552a3 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -5838,6 +5838,10 @@ Packet: '%s'\n"), event->ptid = pid_to_ptid (pid); } break; + case 'N': + event->ws.kind = TARGET_WAITKIND_NO_RESUMED; + event->ptid = minus_one_ptid; + break; } if (non_stop && ptid_equal (event->ptid, null_ptid)) @@ -5939,7 +5943,8 @@ process_stop_reply (struct stop_reply *stop_reply, ptid = inferior_ptid; if (status->kind != TARGET_WAITKIND_EXITED - && status->kind != TARGET_WAITKIND_SIGNALLED) + && status->kind != TARGET_WAITKIND_SIGNALLED + && status->kind != TARGET_WAITKIND_NO_RESUMED) { struct remote_state *rs = get_remote_state (); @@ -6108,7 +6113,7 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options) remote_fileio_request (buf, rs->ctrlc_pending_p); rs->ctrlc_pending_p = 0; break; - case 'T': case 'S': case 'X': case 'W': + case 'N': case 'T': case 'S': case 'X': case 'W': { struct stop_reply *stop_reply = (struct stop_reply *) remote_notif_parse (¬if_client_stop, @@ -6152,7 +6157,9 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options) break; } - if (status->kind == TARGET_WAITKIND_IGNORE) + if (status->kind == TARGET_WAITKIND_NO_RESUMED) + return minus_one_ptid; + else if (status->kind == TARGET_WAITKIND_IGNORE) { /* Nothing interesting happened. If we're doing a non-blocking poll, we're done. Otherwise, go back to waiting. */ -- 1.7.11.7 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/5] Linux waitpid/__WALL emulation wrapper: If WNOHANG is set, don't touch sigprocmask. 2014-01-23 14:10 ` [PATCH 1/5] Move ptid_match to common/ptid.c Pedro Alves 2014-01-23 14:10 ` [PATCH 4/5] Teach gdbserver's linux backend about no unwaited-for children (TARGET_WAITDKIND_NO_RESUMED) Pedro Alves 2014-01-23 14:10 ` [PATCH 5/5] Add TARGET_WAITKIND_NO_RESUMED support to the RSP Pedro Alves @ 2014-01-23 14:10 ` Pedro Alves 2014-02-27 14:47 ` Pedro Alves 2014-01-23 14:10 ` [PATCH 2/5] Move status_to_str to nat/linux-waitpid.c Pedro Alves 3 siblings, 1 reply; 12+ messages in thread From: Pedro Alves @ 2014-01-23 14:10 UTC (permalink / raw) To: gdb-patches; +Cc: teawater, luis_gustavo So the caller can use waitpid WNOHANG+sigsuspend. Otherwise, my_waitpid lets the SIGCHLD handler run by mistake. --- gdb/nat/linux-waitpid.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/gdb/nat/linux-waitpid.c b/gdb/nat/linux-waitpid.c index e9e69db..4693120 100644 --- a/gdb/nat/linux-waitpid.c +++ b/gdb/nat/linux-waitpid.c @@ -92,15 +92,19 @@ my_waitpid (int pid, int *status, int flags) wnohang = (flags & WNOHANG) != 0; flags &= ~(__WALL | __WCLONE); - flags |= WNOHANG; - /* Block all signals while here. This avoids knowing about - LinuxThread's signals. */ - sigfillset (&block_mask); - sigprocmask (SIG_BLOCK, &block_mask, &org_mask); + if (!wnohang) + { + flags |= WNOHANG; + + /* Block all signals while here. This avoids knowing about + LinuxThread's signals. */ + sigfillset (&block_mask); + sigprocmask (SIG_BLOCK, &block_mask, &org_mask); - /* ... except during the sigsuspend below. */ - sigemptyset (&wake_mask); + /* ... except during the sigsuspend below. */ + sigemptyset (&wake_mask); + } while (1) { @@ -129,7 +133,8 @@ my_waitpid (int pid, int *status, int flags) flags ^= __WCLONE; } - sigprocmask (SIG_SETMASK, &org_mask, NULL); + if (!wnohang) + sigprocmask (SIG_SETMASK, &org_mask, NULL); } else { -- 1.7.11.7 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/5] Linux waitpid/__WALL emulation wrapper: If WNOHANG is set, don't touch sigprocmask. 2014-01-23 14:10 ` [PATCH 3/5] Linux waitpid/__WALL emulation wrapper: If WNOHANG is set, don't touch sigprocmask Pedro Alves @ 2014-02-27 14:47 ` Pedro Alves 0 siblings, 0 replies; 12+ messages in thread From: Pedro Alves @ 2014-02-27 14:47 UTC (permalink / raw) To: gdb-patches; +Cc: teawater, luis_gustavo On 01/23/2014 02:10 PM, Pedro Alves wrote: > So the caller can use waitpid WNOHANG+sigsuspend. Otherwise, > my_waitpid lets the SIGCHLD handler run by mistake. Sorry, that rationale was bogus. I've pushed this in, but as an optimization. --- [PATCH] Linux waitpid/__WALL emulation wrapper: If WNOHANG is set, don't touch sigprocmask. Just a small optimization. No need to block/unblock signals if we're not going to call sigsuspend. gdb/ 2014-02-27 Pedro Alves <palves@redhat.com> * nat/linux-waitpid.c (my_waitpid): Only block signals if WNOHANG isn't set. --- gdb/ChangeLog | 5 +++++ gdb/nat/linux-waitpid.c | 21 +++++++++++++-------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index bb5f5aa..6cfb9c5 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,10 @@ 2014-02-27 Pedro Alves <palves@redhat.com> + * nat/linux-waitpid.c (my_waitpid): Only block signals if WNOHANG + isn't set. + +2014-02-27 Pedro Alves <palves@redhat.com> + PR 12702 * linux-nat.c (status_to_str): Moved to nat/linux-waitpid.c. * nat/linux-waitpid.c: Include string.h. diff --git a/gdb/nat/linux-waitpid.c b/gdb/nat/linux-waitpid.c index e9e69db..4693120 100644 --- a/gdb/nat/linux-waitpid.c +++ b/gdb/nat/linux-waitpid.c @@ -92,15 +92,19 @@ my_waitpid (int pid, int *status, int flags) wnohang = (flags & WNOHANG) != 0; flags &= ~(__WALL | __WCLONE); - flags |= WNOHANG; - /* Block all signals while here. This avoids knowing about - LinuxThread's signals. */ - sigfillset (&block_mask); - sigprocmask (SIG_BLOCK, &block_mask, &org_mask); + if (!wnohang) + { + flags |= WNOHANG; + + /* Block all signals while here. This avoids knowing about + LinuxThread's signals. */ + sigfillset (&block_mask); + sigprocmask (SIG_BLOCK, &block_mask, &org_mask); - /* ... except during the sigsuspend below. */ - sigemptyset (&wake_mask); + /* ... except during the sigsuspend below. */ + sigemptyset (&wake_mask); + } while (1) { @@ -129,7 +133,8 @@ my_waitpid (int pid, int *status, int flags) flags ^= __WCLONE; } - sigprocmask (SIG_SETMASK, &org_mask, NULL); + if (!wnohang) + sigprocmask (SIG_SETMASK, &org_mask, NULL); } else { -- 1.7.11.7 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/5] Move status_to_str to nat/linux-waitpid.c. 2014-01-23 14:10 ` [PATCH 1/5] Move ptid_match to common/ptid.c Pedro Alves ` (2 preceding siblings ...) 2014-01-23 14:10 ` [PATCH 3/5] Linux waitpid/__WALL emulation wrapper: If WNOHANG is set, don't touch sigprocmask Pedro Alves @ 2014-01-23 14:10 ` Pedro Alves 3 siblings, 0 replies; 12+ messages in thread From: Pedro Alves @ 2014-01-23 14:10 UTC (permalink / raw) To: gdb-patches; +Cc: teawater, luis_gustavo So that gdbserver's backend can use it too. --- gdb/linux-nat.c | 25 ------------------------- gdb/nat/linux-waitpid.c | 28 ++++++++++++++++++++++++++++ gdb/nat/linux-waitpid.h | 4 ++++ 3 files changed, 32 insertions(+), 25 deletions(-) diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index d144c77..5961867 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -849,31 +849,6 @@ static int linux_thread_alive (ptid_t ptid); static char *linux_child_pid_to_exec_file (int pid); \f -/* Convert wait status STATUS to a string. Used for printing debug - messages only. */ - -static char * -status_to_str (int status) -{ - static char buf[64]; - - if (WIFSTOPPED (status)) - { - if (WSTOPSIG (status) == SYSCALL_SIGTRAP) - snprintf (buf, sizeof (buf), "%s (stopped at syscall)", - strsignal (SIGTRAP)); - else - snprintf (buf, sizeof (buf), "%s (stopped)", - strsignal (WSTOPSIG (status))); - } - else if (WIFSIGNALED (status)) - snprintf (buf, sizeof (buf), "%s (terminated)", - strsignal (WTERMSIG (status))); - else - snprintf (buf, sizeof (buf), "%d (exited)", WEXITSTATUS (status)); - - return buf; -} /* Destroy and free LP. */ diff --git a/gdb/nat/linux-waitpid.c b/gdb/nat/linux-waitpid.c index 433efe7..e9e69db 100644 --- a/gdb/nat/linux-waitpid.c +++ b/gdb/nat/linux-waitpid.c @@ -28,6 +28,8 @@ #include "nat/linux-waitpid.h" #include "gdb_wait.h" +#include <string.h> + /* Print debugging output based on the format string FORMAT and its parameters. */ @@ -47,6 +49,32 @@ linux_debug (const char *format, ...) #endif } +/* Convert wait status STATUS to a string. Used for printing debug + messages only. */ + +char * +status_to_str (int status) +{ + static char buf[64]; + + if (WIFSTOPPED (status)) + { + if (WSTOPSIG (status) == SYSCALL_SIGTRAP) + snprintf (buf, sizeof (buf), "%s (stopped at syscall)", + strsignal (SIGTRAP)); + else + snprintf (buf, sizeof (buf), "%s (stopped)", + strsignal (WSTOPSIG (status))); + } + else if (WIFSIGNALED (status)) + snprintf (buf, sizeof (buf), "%s (terminated)", + strsignal (WTERMSIG (status))); + else + snprintf (buf, sizeof (buf), "%d (exited)", WEXITSTATUS (status)); + + return buf; +} + /* Wrapper function for waitpid which handles EINTR, and emulates __WALL for systems where that is not available. */ diff --git a/gdb/nat/linux-waitpid.h b/gdb/nat/linux-waitpid.h index ae90e50..cac38db 100644 --- a/gdb/nat/linux-waitpid.h +++ b/gdb/nat/linux-waitpid.h @@ -24,4 +24,8 @@ __WALL for systems where that is not available. */ extern int my_waitpid (int pid, int *status, int flags); +/* Convert wait status STATUS to a string. Used for printing debug + messages only. */ +extern char *status_to_str (int status); + #endif /* LINUX_WAITPID_H */ -- 1.7.11.7 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Fix PR 12702 - gdb can hang waiting for thread group leader (gdbserver) 2014-01-23 14:51 ` Pedro Alves 2014-01-23 14:10 ` [PATCH 1/5] Move ptid_match to common/ptid.c Pedro Alves @ 2014-02-27 14:38 ` Pedro Alves 2014-02-27 14:54 ` Luis Machado 1 sibling, 1 reply; 12+ messages in thread From: Pedro Alves @ 2014-02-27 14:38 UTC (permalink / raw) To: Pedro Alves; +Cc: Hui Zhu, gdb-patches ml, Gustavo, Luis On 01/23/2014 02:00 PM, Pedro Alves wrote: > This is main gripe with this patch. I'd really like to avoid > bringing in more of this broken use of waitpid(PID) into gdbserver > (I realize most of this is copied from GDB), and this duplication > of low level wait status handling. I think we can do > better in gdbserver. There's really no need for wait_for_sigstop > to wait for each LWP in turn. I think that with some > refactoring, we can make it reuse linux_wait_for_event, and > only end up with having to handle pending statuses in one place. > > So I've been working on this most of this week, and I got this > almost done but I'm afraid I'll need to give attention to other > work, so I'm not sure I'll be able to be back at this before > February. Anyway, I'll post a WIP series in response to this email. I've now rebased this on current mainline, wrote ChangeLogs, etc. and pushed it. https://sourceware.org/ml/gdb-patches/2014-02/msg00826.html > In addition, it goes a step forward and starts adding support for > TARGET_WAITKIND_NO_RESUMED to the RSP as well, which is needed for > fully fixing no-unwaited-for-left.exp. I didn't get to finish that one yet, so I've left it out for now. -- Pedro Alves ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Fix PR 12702 - gdb can hang waiting for thread group leader (gdbserver) 2014-02-27 14:38 ` [PATCH] Fix PR 12702 - gdb can hang waiting for thread group leader (gdbserver) Pedro Alves @ 2014-02-27 14:54 ` Luis Machado 0 siblings, 0 replies; 12+ messages in thread From: Luis Machado @ 2014-02-27 14:54 UTC (permalink / raw) To: Pedro Alves; +Cc: Hui Zhu, gdb-patches ml On 02/27/2014 11:38 AM, Pedro Alves wrote: > On 01/23/2014 02:00 PM, Pedro Alves wrote: >> This is main gripe with this patch. I'd really like to avoid >> bringing in more of this broken use of waitpid(PID) into gdbserver >> (I realize most of this is copied from GDB), and this duplication >> of low level wait status handling. I think we can do >> better in gdbserver. There's really no need for wait_for_sigstop >> to wait for each LWP in turn. I think that with some >> refactoring, we can make it reuse linux_wait_for_event, and >> only end up with having to handle pending statuses in one place. >> >> So I've been working on this most of this week, and I got this >> almost done but I'm afraid I'll need to give attention to other >> work, so I'm not sure I'll be able to be back at this before >> February. Anyway, I'll post a WIP series in response to this email. > > I've now rebased this on current mainline, wrote ChangeLogs, etc. > and pushed it. > > https://sourceware.org/ml/gdb-patches/2014-02/msg00826.html > >> In addition, it goes a step forward and starts adding support for >> TARGET_WAITKIND_NO_RESUMED to the RSP as well, which is needed for >> fully fixing no-unwaited-for-left.exp. > > I didn't get to finish that one yet, so I've left it out for now. > Thanks Pedro. That is a nice fix to have. Luis ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-02-27 14:54 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-07-20 10:00 [PATCH] Fix PR 12702 - gdb can hang waiting for thread group leader (gdbserver) Hui Zhu 2013-07-24 18:25 ` Pedro Alves 2013-11-19 6:19 ` Hui Zhu 2014-01-23 14:51 ` Pedro Alves 2014-01-23 14:10 ` [PATCH 1/5] Move ptid_match to common/ptid.c Pedro Alves 2014-01-23 14:10 ` [PATCH 4/5] Teach gdbserver's linux backend about no unwaited-for children (TARGET_WAITDKIND_NO_RESUMED) Pedro Alves 2014-01-23 14:10 ` [PATCH 5/5] Add TARGET_WAITKIND_NO_RESUMED support to the RSP Pedro Alves 2014-01-23 14:10 ` [PATCH 3/5] Linux waitpid/__WALL emulation wrapper: If WNOHANG is set, don't touch sigprocmask Pedro Alves 2014-02-27 14:47 ` Pedro Alves 2014-01-23 14:10 ` [PATCH 2/5] Move status_to_str to nat/linux-waitpid.c Pedro Alves 2014-02-27 14:38 ` [PATCH] Fix PR 12702 - gdb can hang waiting for thread group leader (gdbserver) Pedro Alves 2014-02-27 14:54 ` Luis Machado
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox