Patchwork [v2,20/24] Make "frame apply" support -OPT options

login
register
mail settings
Submitter Pedro Alves
Date May 30, 2019, 7:53 p.m.
Message ID <20190530195333.20448-21-palves@redhat.com>
Download mbox | patch
Permalink /patch/32944/
State New
Headers show

Comments

Pedro Alves - May 30, 2019, 7:53 p.m.
This adds support for '-'-style options to the "frame apply" family of
commands -- "frame apply COUNT", "frame apply level", "frame apply
all", "faas" and "tfaas".

The -q/-c/-s flags were already supported, -past-main/-past-entry is
new:

~~~
(gdb) help frame apply all
Apply a command to all frames.

Usage: frame apply all [OPTION]... COMMAND
Prints the frame location information followed by COMMAND output.

By default, an error raised during the execution of COMMAND
aborts "frame apply".

Options:
  -q
    Disables printing the frame location information.

  -c
    Print any error raised by COMMAND and continue.

  -s
    Silently ignore any errors or empty output produced by COMMAND.

  -past-main [on|off]
    Set whether backtraces should continue past "main".
    Normally the caller of "main" is not of interest, so GDB will terminate
    the backtrace at "main".  Set this if you need to see the rest
    of the stack trace.

  -past-entry [on|off]
    Set whether backtraces should continue past the entry point of a program.
    Normally there are no callers beyond the entry point of a program, so GDB
    will terminate the backtrace there.  Set this if you need to see
    the rest of the stack trace.
~~~

TAB completion of options is now supported.  Also, TAB completion of
COMMAND in "frame apply all COMMAND" does the right thing now, making
use of complete_command, added by the previous patch.  E.g.:

 (gdb) thread apply all -ascending frame apply all -past-main print -[TAB]
 -address         -elements        -pretty          -symbol
 -array           -null-stop       -repeats         -union
 -array-indexes   -object          -static-members  -vtbl
 (gdb) thread apply all -ascending frame apply all -past-main print glo[TAB]
 global1         global2

The change to tfaas_command is necessary because otherwise you get
this:

 (gdb) tfaas --
 Unrecognized option at: frame apply all -s --

That's because the above is equivalent to:

 (gdb) thread apply all -s frame apply all -s --

and the "--" instructs "thread apply" to consider everything up to
"--" as its command options.  And from that view, "frame" is an
invalid option.

The change makes tfaas be equivalent to:

 (gdb) thread apply all -s -- frame apply all -s --

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* cli/cli-utils.c (parse_flags_qcs): Use validate_flags_qcs.
	(validate_flags_qcs): New.
	* cli/cli-utils.h (struct qcs_flags): Change field types to int.
	(validate_flags_qcs): Declare.
	* stack.c (qcs_flag_option_def, fr_qcs_flags_option_defs): New.
	(make_frame_apply_options_def_group): New.
	(frame_apply_command_count): Process options with
	gdb::option::process_options.
	(frame_apply_completer): New.
	(frame_apply_level_completer, frame_apply_all_completer)
	(frame_apply_completer): New.
	(_initialize_stack): Update help of "frame apply", "frame apply
	level", "frame apply all" and "faas" to mention supported options
	and install command completers.
	* stack.h (frame_apply_all_completer): Declare.
	* thread.c: Include "stack.h".
	(tfaas_command): Add "--".
	(_initialize_thread): Update help "tfaas" to mention supported
	options and install command completer.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.base/options.exp (test-frame-apply): New.
	(top level): Test print commands with different "frame apply"
	prefixes.
---
 gdb/cli/cli-utils.c                |  14 ++-
 gdb/cli/cli-utils.h                |  14 ++-
 gdb/stack.c                        | 222 +++++++++++++++++++++++++++++++------
 gdb/stack.h                        |   5 +
 gdb/testsuite/gdb.base/options.exp |  97 +++++++++++++++-
 gdb/thread.c                       |  12 +-
 6 files changed, 320 insertions(+), 44 deletions(-)

Patch

diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
index 306b69e3d8f..30d4091a0d1 100644
--- a/gdb/cli/cli-utils.c
+++ b/gdb/cli/cli-utils.c
@@ -573,8 +573,18 @@  parse_flags_qcs (const char *which_command, const char **str,
       gdb_assert_not_reached ("int qcs flag out of bound");
     }
 
-  if (flags->cont && flags->silent)
-    error (_("%s: -c and -s are mutually exclusive"), which_command);
+  validate_flags_qcs (which_command, flags);
 
   return true;
 }
+
+/* See documentation in cli-utils.h.  */
+
+void
+validate_flags_qcs (const char *which_command, qcs_flags *flags)
+{
+  if (flags->cont && flags->silent)
+    error (_("%s: -c and -s are mutually exclusive"), which_command);
+}
+
+/* See documentation in cli-utils.h.  */
diff --git a/gdb/cli/cli-utils.h b/gdb/cli/cli-utils.h
index 41c23561d6f..e6b877d5ab7 100644
--- a/gdb/cli/cli-utils.h
+++ b/gdb/cli/cli-utils.h
@@ -229,13 +229,14 @@  check_for_argument (char **str, const char *arg)
    such that FLAGS [N - 1] is the valid found flag.  */
 extern int parse_flags (const char **str, const char *flags);
 
-/* qcs_flags struct regroups the flags parsed by parse_flags_qcs.  */
+/* qcs_flags struct groups the -q, -c, and -s flags parsed by "thread
+   apply" and "frame apply" commands */
 
 struct qcs_flags
 {
-  bool quiet = false;
-  bool cont = false;
-  bool silent = false;
+  int quiet = false;
+  int cont = false;
+  int silent = false;
 };
 
 /* A helper function that uses parse_flags to handle the flags qcs :
@@ -259,4 +260,9 @@  struct qcs_flags
 extern bool parse_flags_qcs (const char *which_command, const char **str,
 			     qcs_flags *flags);
 
+/* Validate FLAGS.  Throws an error if both FLAGS->CONT and
+   FLAGS->SILENT are true.  WHICH_COMMAND is included in the error
+   message.  */
+extern void validate_flags_qcs (const char *which_command, qcs_flags *flags);
+
 #endif /* CLI_CLI_UTILS_H */
diff --git a/gdb/stack.c b/gdb/stack.c
index 1cb1fc9c38c..a994889af73 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -2823,6 +2823,42 @@  func_command (const char *arg, int from_tty)
     }
 }
 
+/* The qcs command line flags for the "frame apply" commands.  Keep
+   this in sync with the "thread apply" commands.  */
+
+using qcs_flag_option_def
+  = gdb::option::flag_option_def<qcs_flags>;
+
+static const gdb::option::option_def fr_qcs_flags_option_defs[] = {
+  qcs_flag_option_def {
+    "q", [] (qcs_flags *opt) { return &opt->quiet; },
+    N_("Disables printing the frame location information."),
+  },
+
+  qcs_flag_option_def {
+    "c", [] (qcs_flags *opt) { return &opt->cont; },
+    N_("Print any error raised by COMMAND and continue."),
+  },
+
+  qcs_flag_option_def {
+    "s", [] (qcs_flags *opt) { return &opt->silent; },
+    N_("Silently ignore any errors or empty output produced by COMMAND."),
+  },
+};
+
+/* Create an option_def_group array for all the "frame apply" options,
+   with FLAGS and SET_BT_OPTS as context.  */
+
+static inline std::array<gdb::option::option_def_group, 2>
+make_frame_apply_options_def_group (qcs_flags *flags,
+				    set_backtrace_options *set_bt_opts)
+{
+  return {{
+    { {fr_qcs_flags_option_defs}, flags },
+    { {set_backtrace_option_defs}, set_bt_opts },
+  }};
+}
+
 /* Apply a GDB command to all stack frames, or a set of identified frames,
    or innermost COUNT frames.
    With a negative COUNT, apply command on outermost -COUNT frames.
@@ -2852,10 +2888,13 @@  frame_apply_command_count (const char *which_command,
 			   struct frame_info *trailing, int count)
 {
   qcs_flags flags;
-  struct frame_info *fi;
+  set_backtrace_options set_bt_opts = user_set_backtrace_options;
 
-  while (cmd != NULL && parse_flags_qcs (which_command, &cmd, &flags))
-    ;
+  auto group = make_frame_apply_options_def_group (&flags, &set_bt_opts);
+  gdb::option::process_options
+    (&cmd, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group);
+
+  validate_flags_qcs (which_command, &flags);
 
   if (cmd == NULL || *cmd == '\0')
     error (_("Please specify a command to apply on the selected frames"));
@@ -2866,7 +2905,12 @@  frame_apply_command_count (const char *which_command,
      these also.  */
   scoped_restore_current_thread restore_thread;
 
-  for (fi = trailing; fi && count--; fi = get_prev_frame (fi))
+  /* These options are handled quite deep in the unwind machinery, so
+     we get to pass them down by swapping globals.  */
+  scoped_restore restore_set_backtrace_options
+    = make_scoped_restore (&user_set_backtrace_options, set_bt_opts);
+
+  for (frame_info *fi = trailing; fi && count--; fi = get_prev_frame (fi))
     {
       QUIT;
 
@@ -2909,6 +2953,104 @@  frame_apply_command_count (const char *which_command,
     }
 }
 
+/* Completer for the "frame apply ..." commands.  */
+
+static void
+frame_apply_completer (completion_tracker &tracker, const char *text)
+{
+  const auto group = make_frame_apply_options_def_group (nullptr, nullptr);
+  if (gdb::option::complete_options
+      (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group))
+    return;
+
+  complete_command (tracker, text);
+}
+
+/* Completer for the "frame apply" commands.  */
+
+static void
+frame_apply_level_cmd_completer (struct cmd_list_element *ignore,
+				 completion_tracker &tracker,
+				 const char *text, const char */*word*/)
+{
+  /* Do this explicitly because there's an early return below.  */
+  tracker.set_use_custom_word_point (true);
+
+  number_or_range_parser levels (text);
+
+  /* Skip the LEVEL list to find the options and command args.  */
+  try
+    {
+      while (!levels.finished ())
+	{
+	  /* Call for effect.  */
+	  levels.get_number ();
+
+	  if (levels.in_range ())
+	    levels.skip_range ();
+	}
+    }
+  catch (const gdb_exception_error &ex)
+    {
+      /* get_number throws if it parses a negative number, for
+	 example.  But a seemingly negative number may be the start of
+	 an option instead.  */
+    }
+
+  const char *cmd = levels.cur_tok ();
+
+  if (cmd == text)
+    {
+      /* No level list yet.  */
+      return;
+    }
+
+  /* Check if we're past a valid LEVEL already.  */
+  if (levels.finished ()
+      && cmd > text && !isspace (cmd[-1]))
+    return;
+
+  /* We're past LEVELs, advance word point.  */
+  tracker.advance_custom_word_point_by (cmd - text);
+  text = cmd;
+
+  frame_apply_completer (tracker, text);
+}
+
+/* Completer for the "frame apply all" command.  */
+
+void
+frame_apply_all_cmd_completer (struct cmd_list_element *ignore,
+			       completion_tracker &tracker,
+			       const char *text, const char */*word*/)
+{
+  frame_apply_completer (tracker, text);
+}
+
+/* Completer for the "frame apply COUNT" command.  */
+
+static void
+frame_apply_cmd_completer (struct cmd_list_element *ignore,
+			   completion_tracker &tracker,
+			   const char *text, const char */*word*/)
+{
+  const char *cmd = text;
+
+  int count = get_number_trailer (&cmd, 0);
+  if (count == 0)
+    return;
+
+  /* Check if we're past a valid COUNT already.  */
+  if (cmd > text && !isspace (cmd[-1]))
+    return;
+
+  /* We're past COUNT, advance word point.  */
+  tracker.advance_custom_word_point_by (cmd - text);
+  text = cmd;
+
+  frame_apply_completer (tracker, text);
+}
+
 /* Implementation of the "frame apply level" command.  */
 
 static void
@@ -3095,44 +3237,62 @@  A single numerical argument specifies the frame to select."),
 
   add_com_alias ("f", "frame", class_stack, 1);
 
-#define FRAME_APPLY_FLAGS_HELP "\
+#define FRAME_APPLY_OPTION_HELP "\
 Prints the frame location information followed by COMMAND output.\n\
-FLAG arguments are -q (quiet), -c (continue), -s (silent).\n\
-Flag -q disables printing the frame location information.\n\
-By default, if a COMMAND raises an error, frame apply is aborted.\n\
-Flag -c indicates to print the error and continue.\n\
-Flag -s indicates to silently ignore a COMMAND that raises an error\n\
-or produces no output."
-
-  add_prefix_cmd ("apply", class_stack, frame_apply_command,
-		  _("Apply a command to a number of frames.\n\
-Usage: frame apply COUNT [FLAG]... COMMAND\n\
+\n\
+By default, an error raised during the execution of COMMAND\n\
+aborts \"frame apply\".\n\
+\n\
+Options:\n\
+%OPTIONS%"
+
+  const auto frame_apply_opts
+    = make_frame_apply_options_def_group (nullptr, nullptr);
+
+  static std::string frame_apply_cmd_help = gdb::option::build_help (N_("\
+Apply a command to a number of frames.\n\
+Usage: frame apply COUNT [OPTION]... COMMAND\n\
 With a negative COUNT argument, applies the command on outermost -COUNT frames.\n"
-FRAME_APPLY_FLAGS_HELP),
-		  &frame_apply_cmd_list, "frame apply ", 1, &frame_cmd_list);
+				  FRAME_APPLY_OPTION_HELP),
+			       frame_apply_opts);
 
-  add_cmd ("all", class_stack, frame_apply_all_command,
-	   _("\
+  cmd = add_prefix_cmd ("apply", class_stack, frame_apply_command,
+			frame_apply_cmd_help.c_str (),
+			&frame_apply_cmd_list, "frame apply ", 1,
+			&frame_cmd_list);
+  set_cmd_completer_handle_brkchars (cmd, frame_apply_cmd_completer);
+
+  static std::string frame_apply_all_cmd_help = gdb::option::build_help (N_("\
 Apply a command to all frames.\n\
 \n\
-Usage: frame apply all [FLAG]... COMMAND\n"
-FRAME_APPLY_FLAGS_HELP),
-	   &frame_apply_cmd_list);
+Usage: frame apply all [OPTION]... COMMAND\n"
+				  FRAME_APPLY_OPTION_HELP),
+			       frame_apply_opts);
 
-  add_cmd ("level", class_stack, frame_apply_level_command,
-	   _("\
+  cmd = add_cmd ("all", class_stack, frame_apply_all_command,
+		 frame_apply_all_cmd_help.c_str (),
+		 &frame_apply_cmd_list);
+  set_cmd_completer_handle_brkchars (cmd, frame_apply_all_cmd_completer);
+
+  static std::string frame_apply_level_cmd_help = gdb::option::build_help (N_("\
 Apply a command to a list of frames.\n\
 \n\
-Usage: frame apply level LEVEL... [FLAG]... COMMAND\n\
-ID is a space-separated list of LEVELs of frames to apply COMMAND on.\n"
-FRAME_APPLY_FLAGS_HELP),
+Usage: frame apply level LEVEL... [OPTION]... COMMAND\n\
+LEVEL is a space-separated list of levels of frames to apply COMMAND on.\n"
+				  FRAME_APPLY_OPTION_HELP),
+			       frame_apply_opts);
+
+  cmd = add_cmd ("level", class_stack, frame_apply_level_command,
+	   frame_apply_level_cmd_help.c_str (),
 	   &frame_apply_cmd_list);
+  set_cmd_completer_handle_brkchars (cmd, frame_apply_level_cmd_completer);
 
-  add_com ("faas", class_stack, faas_command, _("\
+  cmd = add_com ("faas", class_stack, faas_command, _("\
 Apply a command to all frames (ignoring errors and empty output).\n\
-Usage: faas COMMAND\n\
-shortcut for 'frame apply all -s COMMAND'"));
-
+Usage: faas [OPTION]... COMMAND\n\
+shortcut for 'frame apply all -s [OPTION]... COMMAND'\n\
+See \"help frame apply all\" for available options."));
+  set_cmd_completer_handle_brkchars (cmd, frame_apply_all_cmd_completer);
 
   add_prefix_cmd ("frame", class_stack,
 		  &frame_cmd.base_command, _("\
diff --git a/gdb/stack.h b/gdb/stack.h
index 6c6caa913e2..9ac77c06179 100644
--- a/gdb/stack.h
+++ b/gdb/stack.h
@@ -52,4 +52,9 @@  struct symtab* get_last_displayed_symtab (void);
 int get_last_displayed_line (void);
 symtab_and_line get_last_displayed_sal ();
 
+/* Completer for the "frame apply all" command.  */
+void frame_apply_all_cmd_completer (struct cmd_list_element *ignore,
+				    completion_tracker &tracker,
+				    const char *text, const char */*word*/);
+
 #endif /* #ifndef STACK_H */
diff --git a/gdb/testsuite/gdb.base/options.exp b/gdb/testsuite/gdb.base/options.exp
index 5a35074054e..195bbb168ae 100644
--- a/gdb/testsuite/gdb.base/options.exp
+++ b/gdb/testsuite/gdb.base/options.exp
@@ -25,6 +25,9 @@ 
 #  - print
 #  - compile print
 #  - backtrace
+#  - frame apply
+#  - faas
+#  - tfaas
 
 load_lib completion-support.exp
 
@@ -295,6 +298,79 @@  proc_with_prefix test-backtrace {} {
 	"backtrace (1 + xxx1"
 }
 
+# Basic option-machinery + "frame apply" command integration tests.
+proc_with_prefix test-frame-apply {} {
+    test_gdb_complete_unique "frame apply all" "frame apply all"
+
+    gdb_test "frame apply level 0-" \
+	"Please specify a command to apply on the selected frames"
+    test_gdb_complete_none "frame apply level 0-"
+
+    foreach cmd {
+	"frame apply all"
+	"frame apply 1"
+	"frame apply level 0"
+	"faas"
+	"tfaas"
+    } {
+	test_gdb_completion_offers_commands "$cmd "
+
+	# tfaas is silent on command error by design.  This procedure
+	# hides that aspect.  EXPECTED_RE is only considered when not
+	# testing with "faas"/"tfaas".
+	proc test_error_cmd {cmd arg expected_re} {
+	    if {$cmd == "tfaas"} {
+		gdb_test_no_output "$cmd$arg"
+	    } else {
+		gdb_test "$cmd$arg" $expected_re
+	    }
+	}
+	# Same, but for tests where both "faas" and "tfaas" are
+	# expected to be silent.
+	proc test_error_cmd2 {cmd arg expected_re} {
+	    if {$cmd == "tfaas" || $cmd == "faas"} {
+		gdb_test_no_output "$cmd$arg"
+	    } else {
+		gdb_test "$cmd$arg" $expected_re
+	    }
+	}
+
+	test_error_cmd $cmd " -" "Ambiguous option at: -"
+	test_gdb_complete_multiple "$cmd " "-" "" {
+	    "-c"
+	    "-past-entry"
+	    "-past-main"
+	    "-q"
+	    "-s"
+	}
+
+	with_test_prefix "no-trailing-space" {
+	    test_error_cmd $cmd " --" \
+		"Please specify a command to apply on the selected frames"
+	    test_gdb_complete_unique "$cmd --" "$cmd --"
+	}
+
+	with_test_prefix "trailing-space" {
+	    test_error_cmd $cmd " -- " \
+		"Please specify a command to apply on the selected frames"
+	    test_gdb_completion_offers_commands "$cmd -- "
+	}
+
+	# '-' is a valid TUI command.
+	test_error_cmd2 $cmd " -- -" \
+	    "Cannot enable the TUI when output is not a terminal"
+	test_gdb_complete_unique \
+	    "$cmd -- -" \
+	    "$cmd -- -"
+
+	test_error_cmd2 $cmd " -foo" \
+	    "Undefined command: \"-foo\".  Try \"help\"\\."
+	test_gdb_complete_none "$cmd -foo"
+
+	test_gdb_completion_offers_commands "$cmd -s "
+    }
+}
+
 # Miscellaneous tests.
 proc_with_prefix test-misc {variant} {
     global all_options
@@ -731,13 +807,28 @@  foreach_with_prefix cmd {
     test-enum $cmd
 }
 
-# Run the print integration tests.
-test-print ""
+# Run the print integration tests, both as "standalone", and under
+# "frame apply".  The latter checks that the "frame apply ... COMMAND"
+# commands recurse the completion machinery for COMMAND completion
+# correctly.
+foreach prefix {
+    ""
+    "frame apply all "
+    "frame apply 1 "
+    "frame apply level 0 "
+} {
+    test-print $prefix
+}
 
-# Same for "compile print".
+# Same for "compile print".  Not really a wrapper prefix command like
+# "frame apply", but similar enough that we test pretty much the same
+# things.
 if ![skip_compile_feature_tests] {
     test-print "compile "
 }
 
 # Basic "backtrace" integration tests.
 test-backtrace
+
+# Basic "frame apply" integration tests.
+test-frame-apply
diff --git a/gdb/thread.c b/gdb/thread.c
index a84dbf9fa1e..24906fa7d60 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -46,6 +46,7 @@ 
 #include <algorithm>
 #include "common/gdb_optional.h"
 #include "inline-frame.h"
+#include "stack.h"
 
 /* Definition of struct thread_info exported to gdbthread.h.  */
 
@@ -1653,7 +1654,7 @@  static void
 tfaas_command (const char *cmd, int from_tty)
 {
   std::string expanded
-    = std::string ("thread apply all -s frame apply all -s ") + cmd;
+    = std::string ("thread apply all -s -- frame apply all -s ") + cmd;
   execute_command (expanded.c_str (), from_tty);
 }
 
@@ -1938,6 +1939,7 @@  void
 _initialize_thread (void)
 {
   static struct cmd_list_element *thread_apply_list = NULL;
+  cmd_list_element *c;
 
   add_info ("threads", info_threads_command,
 	    _("Display currently known threads.\n\
@@ -1983,10 +1985,12 @@  Apply a command to all threads (ignoring errors and empty output).\n\
 Usage: taas COMMAND\n\
 shortcut for 'thread apply all -s COMMAND'"));
 
-  add_com ("tfaas", class_run, tfaas_command, _("\
+  c = add_com ("tfaas", class_run, tfaas_command, _("\
 Apply a command to all frames of all threads (ignoring errors and empty output).\n\
-Usage: tfaas COMMAND\n\
-shortcut for 'thread apply all -s frame apply all -s COMMAND'"));
+Usage: tfaas [OPTION]... COMMAND\n\
+shortcut for 'thread apply all -s -- frame apply all -s [OPTION]... COMMAND'\n\
+See \"help frame apply all\" for available options."));
+  set_cmd_completer_handle_brkchars (c, frame_apply_all_cmd_completer);
 
   add_cmd ("name", class_run, thread_name_command,
 	   _("Set the current thread's name.\n\