Multiexec MI
Pedro Alves
pedro@codesourcery.com
Mon Feb 8 19:20:00 GMT 2010
On Wednesday 13 January 2010 20:29:30, Vladimir Prus wrote:
> This patch implements MI support for multiexec. I attach my notes on design, as well
> as patch. The patch also contains documentation updates.
>
> The executive summary is:
> * thread groups of types 'process' are redefined to mean inferior, and therefore
> can exist before process is started, and outlive the process.
> * The --thread-group option, previously available for select MI commands, is now
> globally available. Therefore, things like:
>
> -file-exec-and-symbols --thread-group i1 foobar
>
> works.
> * The --all option to MI exec commands now affects threads in all inferiors.
>
> The important caveat is that multiexec MI is only really working in non-stop.
> At least the --all option, in all-stops, runs into various core issues. It's not
> presently known how many are there and whether they are fixable.
Yeah. I tried it on a simple test I use often, much like
schedlock.c (spawns a few threads that go busy looping), creating
a few inferiors using that program, and `-exec-run --all' didn't
behave correctly; it didn't crash, but a bunch of threads
missing. I haven't investigated fully, but there's the
option of not supporting it until it actually works..
>
> - Volodya
> multiexec-mi.diff
> commit 8fb18657cf8d53b951c84e76002e5a57dc75a944
> Author: Vladimir Prus <vladimir@codesourcery.com>
> Date: Sat Dec 19 17:38:00 2009 +0300
>
> Multiexec MI
>
> gdb/
> * breakpoint.c (clear_syscall_counts): Take struct inferior*.
>
> gdb/doc/
> * gdb.texinfo (GDB/MI Command Syntax): Document notification
> changes.
> (GDB/MI Program Execution): Document current behaviour of
> --all and --thread-group.
> (GDB/MI Miscellaneous Commands): Document -add-inferior and
> -remove-inferior.
> * observer.texi (inferior_added, inferior_removed): New
> observers.
>
> * inferior.c (add_inferior_silent): Notify inferior_added
> observer.
> (delete_inferior_1): Notify inferior_removed observer.
> (exit_inferior_1): Pass inferior, not pid, to observer.
> (inferior_appeared): Likewise.
> (add_inferior_with_spaces): New.
> (add_inferior_command): Use the above.
> * inferior.h (delete_inferior_1, add_inferior_with_spaces):
> Declare.
>
> * inflow.c (inflow_inferior_exit): Likewise.
> * jit.c (jit_inferior_exit_hook): Likewise.
>
> * mi/mi-cmds.c (mi_cmds): Register add-inferior and
> remove-inferior.
> * mi/mi-cmds.h (mi_cmd_add_inferior, mi_cmd_remove_inferior): New.
> * mi/mi-interp.c (mi_inferior_added, mi_inferior_removed): New.
> (report_initial_inferior): New.
> (mi_inferior_removed): Register the above. Make sure
> inferior_added observer is called on the first inferior.
> (mi_new_thread, mi_thread_exit): Thread group is now identified by
> inferior number, not pid.
> (mi_solib_loaded, mi_solib_unloaded): Report which inferiors are
> affected.
> * mi/mi-main.c (current_context): New.
> (proceed_thread_callback): Use typed closure.
> Proceed everything if pid is 0.
> (proceed_thread_callback_wrapper): New.
> (run_one_inferior): New.
> (mi_cmd_exec_continue, mi_cmd_exec_interrupt, mi_cmd_exec_run):
> Adjust for multiexec behaviour.
> (mi_cmd_add_inferior, mi_cmd_remove_inferior): New.
> (mi_cmd_execute): Handle the 'thread-group' option here.
> Do some extra checks.
> * mi-parse.c (mi_parse): Handle the --all and --thread-group
> options.
> * mi-parse.h (struct mi_parse): New fields all and thread_group.
>
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 0dc8474..0d8a471 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -10230,10 +10230,8 @@ add_catch_command (char *name, char *docstring,
> }
>
> static void
> -clear_syscall_counts (int pid)
> +clear_syscall_counts (struct inferior *inf)
> {
> - struct inferior *inf = find_inferior_pid (pid);
> -
> inf->total_syscalls_count = 0;
> inf->any_syscall_count = 0;
> VEC_free (int, inf->syscalls_counts);
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 02e2bbd..6da2d12 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -21528,6 +21528,11 @@ groups can be obtained using @samp{-list-thread-groups --available}.
> In general, the content of a thread group may be only retrieved only
> after attaching to that thread group.
>
> +Thread groups are related to inferiors (@pxref{Inferiors and
> +Programs}). Each inferior corresponds to a thread group of a special
> +type @samp{process}, and some additional operations are permitted on
> +such thread groups.
> +
> @c %%%%%%%%%%%%%%%%%%%%%%%%%%%% SECTION %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
> @node GDB/MI Command Syntax
> @section @sc{gdb/mi} Command Syntax
> @@ -21969,9 +21974,24 @@ several threads in the list. The @var{core} field reports the
> processor core on which the stop event has happened. This field may be absent
> if such information is not available.
>
> -@item =thread-group-created,id="@var{id}"
> +@item =thread-group-added,id="@var{id}"
> +@itemx =thread-group-removed,id="@var{id}"
> +A thread thread group was either added or removed. The @var{id} field
> +contains the @value{GDBN} identifier of the thread group. When a thread
> +group is added, it generally might not be associated with a running
> +process. When a thread group is removed, its id becomes invalid and
> +cannot be used in any way.
> +
> +@item =thread-group-started,id="@var{id}",pid="@var{pid}"
> +A thread group either because associated with a running program,
> +either because the program was started or it the thread group
> +was attached to a program. The @var{id} field contains the
> +@value{GDBN} identifier of the thread group. The @var{pid} field
> +contains process identifier, specific to the operating system.
> +
> @itemx =thread-group-exited,id="@var{id}"
> -A thread thread group either was attached to, or has exited/detached
> +A thread thread group is no longer associated with a running program,
> +either because the program has exited, or because it was detached
> from. The @var{id} field contains the @value{GDBN} identifier of the
> thread group.
>
> @@ -22002,12 +22022,18 @@ opaque identifier of the library. For remote debugging case,
> library file on the target, and on the host respectively. For native
> debugging, both those fields have the same value. The
> @var{symbols-loaded} field reports if the debug symbols for this
> -library are loaded.
> +library are loaded. The @var{thread-group} field, if present,
> +contains the id of the thread group in which the library was loaded.
> +If the field is absent, it means the library was loaded in all present
> +thread groups.
>
> @item =library-unloaded,...
> Reports that a library was unloaded by the program. This notification
> has 3 fields---@var{id}, @var{target-name} and @var{host-name} with
> -the same meaning as for the @code{=library-loaded} notification
> +the same meaning as for the @code{=library-loaded} notification. The
> +@var{thread-group} field, if present, contains the id of the thread
> +group in which the library was loaded. If the field is absent,
> +it means the library was loaded in all present thread groups.
>
> @end table
>
> @@ -23106,7 +23132,7 @@ other cases.
> @subsubheading Synopsis
>
> @smallexample
> - -exec-continue [--all|--thread-group N]
> + -exec-continue [--all | --thread-group N]
> @end smallexample
>
> Resumes the execution of the inferior program until a breakpoint is
> @@ -23116,7 +23142,7 @@ depending on the value of the @samp{scheduler-locking} variable. In
> non-stop mode (@pxref{Non-Stop Mode}), if the @samp{--all} is not
> specified, only the thread specified with the @samp{--thread} option
> (or current thread, if no @samp{--thread} is provided) is resumed. If
> -@samp{--all} is specified, all threads will be resumed. The
> +@samp{--all} is specified, all threads (in all inferiours) will be resumed. The
s/inferiours/inferiors/ there are more instances of this.
> @samp{--all} option is ignored in all-stop mode. If the
> @samp{--thread-group} options is specified, then all threads in that
> thread group are resumed.
> @@ -23206,9 +23232,9 @@ asynchronous just like other execution commands. That is, first the
> reported after that using the @samp{*stopped} notification.
>
> In non-stop mode, only the context thread is interrupted by default.
> -All threads will be interrupted if the @samp{--all} option is
> -specified. If the @samp{--thread-group} option is specified, all
> -threads in that group will be interrupted.
> +All threads (in all inferiours) will be interrupted if the
> +@samp{--all} option is specified. If the @samp{--thread-group}
> +option is specified, all threads in that group will be interrupted.
^ looks like spurious space.
>
> @subsubheading @value{GDBN} Command
>
> @@ -23372,7 +23398,7 @@ fullname="/home/foo/bar/devo/gdb/testsuite/gdb.mi/basics.c",line="18"@}
> @subsubheading Synopsis
>
> @smallexample
> - -exec-run
> + -exec-run [--all | --thread-group N ]
> @end smallexample
>
> Starts execution of the inferior from the beginning. The inferior
> @@ -23380,6 +23406,11 @@ executes until either a breakpoint is encountered or the program
> exits. In the latter case the output will include an exit code, if
> the program has exited exceptionally.
>
> +When no option is specified, the current inferiour is started. If the
> +@samp{--thread-group} option is specified, it should refer to a thread
> +group of type @samp{process}, and that thread group will be started.
> +If the @samp{--all} option is specified, then all inferiours will be started.
> +
> @subsubheading @value{GDBN} Command
>
> The corresponding @value{GDBN} command is @samp{run}.
> @@ -26437,7 +26468,8 @@ have the following fields:
>
> @table @code
> @item id
> -Identifier of the thread group. This field is always present.
> +Identifier of the thread group. This field is always present. The
> +identifier is an opaque string, and is not necessary an integer.
>
> @item type
> The type of the thread group. At present, only @samp{process} is a
> @@ -26445,7 +26477,7 @@ valid type.
>
> @item pid
> The target-specific process identifier. This field is only present
> -for thread groups of type @samp{process}.
> +for thread groups of type @samp{process} and only if the process exists.
>
> @item num_children
> The number of children this thread group has. This field may be
> @@ -26461,6 +26493,11 @@ This field is a list of integers, each identifying a core that one
> thread of the group is running on. This field may be absent if
> such information is not available.
>
> +@item executable
> +The name of the executable file that corresponds to this thread group.
> +The field is only present for thread groups of type @samp{process},
> +and only if there is corresponding executable file.
> +
> @end table
>
> @subheading Example
> @@ -26487,6 +26524,31 @@ such information is not available.
> @{id="2",target-id="Thread 0xb7e14b90",cores=[2]@}]@},...]
> @end smallexample
>
> +
> +@subheading The @code{-add-inferior} Command
> +@findex -add-inferior
> +
> +@subheading Synopsis
> +
> +@smallexample
> +-add-inferior
> +@end smallexample
> +
> +Creates a new inferior (@pxref{Inferiors and Programs}). The created
> +inferior is not associated with any executable. Such associated may
> +be established with the @samp{-file-exec-and-symbols} command
> +(@pxref{GDB/MI File Commands}). The command response has a single
> +field, @samp{thread-group}, whose value is the identifier of the
> +thread group corresponding to the new inferior.
> +
> +@subheading Example
> +
> +@smallexample
> +@value{GDBP}
> +-add-inferior
> +^done,thread-group="i3"
> +@end smallexample
> +
> @subheading The @code{-interpreter-exec} Command
> @findex -interpreter-exec
>
> diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi
> index db3d114..fb0cc9d 100644
> --- a/gdb/doc/observer.texi
> +++ b/gdb/doc/observer.texi
> @@ -199,13 +199,23 @@ The thread's ptid has changed. The @var{old_ptid} parameter specifies
> the old value, and @var{new_ptid} specifies the new value.
> @end deftypefun
>
> -@deftypefun void inferior_appeared (int @var{pid})
> -@value{GDBN} has attached to a new inferior identified by @var{pid}.
> +@deftypefun void inferior_added (struct inferior *@var{inf})
> +The inferior @var{inf} has been added to the list of inferiour. At
> +this point, it might not be associated with any process.
> @end deftypefun
>
> -@deftypefun void inferior_exit (int @var{pid})
> -Either @value{GDBN} detached from the inferior, or the inferior
> -exited. The argument @var{pid} identifies the inferior.
> +@deftypefun void inferior_appeared (struct inferior *@var{inf})
> +The inferior identified by @var{inf} has been attached to a process.
> +@end deftypefun
> +
> +@deftypefun void inferior_exit (struct inferior *@var{inf})
> +Either the inferior associated with @var{inf} has been detached from the
> +process, or the process has exited.
> +@end deftypefun
> +
> +@deftypefun void inferior_removed (struct inferior *@var{inf})
> +The inferior @var{inf} has been removed from the list of inferiors.
> +This method is called immediate before freeing @var{inf}.
> @end deftypefun
>
> @deftypefun void memory_changed (CORE_ADDR @var{addr}, int @var{len}, const bfd_byte *@var{data})
> @@ -213,8 +223,8 @@ Bytes from @var{data} to @var{data} + @var{len} have been written
> to the current inferior at @var{addr}.
> @end deftypefun
>
> - @deftypefun void test_notification (int @var{somearg})
> +@deftypefun void test_notification (int @var{somearg})
> This observer is used for internal testing. Do not use.
> See testsuite/gdb.gdb/observer.exp.
> - @end deftypefun
> +@end deftypefun
>
> diff --git a/gdb/inferior.c b/gdb/inferior.c
> index d27a3e3..783b8fc 100644
> --- a/gdb/inferior.c
> +++ b/gdb/inferior.c
> @@ -126,6 +126,8 @@ add_inferior_silent (int pid)
>
> inferior_alloc_data (inf);
>
> + observer_notify_inferior_added (inf);
> +
> if (pid != 0)
> inferior_appeared (inf, pid);
>
> @@ -187,7 +189,7 @@ delete_threads_of_inferior (int pid)
> /* If SILENT then be quiet -- don't announce a inferior death, or the
> exit of its threads. */
>
> -static void
> +void
> delete_inferior_1 (struct inferior *todel, int silent)
> {
> struct inferior *inf, *infprev;
> @@ -212,6 +214,8 @@ delete_inferior_1 (struct inferior *todel, int silent)
> else
> inferior_list = inf->next;
>
> + observer_notify_inferior_removed (inf);
> +
> free_inferior (inf);
> }
>
> @@ -258,7 +262,7 @@ exit_inferior_1 (struct inferior *inftoex, int silent)
>
> /* Notify the observers before removing the inferior from the list,
> so that the observers have a chance to look it up. */
> - observer_notify_inferior_exit (inf->pid);
> + observer_notify_inferior_exit (inf);
>
> inf->pid = 0;
> if (inf->vfork_parent != NULL)
> @@ -308,7 +312,7 @@ inferior_appeared (struct inferior *inf, int pid)
> {
> inf->pid = pid;
>
> - observer_notify_inferior_appeared (pid);
> + observer_notify_inferior_appeared (inf);
> }
>
> void
> @@ -731,6 +735,24 @@ remove_inferior_command (char *args, int from_tty)
> delete_inferior_1 (inf, 1);
> }
>
> +struct inferior *
> +add_inferior_with_spaces (void)
> +{
> + struct address_space *aspace;
> + struct program_space *pspace;
> + struct inferior *inf;
> +
> + /* If all inferiors share an address space on this system, this
> + doesn't really return a new address space; otherwise, it
> + really does. */
> + aspace = maybe_new_address_space ();
> + pspace = add_program_space (aspace);
> + inf = add_inferior (0);
> + inf->pspace = pspace;
> + inf->aspace = pspace->aspace;
> +
> + return inf;
> +}
>
> /* add-inferior [-copies N] [-exec FILENAME] */
>
> @@ -775,18 +797,7 @@ add_inferior_command (char *args, int from_tty)
>
> for (i = 0; i < copies; ++i)
> {
> - struct address_space *aspace;
> - struct program_space *pspace;
> - struct inferior *inf;
> -
> - /* If all inferiors share an address space on this system, this
> - doesn't really return a new address space; otherwise, it
> - really does. */
> - aspace = maybe_new_address_space ();
> - pspace = add_program_space (aspace);
> - inf = add_inferior (0);
> - inf->pspace = pspace;
> - inf->aspace = pspace->aspace;
> + struct inferior *inf = add_inferior_with_spaces ();
>
> printf_filtered (_("Added inferior %d\n"), inf->num);
>
> @@ -794,7 +805,7 @@ add_inferior_command (char *args, int from_tty)
> {
> /* Switch over temporarily, while reading executable and
> symbols.q */
> - set_current_program_space (pspace);
> + set_current_program_space (inf->pspace);
> set_current_inferior (inf);
> switch_to_thread (null_ptid);
>
> diff --git a/gdb/inferior.h b/gdb/inferior.h
> index 048fc11..cc5e571 100644
> --- a/gdb/inferior.h
> +++ b/gdb/inferior.h
> @@ -520,6 +520,8 @@ extern struct inferior *add_inferior_silent (int pid);
> /* Delete an existing inferior list entry, due to inferior exit. */
> extern void delete_inferior (int pid);
>
> +extern void delete_inferior_1 (struct inferior *todel, int silent);
> +
> /* Same as delete_inferior, but don't print new inferior notifications
> to the CLI. */
> extern void delete_inferior_silent (int pid);
> @@ -606,4 +608,6 @@ extern void prune_inferiors (void);
>
> extern int number_of_inferiors (void);
>
> +extern struct inferior *add_inferior_with_spaces (void);
> +
> #endif /* !defined (INFERIOR_H) */
> diff --git a/gdb/inflow.c b/gdb/inflow.c
> index 599fc69..bb4ca18 100644
> --- a/gdb/inflow.c
> +++ b/gdb/inflow.c
> @@ -504,9 +504,8 @@ get_inflow_inferior_data (struct inferior *inf)
> list. */
>
> static void
> -inflow_inferior_exit (int pid)
> +inflow_inferior_exit (struct inferior *inf)
> {
> - struct inferior *inf = find_inferior_pid (pid);
> struct terminal_info *info;
>
> info = inferior_data (inf, inflow_inferior_data);
> diff --git a/gdb/jit.c b/gdb/jit.c
> index 433746a..e17f0ab 100644
> --- a/gdb/jit.c
> +++ b/gdb/jit.c
> @@ -397,7 +397,7 @@ jit_inferior_created_observer (struct target_ops *objfile, int from_tty)
> for example when it crashes. */
>
> static void
> -jit_inferior_exit_hook (int pid)
> +jit_inferior_exit_hook (struct inferior *inf)
> {
> struct objfile *objf;
> struct objfile *temp;
> diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
> index 729cc5f..9c4ba1f 100644
> --- a/gdb/mi/mi-cmds.c
> +++ b/gdb/mi/mi-cmds.c
> @@ -33,6 +33,7 @@ static void build_table (struct mi_cmd *commands);
>
> struct mi_cmd mi_cmds[] =
> {
> + { "add-inferior", { NULL, 0}, mi_cmd_add_inferior },
^ EMISSINGSPACE
> { "break-after", { "ignore", 1 }, NULL },
> { "break-condition", { "cond", 1 }, NULL },
> { "break-commands", { NULL, 0 }, mi_cmd_break_commands },
> @@ -84,6 +85,7 @@ struct mi_cmd mi_cmds[] =
> { "list-features", { NULL, 0 }, mi_cmd_list_features},
> { "list-target-features", { NULL, 0 }, mi_cmd_list_target_features},
> { "list-thread-groups", { NULL, 0 }, mi_cmd_list_thread_groups },
> + { "remove-inferior", { NULL, 0 }, mi_cmd_remove_inferior },
> { "stack-info-depth", { NULL, 0 }, mi_cmd_stack_info_depth},
> { "stack-info-frame", { NULL, 0 }, mi_cmd_stack_info_frame},
> { "stack-list-arguments", { NULL, 0 }, mi_cmd_stack_list_args},
> diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
> index 7e1c819..d840104 100644
> --- a/gdb/mi/mi-cmds.h
> +++ b/gdb/mi/mi-cmds.h
> @@ -36,6 +36,7 @@ extern const char mi_all_values[];
> typedef void (mi_cmd_argv_ftype) (char *command, char **argv, int argc);
>
> /* Function implementing each command */
> +extern mi_cmd_argv_ftype mi_cmd_add_inferior;
> extern mi_cmd_argv_ftype mi_cmd_break_insert;
> extern mi_cmd_argv_ftype mi_cmd_break_commands;
> extern mi_cmd_argv_ftype mi_cmd_break_watch;
> @@ -72,6 +73,7 @@ extern mi_cmd_argv_ftype mi_cmd_interpreter_exec;
> extern mi_cmd_argv_ftype mi_cmd_list_features;
> extern mi_cmd_argv_ftype mi_cmd_list_target_features;
> extern mi_cmd_argv_ftype mi_cmd_list_thread_groups;
> +extern mi_cmd_argv_ftype mi_cmd_remove_inferior;
> extern mi_cmd_argv_ftype mi_cmd_stack_info_depth;
> extern mi_cmd_argv_ftype mi_cmd_stack_info_frame;
> extern mi_cmd_argv_ftype mi_cmd_stack_list_args;
> diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
> index 41388bb..04c4124 100644
> --- a/gdb/mi/mi-interp.c
> +++ b/gdb/mi/mi-interp.c
> @@ -56,13 +56,17 @@ static void mi_on_normal_stop (struct bpstats *bs, int print_frame);
>
> static void mi_new_thread (struct thread_info *t);
> static void mi_thread_exit (struct thread_info *t, int silent);
> -static void mi_inferior_appeared (int pid);
> -static void mi_inferior_exit (int pid);
> +static void mi_inferior_added (struct inferior *inf);
> +static void mi_inferior_appeared (struct inferior *inf);
> +static void mi_inferior_exit (struct inferior *inf);
> +static void mi_inferior_removed (struct inferior *inf);
> static void mi_on_resume (ptid_t ptid);
> static void mi_solib_loaded (struct so_list *solib);
> static void mi_solib_unloaded (struct so_list *solib);
> static void mi_about_to_proceed (void);
>
> +static int report_initial_inferior (struct inferior *inf, void *closure);
> +
> static void *
> mi_interpreter_init (int top_level)
> {
> @@ -86,13 +90,20 @@ mi_interpreter_init (int top_level)
> {
> observer_attach_new_thread (mi_new_thread);
> observer_attach_thread_exit (mi_thread_exit);
> + observer_attach_inferior_added (mi_inferior_added);
> observer_attach_inferior_appeared (mi_inferior_appeared);
> observer_attach_inferior_exit (mi_inferior_exit);
> + observer_attach_inferior_removed (mi_inferior_removed);
> observer_attach_normal_stop (mi_on_normal_stop);
> observer_attach_target_resumed (mi_on_resume);
> observer_attach_solib_loaded (mi_solib_loaded);
> observer_attach_solib_unloaded (mi_solib_unloaded);
> observer_attach_about_to_proceed (mi_about_to_proceed);
> +
> + /* The initial inferior is created before this function is called, so we
> + need to report it explicitly. Use iteration in case future version
> + of GDB creates more than one inferior up-front. */
> + iterate_over_inferiors (report_initial_inferior, mi);
> }
>
> return mi;
> @@ -285,10 +296,13 @@ static void
> mi_new_thread (struct thread_info *t)
> {
> struct mi_interp *mi = top_level_interpreter_data ();
> + struct inferior *inf = find_inferior_pid (t->ptid.pid);
Use ptid_get_pid.
> +
> + gdb_assert (inf);
>
> fprintf_unfiltered (mi->event_channel,
> - "thread-created,id=\"%d\",group-id=\"%d\"",
> - t->num, t->ptid.pid);
> + "thread-created,id=\"%d\",group-id=\"i%d\"",
> + t->num, inf->num);
> gdb_flush (mi->event_channel);
> }
>
> @@ -296,38 +310,64 @@ static void
> mi_thread_exit (struct thread_info *t, int silent)
> {
> struct mi_interp *mi;
> + struct inferior *inf;
>
> if (silent)
> return;
>
> + inf = find_inferior_pid (t->ptid.pid);
Ditto.
> +
> mi = top_level_interpreter_data ();
> target_terminal_ours ();
> fprintf_unfiltered (mi->event_channel,
> - "thread-exited,id=\"%d\",group-id=\"%d\"",
> - t->num,t->ptid.pid);
> + "thread-exited,id=\"%d\",group-id=\"i%d\"",
> + t->num, inf->num);
> gdb_flush (mi->event_channel);
> }
>
> void
> -mi_inferior_appeared (int pid)
> +mi_inferior_added (struct inferior *inf)
> {
> struct mi_interp *mi = top_level_interpreter_data ();
> target_terminal_ours ();
> - fprintf_unfiltered (mi->event_channel, "thread-group-created,id=\"%d\"",
> - pid);
> + fprintf_unfiltered (mi->event_channel,
> + "thread-group-added,id=\"i%d\"",
> + inf->num);
> + gdb_flush (mi->event_channel);
> +}
> +
> +void
> +mi_inferior_appeared (struct inferior *inf)
Nit: could you make these explicitly "static", although there's
a declaration above that makes them static already?
> +{
> + struct mi_interp *mi = top_level_interpreter_data ();
> + target_terminal_ours ();
> + fprintf_unfiltered (mi->event_channel,
> + "thread-group-started,id=\"i%d\",pid=\"%d\"",
> + inf->num, inf->pid);
> gdb_flush (mi->event_channel);
> }
>
> static void
> -mi_inferior_exit (int pid)
> +mi_inferior_exit (struct inferior *inf)
> {
> struct mi_interp *mi = top_level_interpreter_data ();
> target_terminal_ours ();
> - fprintf_unfiltered (mi->event_channel, "thread-group-exited,id=\"%d\"",
> - pid);
> + fprintf_unfiltered (mi->event_channel, "thread-group-exited,id=\"i%d\"",
> + inf->num);
> gdb_flush (mi->event_channel);
> }
>
> +void
> +mi_inferior_removed (struct inferior *inf)
> +{
> + struct mi_interp *mi = top_level_interpreter_data ();
> + target_terminal_ours ();
> + fprintf_unfiltered (mi->event_channel,
> + "thread-group-removed,id=\"i%d\"",
> + inf->num);
> + gdb_flush (mi->event_channel);
> +}
> +
> static void
> mi_on_normal_stop (struct bpstats *bs, int print_frame)
> {
> @@ -489,10 +529,18 @@ mi_solib_loaded (struct so_list *solib)
> {
> struct mi_interp *mi = top_level_interpreter_data ();
> target_terminal_ours ();
> - fprintf_unfiltered (mi->event_channel,
> - "library-loaded,id=\"%s\",target-name=\"%s\",host-name=\"%s\",symbols-loaded=\"%d\"",
> - solib->so_original_name, solib->so_original_name,
> - solib->so_name, solib->symbols_loaded);
> + if (gdbarch_has_global_solist (target_gdbarch))
> + fprintf_unfiltered (mi->event_channel,
> + "library-loaded,id=\"%s\",target-name=\"%s\",host-name=\"%s\",symbols-loaded=\"%d\"",
> + solib->so_original_name, solib->so_original_name,
> + solib->so_name, solib->symbols_loaded);
> + else
> + fprintf_unfiltered (mi->event_channel,
> + "library-loaded,id=\"%s\",target-name=\"%s\",host-name=\"%s\",symbols-loaded=\"%d\",thread-group=\"i%d\"",
In C, this:
"foo"
"bar"
is a single string "foobar". So you could split these long lines like, e.g.:
"library-loaded,id=\"%s\",target-name=\"%s\","
"host-name=\"%s\",symbols-loaded=\"%d\","
"thread-group=\"i%d\"",
> + solib->so_original_name, solib->so_original_name,
> + solib->so_name, solib->symbols_loaded,
> + current_inferior ()->num);
> +
> gdb_flush (mi->event_channel);
> }
>
> @@ -501,13 +549,35 @@ mi_solib_unloaded (struct so_list *solib)
> {
> struct mi_interp *mi = top_level_interpreter_data ();
> target_terminal_ours ();
> - fprintf_unfiltered (mi->event_channel,
> - "library-unloaded,id=\"%s\",target-name=\"%s\",host-name=\"%s\"",
> - solib->so_original_name, solib->so_original_name,
> - solib->so_name);
> + if (gdbarch_has_global_solist (target_gdbarch))
> + fprintf_unfiltered (mi->event_channel,
> + "library-unloaded,id=\"%s\",target-name=\"%s\",host-name=\"%s\"",
> + solib->so_original_name, solib->so_original_name,
> + solib->so_name);
> + else
> + fprintf_unfiltered (mi->event_channel,
> + "library-unloaded,id=\"%s\",target-name=\"%s\",host-name=\"%s\",thread-group=\"i%d\"",
> + solib->so_original_name, solib->so_original_name,
> + solib->so_name, current_inferior ()->num);
> +
> gdb_flush (mi->event_channel);
> }
>
> +static int
> +report_initial_inferior (struct inferior *inf, void *closure)
> +{
> + /* This function is called from mi_intepreter_init, and since
> + mi_inferior_added assumes that inferior is fully initialized
> + and top_level_interpreter_data is set, we cannot call
> + it here. */
> + struct mi_interp *mi = closure;
> + target_terminal_ours ();
> + fprintf_unfiltered (mi->event_channel,
> + "thread-group-added,id=\"i%d\"",
> + inf->num);
> + gdb_flush (mi->event_channel);
> + return 0;
> +}
>
> extern initialize_file_ftype _initialize_mi_interp; /* -Wmissing-prototypes */
>
> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> index aa1ffaf..0fad5cf 100644
> --- a/gdb/mi/mi-main.c
> +++ b/gdb/mi/mi-main.c
> @@ -78,6 +78,11 @@ static struct mi_timestamp *current_command_ts;
> static int do_timings = 0;
>
> char *current_token;
> +/* Few commands would like to know if options like --thread-group
> + were explicitly specified. This variable keeps the current
> + parsed command including all option, and make it possible. */
> +struct mi_parse *current_context;
This could be static? A global named `current_context' (a very
generic name), is sort of asking for trouble if someone else
adds another `current_context' global somewhere else; the linker
may consider them the same variable, and it could go unnoticed.
Defensively, globals like these should be either
prefixed (e.g., mi_foo), or made static and exported through
a function.
> +
> int running_result_record_printed = 1;
>
> /* Flag indicating that the target has proceeded since the last
> @@ -177,54 +182,83 @@ mi_cmd_exec_jump (char *args, char **argv, int argc)
> /* FIXME: Should call a libgdb function, not a cli wrapper. */
> return mi_execute_async_cli_command ("jump", argv, argc);
> }
> -
> -static int
> -proceed_thread_callback (struct thread_info *thread, void *arg)
> -{
> - int pid = *(int *)arg;
>
> +
> +static void
> +proceed_thread_callback (struct thread_info *thread, int pid)
Pedantically, this one is no longer a callback. You could rename
this one, and name the new function `proceed_thread_callback', instead
of `proceed_thread_callback_wrapper'.
> +{
> if (!is_stopped (thread->ptid))
> - return 0;
> + return;
>
> - if (PIDGET (thread->ptid) != pid)
> - return 0;
> + if (pid != 0 && PIDGET (thread->ptid) != pid)
> + return;
>
> switch_to_thread (thread->ptid);
> clear_proceed_status ();
> proceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 0);
> +}
> +
> +
> +static int
> +proceed_thread_callback_wrapper (struct thread_info *thread, void *arg)
> +{
> + int pid = *(int *)arg;
> + proceed_thread_callback (thread, pid);
> return 0;
> }
>
> void
> mi_cmd_exec_continue (char *command, char **argv, int argc)
> {
> - if (argc == 0)
> - continue_1 (0);
> - else if (argc == 1 && strcmp (argv[0], "--all") == 0)
> - continue_1 (1);
> - else if (argc == 2 && strcmp (argv[0], "--thread-group") == 0)
> + if (non_stop)
> {
> - struct cleanup *old_chain;
> - int pid;
> - if (argv[1] == NULL || argv[1] == '\0')
> - error ("Thread group id not specified");
> - pid = atoi (argv[1]);
> - if (!in_inferior_list (pid))
> - error ("Invalid thread group id '%s'", argv[1]);
> + /* In non-stop mode, 'resume' always resumes a single thread. Therefore,
> + to resume all threads of the current inferior, or all threads in all
> + inferiors, we need to iterate over threads.
> +
> + See comment on infcmd.c:proceed_thread_callback for rationale. */
> + if (current_context->all || current_context->thread_group != -1)
> + {
> + int pid = 0;
> + struct cleanup *back_to = make_cleanup_restore_current_thread ();
>
> - old_chain = make_cleanup_restore_current_thread ();
> - iterate_over_threads (proceed_thread_callback, &pid);
> - do_cleanups (old_chain);
> + if (!current_context->all)
> + {
> + struct inferior *inf = find_inferior_id (current_context->thread_group);
> + pid = inf->pid;
> + }
> + iterate_over_threads (proceed_thread_callback_wrapper, &pid);
> + do_cleanups (back_to);
> + }
> + else
> + {
> + continue_1 (0);
> + }
> }
> else
> - error ("Usage: -exec-continue [--all|--thread-group id]");
> + {
> + struct cleanup *back_to = make_cleanup_restore_integer (&sched_multi);
> + if (current_context->all)
> + {
> + sched_multi = 1;
> + continue_1 (0);
> + }
> + else
> + {
> + /* In all-stop mode, -exec-continue traditionally resumed either
> + all threads, or one thread, depending on the 'scheduler-locking'
> + variable. Let's continue to do the same. */
> + continue_1 (1);
> + }
> + do_cleanups (back_to);
> + }
> }
>
> static int
> interrupt_thread_callback (struct thread_info *thread, void *arg)
> {
> int pid = *(int *)arg;
> -
> +
> if (!is_running (thread->ptid))
> return 0;
>
> @@ -243,36 +277,31 @@ interrupt_thread_callback (struct thread_info *thread, void *arg)
> void
> mi_cmd_exec_interrupt (char *command, char **argv, int argc)
> {
> - if (argc == 0)
> + /* In all-stop mode, everything stops, so we don't need to try
> + anything specific. */
> + if (!non_stop)
> {
> - if (!is_running (inferior_ptid))
> - error ("Current thread is not running.");
> -
> interrupt_target_1 (0);
> + return;
> }
> - else if (argc == 1 && strcmp (argv[0], "--all") == 0)
> +
> + if (current_context->all)
> {
> - if (!any_running ())
> - error ("Inferior not running.");
> -
> + /* This will interrupt all threads in all inferiors. */
> interrupt_target_1 (1);
> }
> - else if (argc == 2 && strcmp (argv[0], "--thread-group") == 0)
> + else if (current_context->thread_group != -1)
> {
> - struct cleanup *old_chain;
> - int pid;
> - if (argv[1] == NULL || argv[1] == '\0')
> - error ("Thread group id not specified");
> - pid = atoi (argv[1]);
> - if (!in_inferior_list (pid))
> - error ("Invalid thread group id '%s'", argv[1]);
> -
> - old_chain = make_cleanup_restore_current_thread ();
> - iterate_over_threads (interrupt_thread_callback, &pid);
> - do_cleanups (old_chain);
> + struct inferior *inf = find_inferior_id (current_context->thread_group);
> + iterate_over_threads (interrupt_thread_callback, &(inf->pid));
Redundant ()'s.
> }
> else
> - error ("Usage: -exec-interrupt [--all|--thread-group id]");
> + {
> + /* Interrupt just the current thread -- either explicitly
> + specified via --thread or whatever was current before
> + MI command was sent. */
> + interrupt_target_1 (0);
> + }
> }
>
> /* Given MI command arguments, recompose then back into a single string
> @@ -349,6 +378,34 @@ mi_cmd_exec_until (char *command, char **argv, int argc)
> do_cleanups (back_to);
> }
>
> +static int run_one_inferior (struct inferior *inf, void *arg)
Function name on column 1, please.
> +{
> + struct thread_info *tp = 0;
> +
> + if (inf->pid != 0)
> + {
> + if (inf->pid != ptid_get_pid (inferior_ptid))
> + {
> + struct thread_info *tp;
> +
> + tp = any_thread_of_process (inf->pid);
> + if (!tp)
> + error (_("Inferior has no threads."));
> +
> + switch_to_thread (tp->ptid);
> + }
> + }
> + else
> + {
> + set_current_inferior (inf);
> + switch_to_thread (null_ptid);
> + set_current_program_space (inf->pspace);
> + }
> + mi_execute_cli_command ("run", target_can_async_p (),
> + target_can_async_p () ? "&" : NULL);
> + return 0;
> +}
> +
> void
> mi_cmd_exec_run (char *command, char **argv, int argc)
> {
> @@ -371,14 +428,26 @@ mi_cmd_exec_run (char *command, char **argv, int argc)
>
> if (argc == 0)
> {
> - mi_execute_cli_command ("run", target_can_async_p (),
> - target_can_async_p () ? "&" : NULL);
> + if (current_context->all)
> + {
> + struct cleanup *back_to = save_current_space_and_thread ();
> + iterate_over_inferiors (run_one_inferior, NULL);
> + do_cleanups (back_to);
> + }
> + else
> + {
> + mi_execute_cli_command ("run", target_can_async_p (),
> + target_can_async_p () ? "&" : NULL);
> + }
> }
> else
> {
> struct cleanup *back_to;
> char *r = recompose_args (argv, argc, target_can_async_p ());
> back_to = make_cleanup (xfree, r);
> +
> + if (current_context->all)
> + error (_("Could not use --all with explicit arguments"));
"Could not", or "Can not" ?
>
> mi_execute_cli_command ("run", 1, r);
>
> @@ -524,13 +593,23 @@ print_one_inferior (struct inferior *inferior, void *xdata)
> struct cleanup *back_to
> = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
>
> - ui_out_field_fmt (uiout, "id", "%d", inferior->pid);
> + ui_out_field_fmt (uiout, "id", "i%d", inferior->num);
> ui_out_field_string (uiout, "type", "process");
> - ui_out_field_int (uiout, "pid", inferior->pid);
> + if (inferior->pid != 0)
> + ui_out_field_int (uiout, "pid", inferior->pid);
> +
> + if (inferior->pspace->ebfd)
> + {
> + ui_out_field_string (uiout, "executable",
> + bfd_get_filename (inferior->pspace->ebfd));
> + }
>
> - data.pid = inferior->pid;
> data.cores = 0;
> - iterate_over_threads (collect_cores, &data);
> + if (inferior->pid != 0)
> + {
> + data.pid = inferior->pid;
> + iterate_over_threads (collect_cores, &data);
> + }
>
> if (!VEC_empty (int, data.cores))
> {
> @@ -1541,6 +1620,40 @@ mi_cmd_list_target_features (char *command, char **argv, int argc)
> error ("-list-target-features should be passed no arguments");
> }
>
> +void
> +mi_cmd_add_inferior (char *command, char **argv, int argc)
> +{
> + struct inferior *inf;
> +
> + if (argc != 0)
> + error ("-add-inferior shuld be passed not arguments");
Something's wrong with this sentence. Also, most MI errors
you've added have the string wrapped in _() for i18n, while these
do not.
> +
> + inf = add_inferior_with_spaces ();
> +
> + ui_out_field_fmt (uiout, "inferior", "i%d", inf->num);
> +}
> +
> +void
> +mi_cmd_remove_inferior (char *command, char **argv, int argc)
> +{
> + int id;
> + struct inferior *inf;
> +
> + if (argc != 1)
> + error ("-remove-iferior should be passed a single argument");
Typo: "inferior"
> +
> + if (sscanf (argv[1], "i%d", &id) != 1)
> + error ("the thread group id is syntactically invalid");
> +
> + inf = find_inferior_id (id);
> + if (!inf)
> + error ("the specified thread group does not exist");
> +
> + delete_inferior_1 (inf, 1 /* silent */);
> +}
> +
> +
> +
> /* Execute a command within a safe environment.
> Return <0 for error; >=0 for ok.
>
> @@ -1740,9 +1853,37 @@ mi_cmd_execute (struct mi_parse *parse)
>
> cleanup = make_cleanup (null_cleanup, NULL);
>
> + if (parse->all && parse->thread_group != -1)
> + error (_("Cannot specify --thread-group together with --all"));
> +
> + if (parse->all && parse->thread != -1)
> + error (_("Cannot specify --thread together with --all"));
> +
> + if (parse->thread_group != -1 && parse->thread != -1)
> + error (_("Cannot specify --thread together with --thread-group"));
> +
> if (parse->frame != -1 && parse->thread == -1)
> error (_("Cannot specify --frame without --thread"));
>
> + if (parse->thread_group != -1)
> + {
> + struct inferior *inf = find_inferior_id (parse->thread_group);
> + struct thread_info *tp = 0;
> +
> + if (!inf)
> + error (_("Invalid thread group for the --tread-group option"));
> +
> + set_current_inferior (inf);
> + /* This behaviour means that if --thread-group option identifies
> + an inferior with multiple threads, then a random one will be picked.
> + This is not a problem -- frontend should always provide --thread if
> + it wishes to operate on a specific thread. */
> + if (inf->pid != 0)
> + tp = any_thread_of_process (inf->pid);
> + switch_to_thread (tp ? tp->ptid : null_ptid);
> + set_current_program_space (inf->pspace);
> + }
> +
> if (parse->thread != -1)
> {
> struct thread_info *tp = find_thread_id (parse->thread);
> @@ -1767,6 +1908,8 @@ mi_cmd_execute (struct mi_parse *parse)
> error (_("Invalid frame id: %d"), frame);
> }
>
> + current_context = parse;
> +
Hmm, aren't the `struct mi_parse' objects leaking
for every MI command? I can't see where they're released in
mi_execute_command ?
> if (parse->cmd->argv_func != NULL)
> parse->cmd->argv_func (parse->command, parse->argv, parse->argc);
> else if (parse->cmd->cli.cmd != 0)
> diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
> index 4ff70ef..ec955aa 100644
> --- a/gdb/mi/mi-parse.c
> +++ b/gdb/mi/mi-parse.c
> @@ -151,6 +151,8 @@ mi_parse (char *cmd)
> char *chp;
> struct mi_parse *parse = XMALLOC (struct mi_parse);
> memset (parse, 0, sizeof (*parse));
> + parse->all = 0;
> + parse->thread_group = -1;
> parse->thread = -1;
> parse->frame = -1;
>
> @@ -210,8 +212,31 @@ mi_parse (char *cmd)
> for (;;)
> {
> char *start = chp;
> + size_t as = sizeof ("--all ") - 1;
> + size_t tgs = sizeof ("--thread-group ") - 1;
> size_t ts = sizeof ("--thread ") - 1;
> size_t fs = sizeof ("--frame ") - 1;
> + if (strncmp (chp, "--all ", as) == 0)
> + {
> + parse->all = 1;
> + chp += as;
> + }
> + /* See if this --all as the last token in the input. */
> + if (strncmp (chp, "--all", as) == 0)
> + {
> + parse->all = 1;
> + chp += (as - 1);
> + }
In the latter strncmp above:
+ if (strncmp (chp, "--all", as) == 0)
AS is always larger than strlen("--all"), so the
strncmp's check on AS is useless and confusing here.
I think you wanted:
if (strcmp (chp, "--all") == 0)
{
parse->all = 1;
chp += strlen (chp);
}
> + if (strncmp (chp, "--thread-group ", tgs) == 0)
> + {
> + if (parse->thread_group != -1)
> + error ("Duplicate '--thread-group' option");
> + chp += tgs;
> + if (*chp != 'i')
> + error ("Invalid thread group id");
> + chp += 1;
> + parse->thread_group = strtol (chp, &chp, 10);
> + }
More of the `to i18n or not to i18n' issue.
> if (strncmp (chp, "--thread ", ts) == 0)
> {
> if (parse->thread != -1)
> diff --git a/gdb/mi/mi-parse.h b/gdb/mi/mi-parse.h
> index a63ee8e..3c6cd9a 100644
> --- a/gdb/mi/mi-parse.h
> +++ b/gdb/mi/mi-parse.h
> @@ -46,6 +46,8 @@ struct mi_parse
> char *args;
> char **argv;
> int argc;
> + int all;
> + int thread_group; /* At present, the same as inferior number. */
> int thread;
> int frame;
> };
> diff --git a/gdb/testsuite/gdb.mi/mi-nonstop.exp b/gdb/testsuite/gdb.mi/mi-nonstop.exp
> index 605f48b..397dec5 100644
> --- a/gdb/testsuite/gdb.mi/mi-nonstop.exp
> +++ b/gdb/testsuite/gdb.mi/mi-nonstop.exp
> @@ -151,7 +151,7 @@ if { [is_remote target] } {
> unsupported $test
> } else {
> gdb_expect {
> - -re ".*=thread-exited,id=\"2\",group-id=\"\[0-9\]+\"\r\n$" {
> + -re ".*=thread-exited,id=\"2\",group-id=\"i\[0-9\]+\"\r\n$" {
> pass $test
> }
> timeout {
Otherwise, looks good to me.
Do you know if current frontends had bad assumptions, and will
behave or misbehave with this change? E.g., CDI, DSF-GDB, kdevelop?
--
Pedro Alves
More information about the Gdb-patches
mailing list