[PATCH v2 15/23] Implement all-stop on top of a target running non-stop mode
Pedro Alves
palves@redhat.com
Wed Apr 8 19:35:00 GMT 2015
On 04/08/2015 12:08 PM, Pedro Alves wrote:
>
> Hmm. Looks like the assertion caught a pre-existing problem.
> This sets up the thread to re-hit the breakpoint at PC once the
> signal handler returns, and lets _all_ threads run. But, what if had
> _other_ threads that needed a step-over too? Those will run too,
> and immediately re-trap the same breakpoint, but GDB will re-report them.
I thought it'd be easy to test that with multiple-step-overs.exp test, and
indeed, bad things happened. I filed bugs and pushed the test in upfront,
as managing all these patch dependencies is getting complicated...
[pushed] Add test for PR18214 and PR18216 - multiple step-overs with queued signals
https://sourceware.org/ml/gdb-patches/2015-04/msg00293.html
> Maybe we should set a step-resume breakpoint on _all_ threads that need
> a step-over, not just the current. I'll need to think a bit about this.
That works. I tested it with native | remote, all-stop | non-stop,
software | hardware single-step, and all pass now.
Both PR18214 and PR18216 end up fixed with this series (and this
patch). I don't exactly (yet) which patch fixed PR18216.
I'll try moving the start_step_over_inferior hunk to the proper patch.
That bit alone should fix the signal-sigtrap.exp assertion you tripped
on. The rest I'm planning on leaving as a separate patch.
>From b0a3b75b242714cbdad0c93ddcabf00af0663d12 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 8 Apr 2015 16:59:56 +0100
Subject: [PATCH] PR18214 - multiple step overs + deliver signal, on sss
targets
The 'resume' code that detects that we're stepping over a breakpoint
and delivering a signal at the same time:
/* Currently, our software single-step implementation leads to different
results than hardware single-stepping in one situation: when stepping
into delivering a signal which has an associated signal handler,
hardware single-step will stop at the first instruction of the handler,
while software single-step will simply skip execution of the handler.
...
Fortunately, we can at least fix this particular issue. We detect
here the case where we are about to deliver a signal while software
single-stepping with breakpoints removed. In this situation, we
revert the decisions to remove all breakpoints and insert single-
step breakpoints, and instead we install a step-resume breakpoint
at the current address, deliver the signal without stepping, and
once we arrive back at the step-resume breakpoint, actually step
over the breakpoint we originally wanted to step over. */
doesn't handle the case of another thread also needing to step over a
breakpoint. It just resumes all threads. And then that other thread
immediately re-traps the same breakpoint it had stopped for.
Fix it by setting a step-resume breakpoint on _all_ threads that need
a step-over, not just the current.
gdb/ChangeLog:
2015-04-08 Pedro Alves <palves@redhat.com>
* infrun.c (start_step_over_inferior): Assert the thread is
executing. If in all-stop, also return early if the thread now
has a step resume set (to skip past a signal handler).
(setup_step_after_step_resume_breakpoint): New function.
(resume): When delivering a signal with software single-step, set
a step-resume breakpoint in all threads that need a step-over, not
just the current thread.
---
gdb/infrun.c | 69 ++++++++++++++++++++++++++++++++++++++++++++----------------
1 file changed, 51 insertions(+), 18 deletions(-)
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 6511c8b..9c85628 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2001,7 +2001,11 @@ start_step_over_inferior (struct inferior *inf)
if (!ecs->wait_some_more)
error (_("Command aborted."));
- if (tp->control.trap_expected)
+ gdb_assert (tp->executing);
+
+ if (tp->control.trap_expected
+ || (tp->step_after_step_resume_breakpoint
+ && !target_is_non_stop_p ()))
{
if (inf->step_over_queue_head == NULL)
{
@@ -2206,6 +2210,26 @@ do_target_resume (ptid_t resume_ptid, int step, enum gdb_signal sig)
target_resume (resume_ptid, step, sig);
}
+/* In order to skip a signal handler, set a high-priority step-resume
+ breakpoint the TP's current frame, and setup to step over it once
+ it is reached. */
+
+static void
+setup_step_after_step_resume_breakpoint (struct thread_info *tp)
+{
+ /* If we have nested signals or a pending signal is delivered
+ immediately after a handler returns, we might already have a
+ step-resume breakpoint set on the earlier handler. We cannot set
+ another step-resume breakpoint; just continue on until the
+ original breakpoint is hit. */
+ if (tp->control.step_resume_breakpoint == NULL)
+ {
+ switch_to_thread (tp->ptid);
+ insert_hp_step_resume_breakpoint_at_frame (get_current_frame ());
+ tp->step_after_step_resume_breakpoint = 1;
+ }
+}
+
/* Resume the inferior, but allow a QUIT. This is useful if the user
wants to interrupt some lengthy single-stepping operation
(for child processes, the SIGINT goes to the inferior, and so
@@ -2470,6 +2494,11 @@ resume (enum gdb_signal sig)
if (use_software_single_step && execution_direction == EXEC_FORWARD)
gdb_assert (!step);
+ /* Decide the set of threads to ask the target to resume. Start
+ by assuming everything will be resumed, than narrow the set
+ by applying increasingly restricting conditions. */
+ resume_ptid = user_visible_resume_ptid (user_step);
+
/* Currently, our software single-step implementation leads to different
results than hardware single-stepping in one situation: when stepping
into delivering a signal which has an associated signal handler,
@@ -2498,21 +2527,30 @@ resume (enum gdb_signal sig)
&& sig != GDB_SIGNAL_0
&& step_over_info_valid_p ())
{
- /* If we have nested signals or a pending signal is delivered
- immediately after a handler returns, might might already have
- a step-resume breakpoint set on the earlier handler. We cannot
- set another step-resume breakpoint; just continue on until the
- original breakpoint is hit. */
- if (tp->control.step_resume_breakpoint == NULL)
- {
- insert_hp_step_resume_breakpoint_at_frame (get_current_frame ());
- tp->step_after_step_resume_breakpoint = 1;
- }
+ clear_step_over_info ();
+ tp->control.trap_expected = 0;
delete_single_step_breakpoints (tp);
- clear_step_over_info ();
- tp->control.trap_expected = 0;
+ setup_step_after_step_resume_breakpoint (tp);
+
+ if (!target_is_non_stop_p ())
+ {
+ struct thread_info *current = tp;
+
+ ALL_NON_EXITED_THREADS (tp)
+ if (ptid_match (tp->ptid, resume_ptid))
+ {
+ if (tp->step_over_next != NULL)
+ {
+ thread_step_over_chain_remove (tp);
+ setup_step_after_step_resume_breakpoint (tp);
+ }
+ }
+
+ tp = current;
+ switch_to_thread (tp->ptid);
+ }
insert_breakpoints ();
}
@@ -2523,11 +2561,6 @@ resume (enum gdb_signal sig)
if (use_software_single_step)
gdb_assert (!(thread_has_single_step_breakpoints_set (tp) && step));
- /* Decide the set of threads to ask the target to resume. Start
- by assuming everything will be resumed, than narrow the set
- by applying increasingly restricting conditions. */
- resume_ptid = user_visible_resume_ptid (user_step);
-
/* Maybe resume a single thread after all. */
if (target_is_non_stop_p ())
{
--
1.9.3
More information about the Gdb-patches
mailing list