How to use -list-target-features MI command

How to use -list-target-features MI command

[Jan Vrany]

Hi there, 

I need to know (in advance) whether the target supports async 
mode or not. 
I know that -list-target-features can be used for that, but
not sure when and how to use -list-target-features. 

The documentation says:

"Whenever a target can change, due to commands such as -target-select,
-target-attach or -exec-run, the list of target features may change,
and the frontend should obtain it again."

However, once I issue `-exec-run` using a target with no async mode
support, I will not get the result of -list-target-feature before
debugee stops. 

Another problem is that user may enter CLI command manually. In case,
how do I find out that target may have changed and I should check for
features? 

Could you give me any advice on how and when to correctly check for
target features? 

Thanks a lot! 

Jan

Re: How to use -list-target-features MI command

[Simon Marchi]

Hi Jan,

On 2018-04-09 10:22, Jan Vrany wrote:
> Hi there,
> 
> I need to know (in advance) whether the target supports async
> mode or not.
> I know that -list-target-features can be used for that, but
> not sure when and how to use -list-target-features.
> 
> The documentation says:
> 
> "Whenever a target can change, due to commands such as -target-select,
> -target-attach or -exec-run, the list of target features may change,
> and the frontend should obtain it again."
> 
> However, once I issue `-exec-run` using a target with no async mode
> support, I will not get the result of -list-target-feature before
> debugee stops.

I think that's kind of expected.  If the run target does not support 
mi-async, GDB can't parse and reply to MI commands while the target is 
executing.  GDB reads and replies to your -list-target-features only 
when the target stops.

One way to hack around it would be to use for example the new "starti" 
command, which breaks at (before?) the first instruction of the program, 
issue -list-target-features, and then continue.  There might be some 
better/cleaner solution, but that's what comes to mind right now.

> Another problem is that user may enter CLI command manually. In case,
> how do I find out that target may have changed and I should check for
> features?

I don't see a way right now that you could know about that.  There would 
need to be a new asynchronous event emitted when the target stack 
changes.

Simon

Re: How to use -list-target-features MI command

[Pedro Alves]

On 04/23/2018 02:28 AM, Simon Marchi wrote:
> On 2018-04-09 10:22, Jan Vrany wrote:
>> I need to know (in advance) whether the target supports async
>> mode or not.
>> I know that -list-target-features can be used for that, but
>> not sure when and how to use -list-target-features.
>>
>> The documentation says:
>>
>> "Whenever a target can change, due to commands such as -target-select,
>> -target-attach or -exec-run, the list of target features may change,
>> and the frontend should obtain it again."
>>
>> However, once I issue `-exec-run` using a target with no async mode
>> support, I will not get the result of -list-target-feature before
>> debugee stops.
> 
> I think that's kind of expected.  If the run target does not support mi-async, GDB can't parse and reply to MI commands while the target is executing.  GDB reads and replies to your -list-target-features only when the target stops.
> 
> One way to hack around it would be to use for example the new "starti" command, which breaks at (before?) the first instruction of the program, issue -list-target-features, and then continue.  There might be some better/cleaner solution, but that's what comes to mind right now.

IMO the clean solution is to connect to the native target
explicitly, with "-target-select native", before -exec-run,
instead of letting -exec-run automatically connect to
the native target if not connected to any other target yet.

 -gdb-set mi-async on
 ^done
 (gdb) 
 -target-select native
 ^connected
 (gdb) 
 -list-target-features
 ^done,features=["async"]
 (gdb) 
 -exec-run ...

I'd also suggest that frontends consider using 
  "set auto-connect-native-target off"
https://sourceware.org/gdb/onlinedocs/gdb/Starting.html

Thanks,
Pedro Alves

Re: How to use -list-target-features MI command

[Jan Vrany]

Hi,

On Mon, 2018-04-23 at 14:45 +0100, Pedro Alves wrote:
> 
> > One way to hack around it would be to use for example the new
> > "starti" command, which breaks at (before?) the first instruction
> > of the program, issue -list-target-features, and then continue. 
> > There might be some better/cleaner solution, but that's what comes
> > to mind right now.
> 
> IMO the clean solution is to connect to the native target
> explicitly, with "-target-select native", before -exec-run,
> instead of letting -exec-run automatically connect to
> the native target if not connected to any other target yet.
> 
>  -gdb-set mi-async on
>  ^done
>  (gdb) 
>  -target-select native
>  ^connected
>  (gdb) 
>  -list-target-features
>  ^done,features=["async"]
>  (gdb) 
>  -exec-run ...
> 
> I'd also suggest that frontends consider using 
>   "set auto-connect-native-target off"
> https://sourceware.org/gdb/onlinedocs/gdb/Starting.html
> 

Thanks! This makes sense. The only problem I see is that it only works
reliably if one does not use CLI. In my case, I want user to be able 
to use the CLI. 


> I don't see a way right now that you could know about that.  There
would
> need to be a new asynchronous event emitted when the target stack 
> changes.

So gave it a go and added a new event. Seems to work fine (for me):

diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 8bfb1298a5..f140a3cca2 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -22,6 +22,7 @@
 #include "event-top.h"
 #include "event-loop.h"
 #include "inferior.h"
+#include "target.h"
 #include "infrun.h"
 #include "ui-out.h"
 #include "top.h"
@@ -85,6 +86,7 @@ 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_current_target_changed (struct target_ops *target);
 
 static int report_initial_inferior (struct inferior *inf, void
*closure);
 
@@ -1280,6 +1282,38 @@ mi_user_selected_context_changed
(user_selected_what selection)
     }
 }
 
+static void 
+mi_current_target_changed (struct target_ops *target) 
+{
+  SWITCH_THRU_ALL_UIS ()
+    {
+      struct mi_interp *mi = as_mi_interp (top_level_interpreter ());
+
+      if (mi == NULL)
+	      continue;
+
+      target_terminal::scoped_restore_terminal_state term_state;
+      target_terminal::ours_for_output ();
+
+      fprintf_unfiltered (mi->event_channel,
+			  "target-changed,features=[");
+      if (mi_async_p ())
+        fprintf_unfiltered (mi->event_channel,
+          "\"async\"");
+      if (target_can_execute_reverse)
+        {
+          if (mi_async_p ()) 
+            fprintf_unfiltered (mi->event_channel,",\"reverse\"");
+          else
+            fprintf_unfiltered (mi->event_channel,"\"reverse\"");
+        }
+      fprintf_unfiltered (mi->event_channel,
+        "]");
+      gdb_flush (mi->event_channel);
+    }
+}	
+
+
 static int
 report_initial_inferior (struct inferior *inf, void *closure)
 {
@@ -1382,4 +1416,5 @@ _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::current_target_changed.attach
(mi_current_target_changed);
 }
diff --git a/gdb/observable.c b/gdb/observable.c
index 5539b9837b..f99a39b909 100644
--- a/gdb/observable.c
+++ b/gdb/observable.c
@@ -74,6 +74,7 @@ DEFINE_OBSERVABLE (inferior_call_pre);
 DEFINE_OBSERVABLE (inferior_call_post);
 DEFINE_OBSERVABLE (register_changed);
 DEFINE_OBSERVABLE (user_selected_context_changed);
+DEFINE_OBSERVABLE (current_target_changed);
 
 } /* namespace observers */
 } /* namespace gdb */
diff --git a/gdb/observable.h b/gdb/observable.h
index 34447b90bb..3f97cc5cb1 100644
--- a/gdb/observable.h
+++ b/gdb/observable.h
@@ -228,6 +228,9 @@ extern observable<struct frame_info *, int>
register_changed;
    frame has changed.  */
 extern observable<user_selected_what> user_selected_context_changed;
 
+/* The current target has changed. */
+extern observable<struct target_ops *> current_target_changed;
+
 } /* namespace observers */
 
 } /* namespace gdb */
diff --git a/gdb/target.c b/gdb/target.c
index e8d4ae7ea8..5cb182c849 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -49,6 +49,7 @@
 #include "byte-vector.h"
 #include "terminal.h"
 #include <algorithm>
+#include <observable.h>
 
 static void generic_tls_error (void) ATTRIBUTE_NORETURN;
 
@@ -719,6 +720,8 @@ update_current_target (void)
 
   if (targetdebug)
     setup_target_debug ();
+
+  gdb::observers::current_target_changed.notify(&current_target);
 }
 
 /* Push a new target type into the stack of the existing target
accessors,

Re: How to use -list-target-features MI command

[Simon Marchi]

On 2018-04-24 06:20, Jan Vrany wrote:
> Hi,
> 
> On Mon, 2018-04-23 at 14:45 +0100, Pedro Alves wrote:
>> 
>> > One way to hack around it would be to use for example the new
>> > "starti" command, which breaks at (before?) the first instruction
>> > of the program, issue -list-target-features, and then continue.
>> > There might be some better/cleaner solution, but that's what comes
>> > to mind right now.
>> 
>> IMO the clean solution is to connect to the native target
>> explicitly, with "-target-select native", before -exec-run,
>> instead of letting -exec-run automatically connect to
>> the native target if not connected to any other target yet.
>> 
>>  -gdb-set mi-async on
>>  ^done
>>  (gdb)
>>  -target-select native
>>  ^connected
>>  (gdb)
>>  -list-target-features
>>  ^done,features=["async"]
>>  (gdb)
>>  -exec-run ...
>> 
>> I'd also suggest that frontends consider using
>>   "set auto-connect-native-target off"
>> https://sourceware.org/gdb/onlinedocs/gdb/Starting.html
>> 
> 
> Thanks! This makes sense. The only problem I see is that it only works
> reliably if one does not use CLI. In my case, I want user to be able
> to use the CLI.
> 
> 
>> I don't see a way right now that you could know about that.  There
> would
>> need to be a new asynchronous event emitted when the target stack
>> changes.
> 
> So gave it a go and added a new event. Seems to work fine (for me):
> 
> diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
> index 8bfb1298a5..f140a3cca2 100644
> --- a/gdb/mi/mi-interp.c
> +++ b/gdb/mi/mi-interp.c
> @@ -22,6 +22,7 @@
>  #include "event-top.h"
>  #include "event-loop.h"
>  #include "inferior.h"
> +#include "target.h"
>  #include "infrun.h"
>  #include "ui-out.h"
>  #include "top.h"
> @@ -85,6 +86,7 @@ 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_current_target_changed (struct target_ops *target);
> 
>  static int report_initial_inferior (struct inferior *inf, void
> *closure);
> 
> @@ -1280,6 +1282,38 @@ mi_user_selected_context_changed
> (user_selected_what selection)
>      }
>  }
> 
> +static void
> +mi_current_target_changed (struct target_ops *target)
> +{
> +  SWITCH_THRU_ALL_UIS ()
> +    {
> +      struct mi_interp *mi = as_mi_interp (top_level_interpreter ());
> +
> +      if (mi == NULL)
> +	      continue;
> +
> +      target_terminal::scoped_restore_terminal_state term_state;
> +      target_terminal::ours_for_output ();
> +
> +      fprintf_unfiltered (mi->event_channel,
> +			  "target-changed,features=[");
> +      if (mi_async_p ())
> +        fprintf_unfiltered (mi->event_channel,
> +          "\"async\"");
> +      if (target_can_execute_reverse)
> +        {
> +          if (mi_async_p ())
> +            fprintf_unfiltered (mi->event_channel,",\"reverse\"");
> +          else
> +            fprintf_unfiltered (mi->event_channel,"\"reverse\"");
> +        }
> +      fprintf_unfiltered (mi->event_channel,
> +        "]");
> +      gdb_flush (mi->event_channel);
> +    }
> +}
> +
> +
>  static int
>  report_initial_inferior (struct inferior *inf, void *closure)
>  {
> @@ -1382,4 +1416,5 @@ _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::current_target_changed.attach
> (mi_current_target_changed);
>  }
> diff --git a/gdb/observable.c b/gdb/observable.c
> index 5539b9837b..f99a39b909 100644
> --- a/gdb/observable.c
> +++ b/gdb/observable.c
> @@ -74,6 +74,7 @@ DEFINE_OBSERVABLE (inferior_call_pre);
>  DEFINE_OBSERVABLE (inferior_call_post);
>  DEFINE_OBSERVABLE (register_changed);
>  DEFINE_OBSERVABLE (user_selected_context_changed);
> +DEFINE_OBSERVABLE (current_target_changed);
> 
>  } /* namespace observers */
>  } /* namespace gdb */
> diff --git a/gdb/observable.h b/gdb/observable.h
> index 34447b90bb..3f97cc5cb1 100644
> --- a/gdb/observable.h
> +++ b/gdb/observable.h
> @@ -228,6 +228,9 @@ extern observable<struct frame_info *, int>
> register_changed;
>     frame has changed.  */
>  extern observable<user_selected_what> user_selected_context_changed;
> 
> +/* The current target has changed. */
> +extern observable<struct target_ops *> current_target_changed;
> +
>  } /* namespace observers */
> 
>  } /* namespace gdb */
> diff --git a/gdb/target.c b/gdb/target.c
> index e8d4ae7ea8..5cb182c849 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -49,6 +49,7 @@
>  #include "byte-vector.h"
>  #include "terminal.h"
>  #include <algorithm>
> +#include <observable.h>
> 
>  static void generic_tls_error (void) ATTRIBUTE_NORETURN;
> 
> @@ -719,6 +720,8 @@ update_current_target (void)
> 
>    if (targetdebug)
>      setup_target_debug ();
> +
> +  gdb::observers::current_target_changed.notify(&current_target);
>  }
> 
>  /* Push a new target type into the stack of the existing target
> accessors,

Thanks, that seems to make sense with the current codebase.  However, 
Pedro is currently working on adding support for GDB to be able to 
connect to multiple targets at the same time.  So it probably be good to 
think ahead about how this event would work in a multi-target scenario.  
Also, how could we make -list-target-features work when connected to 
multiple targets.

If you want to submit a proper patch, please post it to 
gdb-patches@sourceware.org (so we avoid adding review noise on 
gdb@sourceware.org), and see

   http://sourceware.org/gdb/wiki/ContributionChecklist

Thanks,

Simon

Re: How to use -list-target-features MI command

[Jan Vrany]

On Fri, 2018-04-27 at 14:37 -0400, Simon Marchi wrote:
> On 2018-04-24 06:20, Jan Vrany wrote:
> > Hi,
> > 
> > On Mon, 2018-04-23 at 14:45 +0100, Pedro Alves wrote:
> > > 
> > > > One way to hack around it would be to use for example the new
> > > > "starti" command, which breaks at (before?) the first
> > > > instruction
> > > > of the program, issue -list-target-features, and then continue.
> > > > There might be some better/cleaner solution, but that's what
> > > > comes
> > > > to mind right now.
> > > 
> > > IMO the clean solution is to connect to the native target
> > > explicitly, with "-target-select native", before -exec-run,
> > > instead of letting -exec-run automatically connect to
> > > the native target if not connected to any other target yet.
> > > 
> > >  -gdb-set mi-async on
> > >  ^done
> > >  (gdb)
> > >  -target-select native
> > >  ^connected
> > >  (gdb)
> > >  -list-target-features
> > >  ^done,features=["async"]
> > >  (gdb)
> > >  -exec-run ...
> > > 
> > > I'd also suggest that frontends consider using
> > >   "set auto-connect-native-target off"
> > > https://sourceware.org/gdb/onlinedocs/gdb/Starting.html
> > > 
> > 
> > Thanks! This makes sense. The only problem I see is that it only
> > works
> > reliably if one does not use CLI. In my case, I want user to be
> > able
> > to use the CLI.
> > 
> > 
> > > I don't see a way right now that you could know about
> > > that.  There
> > 
> > would
> > > need to be a new asynchronous event emitted when the target stack
> > > changes.
> > 
> > So gave it a go and added a new event. Seems to work fine (for me):
> > 
> > diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
> > index 8bfb1298a5..f140a3cca2 100644
> > --- a/gdb/mi/mi-interp.c
> > +++ b/gdb/mi/mi-interp.c
> > @@ -22,6 +22,7 @@
> >  #include "event-top.h"
> >  #include "event-loop.h"
> >  #include "inferior.h"
> > +#include "target.h"
> >  #include "infrun.h"
> >  #include "ui-out.h"
> >  #include "top.h"
> > @@ -85,6 +86,7 @@ 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_current_target_changed (struct target_ops *target);
> > 
> >  static int report_initial_inferior (struct inferior *inf, void
> > *closure);
> > 
> > @@ -1280,6 +1282,38 @@ mi_user_selected_context_changed
> > (user_selected_what selection)
> >      }
> >  }
> > 
> > +static void
> > +mi_current_target_changed (struct target_ops *target)
> > +{
> > +  SWITCH_THRU_ALL_UIS ()
> > +    {
> > +      struct mi_interp *mi = as_mi_interp (top_level_interpreter
> > ());
> > +
> > +      if (mi == NULL)
> > +	      continue;
> > +
> > +      target_terminal::scoped_restore_terminal_state term_state;
> > +      target_terminal::ours_for_output ();
> > +
> > +      fprintf_unfiltered (mi->event_channel,
> > +			  "target-changed,features=[");
> > +      if (mi_async_p ())
> > +        fprintf_unfiltered (mi->event_channel,
> > +          "\"async\"");
> > +      if (target_can_execute_reverse)
> > +        {
> > +          if (mi_async_p ())
> > +            fprintf_unfiltered (mi->event_channel,",\"reverse\"");
> > +          else
> > +            fprintf_unfiltered (mi->event_channel,"\"reverse\"");
> > +        }
> > +      fprintf_unfiltered (mi->event_channel,
> > +        "]");
> > +      gdb_flush (mi->event_channel);
> > +    }
> > +}
> > +
> > +
> >  static int
> >  report_initial_inferior (struct inferior *inf, void *closure)
> >  {
> > @@ -1382,4 +1416,5 @@ _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::current_target_changed.attach
> > (mi_current_target_changed);
> >  }
> > diff --git a/gdb/observable.c b/gdb/observable.c
> > index 5539b9837b..f99a39b909 100644
> > --- a/gdb/observable.c
> > +++ b/gdb/observable.c
> > @@ -74,6 +74,7 @@ DEFINE_OBSERVABLE (inferior_call_pre);
> >  DEFINE_OBSERVABLE (inferior_call_post);
> >  DEFINE_OBSERVABLE (register_changed);
> >  DEFINE_OBSERVABLE (user_selected_context_changed);
> > +DEFINE_OBSERVABLE (current_target_changed);
> > 
> >  } /* namespace observers */
> >  } /* namespace gdb */
> > diff --git a/gdb/observable.h b/gdb/observable.h
> > index 34447b90bb..3f97cc5cb1 100644
> > --- a/gdb/observable.h
> > +++ b/gdb/observable.h
> > @@ -228,6 +228,9 @@ extern observable<struct frame_info *, int>
> > register_changed;
> >     frame has changed.  */
> >  extern observable<user_selected_what>
> > user_selected_context_changed;
> > 
> > +/* The current target has changed. */
> > +extern observable<struct target_ops *> current_target_changed;
> > +
> >  } /* namespace observers */
> > 
> >  } /* namespace gdb */
> > diff --git a/gdb/target.c b/gdb/target.c
> > index e8d4ae7ea8..5cb182c849 100644
> > --- a/gdb/target.c
> > +++ b/gdb/target.c
> > @@ -49,6 +49,7 @@
> >  #include "byte-vector.h"
> >  #include "terminal.h"
> >  #include <algorithm>
> > +#include <observable.h>
> > 
> >  static void generic_tls_error (void) ATTRIBUTE_NORETURN;
> > 
> > @@ -719,6 +720,8 @@ update_current_target (void)
> > 
> >    if (targetdebug)
> >      setup_target_debug ();
> > +
> > +  gdb::observers::current_target_changed.notify(&current_target);
> >  }
> > 
> >  /* Push a new target type into the stack of the existing target
> > accessors,
> 
> Thanks, that seems to make sense with the current
> codebase.  However, 
> Pedro is currently working on adding support for GDB to be able to 
> connect to multiple targets at the same time.  So it probably be good
> to 
> think ahead about how this event would work in a multi-target
> scenario.  
> Also, how could we make -list-target-features work when connected to 
> multiple targets.

Hmm, just thinking loud: I don't know the details of Pedro's work but 
I guess in case of multiple targets, it'd make sense to have two
"target-connected" and "target-disconnected" events instead of one
"target-changed" (as in my attempt). This would work for current 
codebase with only one target at time. 

As for -list-target-features - I presume each target would have some
kind of identifier (just like threads or breakpoints do). Then one can
just extend -list-target-features with --target <target id> option. 
If --target is not specified, it would return features of "currently
selected target". This way it'd be backward compatible. 

> 
> If you want to submit a proper patch, please post it to 
> gdb-patches@sourceware.org (so we avoid adding review noise on 
> gdb@sourceware.org), and see
> 
>    http://sourceware.org/gdb/wiki/ContributionChecklist
> 

I will try. 

Thanks, Jan

Re: How to use -list-target-features MI command

[Simon Marchi]

On 2018-05-18 17:16, Jan Vrany wrote:
> Hmm, just thinking loud: I don't know the details of Pedro's work but
> I guess in case of multiple targets, it'd make sense to have two
> "target-connected" and "target-disconnected" events instead of one
> "target-changed" (as in my attempt). This would work for current
> codebase with only one target at time.
> 
> As for -list-target-features - I presume each target would have some
> kind of identifier (just like threads or breakpoints do). Then one can
> just extend -list-target-features with --target <target id> option.
> If --target is not specified, it would return features of "currently
> selected target". This way it'd be backward compatible.

Yep, that makes sense.

Simon