[v3,06/17] Use keep_going in proceed and start_step_over too
Commit Message
The main motivation of this patch is sharing more code between the
proceed (starting the inferior for the first time) and keep_going
(restarting the inferior after handling an event) paths and using the
step_over_chain queue now embedded in the thread_info object for
pending in-line step-overs too (instead of just for displaced
stepping).
So this commit:
- splits out a new keep_going_pass function out of keep_going that is
just like keep_going except for the bits that clear the signal to
pass if the signal is set to "handle nopass".
- makes proceed use keep_going too.
- Makes start_step_over use keep_going_pass instead of lower level
displaced stepping things.
One user visible change: if inserting breakpoints while trying to
proceed fails, we now get:
(gdb) si
Warning:
Could not insert hardware watchpoint 7.
Could not insert hardware breakpoints:
You may have requested too many hardware breakpoints/watchpoints.
Command aborted.
(gdb)
while before we only saw warnings with no indication that the command
was cancelled:
(gdb) si
Warning:
Could not insert hardware watchpoint 7.
Could not insert hardware breakpoints:
You may have requested too many hardware breakpoints/watchpoints.
(gdb)
Tested on x86_64-linux-gnu, ppc64-linux-gnu and s390-linux-gnu.
gdb/ChangeLog:
2015-04-17 Pedro Alves <palves@redhat.com>
* gdbthread.h (struct thread_info) <prev_pc>: Extend comment.
* infrun.c (struct execution_control_state): Move higher up in the
file.
(reset_ecs): New function.
(start_step_over): Now returns int. Rewrite to use
keep_going_pass instead of manually starting a displaced step.
(resume): Don't call set_running here. If displaced stepping
can't start now, clear trap_expected.
(find_thread_needs_step_over): Delete function.
(proceed): Set up finish_thread_state_cleanup. Call set_running.
If the current thread needs a step over, push it in the step-over
chain. Don't set insert breakpoints nor call resume directly
here. Instead rewrite to use start_step_over and keep_going_pass.
(finish_step_over): New function.
(handle_signal_stop): Call finish_step_over instead of
start_step_over.
(switch_back_to_stepped_thread): If the event thread needs another
step-over do that first. Use start_step_over.
(keep_going_pass): New function, factored out from ...
(keep_going): ... here.
(_initialize_infrun): Comment moved here.
* thread.c (set_running_thread): New function.
(set_running, finish_thread_state): Use set_running_thread.
v3:
- Adjusted to step_over_chain changes. Fixed gdb internal error
caught by signal-sigtrap.exp on ARM (software single-step target).
v2:
- Testing on x86_64 with software single-step revealed v1 missed
setting prev_pc on proceed, for switch_back_to_stepped_thread.
Rewrite tail end of proceed to fix that. This ends up simplifying
later patches in the series.
---
gdb/gdbthread.h | 6 +-
gdb/infrun.c | 579 ++++++++++++++++++++++++++++----------------------------
gdb/thread.c | 52 +++--
3 files changed, 324 insertions(+), 313 deletions(-)
Comments
Pedro Alves writes:
> The main motivation of this patch is sharing more code between the
> proceed (starting the inferior for the first time) and keep_going
> (restarting the inferior after handling an event) paths and using the
> step_over_chain queue now embedded in the thread_info object for
> pending in-line step-overs too (instead of just for displaced
> stepping).
Hi.
A couple of nits inline.
grep for ====
>
> So this commit:
>
> - splits out a new keep_going_pass function out of keep_going that is
> just like keep_going except for the bits that clear the signal to
> pass if the signal is set to "handle nopass".
>
> - makes proceed use keep_going too.
>
> - Makes start_step_over use keep_going_pass instead of lower level
> displaced stepping things.
>
> One user visible change: if inserting breakpoints while trying to
> proceed fails, we now get:
>
> (gdb) si
> Warning:
> Could not insert hardware watchpoint 7.
> Could not insert hardware breakpoints:
> You may have requested too many hardware breakpoints/watchpoints.
>
> Command aborted.
> (gdb)
>
> while before we only saw warnings with no indication that the command
> was cancelled:
>
> (gdb) si
> Warning:
> Could not insert hardware watchpoint 7.
> Could not insert hardware breakpoints:
> You may have requested too many hardware breakpoints/watchpoints.
>
> (gdb)
>
> Tested on x86_64-linux-gnu, ppc64-linux-gnu and s390-linux-gnu.
>
> gdb/ChangeLog:
> 2015-04-17 Pedro Alves <palves@redhat.com>
>
> * gdbthread.h (struct thread_info) <prev_pc>: Extend comment.
> * infrun.c (struct execution_control_state): Move higher up in the
> file.
> (reset_ecs): New function.
> (start_step_over): Now returns int. Rewrite to use
> keep_going_pass instead of manually starting a displaced step.
> (resume): Don't call set_running here. If displaced stepping
> can't start now, clear trap_expected.
> (find_thread_needs_step_over): Delete function.
> (proceed): Set up finish_thread_state_cleanup. Call set_running.
> If the current thread needs a step over, push it in the step-over
> chain. Don't set insert breakpoints nor call resume directly
> here. Instead rewrite to use start_step_over and keep_going_pass.
> (finish_step_over): New function.
> (handle_signal_stop): Call finish_step_over instead of
> start_step_over.
> (switch_back_to_stepped_thread): If the event thread needs another
> step-over do that first. Use start_step_over.
> (keep_going_pass): New function, factored out from ...
> (keep_going): ... here.
> (_initialize_infrun): Comment moved here.
> * thread.c (set_running_thread): New function.
> (set_running, finish_thread_state): Use set_running_thread.
>
> v3:
>
> - Adjusted to step_over_chain changes. Fixed gdb internal error
> caught by signal-sigtrap.exp on ARM (software single-step target).
>
> v2:
>
> - Testing on x86_64 with software single-step revealed v1 missed
> setting prev_pc on proceed, for switch_back_to_stepped_thread.
> Rewrite tail end of proceed to fix that. This ends up simplifying
> later patches in the series.
>...
>
> diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
> index 2c871a2..7052ee1 100644
> --- a/gdb/gdbthread.h
> +++ b/gdb/gdbthread.h
> @@ -208,8 +208,10 @@ struct thread_info
>
> /* Internal stepping state. */
>
> - /* Record the pc of the thread the last time it stopped. This is
> - maintained by proceed and keep_going, and used in
> + /* Record the pc of the thread the last time it was resumed. (It
> + can't be done on stop as the PC may change since the last stop,
> + e.g., "return" command, or "p $pc = 0xf000"). This is maintained
> + by proceed and keep_going, and among other things, it's used in
> adjust_pc_after_break to distinguish a hardware single-step
> SIGTRAP from a breakpoint SIGTRAP. */
> CORE_ADDR prev_pc;
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index f325a53..beb9ea2 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -1844,30 +1844,61 @@ displaced_step_fixup (ptid_t event_ptid, enum gdb_signal signal)
> displaced->step_ptid = null_ptid;
> }
>
> -/* Are there any pending step-over requests? If so, run one now. */
> +/* Data to be passed around while handling an event. This data is
> + discarded between events. */
> +struct execution_control_state
> +{
> + ptid_t ptid;
> + /* The thread that got the event, if this was a thread event; NULL
> + otherwise. */
> + struct thread_info *event_thread;
> +
> + struct target_waitstatus ws;
> + int stop_func_filled_in;
> + CORE_ADDR stop_func_start;
> + CORE_ADDR stop_func_end;
> + const char *stop_func_name;
> + int wait_some_more;
> +
> + /* True if the event thread hit the single-step breakpoint of
> + another thread. Thus the event doesn't cause a stop, the thread
> + needs to be single-stepped past the single-step breakpoint before
> + we can switch back to the original stepping thread. */
> + int hit_singlestep_breakpoint;
> +};
> +
> +/* Clear ECS and set it to point at TP. */
>
> static void
> +reset_ecs (struct execution_control_state *ecs, struct thread_info *tp)
> +{
> + memset (ecs, 0, sizeof (*ecs));
> + ecs->event_thread = tp;
> + ecs->ptid = tp->ptid;
> +}
> +
> +static void keep_going_pass (struct execution_control_state *ecs);
> +static void prepare_to_wait (struct execution_control_state *ecs);
> +static int thread_still_needs_step_over (struct thread_info *tp);
> +
> +/* Are there any pending step-over requests? If so, run one now and
> + return true. Otherwise, return false. */
> +
> +static int
> start_step_over (void)
> {
> struct thread_info *tp, *next;
>
> for (tp = step_over_queue_head; tp != NULL; tp = next)
> {
> - ptid_t ptid;
> - struct displaced_step_inferior_state *displaced;
> - struct regcache *regcache;
> - struct gdbarch *gdbarch;
> - CORE_ADDR actual_pc;
> - struct address_space *aspace;
> - struct inferior *inf = find_inferior_ptid (tp->ptid);
> + struct execution_control_state ecss;
> + struct execution_control_state *ecs = &ecss;
>
> next = thread_step_over_chain_next (tp);
>
> - displaced = get_displaced_stepping_state (inf->pid);
> -
> /* If this inferior already has a displaced step in process,
> don't start a new one. */
> - if (!ptid_equal (displaced->step_ptid, null_ptid))
> + if (displaced_step_in_progress (ptid_get_pid (tp->ptid)))
> continue;
>
> thread_step_over_chain_remove (tp);
> @@ -1879,74 +1910,57 @@ start_step_over (void)
> "infrun: step-over queue now empty\n");
> }
>
> - ptid = tp->ptid;
> - context_switch (ptid);
> -
> - regcache = get_thread_regcache (ptid);
> - actual_pc = regcache_read_pc (regcache);
> - aspace = get_regcache_aspace (regcache);
> - gdbarch = get_regcache_arch (regcache);
> -
> - if (breakpoint_here_p (aspace, actual_pc))
> + if (tp->control.trap_expected || tp->executing)
> {
> - if (debug_displaced)
> - fprintf_unfiltered (gdb_stdlog,
> - "displaced: stepping queued %s now\n",
> - target_pid_to_str (ptid));
> -
> - displaced_step_prepare (ptid);
> -
> - if (debug_displaced)
> - {
> - CORE_ADDR actual_pc = regcache_read_pc (regcache);
> - gdb_byte buf[4];
> -
> - fprintf_unfiltered (gdb_stdlog, "displaced: run %s: ",
> - paddress (gdbarch, actual_pc));
> - read_memory (actual_pc, buf, sizeof (buf));
> - displaced_step_dump_bytes (gdb_stdlog, buf, sizeof (buf));
> - }
> -
> - if (gdbarch_displaced_step_hw_singlestep (gdbarch,
> - displaced->step_closure))
> - target_resume (ptid, 1, GDB_SIGNAL_0);
> - else
> - target_resume (ptid, 0, GDB_SIGNAL_0);
> -
> - /* Done, we're stepping a thread. */
> - break;
> + internal_error (__FILE__, __LINE__,
> + "[%s] has inconsistent state: "
> + "trap_expected=%d, executing=%d\n",
> + target_pid_to_str (tp->ptid),
> + tp->control.trap_expected,
> + tp->executing);
> }
> - else
> - {
> - int step;
> - struct thread_info *tp = inferior_thread ();
> -
> - /* The breakpoint we were sitting under has since been
> - removed. */
> - tp->control.trap_expected = 0;
>
> - /* Go back to what we were trying to do. */
> - step = currently_stepping (tp);
> -
> - if (step)
> - step = maybe_software_singlestep (gdbarch, actual_pc);
> + if (debug_infrun)
> + fprintf_unfiltered (gdb_stdlog,
> + "infrun: resuming [%s] for step-over\n",
> + target_pid_to_str (tp->ptid));
> +
> + /* keep_going_pass skips the step-over if the breakpoint is no
> + longer inserted. In all-stop, we want to keep looking for a
====
missing word? "... keep looking for a thread" ?
> + that needs a step-over instead of resuming TP, because we
> + wouldn't be able to resume anything else until the target
> + stops again. In non-stop, the resume always resumes only TP,
> + so it's OK to let the thread resume freely. */
> + if (!non_stop && !thread_still_needs_step_over (tp))
> + continue;
>
> - if (debug_displaced)
> - fprintf_unfiltered (gdb_stdlog,
> - "displaced: breakpoint is gone: %s, step(%d)\n",
> - target_pid_to_str (tp->ptid), step);
> + switch_to_thread (tp->ptid);
> + reset_ecs (ecs, tp);
> + keep_going_pass (ecs);
>
> - target_resume (ptid, step, GDB_SIGNAL_0);
> - tp->suspend.stop_signal = GDB_SIGNAL_0;
> + if (!ecs->wait_some_more)
> + error (_("Command aborted."));
>
> - /* This request was discarded. See if there's any other
> - thread waiting for its turn. */
> + if (!non_stop)
> + {
> + /* On all-stop, shouldn't have resumed unless we needed a
> + step over. */
> + gdb_assert (tp->control.trap_expected
> + || tp->step_after_step_resume_breakpoint);
> +
> + /* With remote targets (at least), in all-stop, we can't
> + issue any further remote commands until the program stops
> + again. */
> + return 1;
> }
>
> - /* A new displaced stepping sequence started. Maybe we can
> - start a displaced step on a thread of other process.
> - Continue looking. */
> + /* Either the thread no longer needed a step-over, or a new
> + displaced stepping sequence started. Even in the latter
> + case, continue looking. Maybe we can also start another
> + displaced step on a thread of other process. */
> }
> +
> + return 0;
> }
>
> /* Update global variables holding ptids to hold NEW_PTID if they were
> @@ -2283,16 +2297,11 @@ resume (enum gdb_signal sig)
>
> if (!displaced_step_prepare (inferior_ptid))
> {
> - /* Got placed in displaced stepping queue. Will be resumed
> - later when all the currently queued displaced stepping
> - requests finish. The thread is not executing at this
> - point, and the call to set_executing will be made later.
> - But we need to call set_running here, since from the
> - user/frontend's point of view, threads were set running.
> - Unless we're calling an inferior function, as in that
> - case we pretend the inferior doesn't run at all. */
> - if (!tp->control.in_infcall)
> - set_running (user_visible_resume_ptid (user_step), 1);
> + if (debug_infrun)
> + fprintf_unfiltered (gdb_stdlog,
> + "Got placed in displaced stepping queue\n");
====
Nit: Suggest rewording "displaced stepping queue".
IIUC there is no displaced-stepping specific queue anymore.
> +
> + tp->control.trap_expected = 0;
> discard_cleanups (old_cleanups);
> return;
> }
> @@ -2367,14 +2376,6 @@ resume (enum gdb_signal sig)
> by applying increasingly restricting conditions. */
> resume_ptid = user_visible_resume_ptid (user_step);
>
> - /* Even if RESUME_PTID is a wildcard, and we end up resuming less
> - (e.g., we might need to step over a breakpoint), from the
> - user/frontend's point of view, all threads in RESUME_PTID are now
> - running. Unless we're calling an inferior function, as in that
> - case pretend we inferior doesn't run at all. */
> - if (!tp->control.in_infcall)
> - set_running (resume_ptid, 1);
> -
> /* Maybe resume a single thread after all. */
> if ((step || thread_has_single_step_breakpoints_set (tp))
> && tp->control.trap_expected)
> @@ -2589,48 +2590,6 @@ schedlock_applies (struct thread_info *tp)
> && tp->control.stepping_command));
> }
>
> -/* Look a thread other than EXCEPT that has previously reported a
> - breakpoint event, and thus needs a step-over in order to make
> - progress. Returns NULL is none is found. */
> -
> -static struct thread_info *
> -find_thread_needs_step_over (struct thread_info *except)
> -{
> - struct thread_info *tp, *current;
> -
> - /* With non-stop mode on, threads are always handled individually. */
> - gdb_assert (! non_stop);
> -
> - current = inferior_thread ();
> -
> - /* If scheduler locking applies, we can avoid iterating over all
> - threads. */
> - if (schedlock_applies (except))
> - {
> - if (except != current
> - && thread_still_needs_step_over (current))
> - return current;
> -
> - return NULL;
> - }
> -
> - ALL_NON_EXITED_THREADS (tp)
> - {
> - /* Ignore the EXCEPT thread. */
> - if (tp == except)
> - continue;
> - /* Ignore threads of processes we're not resuming. */
> - if (!sched_multi
> - && ptid_get_pid (tp->ptid) != ptid_get_pid (inferior_ptid))
> - continue;
> -
> - if (thread_still_needs_step_over (tp))
> - return tp;
> - }
> -
> - return NULL;
> -}
> -
> /* Basic routine for continuing the program in various fashions.
>
> ADDR is the address to resume at, or -1 for resume where stopped.
> @@ -2651,6 +2610,11 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
> struct thread_info *tp;
> CORE_ADDR pc;
> struct address_space *aspace;
> + ptid_t resume_ptid;
> + struct execution_control_state ecss;
> + struct execution_control_state *ecs = &ecss;
> + struct cleanup *old_chain;
> + int started;
>
> /* If we're stopped at a fork/vfork, follow the branch set by the
> "set follow-fork-mode" command; otherwise, we'll just proceed
> @@ -2713,7 +2677,23 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
> (next/step/etc.), we'll want to print stop event output to the MI
> console channel (the stepped-to line, etc.), as if the user
> entered the execution command on a real GDB console. */
> - inferior_thread ()->control.command_interp = command_interp ();
> + tp->control.command_interp = command_interp ();
> +
> + resume_ptid = user_visible_resume_ptid (tp->control.stepping_command);
> +
> + /* If an exception is thrown from this point on, make sure to
> + propagate GDB's knowledge of the executing state to the
> + frontend/user running state. */
> + old_chain = make_cleanup (finish_thread_state_cleanup, &resume_ptid);
> +
> + /* Even if RESUME_PTID is a wildcard, and we end up resuming fewer
> + threads (e.g., we might need to set threads stepping over
> + breakpoints first), from the user/frontend's point of view, all
> + threads in RESUME_PTID are now running. Unless we're calling an
> + inferior function, as in that case we pretend the inferior
> + doesn't run at all. */
> + if (!tp->control.in_infcall)
> + set_running (resume_ptid, 1);
>
> if (debug_infrun)
> fprintf_unfiltered (gdb_stdlog,
> @@ -2721,91 +2701,92 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
> paddress (gdbarch, addr),
> gdb_signal_to_symbol_string (siggnal));
>
> - if (non_stop)
> - /* In non-stop, each thread is handled individually. The context
> - must already be set to the right thread here. */
> - ;
> - else
> + annotate_starting ();
> +
> + /* Make sure that output from GDB appears before output from the
> + inferior. */
> + gdb_flush (gdb_stdout);
> +
> + /* In a multi-threaded task we may select another thread and
> + then continue or step.
> +
> + But if a thread that we're resuming had stopped at a breakpoint,
> + it will immediately cause another breakpoint stop without any
> + execution (i.e. it will report a breakpoint hit incorrectly). So
> + we must step over it first.
> +
> + Look for threads other than the current (TP) that reported a
> + breakpoint hit and haven't been resumed yet since. */
> +
> + /* If scheduler locking applies, we can avoid iterating over all
> + threads. */
> + if (!non_stop && !schedlock_applies (tp))
> {
> - struct thread_info *step_over;
> + struct thread_info *current = tp;
> +
> + ALL_NON_EXITED_THREADS (tp)
> + {
> + /* Ignore the current thread here. It's handled
> + afterwards. */
> + if (tp == current)
> + continue;
>
> - /* In a multi-threaded task we may select another thread and
> - then continue or step.
> + /* Ignore threads of processes we're not resuming. */
> + if (!ptid_match (tp->ptid, resume_ptid))
> + continue;
>
> - But if the old thread was stopped at a breakpoint, it will
> - immediately cause another breakpoint stop without any
> - execution (i.e. it will report a breakpoint hit incorrectly).
> - So we must step over it first.
> + if (!thread_still_needs_step_over (tp))
> + continue;
> +
> + gdb_assert (!thread_is_in_step_over_chain (tp));
>
> - Look for a thread other than the current (TP) that reported a
> - breakpoint hit and hasn't been resumed yet since. */
> - step_over = find_thread_needs_step_over (tp);
> - if (step_over != NULL)
> - {
> if (debug_infrun)
> fprintf_unfiltered (gdb_stdlog,
> "infrun: need to step-over [%s] first\n",
> - target_pid_to_str (step_over->ptid));
> + target_pid_to_str (tp->ptid));
>
> - /* Store the prev_pc for the stepping thread too, needed by
> - switch_back_to_stepped_thread. */
> - tp->prev_pc = regcache_read_pc (get_current_regcache ());
> - switch_to_thread (step_over->ptid);
> - tp = step_over;
> + thread_step_over_chain_enqueue (tp);
> }
> - }
> -
> - /* If we need to step over a breakpoint, and we're not using
> - displaced stepping to do so, insert all breakpoints (watchpoints,
> - etc.) but the one we're stepping over, step one instruction, and
> - then re-insert the breakpoint when that step is finished. */
> - if (tp->stepping_over_breakpoint && !use_displaced_stepping (gdbarch))
> - {
> - struct regcache *regcache = get_current_regcache ();
>
> - set_step_over_info (get_regcache_aspace (regcache),
> - regcache_read_pc (regcache), 0);
> + tp = current;
> }
> - else
> - clear_step_over_info ();
>
> - insert_breakpoints ();
> + /* Enqueue the current thread last, so that we move all other
> + threads over their breakpoints first. */
> + if (tp->stepping_over_breakpoint)
> + thread_step_over_chain_enqueue (tp);
>
> - tp->control.trap_expected = tp->stepping_over_breakpoint;
> + /* If the thread isn't started, we'll still need to set its prev_pc,
> + so that switch_back_to_stepped_thread knows the thread hasn't
> + advanced. Must do this before resuming any thread, as in
> + all-stop/remote, once we resume we can't send any other packet
> + until the target stops again. */
> + tp->prev_pc = regcache_read_pc (regcache);
>
> - annotate_starting ();
> + started = start_step_over ();
>
> - /* Make sure that output from GDB appears before output from the
> - inferior. */
> - gdb_flush (gdb_stdout);
> + if (step_over_info_valid_p ())
> + {
> + /* Either this thread started a new in-line step over, or some
> + other thread was already doing one. In either case, don't
> + resume anything else until the step-over is finished. */
> + }
> + else if (started && !non_stop)
> + {
> + /* A new displaced stepping sequence was started. In all-stop,
> + we can't talk to the target anymore until it next stops. */
> + }
> + else if (!tp->executing && !thread_is_in_step_over_chain (tp))
> + {
> + /* The thread wasn't started, and isn't queued, run it now. */
> + reset_ecs (ecs, tp);
> + switch_to_thread (tp->ptid);
> + keep_going_pass (ecs);
> + if (!ecs->wait_some_more)
> + error ("Command aborted.");
> + }
>
> - /* Refresh prev_pc value just prior to resuming. This used to be
> - done in stop_waiting, however, setting prev_pc there did not handle
> - scenarios such as inferior function calls or returning from
> - a function via the return command. In those cases, the prev_pc
> - value was not set properly for subsequent commands. The prev_pc value
> - is used to initialize the starting line number in the ecs. With an
> - invalid value, the gdb next command ends up stopping at the position
> - represented by the next line table entry past our start position.
> - On platforms that generate one line table entry per line, this
> - is not a problem. However, on the ia64, the compiler generates
> - extraneous line table entries that do not increase the line number.
> - When we issue the gdb next command on the ia64 after an inferior call
> - or a return command, we often end up a few instructions forward, still
> - within the original line we started.
> -
> - An attempt was made to refresh the prev_pc at the same time the
> - execution_control_state is initialized (for instance, just before
> - waiting for an inferior event). But this approach did not work
> - because of platforms that use ptrace, where the pc register cannot
> - be read unless the inferior is stopped. At that point, we are not
> - guaranteed the inferior is stopped and so the regcache_read_pc() call
> - can fail. Setting the prev_pc value here ensures the value is updated
> - correctly when the inferior is stopped. */
> - tp->prev_pc = regcache_read_pc (get_current_regcache ());
> -
> - /* Resume inferior. */
> - resume (tp->suspend.stop_signal);
> + discard_cleanups (old_chain);
>
> /* Wait for it to stop (if not standalone)
> and in any case decode why it stopped, and act accordingly. */
> @@ -2873,28 +2854,6 @@ init_wait_for_inferior (void)
> }
>
>
> -/* Data to be passed around while handling an event. This data is
> - discarded between events. */
> -struct execution_control_state
> -{
> - ptid_t ptid;
> - /* The thread that got the event, if this was a thread event; NULL
> - otherwise. */
> - struct thread_info *event_thread;
> -
> - struct target_waitstatus ws;
> - int stop_func_filled_in;
> - CORE_ADDR stop_func_start;
> - CORE_ADDR stop_func_end;
> - const char *stop_func_name;
> - int wait_some_more;
> -
> - /* True if the event thread hit the single-step breakpoint of
> - another thread. Thus the event doesn't cause a stop, the thread
> - needs to be single-stepped past the single-step breakpoint before
> - we can switch back to the original stepping thread. */
> - int hit_singlestep_breakpoint;
> -};
>
> static void handle_inferior_event (struct execution_control_state *ecs);
>
> @@ -2908,7 +2867,6 @@ static void check_exception_resume (struct execution_control_state *,
>
> static void end_stepping_range (struct execution_control_state *ecs);
> static void stop_waiting (struct execution_control_state *ecs);
> -static void prepare_to_wait (struct execution_control_state *ecs);
> static void keep_going (struct execution_control_state *ecs);
> static void process_event_stop_test (struct execution_control_state *ecs);
> static int switch_back_to_stepped_thread (struct execution_control_state *ecs);
> @@ -4261,6 +4219,34 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
> }
> }
>
> +/* Called when we get an event that may finish an in-line or
> + out-of-line (displaced stepping) step-over started previously. */
> +
> +static void
> +finish_step_over (struct execution_control_state *ecs)
> +{
> + displaced_step_fixup (ecs->ptid,
> + ecs->event_thread->suspend.stop_signal);
> +
> + if (step_over_info_valid_p ())
> + {
> + /* If we're stepping over a breakpoint with all threads locked,
> + then only the thread that was stepped should be reporting
> + back an event. */
> + gdb_assert (ecs->event_thread->control.trap_expected);
> +
> + if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
> + clear_step_over_info ();
> + }
> +
> + if (!non_stop)
> + return;
> +
> + /* Start a new step-over in another thread if there's one that
> + needs it. */
> + start_step_over ();
> +}
> +
> /* Come here when the program has stopped with a signal. */
>
> static void
> @@ -4277,9 +4263,7 @@ handle_signal_stop (struct execution_control_state *ecs)
> /* Do we need to clean up the state of a thread that has
> completed a displaced single-step? (Doing so usually affects
> the PC, so do it here, before we set stop_pc.) */
> - displaced_step_fixup (ecs->ptid,
> - ecs->event_thread->suspend.stop_signal);
> - start_step_over ();
> + finish_step_over (ecs);
>
> /* If we either finished a single-step or hit a breakpoint, but
> the user wanted this thread to be stopped, pretend we got a
> @@ -5629,7 +5613,6 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
> {
> struct thread_info *tp;
> struct thread_info *stepping_thread;
> - struct thread_info *step_over;
>
> /* If any thread is blocked on some internal breakpoint, and we
> simply need to step over that breakpoint to get it going
> @@ -5672,14 +5655,20 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
> return 1;
> }
>
> - /* Otherwise, we no longer expect a trap in the current thread.
> - Clear the trap_expected flag before switching back -- this is
> - what keep_going does as well, if we call it. */
> - ecs->event_thread->control.trap_expected = 0;
> -
> - /* Likewise, clear the signal if it should not be passed. */
> - if (!signal_program[ecs->event_thread->suspend.stop_signal])
> - ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
> + /* If this thread needs yet another step-over (e.g., stepping
> + through a delay slot), do it first before moving on to
> + another thread. */
> + if (thread_still_needs_step_over (ecs->event_thread))
> + {
> + if (debug_infrun)
> + {
> + fprintf_unfiltered (gdb_stdlog,
> + "infrun: thread [%s] still needs step-over\n",
> + target_pid_to_str (ecs->event_thread->ptid));
> + }
> + keep_going (ecs);
> + return 1;
> + }
>
> /* If scheduler locking applies even if not stepping, there's no
> need to walk over threads. Above we've checked whether the
> @@ -5689,12 +5678,26 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
> if (schedlock_applies (ecs->event_thread))
> return 0;
>
> - /* Look for the stepping/nexting thread, and check if any other
> - thread other than the stepping thread needs to start a
> - step-over. Do all step-overs before actually proceeding with
> + /* Otherwise, we no longer expect a trap in the current thread.
> + Clear the trap_expected flag before switching back -- this is
> + what keep_going does as well, if we call it. */
> + ecs->event_thread->control.trap_expected = 0;
> +
> + /* Likewise, clear the signal if it should not be passed. */
> + if (!signal_program[ecs->event_thread->suspend.stop_signal])
> + ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
> +
> + /* Do all pending step-overs before actually proceeding with
> step/next/etc. */
> + if (start_step_over ())
> + {
> + prepare_to_wait (ecs);
> + return 1;
> + }
> +
> + /* Look for the stepping/nexting thread. */
> stepping_thread = NULL;
> - step_over = NULL;
> +
> ALL_NON_EXITED_THREADS (tp)
> {
> /* Ignore threads of processes we're not resuming. */
> @@ -5726,37 +5729,6 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
>
> stepping_thread = tp;
> }
> - else if (thread_still_needs_step_over (tp))
> - {
> - step_over = tp;
> -
> - /* At the top we've returned early if the event thread
> - is stepping. If some other thread not the event
> - thread is stepping, then scheduler locking can't be
> - in effect, and we can resume this thread. No need to
> - keep looking for the stepping thread then. */
> - break;
> - }
> - }
> -
> - if (step_over != NULL)
> - {
> - tp = step_over;
> - if (debug_infrun)
> - {
> - fprintf_unfiltered (gdb_stdlog,
> - "infrun: need to step-over [%s]\n",
> - target_pid_to_str (tp->ptid));
> - }
> -
> - /* Only the stepping thread should have this set. */
> - gdb_assert (tp->control.step_range_end == 0);
> -
> - ecs->ptid = tp->ptid;
> - ecs->event_thread = tp;
> - switch_to_thread (ecs->ptid);
> - keep_going (ecs);
> - return 1;
> }
>
> if (stepping_thread != NULL)
> @@ -5855,7 +5827,7 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
> fprintf_unfiltered (gdb_stdlog,
> "infrun: expected thread still "
> "hasn't advanced\n");
> - keep_going (ecs);
> + keep_going_pass (ecs);
> }
>
> return 1;
> @@ -6270,24 +6242,32 @@ stop_waiting (struct execution_control_state *ecs)
> ecs->wait_some_more = 0;
> }
>
> -/* Called when we should continue running the inferior, because the
> - current event doesn't cause a user visible stop. This does the
> - resuming part; waiting for the next event is done elsewhere. */
> +/* Like keep_going, but passes the signal to the inferior, even if the
> + signal is set to nopass. */
>
> static void
> -keep_going (struct execution_control_state *ecs)
> +keep_going_pass (struct execution_control_state *ecs)
====
For whatever weird reasons, "keep_going_pass" doesn't read very well to me.
"pass" as in "pass/fail"?
"pass" as in one of several passes, and this is the "keep going" one?
Can you rename this to keep_going_pass_signal?
TIA
> {
> /* Make sure normal_stop is called if we get a QUIT handled before
> reaching resume. */
> struct cleanup *old_cleanups = make_cleanup (resume_cleanups, 0);
>
> + gdb_assert (ptid_equal (ecs->event_thread->ptid, inferior_ptid));
> +
> /* Save the pc before execution, to compare with pc after stop. */
> ecs->event_thread->prev_pc
> = regcache_read_pc (get_thread_regcache (ecs->ptid));
>
> - if (ecs->event_thread->control.trap_expected
> - && ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_TRAP)
> + if (ecs->event_thread->control.trap_expected)
> {
> + struct thread_info *tp = ecs->event_thread;
> +
> + if (debug_infrun)
> + fprintf_unfiltered (gdb_stdlog,
> + "infrun: %s has trap_expected set, "
> + "resuming to collect trap\n",
> + target_pid_to_str (tp->ptid));
> +
> /* We haven't yet gotten our trap, and either: intercepted a
> non-signal event (e.g., a fork); or took a signal which we
> are supposed to pass through to the inferior. Simply
> @@ -6358,20 +6338,6 @@ keep_going (struct execution_control_state *ecs)
>
> ecs->event_thread->control.trap_expected = (remove_bp || remove_wps);
>
> - /* Do not deliver GDB_SIGNAL_TRAP (except when the user
> - explicitly specifies that such a signal should be delivered
> - to the target program). Typically, that would occur when a
> - user is debugging a target monitor on a simulator: the target
> - monitor sets a breakpoint; the simulator encounters this
> - breakpoint and halts the simulation handing control to GDB;
> - GDB, noting that the stop address doesn't map to any known
> - breakpoint, returns control back to the simulator; the
> - simulator then delivers the hardware equivalent of a
> - GDB_SIGNAL_TRAP to the program being debugged. */
> - if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
> - && !signal_program[ecs->event_thread->suspend.stop_signal])
> - ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
> -
> discard_cleanups (old_cleanups);
> resume (ecs->event_thread->suspend.stop_signal);
> }
> @@ -6379,6 +6345,22 @@ keep_going (struct execution_control_state *ecs)
> prepare_to_wait (ecs);
> }
>
> +/* Called when we should continue running the inferior, because the
> + current event doesn't cause a user visible stop. This does the
> + resuming part; waiting for the next event is done elsewhere. */
> +
> +static void
> +keep_going (struct execution_control_state *ecs)
> +{
> + if (ecs->event_thread->control.trap_expected
> + && ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
> + ecs->event_thread->control.trap_expected = 0;
> +
> + if (!signal_program[ecs->event_thread->suspend.stop_signal])
> + ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
> + keep_going_pass (ecs);
> +}
> +
> /* This function normally comes after a resume, before
> handle_inferior_event exits. It takes care of any last bits of
> housekeeping, and sets the all-important wait_some_more flag. */
> @@ -7806,8 +7788,19 @@ leave it stopped or free to run as needed."),
> signal_catch[i] = 0;
> }
>
> - /* Signals caused by debugger's own actions
> - should not be given to the program afterwards. */
> + /* Signals caused by debugger's own actions should not be given to
> + the program afterwards.
> +
> + Do not deliver GDB_SIGNAL_TRAP by default, except when the user
> + explicitly specifies that it should be delivered to the target
> + program. Typically, that would occur when a user is debugging a
> + target monitor on a simulator: the target monitor sets a
> + breakpoint; the simulator encounters this breakpoint and halts
> + the simulation handing control to GDB; GDB, noting that the stop
> + address doesn't map to any known breakpoint, returns control back
> + to the simulator; the simulator then delivers the hardware
> + equivalent of a GDB_SIGNAL_TRAP to the program being
> + debugged. */
> signal_program[GDB_SIGNAL_TRAP] = 0;
> signal_program[GDB_SIGNAL_INT] = 0;
>
> diff --git a/gdb/thread.c b/gdb/thread.c
> index 28e5ef8..3e3f419 100644
> --- a/gdb/thread.c
> +++ b/gdb/thread.c
> @@ -852,44 +852,62 @@ thread_change_ptid (ptid_t old_ptid, ptid_t new_ptid)
> observer_notify_thread_ptid_changed (old_ptid, new_ptid);
> }
>
> +/* Helper for set_running, that marks one thread either running or
> + stopped. */
> +
> +static int
> +set_running_thread (struct thread_info *tp, int running)
> +{
> + int started = 0;
> +
> + if (running && tp->state == THREAD_STOPPED)
> + started = 1;
> + tp->state = running ? THREAD_RUNNING : THREAD_STOPPED;
> +
> + if (!running)
> + {
> + /* If the thread is now marked stopped, remove it from
> + the step-over queue, so that we don't try to resume
> + it until the user wants it to. */
> + if (tp->step_over_next != NULL)
> + thread_step_over_chain_remove (tp);
> + }
> +
> + return started;
> +}
> +
> void
> set_running (ptid_t ptid, int running)
> {
> struct thread_info *tp;
> int all = ptid_equal (ptid, minus_one_ptid);
> + int any_started = 0;
>
> /* We try not to notify the observer if no thread has actually changed
> the running state -- merely to reduce the number of messages to
> frontend. Frontend is supposed to handle multiple *running just fine. */
> if (all || ptid_is_pid (ptid))
> {
> - int any_started = 0;
> -
> for (tp = thread_list; tp; tp = tp->next)
> if (all || ptid_get_pid (tp->ptid) == ptid_get_pid (ptid))
> {
> if (tp->state == THREAD_EXITED)
> continue;
> - if (running && tp->state == THREAD_STOPPED)
> +
> + if (set_running_thread (tp, running))
> any_started = 1;
> - tp->state = running ? THREAD_RUNNING : THREAD_STOPPED;
> }
> - if (any_started)
> - observer_notify_target_resumed (ptid);
> }
> else
> {
> - int started = 0;
> -
> tp = find_thread_ptid (ptid);
> - gdb_assert (tp);
> + gdb_assert (tp != NULL);
> gdb_assert (tp->state != THREAD_EXITED);
> - if (running && tp->state == THREAD_STOPPED)
> - started = 1;
> - tp->state = running ? THREAD_RUNNING : THREAD_STOPPED;
> - if (started)
> - observer_notify_target_resumed (ptid);
> + if (set_running_thread (tp, running))
> + any_started = 1;
> }
> + if (any_started)
> + observer_notify_target_resumed (ptid);
> }
>
> static int
> @@ -1008,9 +1026,8 @@ finish_thread_state (ptid_t ptid)
> continue;
> if (all || ptid_get_pid (ptid) == ptid_get_pid (tp->ptid))
> {
> - if (tp->executing && tp->state == THREAD_STOPPED)
> + if (set_running_thread (tp, tp->executing))
> any_started = 1;
> - tp->state = tp->executing ? THREAD_RUNNING : THREAD_STOPPED;
> }
> }
> }
> @@ -1020,9 +1037,8 @@ finish_thread_state (ptid_t ptid)
> gdb_assert (tp);
> if (tp->state != THREAD_EXITED)
> {
> - if (tp->executing && tp->state == THREAD_STOPPED)
> + if (set_running_thread (tp, tp->executing))
> any_started = 1;
> - tp->state = tp->executing ? THREAD_RUNNING : THREAD_STOPPED;
> }
> }
>
> --
> 1.9.3
>
On 04/22/2015 06:08 AM, Doug Evans wrote:
> Pedro Alves writes:
> > The main motivation of this patch is sharing more code between the
> > proceed (starting the inferior for the first time) and keep_going
> > (restarting the inferior after handling an event) paths and using the
> > step_over_chain queue now embedded in the thread_info object for
> > pending in-line step-overs too (instead of just for displaced
> > stepping).
>
> Hi.
> A couple of nits inline.
> grep for ====
>
Thanks.
> > + /* keep_going_pass skips the step-over if the breakpoint is no
> > + longer inserted. In all-stop, we want to keep looking for a
>
> ====
> missing word? "... keep looking for a thread" ?
Indeed.
> > - if (!tp->control.in_infcall)
> > - set_running (user_visible_resume_ptid (user_step), 1);
> > + if (debug_infrun)
> > + fprintf_unfiltered (gdb_stdlog,
> > + "Got placed in displaced stepping queue\n");
>
> ====
> Nit: Suggest rewording "displaced stepping queue".
> IIUC there is no displaced-stepping specific queue anymore.
Fixed:
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog,
+ "Got placed in step-over queue\n");
> > -/* Called when we should continue running the inferior, because the
> > - current event doesn't cause a user visible stop. This does the
> > - resuming part; waiting for the next event is done elsewhere. */
> > +/* Like keep_going, but passes the signal to the inferior, even if the
> > + signal is set to nopass. */
> >
> > static void
> > -keep_going (struct execution_control_state *ecs)
> > +keep_going_pass (struct execution_control_state *ecs)
>
> ====
> For whatever weird reasons, "keep_going_pass" doesn't read very well to me.
> "pass" as in "pass/fail"?
> "pass" as in one of several passes, and this is the "keep going" one?
>
> Can you rename this to keep_going_pass_signal?
No problem. Done that.
Thanks,
Pedro Alves
@@ -208,8 +208,10 @@ struct thread_info
/* Internal stepping state. */
- /* Record the pc of the thread the last time it stopped. This is
- maintained by proceed and keep_going, and used in
+ /* Record the pc of the thread the last time it was resumed. (It
+ can't be done on stop as the PC may change since the last stop,
+ e.g., "return" command, or "p $pc = 0xf000"). This is maintained
+ by proceed and keep_going, and among other things, it's used in
adjust_pc_after_break to distinguish a hardware single-step
SIGTRAP from a breakpoint SIGTRAP. */
CORE_ADDR prev_pc;
@@ -1844,30 +1844,61 @@ displaced_step_fixup (ptid_t event_ptid, enum gdb_signal signal)
displaced->step_ptid = null_ptid;
}
-/* Are there any pending step-over requests? If so, run one now. */
+/* Data to be passed around while handling an event. This data is
+ discarded between events. */
+struct execution_control_state
+{
+ ptid_t ptid;
+ /* The thread that got the event, if this was a thread event; NULL
+ otherwise. */
+ struct thread_info *event_thread;
+
+ struct target_waitstatus ws;
+ int stop_func_filled_in;
+ CORE_ADDR stop_func_start;
+ CORE_ADDR stop_func_end;
+ const char *stop_func_name;
+ int wait_some_more;
+
+ /* True if the event thread hit the single-step breakpoint of
+ another thread. Thus the event doesn't cause a stop, the thread
+ needs to be single-stepped past the single-step breakpoint before
+ we can switch back to the original stepping thread. */
+ int hit_singlestep_breakpoint;
+};
+
+/* Clear ECS and set it to point at TP. */
static void
+reset_ecs (struct execution_control_state *ecs, struct thread_info *tp)
+{
+ memset (ecs, 0, sizeof (*ecs));
+ ecs->event_thread = tp;
+ ecs->ptid = tp->ptid;
+}
+
+static void keep_going_pass (struct execution_control_state *ecs);
+static void prepare_to_wait (struct execution_control_state *ecs);
+static int thread_still_needs_step_over (struct thread_info *tp);
+
+/* Are there any pending step-over requests? If so, run one now and
+ return true. Otherwise, return false. */
+
+static int
start_step_over (void)
{
struct thread_info *tp, *next;
for (tp = step_over_queue_head; tp != NULL; tp = next)
{
- ptid_t ptid;
- struct displaced_step_inferior_state *displaced;
- struct regcache *regcache;
- struct gdbarch *gdbarch;
- CORE_ADDR actual_pc;
- struct address_space *aspace;
- struct inferior *inf = find_inferior_ptid (tp->ptid);
+ struct execution_control_state ecss;
+ struct execution_control_state *ecs = &ecss;
next = thread_step_over_chain_next (tp);
- displaced = get_displaced_stepping_state (inf->pid);
-
/* If this inferior already has a displaced step in process,
don't start a new one. */
- if (!ptid_equal (displaced->step_ptid, null_ptid))
+ if (displaced_step_in_progress (ptid_get_pid (tp->ptid)))
continue;
thread_step_over_chain_remove (tp);
@@ -1879,74 +1910,57 @@ start_step_over (void)
"infrun: step-over queue now empty\n");
}
- ptid = tp->ptid;
- context_switch (ptid);
-
- regcache = get_thread_regcache (ptid);
- actual_pc = regcache_read_pc (regcache);
- aspace = get_regcache_aspace (regcache);
- gdbarch = get_regcache_arch (regcache);
-
- if (breakpoint_here_p (aspace, actual_pc))
+ if (tp->control.trap_expected || tp->executing)
{
- if (debug_displaced)
- fprintf_unfiltered (gdb_stdlog,
- "displaced: stepping queued %s now\n",
- target_pid_to_str (ptid));
-
- displaced_step_prepare (ptid);
-
- if (debug_displaced)
- {
- CORE_ADDR actual_pc = regcache_read_pc (regcache);
- gdb_byte buf[4];
-
- fprintf_unfiltered (gdb_stdlog, "displaced: run %s: ",
- paddress (gdbarch, actual_pc));
- read_memory (actual_pc, buf, sizeof (buf));
- displaced_step_dump_bytes (gdb_stdlog, buf, sizeof (buf));
- }
-
- if (gdbarch_displaced_step_hw_singlestep (gdbarch,
- displaced->step_closure))
- target_resume (ptid, 1, GDB_SIGNAL_0);
- else
- target_resume (ptid, 0, GDB_SIGNAL_0);
-
- /* Done, we're stepping a thread. */
- break;
+ internal_error (__FILE__, __LINE__,
+ "[%s] has inconsistent state: "
+ "trap_expected=%d, executing=%d\n",
+ target_pid_to_str (tp->ptid),
+ tp->control.trap_expected,
+ tp->executing);
}
- else
- {
- int step;
- struct thread_info *tp = inferior_thread ();
-
- /* The breakpoint we were sitting under has since been
- removed. */
- tp->control.trap_expected = 0;
- /* Go back to what we were trying to do. */
- step = currently_stepping (tp);
-
- if (step)
- step = maybe_software_singlestep (gdbarch, actual_pc);
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog,
+ "infrun: resuming [%s] for step-over\n",
+ target_pid_to_str (tp->ptid));
+
+ /* keep_going_pass skips the step-over if the breakpoint is no
+ longer inserted. In all-stop, we want to keep looking for a
+ that needs a step-over instead of resuming TP, because we
+ wouldn't be able to resume anything else until the target
+ stops again. In non-stop, the resume always resumes only TP,
+ so it's OK to let the thread resume freely. */
+ if (!non_stop && !thread_still_needs_step_over (tp))
+ continue;
- if (debug_displaced)
- fprintf_unfiltered (gdb_stdlog,
- "displaced: breakpoint is gone: %s, step(%d)\n",
- target_pid_to_str (tp->ptid), step);
+ switch_to_thread (tp->ptid);
+ reset_ecs (ecs, tp);
+ keep_going_pass (ecs);
- target_resume (ptid, step, GDB_SIGNAL_0);
- tp->suspend.stop_signal = GDB_SIGNAL_0;
+ if (!ecs->wait_some_more)
+ error (_("Command aborted."));
- /* This request was discarded. See if there's any other
- thread waiting for its turn. */
+ if (!non_stop)
+ {
+ /* On all-stop, shouldn't have resumed unless we needed a
+ step over. */
+ gdb_assert (tp->control.trap_expected
+ || tp->step_after_step_resume_breakpoint);
+
+ /* With remote targets (at least), in all-stop, we can't
+ issue any further remote commands until the program stops
+ again. */
+ return 1;
}
- /* A new displaced stepping sequence started. Maybe we can
- start a displaced step on a thread of other process.
- Continue looking. */
+ /* Either the thread no longer needed a step-over, or a new
+ displaced stepping sequence started. Even in the latter
+ case, continue looking. Maybe we can also start another
+ displaced step on a thread of other process. */
}
+
+ return 0;
}
/* Update global variables holding ptids to hold NEW_PTID if they were
@@ -2283,16 +2297,11 @@ resume (enum gdb_signal sig)
if (!displaced_step_prepare (inferior_ptid))
{
- /* Got placed in displaced stepping queue. Will be resumed
- later when all the currently queued displaced stepping
- requests finish. The thread is not executing at this
- point, and the call to set_executing will be made later.
- But we need to call set_running here, since from the
- user/frontend's point of view, threads were set running.
- Unless we're calling an inferior function, as in that
- case we pretend the inferior doesn't run at all. */
- if (!tp->control.in_infcall)
- set_running (user_visible_resume_ptid (user_step), 1);
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog,
+ "Got placed in displaced stepping queue\n");
+
+ tp->control.trap_expected = 0;
discard_cleanups (old_cleanups);
return;
}
@@ -2367,14 +2376,6 @@ resume (enum gdb_signal sig)
by applying increasingly restricting conditions. */
resume_ptid = user_visible_resume_ptid (user_step);
- /* Even if RESUME_PTID is a wildcard, and we end up resuming less
- (e.g., we might need to step over a breakpoint), from the
- user/frontend's point of view, all threads in RESUME_PTID are now
- running. Unless we're calling an inferior function, as in that
- case pretend we inferior doesn't run at all. */
- if (!tp->control.in_infcall)
- set_running (resume_ptid, 1);
-
/* Maybe resume a single thread after all. */
if ((step || thread_has_single_step_breakpoints_set (tp))
&& tp->control.trap_expected)
@@ -2589,48 +2590,6 @@ schedlock_applies (struct thread_info *tp)
&& tp->control.stepping_command));
}
-/* Look a thread other than EXCEPT that has previously reported a
- breakpoint event, and thus needs a step-over in order to make
- progress. Returns NULL is none is found. */
-
-static struct thread_info *
-find_thread_needs_step_over (struct thread_info *except)
-{
- struct thread_info *tp, *current;
-
- /* With non-stop mode on, threads are always handled individually. */
- gdb_assert (! non_stop);
-
- current = inferior_thread ();
-
- /* If scheduler locking applies, we can avoid iterating over all
- threads. */
- if (schedlock_applies (except))
- {
- if (except != current
- && thread_still_needs_step_over (current))
- return current;
-
- return NULL;
- }
-
- ALL_NON_EXITED_THREADS (tp)
- {
- /* Ignore the EXCEPT thread. */
- if (tp == except)
- continue;
- /* Ignore threads of processes we're not resuming. */
- if (!sched_multi
- && ptid_get_pid (tp->ptid) != ptid_get_pid (inferior_ptid))
- continue;
-
- if (thread_still_needs_step_over (tp))
- return tp;
- }
-
- return NULL;
-}
-
/* Basic routine for continuing the program in various fashions.
ADDR is the address to resume at, or -1 for resume where stopped.
@@ -2651,6 +2610,11 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
struct thread_info *tp;
CORE_ADDR pc;
struct address_space *aspace;
+ ptid_t resume_ptid;
+ struct execution_control_state ecss;
+ struct execution_control_state *ecs = &ecss;
+ struct cleanup *old_chain;
+ int started;
/* If we're stopped at a fork/vfork, follow the branch set by the
"set follow-fork-mode" command; otherwise, we'll just proceed
@@ -2713,7 +2677,23 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
(next/step/etc.), we'll want to print stop event output to the MI
console channel (the stepped-to line, etc.), as if the user
entered the execution command on a real GDB console. */
- inferior_thread ()->control.command_interp = command_interp ();
+ tp->control.command_interp = command_interp ();
+
+ resume_ptid = user_visible_resume_ptid (tp->control.stepping_command);
+
+ /* If an exception is thrown from this point on, make sure to
+ propagate GDB's knowledge of the executing state to the
+ frontend/user running state. */
+ old_chain = make_cleanup (finish_thread_state_cleanup, &resume_ptid);
+
+ /* Even if RESUME_PTID is a wildcard, and we end up resuming fewer
+ threads (e.g., we might need to set threads stepping over
+ breakpoints first), from the user/frontend's point of view, all
+ threads in RESUME_PTID are now running. Unless we're calling an
+ inferior function, as in that case we pretend the inferior
+ doesn't run at all. */
+ if (!tp->control.in_infcall)
+ set_running (resume_ptid, 1);
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog,
@@ -2721,91 +2701,92 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
paddress (gdbarch, addr),
gdb_signal_to_symbol_string (siggnal));
- if (non_stop)
- /* In non-stop, each thread is handled individually. The context
- must already be set to the right thread here. */
- ;
- else
+ annotate_starting ();
+
+ /* Make sure that output from GDB appears before output from the
+ inferior. */
+ gdb_flush (gdb_stdout);
+
+ /* In a multi-threaded task we may select another thread and
+ then continue or step.
+
+ But if a thread that we're resuming had stopped at a breakpoint,
+ it will immediately cause another breakpoint stop without any
+ execution (i.e. it will report a breakpoint hit incorrectly). So
+ we must step over it first.
+
+ Look for threads other than the current (TP) that reported a
+ breakpoint hit and haven't been resumed yet since. */
+
+ /* If scheduler locking applies, we can avoid iterating over all
+ threads. */
+ if (!non_stop && !schedlock_applies (tp))
{
- struct thread_info *step_over;
+ struct thread_info *current = tp;
+
+ ALL_NON_EXITED_THREADS (tp)
+ {
+ /* Ignore the current thread here. It's handled
+ afterwards. */
+ if (tp == current)
+ continue;
- /* In a multi-threaded task we may select another thread and
- then continue or step.
+ /* Ignore threads of processes we're not resuming. */
+ if (!ptid_match (tp->ptid, resume_ptid))
+ continue;
- But if the old thread was stopped at a breakpoint, it will
- immediately cause another breakpoint stop without any
- execution (i.e. it will report a breakpoint hit incorrectly).
- So we must step over it first.
+ if (!thread_still_needs_step_over (tp))
+ continue;
+
+ gdb_assert (!thread_is_in_step_over_chain (tp));
- Look for a thread other than the current (TP) that reported a
- breakpoint hit and hasn't been resumed yet since. */
- step_over = find_thread_needs_step_over (tp);
- if (step_over != NULL)
- {
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog,
"infrun: need to step-over [%s] first\n",
- target_pid_to_str (step_over->ptid));
+ target_pid_to_str (tp->ptid));
- /* Store the prev_pc for the stepping thread too, needed by
- switch_back_to_stepped_thread. */
- tp->prev_pc = regcache_read_pc (get_current_regcache ());
- switch_to_thread (step_over->ptid);
- tp = step_over;
+ thread_step_over_chain_enqueue (tp);
}
- }
-
- /* If we need to step over a breakpoint, and we're not using
- displaced stepping to do so, insert all breakpoints (watchpoints,
- etc.) but the one we're stepping over, step one instruction, and
- then re-insert the breakpoint when that step is finished. */
- if (tp->stepping_over_breakpoint && !use_displaced_stepping (gdbarch))
- {
- struct regcache *regcache = get_current_regcache ();
- set_step_over_info (get_regcache_aspace (regcache),
- regcache_read_pc (regcache), 0);
+ tp = current;
}
- else
- clear_step_over_info ();
- insert_breakpoints ();
+ /* Enqueue the current thread last, so that we move all other
+ threads over their breakpoints first. */
+ if (tp->stepping_over_breakpoint)
+ thread_step_over_chain_enqueue (tp);
- tp->control.trap_expected = tp->stepping_over_breakpoint;
+ /* If the thread isn't started, we'll still need to set its prev_pc,
+ so that switch_back_to_stepped_thread knows the thread hasn't
+ advanced. Must do this before resuming any thread, as in
+ all-stop/remote, once we resume we can't send any other packet
+ until the target stops again. */
+ tp->prev_pc = regcache_read_pc (regcache);
- annotate_starting ();
+ started = start_step_over ();
- /* Make sure that output from GDB appears before output from the
- inferior. */
- gdb_flush (gdb_stdout);
+ if (step_over_info_valid_p ())
+ {
+ /* Either this thread started a new in-line step over, or some
+ other thread was already doing one. In either case, don't
+ resume anything else until the step-over is finished. */
+ }
+ else if (started && !non_stop)
+ {
+ /* A new displaced stepping sequence was started. In all-stop,
+ we can't talk to the target anymore until it next stops. */
+ }
+ else if (!tp->executing && !thread_is_in_step_over_chain (tp))
+ {
+ /* The thread wasn't started, and isn't queued, run it now. */
+ reset_ecs (ecs, tp);
+ switch_to_thread (tp->ptid);
+ keep_going_pass (ecs);
+ if (!ecs->wait_some_more)
+ error ("Command aborted.");
+ }
- /* Refresh prev_pc value just prior to resuming. This used to be
- done in stop_waiting, however, setting prev_pc there did not handle
- scenarios such as inferior function calls or returning from
- a function via the return command. In those cases, the prev_pc
- value was not set properly for subsequent commands. The prev_pc value
- is used to initialize the starting line number in the ecs. With an
- invalid value, the gdb next command ends up stopping at the position
- represented by the next line table entry past our start position.
- On platforms that generate one line table entry per line, this
- is not a problem. However, on the ia64, the compiler generates
- extraneous line table entries that do not increase the line number.
- When we issue the gdb next command on the ia64 after an inferior call
- or a return command, we often end up a few instructions forward, still
- within the original line we started.
-
- An attempt was made to refresh the prev_pc at the same time the
- execution_control_state is initialized (for instance, just before
- waiting for an inferior event). But this approach did not work
- because of platforms that use ptrace, where the pc register cannot
- be read unless the inferior is stopped. At that point, we are not
- guaranteed the inferior is stopped and so the regcache_read_pc() call
- can fail. Setting the prev_pc value here ensures the value is updated
- correctly when the inferior is stopped. */
- tp->prev_pc = regcache_read_pc (get_current_regcache ());
-
- /* Resume inferior. */
- resume (tp->suspend.stop_signal);
+ discard_cleanups (old_chain);
/* Wait for it to stop (if not standalone)
and in any case decode why it stopped, and act accordingly. */
@@ -2873,28 +2854,6 @@ init_wait_for_inferior (void)
}
-/* Data to be passed around while handling an event. This data is
- discarded between events. */
-struct execution_control_state
-{
- ptid_t ptid;
- /* The thread that got the event, if this was a thread event; NULL
- otherwise. */
- struct thread_info *event_thread;
-
- struct target_waitstatus ws;
- int stop_func_filled_in;
- CORE_ADDR stop_func_start;
- CORE_ADDR stop_func_end;
- const char *stop_func_name;
- int wait_some_more;
-
- /* True if the event thread hit the single-step breakpoint of
- another thread. Thus the event doesn't cause a stop, the thread
- needs to be single-stepped past the single-step breakpoint before
- we can switch back to the original stepping thread. */
- int hit_singlestep_breakpoint;
-};
static void handle_inferior_event (struct execution_control_state *ecs);
@@ -2908,7 +2867,6 @@ static void check_exception_resume (struct execution_control_state *,
static void end_stepping_range (struct execution_control_state *ecs);
static void stop_waiting (struct execution_control_state *ecs);
-static void prepare_to_wait (struct execution_control_state *ecs);
static void keep_going (struct execution_control_state *ecs);
static void process_event_stop_test (struct execution_control_state *ecs);
static int switch_back_to_stepped_thread (struct execution_control_state *ecs);
@@ -4261,6 +4219,34 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
}
}
+/* Called when we get an event that may finish an in-line or
+ out-of-line (displaced stepping) step-over started previously. */
+
+static void
+finish_step_over (struct execution_control_state *ecs)
+{
+ displaced_step_fixup (ecs->ptid,
+ ecs->event_thread->suspend.stop_signal);
+
+ if (step_over_info_valid_p ())
+ {
+ /* If we're stepping over a breakpoint with all threads locked,
+ then only the thread that was stepped should be reporting
+ back an event. */
+ gdb_assert (ecs->event_thread->control.trap_expected);
+
+ if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
+ clear_step_over_info ();
+ }
+
+ if (!non_stop)
+ return;
+
+ /* Start a new step-over in another thread if there's one that
+ needs it. */
+ start_step_over ();
+}
+
/* Come here when the program has stopped with a signal. */
static void
@@ -4277,9 +4263,7 @@ handle_signal_stop (struct execution_control_state *ecs)
/* Do we need to clean up the state of a thread that has
completed a displaced single-step? (Doing so usually affects
the PC, so do it here, before we set stop_pc.) */
- displaced_step_fixup (ecs->ptid,
- ecs->event_thread->suspend.stop_signal);
- start_step_over ();
+ finish_step_over (ecs);
/* If we either finished a single-step or hit a breakpoint, but
the user wanted this thread to be stopped, pretend we got a
@@ -5629,7 +5613,6 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
{
struct thread_info *tp;
struct thread_info *stepping_thread;
- struct thread_info *step_over;
/* If any thread is blocked on some internal breakpoint, and we
simply need to step over that breakpoint to get it going
@@ -5672,14 +5655,20 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
return 1;
}
- /* Otherwise, we no longer expect a trap in the current thread.
- Clear the trap_expected flag before switching back -- this is
- what keep_going does as well, if we call it. */
- ecs->event_thread->control.trap_expected = 0;
-
- /* Likewise, clear the signal if it should not be passed. */
- if (!signal_program[ecs->event_thread->suspend.stop_signal])
- ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
+ /* If this thread needs yet another step-over (e.g., stepping
+ through a delay slot), do it first before moving on to
+ another thread. */
+ if (thread_still_needs_step_over (ecs->event_thread))
+ {
+ if (debug_infrun)
+ {
+ fprintf_unfiltered (gdb_stdlog,
+ "infrun: thread [%s] still needs step-over\n",
+ target_pid_to_str (ecs->event_thread->ptid));
+ }
+ keep_going (ecs);
+ return 1;
+ }
/* If scheduler locking applies even if not stepping, there's no
need to walk over threads. Above we've checked whether the
@@ -5689,12 +5678,26 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
if (schedlock_applies (ecs->event_thread))
return 0;
- /* Look for the stepping/nexting thread, and check if any other
- thread other than the stepping thread needs to start a
- step-over. Do all step-overs before actually proceeding with
+ /* Otherwise, we no longer expect a trap in the current thread.
+ Clear the trap_expected flag before switching back -- this is
+ what keep_going does as well, if we call it. */
+ ecs->event_thread->control.trap_expected = 0;
+
+ /* Likewise, clear the signal if it should not be passed. */
+ if (!signal_program[ecs->event_thread->suspend.stop_signal])
+ ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
+
+ /* Do all pending step-overs before actually proceeding with
step/next/etc. */
+ if (start_step_over ())
+ {
+ prepare_to_wait (ecs);
+ return 1;
+ }
+
+ /* Look for the stepping/nexting thread. */
stepping_thread = NULL;
- step_over = NULL;
+
ALL_NON_EXITED_THREADS (tp)
{
/* Ignore threads of processes we're not resuming. */
@@ -5726,37 +5729,6 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
stepping_thread = tp;
}
- else if (thread_still_needs_step_over (tp))
- {
- step_over = tp;
-
- /* At the top we've returned early if the event thread
- is stepping. If some other thread not the event
- thread is stepping, then scheduler locking can't be
- in effect, and we can resume this thread. No need to
- keep looking for the stepping thread then. */
- break;
- }
- }
-
- if (step_over != NULL)
- {
- tp = step_over;
- if (debug_infrun)
- {
- fprintf_unfiltered (gdb_stdlog,
- "infrun: need to step-over [%s]\n",
- target_pid_to_str (tp->ptid));
- }
-
- /* Only the stepping thread should have this set. */
- gdb_assert (tp->control.step_range_end == 0);
-
- ecs->ptid = tp->ptid;
- ecs->event_thread = tp;
- switch_to_thread (ecs->ptid);
- keep_going (ecs);
- return 1;
}
if (stepping_thread != NULL)
@@ -5855,7 +5827,7 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
fprintf_unfiltered (gdb_stdlog,
"infrun: expected thread still "
"hasn't advanced\n");
- keep_going (ecs);
+ keep_going_pass (ecs);
}
return 1;
@@ -6270,24 +6242,32 @@ stop_waiting (struct execution_control_state *ecs)
ecs->wait_some_more = 0;
}
-/* Called when we should continue running the inferior, because the
- current event doesn't cause a user visible stop. This does the
- resuming part; waiting for the next event is done elsewhere. */
+/* Like keep_going, but passes the signal to the inferior, even if the
+ signal is set to nopass. */
static void
-keep_going (struct execution_control_state *ecs)
+keep_going_pass (struct execution_control_state *ecs)
{
/* Make sure normal_stop is called if we get a QUIT handled before
reaching resume. */
struct cleanup *old_cleanups = make_cleanup (resume_cleanups, 0);
+ gdb_assert (ptid_equal (ecs->event_thread->ptid, inferior_ptid));
+
/* Save the pc before execution, to compare with pc after stop. */
ecs->event_thread->prev_pc
= regcache_read_pc (get_thread_regcache (ecs->ptid));
- if (ecs->event_thread->control.trap_expected
- && ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_TRAP)
+ if (ecs->event_thread->control.trap_expected)
{
+ struct thread_info *tp = ecs->event_thread;
+
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog,
+ "infrun: %s has trap_expected set, "
+ "resuming to collect trap\n",
+ target_pid_to_str (tp->ptid));
+
/* We haven't yet gotten our trap, and either: intercepted a
non-signal event (e.g., a fork); or took a signal which we
are supposed to pass through to the inferior. Simply
@@ -6358,20 +6338,6 @@ keep_going (struct execution_control_state *ecs)
ecs->event_thread->control.trap_expected = (remove_bp || remove_wps);
- /* Do not deliver GDB_SIGNAL_TRAP (except when the user
- explicitly specifies that such a signal should be delivered
- to the target program). Typically, that would occur when a
- user is debugging a target monitor on a simulator: the target
- monitor sets a breakpoint; the simulator encounters this
- breakpoint and halts the simulation handing control to GDB;
- GDB, noting that the stop address doesn't map to any known
- breakpoint, returns control back to the simulator; the
- simulator then delivers the hardware equivalent of a
- GDB_SIGNAL_TRAP to the program being debugged. */
- if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
- && !signal_program[ecs->event_thread->suspend.stop_signal])
- ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
-
discard_cleanups (old_cleanups);
resume (ecs->event_thread->suspend.stop_signal);
}
@@ -6379,6 +6345,22 @@ keep_going (struct execution_control_state *ecs)
prepare_to_wait (ecs);
}
+/* Called when we should continue running the inferior, because the
+ current event doesn't cause a user visible stop. This does the
+ resuming part; waiting for the next event is done elsewhere. */
+
+static void
+keep_going (struct execution_control_state *ecs)
+{
+ if (ecs->event_thread->control.trap_expected
+ && ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
+ ecs->event_thread->control.trap_expected = 0;
+
+ if (!signal_program[ecs->event_thread->suspend.stop_signal])
+ ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
+ keep_going_pass (ecs);
+}
+
/* This function normally comes after a resume, before
handle_inferior_event exits. It takes care of any last bits of
housekeeping, and sets the all-important wait_some_more flag. */
@@ -7806,8 +7788,19 @@ leave it stopped or free to run as needed."),
signal_catch[i] = 0;
}
- /* Signals caused by debugger's own actions
- should not be given to the program afterwards. */
+ /* Signals caused by debugger's own actions should not be given to
+ the program afterwards.
+
+ Do not deliver GDB_SIGNAL_TRAP by default, except when the user
+ explicitly specifies that it should be delivered to the target
+ program. Typically, that would occur when a user is debugging a
+ target monitor on a simulator: the target monitor sets a
+ breakpoint; the simulator encounters this breakpoint and halts
+ the simulation handing control to GDB; GDB, noting that the stop
+ address doesn't map to any known breakpoint, returns control back
+ to the simulator; the simulator then delivers the hardware
+ equivalent of a GDB_SIGNAL_TRAP to the program being
+ debugged. */
signal_program[GDB_SIGNAL_TRAP] = 0;
signal_program[GDB_SIGNAL_INT] = 0;
@@ -852,44 +852,62 @@ thread_change_ptid (ptid_t old_ptid, ptid_t new_ptid)
observer_notify_thread_ptid_changed (old_ptid, new_ptid);
}
+/* Helper for set_running, that marks one thread either running or
+ stopped. */
+
+static int
+set_running_thread (struct thread_info *tp, int running)
+{
+ int started = 0;
+
+ if (running && tp->state == THREAD_STOPPED)
+ started = 1;
+ tp->state = running ? THREAD_RUNNING : THREAD_STOPPED;
+
+ if (!running)
+ {
+ /* If the thread is now marked stopped, remove it from
+ the step-over queue, so that we don't try to resume
+ it until the user wants it to. */
+ if (tp->step_over_next != NULL)
+ thread_step_over_chain_remove (tp);
+ }
+
+ return started;
+}
+
void
set_running (ptid_t ptid, int running)
{
struct thread_info *tp;
int all = ptid_equal (ptid, minus_one_ptid);
+ int any_started = 0;
/* We try not to notify the observer if no thread has actually changed
the running state -- merely to reduce the number of messages to
frontend. Frontend is supposed to handle multiple *running just fine. */
if (all || ptid_is_pid (ptid))
{
- int any_started = 0;
-
for (tp = thread_list; tp; tp = tp->next)
if (all || ptid_get_pid (tp->ptid) == ptid_get_pid (ptid))
{
if (tp->state == THREAD_EXITED)
continue;
- if (running && tp->state == THREAD_STOPPED)
+
+ if (set_running_thread (tp, running))
any_started = 1;
- tp->state = running ? THREAD_RUNNING : THREAD_STOPPED;
}
- if (any_started)
- observer_notify_target_resumed (ptid);
}
else
{
- int started = 0;
-
tp = find_thread_ptid (ptid);
- gdb_assert (tp);
+ gdb_assert (tp != NULL);
gdb_assert (tp->state != THREAD_EXITED);
- if (running && tp->state == THREAD_STOPPED)
- started = 1;
- tp->state = running ? THREAD_RUNNING : THREAD_STOPPED;
- if (started)
- observer_notify_target_resumed (ptid);
+ if (set_running_thread (tp, running))
+ any_started = 1;
}
+ if (any_started)
+ observer_notify_target_resumed (ptid);
}
static int
@@ -1008,9 +1026,8 @@ finish_thread_state (ptid_t ptid)
continue;
if (all || ptid_get_pid (ptid) == ptid_get_pid (tp->ptid))
{
- if (tp->executing && tp->state == THREAD_STOPPED)
+ if (set_running_thread (tp, tp->executing))
any_started = 1;
- tp->state = tp->executing ? THREAD_RUNNING : THREAD_STOPPED;
}
}
}
@@ -1020,9 +1037,8 @@ finish_thread_state (ptid_t ptid)
gdb_assert (tp);
if (tp->state != THREAD_EXITED)
{
- if (tp->executing && tp->state == THREAD_STOPPED)
+ if (set_running_thread (tp, tp->executing))
any_started = 1;
- tp->state = tp->executing ? THREAD_RUNNING : THREAD_STOPPED;
}
}