This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH master+7.12] Send thread, frame and inferior selection events to all uis


Hi Antoine,

On 08/31/2016 02:51 PM, Antoine Tremblay wrote:
> With this patch when a thread, frame or inferior is explicitly selected by
> the user in the console or mi all uis will be notified.
> 
> This patch fixes PR gdb/20487.
> 
> Also, this patch adds a new field to the =thread-selected event for the
> frame, since inferior/thread/frame are most often a composition, thread and
> frame are sent in the same event.
> 
> Front-ends need to handle this new field to properly sync the frame.
> 
> Here's a detailed example for each command:
> 
> - thread
> From console: thread 1.3
> Will generate in the mi channel: =thread-selected,id="3",frame={...}
> 
> From mi: -thread-select 3
> Will generate in the console: [Switching to thread 1.3 ...
> 
> From mi: thread 1.3
> Will generate the following mi:
> &"thread 1.3\n"
> ~"#0  child_sub_function () ...
> =thread-selected,id="3",frame={level="0",...}
> ^done

Does the later generate the event in the console too?  I assume so.

> 
> - frame
> From console: frame 1
> Will generate in the mi channel: =thread-selected,id="3",frame={level="1",...}
> 
> From mi: -stack-select-frame 1
> Will generate in the console: #1  0x00000000004007f0 in child_function...
> 
> From mi: frame 1
> Will generate the following mi:
> &"frame 1\n"
> ~"#1  0x00000000004007f9 in ..."
> =thread-selected,id="3",frame={level="1"...}
> ^done
> 
> - inferior
> 
> For inferior selection however it only goes from the console to mi as
> there's no way to select the inferior in mi except with a cli command.
> 
> - inferior
> From mi: inferior 2
> Will generate the following in mi:
> &"inferior 2\n"
> ~"[Switching to inferior 2 ..."
> =thread-selected,id="4",frame={level="0"...}
> ^done
> 
> From console: inferior 2
> Will generate in the mi channel: =thread-selected,id="3"
> 
> The test thread-selected-sync.exp also tests the select-frame cli command
> and corner cases of all selection commands.
> 
> Note also that this patch makes it possible to suppress notifications
> caused by a cli command, like was done in mi-interp previously.
> 
> This means that it's now possible to use the add_com_suppress_notification
> function to register a command with some event suppressed like is done
> with the select-frame command in this patch.

I should note that this suppression mechanism is broken when you
have multiple MI uis or multiple consoles.  We end up suppressing
the event on all uis that happen run the same top level interpreter...
I.e., with two CLIs, do "thread" on one UI, and notice how the
event is suppressed on the other UI.  Likewise with two MIs, etc.

TBC, it's fine with me to not address this for now.

> 
> No regressions, tested on ubuntu 14.04 x86.
> With gdb and native-extented-gdbserver
> 
> gdb/ChangeLog:
> 
> 	PR gdb/20487
> 	* NEWS: Mention new frame field of =thread-selected event.
> 	* cli/cli-decode.c (add_cmd): Initialize supress_notification.
> 	(add_com_suppress_notification): New function.
> 	(cmd_func): Set the suppress_notification flag and reset it after
> 	cmd->func call.
> 	* cli/cli-decode.h (struct
> 	cmd_list_element)<suppress_notification>: New field.
> 	* cli/cli-interp.c (cli_suppress_notification): Initialize
> 	cli_suppress_notification.
> 	(cli_on_user_selected_inf_thread_frame): New function.
> 	(_initialize_cli_interp): Add user_selected_inf_thread_frame observer.
> 	* command.h (struct cli_suppress_notification): New struct.
> 	(cli_suppress_notification): Struct declaration.
> 	(add_com_suppress_notification): New function.
> 	* defs.h (enum user_selected_what_flag): New enum.
> 	(user_selected_what): New enum flag type.
> 	* frame.h (print_stack_frame_to_uiout): New function.
> 	* gdbthread.h (print_selected_thread_frame): New function declaration.
> 	* inferior.c (print_selected_inferior): New function.
> 	(inferior_command): Remove printing the inferior this is now done
> 	by an event.
> 	(inferior_command): Notify user_selected_inf_thread_frame if the
> 	inferior changed. Let the event print to cli/mi.
> 	* inferior.h (print_selected_inferior): New function declaration.
> 	* mi/mi-cmds.c (struct mi_cmd): Add user_selected_thread_frame
> 	suppression for stack-select-frame, thread-select command.
> 	* mi/mi-interp.c (struct
> 	mi_suppress_notification)<user_selected_inf_thread_frame>: Init.
> 	(mi_memory_changed): mi_user_selected_inf_thread_frame): New
> 	* function.
> 	(_initialize_mi_interp): Add user_selected_inf_thread_frame observer.
> 	* mi/mi-main.c (mi_cmd_thread_select): Print thread selection response.
> 	(mi_execute_command): Don't report thread change on
> 	thread or inferior cli commands, notify if the thread changes.
> 	* mi/mi-main.h (struct
> 	mi_suppress_notification)<user_selected_inf_thread_frame>: New field.
> 	* stack.c: include "observer.h".
> 	(print_stack_frame_to_uiout): New function.
> 	(select_frame_command): Notify user_selected_inf_thread_frame.
> 	(frame_command): Use print_selected_thread_frame if there's no
> 	change or notify if there is one.
> 	(up_command): Notify user_selected_inf_thread_frame.
> 	(down_command): Likewise.
> 	(_initialize_stack): Suppress event in cli for command
> 	select-frame.
> 	* thread.c (thread_command): Print if thread has not changed.
> 	(do_captured_thread_select): Let mi_cmd_thread_select print.
> 	(print_selected_thread_frame): New function.
> 	* tui/tui-interp.c (tui_on_user_selected_inf_thread_frame):
> 	New function.
> 	(_initialize_tui_interp): Add user_selected_inf_thread_frame observer.
> 
> gdb/doc/ChangeLog:
> 
> 	PR gdb/20487
> 	* observer.texi (user_selected_inf_thread_frame): New function.

This needs a manual update:

@subsubsection Threads and Frames

[...]
it is reasonable to switch to the thread where breakpoint is hit.  For
another example, if the user issues the CLI @samp{thread} command via
the frontend, it is desirable to change the frontend's selected thread to the
one specified by user.  @value{GDBN} communicates the suggestion to
change current thread using the @samp{=thread-selected} notification.
No such notification is available for the selected frame at the moment.
[...]
@item =thread-selected,id="@var{id}"
Informs that the selected thread was changed as result of the last
command.
[...]


> 
> gdb/testsuite/ChangeLog:
> 
> 	PR gdb/20487
> 	* gdb.mi/thread-selected-sync.c: New file.
> 	* gdb.mi/thread-selected-sync.exp: Likewise.
> 	* gdb.mi/mi-pthreads.exp(check_mi_thread_command_set): Adapt
> 	=thread-select-event check.
> ---
>  gdb/NEWS                                      |    4 +
>  gdb/cli/cli-decode.c                          |   31 +-
>  gdb/cli/cli-decode.h                          |    6 +
>  gdb/cli/cli-interp.c                          |   36 +
>  gdb/command.h                                 |   16 +
>  gdb/defs.h                                    |   15 +
>  gdb/doc/observer.texi                         |    4 +
>  gdb/frame.h                                   |    8 +
>  gdb/gdbthread.h                               |    4 +
>  gdb/inferior.c                                |   40 +-
>  gdb/inferior.h                                |    3 +
>  gdb/mi/mi-cmds.c                              |    6 +-
>  gdb/mi/mi-interp.c                            |   57 ++
>  gdb/mi/mi-main.c                              |   45 +-
>  gdb/mi/mi-main.h                              |    2 +
>  gdb/stack.c                                   |   39 +-
>  gdb/testsuite/gdb.mi/mi-pthreads.exp          |    2 +-
>  gdb/testsuite/gdb.mi/thread-selected-sync.c   |   64 ++
>  gdb/testsuite/gdb.mi/thread-selected-sync.exp | 1021 +++++++++++++++++++++++++
>  gdb/thread.c                                  |   80 +-
>  gdb/tui/tui-interp.c                          |   31 +
>  21 files changed, 1451 insertions(+), 63 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.mi/thread-selected-sync.c
>  create mode 100644 gdb/testsuite/gdb.mi/thread-selected-sync.exp
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 6f5feb1..b427a7e 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -11,6 +11,10 @@
>  
>  *** Changes in GDB 7.12
>  
> +* MI =thread-selected event now includes the frame field. For example:
> +
> +     =thread-selected,id="3",frame={level="0",addr="0x00000000004007c0"}
> +


Please put this next to the other MI change:

* MI async record =record-started now includes the method and format used for
  recording.  For example:

    =record-started,thread-group="i1",method="btrace",format="bts"

and I guess use "async record" too for consistency.


> @@ -883,6 +884,21 @@ add_com_alias (const char *name, const char *oldname, enum command_class theclas
>  {
>    return add_alias_cmd (name, oldname, theclass, abbrev_flag, &cmdlist);
>  }
> +
> +/* Add an element with a suppress notification to the list of commands.  */
> +
> +struct cmd_list_element *
> +add_com_suppress_notification (const char *name, enum command_class theclass,
> +			       cmd_cfunc_ftype *fun, const char *doc,
> +			       int *suppress_notification)
> +{
> +  struct cmd_list_element *element;
> +
> +  element = add_cmd (name, theclass, fun, doc, &cmdlist);
> +  element->suppress_notification = suppress_notification;
> +
> +  return element;
> +}
>  
>  /* Recursively walk the commandlist structures, and print out the
>     documentation of commands that match our regex in either their
> @@ -1884,8 +1900,21 @@ cmd_func_p (struct cmd_list_element *cmd)
>  void
>  cmd_func (struct cmd_list_element *cmd, char *args, int from_tty)
>  {
> +  struct cleanup *cleanup = NULL;
> +
>    if (cmd_func_p (cmd))
> -    (*cmd->func) (cmd, args, from_tty);
> +    {
> +      if (cmd->suppress_notification != NULL)
> +	{
> +	  cleanup = make_cleanup_restore_integer (cmd->suppress_notification);
> +	  *cmd->suppress_notification = 1;
> +	}
> +
> +      (*cmd->func) (cmd, args, from_tty);
> +
> +      if (cleanup != NULL)
> +	do_cleanups(cleanup);
> +    }
>    else
>      error (_("Invalid command"));
>  }
> diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
> index 4ea8063..4ef2e1b 100644
> --- a/gdb/cli/cli-decode.h
> +++ b/gdb/cli/cli-decode.h
> @@ -218,6 +218,12 @@ struct cmd_list_element
>  
>      /* Link pointer for aliases on an alias list.  */
>      struct cmd_list_element *alias_chain;
> +
> +    /* If non-null, the pointer to a field in 'struct
> +       cli_suppress_notification', which will be set to true in cmd_func
> +       when this command is being executed.  It will be set back to false
> +       when the command has been executed.  */
> +    int *suppress_notification;
>    };
>  
>  extern void help_cmd_list (struct cmd_list_element *, enum command_class,
> diff --git a/gdb/cli/cli-interp.c b/gdb/cli/cli-interp.c
> index 5d67ba4..8f1a034 100644
> --- a/gdb/cli/cli-interp.c
> +++ b/gdb/cli/cli-interp.c
> @@ -37,6 +37,12 @@ struct cli_interp
>    struct ui_out *cli_uiout;
>  };
>  
> +/* Suppress notification struct.  */
> +struct cli_suppress_notification cli_suppress_notification =
> +  {
> +    0   /* user_selected_inf_thread_frame */

Maybe rename this to user_selected_context?  Using "inf_thread_frame"
will get weird once we add more "what"s.

> +  };
> +
>  /* Returns the INTERP's data cast as cli_interp if INTERP is a CLI,
>     and returns NULL otherwise.  */
>  
> @@ -229,6 +235,34 @@ cli_on_command_error (void)
>    display_gdb_prompt (NULL);
>  }
>  
> +/* Observer for the user_selected_inf_thread_frame notification.  */
> +
> +static void
> +cli_on_user_selected_inf_thread_frame (user_selected_what selection)

Likewise, etc.

> +{
> +  struct switch_thru_all_uis state;
> +  struct thread_info *tp = find_thread_ptid (inferior_ptid);
> +
> +  /* This event is suppressed.  */
> +  if (cli_suppress_notification.user_selected_inf_thread_frame)
> +    return;

Move the find_thread_ptid call after this, to avoid an
unnecessary lookup?

> +
> +  SWITCH_THRU_ALL_UIS (state)
> +    {
> +      struct cli_interp *cli = as_cli_interp (top_level_interpreter ());
> +
> +      if (cli == NULL)
> +	continue;
> +
> +      if (selection & USER_SELECTED_INFERIOR)
> +	print_selected_inferior (cli->cli_uiout);
> +
> +      if (tp != NULL
> +	  && ((selection & (USER_SELECTED_THREAD | USER_SELECTED_FRAME))))
> +	print_selected_thread_frame (cli->cli_uiout, selection);
> +    }
> +}
> +
>  /* pre_command_loop implementation.  */
>  
>  void
> @@ -393,4 +427,6 @@ _initialize_cli_interp (void)
>    observer_attach_no_history (cli_on_no_history);
>    observer_attach_sync_execution_done (cli_on_sync_execution_done);
>    observer_attach_command_error (cli_on_command_error);
> +  observer_attach_user_selected_inf_thread_frame
> +    (cli_on_user_selected_inf_thread_frame);
>  }
> diff --git a/gdb/command.h b/gdb/command.h
> index ab62601..a36f05c 100644
> --- a/gdb/command.h
> +++ b/gdb/command.h
> @@ -115,6 +115,17 @@ struct cmd_list_element;
>  
>  typedef void cmd_cfunc_ftype (char *args, int from_tty);
>  
> +/* This structure specifies notifications to be suppressed by a cli
> +   command interpreter.  */
> +
> +struct cli_suppress_notification
> +{
> +  /* Inferior, thread, frame selected notification suppressed?  */
> +  int user_selected_inf_thread_frame;
> +};
> +
> +extern struct cli_suppress_notification cli_suppress_notification;
> +
>  /* Forward-declarations of the entry-points of cli/cli-decode.c.  */
>  
>  /* API to the manipulation of command lists.  */
> @@ -218,6 +229,11 @@ extern struct cmd_list_element *add_com (const char *, enum command_class,
>  extern struct cmd_list_element *add_com_alias (const char *, const char *,
>  					       enum command_class, int);
>  
> +extern struct cmd_list_element *add_com_suppress_notification
> +		       (const char *name, enum command_class theclass,
> +			cmd_cfunc_ftype *fun, const char *doc,
> +			int *supress_notification);
> +
>  extern struct cmd_list_element *add_info (const char *,
>  					  cmd_cfunc_ftype *fun,
>  					  const char *);
> diff --git a/gdb/defs.h b/gdb/defs.h
> index fee5f41..cb180a9 100644
> --- a/gdb/defs.h
> +++ b/gdb/defs.h
> @@ -750,6 +750,21 @@ enum block_enum
>    FIRST_LOCAL_BLOCK = 2
>  };
>  
> +/* User selection used in observer.h and multiple print functions.  */
> +
> +enum user_selected_what_flag
> +  {
> +    /* Inferior selected.  */
> +    USER_SELECTED_INFERIOR = 1 << 1,
> +
> +    /* Thread selected.  */
> +    USER_SELECTED_THREAD = 1 << 2,
> +
> +    /* Frame selected.  */
> +    USER_SELECTED_FRAME = 1 << 3
> +  };
> +DEF_ENUM_FLAGS_TYPE (enum user_selected_what_flag, user_selected_what);
> +

This needs to #include "common/enum-flags.h".

If this didn't cause you a compile error, then it must be some other
header is already pulling the first one in.  I got curious and checked
what it was.  It's this sequence:

In file included from /home/pedro/gdb/mygit/src/gdb/symtab.h:26:0,
                 from /home/pedro/gdb/mygit/src/gdb/language.h:26,
                 from /home/pedro/gdb/mygit/src/gdb/frame.h:72,
                 from /home/pedro/gdb/mygit/src/gdb/gdbarch.h:38,
                 from /home/pedro/gdb/mygit/src/gdb/defs.h:652,
                 from /home/pedro/gdb/mygit/src/gdb/gdb.c:19:
/home/pedro/gdb/mygit/src/gdb/common/enum-flags.h:1:2: error: #error "enum-flags.h" included

In any case, we should #include what we depend on directly
instead of relying on some accidental indirect dependency.


>  #include "utils.h"
>  
>  #endif /* #ifndef DEFS_H */
> diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi
> index fc7aac4..4f8f0bb 100644
> --- a/gdb/doc/observer.texi
> +++ b/gdb/doc/observer.texi
> @@ -307,3 +307,7 @@ This observer is used for internal testing.  Do not use.
>  See testsuite/gdb.gdb/observer.exp.
>  @end deftypefun
>  
> +@deftypefun void user_selected_inf_thread_frame (user_selected_what @var{selection})
> +The user-selected inferior,thread and/or frame has changed.  The user_select_what
> +flag specifies if the inferior, thread or frame has changed.

Missing space in "inferior,thread".

> +@end deftypefun


> diff --git a/gdb/frame.h b/gdb/frame.h
> index 5f21bb8..de13e7d 100644
> --- a/gdb/frame.h
> +++ b/gdb/frame.h
> @@ -704,6 +704,14 @@ extern CORE_ADDR get_pc_function_start (CORE_ADDR);
>  
>  extern struct frame_info *find_relative_frame (struct frame_info *, int *);
>  
> +/* Wrapper over print_stack_frame modifying current_uiout with UIOUT for
> +   the function call.  */
> +
> +extern void print_stack_frame_to_uiout (struct ui_out *uiout,
> +					struct frame_info *, int print_level,
> +					enum print_what print_what,
> +					int set_current_sal);
> +
>  extern void print_stack_frame (struct frame_info *, int print_level,
>  			       enum print_what print_what,
>  			       int set_current_sal);
> diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
> index af2dc86..8f37fbb 100644
> --- a/gdb/gdbthread.h
> +++ b/gdb/gdbthread.h
> @@ -630,6 +630,10 @@ extern void validate_registers_access (void);
>     true iff we ever detected multiple threads.  */
>  extern int show_thread_that_caused_stop (void);
>  
> +/* Print the message for a thread or/and frame selected.  */
> +extern void print_selected_thread_frame (struct ui_out *uiout,
> +					 user_selected_what selection);
> +
>  extern struct thread_info *thread_list;
>  
>  #endif /* GDBTHREAD_H */
> diff --git a/gdb/inferior.c b/gdb/inferior.c
> index 47d91c7..60b3109 100644
> --- a/gdb/inferior.c
> +++ b/gdb/inferior.c
> @@ -548,6 +548,24 @@ inferior_pid_to_str (int pid)
>      return _("<null>");
>  }
>  
> +/* See inferior.h.  */
> +
> +void
> +print_selected_inferior (struct ui_out *uiout)
> +{
> +  char buf[PATH_MAX+256];

Missing spaces.

> +  struct inferior *inf = current_inferior ();
> +
> +  xsnprintf (buf, sizeof (buf),
> +	     _("[Switching to inferior %d [%s] (%s)]\n"),
> +	     inf->num,
> +	     inferior_pid_to_str (inf->pid),
> +	     (inf->pspace->pspace_exec_filename != NULL
> +	      ? inf->pspace->pspace_exec_filename
> +	      : _("<noexec>")));
> +  ui_out_text (uiout, buf);
> +}
> +
>  /* Prints the list of inferiors and their details on UIOUT.  This is a
>     version of 'info_inferior_command' suitable for use from MI.
>  
> @@ -726,13 +744,6 @@ inferior_command (char *args, int from_tty)
>    if (inf == NULL)
>      error (_("Inferior ID %d not known."), num);
>  
> -  printf_filtered (_("[Switching to inferior %d [%s] (%s)]\n"),
> -		   inf->num,
> -		   inferior_pid_to_str (inf->pid),
> -		   (inf->pspace->pspace_exec_filename != NULL
> -		    ? inf->pspace->pspace_exec_filename
> -		    : _("<noexec>")));
> -
>    if (inf->pid != 0)
>      {
>        if (inf->pid != ptid_get_pid (inferior_ptid))
> @@ -746,9 +757,10 @@ inferior_command (char *args, int from_tty)
>  	  switch_to_thread (tp->ptid);
>  	}
>  
> -      printf_filtered (_("[Switching to thread %s (%s)] "),
> -		       print_thread_id (inferior_thread ()),
> -		       target_pid_to_str (inferior_ptid));
> +      observer_notify_user_selected_inf_thread_frame
> +	(USER_SELECTED_INFERIOR
> +	 | USER_SELECTED_THREAD
> +	 | USER_SELECTED_FRAME);
>      }
>    else
>      {
> @@ -758,14 +770,8 @@ inferior_command (char *args, int from_tty)
>        set_current_inferior (inf);
>        switch_to_thread (null_ptid);
>        set_current_program_space (inf->pspace);
> -    }
>  
> -  if (inf->pid != 0 && is_running (inferior_ptid))
> -    ui_out_text (current_uiout, "(running)\n");
> -  else if (inf->pid != 0)
> -    {
> -      ui_out_text (current_uiout, "\n");
> -      print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC, 1);
> +      observer_notify_user_selected_inf_thread_frame (USER_SELECTED_INFERIOR);
>      }
>  }
>  
> diff --git a/gdb/inferior.h b/gdb/inferior.h
> index 571d26a..54c6f65 100644
> --- a/gdb/inferior.h
> +++ b/gdb/inferior.h
> @@ -523,4 +523,7 @@ extern int number_of_inferiors (void);
>  
>  extern struct inferior *add_inferior_with_spaces (void);
>  
> +/* Print the current selected inferior.  */
> +extern void print_selected_inferior (struct ui_out *uiout);
> +
>  #endif /* !defined (INFERIOR_H) */
> diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
> index 4779832..967b921 100644
> --- a/gdb/mi/mi-cmds.c
> +++ b/gdb/mi/mi-cmds.c
> @@ -137,7 +137,8 @@ static struct mi_cmd mi_cmds[] =
>    DEF_MI_CMD_MI ("stack-list-frames", mi_cmd_stack_list_frames),
>    DEF_MI_CMD_MI ("stack-list-locals", mi_cmd_stack_list_locals),
>    DEF_MI_CMD_MI ("stack-list-variables", mi_cmd_stack_list_variables),
> -  DEF_MI_CMD_MI ("stack-select-frame", mi_cmd_stack_select_frame),
> +  DEF_MI_CMD_MI_1 ("stack-select-frame", mi_cmd_stack_select_frame,
> +		   &mi_suppress_notification.user_selected_inf_thread_frame),
>    DEF_MI_CMD_MI ("symbol-list-lines", mi_cmd_symbol_list_lines),
>    DEF_MI_CMD_CLI ("target-attach", "attach", 1),
>    DEF_MI_CMD_MI ("target-detach", mi_cmd_target_detach),
> @@ -149,7 +150,8 @@ static struct mi_cmd mi_cmds[] =
>    DEF_MI_CMD_CLI ("target-select", "target", 1),
>    DEF_MI_CMD_MI ("thread-info", mi_cmd_thread_info),
>    DEF_MI_CMD_MI ("thread-list-ids", mi_cmd_thread_list_ids),
> -  DEF_MI_CMD_MI ("thread-select", mi_cmd_thread_select),
> +  DEF_MI_CMD_MI_1 ("thread-select", mi_cmd_thread_select,
> +		   &mi_suppress_notification.user_selected_inf_thread_frame),
>    DEF_MI_CMD_MI ("trace-define-variable", mi_cmd_trace_define_variable),
>    DEF_MI_CMD_MI_1 ("trace-find", mi_cmd_trace_find,
>  		   &mi_suppress_notification.traceframe),
> diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
> index e3c7dbd..df67c9a 100644
> --- a/gdb/mi/mi-interp.c
> +++ b/gdb/mi/mi-interp.c
> @@ -769,6 +769,7 @@ struct mi_suppress_notification mi_suppress_notification =
>      0,
>      0,
>      0,
> +    0,
>    };
>  
>  /* Emit notification on changing a traceframe.  */
> @@ -1334,6 +1335,60 @@ mi_memory_changed (struct inferior *inferior, CORE_ADDR memaddr,
>      }
>  }
>  
> +/* Emit an event about the user selection of inf,thread or frame.  */
> +
> +static void
> +mi_user_selected_inf_thread_frame (user_selected_what selection)
> +{
> +  struct switch_thru_all_uis state;
> +  struct thread_info *tp = find_thread_ptid (inferior_ptid);
> +
> +  /* Don't send an event if we're responding to an MI command.  */
> +  if (mi_suppress_notification.user_selected_inf_thread_frame)
> +    return;

Same comment about moving find_thread_ptid.

> +
> +  SWITCH_THRU_ALL_UIS (state)
> +  {
> +    struct mi_interp *mi = as_mi_interp (top_level_interpreter ());
> +    struct ui_out *mi_uiout;
> +    struct cleanup *old_chain;
> +
> +    if (mi == NULL)
> +      continue;
> +
> +    mi_uiout = interp_ui_out (top_level_interpreter ());
> +
> +    ui_out_redirect (mi_uiout, mi->event_channel);
> +
> +    old_chain = make_cleanup_restore_target_terminal ();
> +    target_terminal_ours_for_output ();
> +
> +    if (selection & USER_SELECTED_INFERIOR)
> +      print_selected_inferior (mi->cli_uiout);
> +
> +    if (tp != NULL
> +	&& ((selection & (USER_SELECTED_THREAD | USER_SELECTED_FRAME))))

One level of parens too many?

> +      {
> +	print_selected_thread_frame (mi->cli_uiout, selection);
> +
> +	fprintf_unfiltered (mi->event_channel,
> +			    "thread-selected,id=\"%d\"",
> +			    tp->global_num);
> +
> +	if (tp->state != THREAD_RUNNING)
> +	  {
> +	    if (has_stack_frames ())
> +	      print_stack_frame_to_uiout (mi_uiout, get_selected_frame (NULL),
> +					  1, SRC_AND_LOC, 1);
> +	  }
> +      }
> +
> +    ui_out_redirect (mi_uiout, NULL);

Use make_cleanup_ui_out_redirect_pop instead.




> +    gdb_flush (mi->event_channel);
> +    do_cleanups (old_chain);
> +  }
> +}
> +
>  static int
>  report_initial_inferior (struct inferior *inf, void *closure)
>  {
> @@ -1466,4 +1521,6 @@ _initialize_mi_interp (void)
>    observer_attach_command_param_changed (mi_command_param_changed);
>    observer_attach_memory_changed (mi_memory_changed);
>    observer_attach_sync_execution_done (mi_on_sync_execution_done);
> +  observer_attach_user_selected_inf_thread_frame
> +    (mi_user_selected_inf_thread_frame);
>  }
> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> index 1913157..ebd5ee8 100644
> --- a/gdb/mi/mi-main.c
> +++ b/gdb/mi/mi-main.c
> @@ -53,6 +53,7 @@
>  #include "linespec.h"
>  #include "extension.h"
>  #include "gdbcmd.h"
> +#include "observer.h"
>  
>  #include <ctype.h>
>  #include "gdb_sys_time.h"
> @@ -575,6 +576,9 @@ mi_cmd_thread_select (char *command, char **argv, int argc)
>        make_cleanup (xfree, mi_error_message);
>        error ("%s", mi_error_message);
>      }
> +
> +  print_selected_thread_frame (interp_ui_out (top_level_interpreter ()),
> +			       USER_SELECTED_THREAD | USER_SELECTED_FRAME);
>  }
>  
>  void
> @@ -2102,6 +2106,7 @@ mi_execute_command (const char *cmd, int from_tty)
>  {
>    char *token;
>    struct mi_parse *command = NULL;
> +  struct cleanup *cleanup = NULL;

Move this to the scope that needs it.


>  
>    /* This is to handle EOF (^D). We just quit gdb.  */
>    /* FIXME: we should call some API function here.  */
> @@ -2161,10 +2166,15 @@ mi_execute_command (const char *cmd, int from_tty)
>  	  /* Don't try report anything if there are no threads --
>  	     the program is dead.  */
>  	  && thread_count () != 0
> -	  /* -thread-select explicitly changes thread. If frontend uses that
> -	     internally, we don't want to emit =thread-selected, since
> -	     =thread-selected is supposed to indicate user's intentions.  */
> -	  && strcmp (command->command, "thread-select") != 0)
> +	  /* For the cli commands thread and inferior, the event is already sent
> +	     by the command, don't send it again.  */
> +	  && ((command->op == CLI_COMMAND
> +	       && strncmp (command->command, "thread", 6) != 0
> +	       && strncmp (command->command, "inferior", 8) != 0)
> +	      || (command->op == MI_COMMAND && command->argc <= 1)
> +	      || (command->op == MI_COMMAND && command->argc > 1
> +		  && strncmp (command->argv[1], "thread", 6) != 0
> +		  && strncmp (command->argv[1], "inferior", 8) != 0)))

Urgh.  :-/  I wish we'd find some better way...

- This is matching "threadfoo", "inferiorfoo" as well.

- This is matching the "thread" subcommands too.  Probably OK.

- Does this match command aliases?  (alias foo=thread)

- The suppression used to be for "thread-select" only, now
  it's done for all MI commands.  Not sure whether this was intended
  or not.  IOW, this needs an explicit rationale/comments.

- Is the argc checking meant to handle "-interpreter-exec"?
  Shouldn't there be an explicit check for that?

I guess Simon's work on decoupling user-selected state a
little better might help clean this up.
But meanwhile, we need to make sure to err on the
"emit notification" side and the latter two points above worry me.


>  	{
>  	  struct mi_interp *mi
>  	    = (struct mi_interp *) top_level_interpreter_data ();
> @@ -2185,18 +2195,21 @@ mi_execute_command (const char *cmd, int from_tty)
>  
>  	  if (report_change)
>  	    {
> -	      struct thread_info *ti = inferior_thread ();
> -	      struct cleanup *old_chain;
> -
> -	      old_chain = make_cleanup_restore_target_terminal ();
> -	      target_terminal_ours_for_output ();
> -
> -	      fprintf_unfiltered (mi->event_channel,
> -				  "thread-selected,id=\"%d\"",
> -				  ti->global_num);
> -	      gdb_flush (mi->event_channel);
> -
> -	      do_cleanups (old_chain);
> +	      /* Make sure we still keep event suppression.  This is
> +		 handled in mi_cmd_execute so at this point this has been
> +		 reset.  We still need it here however.  */

This raises the question of why is the event suppression done in
mi_cmd_execute instead of in this function, before calling
mi_cmd_execute?


> +	        if (command->cmd->suppress_notification != NULL)
> +		  {
> +		    cleanup = make_cleanup_restore_integer
> +		      (command->cmd->suppress_notification);
> +		    *command->cmd->suppress_notification = 1;
> +		  }
> +
> +		observer_notify_user_selected_inf_thread_frame
> +		  (USER_SELECTED_THREAD | USER_SELECTED_FRAME);
> +
> +		if (cleanup != NULL)
> +		  do_cleanups (cleanup);
>  	    }
>  	}
>  
> diff --git a/gdb/mi/mi-main.h b/gdb/mi/mi-main.h
> index 18000cf..f5b6bcf 100644
> --- a/gdb/mi/mi-main.h
> +++ b/gdb/mi/mi-main.h
> @@ -49,6 +49,8 @@ struct mi_suppress_notification
>    int traceframe;
>    /* Memory changed notification suppressed?  */
>    int memory;
> +  /* Inferior, thread, frame selected notification suppressed?  */
> +  int user_selected_inf_thread_frame;
>  };
>  extern struct mi_suppress_notification mi_suppress_notification;
>  
> diff --git a/gdb/stack.c b/gdb/stack.c
> index 417e887..80ce00b 100644
> --- a/gdb/stack.c
> +++ b/gdb/stack.c
> @@ -51,6 +51,7 @@
>  #include "safe-ctype.h"
>  #include "symfile.h"
>  #include "extension.h"
> +#include "observer.h"
>  
>  /* The possible choices of "set print frame-arguments", and the value
>     of this setting.  */
> @@ -141,6 +142,22 @@ frame_show_address (struct frame_info *frame,
>    return get_frame_pc (frame) != sal.pc;
>  }
>  
> +/* See frame.h  */
> +
> +void
> +print_stack_frame_to_uiout (struct ui_out *uiout, struct frame_info *frame,
> +			    int print_level, enum print_what print_what,
> +			    int set_current_sal)
> +{
> +  struct ui_out *saved_uiout;
> +  saved_uiout = current_uiout;
> +  current_uiout = uiout;
> +
> +  print_stack_frame (frame, print_level, print_what, set_current_sal);
> +
> +  current_uiout = saved_uiout;

Use a cleanup.

> +}
> +
>  /* Show or print a stack frame FRAME briefly.  The output is formatted
>     according to PRINT_LEVEL and PRINT_WHAT printing the frame's
>     relative level, function name, argument list, and file name and
> @@ -2302,7 +2319,11 @@ find_relative_frame (struct frame_info *frame, int *level_offset_ptr)
>  void
>  select_frame_command (char *level_exp, int from_tty)
>  {
> +  struct frame_info *prev_frame = get_selected_frame_if_set ();
> +
>    select_frame (parse_frame_specification (level_exp, NULL));
> +  if (get_selected_frame_if_set () != prev_frame)
> +    observer_notify_user_selected_inf_thread_frame (USER_SELECTED_FRAME);
>  }
>  
>  /* The "frame" command.  With no argument, print the selected frame
> @@ -2312,8 +2333,13 @@ select_frame_command (char *level_exp, int from_tty)
>  static void
>  frame_command (char *level_exp, int from_tty)
>  {
> -  select_frame_command (level_exp, from_tty);
> -  print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC, 1);
> +  struct frame_info *prev_frame = get_selected_frame_if_set ();
> +
> +  select_frame (parse_frame_specification (level_exp, NULL));
> +  if (get_selected_frame_if_set () != prev_frame)
> +    observer_notify_user_selected_inf_thread_frame (USER_SELECTED_FRAME);
> +  else
> +    print_selected_thread_frame (current_uiout, USER_SELECTED_FRAME);
>  }
>  
>  /* Select the frame up one or COUNT_EXP stack levels from the
> @@ -2344,7 +2370,7 @@ static void
>  up_command (char *count_exp, int from_tty)
>  {
>    up_silently_base (count_exp);
> -  print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC, 1);
> +  observer_notify_user_selected_inf_thread_frame (USER_SELECTED_FRAME);
>  }
>  
>  /* Select the frame down one or COUNT_EXP stack levels from the previously
> @@ -2383,7 +2409,7 @@ static void
>  down_command (char *count_exp, int from_tty)
>  {
>    down_silently_base (count_exp);
> -  print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC, 1);
> +  observer_notify_user_selected_inf_thread_frame (USER_SELECTED_FRAME);
>  }
>  
>  
> @@ -2616,10 +2642,11 @@ a command file or a user-defined command."));
>  
>    add_com_alias ("f", "frame", class_stack, 1);
>  
> -  add_com ("select-frame", class_stack, select_frame_command, _("\
> +  add_com_suppress_notification ("select-frame", class_stack, select_frame_command, _("\
>  Select a stack frame without printing anything.\n\
>  An argument specifies the frame to select.\n\
> -It can be a stack frame number or the address of the frame.\n"));
> +It can be a stack frame number or the address of the frame.\n"),
> +		 &cli_suppress_notification.user_selected_inf_thread_frame);
>  
>    add_com ("backtrace", class_stack, backtrace_command, _("\
>  Print backtrace of all stack frames, or innermost COUNT frames.\n\
> diff --git a/gdb/testsuite/gdb.mi/mi-pthreads.exp b/gdb/testsuite/gdb.mi/mi-pthreads.exp
> index 88a600a..a49856e 100644
> --- a/gdb/testsuite/gdb.mi/mi-pthreads.exp
> +++ b/gdb/testsuite/gdb.mi/mi-pthreads.exp
> @@ -53,7 +53,7 @@ proc check_mi_thread_command_set {} {
>  
>    foreach thread $thread_list {
>        mi_gdb_test "-interpreter-exec console \"thread $thread\"" \
> -          ".*\\^done\r\n=thread-selected,id=\"$thread\"" \
> +	  ".*=thread-selected,id=\"$thread\".*\[r\n\]+\\^done\[\r\n\]+" \
>            "check =thread-selected: thread $thread"

Not sure I'm reading this right, but I think this just means that
the notification is now sent before the ^done.  Is that right?
Probably not a problem, just checking.

>    }
>  }
> diff --git a/gdb/testsuite/gdb.mi/thread-selected-sync.c b/gdb/testsuite/gdb.mi/thread-selected-sync.c
> new file mode 100644
> index 0000000..4113d26
> --- /dev/null
> +++ b/gdb/testsuite/gdb.mi/thread-selected-sync.c

Maybe rename this to user-selected-context-sync.c ?


> @@ -0,0 +1,64 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2016 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +*/
> +
> +/* Note that this test is not expected to exit cleanly.  All threads will
> +   block at the barrier and they won't be waken up. */

Please add an alarm call then:

 https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Don.27t_write_tests_that_run_forever

> +
> +#include <pthread.h>
> +#include <unistd.h>
> +
> +#define NUM_THREADS 2
> +
> +pthread_barrier_t barrier;
> +
> +void
> +child_sub_function ()

(void)

> +{
> +  int test = 0;
> +  test++; /* set break here */
> +  pthread_barrier_wait (&barrier);
> +  pthread_exit (NULL);
> +}
> +
> +void *
> +child_function (void *args)
> +{
> +  child_sub_function (); /* caller */
> +}
> +
> +pthread_t child_thread[NUM_THREADS];
> +
> +int
> +main (void)
> +{
> +  int i = 0;
> +  pthread_barrier_init (&barrier, NULL, NUM_THREADS + 1);
> +
> +  for (i = 0; i < NUM_THREADS; i++)
> +    {
> +      pthread_create (&child_thread[i], NULL, child_function, NULL);
> +    }
> +
> +  for (i = 0; i < NUM_THREADS; i++)
> +    {
> +      pthread_join (child_thread[i], NULL);
> +    }
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.mi/thread-selected-sync.exp b/gdb/testsuite/gdb.mi/thread-selected-sync.exp
> new file mode 100644
> index 0000000..c36d018
> --- /dev/null
> +++ b/gdb/testsuite/gdb.mi/thread-selected-sync.exp
> @@ -0,0 +1,1021 @@
> +# Copyright 2016 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# This test checks that thread, select-frame, frame or inferior selection
> +# events are properly sent to all uis.
> +#
> +# This test considers the case where console and mi are two different uis
> +# and mi is created with the new-ui command.
> +#
> +# It also considers the case were the console commands are sent directly in
> +# the mi channel as described in PR 20487.

s/were/where/

> +#
> +# It does so by starting 2 inferiors with 3 threads each.
> +# Thread 1 is the main thread starting the others.
> +# Thread 2 is stopped in non-stop mode.
> +# Thread 3 is the thread used for testing, in non-stop mode this thread is running.
> +
> +load_lib mi-support.exp
> +
> +standard_testfile
> +
> +# Multiple inferiors are needed, therefore only native gdb and extended
> +# gdbserver modes are supported.

Could we restrict this to the specific tests that need multiple inferiors?
I think you'd just need to guard the creation of the second inferior
in test_setup, and then make test_*_select_inferior bail out with
unsupported or some such.

> +if [use_gdb_stub] {
> +    untested ${testfile}.exp
> +    return
> +}
> +
> +set compile_options "debug pthreads"
> +if {[build_executable $testfile.exp $testfile ${srcfile} ${compile_options}] == -1} {
> +    untested "failed to compile $testfile"
> +    return -1
> +}
> +
> +set bp_lineno [gdb_get_line_number "set break here"]
> +set caller_lineno [gdb_get_line_number "caller"]
> +
> +# Make a regular expression for cli.

It'd be nice to extend this comment.  What does the
regular expression match?  What are the parameters for?

> +
> +proc make_cli_re {mode inf thread frame lineend} {
> +    global srcfile bp_lineno caller_lineno
> +
> +    set inf_re "\\\[Switching to inferior $inf.*\\\]"
> +    set all_stop_thread_re "\\\[Switching to thread $thread.*\\\]"
> +    set non_stop_thread_re "$all_stop_thread_re\\\(running\\\)"
> +    set frame_re(0) "#0.*child_sub_function.*$srcfile:$bp_lineno\[\r\n\]+.*set break here \\\*/"
> +    set frame_re(1) "#1.*child_function \\\(args=0x0\\\) at .*$srcfile:$caller_lineno\[\r\n\]+$caller_lineno.*caller \\\*/"
> +    # Special frame for main threads.
> +    set frame_re(2) "#0.*"
> +    set end "\[\r\n\]"
> +
> +    set cli_re ""
> +
> +    if {$inf != -1} {
> +	append cli_re $inf_re
> +    }
> +
> +    if {$thread != "-1"} {
> +	if {$inf != -1} {
> +	    append cli_re "\[\r\n\]+"
> +	}
> +	set thread_re ""
> +	if {$mode == "all-stop"} {
> +	    set thread_re $all_stop_thread_re
> +	} elseif {$mode == "non-stop"} {
> +	    set thread_re $non_stop_thread_re
> +	}
> +	append cli_re $thread_re
> +    }
> +
> +    if {$mode == "all-stop" && $frame != -1} {
> +	if {$thread != -1} {
> +	    append cli_re "\[\r\n\]+"
> +	}
> +	append cli_re $frame_re($frame)
> +    }
> +
> +    if {$lineend == 1} {
> +	append cli_re $end
> +    }
> +
> +    return $cli_re
> +}
> +
> +# Make a regular expression for mi.
> +

Ditto.

> +proc make_mi_re {mode thread event frame lineend} {
> +    global srcfile bp_lineno caller_lineno hex
> +
> +    set mi_re ""
> +    set thread_event_re "=thread-selected,id=\"$thread\""
> +    set thread_answer_re ".*\\^done,new-thread-id=\"$thread\""
> +    set frame_re(0) ",frame=\{level=\"0\",addr=\"$hex\",func=\"child_sub_function\",args=\\\[\\\],file=\".*$srcfile\",.*line=\"$bp_lineno\"\}"
> +    set frame_re(1) ",frame=\{level=\"1\",addr=\"$hex\",func=\"child_function\",args=\\\[\{name=\"args\",value=\"0x0\"\}\\\],file=\".*$srcfile\",.*line=\"$caller_lineno\"\}"
> +    #Special frame for main thread.
> +    set frame_re(2) ",frame=\{level=\"0\",addr=\"$hex\",func=\".*\",args=\\\[\\\],from=\".*\"\}"
> +    set end "\[\r\n\]"
> +
> +    if {$thread != "-1"} {
> +	if {$event == 1} {
> +	    append mi_re $thread_event_re
> +	} elseif {$event == 0} {
> +	    append mi_re $thread_answer_re
> +	}
> +    }
> +
> +    if {$mode == "all-stop" && $frame != -1} {
> +	append mi_re $frame_re($frame)
> +    }
> +
> +    if {$lineend == 1} {
> +	append mi_re $end
> +    }
> +
> +    return $mi_re
> +}
> +
> +# Make a regular expression for cli in mi.
> +
> +proc make_cli_in_mi_re {command mode inf cli_thread mi_thread event frame lineend} {
> +    global srcfile bp_lineno caller_lineno
> +
> +    set cli_in_mi_re ".*$command.*\[\r\n\]+"
> +    set frame_re(0) "~\"#0.*child_sub_function.*$srcfile:$bp_lineno\\\\n.*set break here \\\*/\\\\n\"\[\r\n\]+"
> +    set frame_re(1) "~\"#1.*child_function \\\(args=0x0\\\) at .*$srcfile:$caller_lineno\\\\n\"\[\r\n\]+~\"$caller_lineno.*caller \\\*/\\\\n\"\[\r\n\]+"
> +    set frame_re(2) "~\"#0.*\\\\n\"\[\r\n\]+"
> +
> +    if {$inf != -1} {
> +	append cli_in_mi_re "~\""
> +	append cli_in_mi_re [make_cli_re $mode $inf "-1" -1 0]
> +	append cli_in_mi_re "\\\\n\"\[\r\n\]+"
> +    }
> +
> +    if {$cli_thread != "-1"} {
> +	append cli_in_mi_re "~\""
> +	append cli_in_mi_re [make_cli_re $mode -1 $cli_thread -1 0]
> +	append cli_in_mi_re "\\\\n\"\[\r\n\]+"
> +    }
> +
> +    if {$mode == "all-stop"} {
> +	append cli_in_mi_re $frame_re($frame)
> +    }
> +
> +    if {$event != -1} {
> +	append cli_in_mi_re [make_mi_re $mode $mi_thread $event $frame $lineend]
> +    }
> +    append cli_in_mi_re "\[\r\n\]+\\^done"
> +
> +    return $cli_in_mi_re
> +}
> +
> +# Continue to the breakpoint indicating the start position of the threads.
> +
> +proc test_continue_to_start {mode inf} {
> +    global gdb_prompt mi_spawn_id
> +
> +    if {$mode == "all-stop"} {
> +	for {set i 1} { $i <= 2 } { incr i } {
> +	    gdb_continue_to_breakpoint "barrier breakpoint"
> +	}
> +    } else {
> +	set test "continue&"
> +
> +	gdb_test_multiple $test $test {
> +	    -re "Continuing\\.\[\r\n\]+$gdb_prompt " {
> +		pass $test
> +	    }
> +	}
> +
> +	# Wait until we've hit the breakpoint for the 2 threads.
> +	for {set i 1} { $i <= 2 } { incr i } {
> +	    set test "thread $i started"
> +	    gdb_test_multiple "" $test {
> +		-re "hit Breakpoint" {
> +		    # The prompt was already matched in the "continue &"
> +		    # test above.  We're now consuming asynchronous output
> +		    # that comes after the prompt.
> +		    pass $test
> +		}
> +	    }
> +	}
> +	# Switch to the test thread.
> +	if {$inf == 1} {
> +	    gdb_test "thread 3" [make_cli_re "all-stop" -1 "3" 0 -1] "Switch to test thread"
> +	} else {
> +	    gdb_test "thread 2.3" [make_cli_re "all-stop" -1 "2\\.3" 0 -1] "Switch to test thread"

How came these pass "all-stop", when you're in the "non-stop" if/then branch?


> +	}


> +
> +	gdb_test "continue&" "Continuing\\."
> +    }
> +}
> +
> +# Test context setup.
> +
> +proc test_setup {mode} {
> +    global srcfile srcdir subdir testfile
> +    global gdb_main_spawn_id mi_spawn_id
> +    global decimal binfile bp_lineno
> +    global GDBFLAGS
> +
> +    mi_gdb_exit
> +
> +    set saved_gdbflags $GDBFLAGS
> +
> +    if {$mode == "non-stop"} {
> +	set GDBFLAGS [concat $GDBFLAGS " -ex \"set non-stop 1\""]
> +    }
> +
> +    if {[mi_gdb_start "separate-mi-tty"] != 0} {
> +	return
> +    }
> +
> +    mi_delete_breakpoints
> +    mi_gdb_reinitialize_dir $srcdir/$subdir
> +    mi_gdb_load $binfile
> +
> +    if {[mi_runto main] < 0} {
> +	fail "Can't run to main"
> +	return
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +
> +	gdb_test "break $srcfile:$bp_lineno" \
> +	    "Breakpoint $decimal .*$srcfile, line $bp_lineno\\\." \
> +	    "set breakpoint"
> +
> +	test_continue_to_start $mode 1
> +
> +	# Add a second inferior to test inferior selection.
> +	gdb_test "add-inferior" "Added inferior 2" "Add inferior 2"
> +	gdb_test "inferior 2" [make_cli_re $mode 2 "-1" -1 -1]
> +	gdb_load ${binfile}
> +	gdb_test "start" "Temporary breakpoint.*Starting program.*"
> +	test_continue_to_start $mode 2
> +	gdb_test "inferior 1" [make_cli_re $mode 1 "1\\.1" 2 -1]
> +    }
> +
> +    set GDBFLAGS $saved_gdbflags

There are early returns before you reach here.
Use 'save_vars { GDBFLAGS } { ... }' around the mi_gdb_start
call instead.

> +}
> +
> +# Test selecting an inferior from cli.
> +
> +proc test_cli_select_inferior {mode} {
> +    global gdb_main_spawn_id mi_spawn_id
> +
> +    set mi_re [make_mi_re $mode "4" 1 2 1]
> +    set cli_re [make_cli_re $mode 2 "2\\.1" 2 0]
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	gdb_test "inferior 2" $cli_re "cli select inferior"
> +    }
> +
> +    with_spawn_id $mi_spawn_id {
> +	set test "mi select inferior"
> +	gdb_test_multiple "" $test {
> +	    -re $mi_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +}
> +
> +# Test thread selection from cli.
> +
> +proc test_cli_select_thread {mode} {
> +    global gdb_main_spawn_id mi_spawn_id
> +
> +    set mi_re [make_mi_re $mode "3" 1 0 1]
> +    set cli_re [make_cli_re $mode -1 "1\\.3" 0 0]
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	gdb_test "thread 1.3" $cli_re "cli select thread"
> +    }
> +
> +    with_spawn_id $mi_spawn_id {
> +	set test "mi select thread"
> +	gdb_test_multiple "" $test {
> +	    -re $mi_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +}
> +
> +# Test selecting the same thread twice from the cli.
> +
> +proc test_cli_select_thread_twice {mode} {
> +    global gdb_main_spawn_id mi_spawn_id
> +
> +    set mi_re [make_mi_re $mode "3" 1 0 1]
> +    set cli_re [make_cli_re $mode -1 "1\\.3" 0 0]
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	gdb_test "thread 1.3" $cli_re "cli select thread twice, first call"
> +    }
> +
> +    with_spawn_id $mi_spawn_id {
> +	set test "mi select thread twice, first call"
> +	gdb_test_multiple "" $test {
> +	    -re $mi_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +
> +    # No event for that one.
> +    set mi_re ""
> +    set cli_re [make_cli_re $mode -1 "1\\.3" 0 0]
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	gdb_test "thread 1.3" $cli_re "cli select thread twice, second call"
> +    }
> +
> +    with_spawn_id $mi_spawn_id {
> +	set test "mi select thread twice, second call"
> +	gdb_test_multiple "" $test {
> +	    -re $mi_re {

"$mi_re" is empty here, so this doesn't look it's actually
testing anything?  Several other instances in other tests.

See ensure_no_output in new-ui.exp for an idea of how to
check that no output has been sent.

I don't think we should be using gdb_test_multiple on MI.
It's mostly useless, compared to gdb_expect, since
its internal matchings expect CLI's "$gdb_prompt $", which
won't match in MI.  Ideally we'd refactor out something
similar to gdb_test_multiple out of mi_gdb_test (and then
even try to factor out common bits between the result and
gdb_test_multiple), but nobody's ever bothered.  So you
get to use gdb_expect directly.

> +		pass $test
> +	    }
> +	}
> +    }
> +
> +}
> +
> +# Test frame selection from cli.
> +
> +proc test_cli_select_frame {mode frame} {
> +    global gdb_main_spawn_id mi_spawn_id
> +
> +    if {$mode == "all-stop"} {
> +	set mi_re [make_mi_re $mode "3" 1 $frame 1]
> +	set cli_re [make_cli_re $mode -1 "-1" $frame 0]
> +    } elseif {$mode == "non-stop"} {
> +	set cli_re "Selected thread is running\\."
> +	# No output
> +	set mi_re ""
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	gdb_test "frame $frame" $cli_re "cli select frame"
> +    }
> +
> +    with_spawn_id $mi_spawn_id {
> +	set test "mi select frame"
> +	gdb_test_multiple "" $test {
> +	    -re $mi_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +}
> +
> +# Test frame selection from cli with the select-frame command.
> +
> +proc test_cli_select_select_frame {mode frame} {
> +    global gdb_main_spawn_id mi_spawn_id
> +
> +    if {$mode == "all-stop"} {
> +	set cli_re ""
> +	set mi_re [make_mi_re $mode "3" 1 $frame 1]
> +    } elseif {$mode == "non-stop"} {
> +	set cli_re "Selected thread is running\\."
> +	# No output
> +	set mi_re ""
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	if {$cli_re != ""} {
> +	    gdb_test "select-frame $frame" $cli_re "cli select frame with select-frame"
> +	} else {
> +	    gdb_test_no_output "select-frame $frame" $cli_re "cli select frame with select-frame"
> +	}
> +    }
> +
> +    with_spawn_id $mi_spawn_id {
> +	set test "mi select frame with select-frame"
> +	gdb_test_multiple "" $test {
> +	    -re $mi_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +}
> +
> +# Test doing and up and then down command from cli.

"an up", I guess.

> +
> +proc test_cli_up_down {mode} {
> +    global gdb_main_spawn_id mi_spawn_id
> +
> +    if {$mode == "all-stop"} {
> +	set cli_up_re [make_cli_re $mode -1 "-1" 1 0]
> +	set mi_up_re [make_mi_re $mode "3" 1 1 1]
> +	set cli_down_re [make_cli_re $mode -1 "-1" 0 0]
> +	set mi_down_re [make_mi_re $mode "3" 1 0 1]
> +    } elseif {$mode == "non-stop"} {
> +	set cli_up_re "No stack\\."
> +	set mi_up_re ""
> +	set cli_down_re "No stack\\."
> +	set mi_down_re ""
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	gdb_test "up" $cli_up_re "cli up"
> +    }
> +
> +    with_spawn_id $mi_spawn_id {
> +	set test "mi up"
> +	gdb_test_multiple "" $test {
> +	    -re $mi_up_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	gdb_test "down" $cli_down_re "cli down"
> +    }
> +
> +    with_spawn_id $mi_spawn_id {
> +	set test "mi down"
> +	gdb_test_multiple "" $test {
> +	    -re $mi_down_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +}
> +
> +# Test same frame selection from cli.
> +
> +proc test_cli_select_frame_twice {mode frame} {
> +    global gdb_main_spawn_id mi_spawn_id
> +
> +    if {$mode == "all-stop"} {
> +	set mi_re [make_mi_re $mode "3" 1 $frame 1]
> +	set cli_re [make_cli_re $mode -1 "-1" $frame 0]
> +    } elseif {$mode == "non-stop"} {
> +	set cli_re "Selected thread is running\\."
> +	# No output
> +	set mi_re ""
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	gdb_test "frame $frame" $cli_re "cli select frame twice, first call"
> +    }
> +
> +    with_spawn_id $mi_spawn_id {
> +	set test "mi select frame twice, first call"
> +	gdb_test_multiple "" $test {
> +	    -re $mi_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +
> +    if {$mode == "all-stop"} {
> +	#No output thread has not changed.

Did you mean to put a period or something else after "No output"?
Reads strange as is.

> +	set mi_re ""
> +	set cli_re [make_cli_re $mode -1 "-1" $frame 0]
> +    } elseif {$mode == "non-stop"} {
> +	set cli_re "Selected thread is running\\."
> +	# No output
> +	set mi_re ""
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	gdb_test "frame $frame" $cli_re "cli select frame twice, second call"
> +    }
> +
> +    with_spawn_id $mi_spawn_id {
> +	set test "mi select frame, second call"
> +	gdb_test_multiple "" $test {
> +	    -re $mi_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +}
> +
> +# Test thread command without any arguments fromt the cli.

"from".

> +
> +proc test_cli_select_thread_no_args {mode} {
> +    global gdb_main_spawn_id mi_spawn_id
> +
> +    set cli_re "\\\[Current thread is 1\\.3.*\\\]"
> +    set mi_re ""
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	gdb_test "thread" $cli_re "cli thread no args"
> +    }
> +
> +    with_spawn_id $mi_spawn_id {
> +	set test "mi thread no args"
> +	gdb_test_multiple "" $test {
> +	    -re $mi_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +}
> +
> +# Test frame command without any arguments from the cli.
> +
> +proc test_cli_select_frame_no_args {mode} {
> +    global gdb_main_spawn_id mi_spawn_id
> +
> +    if {$mode == "all-stop"} {
> +	set mi_re ""
> +	set cli_re [make_cli_re $mode -1 "-1" 0 0]
> +    } elseif {$mode == "non-stop"} {
> +	set cli_re "No stack\\."
> +	# No output
> +	set mi_re ""
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	gdb_test "frame" $cli_re "cli frame no args"
> +    }
> +
> +    with_spawn_id $mi_spawn_id {
> +	set test "mi frame no args"
> +	gdb_test_multiple "" $test {
> +	    -re $mi_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +}
> +
> +
> +# Test selecting a thread from mi with a thread group.  This test verifies
> +# that even if the thread GDB would switch to is the same has the
> +# thread-group selected thread, that an event is still sent to cli.
> +# In this case this is thread 1.2
> +
> +proc test_mi_select_thread_with_thread_group {mode} {
> +
> +    # This only applies to non-stop mode.
> +    if {$mode == "all-stop"} {
> +	return;
> +    }
> +    global gdb_main_spawn_id mi_spawn_id
> +
> +    set mi_re [make_mi_re "all-stop" "2" 0 0 0]
> +    set cli_re [make_cli_re "all-stop" -1 "1\\.2" 0 1]
> +
> +    with_spawn_id $mi_spawn_id {
> +	mi_gdb_test "-thread-select --thread-group i1 2" $mi_re "mi select thread with thread group"
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	set test "cli select thread with thread group"
> +	gdb_test_multiple "" $test {
> +	    -re $cli_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +}
> +
> +# Test selecting a thread from mi.
> +
> +proc test_mi_select_thread {mode} {
> +    global gdb_main_spawn_id mi_spawn_id
> +
> +    set mi_re [make_mi_re $mode "3" 0 0 0]
> +    set cli_re [make_cli_re $mode -1 "1\\.3" 0 1]
> +
> +    with_spawn_id $mi_spawn_id {
> +	mi_gdb_test "-thread-select 3" $mi_re "mi select thread"
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	set test "cli select thread"
> +	gdb_test_multiple "" $test {
> +	    -re $cli_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +}
> +
> +# Test selecting the same thread from mi.
> +
> +proc test_mi_select_thread_twice {mode} {
> +    global gdb_main_spawn_id mi_spawn_id
> +
> +    set mi_re [make_mi_re $mode "3" 0 0 0]
> +    set cli_re [make_cli_re $mode -1 "1\\.3" 0 1]
> +
> +    with_spawn_id $mi_spawn_id {
> +	mi_gdb_test "-thread-select 3" $mi_re "mi select thread twice, first call"
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	set test "cli select thread twice, frist call"
> +	gdb_test_multiple "" $test {
> +	    -re $cli_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +
> +    set mi_re [make_mi_re $mode "3" 0 0 0]
> +    # No event here.
> +    set cli_re ""
> +
> +    with_spawn_id $mi_spawn_id {
> +	mi_gdb_test "-thread-select 3" $mi_re "mi select thread twice, second call"
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	set test "cli select thread twice, second call"
> +	gdb_test_multiple "" $test {
> +	    -re $cli_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +}
> +
> +# Test selecting a frame from mi.
> +
> +proc test_mi_select_frame {mode frame} {
> +    global gdb_main_spawn_id mi_spawn_id
> +
> +    if {$mode == "all-stop"} {
> +        set cli_re [make_cli_re $mode -1 "-1" $frame 1]
> +	set mi_re ".*\\^done"
> +    } elseif {$mode == "non-stop"} {
> +	# No output
> +	set cli_re ""
> +	set mi_re ".*\\^error,msg=\"Selected thread is running\\.\""
> +    }
> +
> +    with_spawn_id $mi_spawn_id {
> +	mi_gdb_test "-stack-select-frame $frame" $mi_re "mi select frame"
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	set test "cli select frame"
> +	gdb_test_multiple "" $test {
> +	    -re $cli_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +}
> +
> +# Test selecting the same frame from mi.
> +
> +proc test_mi_select_frame_twice {mode frame} {
> +    global gdb_main_spawn_id mi_spawn_id
> +
> +    if {$mode == "all-stop"} {
> +        set cli_re [make_cli_re $mode -1 "-1" $frame 1]
> +	set mi_re ".*\\^done"
> +    } elseif {$mode == "non-stop"} {
> +	# No output
> +	set cli_re ""
> +	set mi_re ".*\\^error,msg=\"Selected thread is running\\.\""
> +    }
> +
> +    with_spawn_id $mi_spawn_id {
> +	mi_gdb_test "-stack-select-frame $frame" $mi_re "mi select frame twice, frist call"


"first"

> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	set test "cli select frame twice, frist call"


"first"

> +	gdb_test_multiple "" $test {
> +	    -re $cli_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +
> +    if {$mode == "all-stop"} {
> +        set cli_re ""
> +	set mi_re ".*\\^done"
> +    } elseif {$mode == "non-stop"} {
> +	# No output
> +	set cli_re ""
> +	set mi_re ".*\\^error,msg=\"Selected thread is running\\.\""
> +    }
> +
> +    with_spawn_id $mi_spawn_id {
> +	mi_gdb_test "-stack-select-frame $frame" $mi_re "mi select frame twice, second call"
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	set test "cli select frame twice, second call"
> +	gdb_test_multiple "" $test {
> +	    -re $cli_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +
> +}
> +
> +# Test selecting the inferior using a cli command in the mi channel.
> +
> +proc test_cli_in_mi_select_inferior {mode exec_mode} {
> +    global gdb_main_spawn_id mi_spawn_id
> +
> +    if {$exec_mode == "interpreter-exec"} {
> +	set command "-interpreter-exec console \"inferior 2\""
> +    } else {
> +	set command "inferior 2"
> +    }
> +
> +    set mi_re [make_cli_in_mi_re [string_to_regexp $command] $mode 2 "2\\.1" "4" 1 2 0]
> +    set cli_re [make_cli_re $mode 2 "2\\.1" 2 1]
> +
> +    with_spawn_id $mi_spawn_id {
> +	mi_gdb_test $command $mi_re "mi select inferior"
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	set test "cli select inferior"
> +	gdb_test_multiple "" $test {
> +	    -re $cli_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +}
> +
> +# Test selecting the thread using a cli command in the mi channel.
> +
> +proc test_cli_in_mi_select_thread {mode exec_mode} {
> +    global gdb_main_spawn_id mi_spawn_id
> +
> +    if {$exec_mode == "interpreter-exec"} {
> +	set command "-interpreter-exec console \"thread 1.3\""
> +    } else {
> +	set command "thread 1.3"
> +    }
> +
> +    set mi_re [make_cli_in_mi_re [string_to_regexp $command] $mode -1 "1\\.3" "3" 1 0 0]
> +    set cli_re [make_cli_re $mode -1 "1\\.3" 0 1]
> +
> +    with_spawn_id $mi_spawn_id {
> +	mi_gdb_test $command $mi_re "mi select thread"
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	set test "cli select thread"
> +	gdb_test_multiple "" $test {
> +	    -re $cli_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +}
> +
> +# Test selecting the frame using a cli command in the mi channel.
> +
> +proc test_cli_in_mi_select_frame {mode exec_mode frame} {
> +    global gdb_main_spawn_id mi_spawn_id
> +
> +    if {$exec_mode == "interpreter-exec"} {
> +	set command "-interpreter-exec console \"frame $frame\""
> +    } else {
> +	set command "frame $frame"
> +    }
> +
> +    if {$mode == "all-stop"} {
> +	set cli_re [make_cli_re $mode -1 "-1" $frame 1]
> +	set mi_re [make_cli_in_mi_re [string_to_regexp $command] $mode -1 "-1" 3 1 $frame 0]
> +    } elseif {$mode == "non-stop"} {
> +	set cli_re ""
> +	set mi_re ".*\\^error,msg=\"Selected thread is running\\.\""
> +    }
> +
> +    with_spawn_id $mi_spawn_id {
> +	mi_gdb_test $command $mi_re "mi select frame"
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	set test "cli select frame"
> +	gdb_test_multiple "" $test {
> +	    -re $cli_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +}
> +
> +# Test selecting the same thread using a cli command in the mi channel.
> +
> +proc test_cli_in_mi_select_thread_twice {mode exec_mode} {
> +    global gdb_main_spawn_id mi_spawn_id
> +
> +    if {$exec_mode == "interpreter-exec"} {
> +	set command "-interpreter-exec console \"thread 1.3\""
> +    } else {
> +	set command "thread 1.3"
> +    }
> +
> +    set mi_re [make_cli_in_mi_re [string_to_regexp $command] $mode -1 "1\\.3" "3" 1 0 0]
> +    set cli_re [make_cli_re $mode -1 "1\\.3" 0 1]
> +
> +    with_spawn_id $mi_spawn_id {
> +	mi_gdb_test $command $mi_re "mi select thread twice, first call"
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	set test "cli select thread twice, first call"
> +	gdb_test_multiple "" $test {
> +	    -re $cli_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +
> +    set mi_re [make_cli_in_mi_re [string_to_regexp $command] $mode -1 "1\\.3" "3" -1 0 0]
> +    set cli_re ""
> +
> +    with_spawn_id $mi_spawn_id {
> +	mi_gdb_test $command $mi_re "mi select thread twice, second call"
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	set test "cli select thread twice, second call"
> +	gdb_test_multiple "" $test {
> +	    -re $cli_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +
> +}
> +
> +# Test selecting the same frame using a cli command in the mi channel.
> +
> +proc test_cli_in_mi_select_frame_twice {mode exec_mode frame} {
> +    global gdb_main_spawn_id mi_spawn_id
> +
> +    if {$exec_mode == "interpreter-exec"} {
> +	set command "-interpreter-exec console \"frame $frame\""
> +    } else {
> +	set command "frame $frame"
> +    }
> +
> +    if {$mode == "all-stop"} {
> +	set cli_re [make_cli_re $mode -1 "-1" $frame 1]
> +	set mi_re [make_cli_in_mi_re [string_to_regexp $command] $mode -1 "-1" 3 1 $frame 0]
> +    } elseif {$mode == "non-stop"} {
> +	set cli_re ""
> +	set mi_re ".*\\^error,msg=\"Selected thread is running\\.\""
> +    }
> +
> +    with_spawn_id $mi_spawn_id {
> +	mi_gdb_test $command $mi_re "mi select frame twice, first call"
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	set test "cli select frame twice, first call"
> +	gdb_test_multiple "" $test {
> +	    -re $cli_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +
> +    if {$mode == "all-stop"} {
> +	set cli_re ""
> +	set mi_re [make_cli_in_mi_re [string_to_regexp $command] $mode -1 "-1" 3 -1 $frame 0]
> +    } elseif {$mode == "non-stop"} {
> +	set cli_re ""
> +	set mi_re ".*\\^error,msg=\"Selected thread is running\\.\""
> +    }
> +
> +    with_spawn_id $mi_spawn_id {
> +	mi_gdb_test $command $mi_re "mi select frame twice, second call"
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	set test "cli select frame twice, second call"
> +	gdb_test_multiple "" $test {
> +	    -re $cli_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +
> +}
> +
> +# Test printing the current thread using a cli command in the mi channel .
> +
> +proc test_cli_in_mi_select_thread_no_args {mode exec_mode} {
> +    global gdb_main_spawn_id mi_spawn_id
> +
> +    if {$exec_mode == "interpreter-exec"} {
> +	set command "-interpreter-exec console \"thread\""
> +    } else {
> +	set command "thread"
> +    }
> +
> +    set mi_re ".*$command.*\[\r\n\]+"
> +    append mi_re "~\"\\\[Current thread is 1\\.3.*\\\]\\\\n\""
> +    append mi_re "\[\r\n\]+\\^done"
> +
> +    set cli_re ""
> +
> +    with_spawn_id $mi_spawn_id {
> +	mi_gdb_test $command $mi_re "mi select thread no args"
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	set test "cli select thread no args"
> +	gdb_test_multiple "" $test {
> +	    -re $cli_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +}
> +
> +# Test printing the current frame using a cli command in the mi channel .
> +
> +proc test_cli_in_mi_select_frame_no_args {mode exec_mode frame} {
> +    global gdb_main_spawn_id mi_spawn_id
> +
> +    if {$exec_mode == "interpreter-exec"} {
> +	set command "-interpreter-exec console \"frame\""
> +    } else {
> +	set command "frame"
> +    }
> +
> +    if {$mode == "all-stop"} {
> +	set cli_re ""
> +	set mi_re [make_cli_in_mi_re "frame" $mode -1 "-1" -1 -1 $frame 0]
> +    } elseif {$mode == "non-stop"} {
> +	set cli_re ""
> +	set mi_re ".*\\^error,msg=\"No stack\\.\""
> +    }
> +
> +    with_spawn_id $mi_spawn_id {
> +	mi_gdb_test $command $mi_re "mi select frame"
> +    }
> +
> +    with_spawn_id $gdb_main_spawn_id {
> +	set test "cli select frame"
> +	gdb_test_multiple "" $test {
> +	    -re $cli_re {
> +		pass $test
> +	    }
> +	}
> +    }
> +}
> +
> +# Test all-stop and non-stop mode.
> +
> +foreach mode {"all-stop" "non-stop"} {
> +    with_test_prefix $mode {
> +	test_setup $mode
> +	with_test_prefix "from cli" {
> +	    test_cli_select_inferior $mode
> +	    test_cli_select_thread $mode
> +	    test_cli_select_frame $mode 1
> +	    # Reset the frame for up/down test.
> +	    test_cli_select_frame $mode 0
> +	    test_cli_select_select_frame $mode 1
> +	    test_cli_select_select_frame $mode 0
> +	    test_cli_up_down $mode
> +	    # Reset thread/frame.
> +	    test_cli_select_inferior $mode
> +	    test_cli_select_thread_twice $mode
> +	    test_cli_select_frame_twice $mode 1
> +	    # Reset frame to 0.
> +	    test_cli_select_frame $mode 0
> +	    test_cli_select_frame_no_args $mode
> +	    test_cli_select_thread_no_args $mode
> +	}
> +	with_test_prefix "from mi" {
> +	    # Reset thread/frame.
> +	    test_cli_select_inferior $mode
> +	    test_mi_select_thread_with_thread_group $mode
> +	    test_mi_select_thread $mode
> +	    test_mi_select_frame $mode 1
> +	    test_mi_select_frame $mode 0
> +	    # Reset thread/frame.
> +	    test_cli_select_inferior $mode
> +	    test_mi_select_thread_twice $mode
> +	    test_mi_select_frame_twice $mode 1
> +	}
> +
> +	# Test with a direct command from cli in mi like "thread 1"
> +	# Or test with the interpreter-exec like -interpreter-exec "thread 1"
> +	foreach exec_mode {"from cli in mi" "interpreter-exec"} {
> +	    with_test_prefix $exec_mode {
> +		test_cli_in_mi_select_inferior $mode $exec_mode
> +		test_cli_in_mi_select_thread $mode $exec_mode
> +		test_cli_in_mi_select_frame $mode $exec_mode 1 
> +		test_cli_in_mi_select_frame $mode $exec_mode 0
> +		# Reset thread/frame.
> +		test_cli_in_mi_select_inferior $mode $exec_mode
> +		test_cli_in_mi_select_thread_twice $mode $exec_mode
> +		test_cli_in_mi_select_frame_twice $mode $exec_mode 1
> +		test_cli_in_mi_select_frame $mode $exec_mode 0
> +		test_cli_in_mi_select_thread_no_args $mode $exec_mode
> +		test_cli_in_mi_select_frame_no_args $mode $exec_mode 0
> +	    }
> +	}
> +    }
> +}
> diff --git a/gdb/thread.c b/gdb/thread.c
> index ab98777..f3c4879 100644
> --- a/gdb/thread.c
> +++ b/gdb/thread.c
> @@ -1923,6 +1923,9 @@ thread_apply_command (char *tidlist, int from_tty)
>  void
>  thread_command (char *tidstr, int from_tty)
>  {
> +  ptid_t previous_ptid = inferior_ptid;
> +  enum gdb_rc result;
> +
>    if (!tidstr)
>      {
>        if (ptid_equal (inferior_ptid, null_ptid))
> @@ -1946,7 +1949,23 @@ thread_command (char *tidstr, int from_tty)
>        return;
>      }
>  
> -  gdb_thread_select (current_uiout, tidstr, NULL);
> +  result = gdb_thread_select (current_uiout, tidstr, NULL);
> +
> +  /* If thread switch did not succeed don't notify or print.  */
> +  if (result == GDB_RC_FAIL)
> +    return;
> +
> +  /* Print if the thread has not changed, otherwise an event will be sent.  */
> +  if (ptid_equal (inferior_ptid, previous_ptid))
> +    {
> +      print_selected_thread_frame (current_uiout, USER_SELECTED_THREAD
> +				   | USER_SELECTED_FRAME);

The "|" should be aligned under "USER_", and you'd need an extra set of
parens to make emacs happy:

      print_selected_thread_frame (current_uiout, (USER_SELECTED_THREAD
						   | USER_SELECTED_FRAME));

but in this case it's more readable IMO to put the USER_SELECTED_THREAD
on the next line too:

      print_selected_thread_frame (current_uiout,
				   USER_SELECTED_THREAD | USER_SELECTED_FRAME);


> +    }
> +  else
> +    {
> +      observer_notify_user_selected_inf_thread_frame (USER_SELECTED_THREAD
> +						      | USER_SELECTED_FRAME);

This one's fine.

> +    }
>  }
>  
>  /* Implementation of `thread name'.  */
> @@ -2058,32 +2077,53 @@ do_captured_thread_select (struct ui_out *uiout, void *tidstr_v)
>  
>    annotate_thread_changed ();
>  
> -  if (ui_out_is_mi_like_p (uiout))
> -    ui_out_field_int (uiout, "new-thread-id", inferior_thread ()->global_num);
> -  else
> +  /* Since the current thread may have changed, see if there is any
> +     exited thread we can now delete.  */
> +  prune_threads ();
> +
> +  return GDB_RC_OK;
> +}
> +
> +/* Print thread and frame switch command response.  */
> +
> +void
> +print_selected_thread_frame (struct ui_out *uiout,
> +			     user_selected_what selection)
> +{
> +  struct thread_info *tp = inferior_thread ();
> +  struct inferior *inf = current_inferior ();
> +
> +  if (selection & USER_SELECTED_THREAD)
>      {
> -      ui_out_text (uiout, "[Switching to thread ");
> -      ui_out_field_string (uiout, "new-thread-id", print_thread_id (tp));
> -      ui_out_text (uiout, " (");
> -      ui_out_text (uiout, target_pid_to_str (inferior_ptid));
> -      ui_out_text (uiout, ")]");
> +      if (ui_out_is_mi_like_p (uiout))
> +	{
> +	  ui_out_field_int (uiout, "new-thread-id",
> +			    inferior_thread ()->global_num);
> +	}
> +      else
> +	{
> +	  ui_out_text (uiout, "[Switching to thread ");
> +	  ui_out_field_string (uiout, "new-thread-id", print_thread_id (tp));
> +	  ui_out_text (uiout, " (");
> +	  ui_out_text (uiout, target_pid_to_str (inferior_ptid));
> +	  ui_out_text (uiout, ")]");
> +	}
>      }
>  
> -  /* Note that we can't reach this with an exited thread, due to the
> -     thread_alive check above.  */
>    if (tp->state == THREAD_RUNNING)
> -    ui_out_text (uiout, "(running)\n");
> -  else
>      {
> -      ui_out_text (uiout, "\n");
> -      print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC, 1);
> +      if (selection & USER_SELECTED_THREAD)
> +	ui_out_text (uiout, "(running)\n");
>      }
> +  else if (selection & USER_SELECTED_FRAME)
> +    {
> +      if (selection & USER_SELECTED_THREAD)
> +	ui_out_text (uiout, "\n");
>  
> -  /* Since the current thread may have changed, see if there is any
> -     exited thread we can now delete.  */
> -  prune_threads ();
> -
> -  return GDB_RC_OK;
> +      if (has_stack_frames ())
> +	print_stack_frame_to_uiout (uiout, get_selected_frame (NULL),
> +				    1, SRC_AND_LOC, 1);
> +    }
>  }
>  
>  enum gdb_rc
> diff --git a/gdb/tui/tui-interp.c b/gdb/tui/tui-interp.c
> index 3856382..22a5dab 100644
> --- a/gdb/tui/tui-interp.c
> +++ b/gdb/tui/tui-interp.c
> @@ -206,6 +206,35 @@ tui_on_command_error (void)
>    display_gdb_prompt (NULL);
>  }
>  
> +/* Observer for the user_selected_thread_frame notification.  */
> +
> +static void
> +tui_on_user_selected_inf_thread_frame (user_selected_what selection)
> +{
> +  struct switch_thru_all_uis state;
> +  struct thread_info *tp = find_thread_ptid (inferior_ptid);

Same comment as in CLI/MI.

> +
> +  /* This event is suppressed.  */
> +  if (cli_suppress_notification.user_selected_inf_thread_frame)
> +    return;
> +

I tried running the new test here, and got:

Running /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/thread-selected-sync.exp ...
FAIL: gdb.mi/thread-selected-sync.exp: all-stop: from cli: mi select inferior (timeout)
FAIL: gdb.mi/thread-selected-sync.exp: all-stop: from cli: mi select inferior (timeout)
FAIL: gdb.mi/thread-selected-sync.exp: all-stop: from mi: mi select inferior (timeout)
FAIL: gdb.mi/thread-selected-sync.exp: all-stop: from mi: mi select inferior (timeout)
FAIL: gdb.mi/thread-selected-sync.exp: all-stop: from cli in mi: mi select inferior
FAIL: gdb.mi/thread-selected-sync.exp: all-stop: from cli in mi: mi select inferior
FAIL: gdb.mi/thread-selected-sync.exp: all-stop: interpreter-exec: mi select inferior
FAIL: gdb.mi/thread-selected-sync.exp: all-stop: interpreter-exec: mi select inferior

gdb.log shows, for the first FAIL above:

[...]
~"[Switching to inferior 2 [process 10838] (/home/pedro/brno/pedro/gdb/mygit/build/gdb/testsuite/outputs/gdb.mi/thread-selected-sync/thread-selected-sync)]\n"
~"[Switching to thread 2.1 (Thread 0x7ffff7fc7700 (LWP 10838))]\n"
~"#0  0x00007ffff7bc66bd in pthread_join (threadid=140737342580480, thread_return=0x0) at pthread_join.c:90\n"
~"90\t    lll_wait_tid (pd->tid);\n"
=thread-selected,id="4",frame={level="0",addr="0x00007ffff7bc66bd",func="pthread_join",args=[{name="threadid",value="140737342580480"},{name="thread_return",value="0x0"}],file="pthread_join.c",fullname="/usr/src/debug/glibc-2.22/nptl/pthread_join.c",line="90"}
FAIL: gdb.mi/thread-selected-sync.exp: all-stop: from cli: mi select inferior (timeout)
[...]


Try to check with "make check-read1" too.


I also notice that the test has many duplicated test messages:
 https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Make_sure_test_messages_are_unique

Thanks,
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]