* [PATCH/7.10 0/2] gdbserver: Fix several fork support (& co) issues
@ 2015-07-31 17:03 Pedro Alves
2015-07-31 17:04 ` [PATCH/7.10 1/2] Linux gdbserver confused when event randomization returns a process exit event Pedro Alves
2015-07-31 17:04 ` [PATCH/7.10 2/2] gdbserver: Fix non-stop / fork / step-over issues Pedro Alves
0 siblings, 2 replies; 16+ messages in thread
From: Pedro Alves @ 2015-07-31 17:03 UTC (permalink / raw)
To: gdb-patches
In the "[PATCH 3/N] remote follow fork and spurious child stops in
non-stop mode" thread:
https://sourceware.org/ml/gdb-patches/2015-07/msg00868.html
several issues were identified in the new gdbserver follow fork
support in 7.10. This addresses all I found. I'm thinking these are
7.10 worthy.
The test added exposes issues in gdb's native target, but I won't have
time to address them myself right now, so the test is kfailed there.
Pedro Alves (2):
Linux gdbserver confused when event randomization returns a process
exit event
gdbserver: Fix non-stop / fork / step-over issues
gdb/gdbserver/linux-low.c | 335 +++++++++++++++++-----
gdb/gdbserver/linux-low.h | 11 +-
gdb/target/waitstatus.h | 5 +-
gdb/testsuite/gdb.threads/fork-plus-threads-2.c | 129 +++++++++
gdb/testsuite/gdb.threads/fork-plus-threads-2.exp | 116 ++++++++
5 files changed, 523 insertions(+), 73 deletions(-)
create mode 100644 gdb/testsuite/gdb.threads/fork-plus-threads-2.c
create mode 100644 gdb/testsuite/gdb.threads/fork-plus-threads-2.exp
--
1.9.3
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH/7.10 1/2] Linux gdbserver confused when event randomization returns a process exit event
2015-07-31 17:03 [PATCH/7.10 0/2] gdbserver: Fix several fork support (& co) issues Pedro Alves
@ 2015-07-31 17:04 ` Pedro Alves
2015-08-03 10:59 ` Yao Qi
2015-07-31 17:04 ` [PATCH/7.10 2/2] gdbserver: Fix non-stop / fork / step-over issues Pedro Alves
1 sibling, 1 reply; 16+ messages in thread
From: Pedro Alves @ 2015-07-31 17:04 UTC (permalink / raw)
To: gdb-patches
The tail end of linux_wait_1 isn't expecting that the select_event_lwp
machinery can pick a whole-process exit event to report to GDB. When
that happens, both gdb and gdbserver end up quite confused:
...
(gdb)
[Thread 24971.24971] #1 stopped.
0x0000003615a011f0 in ?? ()
c&
Continuing.
(gdb) [New Thread 24971.24981]
[New Thread 24983.24983]
[New Thread 24971.24982]
[Thread 24983.24983] #3 stopped.
0x0000003615ebc7cc in __libc_fork () at ../nptl/sysdeps/unix/sysv/linux/fork.c:130
130 pid = ARCH_FORK ();
[New Thread 24984.24984]
Error in re-setting breakpoint -16: PC register is not available
Error in re-setting breakpoint -17: PC register is not available
Error in re-setting breakpoint -18: PC register is not available
Error in re-setting breakpoint -19: PC register is not available
Error in re-setting breakpoint -24: PC register is not available
Error in re-setting breakpoint -25: PC register is not available
Error in re-setting breakpoint -26: PC register is not available
Error in re-setting breakpoint -27: PC register is not available
Error in re-setting breakpoint -28: PC register is not available
Error in re-setting breakpoint -29: PC register is not available
Error in re-setting breakpoint -30: PC register is not available
PC register is not available
(gdb)
gdb/testsuite/ChangeLog:
2015-07-31 Pedro Alves <palves@redhat.com>
* linux-low.c (add_lwp): Set waitstatus to TARGET_WAITKIND_IGNORE.
(linux_thread_alive): Use lwp_is_marked_dead.
(extended_event_reported): Delete.
(linux_wait_1): Check if waitstatus is TARGET_WAITKIND_IGNORE
instead of extended_event_reported. Pass event_child->waitstatus
to target_waitstatus_to_string instead of ourstatus.
(mark_lwp_dead): Don't set the 'dead' flag. Store the waitstatus
as well.
(lwp_is_marked_dead): New function.
(lwp_running): Use lwp_is_marked_dead.
* linux-low.h: Delete 'dead' field, and update 'waitstatus's
comment.
---
gdb/gdbserver/linux-low.c | 68 +++++++++++++++++++++++++++--------------------
gdb/gdbserver/linux-low.h | 11 +++-----
2 files changed, 43 insertions(+), 36 deletions(-)
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 82fb7f9..4cdedb4 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -264,6 +264,7 @@ static int linux_wait_for_event (ptid_t ptid, int *wstat, int options);
static struct lwp_info *add_lwp (ptid_t ptid);
static int linux_stopped_by_watchpoint (void);
static void mark_lwp_dead (struct lwp_info *lwp, int wstat);
+static int lwp_is_marked_dead (struct lwp_info *lwp);
static void proceed_all_lwps (void);
static int finish_step_over (struct lwp_info *lwp);
static int kill_lwp (unsigned long lwpid, int signo);
@@ -743,8 +744,9 @@ add_lwp (ptid_t ptid)
{
struct lwp_info *lwp;
- lwp = (struct lwp_info *) xmalloc (sizeof (*lwp));
- memset (lwp, 0, sizeof (*lwp));
+ lwp = (struct lwp_info *) xcalloc (1, sizeof (*lwp));
+
+ lwp->waitstatus.kind = TARGET_WAITKIND_IGNORE;
if (the_low_target.new_thread != NULL)
the_low_target.new_thread (lwp);
@@ -1393,7 +1395,7 @@ linux_thread_alive (ptid_t ptid)
exited but we still haven't been able to report it to GDB, we'll
hold on to the last lwp of the dead process. */
if (lwp != NULL)
- return !lwp->dead;
+ return !lwp_is_marked_dead (lwp);
else
return 0;
}
@@ -2747,20 +2749,6 @@ ignore_event (struct target_waitstatus *ourstatus)
return null_ptid;
}
-/* Return non-zero if WAITSTATUS reflects an extended linux
- event. Otherwise, return zero. */
-
-static int
-extended_event_reported (const struct target_waitstatus *waitstatus)
-{
- if (waitstatus == NULL)
- return 0;
-
- return (waitstatus->kind == TARGET_WAITKIND_FORKED
- || waitstatus->kind == TARGET_WAITKIND_VFORKED
- || waitstatus->kind == TARGET_WAITKIND_VFORK_DONE);
-}
-
/* Wait for process, returns status. */
static ptid_t
@@ -3128,7 +3116,7 @@ linux_wait_1 (ptid_t ptid,
|| (gdb_breakpoint_here (event_child->stop_pc)
&& gdb_condition_true_at_breakpoint (event_child->stop_pc)
&& gdb_no_commands_at_breakpoint (event_child->stop_pc))
- || extended_event_reported (&event_child->waitstatus));
+ || event_child->waitstatus.kind != TARGET_WAITKIND_IGNORE);
run_breakpoint_commands (event_child->stop_pc);
@@ -3150,9 +3138,11 @@ linux_wait_1 (ptid_t ptid,
paddress (event_child->stop_pc),
paddress (event_child->step_range_start),
paddress (event_child->step_range_end));
- if (extended_event_reported (&event_child->waitstatus))
+ if (event_child->waitstatus.kind != TARGET_WAITKIND_IGNORE)
{
- char *str = target_waitstatus_to_string (ourstatus);
+ char *str;
+
+ str = target_waitstatus_to_string (&event_child->waitstatus);
debug_printf ("LWP %ld: extended event with waitstatus %s\n",
lwpid_of (get_lwp_thread (event_child)), str);
xfree (str);
@@ -3266,12 +3256,11 @@ linux_wait_1 (ptid_t ptid,
unstop_all_lwps (1, event_child);
}
- if (extended_event_reported (&event_child->waitstatus))
+ if (event_child->waitstatus.kind != TARGET_WAITKIND_IGNORE)
{
- /* If the reported event is a fork, vfork or exec, let GDB know. */
- ourstatus->kind = event_child->waitstatus.kind;
- ourstatus->value = event_child->waitstatus.value;
-
+ /* If the reported event is an exit, fork, vfork or exec, let
+ GDB know. */
+ *ourstatus = event_child->waitstatus;
/* Clear the event lwp's waitstatus since we handled it already. */
event_child->waitstatus.kind = TARGET_WAITKIND_IGNORE;
}
@@ -3479,13 +3468,23 @@ suspend_and_send_sigstop_callback (struct inferior_list_entry *entry,
static void
mark_lwp_dead (struct lwp_info *lwp, int wstat)
{
- /* It's dead, really. */
- lwp->dead = 1;
-
/* Store the exit status for later. */
lwp->status_pending_p = 1;
lwp->status_pending = wstat;
+ /* Store in waitstatus as well, as there's nothing else to process
+ for this event. */
+ if (WIFEXITED (wstat))
+ {
+ lwp->waitstatus.kind = TARGET_WAITKIND_EXITED;
+ lwp->waitstatus.value.integer = WEXITSTATUS (wstat);
+ }
+ else if (WIFSIGNALED (wstat))
+ {
+ lwp->waitstatus.kind = TARGET_WAITKIND_SIGNALLED;
+ lwp->waitstatus.value.sig = gdb_signal_from_host (WTERMSIG (wstat));
+ }
+
/* Prevent trying to stop it. */
lwp->stopped = 1;
@@ -3493,6 +3492,17 @@ mark_lwp_dead (struct lwp_info *lwp, int wstat)
lwp->stop_expected = 0;
}
+/* Return true if LWP has exited already, and has a pending exit event
+ to report to GDB. */
+
+static int
+lwp_is_marked_dead (struct lwp_info *lwp)
+{
+ return (lwp->status_pending_p
+ && (WIFEXITED (lwp->status_pending)
+ || WIFSIGNALED (lwp->status_pending)));
+}
+
/* Wait for all children to stop for the SIGSTOPs we just queued. */
static void
@@ -3609,7 +3619,7 @@ lwp_running (struct inferior_list_entry *entry, void *data)
struct thread_info *thread = (struct thread_info *) entry;
struct lwp_info *lwp = get_thread_lwp (thread);
- if (lwp->dead)
+ if (lwp_is_marked_dead (lwp))
return 0;
if (lwp->stopped)
return 0;
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 5a3697b..4afd6ef 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -261,16 +261,13 @@ struct lwp_info
event already received in a wait()). */
int stopped;
- /* 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). */
- int dead;
-
/* When stopped is set, the last wait status recorded for this lwp. */
int last_status;
- /* This is used to store extended ptrace event information until
- it is reported to GDB. */
+ /* If WAITSTATUS->KIND != TARGET_WAITKIND_IGNORE, the waitstatus for
+ this LWP's last event, to pass to GDB without any further
+ processing. This is used to store extended ptrace event
+ information or exit status until it can be reported to GDB. */
struct target_waitstatus waitstatus;
/* When stopped is set, this is where the lwp last stopped, with
--
1.9.3
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH/7.10 2/2] gdbserver: Fix non-stop / fork / step-over issues
2015-07-31 17:03 [PATCH/7.10 0/2] gdbserver: Fix several fork support (& co) issues Pedro Alves
2015-07-31 17:04 ` [PATCH/7.10 1/2] Linux gdbserver confused when event randomization returns a process exit event Pedro Alves
@ 2015-07-31 17:04 ` Pedro Alves
2015-07-31 18:04 ` Don Breazeal
2015-08-03 15:14 ` Yao Qi
1 sibling, 2 replies; 16+ messages in thread
From: Pedro Alves @ 2015-07-31 17:04 UTC (permalink / raw)
To: gdb-patches
Ref: https://sourceware.org/ml/gdb-patches/2015-07/msg00868.html
This adds a test that has a multithreaded program have several threads
continuously fork, while another thread continuously steps over a
breakpoint.
This exposes several intertwined issues, which this patch addresses:
- When we're stopping and suspending threads, some thread may fork,
and we missed setting its suspend count to 1, like we do when a new
clone/thread is detected. When we next unsuspend threads, the fork
child's suspend count goes below 0, which is bogus and fails an
assertion.
- If a step-over is cancelled because a signal arrives, but then gdb
is not interested in the signal, we pass the signal straight back
to the inferior. However, we miss that we need to re-increment the
suspend counts of all other threads that had been paused for the
step-over. As a result, other threads indefinitely end up stuck
stopped.
- OTOH, if a thread exits the whole process just while we're stopping
threads to start a step-over, gdbserver crashes or hits several
assertions.
- If a detach request comes in just while gdbserver is handling a
step-over (in the test at hand, this is GDB detaching the fork
child), gdbserver internal errors in stabilize_thread's helpers,
which assert that all thread's suspend counts are 0 (otherwise we
wouldn't be able to move threads out of the jump pads). The
suspend counts aren't 0 while a step-over is in progress, because
all threads but the one stepping past the breakpoint must remain
paused until the step-over finishes and the breakpoint can be
reinserted.
- Occasionally, we see "BAD - reinserting but not stepping." being
output (from within linux_resume_one_lwp_throw). That was because
GDB pokes memory while gdbserver is busy with a step-over, and that
suspends threads, and then re-resumes them with proceed_one_lwp,
which missed another reason to tell linux_resume_one_lwp that the
thread should be set back to stepping.
- In a couple places, we were resuming threads that are meant to be
suspended. E.g., when a vCont;c/s request for thread B comes in
just while gdbserver is stepping thread A past a breakpoint. The
resume for thread B must be deferred until the step-over finishes.
- The test runs with both "set detach-on-fork" on and off. When off,
it exercises the case of GDB detaching the fork child explicitly.
When on, it exercises the case of gdb resuming the child
explicitly. In the "off" case, gdb seems to exponentially become
slower as new inferiors are created. This is _very_ noticeable as
with only 100 inferiors gdb is crawling already, which makes the
test take quite a bit to run. For that reason, I've disabled the
"off" variant for now.
- The test fails occasionally with the native target, because several
code paths aren't expecting that a stopped thread may disappear
(the test has the leader thread of the parent process exit the
whole process, just while gdb is handling an event for a non-leader
thread). E.g.,:
[Thread 0x7ffff67bd700 (LWP 11210) exited]
Cannot find user-level thread for LWP 11217: generic error
(gdb) FAIL: gdb.threads/fork-plus-threads-2.exp: detach-on-fork=on: inferior 1 exited
[Thread 0x7ffff7fc1740 (LWP 11203) exited]
info threads
Id Target Id Frame
12 Thread 0x7ffff2fb6700 (LWP 11217) (running)
The current thread <Thread ID 1> has terminated. See `help thread'.
(gdb) FAIL: gdb.threads/fork-plus-threads-2.exp: detach-on-fork=on: no threads left
I fixed some of these issues recently, but there's a lot more to
do. Fixing that one just exposes other similar problems elsewhere.
Meanwhile, I've filed PR18749 and kfailed the test for native.
gdb/ChangeLog:
2015-07-31 Pedro Alves <palves@redhat.com>
PR gdb/18749
* target/waitstatus.h (enum target_stop_reason)
<TARGET_STOPPED_BY_SINGLE_STEP>: New value.
gdb/gdbserver/ChangeLog:
2015-07-31 Pedro Alves <palves@redhat.com>
PR gdb/18749
* linux-low.c (handle_extended_wait): Set the fork child's suspend
count if stopping and suspending threads.
(check_stopped_by_breakpoint): If stopped by trace, set the LWP's
stop reason to TARGET_STOPPED_BY_SINGLE_STEP.
(linux_detach): Complete an ongoing step-over.
(lwp_suspended_inc, lwp_suspended_decr): New functions. Use
throughout.
(resume_stopped_resumed_lwps): Don't resume a suspended thread.
(linux_wait_1): If passing a signal to the inferior after
finishing a step-over, unsuspend and re-resume all lwps. If we
see a single-step event but the thread should be continuing, don't
pass the trap to gdb.
(stuck_in_jump_pad_callback, move_out_of_jump_pad_callback): Use
internal_error instead of gdb_assert.
(enqueue_pending_signal): New function.
(check_ptrace_stopped_lwp_gone): Add debug output.
(start_step_over): Handle the case of the LWP we're about to
step-over exiting. Use internal_error instead of gdb_assert.
(complete_ongoing_step_over): New function.
(linux_resume_one_thread): Don't resume a suspended thread.
(proceed_one_lwp): If the LWP is stepping over a breakpoint, reset
it stepping.
(proceed_all_lwps): If a step-over fails to start, look for
another thread that might need a step-over.
gdb/testsuite/ChangeLog:
2015-07-31 Pedro Alves <palves@redhat.com>
PR gdb/18749
* gdb.threads/fork-plus-threads-2.exp: New file.
* gdb.threads/fork-plus-threads-2.c: New file.
---
gdb/gdbserver/linux-low.c | 267 +++++++++++++++++++---
gdb/target/waitstatus.h | 5 +-
gdb/testsuite/gdb.threads/fork-plus-threads-2.c | 129 +++++++++++
gdb/testsuite/gdb.threads/fork-plus-threads-2.exp | 116 ++++++++++
4 files changed, 480 insertions(+), 37 deletions(-)
create mode 100644 gdb/testsuite/gdb.threads/fork-plus-threads-2.c
create mode 100644 gdb/testsuite/gdb.threads/fork-plus-threads-2.exp
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 4cdedb4..66d57ec 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -268,6 +268,8 @@ static int lwp_is_marked_dead (struct lwp_info *lwp);
static void proceed_all_lwps (void);
static int finish_step_over (struct lwp_info *lwp);
static int kill_lwp (unsigned long lwpid, int signo);
+static void enqueue_pending_signal (struct lwp_info *lwp, int signal, siginfo_t *info);
+static void complete_ongoing_step_over (void);
/* When the event-loop is doing a step-over, this points at the thread
being stepped. */
@@ -486,6 +488,15 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat)
child_thr->last_resume_kind = resume_stop;
child_thr->last_status.kind = TARGET_WAITKIND_STOPPED;
+ /* If we're suspending all threads, leave this one suspended
+ too. */
+ if (stopping_threads == STOPPING_AND_SUSPENDING_THREADS)
+ {
+ if (debug_threads)
+ debug_printf ("HEW: leaving child suspended\n");
+ child_lwp->suspended = 1;
+ }
+
parent_proc = get_thread_process (event_thr);
child_proc->attached = parent_proc->attached;
clone_all_breakpoints (&child_proc->breakpoints,
@@ -685,6 +696,8 @@ check_stopped_by_breakpoint (struct lwp_info *lwp)
debug_printf ("CSBB: %s stopped by trace\n",
target_pid_to_str (ptid_of (thr)));
}
+
+ lwp->stop_reason = TARGET_STOPPED_BY_SINGLE_STEP;
}
}
}
@@ -1315,6 +1328,11 @@ linux_detach (int pid)
if (process == NULL)
return -1;
+ /* As there's a step over already in progress, let it finish first,
+ otherwise nesting a stabilize_threads operation on top gets real
+ messy. */
+ complete_ongoing_step_over ();
+
/* Stop all threads before detaching. First, ptrace requires that
the thread is stopped to sucessfully detach. Second, thread_db
may need to uninstall thread event breakpoints from memory, which
@@ -1683,6 +1701,39 @@ not_stopped_callback (struct inferior_list_entry *entry, void *arg)
return 0;
}
+/* Increment LWP's suspend count. */
+
+static void
+lwp_suspended_inc (struct lwp_info *lwp)
+{
+ lwp->suspended++;
+
+ if (debug_threads && lwp->suspended > 4)
+ {
+ struct thread_info *thread = get_lwp_thread (lwp);
+
+ debug_printf ("LWP %ld has a suspiciously high suspend count,"
+ " suspended=%d\n", lwpid_of (thread), lwp->suspended);
+ }
+}
+
+/* Decrement LWP's suspend count. */
+
+static void
+lwp_suspended_decr (struct lwp_info *lwp)
+{
+ lwp->suspended--;
+
+ if (lwp->suspended < 0)
+ {
+ struct thread_info *thread = get_lwp_thread (lwp);
+
+ internal_error (__FILE__, __LINE__,
+ "unsuspend LWP %ld, suspended=%d\n", lwpid_of (thread),
+ lwp->suspended);
+ }
+}
+
/* This function should only be called if the LWP got a SIGTRAP.
Handle any tracepoint steps or hits. Return true if a tracepoint
@@ -1700,7 +1751,7 @@ handle_tracepoints (struct lwp_info *lwp)
uninsert tracepoints. To do this, we temporarily pause all
threads, unpatch away, and then unpause threads. We need to make
sure the unpausing doesn't resume LWP too. */
- lwp->suspended++;
+ lwp_suspended_inc (lwp);
/* And we need to be sure that any all-threads-stopping doesn't try
to move threads out of the jump pads, as it could deadlock the
@@ -1716,7 +1767,7 @@ handle_tracepoints (struct lwp_info *lwp)
actions. */
tpoint_related_event |= tracepoint_was_hit (tinfo, lwp->stop_pc);
- lwp->suspended--;
+ lwp_suspended_decr (lwp);
gdb_assert (lwp->suspended == 0);
gdb_assert (!stabilizing_threads || lwp->collecting_fast_tracepoint);
@@ -2176,10 +2227,13 @@ linux_low_filter_event (int lwpid, int wstat)
/* Note that TRAP_HWBKPT can indicate either a hardware breakpoint
or hardware watchpoint. Check which is which if we got
- TARGET_STOPPED_BY_HW_BREAKPOINT. */
+ TARGET_STOPPED_BY_HW_BREAKPOINT. Likewise, we may have single
+ stepped an instruction that triggered a watchpoint. In that
+ case, on some architectures (such as x86), instead of
+ TRAP_HWBKPT, si_code indicates TRAP_TRACE, and we need to check
+ the debug registers separately. */
if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SIGTRAP
- && (child->stop_reason == TARGET_STOPPED_BY_NO_REASON
- || child->stop_reason == TARGET_STOPPED_BY_HW_BREAKPOINT))
+ && child->stop_reason != TARGET_STOPPED_BY_SW_BREAKPOINT)
check_stopped_by_watchpoint (child);
if (!have_stop_pc)
@@ -2238,6 +2292,7 @@ resume_stopped_resumed_lwps (struct inferior_list_entry *entry)
struct lwp_info *lp = get_thread_lwp (thread);
if (lp->stopped
+ && !lp->suspended
&& !lp->status_pending_p
&& thread->last_resume_kind != resume_stop
&& thread->last_status.kind == TARGET_WAITKIND_IGNORE)
@@ -2608,9 +2663,7 @@ unsuspend_one_lwp (struct inferior_list_entry *entry, void *except)
if (lwp == except)
return 0;
- lwp->suspended--;
-
- gdb_assert (lwp->suspended >= 0);
+ lwp_suspended_decr (lwp);
return 0;
}
@@ -2703,7 +2756,7 @@ linux_stabilize_threads (void)
lwp = get_thread_lwp (current_thread);
/* Lock it. */
- lwp->suspended++;
+ lwp_suspended_inc (lwp);
if (ourstatus.value.sig != GDB_SIGNAL_0
|| current_thread->last_resume_kind == resume_stop)
@@ -3089,8 +3142,25 @@ linux_wait_1 (ptid_t ptid,
info_p = &info;
else
info_p = NULL;
- linux_resume_one_lwp (event_child, event_child->stepping,
- WSTOPSIG (w), info_p);
+
+ if (step_over_finished)
+ {
+ /* We cancelled this thread's step-over above. We still
+ need to unsuspend all other LWPs, and set then back
+ running again while the signal handler runs. */
+ unsuspend_all_lwps (event_child);
+
+ /* Enqueue the pending signal info so that proceed_all_lwps
+ doesn't lose it. */
+ enqueue_pending_signal (event_child, WSTOPSIG (w), info_p);
+
+ proceed_all_lwps ();
+ }
+ else
+ {
+ linux_resume_one_lwp (event_child, event_child->stepping,
+ WSTOPSIG (w), info_p);
+ }
return ignore_event (ourstatus);
}
@@ -3111,8 +3181,15 @@ linux_wait_1 (ptid_t ptid,
|| (current_thread->last_resume_kind == resume_step
&& !in_step_range)
|| event_child->stop_reason == TARGET_STOPPED_BY_WATCHPOINT
- || (!step_over_finished && !in_step_range
- && !bp_explains_trap && !trace_event)
+ || (!in_step_range
+ && !bp_explains_trap
+ && !trace_event
+ /* A step-over was finished just now? */
+ && !step_over_finished
+ /* A step-over had been finished previously,
+ and the single-step was left pending? */
+ && !(current_thread->last_resume_kind == resume_continue
+ && event_child->stop_reason == TARGET_STOPPED_BY_SINGLE_STEP))
|| (gdb_breakpoint_here (event_child->stop_pc)
&& gdb_condition_true_at_breakpoint (event_child->stop_pc)
&& gdb_no_commands_at_breakpoint (event_child->stop_pc))
@@ -3460,7 +3537,7 @@ suspend_and_send_sigstop_callback (struct inferior_list_entry *entry,
if (lwp == except)
return 0;
- lwp->suspended++;
+ lwp_suspended_inc (lwp);
return send_sigstop_callback (entry, except);
}
@@ -3562,7 +3639,12 @@ stuck_in_jump_pad_callback (struct inferior_list_entry *entry, void *data)
struct thread_info *thread = (struct thread_info *) entry;
struct lwp_info *lwp = get_thread_lwp (thread);
- gdb_assert (lwp->suspended == 0);
+ if (lwp->suspended != 0)
+ {
+ internal_error (__FILE__, __LINE__,
+ "LWP %ld is suspended, suspended=%d\n",
+ lwpid_of (thread), lwp->suspended);
+ }
gdb_assert (lwp->stopped);
/* Allow debugging the jump pad, gdb_collect, etc.. */
@@ -3581,7 +3663,12 @@ move_out_of_jump_pad_callback (struct inferior_list_entry *entry)
struct lwp_info *lwp = get_thread_lwp (thread);
int *wstat;
- gdb_assert (lwp->suspended == 0);
+ if (lwp->suspended != 0)
+ {
+ internal_error (__FILE__, __LINE__,
+ "LWP %ld is suspended, suspended=%d\n",
+ lwpid_of (thread), lwp->suspended);
+ }
gdb_assert (lwp->stopped);
wstat = lwp->status_pending_p ? &lwp->status_pending : NULL;
@@ -3610,7 +3697,7 @@ move_out_of_jump_pad_callback (struct inferior_list_entry *entry)
linux_resume_one_lwp (lwp, 0, 0, NULL);
}
else
- lwp->suspended++;
+ lwp_suspended_inc (lwp);
}
static int
@@ -3665,6 +3752,24 @@ stop_all_lwps (int suspend, struct lwp_info *except)
}
}
+/* Enqueue one signal in the chain of signals which need to be
+ delivered to this process on next resume. */
+
+static void
+enqueue_pending_signal (struct lwp_info *lwp, int signal, siginfo_t *info)
+{
+ struct pending_signals *p_sig;
+
+ p_sig = xmalloc (sizeof (*p_sig));
+ p_sig->prev = lwp->pending_signals;
+ p_sig->signal = signal;
+ if (info == NULL)
+ memset (&p_sig->info, 0, sizeof (siginfo_t));
+ else
+ memcpy (&p_sig->info, info, sizeof (siginfo_t));
+ lwp->pending_signals = p_sig;
+}
+
/* Resume execution of LWP. If STEP is nonzero, single-step it. If
SIGNAL is nonzero, give it that signal. */
@@ -3906,6 +4011,10 @@ check_ptrace_stopped_lwp_gone (struct lwp_info *lp)
/* Don't assume anything if /proc/PID/status can't be read. */
if (linux_proc_pid_is_trace_stopped_nowarn (lwpid_of (thread)) == 0)
{
+ if (debug_threads)
+ debug_printf ("lwp %ld exited while being resumed\n",
+ lwpid_of (thread));
+
lp->stop_reason = TARGET_STOPPED_BY_NO_REASON;
lp->status_pending_p = 0;
return 1;
@@ -4189,16 +4298,36 @@ static int
start_step_over (struct lwp_info *lwp)
{
struct thread_info *thread = get_lwp_thread (lwp);
+ ptid_t thread_ptid;
struct thread_info *saved_thread;
CORE_ADDR pc;
int step;
+ thread_ptid = ptid_of (thread);
+
if (debug_threads)
debug_printf ("Starting step-over on LWP %ld. Stopping all threads\n",
lwpid_of (thread));
stop_all_lwps (1, lwp);
- gdb_assert (lwp->suspended == 0);
+
+ /* Re-find the LWP as it may have exited. */
+ lwp = find_lwp_pid (thread_ptid);
+ if (lwp == NULL || lwp_is_marked_dead (lwp))
+ {
+ if (debug_threads)
+ debug_printf ("Step-over thread died "
+ "(another thread exited the process?).\n");
+ unstop_all_lwps (1, lwp);
+ return 0;
+ }
+
+ if (lwp->suspended != 0)
+ {
+ internal_error (__FILE__, __LINE__,
+ "LWP %ld suspended=%d\n", lwpid_of (thread),
+ lwp->suspended);
+ }
if (debug_threads)
debug_printf ("Done stopping all threads for step-over.\n");
@@ -4229,7 +4358,19 @@ start_step_over (struct lwp_info *lwp)
current_thread = saved_thread;
- linux_resume_one_lwp (lwp, step, 0, NULL);
+ TRY
+ {
+ linux_resume_one_lwp_throw (lwp, step, 0, NULL);
+ }
+ CATCH (ex, RETURN_MASK_ERROR)
+ {
+ unstop_all_lwps (1, lwp);
+
+ if (!check_ptrace_stopped_lwp_gone (lwp))
+ throw_exception (ex);
+ return 0;
+ }
+ END_CATCH
/* Require next event from this LWP. */
step_over_bkpt = thread->entry.id;
@@ -4270,6 +4411,39 @@ finish_step_over (struct lwp_info *lwp)
return 0;
}
+/* If there's a step over in progress, wait until all threads stop
+ (that is, until the stepping thread finishes its step), and
+ unsuspend all lwps. The stepping thread ends with its status
+ pending, which is processed later when we get back to processing
+ events. */
+
+static void
+complete_ongoing_step_over (void)
+{
+ if (!ptid_equal (step_over_bkpt, null_ptid))
+ {
+ struct lwp_info *lwp;
+ int wstat;
+ int ret;
+
+ if (debug_threads)
+ debug_printf ("detach: step over in progress, finish it first\n");
+
+ /* 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);
+
+ lwp = find_lwp_pid (step_over_bkpt);
+ if (lwp != NULL)
+ finish_step_over (lwp);
+ step_over_bkpt = null_ptid;
+ unsuspend_all_lwps (lwp);
+ }
+}
+
/* This function is called once per thread. We check the thread's resume
request, which will tell us whether to resume, step, or leave the thread
stopped; and what signal, if any, it should be sent.
@@ -4344,13 +4518,16 @@ linux_resume_one_thread (struct inferior_list_entry *entry, void *arg)
}
/* If this thread which is about to be resumed has a pending status,
- then don't resume any threads - we can just report the pending
- status. Make sure to queue any signals that would otherwise be
- sent. In all-stop mode, we do this decision based on if *any*
- thread has a pending status. If there's a thread that needs the
- step-over-breakpoint dance, then don't resume any other thread
- but that particular one. */
- leave_pending = (lwp->status_pending_p || leave_all_stopped);
+ then don't resume it - we can just report the pending status.
+ Likewise if it is suspended, because e.g., another thread is
+ stepping past a breakpoint. Make sure to queue any signals that
+ would otherwise be sent. In all-stop mode, we do this decision
+ based on if *any* thread has a pending status. If there's a
+ thread that needs the step-over-breakpoint dance, then don't
+ resume any other thread but that particular one. */
+ leave_pending = (lwp->suspended
+ || lwp->status_pending_p
+ || leave_all_stopped);
if (!leave_pending)
{
@@ -4533,7 +4710,23 @@ proceed_one_lwp (struct inferior_list_entry *entry, void *except)
send_sigstop (lwp);
}
- step = thread->last_resume_kind == resume_step;
+ if (thread->last_resume_kind == resume_step)
+ {
+ if (debug_threads)
+ debug_printf (" stepping LWP %ld, client wants it stepping\n",
+ lwpid_of (thread));
+ step = 1;
+ }
+ else if (lwp->bp_reinsert != 0)
+ {
+ if (debug_threads)
+ debug_printf (" stepping LWP %ld, reinsert set\n",
+ lwpid_of (thread));
+ step = 1;
+ }
+ else
+ step = 0;
+
linux_resume_one_lwp (lwp, step, 0, NULL);
return 0;
}
@@ -4547,8 +4740,7 @@ unsuspend_and_proceed_one_lwp (struct inferior_list_entry *entry, void *except)
if (lwp == except)
return 0;
- lwp->suspended--;
- gdb_assert (lwp->suspended >= 0);
+ lwp_suspended_decr (lwp);
return proceed_one_lwp (entry, except);
}
@@ -4569,19 +4761,22 @@ proceed_all_lwps (void)
if (supports_breakpoints ())
{
- need_step_over
- = (struct thread_info *) find_inferior (&all_threads,
- need_step_over_p, NULL);
-
- if (need_step_over != NULL)
+ while (1)
{
+ need_step_over
+ = (struct thread_info *) find_inferior (&all_threads,
+ need_step_over_p, NULL);
+
+ if (need_step_over == NULL)
+ break;
+
if (debug_threads)
debug_printf ("proceed_all_lwps: found "
"thread %ld needing a step-over\n",
lwpid_of (need_step_over));
- start_step_over (get_thread_lwp (need_step_over));
- return;
+ if (start_step_over (get_thread_lwp (need_step_over)))
+ return;
}
}
diff --git a/gdb/target/waitstatus.h b/gdb/target/waitstatus.h
index d4ef3b8..ffaddc1 100644
--- a/gdb/target/waitstatus.h
+++ b/gdb/target/waitstatus.h
@@ -131,7 +131,10 @@ enum target_stop_reason
TARGET_STOPPED_BY_HW_BREAKPOINT,
/* Stopped by a watchpoint. */
- TARGET_STOPPED_BY_WATCHPOINT
+ TARGET_STOPPED_BY_WATCHPOINT,
+
+ /* Stopped by a single step finishing. */
+ TARGET_STOPPED_BY_SINGLE_STEP
};
/* Prototypes */
diff --git a/gdb/testsuite/gdb.threads/fork-plus-threads-2.c b/gdb/testsuite/gdb.threads/fork-plus-threads-2.c
new file mode 100644
index 0000000..b66d17eb7
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/fork-plus-threads-2.c
@@ -0,0 +1,129 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2015 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include <assert.h>
+#include <pthread.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <stdlib.h>
+
+/* Number of threads. Each thread continuously spawns a fork and wait
+ for it. If we have another thread continuously start a step over,
+ gdbserver should end up finding new forks while suspending
+ threads. */
+#define NTHREADS 10
+
+pthread_t threads[NTHREADS];
+
+pthread_barrier_t barrier;
+
+#define NFORKS 10
+
+/* Used to create a conditional breakpoint that always fails. */
+volatile int zero;
+
+static void *
+thread_forks (void *arg)
+{
+ int i;
+
+ pthread_barrier_wait (&barrier);
+
+ for (i = 0; i < NFORKS; i++)
+ {
+ pid_t pid;
+
+ pid = fork ();
+
+ if (pid > 0)
+ {
+ int status;
+
+ /* Parent. */
+ pid = waitpid (pid, &status, 0);
+ if (pid == -1)
+ {
+ perror ("wait");
+ exit (1);
+ }
+
+ if (!WIFEXITED (status))
+ {
+ printf ("Unexpected wait status 0x%x from child %d\n",
+ status, pid);
+ }
+ }
+ else if (pid == 0)
+ {
+ /* Child. */
+ exit (0);
+ }
+ else
+ {
+ perror ("fork");
+ exit (1);
+ }
+ }
+}
+
+static void *
+thread_breakpoint (void *arg)
+{
+ pthread_barrier_wait (&barrier);
+
+ while (1)
+ {
+ usleep (1); /* set break here */
+ }
+}
+
+pthread_barrier_t barrier;
+
+int
+main (void)
+{
+ int i;
+ int ret;
+
+ /* Don't run forever. */
+ alarm (180);
+
+ pthread_barrier_init (&barrier, NULL, NTHREADS + 1);
+
+ /* Start the threads that constantly fork. */
+ for (i = 0; i < NTHREADS; i++)
+ {
+ ret = pthread_create (&threads[i], NULL, thread_forks, NULL);
+ assert (ret == 0);
+ }
+
+ /* Start the thread that constantly hit a conditional breakpoint
+ that needs to be stepped over. */
+ ret = pthread_create (&threads[i], NULL, thread_breakpoint, NULL);
+ assert (ret == 0);
+
+ /* Wait for forking to stop. */
+ for (i = 0; i < NTHREADS; i++)
+ {
+ ret = pthread_join (threads[i], NULL);
+ assert (ret == 0);
+ }
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/fork-plus-threads-2.exp b/gdb/testsuite/gdb.threads/fork-plus-threads-2.exp
new file mode 100644
index 0000000..dc4e119
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/fork-plus-threads-2.exp
@@ -0,0 +1,116 @@
+# Copyright (C) 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# This test verifies that several threads forking while another thread
+# is constantly stepping over a breakpoint is properly handled.
+
+standard_testfile
+
+set linenum [gdb_get_line_number "set break here"]
+
+proc do_test { detach_on_fork } {
+ global GDBFLAGS
+ global srcfile testfile
+ global decimal gdb_prompt
+ global linenum
+ global is_remote_target
+
+ set saved_gdbflags $GDBFLAGS
+ set GDBFLAGS [concat $GDBFLAGS " -ex \"set non-stop on\""]
+
+ if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug pthreads}] == -1} {
+ set GDBFLAGS $saved_gdbflags
+ return -1
+ }
+
+ set GDBFLAGS $saved_gdbflags
+
+ if ![runto_main] then {
+ fail "Can't run to main"
+ return 0
+ }
+
+ set is_remote_target [gdb_is_target_remote]
+
+ gdb_test_no_output "set detach-on-fork $detach_on_fork"
+
+ gdb_test "break $linenum if zero == 1" \
+ "Breakpoint .*" \
+ "set breakpoint that evals false"
+
+ set test "continue &"
+ gdb_test_multiple $test $test {
+ -re "$gdb_prompt " {
+ pass $test
+ }
+ }
+
+ set fork_count 0
+ set ok 0
+
+ set test "inferior 1 exited"
+ gdb_test_multiple "" $test {
+ -re "Inferior 1 \(\[^\r\n\]+\) exited normally" {
+ pass $test
+ set ok 1
+ }
+ -re "Inferior $decimal \(\[^\r\n\]+\) exited normally" {
+ incr fork_count
+ if {$fork_count <= 100} {
+ exp_continue
+ } else {
+ fail "$test (too many forks)"
+ }
+ }
+
+ -re "$gdb_prompt " {
+ # Several errors end up at the top level, and printing the
+ # prompt.
+ if {!$is_remote_target} {
+ setup_kfail "gdb/18749" "*-*-linux*"
+ }
+ fail $test
+ }
+ -re "Cannot access memory" {
+ if {!$is_remote_target} {
+ setup_kfail "gdb/18749" "*-*-linux*"
+ }
+ fail $test
+ }
+ }
+
+ if {!$ok} {
+ # No use testing further.
+ return
+ }
+
+ gdb_test "info threads" "No threads\." \
+ "no threads left"
+
+ gdb_test "info inferiors" \
+ "Num\[ \t\]+Description\[ \t\]+Executable\[ \t\]+\r\n\\* 1 \[^\r\n\]+" \
+ "only inferior 1 left"
+}
+
+foreach detach_on_fork {"on" "off"} {
+ with_test_prefix "detach-on-fork=$detach_on_fork" {
+ do_test $detach_on_fork
+ }
+
+ # The test passes with detach-on-fork off, but gdb seems to slow
+ # down quadratically as inferiors are created, and then the test
+ # takes annoyingly long to complete...
+ break
+}
--
1.9.3
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH/7.10 2/2] gdbserver: Fix non-stop / fork / step-over issues
2015-07-31 17:04 ` [PATCH/7.10 2/2] gdbserver: Fix non-stop / fork / step-over issues Pedro Alves
@ 2015-07-31 18:04 ` Don Breazeal
2015-07-31 19:02 ` Pedro Alves
2015-08-03 15:14 ` Yao Qi
1 sibling, 1 reply; 16+ messages in thread
From: Don Breazeal @ 2015-07-31 18:04 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 7/31/2015 10:03 AM, Pedro Alves wrote:
> Ref: https://sourceware.org/ml/gdb-patches/2015-07/msg00868.html
>
> This adds a test that has a multithreaded program have several threads
> continuously fork, while another thread continuously steps over a
> breakpoint.
Wow.
>
> This exposes several intertwined issues, which this patch addresses:
>
Thanks again for digging into these issues.
---snip---
>
> - The test runs with both "set detach-on-fork" on and off. When off,
> it exercises the case of GDB detaching the fork child explicitly.
> When on, it exercises the case of gdb resuming the child
> explicitly. In the "off" case, gdb seems to exponentially become
> slower as new inferiors are created. This is _very_ noticeable as
> with only 100 inferiors gdb is crawling already, which makes the
> test take quite a bit to run. For that reason, I've disabled the
> "off" variant for now.
Bummer. I was going to ask whether this use-case justifies disabling
the feature completely, but since the whole follow-fork mechanism is of
limited usefulness without exec events, the question is likely moot
anyway.
Do you have any thoughts about whether this slowdown is caused by the
fork event machinery or by some more general gdbserver multiple
inferior problem?
Are you planning to look at the slowdown? Can I help out? I have an
interest in having detach-on-fork 'off' enabled. :-S
thanks
--Don
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH/7.10 2/2] gdbserver: Fix non-stop / fork / step-over issues
2015-07-31 18:04 ` Don Breazeal
@ 2015-07-31 19:02 ` Pedro Alves
2015-08-05 22:19 ` Don Breazeal
0 siblings, 1 reply; 16+ messages in thread
From: Pedro Alves @ 2015-07-31 19:02 UTC (permalink / raw)
To: Don Breazeal, gdb-patches
On 07/31/2015 07:04 PM, Don Breazeal wrote:
> On 7/31/2015 10:03 AM, Pedro Alves wrote:
>> Ref: https://sourceware.org/ml/gdb-patches/2015-07/msg00868.html
>>
>> This adds a test that has a multithreaded program have several threads
>> continuously fork, while another thread continuously steps over a
>> breakpoint.
>
> Wow.
>
If gdb survives these stress tests, it can hold up to anything. :-)
>> - The test runs with both "set detach-on-fork" on and off. When off,
>> it exercises the case of GDB detaching the fork child explicitly.
>> When on, it exercises the case of gdb resuming the child
>> explicitly. In the "off" case, gdb seems to exponentially become
>> slower as new inferiors are created. This is _very_ noticeable as
>> with only 100 inferiors gdb is crawling already, which makes the
>> test take quite a bit to run. For that reason, I've disabled the
>> "off" variant for now.
>
> Bummer. I was going to ask whether this use-case justifies disabling
> the feature completely,
Note that this being a stress test, may not be representative of a
real work load. I'm assuming most real use cases won't be
so demanding.
> but since the whole follow-fork mechanism is of
> limited usefulness without exec events, the question is likely moot
> anyway.
Yeah. There are use cases with fork alone, but combined with exec is
much more useful. I'll take a look at your exec patches soon; I'm very
much looking forward to have that in.
>
> Do you have any thoughts about whether this slowdown is caused by the
> fork event machinery or by some more general gdbserver multiple
> inferior problem?
Not sure.
The number of forks live at a given time in the test is constant
-- each thread forks and waits for the child to exit until it forks
again. But if you run the test, you see that the first
few inferiors are created quickly, and then as the inferior number
grows, new inferiors are added at a slower and slower.
I'd suspect the problem to be on the gdb side. But the test
fails on native, so it's not easy to get gdbserver out of
the picture for a quick check.
It feels like some data structures are leaking, but
still reacheable, and then a bunch of linear walks end up costing
more and more. I once added the prune_inferiors call at the end
of normal_stop to handle a slowdown like this. It feels like
something similar to that.
With detach "on" alone, it takes under 2 seconds against gdbserver
for me.
If I remove the breakpoint from the test, and reenable both detach on/off,
it ends in around 10-20 seconds. That's still a lot slower
than "detach on" along, but gdb has to insert/remove breakpoints in the
child and load its symbols (well, it could avoid that, given the
child is a clone of the parent, but we're not there yet), so
not entirely unexpected.
But pristine, with both detach on/off, it takes almost 2 minutes
here. ( and each thread only spawns 10 forks, my first attempt
was shooting for 100 :-) )
I also suspected all the thread stop/restarting gdbserver does
both to step over breakpoints, and to insert/remove breakpoints.
But then again with detach on, there are 12 threads, with detach
off, at most 22. So that'd be odd. Unless the data structure
leaks are on gdbserver's side. But then I'd think that tests
like attach-many-short-lived-threads.exp or non-stop-fair-events.exp
would have already exposed something like that.
>
> Are you planning to look at the slowdown?
Nope, at least not in the immediate future.
> Can I help out? I have an
> interest in having detach-on-fork 'off' enabled. :-S
That'd be much appreciated. :-) At least identifying the
culprit would be very nice. I too would love for our
multi-process support to be rock solid.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH/7.10 1/2] Linux gdbserver confused when event randomization returns a process exit event
2015-07-31 17:04 ` [PATCH/7.10 1/2] Linux gdbserver confused when event randomization returns a process exit event Pedro Alves
@ 2015-08-03 10:59 ` Yao Qi
2015-08-06 9:34 ` [pushed][PATCH 1/3] Linux gdbserver fork event debug output (Re: [PATCH/7.10 1/2] Linux gdbserver confused when event randomization returns a process exit event) Pedro Alves
2015-08-06 9:43 ` [pushed]Re: [PATCH/7.10 1/2] Linux gdbserver confused when event randomization returns a process exit event Pedro Alves
0 siblings, 2 replies; 16+ messages in thread
From: Yao Qi @ 2015-08-03 10:59 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves <palves@redhat.com> writes:
> @@ -3128,7 +3116,7 @@ linux_wait_1 (ptid_t ptid,
> || (gdb_breakpoint_here (event_child->stop_pc)
> && gdb_condition_true_at_breakpoint (event_child->stop_pc)
> && gdb_no_commands_at_breakpoint (event_child->stop_pc))
> - || extended_event_reported (&event_child->waitstatus));
> + || event_child->waitstatus.kind != TARGET_WAITKIND_IGNORE);
>
> run_breakpoint_commands (event_child->stop_pc);
>
> @@ -3150,9 +3138,11 @@ linux_wait_1 (ptid_t ptid,
> paddress (event_child->stop_pc),
> paddress (event_child->step_range_start),
> paddress (event_child->step_range_end));
> - if (extended_event_reported (&event_child->waitstatus))
> + if (event_child->waitstatus.kind != TARGET_WAITKIND_IGNORE)
> {
> - char *str = target_waitstatus_to_string (ourstatus);
> + char *str;
> +
> + str = target_waitstatus_to_string (&event_child->waitstatus);
> debug_printf ("LWP %ld: extended event with waitstatus %s\n",
> lwpid_of (get_lwp_thread (event_child)), str);
> xfree (str);
Looks this code is dead code even without your patch. This code is
guarded by "if (!report_to_gdb)", but if report_to_gdb is false,
"(event_child->waitstatus.kind != TARGET_WAITKIND_IGNORE)" is false too.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH/7.10 2/2] gdbserver: Fix non-stop / fork / step-over issues
2015-07-31 17:04 ` [PATCH/7.10 2/2] gdbserver: Fix non-stop / fork / step-over issues Pedro Alves
2015-07-31 18:04 ` Don Breazeal
@ 2015-08-03 15:14 ` Yao Qi
2015-08-03 16:20 ` Pedro Alves
1 sibling, 1 reply; 16+ messages in thread
From: Yao Qi @ 2015-08-03 15:14 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves <palves@redhat.com> writes:
> I fixed some of these issues recently, but there's a lot more to
> do. Fixing that one just exposes other similar problems elsewhere.
> Meanwhile, I've filed PR18749 and kfailed the test for native.
This test case also exposes the issue on arm-linux with gdbserver,
(gdb) PASS: gdb.threads/fork-plus-threads-2.exp: detach-on-fork=on: continue &
[New Thread 29905]^M
[New Thread 29900]^M
[New Thread 29895]^M
[New Thread 29898]^M
[New Thread 29902]^M
[New Thread 29896]^M
[New Thread 29903]^M
[New Thread 29901]^M
[New Thread 29899]^M
[New Thread 29904]^M
[New Thread 29897]^M
Error in testing breakpoint condition:^M
Cannot access memory at address 0x11094^M
FAIL: gdb.threads/fork-plus-threads-2.exp: detach-on-fork=on: inferior 1 exited
Remote debugging from host 127.0.0.1^M
^M
Child exited with status 0^M
GDBserver exiting^M
../../binutils-gdb/gdb/thread.c:936: internal-error: finish_thread_state: Assertion `tp' failed.^M
A problem internal to GDB has been detected,^M
further debugging may prove unreliable.^M
Quit this debugging session? (y or n) monitor exit^M
Please answer y or n.^M
../../binutils-gdb/gdb/thread.c:936: internal-error: finish_thread_state: Assertion `tp' failed.^
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH/7.10 2/2] gdbserver: Fix non-stop / fork / step-over issues
2015-08-03 15:14 ` Yao Qi
@ 2015-08-03 16:20 ` Pedro Alves
2015-08-04 16:40 ` Pedro Alves
0 siblings, 1 reply; 16+ messages in thread
From: Pedro Alves @ 2015-08-03 16:20 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 08/03/2015 04:14 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> I fixed some of these issues recently, but there's a lot more to
>> do. Fixing that one just exposes other similar problems elsewhere.
>> Meanwhile, I've filed PR18749 and kfailed the test for native.
>
> This test case also exposes the issue on arm-linux with gdbserver,
>
> (gdb) PASS: gdb.threads/fork-plus-threads-2.exp: detach-on-fork=on: continue &
> [New Thread 29905]^M
> [New Thread 29900]^M
> [New Thread 29895]^M
> [New Thread 29898]^M
> [New Thread 29902]^M
> [New Thread 29896]^M
> [New Thread 29903]^M
> [New Thread 29901]^M
> [New Thread 29899]^M
> [New Thread 29904]^M
> [New Thread 29897]^M
> Error in testing breakpoint condition:^M
> Cannot access memory at address 0x11094^M
> FAIL: gdb.threads/fork-plus-threads-2.exp: detach-on-fork=on: inferior 1 exited
Ah, OK, so I guess it'll fail on all targets where gdbserver
does not handle the breakpoint condition server-side.
> Remote debugging from host 127.0.0.1^M
> ^M
> Child exited with status 0^M
> GDBserver exiting^M
> ../../binutils-gdb/gdb/thread.c:936: internal-error: finish_thread_state: Assertion `tp' failed.^M
> A problem internal to GDB has been detected,^M
> further debugging may prove unreliable.^M
> Quit this debugging session? (y or n) monitor exit^M
> Please answer y or n.^M
> ../../binutils-gdb/gdb/thread.c:936: internal-error: finish_thread_state: Assertion `tp' failed.^
I've seem this too while developing the patch. I saw it happen when the
connection is abruptly closed while there's a finish_thread_state cleanup
installed (while gdb is handling an event, within fetch_inferior_event). The
connection-close deletes all threads, and the cleanup wants to finish the
state of a now-nonexisting thread. See [palves/fork_stale_running] on my github.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH/7.10 2/2] gdbserver: Fix non-stop / fork / step-over issues
2015-08-03 16:20 ` Pedro Alves
@ 2015-08-04 16:40 ` Pedro Alves
2015-08-05 11:41 ` Yao Qi
0 siblings, 1 reply; 16+ messages in thread
From: Pedro Alves @ 2015-08-04 16:40 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 08/03/2015 05:20 PM, Pedro Alves wrote:
> On 08/03/2015 04:14 PM, Yao Qi wrote:
>> Pedro Alves <palves@redhat.com> writes:
>>
>>> I fixed some of these issues recently, but there's a lot more to
>>> do. Fixing that one just exposes other similar problems elsewhere.
>>> Meanwhile, I've filed PR18749 and kfailed the test for native.
>>
>> This test case also exposes the issue on arm-linux with gdbserver,
>>
>> (gdb) PASS: gdb.threads/fork-plus-threads-2.exp: detach-on-fork=on: continue &
>> [New Thread 29905]^M
>> [New Thread 29900]^M
>> [New Thread 29895]^M
>> [New Thread 29898]^M
>> [New Thread 29902]^M
>> [New Thread 29896]^M
>> [New Thread 29903]^M
>> [New Thread 29901]^M
>> [New Thread 29899]^M
>> [New Thread 29904]^M
>> [New Thread 29897]^M
>> Error in testing breakpoint condition:^M
>> Cannot access memory at address 0x11094^M
>> FAIL: gdb.threads/fork-plus-threads-2.exp: detach-on-fork=on: inferior 1 exited
>
> Ah, OK, so I guess it'll fail on all targets where gdbserver
> does not handle the breakpoint condition server-side.
I've now extended the test to run in a few different modes, along
a couple different axis. One axis is with/without conditional
breakpoints on the target enabled. That exposes the same fails you
saw on arm, on x86 gdbserver as well. And then the other axis is
with/without joining _all_ threads before exiting. If we gracefully
terminate the breakpoint thread (new mode), then the test should be
passing everywhere, because what fails is gdb's handling of the inferior
disappearing while a thread is stopped (and being inspected).
Therefore that new mode is not kfailed.
For testing convenience, I've pushed this along with the previous patch
to the users/palves/gdbserver-fork-issues branch on sourceware.org.
Let me know if this works for you.
----
From fa63541a7c101ddc340ba33955436295a4f39909 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 4 Aug 2015 17:21:30 +0100
Subject: [PATCH v2 2/2] gdbserver: Fix non-stop / fork / step-over issues
Ref: https://sourceware.org/ml/gdb-patches/2015-07/msg00868.html
This adds a test that has a multithreaded program have several threads
continuously fork, while another thread continuously steps over a
breakpoint.
This exposes several intertwined issues, which this patch addresses:
- When we're stopping and suspending threads, some thread may fork,
and we missed setting its suspend count to 1, like we do when a new
clone/thread is detected. When we next unsuspend threads, the fork
child's suspend count goes below 0, which is bogus and fails an
assertion.
- If a step-over is cancelled because a signal arrives, but then gdb
is not interested in the signal, we pass the signal straight back
to the inferior. However, we miss that we need to re-increment the
suspend counts of all other threads that had been paused for the
step-over. As a result, other threads indefinitely end up stuck
stopped.
- OTOH, if a thread exits the whole process just while we're stopping
threads to start a step-over, gdbserver crashes or hits several
assertions.
- If a detach request comes in just while gdbserver is handling a
step-over (in the test at hand, this is GDB detaching the fork
child), gdbserver internal errors in stabilize_thread's helpers,
which assert that all thread's suspend counts are 0 (otherwise we
wouldn't be able to move threads out of the jump pads). The
suspend counts aren't 0 while a step-over is in progress, because
all threads but the one stepping past the breakpoint must remain
paused until the step-over finishes and the breakpoint can be
reinserted.
- Occasionally, we see "BAD - reinserting but not stepping." being
output (from within linux_resume_one_lwp_throw). That was because
GDB pokes memory while gdbserver is busy with a step-over, and that
suspends threads, and then re-resumes them with proceed_one_lwp,
which missed another reason to tell linux_resume_one_lwp that the
thread should be set back to stepping.
- In a couple places, we were resuming threads that are meant to be
suspended. E.g., when a vCont;c/s request for thread B comes in
just while gdbserver is stepping thread A past a breakpoint. The
resume for thread B must be deferred until the step-over finishes.
- The test runs with both "set detach-on-fork" on and off. When off,
it exercises the case of GDB detaching the fork child explicitly.
When on, it exercises the case of gdb resuming the child
explicitly. In the "off" case, gdb seems to exponentially become
slower as new inferiors are created. This is _very_ noticeable as
with only 100 inferiors gdb is crawling already, which makes the
test take quite a bit to run. For that reason, I've disabled the
"off" variant for now.
- The test fails occasionally with the native target, because several
code paths aren't expecting that a stopped thread may disappear
(the test has the leader thread of the parent process exit the
whole process, just while gdb is handling an event for a non-leader
thread). E.g.,:
[Thread 0x7ffff67bd700 (LWP 11210) exited]
Cannot find user-level thread for LWP 11217: generic error
(gdb) FAIL: gdb.threads/fork-plus-threads-2.exp: detach-on-fork=on: inferior 1 exited
[Thread 0x7ffff7fc1740 (LWP 11203) exited]
info threads
Id Target Id Frame
12 Thread 0x7ffff2fb6700 (LWP 11217) (running)
The current thread <Thread ID 1> has terminated. See `help thread'.
(gdb) FAIL: gdb.threads/fork-plus-threads-2.exp: detach-on-fork=on: no threads left
I fixed some of these issues recently, but there's a lot more to
do. Fixing that one just exposes other similar problems elsewhere.
Meanwhile, I've filed PR18749 and kfailed the test for native.
gdb/ChangeLog:
2015-08-04 Pedro Alves <palves@redhat.com>
PR gdb/18749
* target/waitstatus.h (enum target_stop_reason)
<TARGET_STOPPED_BY_SINGLE_STEP>: New value.
gdb/gdbserver/ChangeLog:
2015-08-04 Pedro Alves <palves@redhat.com>
PR gdb/18749
* linux-low.c (handle_extended_wait): Set the fork child's suspend
count if stopping and suspending threads.
(check_stopped_by_breakpoint): If stopped by trace, set the LWP's
stop reason to TARGET_STOPPED_BY_SINGLE_STEP.
(linux_detach): Complete an ongoing step-over.
(lwp_suspended_inc, lwp_suspended_decr): New functions. Use
throughout.
(resume_stopped_resumed_lwps): Don't resume a suspended thread.
(linux_wait_1): If passing a signal to the inferior after
finishing a step-over, unsuspend and re-resume all lwps. If we
see a single-step event but the thread should be continuing, don't
pass the trap to gdb.
(stuck_in_jump_pad_callback, move_out_of_jump_pad_callback): Use
internal_error instead of gdb_assert.
(enqueue_pending_signal): New function.
(check_ptrace_stopped_lwp_gone): Add debug output.
(start_step_over): Handle the case of the LWP we're about to
step-over exiting. Use internal_error instead of gdb_assert.
(complete_ongoing_step_over): New function.
(linux_resume_one_thread): Don't resume a suspended thread.
(proceed_one_lwp): If the LWP is stepping over a breakpoint, reset
it stepping.
(proceed_all_lwps): If a step-over fails to start, look for
another thread that might need a step-over.
gdb/testsuite/ChangeLog:
2015-08-04 Pedro Alves <palves@redhat.com>
PR gdb/18749
* gdb.threads/fork-plus-threads-2.exp: New file.
* gdb.threads/fork-plus-threads-2.c: New file.
---
gdb/gdbserver/linux-low.c | 267 +++++++++++++++++++---
gdb/target/waitstatus.h | 5 +-
gdb/testsuite/gdb.threads/fork-plus-threads-2.c | 146 ++++++++++++
gdb/testsuite/gdb.threads/fork-plus-threads-2.exp | 176 ++++++++++++++
4 files changed, 557 insertions(+), 37 deletions(-)
create mode 100644 gdb/testsuite/gdb.threads/fork-plus-threads-2.c
create mode 100644 gdb/testsuite/gdb.threads/fork-plus-threads-2.exp
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 4cdedb4..66d57ec 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -268,6 +268,8 @@ static int lwp_is_marked_dead (struct lwp_info *lwp);
static void proceed_all_lwps (void);
static int finish_step_over (struct lwp_info *lwp);
static int kill_lwp (unsigned long lwpid, int signo);
+static void enqueue_pending_signal (struct lwp_info *lwp, int signal, siginfo_t *info);
+static void complete_ongoing_step_over (void);
/* When the event-loop is doing a step-over, this points at the thread
being stepped. */
@@ -486,6 +488,15 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat)
child_thr->last_resume_kind = resume_stop;
child_thr->last_status.kind = TARGET_WAITKIND_STOPPED;
+ /* If we're suspending all threads, leave this one suspended
+ too. */
+ if (stopping_threads == STOPPING_AND_SUSPENDING_THREADS)
+ {
+ if (debug_threads)
+ debug_printf ("HEW: leaving child suspended\n");
+ child_lwp->suspended = 1;
+ }
+
parent_proc = get_thread_process (event_thr);
child_proc->attached = parent_proc->attached;
clone_all_breakpoints (&child_proc->breakpoints,
@@ -685,6 +696,8 @@ check_stopped_by_breakpoint (struct lwp_info *lwp)
debug_printf ("CSBB: %s stopped by trace\n",
target_pid_to_str (ptid_of (thr)));
}
+
+ lwp->stop_reason = TARGET_STOPPED_BY_SINGLE_STEP;
}
}
}
@@ -1315,6 +1328,11 @@ linux_detach (int pid)
if (process == NULL)
return -1;
+ /* As there's a step over already in progress, let it finish first,
+ otherwise nesting a stabilize_threads operation on top gets real
+ messy. */
+ complete_ongoing_step_over ();
+
/* Stop all threads before detaching. First, ptrace requires that
the thread is stopped to sucessfully detach. Second, thread_db
may need to uninstall thread event breakpoints from memory, which
@@ -1683,6 +1701,39 @@ not_stopped_callback (struct inferior_list_entry *entry, void *arg)
return 0;
}
+/* Increment LWP's suspend count. */
+
+static void
+lwp_suspended_inc (struct lwp_info *lwp)
+{
+ lwp->suspended++;
+
+ if (debug_threads && lwp->suspended > 4)
+ {
+ struct thread_info *thread = get_lwp_thread (lwp);
+
+ debug_printf ("LWP %ld has a suspiciously high suspend count,"
+ " suspended=%d\n", lwpid_of (thread), lwp->suspended);
+ }
+}
+
+/* Decrement LWP's suspend count. */
+
+static void
+lwp_suspended_decr (struct lwp_info *lwp)
+{
+ lwp->suspended--;
+
+ if (lwp->suspended < 0)
+ {
+ struct thread_info *thread = get_lwp_thread (lwp);
+
+ internal_error (__FILE__, __LINE__,
+ "unsuspend LWP %ld, suspended=%d\n", lwpid_of (thread),
+ lwp->suspended);
+ }
+}
+
/* This function should only be called if the LWP got a SIGTRAP.
Handle any tracepoint steps or hits. Return true if a tracepoint
@@ -1700,7 +1751,7 @@ handle_tracepoints (struct lwp_info *lwp)
uninsert tracepoints. To do this, we temporarily pause all
threads, unpatch away, and then unpause threads. We need to make
sure the unpausing doesn't resume LWP too. */
- lwp->suspended++;
+ lwp_suspended_inc (lwp);
/* And we need to be sure that any all-threads-stopping doesn't try
to move threads out of the jump pads, as it could deadlock the
@@ -1716,7 +1767,7 @@ handle_tracepoints (struct lwp_info *lwp)
actions. */
tpoint_related_event |= tracepoint_was_hit (tinfo, lwp->stop_pc);
- lwp->suspended--;
+ lwp_suspended_decr (lwp);
gdb_assert (lwp->suspended == 0);
gdb_assert (!stabilizing_threads || lwp->collecting_fast_tracepoint);
@@ -2176,10 +2227,13 @@ linux_low_filter_event (int lwpid, int wstat)
/* Note that TRAP_HWBKPT can indicate either a hardware breakpoint
or hardware watchpoint. Check which is which if we got
- TARGET_STOPPED_BY_HW_BREAKPOINT. */
+ TARGET_STOPPED_BY_HW_BREAKPOINT. Likewise, we may have single
+ stepped an instruction that triggered a watchpoint. In that
+ case, on some architectures (such as x86), instead of
+ TRAP_HWBKPT, si_code indicates TRAP_TRACE, and we need to check
+ the debug registers separately. */
if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SIGTRAP
- && (child->stop_reason == TARGET_STOPPED_BY_NO_REASON
- || child->stop_reason == TARGET_STOPPED_BY_HW_BREAKPOINT))
+ && child->stop_reason != TARGET_STOPPED_BY_SW_BREAKPOINT)
check_stopped_by_watchpoint (child);
if (!have_stop_pc)
@@ -2238,6 +2292,7 @@ resume_stopped_resumed_lwps (struct inferior_list_entry *entry)
struct lwp_info *lp = get_thread_lwp (thread);
if (lp->stopped
+ && !lp->suspended
&& !lp->status_pending_p
&& thread->last_resume_kind != resume_stop
&& thread->last_status.kind == TARGET_WAITKIND_IGNORE)
@@ -2608,9 +2663,7 @@ unsuspend_one_lwp (struct inferior_list_entry *entry, void *except)
if (lwp == except)
return 0;
- lwp->suspended--;
-
- gdb_assert (lwp->suspended >= 0);
+ lwp_suspended_decr (lwp);
return 0;
}
@@ -2703,7 +2756,7 @@ linux_stabilize_threads (void)
lwp = get_thread_lwp (current_thread);
/* Lock it. */
- lwp->suspended++;
+ lwp_suspended_inc (lwp);
if (ourstatus.value.sig != GDB_SIGNAL_0
|| current_thread->last_resume_kind == resume_stop)
@@ -3089,8 +3142,25 @@ linux_wait_1 (ptid_t ptid,
info_p = &info;
else
info_p = NULL;
- linux_resume_one_lwp (event_child, event_child->stepping,
- WSTOPSIG (w), info_p);
+
+ if (step_over_finished)
+ {
+ /* We cancelled this thread's step-over above. We still
+ need to unsuspend all other LWPs, and set then back
+ running again while the signal handler runs. */
+ unsuspend_all_lwps (event_child);
+
+ /* Enqueue the pending signal info so that proceed_all_lwps
+ doesn't lose it. */
+ enqueue_pending_signal (event_child, WSTOPSIG (w), info_p);
+
+ proceed_all_lwps ();
+ }
+ else
+ {
+ linux_resume_one_lwp (event_child, event_child->stepping,
+ WSTOPSIG (w), info_p);
+ }
return ignore_event (ourstatus);
}
@@ -3111,8 +3181,15 @@ linux_wait_1 (ptid_t ptid,
|| (current_thread->last_resume_kind == resume_step
&& !in_step_range)
|| event_child->stop_reason == TARGET_STOPPED_BY_WATCHPOINT
- || (!step_over_finished && !in_step_range
- && !bp_explains_trap && !trace_event)
+ || (!in_step_range
+ && !bp_explains_trap
+ && !trace_event
+ /* A step-over was finished just now? */
+ && !step_over_finished
+ /* A step-over had been finished previously,
+ and the single-step was left pending? */
+ && !(current_thread->last_resume_kind == resume_continue
+ && event_child->stop_reason == TARGET_STOPPED_BY_SINGLE_STEP))
|| (gdb_breakpoint_here (event_child->stop_pc)
&& gdb_condition_true_at_breakpoint (event_child->stop_pc)
&& gdb_no_commands_at_breakpoint (event_child->stop_pc))
@@ -3460,7 +3537,7 @@ suspend_and_send_sigstop_callback (struct inferior_list_entry *entry,
if (lwp == except)
return 0;
- lwp->suspended++;
+ lwp_suspended_inc (lwp);
return send_sigstop_callback (entry, except);
}
@@ -3562,7 +3639,12 @@ stuck_in_jump_pad_callback (struct inferior_list_entry *entry, void *data)
struct thread_info *thread = (struct thread_info *) entry;
struct lwp_info *lwp = get_thread_lwp (thread);
- gdb_assert (lwp->suspended == 0);
+ if (lwp->suspended != 0)
+ {
+ internal_error (__FILE__, __LINE__,
+ "LWP %ld is suspended, suspended=%d\n",
+ lwpid_of (thread), lwp->suspended);
+ }
gdb_assert (lwp->stopped);
/* Allow debugging the jump pad, gdb_collect, etc.. */
@@ -3581,7 +3663,12 @@ move_out_of_jump_pad_callback (struct inferior_list_entry *entry)
struct lwp_info *lwp = get_thread_lwp (thread);
int *wstat;
- gdb_assert (lwp->suspended == 0);
+ if (lwp->suspended != 0)
+ {
+ internal_error (__FILE__, __LINE__,
+ "LWP %ld is suspended, suspended=%d\n",
+ lwpid_of (thread), lwp->suspended);
+ }
gdb_assert (lwp->stopped);
wstat = lwp->status_pending_p ? &lwp->status_pending : NULL;
@@ -3610,7 +3697,7 @@ move_out_of_jump_pad_callback (struct inferior_list_entry *entry)
linux_resume_one_lwp (lwp, 0, 0, NULL);
}
else
- lwp->suspended++;
+ lwp_suspended_inc (lwp);
}
static int
@@ -3665,6 +3752,24 @@ stop_all_lwps (int suspend, struct lwp_info *except)
}
}
+/* Enqueue one signal in the chain of signals which need to be
+ delivered to this process on next resume. */
+
+static void
+enqueue_pending_signal (struct lwp_info *lwp, int signal, siginfo_t *info)
+{
+ struct pending_signals *p_sig;
+
+ p_sig = xmalloc (sizeof (*p_sig));
+ p_sig->prev = lwp->pending_signals;
+ p_sig->signal = signal;
+ if (info == NULL)
+ memset (&p_sig->info, 0, sizeof (siginfo_t));
+ else
+ memcpy (&p_sig->info, info, sizeof (siginfo_t));
+ lwp->pending_signals = p_sig;
+}
+
/* Resume execution of LWP. If STEP is nonzero, single-step it. If
SIGNAL is nonzero, give it that signal. */
@@ -3906,6 +4011,10 @@ check_ptrace_stopped_lwp_gone (struct lwp_info *lp)
/* Don't assume anything if /proc/PID/status can't be read. */
if (linux_proc_pid_is_trace_stopped_nowarn (lwpid_of (thread)) == 0)
{
+ if (debug_threads)
+ debug_printf ("lwp %ld exited while being resumed\n",
+ lwpid_of (thread));
+
lp->stop_reason = TARGET_STOPPED_BY_NO_REASON;
lp->status_pending_p = 0;
return 1;
@@ -4189,16 +4298,36 @@ static int
start_step_over (struct lwp_info *lwp)
{
struct thread_info *thread = get_lwp_thread (lwp);
+ ptid_t thread_ptid;
struct thread_info *saved_thread;
CORE_ADDR pc;
int step;
+ thread_ptid = ptid_of (thread);
+
if (debug_threads)
debug_printf ("Starting step-over on LWP %ld. Stopping all threads\n",
lwpid_of (thread));
stop_all_lwps (1, lwp);
- gdb_assert (lwp->suspended == 0);
+
+ /* Re-find the LWP as it may have exited. */
+ lwp = find_lwp_pid (thread_ptid);
+ if (lwp == NULL || lwp_is_marked_dead (lwp))
+ {
+ if (debug_threads)
+ debug_printf ("Step-over thread died "
+ "(another thread exited the process?).\n");
+ unstop_all_lwps (1, lwp);
+ return 0;
+ }
+
+ if (lwp->suspended != 0)
+ {
+ internal_error (__FILE__, __LINE__,
+ "LWP %ld suspended=%d\n", lwpid_of (thread),
+ lwp->suspended);
+ }
if (debug_threads)
debug_printf ("Done stopping all threads for step-over.\n");
@@ -4229,7 +4358,19 @@ start_step_over (struct lwp_info *lwp)
current_thread = saved_thread;
- linux_resume_one_lwp (lwp, step, 0, NULL);
+ TRY
+ {
+ linux_resume_one_lwp_throw (lwp, step, 0, NULL);
+ }
+ CATCH (ex, RETURN_MASK_ERROR)
+ {
+ unstop_all_lwps (1, lwp);
+
+ if (!check_ptrace_stopped_lwp_gone (lwp))
+ throw_exception (ex);
+ return 0;
+ }
+ END_CATCH
/* Require next event from this LWP. */
step_over_bkpt = thread->entry.id;
@@ -4270,6 +4411,39 @@ finish_step_over (struct lwp_info *lwp)
return 0;
}
+/* If there's a step over in progress, wait until all threads stop
+ (that is, until the stepping thread finishes its step), and
+ unsuspend all lwps. The stepping thread ends with its status
+ pending, which is processed later when we get back to processing
+ events. */
+
+static void
+complete_ongoing_step_over (void)
+{
+ if (!ptid_equal (step_over_bkpt, null_ptid))
+ {
+ struct lwp_info *lwp;
+ int wstat;
+ int ret;
+
+ if (debug_threads)
+ debug_printf ("detach: step over in progress, finish it first\n");
+
+ /* 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);
+
+ lwp = find_lwp_pid (step_over_bkpt);
+ if (lwp != NULL)
+ finish_step_over (lwp);
+ step_over_bkpt = null_ptid;
+ unsuspend_all_lwps (lwp);
+ }
+}
+
/* This function is called once per thread. We check the thread's resume
request, which will tell us whether to resume, step, or leave the thread
stopped; and what signal, if any, it should be sent.
@@ -4344,13 +4518,16 @@ linux_resume_one_thread (struct inferior_list_entry *entry, void *arg)
}
/* If this thread which is about to be resumed has a pending status,
- then don't resume any threads - we can just report the pending
- status. Make sure to queue any signals that would otherwise be
- sent. In all-stop mode, we do this decision based on if *any*
- thread has a pending status. If there's a thread that needs the
- step-over-breakpoint dance, then don't resume any other thread
- but that particular one. */
- leave_pending = (lwp->status_pending_p || leave_all_stopped);
+ then don't resume it - we can just report the pending status.
+ Likewise if it is suspended, because e.g., another thread is
+ stepping past a breakpoint. Make sure to queue any signals that
+ would otherwise be sent. In all-stop mode, we do this decision
+ based on if *any* thread has a pending status. If there's a
+ thread that needs the step-over-breakpoint dance, then don't
+ resume any other thread but that particular one. */
+ leave_pending = (lwp->suspended
+ || lwp->status_pending_p
+ || leave_all_stopped);
if (!leave_pending)
{
@@ -4533,7 +4710,23 @@ proceed_one_lwp (struct inferior_list_entry *entry, void *except)
send_sigstop (lwp);
}
- step = thread->last_resume_kind == resume_step;
+ if (thread->last_resume_kind == resume_step)
+ {
+ if (debug_threads)
+ debug_printf (" stepping LWP %ld, client wants it stepping\n",
+ lwpid_of (thread));
+ step = 1;
+ }
+ else if (lwp->bp_reinsert != 0)
+ {
+ if (debug_threads)
+ debug_printf (" stepping LWP %ld, reinsert set\n",
+ lwpid_of (thread));
+ step = 1;
+ }
+ else
+ step = 0;
+
linux_resume_one_lwp (lwp, step, 0, NULL);
return 0;
}
@@ -4547,8 +4740,7 @@ unsuspend_and_proceed_one_lwp (struct inferior_list_entry *entry, void *except)
if (lwp == except)
return 0;
- lwp->suspended--;
- gdb_assert (lwp->suspended >= 0);
+ lwp_suspended_decr (lwp);
return proceed_one_lwp (entry, except);
}
@@ -4569,19 +4761,22 @@ proceed_all_lwps (void)
if (supports_breakpoints ())
{
- need_step_over
- = (struct thread_info *) find_inferior (&all_threads,
- need_step_over_p, NULL);
-
- if (need_step_over != NULL)
+ while (1)
{
+ need_step_over
+ = (struct thread_info *) find_inferior (&all_threads,
+ need_step_over_p, NULL);
+
+ if (need_step_over == NULL)
+ break;
+
if (debug_threads)
debug_printf ("proceed_all_lwps: found "
"thread %ld needing a step-over\n",
lwpid_of (need_step_over));
- start_step_over (get_thread_lwp (need_step_over));
- return;
+ if (start_step_over (get_thread_lwp (need_step_over)))
+ return;
}
}
diff --git a/gdb/target/waitstatus.h b/gdb/target/waitstatus.h
index d4ef3b8..ffaddc1 100644
--- a/gdb/target/waitstatus.h
+++ b/gdb/target/waitstatus.h
@@ -131,7 +131,10 @@ enum target_stop_reason
TARGET_STOPPED_BY_HW_BREAKPOINT,
/* Stopped by a watchpoint. */
- TARGET_STOPPED_BY_WATCHPOINT
+ TARGET_STOPPED_BY_WATCHPOINT,
+
+ /* Stopped by a single step finishing. */
+ TARGET_STOPPED_BY_SINGLE_STEP
};
/* Prototypes */
diff --git a/gdb/testsuite/gdb.threads/fork-plus-threads-2.c b/gdb/testsuite/gdb.threads/fork-plus-threads-2.c
new file mode 100644
index 0000000..b6baaf6
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/fork-plus-threads-2.c
@@ -0,0 +1,146 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2015 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include <assert.h>
+#include <pthread.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <stdlib.h>
+
+/* Number of threads. Each thread continuously spawns a fork and wait
+ for it. If we have another thread continuously start a step over,
+ gdbserver should end up finding new forks while suspending
+ threads. */
+#define NTHREADS 10
+
+pthread_t threads[NTHREADS];
+
+pthread_barrier_t barrier;
+
+#define NFORKS 10
+
+/* Used to create a conditional breakpoint that always fails. */
+volatile int zero;
+
+static void *
+thread_forks (void *arg)
+{
+ int i;
+
+ pthread_barrier_wait (&barrier);
+
+ for (i = 0; i < NFORKS; i++)
+ {
+ pid_t pid;
+
+ pid = fork ();
+
+ if (pid > 0)
+ {
+ int status;
+
+ /* Parent. */
+ pid = waitpid (pid, &status, 0);
+ if (pid == -1)
+ {
+ perror ("wait");
+ exit (1);
+ }
+
+ if (!WIFEXITED (status))
+ {
+ printf ("Unexpected wait status 0x%x from child %d\n",
+ status, pid);
+ }
+ }
+ else if (pid == 0)
+ {
+ /* Child. */
+ exit (0);
+ }
+ else
+ {
+ perror ("fork");
+ exit (1);
+ }
+ }
+}
+
+/* Set by GDB. If set, then the thread_breakpoint thread exits
+ gracefully before the whole process exits. */
+volatile int should_break_out;
+
+/* Set this to tell the thread_breakpoint thread to exit. */
+volatile int break_out;
+
+static void *
+thread_breakpoint (void *arg)
+{
+ pthread_barrier_wait (&barrier);
+
+ while (!break_out)
+ {
+ usleep (1); /* set break here */
+ }
+
+ return NULL;
+}
+
+pthread_barrier_t barrier;
+
+int
+main (void)
+{
+ int i;
+ int ret;
+ pthread_t bp_thread;
+
+ /* Don't run forever. */
+ alarm (180);
+
+ pthread_barrier_init (&barrier, NULL, NTHREADS + 1);
+
+ /* Start the threads that constantly fork. */
+ for (i = 0; i < NTHREADS; i++)
+ {
+ ret = pthread_create (&threads[i], NULL, thread_forks, NULL);
+ assert (ret == 0);
+ }
+
+ /* Start the thread that constantly hit a conditional breakpoint
+ that needs to be stepped over. */
+ ret = pthread_create (&bp_thread, NULL, thread_breakpoint, NULL);
+ assert (ret == 0);
+
+ /* Wait for forking to stop. */
+ for (i = 0; i < NTHREADS; i++)
+ {
+ ret = pthread_join (threads[i], NULL);
+ assert (ret == 0);
+ }
+
+ if (should_break_out)
+ {
+ break_out = 1;
+ pthread_join (bp_thread, NULL);
+ assert (ret == 0);
+ }
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/fork-plus-threads-2.exp b/gdb/testsuite/gdb.threads/fork-plus-threads-2.exp
new file mode 100644
index 0000000..d36ea27
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/fork-plus-threads-2.exp
@@ -0,0 +1,176 @@
+# Copyright (C) 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# This test verifies that several threads forking while another thread
+# is constantly stepping over a breakpoint is properly handled.
+
+standard_testfile
+
+set linenum [gdb_get_line_number "set break here"]
+
+set saved_gdbflags $GDBFLAGS
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set non-stop on\""]
+
+if {[build_executable "failed to prepare" $testfile $srcfile {debug pthreads}] == -1} {
+ set GDBFLAGS $saved_gdbflags
+ return -1
+}
+
+set GDBFLAGS $saved_gdbflags
+
+# The test proper. If JOIN_BP_THREAD is true, then we'll ask the
+# thread_breakpoint thread to exit, and join it. If JOIN_BP_THREAD is
+# false, the main process exits as soon as all fork threads exit,
+# which frequently happens while GDB is handling a breakpoint event
+# for the thread_breakpoint thread (that is, while that thread is
+# stopped). GDB should be able to handle that too. If COND_BP_TARGET
+# is true, then test with conditional breakpoints evaluated on the
+# target side, if possible.
+proc do_test { join_bp_thread cond_bp_target detach_on_fork } {
+ global GDBFLAGS
+ global srcfile testfile binfile
+ global decimal gdb_prompt
+ global linenum
+ global is_remote_target
+
+ clean_restart $binfile
+
+ if ![runto_main] then {
+ fail "Can't run to main"
+ return 0
+ }
+
+ # Whether it's known that the !$join_bp_thread variant of the test
+ # fails.
+ set should_kfail 0
+
+ if {![gdb_is_target_remote]} {
+ set should_kfail 1
+ } else {
+ if {!$cond_bp_target} {
+ # Leaving breakpoint evaluation to GDB exposes failures
+ # similar to native debugging.
+ gdb_test_no_output "set remote conditional-breakpoints-packet off"
+ set should_kfail 1
+ } else {
+ # With GDBserver testing and breakpoint evaluation on the
+ # target, the test seems to always pass.
+ set test "show remote conditional-breakpoints-packet"
+ gdb_test_multiple $test $test {
+ -re "currently enabled\.\r\n$gdb_prompt $" {
+ pass $test
+ }
+ -re "currently disabled\.\r\n$gdb_prompt $" {
+ unsupported "no support for target-side conditional breakpoints"
+ return
+ }
+ }
+ }
+ }
+
+ gdb_test "print should_break_out = $join_bp_thread" " = $join_bp_thread"
+
+ gdb_test_no_output "set detach-on-fork $detach_on_fork"
+
+ gdb_test "break $linenum if zero == 1" \
+ "Breakpoint .*" \
+ "set breakpoint that evals false"
+
+ set test "continue &"
+ gdb_test_multiple $test $test {
+ -re "$gdb_prompt " {
+ pass $test
+ }
+ }
+
+ set fork_count 0
+ set ok 0
+
+ # Setup the kfail upfront in order to also catch GDB internal
+ # errors.
+ if {!$join_bp_thread && $should_kfail} {
+ setup_kfail "gdb/18749" "*-*-linux*"
+ }
+
+ set test "inferior 1 exited"
+ gdb_test_multiple "" $test {
+ -re "Inferior 1 \(\[^\r\n\]+\) exited normally" {
+ set ok 1
+
+ # Clear the kfail to avoid a PASS -> KPASS dance across
+ # runs.
+ clear_kfail "*-*-linux*"
+
+ pass $test
+ }
+ -re "Inferior $decimal \(\[^\r\n\]+\) exited normally" {
+ incr fork_count
+ if {$fork_count <= 100} {
+ exp_continue
+ } else {
+ fail "$test (too many forks)"
+ }
+ }
+ -re "$gdb_prompt " {
+ # Several errors end up at the top level, and printing the
+ # prompt.
+ fail "$test (prompt)"
+ }
+ -re "Cannot access memory" {
+ fail "$test (memory error)"
+ }
+ eof {
+ fail "$test (GDB died)"
+ }
+ }
+
+ if {!$ok} {
+ # No use testing further.
+ return
+ }
+
+ gdb_test "info threads" "No threads\." \
+ "no threads left"
+
+ gdb_test "info inferiors" \
+ "Num\[ \t\]+Description\[ \t\]+Executable\[ \t\]+\r\n\\* 1 \[^\r\n\]+" \
+ "only inferior 1 left"
+}
+
+# Wrapper for foreach that calls with_test_prefix on each iteration,
+# including the iterator's current value in the prefix.
+
+proc foreach_with_prefix {var list body} {
+ upvar 1 $var myvar
+ foreach myvar $list {
+ with_test_prefix "$var=$myvar" {
+ uplevel 1 $body
+ }
+ }
+}
+
+foreach_with_prefix join_bp_thread {1 0} {
+ foreach_with_prefix cond_bp_target {1 0} {
+ foreach_with_prefix detach_on_fork {"on" "off"} {
+ do_test $join_bp_thread $cond_bp_target $detach_on_fork
+
+ # Disable "off" for now. The test does pass with
+ # detach-on-fork off (at the time of writing), but gdb seems
+ # to slow down quadratically as inferiors are created, and
+ # then the test takes annoyingly long to complete...
+ break
+ }
+ }
+}
--
1.9.3
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH/7.10 2/2] gdbserver: Fix non-stop / fork / step-over issues
2015-08-04 16:40 ` Pedro Alves
@ 2015-08-05 11:41 ` Yao Qi
2015-08-05 15:10 ` Pedro Alves
0 siblings, 1 reply; 16+ messages in thread
From: Yao Qi @ 2015-08-05 11:41 UTC (permalink / raw)
To: Pedro Alves; +Cc: Yao Qi, gdb-patches
Pedro Alves <palves@redhat.com> writes:
> I've now extended the test to run in a few different modes, along
> a couple different axis. One axis is with/without conditional
> breakpoints on the target enabled. That exposes the same fails you
> saw on arm, on x86 gdbserver as well. And then the other axis is
> with/without joining _all_ threads before exiting. If we gracefully
> terminate the breakpoint thread (new mode), then the test should be
> passing everywhere, because what fails is gdb's handling of the inferior
> disappearing while a thread is stopped (and being inspected).
> Therefore that new mode is not kfailed.
>
> For testing convenience, I've pushed this along with the previous patch
> to the users/palves/gdbserver-fork-issues branch on sourceware.org.
> Let me know if this works for you.
Thanks, Pedro. There are no fails on arm-linux with GDBserver. Some
comments on your patch below,
> @@ -3089,8 +3142,25 @@ linux_wait_1 (ptid_t ptid,
> info_p = &info;
> else
> info_p = NULL;
> - linux_resume_one_lwp (event_child, event_child->stepping,
> - WSTOPSIG (w), info_p);
> +
> + if (step_over_finished)
> + {
> + /* We cancelled this thread's step-over above. We still
> + need to unsuspend all other LWPs, and set then back
s/set then/set them/?
> + running again while the signal handler runs. */
> + unsuspend_all_lwps (event_child);
> +
> + /* Enqueue the pending signal info so that proceed_all_lwps
> + doesn't lose it. */
> + enqueue_pending_signal (event_child, WSTOPSIG (w), info_p);
> +
> + proceed_all_lwps ();
> + }
> + else
> + {
> + linux_resume_one_lwp (event_child, event_child->stepping,
> + WSTOPSIG (w), info_p);
> + }
> return ignore_event (ourstatus);
> }
>
> @@ -3111,8 +3181,15 @@ linux_wait_1 (ptid_t ptid,
> || (current_thread->last_resume_kind == resume_step
> && !in_step_range)
> || event_child->stop_reason == TARGET_STOPPED_BY_WATCHPOINT
> - || (!step_over_finished && !in_step_range
> - && !bp_explains_trap && !trace_event)
> + || (!in_step_range
> + && !bp_explains_trap
> + && !trace_event
> + /* A step-over was finished just now? */
> + && !step_over_finished
> + /* A step-over had been finished previously,
> + and the single-step was left pending? */
> + && !(current_thread->last_resume_kind == resume_continue
> + && event_child->stop_reason == TARGET_STOPPED_BY_SINGLE_STEP))
> || (gdb_breakpoint_here (event_child->stop_pc)
I don't fully understand this, what is a case that "step-over had been
finished previously, but the single-step was left pending"?
> (linux_wait_1): If passing a signal to the inferior after
> finishing a step-over, unsuspend and re-resume all lwps. If we
> see a single-step event but the thread should be continuing, don't
> pass the trap to gdb.
however, the explanations in ChangeLog look more reasonable.
> && gdb_condition_true_at_breakpoint (event_child->stop_pc)
> && gdb_no_commands_at_breakpoint (event_child->stop_pc))
> @@ -4189,16 +4298,36 @@ static int
> start_step_over (struct lwp_info *lwp)
> {
> struct thread_info *thread = get_lwp_thread (lwp);
> + ptid_t thread_ptid;
> struct thread_info *saved_thread;
> CORE_ADDR pc;
> int step;
>
> + thread_ptid = ptid_of (thread);
> +
> if (debug_threads)
> debug_printf ("Starting step-over on LWP %ld. Stopping all threads\n",
> lwpid_of (thread));
>
> stop_all_lwps (1, lwp);
> - gdb_assert (lwp->suspended == 0);
> +
> + /* Re-find the LWP as it may have exited. */
> + lwp = find_lwp_pid (thread_ptid);
> + if (lwp == NULL || lwp_is_marked_dead (lwp))
> + {
> + if (debug_threads)
> + debug_printf ("Step-over thread died "
> + "(another thread exited the process?).\n");
> + unstop_all_lwps (1, lwp);
> + return 0;
> + }
> +
> + if (lwp->suspended != 0)
> + {
> + internal_error (__FILE__, __LINE__,
> + "LWP %ld suspended=%d\n", lwpid_of (thread),
> + lwp->suspended);
> + }
>
> if (debug_threads)
> debug_printf ("Done stopping all threads for step-over.\n");
> @@ -4229,7 +4358,19 @@ start_step_over (struct lwp_info *lwp)
>
> current_thread = saved_thread;
>
> - linux_resume_one_lwp (lwp, step, 0, NULL);
> + TRY
> + {
> + linux_resume_one_lwp_throw (lwp, step, 0, NULL);
> + }
IIUC, we do TRY/CATCH here because thread may have exited, but we've
done that in this function (start_step_over), do we still need to worry
about these exited threads?
> + CATCH (ex, RETURN_MASK_ERROR)
> + {
> + unstop_all_lwps (1, lwp);
> +
> + if (!check_ptrace_stopped_lwp_gone (lwp))
I am thinking about the order of these two function calls. Does the
order matter here? Probably we need to check_ptrace_stopped_lwp_gone
first before unstop_all_lwps.
> + throw_exception (ex);
> + return 0;
> + }
> + END_CATCH
>
> /* Require next event from this LWP. */
> step_over_bkpt = thread->entry.id;
> @@ -4270,6 +4411,39 @@ finish_step_over (struct lwp_info *lwp)
> return 0;
> }
>
> +/* If there's a step over in progress, wait until all threads stop
> + (that is, until the stepping thread finishes its step), and
> + unsuspend all lwps. The stepping thread ends with its status
> + pending, which is processed later when we get back to processing
> + events. */
> +
> +static void
> +complete_ongoing_step_over (void)
> +{
> + if (!ptid_equal (step_over_bkpt, null_ptid))
> + {
> + struct lwp_info *lwp;
> + int wstat;
> + int ret;
> +
> + if (debug_threads)
> + debug_printf ("detach: step over in progress, finish it first\n");
"detach:" in the debug output looks obsolete.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH/7.10 2/2] gdbserver: Fix non-stop / fork / step-over issues
2015-08-05 11:41 ` Yao Qi
@ 2015-08-05 15:10 ` Pedro Alves
2015-08-06 9:44 ` [pushed] " Pedro Alves
0 siblings, 1 reply; 16+ messages in thread
From: Pedro Alves @ 2015-08-05 15:10 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 08/05/2015 12:41 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> I've now extended the test to run in a few different modes, along
>> a couple different axis. One axis is with/without conditional
>> breakpoints on the target enabled. That exposes the same fails you
>> saw on arm, on x86 gdbserver as well. And then the other axis is
>> with/without joining _all_ threads before exiting. If we gracefully
>> terminate the breakpoint thread (new mode), then the test should be
>> passing everywhere, because what fails is gdb's handling of the inferior
>> disappearing while a thread is stopped (and being inspected).
>> Therefore that new mode is not kfailed.
>>
>> For testing convenience, I've pushed this along with the previous patch
>> to the users/palves/gdbserver-fork-issues branch on sourceware.org.
>> Let me know if this works for you.
>
> Thanks, Pedro. There are no fails on arm-linux with GDBserver. Some
> comments on your patch below,
Thanks Yao, that's great.
Thinking about this some more made me want to split both the patch
and test in two. The process-exits-while-handling-breakpoints issues
happen independently of the fork issues, so I'll make the fork test always
join all threads before exiting. Splitting will result in two
cleaner and more directed tests. As per your testing, the fork test
will not need any kfail.
I'll be back to answer your questions and address your review when
I have a moment.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH/7.10 2/2] gdbserver: Fix non-stop / fork / step-over issues
2015-07-31 19:02 ` Pedro Alves
@ 2015-08-05 22:19 ` Don Breazeal
2015-08-06 10:09 ` Pedro Alves
0 siblings, 1 reply; 16+ messages in thread
From: Don Breazeal @ 2015-08-05 22:19 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 7/31/2015 12:02 PM, Pedro Alves wrote:
> On 07/31/2015 07:04 PM, Don Breazeal wrote:
>> On 7/31/2015 10:03 AM, Pedro Alves wrote:
>>> Ref: https://sourceware.org/ml/gdb-patches/2015-07/msg00868.html
>>>
>>> This adds a test that has a multithreaded program have several threads
>>> continuously fork, while another thread continuously steps over a
>>> breakpoint.
>>
>> Wow.
>>
>
> If gdb survives these stress tests, it can hold up to anything. :-)
>
>>> - The test runs with both "set detach-on-fork" on and off. When off,
>>> it exercises the case of GDB detaching the fork child explicitly.
>>> When on, it exercises the case of gdb resuming the child
>>> explicitly. In the "off" case, gdb seems to exponentially become
>>> slower as new inferiors are created. This is _very_ noticeable as
>>> with only 100 inferiors gdb is crawling already, which makes the
>>> test take quite a bit to run. For that reason, I've disabled the
>>> "off" variant for now.
>>
>> Bummer. I was going to ask whether this use-case justifies disabling
>> the feature completely,
>
> Note that this being a stress test, may not be representative of a
> real work load. I'm assuming most real use cases won't be
> so demanding.
>
>> but since the whole follow-fork mechanism is of
>> limited usefulness without exec events, the question is likely moot
>> anyway.
>
> Yeah. There are use cases with fork alone, but combined with exec is
> much more useful. I'll take a look at your exec patches soon; I'm very
> much looking forward to have that in.
>
>>
>> Do you have any thoughts about whether this slowdown is caused by the
>> fork event machinery or by some more general gdbserver multiple
>> inferior problem?
>
> Not sure.
>
> The number of forks live at a given time in the test is constant
> -- each thread forks and waits for the child to exit until it forks
> again. But if you run the test, you see that the first
> few inferiors are created quickly, and then as the inferior number
> grows, new inferiors are added at a slower and slower.
> I'd suspect the problem to be on the gdb side. But the test
> fails on native, so it's not easy to get gdbserver out of
> the picture for a quick check.
>
> It feels like some data structures are leaking, but
> still reacheable, and then a bunch of linear walks end up costing
> more and more. I once added the prune_inferiors call at the end
> of normal_stop to handle a slowdown like this. It feels like
> something similar to that.
>
> With detach "on" alone, it takes under 2 seconds against gdbserver
> for me.
>
> If I remove the breakpoint from the test, and reenable both detach on/off,
> it ends in around 10-20 seconds. That's still a lot slower
> than "detach on" along, but gdb has to insert/remove breakpoints in the
> child and load its symbols (well, it could avoid that, given the
> child is a clone of the parent, but we're not there yet), so
> not entirely unexpected.
>
> But pristine, with both detach on/off, it takes almost 2 minutes
> here. ( and each thread only spawns 10 forks, my first attempt
> was shooting for 100 :-) )
>
> I also suspected all the thread stop/restarting gdbserver does
> both to step over breakpoints, and to insert/remove breakpoints.
> But then again with detach on, there are 12 threads, with detach
> off, at most 22. So that'd be odd. Unless the data structure
> leaks are on gdbserver's side. But then I'd think that tests
> like attach-many-short-lived-threads.exp or non-stop-fair-events.exp
> would have already exposed something like that.
>
>>
>> Are you planning to look at the slowdown?
>
> Nope, at least not in the immediate future.
>
>> Can I help out? I have an
>> interest in having detach-on-fork 'off' enabled. :-S
>
> That'd be much appreciated. :-) At least identifying the
> culprit would be very nice. I too would love for our
> multi-process support to be rock solid.
>
Hi Pedro,
I spent some time looking at this, and I found at least one of the
culprits affecting performance. Without going through the details of
how I arrived at this conclusion, if I insert
gdb_test_no_output "set sysroot /"
just before the call to runto_main, it cuts the wall clock time by at
least half. Running with just the 'detach-on-fork=off' case, it went
from 41 secs to 20 secs on one system, and 1:21 to 0:27 and 1:50 to 0:41
on another. Successive runs without set sysroot resulted in
successively decreasing run times, presumably due to filesystem caching.
I ran strace -cw to collect wall clock time (strace 4.9 and above
support '-w' for wall time), and saw this:
Without set sysroot /:
% time seconds usecs/call calls errors syscall^M
------ ----------- ----------- --------- --------- ----------------^M
25.90 14.620339 4 3666141 202 ptrace^M
25.21 14.229421 81 175135 57 select^M
14.42 8.139715 13 641874 7 write^M
10.65 6.012699 4 1397576 670469 read^M
7.52 4.245209 4 1205014 104 wait4^M
4.90 2.765111 3 847985 rt_sigprocmask^M
With set sysroot /:
% time seconds usecs/call calls errors syscall^M
------ ----------- ----------- --------- --------- ----------------^M
32.91 6.885008 148 46665 43 select^M
21.59 4.516311 4 1158530 202 ptrace^M
11.15 2.332491 13 184229 2 write^M
9.07 1.897401 4 422122 203552 read^M
6.77 1.415918 42 34076 53 open^M
6.27 1.312490 3 378702 103 wait4^M
4.00 0.835731 3 262195 rt_sigprocmask^M
The # calls and times for each case varied from run to run, but the
relative proportions stayed reasonably similar. I'm not sure why the
unmodified case has so many more calls to ptrace, but it was not an
anomaly, I saw this in multiple runs.
Note that I used the original version of the test that you posted, not
the update on your branch. Also, I didn't make the set sysroot command
conditional on running with a remote or gdbserver target, since it was
just an experiment.
Do you think there is more to the slowdown than this? As you said
above, detach-on-fork 'off' is going to take longer than 'on'. It may
be a little while before I can get back to this, so I thought I'd share
what I found. Let me know if you think this change will be sufficient.
thanks
--Don
^ permalink raw reply [flat|nested] 16+ messages in thread
* [pushed][PATCH 1/3] Linux gdbserver fork event debug output (Re: [PATCH/7.10 1/2] Linux gdbserver confused when event randomization returns a process exit event)
2015-08-03 10:59 ` Yao Qi
@ 2015-08-06 9:34 ` Pedro Alves
2015-08-06 9:43 ` [pushed]Re: [PATCH/7.10 1/2] Linux gdbserver confused when event randomization returns a process exit event Pedro Alves
1 sibling, 0 replies; 16+ messages in thread
From: Pedro Alves @ 2015-08-06 9:34 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 08/03/2015 11:58 AM, Yao Qi wrote:
> Looks this code is dead code even without your patch. This code is
> guarded by "if (!report_to_gdb)", but if report_to_gdb is false,
> "(event_child->waitstatus.kind != TARGET_WAITKIND_IGNORE)" is false too.
>
Indeed. I've pushed this preliminary patch, to address that.
Thanks!
--------
From ad071a3055b9b47dad340c1a1cb3a9b39529cef0 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 6 Aug 2015 10:30:16 +0100
Subject: [PATCH 1/3] Linux gdbserver fork event debug output
The "extended event with waitstatus" debug output is unreachable, as
it is guarded by "if (!report_to_gdb)". If extended_event_reported is
true, then so is report_to_gdb. Move it to where we print why we're
reporting an event to GDB.
Also, the debug output currently tries to print the wrong struct
target_waitstatus.
gdb/gdbserver/ChangeLog:
2015-08-06 Pedro Alves <palves@redhat.com>
* linux-low.c (linux_wait_1): Move fork event output out of the
!report_to_gdb check. Pass event_child->waitstatus to
target_waitstatus_to_string instead of ourstatus.
---
gdb/gdbserver/ChangeLog | 6 ++++++
gdb/gdbserver/linux-low.c | 16 +++++++++-------
2 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 43cae40..eb1101c 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,9 @@
+2015-08-06 Pedro Alves <palves@redhat.com>
+
+ * linux-low.c (linux_wait_1): Move fork event output out of the
+ !report_to_gdb check. Pass event_child->waitstatus to
+ target_waitstatus_to_string instead of ourstatus.
+
2015-08-04 Yao Qi <yao.qi@linaro.org>
* linux-aarch64-low.c (aarch64_supports_tracepoints): Return 0
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index f1e1d72..76b212d 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -3153,13 +3153,6 @@ linux_wait_1 (ptid_t ptid,
paddress (event_child->stop_pc),
paddress (event_child->step_range_start),
paddress (event_child->step_range_end));
- if (extended_event_reported (&event_child->waitstatus))
- {
- char *str = target_waitstatus_to_string (ourstatus);
- debug_printf ("LWP %ld: extended event with waitstatus %s\n",
- lwpid_of (get_lwp_thread (event_child)), str);
- xfree (str);
- }
}
/* We're not reporting this breakpoint to GDB, so apply the
@@ -3190,6 +3183,15 @@ linux_wait_1 (ptid_t ptid,
if (debug_threads)
{
+ if (extended_event_reported (&event_child->waitstatus))
+ {
+ char *str;
+
+ str = target_waitstatus_to_string (&event_child->waitstatus);
+ debug_printf ("LWP %ld: extended event with waitstatus %s\n",
+ lwpid_of (get_lwp_thread (event_child)), str);
+ xfree (str);
+ }
if (current_thread->last_resume_kind == resume_step)
{
if (event_child->step_range_start == event_child->step_range_end)
--
1.9.3
^ permalink raw reply [flat|nested] 16+ messages in thread
* [pushed]Re: [PATCH/7.10 1/2] Linux gdbserver confused when event randomization returns a process exit event
2015-08-03 10:59 ` Yao Qi
2015-08-06 9:34 ` [pushed][PATCH 1/3] Linux gdbserver fork event debug output (Re: [PATCH/7.10 1/2] Linux gdbserver confused when event randomization returns a process exit event) Pedro Alves
@ 2015-08-06 9:43 ` Pedro Alves
1 sibling, 0 replies; 16+ messages in thread
From: Pedro Alves @ 2015-08-06 9:43 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 08/03/2015 11:58 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> @@ -3128,7 +3116,7 @@ linux_wait_1 (ptid_t ptid,
>> || (gdb_breakpoint_here (event_child->stop_pc)
>> && gdb_condition_true_at_breakpoint (event_child->stop_pc)
>> && gdb_no_commands_at_breakpoint (event_child->stop_pc))
>> - || extended_event_reported (&event_child->waitstatus));
>> + || event_child->waitstatus.kind != TARGET_WAITKIND_IGNORE);
>>
>> run_breakpoint_commands (event_child->stop_pc);
>>
>> @@ -3150,9 +3138,11 @@ linux_wait_1 (ptid_t ptid,
>> paddress (event_child->stop_pc),
>> paddress (event_child->step_range_start),
>> paddress (event_child->step_range_end));
>> - if (extended_event_reported (&event_child->waitstatus))
>> + if (event_child->waitstatus.kind != TARGET_WAITKIND_IGNORE)
>> {
>> - char *str = target_waitstatus_to_string (ourstatus);
>> + char *str;
>> +
>> + str = target_waitstatus_to_string (&event_child->waitstatus);
>> debug_printf ("LWP %ld: extended event with waitstatus %s\n",
>> lwpid_of (get_lwp_thread (event_child)), str);
>> xfree (str);
>
> Looks this code is dead code even without your patch. This code is
> guarded by "if (!report_to_gdb)", but if report_to_gdb is false,
> "(event_child->waitstatus.kind != TARGET_WAITKIND_IGNORE)" is false too.
That bit went in separately, so here's the updated patch that I pushed.
I'll push it to 7.10 too in a bit.
---
From 00db26facc14ac830adef704bba9b24d0d366ddf Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 6 Aug 2015 10:30:17 +0100
Subject: [PATCH 2/3] Linux gdbserver confused when event randomization picks
process exit event
The tail end of linux_wait_1 isn't expecting that the select_event_lwp
machinery can pick a whole-process exit event to report to GDB. When
that happens, both gdb and gdbserver end up quite confused:
...
(gdb)
[Thread 24971.24971] #1 stopped.
0x0000003615a011f0 in ?? ()
c&
Continuing.
(gdb) [New Thread 24971.24981]
[New Thread 24983.24983]
[New Thread 24971.24982]
[Thread 24983.24983] #3 stopped.
0x0000003615ebc7cc in __libc_fork () at ../nptl/sysdeps/unix/sysv/linux/fork.c:130
130 pid = ARCH_FORK ();
[New Thread 24984.24984]
Error in re-setting breakpoint -16: PC register is not available
Error in re-setting breakpoint -17: PC register is not available
Error in re-setting breakpoint -18: PC register is not available
Error in re-setting breakpoint -19: PC register is not available
Error in re-setting breakpoint -24: PC register is not available
Error in re-setting breakpoint -25: PC register is not available
Error in re-setting breakpoint -26: PC register is not available
Error in re-setting breakpoint -27: PC register is not available
Error in re-setting breakpoint -28: PC register is not available
Error in re-setting breakpoint -29: PC register is not available
Error in re-setting breakpoint -30: PC register is not available
PC register is not available
(gdb)
gdb/gdbserver/ChangeLog:
2015-08-06 Pedro Alves <palves@redhat.com>
* linux-low.c (add_lwp): Set waitstatus to TARGET_WAITKIND_IGNORE.
(linux_thread_alive): Use lwp_is_marked_dead.
(extended_event_reported): Delete.
(linux_wait_1): Check if waitstatus is TARGET_WAITKIND_IGNORE
instead of extended_event_reported.
(mark_lwp_dead): Don't set the 'dead' flag. Store the waitstatus
as well.
(lwp_is_marked_dead): New function.
(lwp_running): Use lwp_is_marked_dead.
* linux-low.h: Delete 'dead' field, and update 'waitstatus's
comment.
---
gdb/gdbserver/ChangeLog | 14 +++++++++++
gdb/gdbserver/linux-low.c | 64 ++++++++++++++++++++++++++---------------------
gdb/gdbserver/linux-low.h | 11 +++-----
3 files changed, 54 insertions(+), 35 deletions(-)
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index eb1101c..77b4330 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,5 +1,19 @@
2015-08-06 Pedro Alves <palves@redhat.com>
+ * linux-low.c (add_lwp): Set waitstatus to TARGET_WAITKIND_IGNORE.
+ (linux_thread_alive): Use lwp_is_marked_dead.
+ (extended_event_reported): Delete.
+ (linux_wait_1): Check if waitstatus is TARGET_WAITKIND_IGNORE
+ instead of extended_event_reported.
+ (mark_lwp_dead): Don't set the 'dead' flag. Store the waitstatus
+ as well.
+ (lwp_is_marked_dead): New function.
+ (lwp_running): Use lwp_is_marked_dead.
+ * linux-low.h: Delete 'dead' field, and update 'waitstatus's
+ comment.
+
+2015-08-06 Pedro Alves <palves@redhat.com>
+
* linux-low.c (linux_wait_1): Move fork event output out of the
!report_to_gdb check. Pass event_child->waitstatus to
target_waitstatus_to_string instead of ourstatus.
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 76b212d..6a7182a 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -267,6 +267,7 @@ static int linux_wait_for_event (ptid_t ptid, int *wstat, int options);
static struct lwp_info *add_lwp (ptid_t ptid);
static int linux_stopped_by_watchpoint (void);
static void mark_lwp_dead (struct lwp_info *lwp, int wstat);
+static int lwp_is_marked_dead (struct lwp_info *lwp);
static void proceed_all_lwps (void);
static int finish_step_over (struct lwp_info *lwp);
static int kill_lwp (unsigned long lwpid, int signo);
@@ -746,8 +747,9 @@ add_lwp (ptid_t ptid)
{
struct lwp_info *lwp;
- lwp = (struct lwp_info *) xmalloc (sizeof (*lwp));
- memset (lwp, 0, sizeof (*lwp));
+ lwp = (struct lwp_info *) xcalloc (1, sizeof (*lwp));
+
+ lwp->waitstatus.kind = TARGET_WAITKIND_IGNORE;
if (the_low_target.new_thread != NULL)
the_low_target.new_thread (lwp);
@@ -1396,7 +1398,7 @@ linux_thread_alive (ptid_t ptid)
exited but we still haven't been able to report it to GDB, we'll
hold on to the last lwp of the dead process. */
if (lwp != NULL)
- return !lwp->dead;
+ return !lwp_is_marked_dead (lwp);
else
return 0;
}
@@ -2750,20 +2752,6 @@ ignore_event (struct target_waitstatus *ourstatus)
return null_ptid;
}
-/* Return non-zero if WAITSTATUS reflects an extended linux
- event. Otherwise, return zero. */
-
-static int
-extended_event_reported (const struct target_waitstatus *waitstatus)
-{
- if (waitstatus == NULL)
- return 0;
-
- return (waitstatus->kind == TARGET_WAITKIND_FORKED
- || waitstatus->kind == TARGET_WAITKIND_VFORKED
- || waitstatus->kind == TARGET_WAITKIND_VFORK_DONE);
-}
-
/* Wait for process, returns status. */
static ptid_t
@@ -3131,7 +3119,7 @@ linux_wait_1 (ptid_t ptid,
|| (gdb_breakpoint_here (event_child->stop_pc)
&& gdb_condition_true_at_breakpoint (event_child->stop_pc)
&& gdb_no_commands_at_breakpoint (event_child->stop_pc))
- || extended_event_reported (&event_child->waitstatus));
+ || event_child->waitstatus.kind != TARGET_WAITKIND_IGNORE);
run_breakpoint_commands (event_child->stop_pc);
@@ -3183,7 +3171,7 @@ linux_wait_1 (ptid_t ptid,
if (debug_threads)
{
- if (extended_event_reported (&event_child->waitstatus))
+ if (event_child->waitstatus.kind != TARGET_WAITKIND_IGNORE)
{
char *str;
@@ -3271,12 +3259,11 @@ linux_wait_1 (ptid_t ptid,
unstop_all_lwps (1, event_child);
}
- if (extended_event_reported (&event_child->waitstatus))
+ if (event_child->waitstatus.kind != TARGET_WAITKIND_IGNORE)
{
- /* If the reported event is a fork, vfork or exec, let GDB know. */
- ourstatus->kind = event_child->waitstatus.kind;
- ourstatus->value = event_child->waitstatus.value;
-
+ /* If the reported event is an exit, fork, vfork or exec, let
+ GDB know. */
+ *ourstatus = event_child->waitstatus;
/* Clear the event lwp's waitstatus since we handled it already. */
event_child->waitstatus.kind = TARGET_WAITKIND_IGNORE;
}
@@ -3484,13 +3471,23 @@ suspend_and_send_sigstop_callback (struct inferior_list_entry *entry,
static void
mark_lwp_dead (struct lwp_info *lwp, int wstat)
{
- /* It's dead, really. */
- lwp->dead = 1;
-
/* Store the exit status for later. */
lwp->status_pending_p = 1;
lwp->status_pending = wstat;
+ /* Store in waitstatus as well, as there's nothing else to process
+ for this event. */
+ if (WIFEXITED (wstat))
+ {
+ lwp->waitstatus.kind = TARGET_WAITKIND_EXITED;
+ lwp->waitstatus.value.integer = WEXITSTATUS (wstat);
+ }
+ else if (WIFSIGNALED (wstat))
+ {
+ lwp->waitstatus.kind = TARGET_WAITKIND_SIGNALLED;
+ lwp->waitstatus.value.sig = gdb_signal_from_host (WTERMSIG (wstat));
+ }
+
/* Prevent trying to stop it. */
lwp->stopped = 1;
@@ -3498,6 +3495,17 @@ mark_lwp_dead (struct lwp_info *lwp, int wstat)
lwp->stop_expected = 0;
}
+/* Return true if LWP has exited already, and has a pending exit event
+ to report to GDB. */
+
+static int
+lwp_is_marked_dead (struct lwp_info *lwp)
+{
+ return (lwp->status_pending_p
+ && (WIFEXITED (lwp->status_pending)
+ || WIFSIGNALED (lwp->status_pending)));
+}
+
/* Wait for all children to stop for the SIGSTOPs we just queued. */
static void
@@ -3614,7 +3622,7 @@ lwp_running (struct inferior_list_entry *entry, void *data)
struct thread_info *thread = (struct thread_info *) entry;
struct lwp_info *lwp = get_thread_lwp (thread);
- if (lwp->dead)
+ if (lwp_is_marked_dead (lwp))
return 0;
if (lwp->stopped)
return 0;
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 24fb015..f8f6e78 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -261,16 +261,13 @@ struct lwp_info
event already received in a wait()). */
int stopped;
- /* 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). */
- int dead;
-
/* When stopped is set, the last wait status recorded for this lwp. */
int last_status;
- /* This is used to store extended ptrace event information until
- it is reported to GDB. */
+ /* If WAITSTATUS->KIND != TARGET_WAITKIND_IGNORE, the waitstatus for
+ this LWP's last event, to pass to GDB without any further
+ processing. This is used to store extended ptrace event
+ information or exit status until it can be reported to GDB. */
struct target_waitstatus waitstatus;
/* When stopped is set, this is where the lwp last stopped, with
--
1.9.3
^ permalink raw reply [flat|nested] 16+ messages in thread
* [pushed] Re: [PATCH/7.10 2/2] gdbserver: Fix non-stop / fork / step-over issues
2015-08-05 15:10 ` Pedro Alves
@ 2015-08-06 9:44 ` Pedro Alves
0 siblings, 0 replies; 16+ messages in thread
From: Pedro Alves @ 2015-08-06 9:44 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 08/05/2015 04:10 PM, Pedro Alves wrote:
> On 08/05/2015 12:41 PM, Yao Qi wrote:
>> Pedro Alves <palves@redhat.com> writes:
>>
>>> I've now extended the test to run in a few different modes, along
>>> a couple different axis. One axis is with/without conditional
>>> breakpoints on the target enabled. That exposes the same fails you
>>> saw on arm, on x86 gdbserver as well. And then the other axis is
>>> with/without joining _all_ threads before exiting. If we gracefully
>>> terminate the breakpoint thread (new mode), then the test should be
>>> passing everywhere, because what fails is gdb's handling of the inferior
>>> disappearing while a thread is stopped (and being inspected).
>>> Therefore that new mode is not kfailed.
>>>
>>> For testing convenience, I've pushed this along with the previous patch
>>> to the users/palves/gdbserver-fork-issues branch on sourceware.org.
>>> Let me know if this works for you.
>>
>> Thanks, Pedro. There are no fails on arm-linux with GDBserver. Some
>> comments on your patch below,
>
> Thanks Yao, that's great.
>
> Thinking about this some more made me want to split both the patch
> and test in two. The process-exits-while-handling-breakpoints issues
> happen independently of the fork issues, so I'll make the fork test always
> join all threads before exiting. Splitting will result in two
> cleaner and more directed tests. As per your testing, the fork test
> will not need any kfail.
I've done that now. I'll send the
process-exits-while-handling-breakpoints test and patch separately.
>> @@ -3089,8 +3142,25 @@ linux_wait_1 (ptid_t ptid,
>> info_p = &info;
>> else
>> info_p = NULL;
>> - linux_resume_one_lwp (event_child, event_child->stepping,
>> - WSTOPSIG (w), info_p);
>> +
>> + if (step_over_finished)
>> + {
>> + /* We cancelled this thread's step-over above. We still
>> + need to unsuspend all other LWPs, and set then back
>
> s/set then/set them/?
Fixed.
>> @@ -3111,8 +3181,15 @@ linux_wait_1 (ptid_t ptid,
>> || (current_thread->last_resume_kind == resume_step
>> && !in_step_range)
>> || event_child->stop_reason == TARGET_STOPPED_BY_WATCHPOINT
>> - || (!step_over_finished && !in_step_range
>> - && !bp_explains_trap && !trace_event)
>> + || (!in_step_range
>> + && !bp_explains_trap
>> + && !trace_event
>> + /* A step-over was finished just now? */
>> + && !step_over_finished
>> + /* A step-over had been finished previously,
>> + and the single-step was left pending? */
>> + && !(current_thread->last_resume_kind == resume_continue
>> + && event_child->stop_reason == TARGET_STOPPED_BY_SINGLE_STEP))
>> || (gdb_breakpoint_here (event_child->stop_pc)
>
> I don't fully understand this, what is a case that "step-over had been
> finished previously, but the single-step was left pending"?
>
>> (linux_wait_1): If passing a signal to the inferior after
>> finishing a step-over, unsuspend and re-resume all lwps. If we
>> see a single-step event but the thread should be continuing, don't
>> pass the trap to gdb.
>
> however, the explanations in ChangeLog look more reasonable.
Indeed. I've moved the comment above to where the rest of the
explanations are and clarified it. I also renamed the test,
and cleaned it up a little bit.
Below's what I pushed to master. I'll push it to 7.10 too in a bit.
Thanks!
---
From 863d01bde2725d009c45ab7e9ba1dbf3f5b923f8 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 6 Aug 2015 10:30:18 +0100
Subject: [PATCH 3/3] gdbserver: Fix non-stop / fork / step-over issues
Ref: https://sourceware.org/ml/gdb-patches/2015-07/msg00868.html
This adds a test that has a multithreaded program have several threads
continuously fork, while another thread continuously steps over a
breakpoint.
This exposes several intertwined issues, which this patch addresses:
- When we're stopping and suspending threads, some thread may fork,
and we missed setting its suspend count to 1, like we do when a new
clone/thread is detected. When we next unsuspend threads, the fork
child's suspend count goes below 0, which is bogus and fails an
assertion.
- If a step-over is cancelled because a signal arrives, but then gdb
is not interested in the signal, we pass the signal straight back
to the inferior. However, we miss that we need to re-increment the
suspend counts of all other threads that had been paused for the
step-over. As a result, other threads indefinitely end up stuck
stopped.
- If a detach request comes in just while gdbserver is handling a
step-over (in the test at hand, this is GDB detaching the fork
child), gdbserver internal errors in stabilize_thread's helpers,
which assert that all thread's suspend counts are 0 (otherwise we
wouldn't be able to move threads out of the jump pads). The
suspend counts aren't 0 while a step-over is in progress, because
all threads but the one stepping past the breakpoint must remain
paused until the step-over finishes and the breakpoint can be
reinserted.
- Occasionally, we see "BAD - reinserting but not stepping." being
output (from within linux_resume_one_lwp_throw). That was because
GDB pokes memory while gdbserver is busy with a step-over, and that
suspends threads, and then re-resumes them with proceed_one_lwp,
which missed another reason to tell linux_resume_one_lwp that the
thread should be set back to stepping.
- In a couple places, we were resuming threads that are meant to be
suspended. E.g., when a vCont;c/s request for thread B comes in
just while gdbserver is stepping thread A past a breakpoint. The
resume for thread B must be deferred until the step-over finishes.
- The test runs with both "set detach-on-fork" on and off. When off,
it exercises the case of GDB detaching the fork child explicitly.
When on, it exercises the case of gdb resuming the child
explicitly. In the "off" case, gdb seems to exponentially become
slower as new inferiors are created. This is _very_ noticeable as
with only 100 inferiors gdb is crawling already, which makes the
test take quite a bit to run. For that reason, I've disabled the
"off" variant for now.
gdb/ChangeLog:
2015-08-06 Pedro Alves <palves@redhat.com>
* target/waitstatus.h (enum target_stop_reason)
<TARGET_STOPPED_BY_SINGLE_STEP>: New value.
gdb/gdbserver/ChangeLog:
2015-08-06 Pedro Alves <palves@redhat.com>
* linux-low.c (handle_extended_wait): Set the fork child's suspend
count if stopping and suspending threads.
(check_stopped_by_breakpoint): If stopped by trace, set the LWP's
stop reason to TARGET_STOPPED_BY_SINGLE_STEP.
(linux_detach): Complete an ongoing step-over.
(lwp_suspended_inc, lwp_suspended_decr): New functions. Use
throughout.
(resume_stopped_resumed_lwps): Don't resume a suspended thread.
(linux_wait_1): If passing a signal to the inferior after
finishing a step-over, unsuspend and re-resume all lwps. If we
see a single-step event but the thread should be continuing, don't
pass the trap to gdb.
(stuck_in_jump_pad_callback, move_out_of_jump_pad_callback): Use
internal_error instead of gdb_assert.
(enqueue_pending_signal): New function.
(check_ptrace_stopped_lwp_gone): Add debug output.
(start_step_over): Use internal_error instead of gdb_assert.
(complete_ongoing_step_over): New function.
(linux_resume_one_thread): Don't resume a suspended thread.
(proceed_one_lwp): If the LWP is stepping over a breakpoint, reset
it stepping.
gdb/testsuite/ChangeLog:
2015-08-06 Pedro Alves <palves@redhat.com>
* gdb.threads/forking-threads-plus-breakpoint.exp: New file.
* gdb.threads/forking-threads-plus-breakpoint.c: New file.
---
gdb/ChangeLog | 5 +
gdb/gdbserver/ChangeLog | 24 +++
gdb/testsuite/ChangeLog | 5 +
gdb/gdbserver/linux-low.c | 221 ++++++++++++++++++---
gdb/target/waitstatus.h | 5 +-
.../gdb.threads/forking-threads-plus-breakpoint.c | 139 +++++++++++++
.../forking-threads-plus-breakpoint.exp | 117 +++++++++++
7 files changed, 486 insertions(+), 30 deletions(-)
create mode 100644 gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.c
create mode 100644 gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7ea7a02..65a540c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2015-08-06 Pedro Alves <palves@redhat.com>
+
+ * target/waitstatus.h (enum target_stop_reason)
+ <TARGET_STOPPED_BY_SINGLE_STEP>: New value.
+
2015-08-05 Pedro Alves <palves@redhat.com>
Joel Brobecker <brobecker@adacore.com>
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 77b4330..5588fb0 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,5 +1,29 @@
2015-08-06 Pedro Alves <palves@redhat.com>
+ * linux-low.c (handle_extended_wait): Set the fork child's suspend
+ count if stopping and suspending threads.
+ (check_stopped_by_breakpoint): If stopped by trace, set the LWP's
+ stop reason to TARGET_STOPPED_BY_SINGLE_STEP.
+ (linux_detach): Complete an ongoing step-over.
+ (lwp_suspended_inc, lwp_suspended_decr): New functions. Use
+ throughout.
+ (resume_stopped_resumed_lwps): Don't resume a suspended thread.
+ (linux_wait_1): If passing a signal to the inferior after
+ finishing a step-over, unsuspend and re-resume all lwps. If we
+ see a single-step event but the thread should be continuing, don't
+ pass the trap to gdb.
+ (stuck_in_jump_pad_callback, move_out_of_jump_pad_callback): Use
+ internal_error instead of gdb_assert.
+ (enqueue_pending_signal): New function.
+ (check_ptrace_stopped_lwp_gone): Add debug output.
+ (start_step_over): Use internal_error instead of gdb_assert.
+ (complete_ongoing_step_over): New function.
+ (linux_resume_one_thread): Don't resume a suspended thread.
+ (proceed_one_lwp): If the LWP is stepping over a breakpoint, reset
+ it stepping.
+
+2015-08-06 Pedro Alves <palves@redhat.com>
+
* linux-low.c (add_lwp): Set waitstatus to TARGET_WAITKIND_IGNORE.
(linux_thread_alive): Use lwp_is_marked_dead.
(extended_event_reported): Delete.
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 59d0be7..ad63b02 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2015-08-06 Pedro Alves <palves@redhat.com>
+
+ * gdb.threads/forking-threads-plus-breakpoint.exp: New file.
+ * gdb.threads/forking-threads-plus-breakpoint.c: New file.
+
2015-08-05 Pedro Alves <palves@redhat.com>
* gdb.threads/next-while-other-thread-longjmps.c: New file.
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 6a7182a..98fffc9 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -271,6 +271,8 @@ static int lwp_is_marked_dead (struct lwp_info *lwp);
static void proceed_all_lwps (void);
static int finish_step_over (struct lwp_info *lwp);
static int kill_lwp (unsigned long lwpid, int signo);
+static void enqueue_pending_signal (struct lwp_info *lwp, int signal, siginfo_t *info);
+static void complete_ongoing_step_over (void);
/* When the event-loop is doing a step-over, this points at the thread
being stepped. */
@@ -489,6 +491,15 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat)
child_thr->last_resume_kind = resume_stop;
child_thr->last_status.kind = TARGET_WAITKIND_STOPPED;
+ /* If we're suspending all threads, leave this one suspended
+ too. */
+ if (stopping_threads == STOPPING_AND_SUSPENDING_THREADS)
+ {
+ if (debug_threads)
+ debug_printf ("HEW: leaving child suspended\n");
+ child_lwp->suspended = 1;
+ }
+
parent_proc = get_thread_process (event_thr);
child_proc->attached = parent_proc->attached;
clone_all_breakpoints (&child_proc->breakpoints,
@@ -688,6 +699,8 @@ check_stopped_by_breakpoint (struct lwp_info *lwp)
debug_printf ("CSBB: %s stopped by trace\n",
target_pid_to_str (ptid_of (thr)));
}
+
+ lwp->stop_reason = TARGET_STOPPED_BY_SINGLE_STEP;
}
}
}
@@ -1318,6 +1331,11 @@ linux_detach (int pid)
if (process == NULL)
return -1;
+ /* As there's a step over already in progress, let it finish first,
+ otherwise nesting a stabilize_threads operation on top gets real
+ messy. */
+ complete_ongoing_step_over ();
+
/* Stop all threads before detaching. First, ptrace requires that
the thread is stopped to sucessfully detach. Second, thread_db
may need to uninstall thread event breakpoints from memory, which
@@ -1686,6 +1704,39 @@ not_stopped_callback (struct inferior_list_entry *entry, void *arg)
return 0;
}
+/* Increment LWP's suspend count. */
+
+static void
+lwp_suspended_inc (struct lwp_info *lwp)
+{
+ lwp->suspended++;
+
+ if (debug_threads && lwp->suspended > 4)
+ {
+ struct thread_info *thread = get_lwp_thread (lwp);
+
+ debug_printf ("LWP %ld has a suspiciously high suspend count,"
+ " suspended=%d\n", lwpid_of (thread), lwp->suspended);
+ }
+}
+
+/* Decrement LWP's suspend count. */
+
+static void
+lwp_suspended_decr (struct lwp_info *lwp)
+{
+ lwp->suspended--;
+
+ if (lwp->suspended < 0)
+ {
+ struct thread_info *thread = get_lwp_thread (lwp);
+
+ internal_error (__FILE__, __LINE__,
+ "unsuspend LWP %ld, suspended=%d\n", lwpid_of (thread),
+ lwp->suspended);
+ }
+}
+
/* This function should only be called if the LWP got a SIGTRAP.
Handle any tracepoint steps or hits. Return true if a tracepoint
@@ -1703,7 +1754,7 @@ handle_tracepoints (struct lwp_info *lwp)
uninsert tracepoints. To do this, we temporarily pause all
threads, unpatch away, and then unpause threads. We need to make
sure the unpausing doesn't resume LWP too. */
- lwp->suspended++;
+ lwp_suspended_inc (lwp);
/* And we need to be sure that any all-threads-stopping doesn't try
to move threads out of the jump pads, as it could deadlock the
@@ -1719,7 +1770,7 @@ handle_tracepoints (struct lwp_info *lwp)
actions. */
tpoint_related_event |= tracepoint_was_hit (tinfo, lwp->stop_pc);
- lwp->suspended--;
+ lwp_suspended_decr (lwp);
gdb_assert (lwp->suspended == 0);
gdb_assert (!stabilizing_threads || lwp->collecting_fast_tracepoint);
@@ -2179,10 +2230,13 @@ linux_low_filter_event (int lwpid, int wstat)
/* Note that TRAP_HWBKPT can indicate either a hardware breakpoint
or hardware watchpoint. Check which is which if we got
- TARGET_STOPPED_BY_HW_BREAKPOINT. */
+ TARGET_STOPPED_BY_HW_BREAKPOINT. Likewise, we may have single
+ stepped an instruction that triggered a watchpoint. In that
+ case, on some architectures (such as x86), instead of
+ TRAP_HWBKPT, si_code indicates TRAP_TRACE, and we need to check
+ the debug registers separately. */
if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SIGTRAP
- && (child->stop_reason == TARGET_STOPPED_BY_NO_REASON
- || child->stop_reason == TARGET_STOPPED_BY_HW_BREAKPOINT))
+ && child->stop_reason != TARGET_STOPPED_BY_SW_BREAKPOINT)
check_stopped_by_watchpoint (child);
if (!have_stop_pc)
@@ -2241,6 +2295,7 @@ resume_stopped_resumed_lwps (struct inferior_list_entry *entry)
struct lwp_info *lp = get_thread_lwp (thread);
if (lp->stopped
+ && !lp->suspended
&& !lp->status_pending_p
&& thread->last_resume_kind != resume_stop
&& thread->last_status.kind == TARGET_WAITKIND_IGNORE)
@@ -2611,9 +2666,7 @@ unsuspend_one_lwp (struct inferior_list_entry *entry, void *except)
if (lwp == except)
return 0;
- lwp->suspended--;
-
- gdb_assert (lwp->suspended >= 0);
+ lwp_suspended_decr (lwp);
return 0;
}
@@ -2706,7 +2759,7 @@ linux_stabilize_threads (void)
lwp = get_thread_lwp (current_thread);
/* Lock it. */
- lwp->suspended++;
+ lwp_suspended_inc (lwp);
if (ourstatus.value.sig != GDB_SIGNAL_0
|| current_thread->last_resume_kind == resume_stop)
@@ -3092,8 +3145,25 @@ linux_wait_1 (ptid_t ptid,
info_p = &info;
else
info_p = NULL;
- linux_resume_one_lwp (event_child, event_child->stepping,
- WSTOPSIG (w), info_p);
+
+ if (step_over_finished)
+ {
+ /* We cancelled this thread's step-over above. We still
+ need to unsuspend all other LWPs, and set them back
+ running again while the signal handler runs. */
+ unsuspend_all_lwps (event_child);
+
+ /* Enqueue the pending signal info so that proceed_all_lwps
+ doesn't lose it. */
+ enqueue_pending_signal (event_child, WSTOPSIG (w), info_p);
+
+ proceed_all_lwps ();
+ }
+ else
+ {
+ linux_resume_one_lwp (event_child, event_child->stepping,
+ WSTOPSIG (w), info_p);
+ }
return ignore_event (ourstatus);
}
@@ -3109,13 +3179,21 @@ linux_wait_1 (ptid_t ptid,
do, we're be able to handle GDB breakpoints on top of internal
breakpoints, by handling the internal breakpoint and still
reporting the event to GDB. If we don't, we're out of luck, GDB
- won't see the breakpoint hit. */
+ won't see the breakpoint hit. If we see a single-step event but
+ the thread should be continuing, don't pass the trap to gdb.
+ That indicates that we had previously finished a single-step but
+ left the single-step pending -- see
+ complete_ongoing_step_over. */
report_to_gdb = (!maybe_internal_trap
|| (current_thread->last_resume_kind == resume_step
&& !in_step_range)
|| event_child->stop_reason == TARGET_STOPPED_BY_WATCHPOINT
- || (!step_over_finished && !in_step_range
- && !bp_explains_trap && !trace_event)
+ || (!in_step_range
+ && !bp_explains_trap
+ && !trace_event
+ && !step_over_finished
+ && !(current_thread->last_resume_kind == resume_continue
+ && event_child->stop_reason == TARGET_STOPPED_BY_SINGLE_STEP))
|| (gdb_breakpoint_here (event_child->stop_pc)
&& gdb_condition_true_at_breakpoint (event_child->stop_pc)
&& gdb_no_commands_at_breakpoint (event_child->stop_pc))
@@ -3463,7 +3541,7 @@ suspend_and_send_sigstop_callback (struct inferior_list_entry *entry,
if (lwp == except)
return 0;
- lwp->suspended++;
+ lwp_suspended_inc (lwp);
return send_sigstop_callback (entry, except);
}
@@ -3565,7 +3643,12 @@ stuck_in_jump_pad_callback (struct inferior_list_entry *entry, void *data)
struct thread_info *thread = (struct thread_info *) entry;
struct lwp_info *lwp = get_thread_lwp (thread);
- gdb_assert (lwp->suspended == 0);
+ if (lwp->suspended != 0)
+ {
+ internal_error (__FILE__, __LINE__,
+ "LWP %ld is suspended, suspended=%d\n",
+ lwpid_of (thread), lwp->suspended);
+ }
gdb_assert (lwp->stopped);
/* Allow debugging the jump pad, gdb_collect, etc.. */
@@ -3584,7 +3667,12 @@ move_out_of_jump_pad_callback (struct inferior_list_entry *entry)
struct lwp_info *lwp = get_thread_lwp (thread);
int *wstat;
- gdb_assert (lwp->suspended == 0);
+ if (lwp->suspended != 0)
+ {
+ internal_error (__FILE__, __LINE__,
+ "LWP %ld is suspended, suspended=%d\n",
+ lwpid_of (thread), lwp->suspended);
+ }
gdb_assert (lwp->stopped);
wstat = lwp->status_pending_p ? &lwp->status_pending : NULL;
@@ -3613,7 +3701,7 @@ move_out_of_jump_pad_callback (struct inferior_list_entry *entry)
linux_resume_one_lwp (lwp, 0, 0, NULL);
}
else
- lwp->suspended++;
+ lwp_suspended_inc (lwp);
}
static int
@@ -3668,6 +3756,24 @@ stop_all_lwps (int suspend, struct lwp_info *except)
}
}
+/* Enqueue one signal in the chain of signals which need to be
+ delivered to this process on next resume. */
+
+static void
+enqueue_pending_signal (struct lwp_info *lwp, int signal, siginfo_t *info)
+{
+ struct pending_signals *p_sig;
+
+ p_sig = xmalloc (sizeof (*p_sig));
+ p_sig->prev = lwp->pending_signals;
+ p_sig->signal = signal;
+ if (info == NULL)
+ memset (&p_sig->info, 0, sizeof (siginfo_t));
+ else
+ memcpy (&p_sig->info, info, sizeof (siginfo_t));
+ lwp->pending_signals = p_sig;
+}
+
/* Resume execution of LWP. If STEP is nonzero, single-step it. If
SIGNAL is nonzero, give it that signal. */
@@ -4201,7 +4307,13 @@ start_step_over (struct lwp_info *lwp)
lwpid_of (thread));
stop_all_lwps (1, lwp);
- gdb_assert (lwp->suspended == 0);
+
+ if (lwp->suspended != 0)
+ {
+ internal_error (__FILE__, __LINE__,
+ "LWP %ld suspended=%d\n", lwpid_of (thread),
+ lwp->suspended);
+ }
if (debug_threads)
debug_printf ("Done stopping all threads for step-over.\n");
@@ -4273,6 +4385,39 @@ finish_step_over (struct lwp_info *lwp)
return 0;
}
+/* If there's a step over in progress, wait until all threads stop
+ (that is, until the stepping thread finishes its step), and
+ unsuspend all lwps. The stepping thread ends with its status
+ pending, which is processed later when we get back to processing
+ events. */
+
+static void
+complete_ongoing_step_over (void)
+{
+ if (!ptid_equal (step_over_bkpt, null_ptid))
+ {
+ struct lwp_info *lwp;
+ int wstat;
+ int ret;
+
+ if (debug_threads)
+ debug_printf ("detach: step over in progress, finish it first\n");
+
+ /* 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);
+
+ lwp = find_lwp_pid (step_over_bkpt);
+ if (lwp != NULL)
+ finish_step_over (lwp);
+ step_over_bkpt = null_ptid;
+ unsuspend_all_lwps (lwp);
+ }
+}
+
/* This function is called once per thread. We check the thread's resume
request, which will tell us whether to resume, step, or leave the thread
stopped; and what signal, if any, it should be sent.
@@ -4347,13 +4492,16 @@ linux_resume_one_thread (struct inferior_list_entry *entry, void *arg)
}
/* If this thread which is about to be resumed has a pending status,
- then don't resume any threads - we can just report the pending
- status. Make sure to queue any signals that would otherwise be
- sent. In all-stop mode, we do this decision based on if *any*
- thread has a pending status. If there's a thread that needs the
- step-over-breakpoint dance, then don't resume any other thread
- but that particular one. */
- leave_pending = (lwp->status_pending_p || leave_all_stopped);
+ then don't resume it - we can just report the pending status.
+ Likewise if it is suspended, because e.g., another thread is
+ stepping past a breakpoint. Make sure to queue any signals that
+ would otherwise be sent. In all-stop mode, we do this decision
+ based on if *any* thread has a pending status. If there's a
+ thread that needs the step-over-breakpoint dance, then don't
+ resume any other thread but that particular one. */
+ leave_pending = (lwp->suspended
+ || lwp->status_pending_p
+ || leave_all_stopped);
if (!leave_pending)
{
@@ -4536,7 +4684,23 @@ proceed_one_lwp (struct inferior_list_entry *entry, void *except)
send_sigstop (lwp);
}
- step = thread->last_resume_kind == resume_step;
+ if (thread->last_resume_kind == resume_step)
+ {
+ if (debug_threads)
+ debug_printf (" stepping LWP %ld, client wants it stepping\n",
+ lwpid_of (thread));
+ step = 1;
+ }
+ else if (lwp->bp_reinsert != 0)
+ {
+ if (debug_threads)
+ debug_printf (" stepping LWP %ld, reinsert set\n",
+ lwpid_of (thread));
+ step = 1;
+ }
+ else
+ step = 0;
+
linux_resume_one_lwp (lwp, step, 0, NULL);
return 0;
}
@@ -4550,8 +4714,7 @@ unsuspend_and_proceed_one_lwp (struct inferior_list_entry *entry, void *except)
if (lwp == except)
return 0;
- lwp->suspended--;
- gdb_assert (lwp->suspended >= 0);
+ lwp_suspended_decr (lwp);
return proceed_one_lwp (entry, except);
}
diff --git a/gdb/target/waitstatus.h b/gdb/target/waitstatus.h
index d4ef3b8..ffaddc1 100644
--- a/gdb/target/waitstatus.h
+++ b/gdb/target/waitstatus.h
@@ -131,7 +131,10 @@ enum target_stop_reason
TARGET_STOPPED_BY_HW_BREAKPOINT,
/* Stopped by a watchpoint. */
- TARGET_STOPPED_BY_WATCHPOINT
+ TARGET_STOPPED_BY_WATCHPOINT,
+
+ /* Stopped by a single step finishing. */
+ TARGET_STOPPED_BY_SINGLE_STEP
};
/* Prototypes */
diff --git a/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.c b/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.c
new file mode 100644
index 0000000..a6ff0fd
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.c
@@ -0,0 +1,139 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2015 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include <assert.h>
+#include <pthread.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <stdlib.h>
+
+/* Number of threads. Each thread continuously spawns a fork and wait
+ for it. If we have another thread continuously start a step over,
+ gdbserver should end up finding new forks while suspending
+ threads. */
+#define NTHREADS 10
+
+pthread_t threads[NTHREADS];
+
+pthread_barrier_t barrier;
+
+#define NFORKS 10
+
+/* Used to create a conditional breakpoint that always fails. */
+volatile int zero;
+
+static void *
+thread_forks (void *arg)
+{
+ int i;
+
+ pthread_barrier_wait (&barrier);
+
+ for (i = 0; i < NFORKS; i++)
+ {
+ pid_t pid;
+
+ pid = fork ();
+
+ if (pid > 0)
+ {
+ int status;
+
+ /* Parent. */
+ pid = waitpid (pid, &status, 0);
+ if (pid == -1)
+ {
+ perror ("wait");
+ exit (1);
+ }
+
+ if (!WIFEXITED (status))
+ {
+ printf ("Unexpected wait status 0x%x from child %d\n",
+ status, pid);
+ }
+ }
+ else if (pid == 0)
+ {
+ /* Child. */
+ exit (0);
+ }
+ else
+ {
+ perror ("fork");
+ exit (1);
+ }
+ }
+}
+
+/* Set this to tell the thread_breakpoint thread to exit. */
+volatile int break_out;
+
+static void *
+thread_breakpoint (void *arg)
+{
+ pthread_barrier_wait (&barrier);
+
+ while (!break_out)
+ {
+ usleep (1); /* set break here */
+ }
+
+ return NULL;
+}
+
+pthread_barrier_t barrier;
+
+int
+main (void)
+{
+ int i;
+ int ret;
+ pthread_t bp_thread;
+
+ /* Don't run forever. */
+ alarm (180);
+
+ pthread_barrier_init (&barrier, NULL, NTHREADS + 1);
+
+ /* Start the threads that constantly fork. */
+ for (i = 0; i < NTHREADS; i++)
+ {
+ ret = pthread_create (&threads[i], NULL, thread_forks, NULL);
+ assert (ret == 0);
+ }
+
+ /* Start the thread that constantly hit a conditional breakpoint
+ that needs to be stepped over. */
+ ret = pthread_create (&bp_thread, NULL, thread_breakpoint, NULL);
+ assert (ret == 0);
+
+ /* Wait for forking to stop. */
+ for (i = 0; i < NTHREADS; i++)
+ {
+ ret = pthread_join (threads[i], NULL);
+ assert (ret == 0);
+ }
+
+ break_out = 1;
+ pthread_join (bp_thread, NULL);
+ assert (ret == 0);
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp b/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp
new file mode 100644
index 0000000..448b734
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp
@@ -0,0 +1,117 @@
+# Copyright (C) 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# This test verifies that several threads forking while another thread
+# is constantly stepping over a breakpoint is properly handled.
+
+standard_testfile
+
+set linenum [gdb_get_line_number "set break here"]
+
+if {[build_executable "failed to prepare" $testfile $srcfile {debug pthreads}] == -1} {
+ return -1
+}
+
+# The test proper. If COND_BP_TARGET is true, then test with
+# conditional breakpoints evaluated on the target side, if possible.
+# DETACH_ON_FORK is used as value for the "set detach-on-fork"
+# setting. If "on", this exercises GDB explicitly continuing the fork
+# child until exit. If "off", this exercises GDB detaching the fork
+# child.
+proc do_test { cond_bp_target detach_on_fork } {
+ global GDBFLAGS
+ global srcfile testfile binfile
+ global decimal gdb_prompt
+ global linenum
+ global is_remote_target
+
+ set saved_gdbflags $GDBFLAGS
+ set GDBFLAGS [concat $GDBFLAGS " -ex \"set non-stop on\""]
+ clean_restart $binfile
+ set GDBFLAGS $saved_gdbflags
+
+ if ![runto_main] then {
+ fail "Can't run to main"
+ return 0
+ }
+
+ gdb_test_no_output "set detach-on-fork $detach_on_fork"
+
+ gdb_test "break $linenum if zero == 1" \
+ "Breakpoint .*" \
+ "set breakpoint that evals false"
+
+ set test "continue &"
+ gdb_test_multiple $test $test {
+ -re "$gdb_prompt " {
+ pass $test
+ }
+ }
+
+ set fork_count 0
+ set ok 0
+
+ set test "inferior 1 exited"
+ gdb_test_multiple "" $test {
+ -re "Inferior 1 \(\[^\r\n\]+\) exited normally" {
+ set ok 1
+ pass $test
+ }
+ -re "Inferior $decimal \(\[^\r\n\]+\) exited normally" {
+ incr fork_count
+ if {$fork_count <= 100} {
+ exp_continue
+ } else {
+ fail "$test (too many forks)"
+ }
+ }
+ }
+
+ if {!$ok} {
+ # No use testing further.
+ return
+ }
+
+ gdb_test "info threads" "No threads\." \
+ "no threads left"
+
+ gdb_test "info inferiors" \
+ "Num\[ \t\]+Description\[ \t\]+Executable\[ \t\]+\r\n\\* 1 \[^\r\n\]+" \
+ "only inferior 1 left"
+}
+
+# Wrapper for foreach that calls with_test_prefix on each iteration,
+# including the iterator's current value in the prefix.
+
+proc foreach_with_prefix {var list body} {
+ upvar 1 $var myvar
+ foreach myvar $list {
+ with_test_prefix "$var=$myvar" {
+ uplevel 1 $body
+ }
+ }
+}
+
+foreach_with_prefix cond_bp_target {1 0} {
+ foreach_with_prefix detach_on_fork {"on" "off"} {
+ do_test $cond_bp_target $detach_on_fork
+
+ # Disable "off" for now. The test does pass with
+ # detach-on-fork off (at the time of writing), but gdb seems
+ # to slow down quadratically as inferiors are created, and
+ # then the test takes annoyingly long to complete...
+ break
+ }
+}
--
1.9.3
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH/7.10 2/2] gdbserver: Fix non-stop / fork / step-over issues
2015-08-05 22:19 ` Don Breazeal
@ 2015-08-06 10:09 ` Pedro Alves
0 siblings, 0 replies; 16+ messages in thread
From: Pedro Alves @ 2015-08-06 10:09 UTC (permalink / raw)
To: Don Breazeal, gdb-patches
On 08/05/2015 11:19 PM, Don Breazeal wrote:
> Hi Pedro,
> I spent some time looking at this, and I found at least one of the
> culprits affecting performance. Without going through the details of
> how I arrived at this conclusion, if I insert
>
> gdb_test_no_output "set sysroot /"
>
Yeah, I had suspected that too, but discarded it as while it does
make the test go faster, it doesn't seem to explain the
seemingly progressively increasing slowdown. See below.
> just before the call to runto_main, it cuts the wall clock time by at
> least half. Running with just the 'detach-on-fork=off' case, it went
> from 41 secs to 20 secs on one system, and 1:21 to 0:27 and 1:50 to 0:41
> on another. Successive runs without set sysroot resulted in
> successively decreasing run times, presumably due to filesystem caching.
>
> I ran strace -cw to collect wall clock time (strace 4.9 and above
> support '-w' for wall time), and saw this:
>
> Without set sysroot /:
> % time seconds usecs/call calls errors syscall^M
> ------ ----------- ----------- --------- --------- ----------------^M
> 25.90 14.620339 4 3666141 202 ptrace^M
> 25.21 14.229421 81 175135 57 select^M
> 14.42 8.139715 13 641874 7 write^M
> 10.65 6.012699 4 1397576 670469 read^M
> 7.52 4.245209 4 1205014 104 wait4^M
> 4.90 2.765111 3 847985 rt_sigprocmask^M
>
> With set sysroot /:
> % time seconds usecs/call calls errors syscall^M
> ------ ----------- ----------- --------- --------- ----------------^M
> 32.91 6.885008 148 46665 43 select^M
> 21.59 4.516311 4 1158530 202 ptrace^M
> 11.15 2.332491 13 184229 2 write^M
> 9.07 1.897401 4 422122 203552 read^M
> 6.77 1.415918 42 34076 53 open^M
> 6.27 1.312490 3 378702 103 wait4^M
> 4.00 0.835731 3 262195 rt_sigprocmask^M
>
> The # calls and times for each case varied from run to run, but the
> relative proportions stayed reasonably similar. I'm not sure why the
> unmodified case has so many more calls to ptrace, but it was not an
> anomaly, I saw this in multiple runs.
>
That's very interesting. Something
around "strace gdb | grep ptrace | sort" might give an idea of what ptrace
calls those are. Maybe there's low hanging fruit waiting to be picked,
regarding speeding up file transfers caused by
the "target:" sysroot default.
> Note that I used the original version of the test that you posted, not
> the update on your branch. Also, I didn't make the set sysroot command
> conditional on running with a remote or gdbserver target, since it was
> just an experiment.
>
> Do you think there is more to the slowdown than this? As you said
> above, detach-on-fork 'off' is going to take longer than 'on'. It may
> be a little while before I can get back to this, so I thought I'd share
> what I found. Let me know if you think this change will be sufficient.
>
I do. The test is now in master (note it was renamed.)
Try the patch below, and while
$ make check RUNTESTFLAGS="--target_board=native-extended-gdbserver forking-threads-plus-breakpoint.exp"
is running on one terminal, in another do:
$ tail -f testsuite/gdb.log
On my box, the first 50 or inferiors are created at roughly 3 inferiors per second,
but by the time you get to inferior 100 or so, GDB is taking over one second or
so to create each inferior. Be inferior 200 it's taking around 3 seconds or so.
gdb is getting slower and slower.
From d55465edd4ad75e963b1d2df201f2fd2950542bf Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 6 Aug 2015 09:55:42 +0100
Subject: [PATCH] test slowdown
---
gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.c | 4 ++--
gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp | 4 +++-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.c b/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.c
index a6ff0fd..14b4240 100644
--- a/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.c
+++ b/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.c
@@ -33,7 +33,7 @@ pthread_t threads[NTHREADS];
pthread_barrier_t barrier;
-#define NFORKS 10
+#define NFORKS 1000
/* Used to create a conditional breakpoint that always fails. */
volatile int zero;
@@ -108,7 +108,7 @@ main (void)
pthread_t bp_thread;
/* Don't run forever. */
- alarm (180);
+ // alarm (180);
pthread_barrier_init (&barrier, NULL, NTHREADS + 1);
diff --git a/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp b/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp
index 448b734..b50e6ec 100644
--- a/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp
+++ b/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp
@@ -39,6 +39,7 @@ proc do_test { cond_bp_target detach_on_fork } {
set saved_gdbflags $GDBFLAGS
set GDBFLAGS [concat $GDBFLAGS " -ex \"set non-stop on\""]
+ set GDBFLAGS [concat $GDBFLAGS " -ex \"set sysroot /\""]
clean_restart $binfile
set GDBFLAGS $saved_gdbflags
@@ -71,6 +72,7 @@ proc do_test { cond_bp_target detach_on_fork } {
}
-re "Inferior $decimal \(\[^\r\n\]+\) exited normally" {
incr fork_count
+ exp_continue
if {$fork_count <= 100} {
exp_continue
} else {
@@ -105,7 +107,7 @@ proc foreach_with_prefix {var list body} {
}
foreach_with_prefix cond_bp_target {1 0} {
- foreach_with_prefix detach_on_fork {"on" "off"} {
+ foreach_with_prefix detach_on_fork {"off" "off"} {
do_test $cond_bp_target $detach_on_fork
# Disable "off" for now. The test does pass with
--
1.9.3
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2015-08-06 10:09 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-31 17:03 [PATCH/7.10 0/2] gdbserver: Fix several fork support (& co) issues Pedro Alves
2015-07-31 17:04 ` [PATCH/7.10 1/2] Linux gdbserver confused when event randomization returns a process exit event Pedro Alves
2015-08-03 10:59 ` Yao Qi
2015-08-06 9:34 ` [pushed][PATCH 1/3] Linux gdbserver fork event debug output (Re: [PATCH/7.10 1/2] Linux gdbserver confused when event randomization returns a process exit event) Pedro Alves
2015-08-06 9:43 ` [pushed]Re: [PATCH/7.10 1/2] Linux gdbserver confused when event randomization returns a process exit event Pedro Alves
2015-07-31 17:04 ` [PATCH/7.10 2/2] gdbserver: Fix non-stop / fork / step-over issues Pedro Alves
2015-07-31 18:04 ` Don Breazeal
2015-07-31 19:02 ` Pedro Alves
2015-08-05 22:19 ` Don Breazeal
2015-08-06 10:09 ` Pedro Alves
2015-08-03 15:14 ` Yao Qi
2015-08-03 16:20 ` Pedro Alves
2015-08-04 16:40 ` Pedro Alves
2015-08-05 11:41 ` Yao Qi
2015-08-05 15:10 ` Pedro Alves
2015-08-06 9:44 ` [pushed] " Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).