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

[Simon Marchi <simon.marchi@polymtl.ca>]

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