Re: [PATCH 9/9] gdb: Change how frames are selected for 'frame' and 'info frame'.

[Andrew Burgess <andrew dot burgess at embecosm dot com>]

Revised version of this patch.  Changes are:

  - Longer entry in NEWS file to address Eli's feedback.
  - Updated doc string for "frame", "select-frame", and "info frame"
    commands within gdb.  I forgot to do this in the first version of
    the patch.

Thanks,
Andrew

---

The 'frame' command, and thanks to code reuse the 'frame info' and
'select-frame' commands, currently have an overloaded mechanism for
selecting a frame.

These commands take one or two parameters, if it's one parameter then we
first try to use the parameter as an integer to select a frame by
index.  If that fails then we treat the parameter as an address and try
to select a stack frame by stack-address.  If we still have not selected
a stack frame, or we had two parameters initially, then we create a new
stack frame and select that.

The result of this is that a typo by the user, entering the wrong stack
frame index for example, can result in a brand new frame being created
rather than an error.

The purpose of this commit is to remove this overloading, while
hopefully offering some level of backward compatibility.

The 'frame', 'select-frame', and 'info frame' commands now all take a
frame specification string as an argument, this string can be any of the
following:

  (1) An integer.  This is treated as a frame number.  If a frame for
  that number does not exist then the user gets an error.

  (2) A string like 'level <NUMBER>', where <NUMBER> is a frame number
  as in option (1) above.

  (3) A string like 'address <ADDRESS>', where <ADDRESS> is a
  stack-frame address.  If there is no frame for this address then the
  user gets an error.

  (4) A string like 'function <NAME>', where <NAME> is a function name,
  the inner most frame for function <NAME> is selected.  If there is no
  frame for function <NAME> then the user gets an error.

  (5) A string like 'create <STACK-ADDRESS>', this creates a new frame
  with stack address <STACK-ADDRESS>.

  (6) A string like 'create <STACK-ADDRESS> <PC-ADDRESS>', this creates
  a new frame with stack address <STACK-ADDRESS> an the pc <PC-ADDRESS>.

This change assumes that the most common use of the commands like
'frame' is to select a frame by frame level number, it is for this
reason that this is the behaviour that is kept for backwards
compatibility.  Any of the alternative behaviours, which are assumed to
be less used, now require a change in user behaviour.

gdb/ChangeLog:

	* stack.c (find_frame_for_function): Add declaration.
	(find_frame_for_address): New function.
	(enum frame_select_mode): New enum.
	(FRAME_SELECTION_MAX_ARGS): New define.
	(struct frame_selection_spec): New structure.
	(is_frame_mode_string): New function.
	(parse_frame_selection_spec): New function.
	(free_frame_selection_spec): New function.
	(parse_frame_specification): Rewrite.
	(frame_info): Build argv for frame selection spec.
	(select_frame_from_spec): New function.
	(select_frame_command): Make static, build argv for frame
	selection spec.
	(frame_selection_completer): New function.
	(_initialize_stack): Add completion function, and update the help
	text for 'frame', 'select-frame', and 'info frame' commands.
	* stack.h (select_frame_command): Delete declaration.
	(select_frame_from_spec): New declaration.
	* mi/mi-cmd-stack.c (mi_cmd_stack_select_frame): Use
	select_frame_from_spec.  Catch errors.
	(NEWS): Mention changes to frame related commands.

gdb/doc/ChangeLog:

	* gdb.texinfo (Selection): Rewrite documentation for 'frame' and
	'select-frame' commands.
	(Frame Info): Rewrite documentation for 'info frame' command.

gdb/testsuite/ChangeLog:

	* gdb.base/frame-cmds.exp: Almost complete rewrite.
	* gdb.base/frame-cmds.c: Extend.
	* gdb.mi/mi-var-frame.exp: Update a few tests.  Add one new test.
---
 gdb/ChangeLog                         |  24 ++
 gdb/NEWS                              |  20 ++
 gdb/doc/ChangeLog                     |   6 +
 gdb/doc/gdb.texinfo                   |  84 +++++--
 gdb/mi/mi-cmd-stack.c                 |  16 +-
 gdb/stack.c                           | 436 +++++++++++++++++++++++++---------
 gdb/stack.h                           |   2 +-
 gdb/testsuite/ChangeLog               |   6 +
 gdb/testsuite/gdb.base/frame-cmds.c   |  20 +-
 gdb/testsuite/gdb.mi/mi-var-frame.exp |  17 +-
 10 files changed, 481 insertions(+), 150 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9da954d..c72ea81 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,29 @@
 2015-09-11  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* stack.c (find_frame_for_function): Add declaration.
+	(find_frame_for_address): New function.
+	(enum frame_select_mode): New enum.
+	(FRAME_SELECTION_MAX_ARGS): New define.
+	(struct frame_selection_spec): New structure.
+	(is_frame_mode_string): New function.
+	(parse_frame_selection_spec): New function.
+	(free_frame_selection_spec): New function.
+	(parse_frame_specification): Rewrite.
+	(frame_info): Build argv for frame selection spec.
+	(select_frame_from_spec): New function.
+	(select_frame_command): Make static, build argv for frame
+	selection spec.
+	(frame_selection_completer): New function.
+	(_initialize_stack): Add completion function, and update the help
+	text for 'frame', 'select-frame', and 'info frame' commands.
+	* stack.h (select_frame_command): Delete declaration.
+	(select_frame_from_spec): New declaration.
+	* mi/mi-cmd-stack.c (mi_cmd_stack_select_frame): Use
+	select_frame_from_spec.  Catch errors.
+	(NEWS): Mention changes to frame related commands.
+
+2015-09-11  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* stack.c (select_and_print_frame): Delete.
 	(func_command): Most content moved into new function
 	find_frame_for_function, use new function, print result, add
diff --git a/gdb/NEWS b/gdb/NEWS
index 0cf51e1..3d15d46 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,26 @@
 
 *** Changes since GDB 7.10
 
+* Changes to the "frame", "select-frame", and "info frame" commands.
+  Selecting frames by number remains unchanged, however, selecting
+  frames by stack-address, or creating new frames now requires the use
+  of a sub-command.  Various sub-commands now exist for the various
+  methods of selecting a frame; level, address, function, and create.
+  As an example, here are the variants of "frame" that are now available:
+  - frame <number>
+  - frame level <number>
+    Both of these select frame at level <number>.
+  - frame address <address>
+    Select frame for <address>.
+  - frame function <name>
+    Select inner most frame for function <name>.
+  - frame create <stack-address>
+    Create a frame for <stack-address>.
+  - frame create <stack-address> <pc-address>
+    Create a frame for <stack-address> <pc-address>.
+  There are similar variants for the "select-frame" and "info frame"
+  commands.
+
 * Support for tracepoints on aarch64-linux was added in GDBserver.
 
 * The 'record instruction-history' command now indicates speculative execution
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index f7d7a94..bb82cf9 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,5 +1,11 @@
 2015-09-11  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* gdb.texinfo (Selection): Rewrite documentation for 'frame' and
+	'select-frame' commands.
+	(Frame Info): Rewrite documentation for 'info frame' command.
+
+2015-09-11  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* gdb.texinfo (Frames): Remove 'frame' and 'select-frame'
 	description.
 	(Frame Filter Management): Move to later in the 'Examining the
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 225e57d..b27aaf8 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -7158,21 +7158,58 @@ of the stack frame just selected.
 @table @code
 @kindex frame@r{, selecting}
 @kindex f @r{(@code{frame})}
-@item frame @var{n}
-@itemx f @var{n}
-Select frame number @var{n}.  Recall that frame zero is the innermost
-(currently executing) frame, frame one is the frame that called the
-innermost one, and so on.  The highest-numbered frame is the one for
-@code{main}.
+@item frame @r{[} @var{frame-selection-spec} @r{]}
+@item f @r{[} @var{frame-selection-spec} @r{]}
+The @command{frame} command allows different stack frames to be
+selected.  The @var{frame-selection-spec} can be any of the following:
+
+@table @code
+@kindex frame level
+@item @var{num}
+@item level @var{num}
+Select frame number @var{num}.  Recall that frame zero is the
+innermost (currently executing) frame, frame one is the frame that
+called the innermost one, and so on.  The highest-numbered frame is
+the one for @code{main}.
+
+As this is the most common method of navigating the frame stack then
+the string @command{level} can be dropped, the following two commands
+are equivalent:
+
+@smallexample
+(@value{GDBP}) frame 3
+(@value{GDBP}) frame level 3
+@end smallexample
 
-@item frame @var{stack-addr} [ @var{pc-addr} ]
-@itemx f @var{stack-addr} [ @var{pc-addr} ]
-Select the frame at address @var{stack-addr}.  This is useful mainly if the
-chaining of stack frames has been damaged by a bug, making it
-impossible for @value{GDBN} to assign numbers properly to all frames.  In
-addition, this can be useful when your program has multiple stacks and
-switches between them.  The optional @var{pc-addr} can also be given to
-specify the value of PC for the stack frame.
+@kindex frame address
+@item address @var{stack-address}
+Select the frame with stack address @var{stack-address}.  Recall that
+each stack frame has a stack frame address, which roughly corresponds
+to the location on the stack where the stack frame local variables are
+stored.
+
+@kindex frame function
+@item function @var{function-name}
+Select the stack frame for function @var{function-name}.  If there are
+multiple stack frames for function @var{function-name} then the inner
+most stack frame is selected.
+
+@kindex frame create
+@item create @var{stack-address} @r{[} @var{pc-addr} @r{]}
+Create and then select a new frame at stack address @var{stack-addr}.
+This is useful mainly if the chaining of stack frames has been damaged
+by a bug, making it impossible for @value{GDBN} to assign numbers
+properly to all frames.  In addition, this can be useful when your
+program has multiple stacks and switches between them.  The optional
+@var{pc-addr} can also be given to specify the value of PC for the
+stack frame.
+
+When a frame has been created and selected uing @command{frame create}
+then you can always return to the original stack using one of the
+previous stack frame selection instructions, for example
+@command{frame level 0}.
+
+@end table
 
 @kindex up
 @item up @var{n}
@@ -7215,11 +7252,13 @@ for details.
 
 @table @code
 @kindex select-frame
-@item select-frame
+@item select-frame @r{[} @var{frame-selection-spec} @r{]}
 The @code{select-frame} command is a variant of @code{frame} that does
 not display the new frame after selecting it.  This command is
 intended primarily for use in @value{GDBN} command scripts, where the
-output might be unnecessary and distracting.
+output might be unnecessary and distracting.  The
+@var{frame-selection-spec} is as for the @command{frame} command
+described in @ref{Selection, ,Selecting a Frame}.
 
 @kindex down-silently
 @kindex up-silently
@@ -7277,13 +7316,12 @@ which registers were saved in the frame
 something has gone wrong that has made the stack format fail to fit
 the usual conventions.
 
-@item info frame @var{addr}
-@itemx info f @var{addr}
-Print a verbose description of the frame at address @var{addr}, without
-selecting that frame.  The selected frame remains unchanged by this
-command.  This requires the same kind of address (more than one for some
-architectures) that you specify in the @code{frame} command.
-@xref{Selection, ,Selecting a Frame}.
+@item info frame @r{[} @var{frame-selection-spec} @r{]}
+@itemx info f @r{[} @var{frame-selection-spec} @r{]}
+Print a verbose description of the frame selected by
+@var{frame-selection-spec}.  The @var{frame-selection-spec} is the
+same as for the @command{frame} command (@pxref{Selection, ,Selecting
+a Frame}).  The selected frame remains unchanged by this command.
 
 @kindex info args
 @item info args
diff --git a/gdb/mi/mi-cmd-stack.c b/gdb/mi/mi-cmd-stack.c
index 520db2b..ede23a1 100644
--- a/gdb/mi/mi-cmd-stack.c
+++ b/gdb/mi/mi-cmd-stack.c
@@ -697,10 +697,18 @@ list_args_or_locals (enum what_to_list what, enum print_values values,
 void
 mi_cmd_stack_select_frame (char *command, char **argv, int argc)
 {
-  if (argc == 0 || argc > 1)
-    error (_("-stack-select-frame: Usage: FRAME_SPEC"));
-
-  select_frame_command (argv[0], 1 /* not used */ );
+  TRY
+    {
+      select_frame_from_spec (argv);
+    }
+  CATCH (except, RETURN_MASK_ERROR)
+    {
+      const char *msg = except.message;
+      if (msg == NULL)
+	msg = _("Unknown error");
+      error (_("-stack-select-frame: %s"), msg);
+    }
+  END_CATCH
 }
 
 void
diff --git a/gdb/stack.c b/gdb/stack.c
index 71e171a..9db63ae 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -100,6 +100,9 @@ static void set_last_displayed_sal (int valid,
 				    struct symtab *symtab,
 				    int line);
 
+static struct frame_info * find_frame_for_function (char *);
+static struct frame_info * find_frame_for_address (CORE_ADDR);
+
 /* Zero means do things normally; we are interacting directly with the
    user.  One means print the full filename and linenumber when a
    frame is printed, and do so in a format emacs18/emacs19.22 can
@@ -1273,130 +1276,254 @@ print_frame (struct frame_info *frame, int print_level,
 }
 
 
-/* Read a frame specification in whatever the appropriate format is from
-   FRAME_EXP.  Call error() if the specification is in any way invalid (so
-   this function never returns NULL).  When SEPECTED_P is non-NULL set its
-   target to indicate that the default selected frame was used.  */
+/* The stack frame selection mode as specified by the user in commands such
+   as "info frame <SPEC>" and "frame <SPEC>".  */
+enum frame_selection_mode
+  {
+    /* Default is what you get when the user does not specify a specific
+       mode.  */
+    frame_mode_default,
+
+    /* Select a frame by level in the stack.  Takes a single integer
+       parameter.  */
+    frame_mode_level,
 
-static struct frame_info *
-parse_frame_specification (const char *frame_exp, int *selected_frame_p)
+    /* Select a frame by stack frame address.  Takes a single address
+       parameter.  */
+    frame_mode_address,
+
+    /* Select a frame by function name.  Takes the name of a function as a
+       parameter, selects the inner-most frame matching that function.  */
+    frame_mode_function,
+
+    /* Create a new frame.  Takes either one or two address parameters and
+       constructs a new frame.  */
+    frame_mode_create,
+  };
+
+/* Maximum number of args for frame selection specification.  */
+
+#define FRAME_SELECTION_MAX_ARGS 2
+
+/* Structure to hold a parsed frame selection specification.  */
+
+struct frame_selection_spec
 {
+  /* The mode of the frame selection.  */
+  enum frame_selection_mode mode;
+
+  /* The number of arguments in the ARGS array.  */
   int numargs;
-  struct value *args[4];
-  CORE_ADDR addrs[ARRAY_SIZE (args)];
 
-  if (frame_exp == NULL)
-    numargs = 0;
-  else
-    {
-      numargs = 0;
-      while (1)
-	{
-	  char *addr_string;
-	  struct cleanup *cleanup;
-	  const char *p;
+  /* Array of strings, copied, and from the frame selection specification
+     string.  Each is either NULL or allocated with malloc.  */
+  char *args [FRAME_SELECTION_MAX_ARGS];
+};
 
-	  /* Skip leading white space, bail of EOL.  */
-	  frame_exp = skip_spaces_const (frame_exp);
-	  if (!*frame_exp)
-	    break;
+/* Compare ARG to all the possible frame selection mode strings.  If ARG
+   matches then the selected frame mode is placed into MODE and true is
+   returned.  If ARG does not match any of the frame selection modes then
+   the contents of MODE are unchanged, and false is returned.
+
+   The matching of ARG against the mode selection strings is case
+   insensitive, and ARG can be any sub-string of a mode string, starting
+   from the first character to count as a match.  So, "LE" matches
+   "level", and "a" matches "address".  */
+
+static int
+is_frame_mode_string (const char *arg, enum frame_selection_mode *mode)
+{
+  int i;
+  static const struct
+  {
+    const char *mode_str;
+    enum frame_selection_mode mode_val;
+  } mode_list [] =
+      {
+	{ "level", frame_mode_level },
+	{ "address", frame_mode_address },
+	{ "function", frame_mode_function },
+	{ "create", frame_mode_create }
+      };
 
-	  /* Parse the argument, extract it, save it.  */
-	  for (p = frame_exp;
-	       *p && !ISSPACE (*p);
-	       p++);
-	  addr_string = savestring (frame_exp, p - frame_exp);
-	  frame_exp = p;
-	  cleanup = make_cleanup (xfree, addr_string);
-	  
-	  /* NOTE: Parse and evaluate expression, but do not use
-	     functions such as parse_and_eval_long or
-	     parse_and_eval_address to also extract the value.
-	     Instead value_as_long and value_as_address are used.
-	     This avoids problems with expressions that contain
-	     side-effects.  */
-	  if (numargs >= ARRAY_SIZE (args))
-	    error (_("Too many args in frame specification"));
-	  args[numargs++] = parse_and_eval (addr_string);
-
-	  do_cleanups (cleanup);
+  for (i = 0; i < ARRAY_SIZE (mode_list); ++i)
+    {
+      if ((strlen (arg) <= strlen (mode_list [i].mode_str))
+	  && (strncasecmp (arg, mode_list [i].mode_str, strlen (arg)) == 0))
+	{
+	  *mode = mode_list [i].mode_val;
+	  return 1;
 	}
     }
+  return 0;
+}
 
-  /* If no args, default to the selected frame.  */
-  if (numargs == 0)
+/* Parse a frame selection specification in ARGV and place the result into
+   SPEC.  If the specification in ARGV is invalid then an error is thrown,
+   in which case the contents of SPEC are undefined, but no memory will
+   have been allocated for the args in SPEC.  If this function returns then
+   SPEC is valid.  */
+
+static void
+parse_frame_selection_spec (char **argv,
+			    struct frame_selection_spec *spec)
+{
+  int i;
+
+  spec->mode = frame_mode_default;
+  spec->numargs = 0;
+  for (i = 0; i < FRAME_SELECTION_MAX_ARGS; ++i)
+    spec->args [i] = NULL;
+
+  /* Check for no args case.  */
+  if (argv == NULL || *argv == NULL)
+    return;
+
+  /* Is the first arg a frame mode string?  */
+  if (is_frame_mode_string (*argv, &spec->mode))
+    argv++;
+
+  /* Now process (copy) the remaining args.  */
+  for (/* */ ;*argv != NULL; ++argv)
     {
-      if (selected_frame_p != NULL)
-	(*selected_frame_p) = 1;
-      return get_selected_frame (_("No stack."));
+      if (spec->numargs >= FRAME_SELECTION_MAX_ARGS)
+	{
+	  for (i = 0; i < spec->numargs; ++i)
+	    xfree (spec->args [i]);
+	  error (_("Too many args in frame specification."));
+	}
+
+      spec->args [spec->numargs++] = xstrdup (*argv);
     }
+}
+
+/* Free resource associated with a struct frame_selection_spec passed in
+   ARG.  */
 
-  /* None of the remaining use the selected frame.  */
-  if (selected_frame_p != NULL)
-    (*selected_frame_p) = 0;
+static void
+free_frame_selection_spec (void *arg)
+{
+  int i;
+  struct frame_selection_spec *spec = (struct frame_selection_spec *) arg;
 
-  /* Assume the single arg[0] is an integer, and try using that to
-     select a frame relative to current.  */
-  if (numargs == 1)
+  for (i = 0; i < spec->numargs; ++i)
+    xfree (spec->args [i]);
+}
+
+/* Read a frame specification that has been split into ARGV, and return the
+   specified frame.  Call error() if the specification is in any way
+   invalid (so this function never returns NULL).  When SEPECTED_P is
+   non-NULL set its target to indicate that the default selected frame was
+   used.  */
+
+static struct frame_info *
+parse_frame_specification (char **argv, int *selected_frame_p)
+{
+  struct frame_selection_spec spec;
+  struct cleanup *old_chain;
+  struct frame_info *fid = NULL;
+
+  parse_frame_selection_spec (argv, &spec);
+  old_chain = make_cleanup (free_frame_selection_spec, &spec);
+
+  do
     {
-      struct frame_info *fid;
-      int level = value_as_long (args[0]);
+      if (spec.mode == frame_mode_default && spec.numargs == 0)
+	{
+	  if (selected_frame_p != NULL)
+	    (*selected_frame_p) = 1;
+	  fid = get_selected_frame (_("No stack."));
+	  break;
+	}
 
-      fid = find_relative_frame (get_current_frame (), &level);
-      if (level == 0)
-	/* find_relative_frame was successful.  */
-	return fid;
-    }
+      /* None of the remaining use the selected frame.  */
+      if (selected_frame_p != NULL)
+	(*selected_frame_p) = 0;
 
-  /* Convert each value into a corresponding address.  */
-  {
-    int i;
+      if ((spec.mode == frame_mode_create && spec.numargs > 2)
+	  || (spec.mode != frame_mode_create && spec.numargs > 1))
+	error (_("Too many args in frame specification."));
 
-    for (i = 0; i < numargs; i++)
-      addrs[i] = value_as_address (args[i]);
-  }
+      if (spec.mode == frame_mode_level || spec.mode == frame_mode_default)
+	{
+	  int level;
 
-  /* Assume that the single arg[0] is an address, use that to identify
-     a frame with a matching ID.  Should this also accept stack/pc or
-     stack/pc/special.  */
-  if (numargs == 1)
-    {
-      struct frame_id id = frame_id_build_wild (addrs[0]);
-      struct frame_info *fid;
-
-      /* If (s)he specifies the frame with an address, he deserves
-	 what (s)he gets.  Still, give the highest one that matches.
-	 (NOTE: cagney/2004-10-29: Why highest, or outer-most, I don't
-	 know).  */
-      for (fid = get_current_frame ();
-	   fid != NULL;
-	   fid = get_prev_frame (fid))
+	  level = value_as_long (parse_and_eval (spec.args [0]));
+	  fid = find_relative_frame (get_current_frame (), &level);
+	  if (level == 0)
+	    /* find_relative_frame was successful.  */
+	    break;
+	  error (_("No frame at level %s."), spec.args [0]);
+	}
+      else if (spec.mode == frame_mode_function)
 	{
-	  if (frame_id_eq (id, get_frame_id (fid)))
-	    {
-	      struct frame_info *prev_frame;
+	  fid = find_frame_for_function (spec.args [0]);
+	  if (fid != NULL)
+	    break;
+	  error (_("No frame for function \"%s\"."), spec.args [0]);
+	}
+      else
+	{
+	  CORE_ADDR addr [FRAME_SELECTION_MAX_ARGS] = { 0 };
+	  int i;
 
-	      while (1)
-		{
-		  prev_frame = get_prev_frame (fid);
-		  if (!prev_frame
-		      || !frame_id_eq (id, get_frame_id (prev_frame)))
-		    break;
-		  fid = prev_frame;
-		}
-	      return fid;
+	  for (i = 0; i < spec.numargs; ++i)
+	    addr [i] = value_as_address (parse_and_eval (spec.args [i]));
+
+	  if (spec.mode == frame_mode_address)
+	    {
+	      fid = find_frame_for_address (addr [0]);
+	      if (fid != NULL)
+		break;
+	      error (_("No frame at address %s."), spec.args [0]);
+	    }
+	  else if (spec.mode == frame_mode_create)
+	    {
+	      if (spec.numargs == 1)
+		fid = create_new_frame (addr [0], 0);
+	      else if (spec.numargs == 2)
+		fid = create_new_frame (addr [0], addr [1]);
+	      break;
 	    }
 	}
-      }
 
-  /* We couldn't identify the frame as an existing frame, but
-     perhaps we can create one with a single argument.  */
-  if (numargs == 1)
-    return create_new_frame (addrs[0], 0);
-  else if (numargs == 2)
-    return create_new_frame (addrs[0], addrs[1]);
-  else
-    error (_("Too many args in frame specification"));
+      internal_error (__FILE__, __LINE__,
+		      "unhandled case during frame selection");
+    }
+  while (0);
+
+  /* All error cases should be picked up in the above code.  If we get here
+     then we should have found a valid frame.  */
+  gdb_assert (fid != NULL);
+  do_cleanups (old_chain);
+  return fid;
+}
+
+/* Completion function for "frame" and "info frame" commands.  */
+
+static VEC (char_ptr) *
+frame_selection_completer (struct cmd_list_element *ignore,
+			   const char *text, const char *word)
+{
+  if (text == word)
+    {
+      VEC (const_char_ptr) *completion_name_vec = NULL;
+      VEC (char_ptr) *matches_vec;
+
+      VEC_safe_push (const_char_ptr, completion_name_vec, "level");
+      VEC_safe_push (const_char_ptr, completion_name_vec, "function");
+      VEC_safe_push (const_char_ptr, completion_name_vec, "address");
+      VEC_safe_push (const_char_ptr, completion_name_vec, "create");
+      VEC_safe_push (const_char_ptr, completion_name_vec, NULL);
+
+      matches_vec
+	= complete_on_enum (VEC_address (const_char_ptr, completion_name_vec),
+			    text, word);
+      VEC_free (const_char_ptr, completion_name_vec);
+      return matches_vec;
+    }
+
+  return make_symbol_completion_list (text, word);
 }
 
 /* Print verbosely the selected frame or the frame at address
@@ -1422,8 +1549,11 @@ frame_info (char *addr_exp, int from_tty)
   /* Initialize it to avoid "may be used uninitialized" warning.  */
   CORE_ADDR caller_pc = 0;
   int caller_pc_p = 0;
+  char **argv;
 
-  fi = parse_frame_specification (addr_exp, &selected_frame_p);
+  argv = gdb_buildargv (addr_exp);
+  make_cleanup_freeargv (argv);
+  fi = parse_frame_specification (argv, &selected_frame_p);
   gdbarch = get_frame_arch (fi);
 
   /* During the following value will be created and then displayed.
@@ -2266,17 +2396,28 @@ find_relative_frame (struct frame_info *frame, int *level_offset_ptr)
   return frame;
 }
 
+void
+select_frame_from_spec (char **argv)
+{
+  select_frame (parse_frame_specification (argv, NULL));
+}
+
 /* The "select_frame" command.  With no argument this is a NOP.
-   Select the frame at level LEVEL_EXP if it is a valid level.
-   Otherwise, treat LEVEL_EXP as an address expression and select it.
+   Select the frame according to LEVEL_EXP.
 
    See parse_frame_specification for more info on proper frame
    expressions.  */
 
-void
+static void
 select_frame_command (char *level_exp, int from_tty)
 {
-  select_frame (parse_frame_specification (level_exp, NULL));
+  char **argv;
+  struct cleanup *old_chain;
+
+  argv = gdb_buildargv (level_exp);
+  old_chain = make_cleanup_freeargv (argv);
+  select_frame_from_spec (argv);
+  do_cleanups (old_chain);
 }
 
 /* The "frame" command.  With no argument, print the selected frame
@@ -2574,6 +2715,43 @@ func_command (char *arg, int from_tty)
     }
 }
 
+/* Find inner-mode frame with frame address ADDRESS.  Return NULL if no
+   matching frame can be found.  */
+
+static struct frame_info *
+find_frame_for_address (CORE_ADDR address)
+{
+  struct frame_id id;
+  struct frame_info *fid;
+
+  id = frame_id_build_wild (address);
+
+  /* If (s)he specifies the frame with an address, he deserves
+     what (s)he gets.  Still, give the highest one that matches.
+     (NOTE: cagney/2004-10-29: Why highest, or outer-most, I don't
+     know).  */
+  for (fid = get_current_frame ();
+       fid != NULL;
+       fid = get_prev_frame (fid))
+    {
+      if (frame_id_eq (id, get_frame_id (fid)))
+	{
+	  struct frame_info *prev_frame;
+
+	  while (1)
+	    {
+	      prev_frame = get_prev_frame (fid);
+	      if (!prev_frame
+		  || !frame_id_eq (id, get_frame_id (prev_frame)))
+		break;
+	      fid = prev_frame;
+	    }
+	  return fid;
+	}
+    }
+  return NULL;
+}
+
 
 
 /* Provide a prototype to silence -Wmissing-prototypes.  */
@@ -2582,6 +2760,8 @@ void _initialize_stack (void);
 void
 _initialize_stack (void)
 {
+  struct cmd_list_element *cmd;
+
   add_com ("return", class_stack, return_command, _("\
 Make selected stack frame return to its caller.\n\
 Control remains in the debugger, but when you continue\n\
@@ -2604,20 +2784,35 @@ An argument says how many frames down to go."));
 Same as the `down' command, but does not print anything.\n\
 This is useful in command scripts."));
 
-  add_com ("frame", class_stack, frame_command, _("\
-Select and print a stack frame.\nWith no argument, \
-print the selected stack frame.  (See also \"info frame\").\n\
-An argument specifies the frame to select.\n\
-It can be a stack frame number or the address of the frame.\n\
-With argument, nothing is printed if input is coming from\n\
-a command file or a user-defined command."));
+  cmd = add_com ("frame", class_stack, frame_command, _("\
+Select and print a stack frame.\n\
+With no arguments, print the selected stack frame.  \
+(See also \"info frame\").\n\
+Alternatively a frame specification may be given to select a specific\n\
+stack frame.  The available frame specifications are:\n\
+\n\
+Select a stack frame by number:\n\
+  frame <number>\n\
+  frame level <number>\n\
+\n\
+Select a stack frame by the frames stack frame address:\n\
+  frame address <stack-address>\n\
+\n\
+Select the inner most frame for a function of a given name:\n\
+  frame function <name>\n\
+\n\
+Create a new temporary frame for a given stack frame address and PC:\n\
+  frame create <stack-address>\n\
+  frame create <stack-address> <pc-address>"));
+  set_cmd_completer (cmd, frame_selection_completer);
 
   add_com_alias ("f", "frame", class_stack, 1);
 
-  add_com ("select-frame", class_stack, select_frame_command, _("\
+  cmd = add_com ("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"));
+An argument specifies the frame to select, and is the same frame\n\
+specification as for the \"frame\" command."));
+  set_cmd_completer (cmd, frame_selection_completer);
 
   add_com ("backtrace", class_stack, backtrace_command, _("\
 Print backtrace of all stack frames, or innermost COUNT frames.\n\
@@ -2631,8 +2826,13 @@ on this backtrace.\n"));
   add_info ("stack", backtrace_command,
 	    _("Backtrace of the stack, or innermost COUNT frames."));
   add_info_alias ("s", "stack", 1);
-  add_info ("frame", frame_info,
-	    _("All about selected stack frame, or frame at ADDR."));
+  cmd = add_info ("frame", frame_info,
+		  _("All about the selected stack frame.\n\
+With no arguments, displays information about the currently selected stack\n\
+frame.  Alternatively a frame specification may be provided (See \"frame\")\n\
+the information is then printed about the specified frame."));
+  set_cmd_completer (cmd, frame_selection_completer);
+
   add_info_alias ("f", "frame", 1);
   add_info ("locals", locals_info,
 	    _("Local variables of current stack frame."));
diff --git a/gdb/stack.h b/gdb/stack.h
index 21ddac5..26566a2 100644
--- a/gdb/stack.h
+++ b/gdb/stack.h
@@ -20,7 +20,7 @@
 #ifndef STACK_H
 #define STACK_H
 
-void select_frame_command (char *level_exp, int from_tty);
+void select_frame_from_spec (char **argv);
 
 void find_frame_funname (struct frame_info *frame, char **funname,
 			 enum language *funlang, struct symbol **funcp);
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 272d3fc..bdd8be4 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,11 @@
 2015-09-11  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* gdb.base/frame-cmds.exp: Almost complete rewrite.
+	* gdb.base/frame-cmds.c: Extend.
+	* gdb.mi/mi-var-frame.exp: Update a few tests.  Add one new test.
+
+2015-09-11  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* gdb.base/dbx.exp (test_func): Remove xfails, update expected
 	results.
 
diff --git a/gdb/testsuite/gdb.base/frame-cmds.c b/gdb/testsuite/gdb.base/frame-cmds.c
index 53b222d..24a46f3 100644
--- a/gdb/testsuite/gdb.base/frame-cmds.c
+++ b/gdb/testsuite/gdb.base/frame-cmds.c
@@ -28,7 +28,25 @@ frame_1 (void)
 }
 
 int
+recursive (int arg)
+{
+  int v;
+
+  if (arg < 2)
+    v = recursive (arg + 1);
+  else
+    v = frame_2 ();
+
+  return v;
+}
+
+int
 main (void)
 {
-  return frame_1 ();
+  int i, j;
+
+  i = frame_1 ();
+  j = recursive (0);
+
+  return i + j;
 }
diff --git a/gdb/testsuite/gdb.mi/mi-var-frame.exp b/gdb/testsuite/gdb.mi/mi-var-frame.exp
index 5c3f196..247675a 100644
--- a/gdb/testsuite/gdb.mi/mi-var-frame.exp
+++ b/gdb/testsuite/gdb.mi/mi-var-frame.exp
@@ -60,11 +60,22 @@ mi_gdb_test "-var-create R * $register" \
     "create varobj 'R' for register '$register'"
 
 for {set i 0} {$i < 5} {incr i} {
-    mi_gdb_test "-stack-select-frame $i" \
-	{\^done} \
-	"-stack-select-frame $i"
+
+    if { $i < 3 } then {
+	mi_gdb_test "-stack-select-frame $i" \
+	    {\^done} \
+	    "-stack-select-frame $i"
+    } else {
+	mi_gdb_test "-stack-select-frame create $i" \
+	    {\^done} \
+	    "-stack-select-frame create frame at $i"
+    }
 
     mi_gdb_test "-var-update R" \
 	"\\^done,changelist=\\\[\\\]" \
 	"update 'R' in frame $i: no change"
 }
+
+mi_gdb_test "-stack-select-frame 5" \
+    {\^error,msg="-stack-select-frame: No frame at level 5."} \
+    "-stack-select-frame 5"
-- 
2.5.1