RE: [RFC] Allowing all threads of all|current process(es) to be resumed [new command + docs]

Marc Khouzam
2009-06-01T14:29:38Z

> -----Original Message-----
> From: Pedro Alves [mailto:pedro@codesourcery.com]
> Sent: May-30-09 6:52 AM
> To: gdb-patches@sourceware.org
> Cc: Marc Khouzam
> Subject: [RFC] Allowing all threads of all|current
> process(es) to be resumed [new command + docs]
>
> Hi guys,
>
> While moving the linux native multi-forks support ("info forks") into
> the generic multi-inferior framework I've ran into a little snag.
>
> Currently, with the generic framework, if GDB is attached to
> multiple processes, issuing a "continue", "next", etc., makes GDB
> resume all threads of all processes. But, with the multi-forks
> framework, GDB only debugs one of the forks at a given time, while
> leaving the others stopped. E.g.,
>
> (gdb) set detach-on-fork off
> (gdb) catch fork
> Catchpoint 1 (fork)
> (gdb) r
> Starting program:
> /home/pedro/gdb/sspaces/build/gdb/testsuite/gdb.base/foll-fork
>
> Catchpoint 1 (forked process 23902), 0x00007ffff789ac4b in
> fork () from /lib/libc.so.6
> (gdb)
>
> A 'continue' command here, will only resume the parent
> process, and leave
> the child process stopped.
>
> This means that moving multi-forks into multi-inferior
> framework changes
> its behaviour. If we make execution commands resume only
> threads of the current inferior, then we're changing the current
> default for targets making use of the current
> multi-inferior framework... So, we've got a conflict, and
> something's got
> to give. :-/ Both variants are reasonable and which one is
> right depends on
> what you're debugging. I'm thinking that the multi-forks behaviour
> should be the default, as it seems to be less surprising, and, it was
> here before...
>
> So, I've came up with this new command so that the user
> can select the behaviour desired. This currently only affects
> all-stop mode.
>
> What do you think of this?
>
> I'd prefer that GDB had notion of dynamic thread groups, a-la
> totalview, but I think we're a bit far for that yet, and
> it sounds like we'd still make use of something like this
> to affect the defaults.
>
> [Marc, you've guessed it, I'm CCing you because this may
> affect your current eclipse all-stop multi-process support. If this
> sticks, you'll just need to issue "set schedule-multiple on" once
> to get the current behaviour.]

Although it is possible, I don't believe anyone uses all-stop with
multi-process, we always use non-stop. So such a change is not
currently a big problem for Eclipse. So you get my vote for making
the behaviour more 'expected', as you suggest.

However, this whole thing forces me to finally take my head out
of the sand and address this scheduler-locking thing. As you may
guess, eclipse currently uses the GDB defaults for the all-stop
behaviour.
However, (and thanks to your explanation, which finally cleared things
up for me :-)), I think eclipse could/should take advantage of the
different options. Specifically, if a user is focused on a thread and
resumes: only that thread should be resumed (scheduler-locking on);
if the user if focused on an inferior: then all threads of that
inferior should be resumed (schedule-multiple off + scheduler-locking
off);
and if the user focuses on the entire debugging session: then
all threads of all inferiors should be resumed (schedule-multiple on +
scheduler-locking off). So, I like your new option, as it gives
more choices for the user. This makes all-stop resuming look more
like non-stop resuming.

Let me ask you, what is the way in non-stop to resume all threads
of the current process? -exec-continue is for a single thread,
-exec-continue --all is for all threads of all processes. Your
proposal addresses this issue for only for all-stop?

Thanks

marc

>
> --
> Pedro Alves
>
> 2009-05-30 Pedro Alves <pedro@codesourcery.com>
>
> * gdb.texinfo (All-Stop): Document new 'set schedule-multiple'
> command.
>
> 2009-05-30 Pedro Alves <pedro@codesourcery.com>
>
> * infrun.c (sched_multi): New global.
> (resume): If sched_multi is set, resume only threads of the
> current inferior.
> (prepare_to_proceed): Don't switch over to wait_ptid if we're
> resuming a different inferior, and sched_multi is off.
> (show_schedule_multiple): New.
> (_initialize_infrun): Register new "schedule-multiple" command.
> * inferior.h (sched_multi): Declare.
>
> ---
> gdb/doc/gdb.texinfo | 27 +++++++++++++++++++++
> gdb/inferior.h | 2 +
> gdb/infrun.c | 64
> +++++++++++++++++++++++++++++++++++++++++++---------
> 3 files changed, 83 insertions(+), 10 deletions(-)
>
> Index: src/gdb/doc/gdb.texinfo
> ===================================================================
> --- src.orig/gdb/doc/gdb.texinfo 2009-05-30
> 10:28:15.000000000 +0100
> +++ src/gdb/doc/gdb.texinfo 2009-05-30 11:36:02.000000000 +0100
> @@ -4650,6 +4650,33 @@ the current thread away from the thread
> Display the current scheduler locking mode.
> @end table
>
> +@cindex resume multiple processes
> +By default, @value{GDBN} allows only threads of the current inferior
> +to run in response to execution commands such as @code{continue}.
> +E.g., if @value{GDBN} is attached to two inferiors, each with two
> +threads, the @code{continue} command resumes only the two threads of
> +the current inferior. You can modify @value{GDBN}'s default behavior
> +by allowing all threads of all inferiors to be scheduled.
> +
> +@table @code
> +@kindex set schedule-multiple
> +@item set schedule-multiple
> +@cindex resume multiple processes
> +Sets the mode for scheduling multiple processes. When set, all
> +threads of all processes are allowed to run. When not set (which is
> +the default), only the threads of the current process are
> schedulable.
> +
> +@item show schedule-multiple
> +Display the current mode for scheduling multiple processes.
> +@end table
> +
> +The final set of threads that are allowed to run by
> execution commands
> +is defined by the union of the restrictions implied by both the
> +@code{schedule-multiple} and @code{scheduler-locking} modes.
> E.g., if
> +the @code{schedule-multiple} mode is set to @code{on}, but the
> +scheduler locking mode is also set to @code{on}, then only
> the current
> +thread is made schedulable.
> +
> @node Non-Stop Mode
> @subsection Non-Stop Mode
>
> Index: src/gdb/infrun.c
> ===================================================================
> --- src.orig/gdb/infrun.c 2009-05-30 01:36:41.000000000 +0100
> +++ src/gdb/infrun.c 2009-05-30 11:35:01.000000000 +0100
> @@ -1091,6 +1091,11 @@ set_schedlock_func (char *args, int from
> }
> }
>
> +/* True if execution commands resume all threads of all processes by
> + default; otherwise, resume only threads of the current inferior
> + process. */
> +int sched_multi = 0;
> +
> /* Try to setup for software single stepping over the
> specified location.
> Return 1 if target_resume() should use hardware single step.
>
> @@ -1201,13 +1206,25 @@ a command like `return' or `jump' to con
> {
> ptid_t resume_ptid;
>
> - resume_ptid = RESUME_ALL; /* Default */
> -
> /* If STEP is set, it's a request to use hardware stepping
> facilities. But in that case, we should never
> use singlestep breakpoint. */
> gdb_assert (!(singlestep_breakpoints_inserted_p && 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. */
> +
> + /* By default, resume all threads of all processes. */
> + resume_ptid = RESUME_ALL;
> +
> + /* Maybe resume only all threads of the current process. */
> + if ((!sched_multi || step) && target_supports_multi_process ())
> + {
> + resume_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
> + }
> +
> + /* Maybe resume a single thread after all. */
> if (singlestep_breakpoints_inserted_p
> && stepping_past_singlestep_breakpoint)
> {
> @@ -1224,9 +1241,8 @@ a command like `return' or `jump' to con
> to support, and has no value. */
> resume_ptid = inferior_ptid;
> }
> -
> - if ((step || singlestep_breakpoints_inserted_p)
> - && tp->trap_expected)
> + else if ((step || singlestep_breakpoints_inserted_p)
> + && tp->trap_expected)
> {
> /* We're allowing a thread to run past a breakpoint it has
> hit, by single-stepping the thread with the breakpoint
> @@ -1240,8 +1256,7 @@ a command like `return' or `jump' to con
> breakpoint, not just the one at PC. */
> resume_ptid = inferior_ptid;
> }
> -
> - if (non_stop)
> + else if (non_stop)
> {
> /* With non-stop mode on, threads are always handled
> individually. */
> @@ -1394,11 +1409,19 @@ prepare_to_proceed (int step)
> || (scheduler_mode == schedlock_step
> && step));
>
> + /* Don't switch over to WAIT_PTID if scheduler locking is on. */
> + if (schedlock_enabled)
> + return 0;
> +
> + /* Don't switch over if we're about to resume some other process
> + other than WAIT_PTID's, and schedule-multiple is off. */
> + if (!sched_multi
> + && ptid_get_pid (wait_ptid) != ptid_get_pid (inferior_ptid))
> + return 0;
> +
> /* Switched over from WAIT_PID. */
> if (!ptid_equal (wait_ptid, minus_one_ptid)
> - && !ptid_equal (inferior_ptid, wait_ptid)
> - /* Don't single step WAIT_PID if scheduler locking is on. */
> - && !schedlock_enabled)
> + && !ptid_equal (inferior_ptid, wait_ptid))
> {
> struct regcache *regcache = get_thread_regcache (wait_ptid);
>
> @@ -5573,6 +5596,14 @@ show_non_stop (struct ui_file *file, int
> value);
> }
>
> +static void
> +show_schedule_multiple (struct ui_file *file, int from_tty,
> + struct cmd_list_element *c, const char *value)
> +{
> + fprintf_filtered (file,
> + _("Scheduling multiple processes
> simultaneously is %s.\n"),
> + value);
> +}
>
> void
> _initialize_infrun (void)
> @@ -5752,6 +5783,19 @@ step == scheduler locked during every si
> show_scheduler_mode,
> &setlist, &showlist);
>
> + add_setshow_boolean_cmd ("schedule-multiple", class_run,
> &sched_multi, _("\
> +Set mode for scheduling multiple processes simultaneously."), _("\
> +Show mode for scheduling multiple processes simultaneously."), _("\
> +When set, all threads of all processes are affected by execution\n\
> +commands (such as e.g., 'continue'). When not set (which is the\n\
> +default), only the threads of the current process are made\n\
> +schedulable. The set of threads that are resumed is further\n\
> +refined by the scheduler-locking mode (see help set\n\
> +scheduler-locking)."),
> + NULL,
> + show_schedule_multiple,
> + &setlist, &showlist);
> +
> add_setshow_boolean_cmd ("step-mode", class_run,
> &step_stop_if_no_debug, _("\
> Set mode of the step operation."), _("\
> Show mode of the step operation."), _("\
> Index: src/gdb/inferior.h
> ===================================================================
> --- src.orig/gdb/inferior.h 2009-05-30 01:36:12.000000000 +0100
> +++ src/gdb/inferior.h 2009-05-30 01:37:18.000000000 +0100
> @@ -135,6 +135,8 @@ extern void clear_proceed_status (void);
>
> extern void proceed (CORE_ADDR, enum target_signal, int);
>
> +extern int sched_multi;
> +
> /* When set, stop the 'step' command if we enter a function which has
> no line number information. The normal behavior is that we step
> over such function. */
>