From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1509 invoked by alias); 30 May 2009 16:01:45 -0000 Received: (qmail 305 invoked by uid 22791); 30 May 2009 16:01:39 -0000 X-SWARE-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_37,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 30 May 2009 16:01:34 +0000 Received: (qmail 28227 invoked from network); 30 May 2009 16:01:31 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 30 May 2009 16:01:31 -0000 From: Pedro Alves To: gdb-patches@sourceware.org, Eli Zaretskii Subject: Re: [RFC] Allowing all threads of all|current process(es) to be resumed [new command + docs] Date: Sat, 30 May 2009 16:01:00 -0000 User-Agent: KMail/1.9.10 Cc: marc.khouzam@ericsson.com References: <200905301151.52892.pedro@codesourcery.com> <83fxem4tz4.fsf@gnu.org> In-Reply-To: <83fxem4tz4.fsf@gnu.org> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200905301701.53207.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-05/txt/msg00650.txt.bz2 On Saturday 30 May 2009 14:10:23, Eli Zaretskii wrote: > > From: Pedro Alves > > Date: Sat, 30 May 2009 11:51:52 +0100 > > Cc: Marc Khouzam > > > > 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. > > Can you give at least 2 different use-cases, each one favoring one of > the possible behaviors? I'm not sure I understand the nature of the > conflict. As I've explained above. When debugging with a multi-process aware target, and you're debugging multiple inferiors, "continue" resumes all threads of all processes. When debugging multi-forks on linux native, and you're attached to multiple forks, "continue" resumes all threads of *the current process* only. The conflict is that both "multi-process" implementations behave differently, and I'm getting rid of the multi-forks support as-is. But, I'd like to keep the option of having its behaviour. So, I'm adding the command so that the user can select what behaviour is wanted, either resume all processes, or current process only. Use case is "user wants to resume all processes" vs "user wants to resume only the current process". The multi-forks.exp test makes use of the "resume threads of the current process only" variant. It breaks badly if I allow all threads of all processes to be resumed. I've chosen the conservative default of behaving like the multi-forks.exp test expects. > A few comments about the patch for the manual: > > > +@cindex resume multiple processes > > This index entry is too general. I suggest something more specific to > the aspects you are describing. Putting myself in a user's perspective, if I wanted to know how to resume multiple process simultaneously, I'd go look for resume, multiple, maybe I should add "simultaneously". Did you have something completely different in mind? > > +By default, @value{GDBN} allows only threads of the current inferior > > +to run in response to execution commands such as @code{continue}. > > We don't introduce the notion of "execution commands" until the node > that's after this one. So if you want to use it here, I would suggest > to add the definition of this term in the parent node ("Thread > Stops"), and while at that, give a more-or-less comprehensive list of > the commands that are considered to be "execution commands". I do mention "such as @code{continue}". I'm now including a couple more examples. I can't see where "execution commands" are defined anywhere in other contexts either, so this hasn't been a problem so far. I think that such a change should be done independently of this one, perhaps then adding cross references everywhere appropriate. gdb.texinfo:to run in response to execution commands such as @code{continue}. gdb.texinfo:The final set of threads that are allowed to run by execution commands gdb.texinfo:execution commands such as @code{continue} and @code{step} apply by default gdb.texinfo:In non-stop mode, all execution commands apply only to the current thread gdb.texinfo:You can use @value{GDBN}'s background execution commands gdb.texinfo:The MI execution commands (@pxref{GDB/MI Program Execution}) are gdb.texinfo:Other execution commands do not currently support the @code{-a} option. gdb.texinfo:@value{GDBN}'s execution commands have two variants: the normal gdb.texinfo:background execution commands. You can use these commands to gdb.texinfo:message if you attempt to use the background execution commands. gdb.texinfo:just @code{c&}. The execution commands that accept background execution gdb.texinfo:the restriction that you cannot issue another execution command until the gdb.texinfo:@value{GDBN} will perform all execution commands in reverse, until the gdb.texinfo:@value{GDBN} will perform all execution commands in the normal fashion. gdb.texinfo:replay it later with both forward and reverse execution commands. gdb.texinfo:associated with the stop message is the one for the execution command gdb.texinfo:asynchronous just like other execution commands. That is, first the > > > +@cindex resume multiple processes > > This second instance of the same index item is redundant. > > > +Sets the mode for scheduling multiple processes. When set, all > ^^^^ > We use "Set", not "Sets", throughout the manual. Ok. > > Also, I suggest to mention threads: > > Set the mode for scheduling threads of multiple processes. > > On second thought, "scheduling" is not the best word here, as GDB is > not a scheduler. How about "continuing the execution"? as in: Set the mode for continuing the execution of multiple processes. ? I think that "continuing" may trigger confusion by making it look like only the "continue" command is affected. See new patch below. > > > +threads of all processes are allowed to run. When not set (which is > > +the default), only the threads of the current process are schedulable. > > I think we say "when OFF" and "when ON", rather than "when set" and > "when not set". Ah, I see that, in the manual. There are a few examples of boolean settings in GDB's online help that use "When set", and I happened to copy from one of those ("set step-mode", which is right below the new setting). I've used ON/OFF everywhere now. > > +@item show schedule-multiple > > +Display the current mode for scheduling multiple processes. > > Same comment about "threads" and "scheduling". > > > +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. > > It might be easier and simpler to tell that scheduler-locking takes > precedence. It does not really take scrict precedence, due to the "step" scheduler-locking variant. I think it would be odd because I'd have to explain that modes other than "scheduler-locking off" takes some sort of precedence. Here's the set of modes this allows. - continue all threads of all processes schedule-multiple on + scheduler-locking off - continue all threads of the current process (default) schedule-multiple off + scheduler-locking off - continue all threads of all processes, unless step is issued, in that case, step only the current thread. schedule-multiple on + scheduler-locking step - continue all threads of the current process, unless step is issued, in that case, step only the current thread. schedule-multiple off + scheduler-locking step - step only the current thread. schedule-multiple X + scheduler-locking on IMO, these are intuitive given the nature and description of both commands. I thought of implementing this new feature new modes of scheduler-locking instead, something like so: set scheduler-locking off (all threads of all processes can run) set scheduler-locking all-not-current-process (all threads of the current process can run, others remain locked) set scheduler-locking all-not-current-process-step (all threads not of the current process remain locked, except when stepping, in which case, lock all except the current thread) set scheduler-locking step (no thread is locked (all threads of all processes can run), except when stepping, in that case, lock all except the current thread) set scheduler-locking on (all threads but the current remain locked) But I looked to me that this is harder to understand. The nature of the "step" variant seems to want to be a different setting from "resume all processes or just current process". But I'm certainly open for discussion, hence the RFC. It's certainly possible that experience shows that these modes should be refined a bit differently. In the mean time, this allows me to go on with adding support for linux multi-process/multi-exec. > > > +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\ > > You do not need both "such as" and "e.g.", one or the other is enough. Ok. > Also, the same comments as above about "ON/OFF" vs "set/not set" apply > here. Ok. > We will also need an entry in NEWS for this new option. Yep. I was holding for the RFC to settle, but I've done this now. New patch below. Thanks. -- Pedro Alves 2009-05-30 Pedro Alves * gdb.texinfo (All-Stop): Document new 'set schedule-multiple' command. 2009-05-30 Pedro Alves * 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 "set schedule-multiple" command. * inferior.h (sched_multi): Declare. * NEWS: Mention new "schedule-multiple" setting. --- gdb/NEWS | 5 ++++ gdb/doc/gdb.texinfo | 30 ++++++++++++++++++++++++ gdb/inferior.h | 2 + gdb/infrun.c | 63 +++++++++++++++++++++++++++++++++++++++++++--------- 4 files changed, 90 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 16:34:46.000000000 +0100 @@ -4650,6 +4650,36 @@ the current thread away from the thread Display the current scheduler locking mode. @end table +@cindex resume threads of multiple processes simultaneously +By default, @value{GDBN} allows only threads of the current inferior +to run in response to execution commands such as @code{continue}, +@code{next} or @code{step}. 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 run instead. + +@table @code +@kindex set schedule-multiple +@item set schedule-multiple +Set the mode for allowing threads of multiple processes to be +scheduled when an execution command is issued. When @code{on}, all +threads of all processes are allowed to run. When @code{off}, only +the threads of the current process are schedulable. The default is +@code{off}. + +@item show schedule-multiple +Display the current mode for allowing threads of multiple processes to +run. +@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 16:19:21.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 && 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, _("\ +Allowing threads of multiple processes be scheduled simultaneously is %s.\n"), + value); +} void _initialize_infrun (void) @@ -5752,6 +5783,18 @@ step == scheduler locked during every si show_scheduler_mode, &setlist, &showlist); + add_setshow_boolean_cmd ("schedule-multiple", class_run, &sched_multi, _("\ +Set mode for allowing threads of multiple processes to be scheduled."), _("\ +Show mode for allowing threads of multiple processes to be scheduled."), _("\ +When on, execution commands (such as 'continue') resume all threads of\n\ +all processes. When off (which is the default), execution commands\n\ +only resume the threads of the current process. The set of threads\n\ +that are resumed is further refined by the scheduler-locking mode (see\n\ +help set 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 12:49:53.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. */ Index: src/gdb/NEWS =================================================================== --- src.orig/gdb/NEWS 2009-05-30 16:22:56.000000000 +0100 +++ src/gdb/NEWS 2009-05-30 16:35:58.000000000 +0100 @@ -305,6 +305,11 @@ show libthread-db-search-path Control list of directories which GDB will search for appropriate libthread_db. +set schedule-multiple (on|off) +show schedule-multiple + Allow GDB to resume all threads of all processes or only threads of + the current process. + * New native configurations x86/x86_64 Darwin i[34567]86-*-darwin*