This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v3 10/17] Implement all-stop on top of a target running non-stop mode
- From: Doug Evans <dje at google dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Mon, 18 May 2015 21:27:50 +0000
- Subject: Re: [PATCH v3 10/17] Implement all-stop on top of a target running non-stop mode
- Authentication-results: sourceware.org; auth=none
Pedro Alves writes:
...
> gdb/ChangeLog:
> 2015-04-17 Pedro Alves <palves@redhat.com>
>
> * NEWS: Mention "maint set/show target-non-stop".
> * breakpoint.c (update_global_location_list): Check
> target_is_non_stop_p instead of non_stop.
> * infcmd.c (attach_command_post_wait, attach_command): Likewise.
> * infrun.c (show_can_use_displaced_stepping)
> (can_use_displaced_stepping_p, start_step_over_inferior):
> Likewise.
> (resume): Always resume a single thread if the target is in
> non-stop mode.
> (proceed): Check target_is_non_stop_p instead of non_stop. If in
> all-mode but the target is always in non-stop mode, start all the
> other threads that are implicitly resumed too.
> (for_each_just_stopped_thread, fetch_inferior_event)
> (adjust_pc_after_break, stop_all_threads): Check
> target_is_non_stop_p instead of non_stop.
> (handle_inferior_event): Likewise. Handle detach-fork in all-stop
> with the target always in non-stop mode.
> (handle_signal_stop) <random signal>: If we get a signal while
> stepping over a breakpoint, and the target is always in non-stop
> mode, restart all threads.
> (switch_back_to_stepped_thread): Check target_is_non_stop_p
> instead of non_stop.
> (keep_going_stepped_thread): Always resume a single thread if the
> target is in non-stop mode.
> (stop_waiting): If in all-stop mode, and the target is in non-stop
> mode, stop all threads.
> (keep_going_pass): Likewise, when starting a new in-line step-over
> sequence.
> * linux-nat.c (get_pending_status, select_event_lwp)
> (linux_nat_filter_event, linux_nat_wait_1, linux_nat_wait): Check
> target_is_non_stop_p instead of non_stop.
> (linux_nat_always_non_stop_p): New function.
> (linux_nat_stop): Check target_is_non_stop_p instead of non_stop.
> (linux_nat_add_target): Install linux_nat_always_non_stop_p.
> * target-delegates.c: Regenerate.
> * target.c (target_is_non_stop_p): New function.
> (target_non_stop_enabled, target_non_stop_enabled_1): New globals.
> (maint_set_target_non_stop_command)
> (maint_show_target_non_stop_command): New functions.
> (_initilize_target): Install "maint set/show target-non-stop"
> commands.
> * target.h (struct target_ops) <to_always_non_stop_p>: New field.
> (target_non_stop_enabled): New declaration.
> (target_is_non_stop_p): New declaration.
>
> gdb/doc/ChangeLog:
> 2015-04-07 Pedro Alves <palves@redhat.com>
>
> * gdb.texinfo (Maintenance Commands): Document "maint set/show
> target-non-stop".
>
...
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 8859b9f..f6d3e07 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -1632,7 +1632,7 @@ show_can_use_displaced_stepping (struct ui_file
*file, int from_tty,
> fprintf_filtered (file,
> _("Debugger's willingness to use displaced stepping "
> "to step over breakpoints is %s (currently %s).\n"),
> - value, non_stop ? "on" : "off");
> + value, target_is_non_stop_p () ? "on" : "off");
> else
> fprintf_filtered (file,
> _("Debugger's willingness to use displaced stepping "
> @@ -1645,7 +1645,8 @@ show_can_use_displaced_stepping (struct ui_file
*file, int from_tty,
> static int
> use_displaced_stepping (struct gdbarch *gdbarch)
> {
> - return (((can_use_displaced_stepping == AUTO_BOOLEAN_AUTO && non_stop)
> + return (((can_use_displaced_stepping == AUTO_BOOLEAN_AUTO
> + && target_is_non_stop_p ())
> || can_use_displaced_stepping == AUTO_BOOLEAN_TRUE)
> && gdbarch_displaced_step_copy_insn_p (gdbarch)
> && find_record_target () == NULL);
> @@ -2016,7 +2017,7 @@ start_step_over (void)
> 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 && !step_what)
> + if (!target_is_non_stop_p () && !step_what)
> continue;
>
> switch_to_thread (tp->ptid);
> @@ -2035,7 +2036,7 @@ start_step_over (void)
> return 1;
> }
>
> - if (!non_stop)
> + if (!target_is_non_stop_p ())
> {
> /* On all-stop, shouldn't have resumed unless we needed a
> step over. */
> @@ -2383,7 +2384,10 @@ resume (enum gdb_signal sig)
> insert_single_step_breakpoint (gdbarch, aspace, pc);
> insert_breakpoints ();
>
> - resume_ptid = user_visible_resume_ptid (user_step);
> + if (target_is_non_stop_p ())
> + resume_ptid = inferior_ptid;
> + else
> + resume_ptid = user_visible_resume_ptid (user_step);
Hi.
For my own education, why is this change needed?
> do_target_resume (resume_ptid, 0, GDB_SIGNAL_0);
> discard_cleanups (old_cleanups);
> tp->resumed = 1;
> @@ -2498,8 +2502,14 @@ resume (enum gdb_signal sig)
> resume_ptid = user_visible_resume_ptid (user_step);
>
> /* Maybe resume a single thread after all. */
> - if ((step || thread_has_single_step_breakpoints_set (tp))
> - && tp->control.trap_expected)
> + if (target_is_non_stop_p ())
> + {
> + /* If non-stop mode, threads are always controlled
> + individually. */
> + resume_ptid = inferior_ptid;
> + }
> + else if ((step || thread_has_single_step_breakpoints_set (tp))
> + && tp->control.trap_expected)
> {
> /* We're allowing a thread to run past a breakpoint it has
> hit, by single-stepping the thread with the breakpoint
> @@ -2935,11 +2945,51 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
> 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)
> + else if (started && !target_is_non_stop_p ())
> {
> /* A new displaced stepping sequence was started. In all-stop,
> we can't talk to the target anymore until it next stops. */
> }
> + else if (!non_stop && target_is_non_stop_p ())
This takes a bit of reading to grok (the difference
b/w non_stop and target_is_non_stop_p).
Can you copy the comment below (marked XYZ) here?
> + {
> + /* Start all other threads that are implicitly resumed too. */
> + ALL_NON_EXITED_THREADS (tp)
> + {
> + /* Ignore threads of processes we're not resuming. */
> + if (!ptid_match (tp->ptid, resume_ptid))
> + continue;
> +
> + if (tp->resumed)
> + {
> + if (debug_infrun)
> + fprintf_unfiltered (gdb_stdlog,
> + "infrun: proceed: [%s] resumed\n",
> + target_pid_to_str (tp->ptid));
> + gdb_assert (tp->executing || tp->suspend.waitstatus_pending_p);
> + continue;
> + }
> +
> + if (thread_is_in_step_over_chain (tp))
> + {
> + if (debug_infrun)
> + fprintf_unfiltered (gdb_stdlog,
> + "infrun: proceed: [%s] needs step-over\n",
> + target_pid_to_str (tp->ptid));
> + continue;
> + }
> +
> + if (debug_infrun)
> + fprintf_unfiltered (gdb_stdlog,
> + "infrun: proceed: resuming %s\n",
> + target_pid_to_str (tp->ptid));
> +
> + reset_ecs (ecs, tp);
> + switch_to_thread (tp->ptid);
> + keep_going_pass (ecs);
> + if (!ecs->wait_some_more)
> + error ("Command aborted.");
> + }
> + }
> else if (!tp->resumed && !thread_is_in_step_over_chain (tp))
> {
> /* The thread wasn't started, and isn't queued, run it now. */
...
> @@ -5037,7 +5089,7 @@ finish_step_over (struct execution_control_state
*ecs)
> clear_step_over_info ();
> }
>
> - if (!non_stop)
> + if (!target_is_non_stop_p ())
> return 0;
>
> /* Start a new step-over in another thread if there's one that
> @@ -5614,6 +5666,17 @@ handle_signal_stop (struct
execution_control_state *ecs)
> /* Reset trap_expected to ensure breakpoints are re-inserted. */
> ecs->event_thread->control.trap_expected = 0;
>
> + if (!non_stop && target_is_non_stop_p ())
Ditto. Copy XYZ here?
> + {
> + keep_going (ecs);
> +
> + /* We've canceled the step-over temporarily while the
> + signal handler executes. Let other threads run,
> + according to schedlock. */
> + restart_threads (ecs->event_thread);
> + return;
> + }
> +
> /* If we were nexting/stepping some other thread, switch to
> it, so that we don't continue it, losing control. */
> if (!switch_back_to_stepped_thread (ecs))
...
> @@ -7153,6 +7220,11 @@ stop_waiting (struct execution_control_state *ecs)
>
> /* Let callers know we don't want to wait for the inferior anymore.
*/
> ecs->wait_some_more = 0;
> +
> + /* If all-stop, but the target is always in non-stop mode, stop all
> + threads now that we're presenting the stop to the user. */
XYZ^^^
"If all-stop, but the target is always in non-stop mode, ..."
> + if (!non_stop && target_is_non_stop_p ())
> + stop_all_threads ();
> }
>
> /* Like keep_going, but passes the signal to the inferior, even if the
> @@ -7267,7 +7339,7 @@ keep_going_pass (struct execution_control_state
*ecs)
> insert_breakpoints below, because that removes the breakpoint
> we're about to step over, otherwise other threads could miss
> it. */
> - if (step_over_info_valid_p () && non_stop)
> + if (step_over_info_valid_p () && target_is_non_stop_p ())
> stop_all_threads ();
>
> /* Stop stepping if inserting breakpoints fails. */