[RFC,2/2] gdb/mi: add new async events =target-connected and =target-disconnected
Commit Message
Whenever a target is connected or disconnected, emit new asynchronous
event =target-connected and =target-disconnected. Events report
both short name and full name of connected or disconnected target.
In addition, =target-connected report a set of target features.
This allows frontends to keep track of current target and its features
regardless whether target is changed explicitly by MI -target-select
command, CLI target command or implicitly by native target auto-connect.
gdb/Changelog:
* mi/mi-interp.c (mi_target_connected): New function.
(mi_target_disconnected): New function.
(_initialize_mi_interp): Register new observers.
gdb/doc/Changelog
* gdb.texinfo (GDB/MI Async Records): Document new async
records =target-connected and target-disconnected.
---
gdb/ChangeLog | 12 +++++--
gdb/doc/ChangeLog | 5 +++
gdb/doc/gdb.texinfo | 7 +++++
gdb/mi/mi-interp.c | 77 +++++++++++++++++++++++++++++++++++++++++++--
4 files changed, 96 insertions(+), 5 deletions(-)
Comments
> From: Jan Vrany <jan.vrany@fit.cvut.cz>
> Cc: Jan Vrany <jan.vrany@fit.cvut.cz>
> Date: Sun, 14 Oct 2018 14:55:34 +0200
>
> +@item =target-connected,type="@var{type}",name="@var{name}",features=[@var{features},...]
> +@itemx =target-disconnected,type="@var{type}",name="@var{name}"
> +Report that a target has been connected or disconnected. The @var{type} is
> +the internal target type, the @var{name} is a name of the target to be
> +used for printing. The @var{features} lists connected target's features.
^^^^^
"list", in plural.
Also, please leave 2 spaces between sentences, per our conventions.
The documentation part is approved with these n its fixed.
Thanks.
On 2018-10-14 8:55 a.m., Jan Vrany wrote:
> Whenever a target is connected or disconnected, emit new asynchronous
> event =target-connected and =target-disconnected. Events report
> both short name and full name of connected or disconnected target.
> In addition, =target-connected report a set of target features.
>
> This allows frontends to keep track of current target and its features
> regardless whether target is changed explicitly by MI -target-select
> command, CLI target command or implicitly by native target auto-connect.
Thanks, I like the idea. A non-RFC version of this would require corresponding
tests to be accepted.
> @@ -1271,6 +1275,73 @@ mi_user_selected_context_changed (user_selected_what selection)
> }
> }
>
> +static void
> +mi_target_connected (struct target_ops *target)
> +{
> + SWITCH_THRU_ALL_UIS ()
> + {
> + struct mi_interp *mi = as_mi_interp (top_level_interpreter ());
> + struct ui_out *mi_uiout;
> +
> + if (mi == NULL)
> + continue;
> +
> + mi_uiout = top_level_interpreter ()->interp_ui_out ();
> +
> + target_terminal::scoped_restore_terminal_state term_state;
> + target_terminal::ours_for_output ();
> +
> + fprintf_unfiltered (mi->event_channel,"target-connected");
> +
> + mi_uiout->redirect (mi->event_channel);
> +
> + mi_uiout->field_string ("type", target->shortname());
> + mi_uiout->field_string ("name", target->longname());
> +
> + {
> + ui_out_emit_list list_emitter (mi_uiout, "features");
> +
> + if (mi_async_p ())
> + mi_uiout->field_string (NULL, "async");
> + if (target_can_execute_reverse)
> + mi_uiout->field_string (NULL, "reverse");
> + }
> +
> + mi_uiout->redirect (NULL);
> +
> + gdb_flush (mi->event_channel);
> + }
> +}
I think there is a (kind of corner-case, but still) bug with using
mi_async_p and target_can_execute_reverse. Here are some CLI commands
I type in a "gdb -i mi", and the corresponding
=target-connected/disconnected event:
set mi-async on
file test
=target-connected,type="exec",name="Local exec file",features=[]
start
=target-connected,type="native",name="Native process",features=["async"]
record
=target-connected,type="record-full",name="Process record and replay target",features=["async","reverse"]
file
=target-disonnected,type="exec",name="Local exec file"
file /bin/ls
=target-connected,type="exec",name="Local exec file",features=["async","reverse"]
That last event says the exec target supports async and reverse, which is wrong.
So you would need to write an equivalent of mi_async_p/target_can_execute_reverse
to which you can pass a target_ops*, it should not be too hard.
Simon
Hi,
sorry for late answer, I was traveling last couple weeks.
On Thu, 2018-10-18 at 01:31 +0000, Simon Marchi wrote:
> On 2018-10-14 8:55 a.m., Jan Vrany wrote:
> > Whenever a target is connected or disconnected, emit new asynchronous
> > event =target-connected and =target-disconnected. Events report
> > both short name and full name of connected or disconnected target.
> > In addition, =target-connected report a set of target features.
> >
> > This allows frontends to keep track of current target and its features
> > regardless whether target is changed explicitly by MI -target-select
> > command, CLI target command or implicitly by native target auto-connect.
>
> Thanks, I like the idea. A non-RFC version of this would require corresponding
> tests to be accepted.
I'll add some.
>
> > @@ -1271,6 +1275,73 @@ mi_user_selected_context_changed (user_selected_what selection)
> > }
> > }
> >
> > +static void
> > +mi_target_connected (struct target_ops *target)
> > +{
> > + SWITCH_THRU_ALL_UIS ()
> > + {
> > + struct mi_interp *mi = as_mi_interp (top_level_interpreter ());
> > + struct ui_out *mi_uiout;
> > +
> > + if (mi == NULL)
> > + continue;
> > +
> > + mi_uiout = top_level_interpreter ()->interp_ui_out ();
> > +
> > + target_terminal::scoped_restore_terminal_state term_state;
> > + target_terminal::ours_for_output ();
> > +
> > + fprintf_unfiltered (mi->event_channel,"target-connected");
> > +
> > + mi_uiout->redirect (mi->event_channel);
> > +
> > + mi_uiout->field_string ("type", target->shortname());
> > + mi_uiout->field_string ("name", target->longname());
> > +
> > + {
> > + ui_out_emit_list list_emitter (mi_uiout, "features");
> > +
> > + if (mi_async_p ())
> > + mi_uiout->field_string (NULL, "async");
> > + if (target_can_execute_reverse)
> > + mi_uiout->field_string (NULL, "reverse");
> > + }
> > +
> > + mi_uiout->redirect (NULL);
> > +
> > + gdb_flush (mi->event_channel);
> > + }
> > +}
>
> I think there is a (kind of corner-case, but still) bug with using
> mi_async_p and target_can_execute_reverse. Here are some CLI commands
> I type in a "gdb -i mi", and the corresponding
> =target-connected/disconnected event:
>
> set mi-async on
> file test
> =target-connected,type="exec",name="Local exec file",features=[]
>
> start
> =target-connected,type="native",name="Native process",features=["async"]
>
> record
> =target-connected,type="record-full",name="Process record and replay target",features=["async","reverse"]
>
> file
> =target-disonnected,type="exec",name="Local exec file"
>
> file /bin/ls
> =target-connected,type="exec",name="Local exec file",features=["async","reverse"]
>
> That last event says the exec target supports async and reverse, which is wrong.
> So you would need to write an equivalent of mi_async_p/target_can_execute_reverse
> to which you can pass a target_ops*, it should not be too hard.
Yes, you're right, will fix that.
But this example shows a deeper problem - it is hard to interpret these events.
Perhaps I'm wrong but: from the user point of view, GDB can be connected to only
one target, no? But if you look at events, it looks like targets
"native" and "record-full" are still connected. They are, really, the inferiror
is still running, can be single-stepped and so on.
So actually I'd expect something like:
file test
=target-connected,type="exec",name="Local exec file",features=[]
start
=target-disonnected,type="exec",name="Local exec file"Simon=target-connected,type="native",name="Native process",features=["async"]
record
=target-disconnected,type="native",name="Native process"
=target-connected,type="record-full",name="Process record and replay target",features=["async","reverse"]
file
# Nothing, we're still connected and still recording
file /bin/ls
# Nothing, we're still connected and still recording
record stop
=target-disconnected,type="record-full",name="Process record and replay target"
=target-connected,type="native",name="Native process",features=["async"]
kill
=target-disconnected,type="native",name="Native process"
=target-connected,type="exec",name="Local exec file",features=[]
Makes sense?
We may say that =target-connected means that previously connected target has disconnected.
I'd prefer not to, since you mentionedsome time ago that Pedro is working on multiple
target support (in that case all we need is to add a kind of "id" field to target
notification events)
In any case I find =target-connected after issuing "file" command rather confusing.
Makes sense?
Jan
@@ -1,4 +1,10 @@
-2018-07-03 Jan Vrany <jan.vrany@fit.cvut.cz>
+2018-10-13 Jan Vrany <jan.vrany@fit.cvut.cz>
+
+ * mi/mi-interp.c (mi_target_connected): New function.
+ (mi_target_disconnected): New function.
+ (_initialize_mi_interp): Register new observers.
+
+2018-10-13 Jan Vrany <jan.vrany@fit.cvut.cz>
* observable.h: Define new observables target_connected and
target_disconnected.
@@ -8021,7 +8027,7 @@
of the new expedite_regs parameter to init_target_desc.
2018-05-10 Omair Javaid <omair.javaid@linaro.org>
-
+
PR gdb/23127
* aarch64-linux-tdep.c (aarch64_linux_init_abi): Add call to
set_gdbarch_significant_addr_bit.
@@ -11228,7 +11234,7 @@
2018-04-02 Weimin Pan <weimin.pan@oracle.com>
PR gdb/16959
- * cp-valprint.c: (cp_print_static_field) Fix infinite recursion when
+ * cp-valprint.c: (cp_print_static_field) Fix infinite recursion when
printing static type.
2018-04-01 Tom Tromey <tom@tromey.com>
@@ -1,3 +1,8 @@
+2018-10-13 Jan Vrany <jan.vrany@fit.cvut.cz>
+
+ * gdb.texinfo (GDB/MI Async Records): Document new async
+ records =target-connected and target-disconnected.
+
2018-10-09 Tom Tromey <tom@tromey.com>
* python.texi (Inferiors In Python): Link to "Frames In Python",
@@ -27902,6 +27902,13 @@ written in an inferior. The @var{id} is the identifier of the
thread group corresponding to the affected inferior. The optional
@code{type="code"} part is reported if the memory written to holds
executable code.
+
+@item =target-connected,type="@var{type}",name="@var{name}",features=[@var{features},...]
+@itemx =target-disconnected,type="@var{type}",name="@var{name}"
+Report that a target has been connected or disconnected. The @var{type} is
+the internal target type, the @var{name} is a name of the target to be
+used for printing. The @var{features} lists connected target's features.
+See @ref{GDB/MI Support Commands} for existing target features.
@end table
@node GDB/MI Breakpoint Information
@@ -38,6 +38,7 @@
#include "cli-out.h"
#include "thread-fsm.h"
#include "cli/cli-interp.h"
+#include "target.h"
/* These are the interpreter setup, etc. functions for the MI
interpreter. */
@@ -85,6 +86,9 @@ static void mi_command_param_changed (const char *param, const char *value);
static void mi_memory_changed (struct inferior *inf, CORE_ADDR memaddr,
ssize_t len, const bfd_byte *myaddr);
static void mi_on_sync_execution_done (void);
+static void mi_target_connected (struct target_ops *target);
+static void mi_target_disconnected (struct target_ops *target);
+
static int report_initial_inferior (struct inferior *inf, void *closure);
@@ -658,7 +662,7 @@ mi_on_normal_stop_1 (struct bpstats *bs, int print_frame)
if (core != -1)
mi_uiout->field_int ("core", core);
}
-
+
fputs_unfiltered ("*stopped", mi->raw_stdout);
mi_out_put (mi_uiout, mi->raw_stdout);
mi_out_rewind (mi_uiout);
@@ -1271,6 +1275,73 @@ mi_user_selected_context_changed (user_selected_what selection)
}
}
+static void
+mi_target_connected (struct target_ops *target)
+{
+ SWITCH_THRU_ALL_UIS ()
+ {
+ struct mi_interp *mi = as_mi_interp (top_level_interpreter ());
+ struct ui_out *mi_uiout;
+
+ if (mi == NULL)
+ continue;
+
+ mi_uiout = top_level_interpreter ()->interp_ui_out ();
+
+ target_terminal::scoped_restore_terminal_state term_state;
+ target_terminal::ours_for_output ();
+
+ fprintf_unfiltered (mi->event_channel,"target-connected");
+
+ mi_uiout->redirect (mi->event_channel);
+
+ mi_uiout->field_string ("type", target->shortname());
+ mi_uiout->field_string ("name", target->longname());
+
+ {
+ ui_out_emit_list list_emitter (mi_uiout, "features");
+
+ if (mi_async_p ())
+ mi_uiout->field_string (NULL, "async");
+ if (target_can_execute_reverse)
+ mi_uiout->field_string (NULL, "reverse");
+ }
+
+ mi_uiout->redirect (NULL);
+
+ gdb_flush (mi->event_channel);
+ }
+}
+
+static void
+mi_target_disconnected (struct target_ops *target)
+{
+ SWITCH_THRU_ALL_UIS ()
+ {
+ struct mi_interp *mi = as_mi_interp (top_level_interpreter ());
+ struct ui_out *mi_uiout;
+
+ if (mi == NULL)
+ continue;
+
+ mi_uiout = top_level_interpreter ()->interp_ui_out ();
+
+ target_terminal::scoped_restore_terminal_state term_state;
+ target_terminal::ours_for_output ();
+
+ fprintf_unfiltered (mi->event_channel, "target-disonnected");
+
+ mi_uiout->redirect (mi->event_channel);
+
+ mi_uiout->field_string ("type", target->shortname());
+ mi_uiout->field_string ("name", target->longname());
+
+ mi_uiout->redirect (NULL);
+
+ gdb_flush (mi->event_channel);
+ }
+}
+
static int
report_initial_inferior (struct inferior *inf, void *closure)
{
@@ -1319,7 +1390,7 @@ mi_interp::set_logging (ui_file_up logfile, bool logging_redirect)
mi->raw_stdout = mi->saved_raw_stdout;
mi->saved_raw_stdout = NULL;
}
-
+
mi->out->set_raw (mi->raw_stdout);
mi->err->set_raw (mi->raw_stdout);
mi->log->set_raw (mi->raw_stdout);
@@ -1373,4 +1444,6 @@ _initialize_mi_interp (void)
gdb::observers::sync_execution_done.attach (mi_on_sync_execution_done);
gdb::observers::user_selected_context_changed.attach
(mi_user_selected_context_changed);
+ gdb::observers::target_connected.attach (mi_target_connected);
+ gdb::observers::target_disconnected.attach (mi_target_disconnected);
}