[PATCH 22/24] Make "thread apply" use the gdb::option framework

Pedro Alves
2019-05-22T20:54:07Z

Similarly to the "frame apply" patch, this makes the "thread apply"
family of commands -- "thread apply TID", "thread apply all" and
"taas" use the gdb::option framework for '-'-style options.

No new options are added, but there are some user-visible changes:

- Can now abbreviate and complete "-ascending"

- We now have a completer for "thread apply" commands

Can now complete options ("thread apply all -[TAB]"), and also,
'thread apply all COMMAND[TAB]' now does what you'd expect, by
making use of the new complete_command routine.

- "help" output tweaked with auto-generated option descriptions:

~~~
Usage: thread apply all [OPTION]... COMMAND
Prints per-inferior thread number and target system's thread id
followed by COMMAND output.

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

Options:
-ascending
Call COMMAND for all threads in ascending order.
The default is descending order.

-q
Disables printing the thread information.

-c
Print any error raised by COMMAND and continue.

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

The "By default ..." sentence is new as well.

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

* thread.c: Include "cli/cli-option.h".
(tp_array_compar_ascending): Global.
(tp_array_compar): Delete function.
(tp_array_compar_ascending, tp_array_compar_descending): New
functions.
(ascending_option_def, qcs_flag_option_def)
(thr_qcs_flags_option_defs)
(make_thread_apply_all_options_def_group)
(make_thread_apply_options_def_group): New.
(thread_apply_all_command): Use gdb::option::process_options.
(thread_apply_command_completer)
(thread_apply_all_command_completer): New.
(thread_apply_command): Use gdb::option::process_options.
(_initialize_thread): Delete THREAD_APPLY_FLAGS_HELP, replace it
with a new THREAD_APPLY_OPTION_HELP. Use gdb::option::build_help
to generate help text of "thread apply". Adjust "taas"'s help.
* tid-parse.c (tid_range_parser::in_thread_range): New method.
* tid-parse.h (tid_range_parser::in_thread_range): New method.

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

* gdb.base/options.exp (test-thread-apply): New.
(top level): Call it.
---
gdb/testsuite/gdb.base/options.exp | 95 +++++++++++++-
gdb/thread.c | 258 ++++++++++++++++++++++++++++---------
gdb/tid-parse.c | 6 +
gdb/tid-parse.h | 3 +
4 files changed, 301 insertions(+), 61 deletions(-)

diff --git a/gdb/testsuite/gdb.base/options.exp b/gdb/testsuite/gdb.base/options.exp
index df5d86c0ece..a4954b5041c 100644
--- a/gdb/testsuite/gdb.base/options.exp
+++ b/gdb/testsuite/gdb.base/options.exp
@@ -28,6 +28,8 @@
# - frame apply
# - faas
# - tfaas
+# - thread apply
+# - taas

load_lib completion-support.exp

@@ -371,6 +373,87 @@ proc_with_prefix test-frame-apply {} {
}
}

+# Basic option-machinery + "thread apply" command integration tests.
+proc_with_prefix test-thread-apply {} {
+
+ test_gdb_complete_unique "thread apply all" "thread apply all"
+ test_gdb_complete_unique "taas" "taas"
+
+ gdb_test "thread apply 1-" \
+ "inverted range"
+ test_gdb_complete_none "frame apply level 1-"
+
+ foreach cmd {
+ "thread apply all"
+ "thread apply 1"
+ "taas"
+ } {
+ test_gdb_completion_offers_commands "$cmd "
+
+ # taas is silent on comand error by design. This procedure
+ # hides the difference. EXPECTED_RE is only considered when
+ # not testing with "tfaas".
+ proc test_invalid_cmd {cmd arg expected_re} {
+ if {$cmd != "taas"} {
+ gdb_test "$cmd$arg" $expected_re
+ } else {
+ gdb_test_no_output "$cmd$arg"
+ }
+ }
+
+ gdb_test "$cmd -" "Ambiguous option at: -"
+
+ if {$cmd != "thread apply 1"} {
+ test_gdb_complete_multiple "$cmd " "-" "" {
+ "-ascending"
+ "-c"
+ "-q"
+ "-s"
+ }
+ } else {
+ # "-ascending" only works with "all".
+ test_gdb_complete_multiple "$cmd " "-" "" {
+ "-c"
+ "-q"
+ "-s"
+ }
+ }
+
+ if {$cmd == "thread apply all" || $cmd == "taas"} {
+ set errmsg \
+ "Please specify a command at the end of 'thread apply all'"
+ } elseif {$cmd == "thread apply 1"} {
+ set errmsg \
+ "Please specify a command following the thread ID list"
+ } else {
+ error "unexpected cmd: $cmd"
+ }
+
+ with_test_prefix "no-trailing-space" {
+ gdb_test "$cmd --" $errmsg
+ test_gdb_complete_unique "$cmd --" "$cmd --"
+ }
+
+ with_test_prefix "trailing-space" {
+ gdb_test "$cmd -- " $errmsg
+ test_gdb_completion_offers_commands "$cmd -- "
+ }
+
+ # '-' is a valid TUI command.
+ test_invalid_cmd "$cmd" " -- -" \
+ "Cannot enable the TUI when output is not a terminal"
+ test_gdb_complete_unique \
+ "$cmd -- -" \
+ "$cmd -- -"
+
+ test_invalid_cmd $cmd " -foo" \
+ "Undefined command: \"-foo\". Try \"help\"\\."
+ test_gdb_complete_none "$cmd -foo"
+
+ test_gdb_completion_offers_commands "$cmd -c "
+ }
+}
+
# Miscelaneous tests.
proc_with_prefix test-misc {variant} {
global all_options
@@ -808,14 +891,17 @@ foreach_with_prefix cmd {
}

# 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.
+# "frame/thread apply". The latter checks that the "frame/thread
+# apply ... COMMAND" commands recurse the completion machinery for
+# COMMAND completion correctly.
foreach prefix {
""
"frame apply all "
"frame apply 1 "
"frame apply level 0 "
+ "thread apply all "
+ "thread apply 1 "
+ "thread apply 1 frame apply 1 "
} {
test-print $prefix
}
@@ -832,3 +918,6 @@ test-backtrace

# Basic "frame apply" integration tests.
test-frame-apply
+
+# Basic "thread apply" integration tests.
+test-thread-apply
diff --git a/gdb/thread.c b/gdb/thread.c
index ea87f51c6e6..d3362090244 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -39,6 +39,7 @@
#include "observable.h"
#include "annotate.h"
#include "cli/cli-decode.h"
+#include "cli/cli-option.h"
#include "gdb_regex.h"
#include "cli/cli-utils.h"
#include "thread-fsm.h"
@@ -1426,30 +1427,30 @@ print_thread_id (struct thread_info *thr)
return s;
}

-/* If true, tp_array_compar should sort in ascending order, otherwise
- in descending order. */
+/* Sort an array of struct thread_info pointers by thread ID (first by
+ inferior number, and then by per-inferior thread number). Sorts in
+ ascending order. */

-static bool tp_array_compar_ascending;
+static bool
+tp_array_compar_ascending (const thread_info *a, const thread_info *b)
+{
+ if (a->inf->num != b->inf->num)
+ return a->inf->num < b->inf->num;
+
+ return (a->per_inf_num < b->per_inf_num);
+}

-/* Sort an array for struct thread_info pointers by thread ID (first
- by inferior number, and then by per-inferior thread number). The
- order is determined by TP_ARRAY_COMPAR_ASCENDING. */
+/* Sort an array of struct thread_info pointers by thread ID (first by
+ inferior number, and then by per-inferior thread number). Sorts in
+ descending order. */

static bool
-tp_array_compar (const thread_info *a, const thread_info *b)
+tp_array_compar_descending (const thread_info *a, const thread_info *b)
{
if (a->inf->num != b->inf->num)
- {
- if (tp_array_compar_ascending)
- return a->inf->num < b->inf->num;
- else
- return a->inf->num > b->inf->num;
- }
+ return a->inf->num > b->inf->num;

- if (tp_array_compar_ascending)
- return (a->per_inf_num < b->per_inf_num);
- else
- return (a->per_inf_num > b->per_inf_num);
+ return (a->per_inf_num > b->per_inf_num);
}

/* Switch to thread THR and execute CMD.
@@ -1490,6 +1491,60 @@ thr_try_catch_cmd (thread_info *thr, const char *cmd, int from_tty,
}
}

+/* Option definition of "thread apply"'s "-ascending" option. */
+
+static const gdb::option::flag_option_def<> ascending_option_def = {
+ "ascending",
+ N_("\
+Call COMMAND for all threads in ascending order.\n\
+The default is descending order."),
+};
+
+/* The qcs command line flags for the "thread apply" commands. Keep
+ this in sync with the "frame apply" commands. */
+
+using qcs_flag_option_def
+ = gdb::option::flag_option_def<qcs_flags>;
+
+static const gdb::option::option_def thr_qcs_flags_option_defs[] = {
+ qcs_flag_option_def {
+ "q", [] (qcs_flags *opt) { return &opt->quiet; },
+ N_("Disables printing the thread 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 for the "thread apply all" options, with
+ ASCENDING and FLAGS as context. */
+
+static inline std::array<gdb::option::option_def_group, 2>
+make_thread_apply_all_options_def_group (int *ascending,
+ qcs_flags *flags)
+{
+ return {{
+ { ascending_option_def.def (), ascending},
+ { thr_qcs_flags_option_defs, flags },
+ }};
+}
+
+/* Create an option_def_group for the "thread apply" options, with
+ FLAGS as context. */
+
+static inline gdb::option::option_def_group
+make_thread_apply_options_def_group (qcs_flags *flags)
+{
+ return {thr_qcs_flags_option_defs, flags};
+}
+
/* Apply a GDB command to a list of threads. List syntax is a whitespace
separated list of numbers, or ranges, or the keyword `all'. Ranges consist
of two numbers separated by a hyphen. Examples:
@@ -1501,24 +1556,15 @@ thr_try_catch_cmd (thread_info *thr, const char *cmd, int from_tty,
static void
thread_apply_all_command (const char *cmd, int from_tty)
{
+ int ascending = false;
qcs_flags flags;

- tp_array_compar_ascending = false;
+ auto group = make_thread_apply_all_options_def_group (&ascending,
+ &flags);
+ gdb::option::process_options
+ (&cmd, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group);

- while (cmd != NULL)
- {
- if (check_for_argument (&cmd, "-ascending", strlen ("-ascending")))
- {
- cmd = skip_spaces (cmd);
- tp_array_compar_ascending = true;
- continue;
- }
-
- if (parse_flags_qcs ("thread apply all", &cmd, &flags))
- continue;
-
- break;
- }
+ validate_flags_qcs ("thread apply all", &flags);

if (cmd == NULL || *cmd == '\000')
error (_("Please specify a command at the end of 'thread apply all'"));
@@ -1544,7 +1590,10 @@ thread_apply_all_command (const char *cmd, int from_tty)
exit. */
scoped_inc_dec_ref inc_dec_ref (thr_list_cpy);

- std::sort (thr_list_cpy.begin (), thr_list_cpy.end (), tp_array_compar);
+ auto *sorter = (ascending
+ ? tp_array_compar_ascending
+ : tp_array_compar_descending);
+ std::sort (thr_list_cpy.begin (), thr_list_cpy.end (), sorter);

scoped_restore_current_thread restore_thread;

@@ -1554,6 +1603,81 @@ thread_apply_all_command (const char *cmd, int from_tty)
}
}

+/* Completer for "thread apply [ID list]". */
+
+static void
+thread_apply_command_completer (cmd_list_element *ignore,
+ completion_tracker &tracker,
+ const char *text, const char * /*word*/)
+{
+ /* Don't leave this to complete_options because there's an early
+ return below. */
+ tracker.set_use_custom_word_point (true);
+
+ tid_range_parser parser;
+ parser.init (text, current_inferior ()->num);
+
+ try
+ {
+ while (!parser.finished ())
+ {
+ int inf_num, thr_start, thr_end;
+
+ if (!parser.get_tid_range (&inf_num, &thr_start, &thr_end))
+ break;
+
+ if (parser.in_star_range () || parser.in_thread_range ())
+ parser.skip_range ();
+ }
+ }
+ catch (const gdb_exception_error &ex)
+ {
+ /* get_tid_range 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 = parser.cur_tok ();
+
+ if (cmd == text)
+ {
+ /* No thread ID list yet. */
+ return;
+ }
+
+ /* Check if we're past a valid thread ID list already. */
+ if (parser.finished ()
+ && cmd > text && !isspace (cmd[-1]))
+ return;
+
+ /* We're past the thread ID list, advance word point. */
+ tracker.advance_custom_word_point_by (cmd - text);
+ text = cmd;
+
+ const auto group = make_thread_apply_options_def_group (nullptr);
+ if (gdb::option::complete_options
+ (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group))
+ return;
+
+ complete_command (tracker, text);
+}
+
+/* Completer for "thread apply all". */
+
+static void
+thread_apply_all_command_completer (cmd_list_element *ignore,
+ completion_tracker &tracker,
+ const char *text, const char *word)
+{
+ const auto group = make_thread_apply_all_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);
+}
+
/* Implementation of the "thread apply" command. */

static void
@@ -1577,8 +1701,11 @@ thread_apply_command (const char *tidlist, int from_tty)

cmd = parser.cur_tok ();

- while (parse_flags_qcs ("thread apply", &cmd, &flags))
- ;
+ auto group = make_thread_apply_options_def_group (&flags);
+ gdb::option::process_options
+ (&cmd, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group);
+
+ validate_flags_qcs ("thread apply", &flags);

if (*cmd == '\0')
error (_("Please specify a command following the thread ID list"));
@@ -1953,37 +2080,52 @@ Use this command to switch between threads.\n\
The new thread ID must be currently known."),
&thread_cmd_list, "thread ", 1, &cmdlist);

-#define THREAD_APPLY_FLAGS_HELP "\
+#define THREAD_APPLY_OPTION_HELP "\
Prints per-inferior thread number and target system's thread id\n\
followed by COMMAND output.\n\
-FLAG arguments are -q (quiet), -c (continue), -s (silent).\n\
-Flag -q disables printing the thread information.\n\
-By default, if a COMMAND raises an error, thread 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_run, thread_apply_command,
- _("Apply a command to a list of threads.\n\
-Usage: thread apply ID... [FLAG]... COMMAND\n\
+\n\
+By default, an error raised during the execution of COMMAND\n\
+aborts \"thread apply\".\n\
+\n\
+Options:\n\
+%OPTIONS%"
+
+ const auto thread_apply_opts = make_thread_apply_options_def_group (nullptr);
+
+ static std::string thread_apply_help = gdb::option::build_help (N_("\
+Apply a command to a list of threads.\n\
+Usage: thread apply ID... [OPTION]... COMMAND\n\
ID is a space-separated list of IDs of threads to apply COMMAND on.\n"
-THREAD_APPLY_FLAGS_HELP),
- &thread_apply_list, "thread apply ", 1, &thread_cmd_list);
+THREAD_APPLY_OPTION_HELP),
+ thread_apply_opts);
+
+ c = add_prefix_cmd ("apply", class_run, thread_apply_command,
+ thread_apply_help.c_str (),
+ &thread_apply_list, "thread apply ", 1,
+ &thread_cmd_list);
+ set_cmd_completer_handle_brkchars (c, thread_apply_command_completer);

- add_cmd ("all", class_run, thread_apply_all_command,
- _("\
+ const auto thread_apply_all_opts
+ = make_thread_apply_all_options_def_group (nullptr, nullptr);
+
+ static std::string thread_apply_all_help = gdb::option::build_help (N_("\
Apply a command to all threads.\n\
\n\
-Usage: thread apply all [-ascending] [FLAG]... COMMAND\n\
--ascending: Call COMMAND for all threads in ascending order.\n\
- The default is descending order.\n"
-THREAD_APPLY_FLAGS_HELP),
- &thread_apply_list);
+Usage: thread apply all [OPTION]... COMMAND\n"
+THREAD_APPLY_OPTION_HELP),
+ thread_apply_all_opts);
+
+ c = add_cmd ("all", class_run, thread_apply_all_command,
+ thread_apply_all_help.c_str (),
+ &thread_apply_list);
+ set_cmd_completer_handle_brkchars (c, thread_apply_all_command_completer);

- add_com ("taas", class_run, taas_command, _("\
+ c = add_com ("taas", class_run, taas_command, _("\
Apply a command to all threads (ignoring errors and empty output).\n\
-Usage: taas COMMAND\n\
-shortcut for 'thread apply all -s COMMAND'"));
+Usage: taas [OPTION]... COMMAND\n\
+shortcut for 'thread apply all -s [OPTION]... COMMAND'\n\
+See \"help thread apply all\" for available options."));
+ set_cmd_completer_handle_brkchars (c, thread_apply_all_command_completer);

c = add_com ("tfaas", class_run, tfaas_command, _("\
Apply a command to all frames of all threads (ignoring errors and empty output).\n\
diff --git a/gdb/tid-parse.c b/gdb/tid-parse.c
index 07d7d2c3b2a..6ad8b1f4aff 100644
--- a/gdb/tid-parse.c
+++ b/gdb/tid-parse.c
@@ -307,6 +307,12 @@ tid_range_parser::in_star_range () const
return m_state == STATE_STAR_RANGE;
}

+bool
+tid_range_parser::in_thread_range () const
+{
+ return m_state == STATE_THREAD_RANGE;
+}
+
/* See tid-parse.h. */

int
diff --git a/gdb/tid-parse.h b/gdb/tid-parse.h
index bc70b2d4e4c..da8ead13653 100644
--- a/gdb/tid-parse.h
+++ b/gdb/tid-parse.h
@@ -114,6 +114,9 @@ public:
range. */
bool in_star_range () const;

+ /* Returns true if processing a thread range (e.g., 1.2-3). */
+ bool in_thread_range () const;
+
/* Returns true if parsing has completed. */
bool finished () const;

--
2.14.5