From patchwork Tue Jun 4 22:34:42 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 33003 Received: (qmail 60081 invoked by alias); 4 Jun 2019 22:40:40 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 60072 invoked by uid 89); 4 Jun 2019 22:40:39 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.8 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 04 Jun 2019 22:40:37 +0000 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9764F307D849 for ; Tue, 4 Jun 2019 22:35:22 +0000 (UTC) Received: from localhost.localdomain (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1B8C619C69 for ; Tue, 4 Jun 2019 22:35:21 +0000 (UTC) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH v3 22/24] Make "thread apply" use the gdb::option framework Date: Tue, 4 Jun 2019 23:34:42 +0100 Message-Id: <20190604223444.26472-23-palves@redhat.com> In-Reply-To: <20190604223444.26472-1-palves@redhat.com> References: <20190604223444.26472-1-palves@redhat.com> 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 * 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 * 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 195bbb168ae..4570c2a93d4 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 command error by design. This procedure + # hides the difference. EXPECTED_RE is only considered when + # not testing with "taas". + 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 " + } +} + # Miscellaneous 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..947427aa046 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; + +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 +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_nested_command_line (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_nested_command_line (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;