* [RFA 5/8] Announce in NEWS 'frame apply', faas, taas, tfaas commands and -FLAGS... arg for thread apply
2018-05-21 11:07 [RFA 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND' Philippe Waroquiers
@ 2018-05-21 11:07 ` Philippe Waroquiers
2018-05-21 16:26 ` Eli Zaretskii
2018-05-21 11:07 ` [RFA 3/8] Add -FLAGS... argument to 'thread apply' Philippe Waroquiers
` (8 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Philippe Waroquiers @ 2018-05-21 11:07 UTC (permalink / raw)
To: gdb-patches; +Cc: Philippe Waroquiers
Announce the user visible changes in NEWS:
'frame apply', faas, taas, tfaas commands and -FLAGS... arg for thread apply.
---
gdb/NEWS | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/gdb/NEWS b/gdb/NEWS
index cef558039e..b346adb0b6 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -14,6 +14,24 @@
* New commands
+frame apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND
+ Apply a command to a number of frames.
+ The FLAGS allows to control what output to produce and how to handle
+ errors raised when applying COMMAND to a frame.
+
+taas COMMAND
+ Apply a command to all threads (ignoring errors and empty output).
+ Shortcut for 'thread apply all -s COMMAND'.
+
+faas COMMAND
+ Apply a command to all frames (ignoring errors and empty output).
+ Shortcut for 'frame apply all -s COMMAND'.
+
+tfaas COMMAND
+ Apply a command to all frames of all threads (ignoring errors and empty
+ output).
+ Shortcut for 'thread apply all -s frame apply all -s COMMAND'.
+
set debug fbsd-nat
show debug fbsd-nat
Control display of debugging info regarding the FreeBSD native target.
@@ -27,6 +45,13 @@ set|show record btrace cpu
Controls the processor to be used for enabling errata workarounds for
branch trace decode.
+* Changed commands
+
+thread apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND
+ The 'thread apply' command accepts a new argument FLAGS.
+ The FLAGS allows to control what output to produce and how to handle errors
+ raised when applying COMMAND to a frame.
+
* Python API
** Type alignment is now exposed via the "align" attribute of a gdb.Type.
--
2.11.0
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [RFA 5/8] Announce in NEWS 'frame apply', faas, taas, tfaas commands and -FLAGS... arg for thread apply
2018-05-21 11:07 ` [RFA 5/8] Announce in NEWS 'frame apply', faas, taas, tfaas commands and -FLAGS... arg for thread apply Philippe Waroquiers
@ 2018-05-21 16:26 ` Eli Zaretskii
0 siblings, 0 replies; 17+ messages in thread
From: Eli Zaretskii @ 2018-05-21 16:26 UTC (permalink / raw)
To: Philippe Waroquiers; +Cc: gdb-patches
> From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Date: Mon, 21 May 2018 13:06:48 +0200
>
> Announce the user visible changes in NEWS:
> 'frame apply', faas, taas, tfaas commands and -FLAGS... arg for thread apply.
> ---
> gdb/NEWS | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
This part is OK, thanks.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFA 3/8] Add -FLAGS... argument to 'thread apply'.
2018-05-21 11:07 [RFA 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND' Philippe Waroquiers
2018-05-21 11:07 ` [RFA 5/8] Announce in NEWS 'frame apply', faas, taas, tfaas commands and -FLAGS... arg for thread apply Philippe Waroquiers
@ 2018-05-21 11:07 ` Philippe Waroquiers
2018-05-21 11:07 ` [RFA 7/8] Modify gdb.threads/threads.exp to test FLAGS vqcs for thread apply Philippe Waroquiers
` (7 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Philippe Waroquiers @ 2018-05-21 11:07 UTC (permalink / raw)
To: gdb-patches; +Cc: Philippe Waroquiers
Add -FLAGS... argument to 'thread apply'.
Also implements 2 new commands:
taas COMMAND
shortcut for 'thread apply all -s COMMAND'
tfaas COMMAND
shortcut for 'thread apply all -s frame apply all -s COMMAND'
---
gdb/thread.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 112 insertions(+), 20 deletions(-)
diff --git a/gdb/thread.c b/gdb/thread.c
index a09d7e0ba0..9d0414e22f 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1581,6 +1581,45 @@ tp_array_compar (const thread_info *a, const thread_info *b)
return (a->per_inf_num > b->per_inf_num);
}
+/* Switch to thread THR and executes CMD.
+ PRINT_WHAT_V verbosity controls the printing of the thread information.
+ CONT and SILENT control how to handle errors. */
+
+static void
+thr_try_catch_cmd (thread_info *thr, const char *cmd, int from_tty,
+ int print_what_v,
+ bool cont, bool silent)
+{
+ switch_to_thread (thr->ptid);
+ TRY
+ {
+ std::string cmd_result = execute_command_to_string (cmd, from_tty);
+ if (!silent || cmd_result.length () > 0)
+ {
+ if (print_what_v > 0)
+ printf_filtered (_("\nThread %s (%s):\n"),
+ print_thread_id (thr),
+ target_pid_to_str (inferior_ptid));
+ printf_filtered ("%s", cmd_result.c_str ());
+ }
+ }
+ CATCH (ex, RETURN_MASK_ERROR)
+ {
+ if (!silent)
+ {
+ if (print_what_v > 0)
+ printf_filtered (_("\nThread %s (%s):\n"),
+ print_thread_id (thr),
+ target_pid_to_str (inferior_ptid));
+ if (cont)
+ printf_filtered ("%s\n", ex.message);
+ else
+ throw_exception (ex);
+ }
+ }
+ END_CATCH;
+}
+
/* 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:
@@ -1592,6 +1631,10 @@ tp_array_compar (const thread_info *a, const thread_info *b)
static void
thread_apply_all_command (const char *cmd, int from_tty)
{
+ int print_what_v = 1; /* Print thread id/thread/lwp. */
+ bool cont;
+ bool silent;
+
tp_array_compar_ascending = false;
if (cmd != NULL
&& check_for_argument (&cmd, "-ascending", strlen ("-ascending")))
@@ -1600,8 +1643,13 @@ thread_apply_all_command (const char *cmd, int from_tty)
tp_array_compar_ascending = true;
}
+ if (cmd != NULL)
+ check_for_flags_vqcs ("thread apply all", &cmd,
+ &print_what_v, 1,
+ &cont, &silent);
+
if (cmd == NULL || *cmd == '\000')
- error (_("Please specify a command following the thread ID list"));
+ error (_("Please specify a command at the end of 'thread apply all'"));
update_thread_list ();
@@ -1637,14 +1685,9 @@ thread_apply_all_command (const char *cmd, int from_tty)
for (thread_info *thr : thr_list_cpy)
if (thread_alive (thr))
- {
- switch_to_thread (thr->ptid);
- printf_filtered (_("\nThread %s (%s):\n"),
- print_thread_id (thr),
- target_pid_to_str (inferior_ptid));
-
- execute_command (cmd, from_tty);
- }
+ thr_try_catch_cmd (thr, cmd, from_tty,
+ print_what_v,
+ cont, silent);
}
}
@@ -1653,7 +1696,11 @@ thread_apply_all_command (const char *cmd, int from_tty)
static void
thread_apply_command (const char *tidlist, int from_tty)
{
+ int print_what_v = 1; /* Print thread id/thread/lwp. */
+ bool cont;
+ bool silent;
const char *cmd = NULL;
+ const char *cmd_or_flags;
tid_range_parser parser;
if (tidlist == NULL || *tidlist == '\000')
@@ -1671,6 +1718,12 @@ thread_apply_command (const char *tidlist, int from_tty)
}
}
+ cmd_or_flags = cmd;
+ if (cmd != NULL)
+ check_for_flags_vqcs ("thread apply", &cmd,
+ &print_what_v, 1,
+ &cont, &silent);
+
if (cmd == NULL)
error (_("Please specify a command following the thread ID list"));
@@ -1680,7 +1733,7 @@ thread_apply_command (const char *tidlist, int from_tty)
scoped_restore_current_thread restore_thread;
parser.init (tidlist, current_inferior ()->num);
- while (!parser.finished () && parser.cur_tok () < cmd)
+ while (!parser.finished () && parser.cur_tok () < cmd_or_flags)
{
struct thread_info *tp = NULL;
struct inferior *inf;
@@ -1725,14 +1778,31 @@ thread_apply_command (const char *tidlist, int from_tty)
continue;
}
- switch_to_thread (tp->ptid);
-
- printf_filtered (_("\nThread %s (%s):\n"), print_thread_id (tp),
- target_pid_to_str (inferior_ptid));
- execute_command (cmd, from_tty);
+ thr_try_catch_cmd (tp, cmd, from_tty,
+ print_what_v,
+ cont, silent);
}
}
+
+/* Implementation of the "taas" command. */
+
+static void
+taas_command (const char *cmd, int from_tty)
+{
+ std::string expanded = std::string ("thread apply all -s ") + std::string (cmd);
+ execute_command (expanded.c_str (), from_tty);
+}
+
+/* Implementation of the "tfaas" command. */
+
+static void
+tfaas_command (const char *cmd, int from_tty)
+{
+ std::string expanded = std::string ("thread apply all -s frame apply all -s ") + std::string (cmd);
+ execute_command (expanded.c_str (), from_tty);
+}
+
/* Switch to the specified thread, or print the current thread. */
void
@@ -2027,22 +2097,44 @@ 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 "\
+FLAGS letters are v(increase verbosity), q(decrease verbosity)\n\
+ c(continue), s(silent).\n\
+Verbosity (default 1) controls what to print for a thread:\n\
+ 0 : no thread info is printed\n\
+ 1 : print per-inferior thread number and target system's thread id\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... COMMAND\n\
-ID is a space-separated list of IDs of threads to apply COMMAND on."),
+Usage: thread apply ID... [-FLAGS...] 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);
add_cmd ("all", class_run, thread_apply_all_command,
_("\
Apply a command to all threads.\n\
\n\
-Usage: thread apply all [-ascending] COMMAND\n\
+Usage: thread apply all [-ascending] [-FLAGS...] COMMAND\n\
-ascending: Call COMMAND for all threads in ascending order.\n\
- The default is descending order.\
-"),
+ The default is descending order.\n"
+THREAD_APPLY_FLAGS_HELP),
&thread_apply_list);
+ 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'"));
+
+ 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'"));
+
add_cmd ("name", class_run, thread_name_command,
_("Set the current thread's name.\n\
Usage: thread name [NAME]\n\
--
2.11.0
^ permalink raw reply [flat|nested] 17+ messages in thread* [RFA 7/8] Modify gdb.threads/threads.exp to test FLAGS vqcs for thread apply
2018-05-21 11:07 [RFA 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND' Philippe Waroquiers
2018-05-21 11:07 ` [RFA 5/8] Announce in NEWS 'frame apply', faas, taas, tfaas commands and -FLAGS... arg for thread apply Philippe Waroquiers
2018-05-21 11:07 ` [RFA 3/8] Add -FLAGS... argument to 'thread apply' Philippe Waroquiers
@ 2018-05-21 11:07 ` Philippe Waroquiers
2018-05-21 11:07 ` [RFA 1/8] Add helper functions check_for_flags and check_for_flags_vqcs Philippe Waroquiers
` (6 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Philippe Waroquiers @ 2018-05-21 11:07 UTC (permalink / raw)
To: gdb-patches; +Cc: Philippe Waroquiers
---
gdb/testsuite/gdb.threads/pthreads.exp | 65 ++++++++++++++++++++++++++++++++++
1 file changed, 65 insertions(+)
diff --git a/gdb/testsuite/gdb.threads/pthreads.exp b/gdb/testsuite/gdb.threads/pthreads.exp
index 830432b833..5e05716117 100644
--- a/gdb/testsuite/gdb.threads/pthreads.exp
+++ b/gdb/testsuite/gdb.threads/pthreads.exp
@@ -267,6 +267,70 @@ proc check_backtraces {} {
}
}
+proc check_vqcs {} {
+ set any "\[^\r\n\]*"
+ set ws "\[ \t\]\+"
+ set number "\[0-9]\+"
+
+ # check -c (continue) and -s (silently continue) flags
+ gdb_test "thread apply 2-3 p notfound" \
+ [multi_line \
+ "" \
+ "Thread 2 ${any}" \
+ "No symbol \\\"notfound\\\" in current context." \
+ ] \
+ "Run a failing command that aborts thread apply"
+
+ gdb_test "thread apply 2-3 -c p notfound" \
+ [multi_line \
+ "" \
+ "Thread 2 ${any}" \
+ "No symbol \\\"notfound\\\" in current context." \
+ "" \
+ "Thread 3 ${any}" \
+ "No symbol \\\"notfound\\\" in current context." \
+ ] \
+ "Run a failing command, -c to continue"
+
+ foreach cmd {"thread apply all -s frame apply all -s p i" "tfaas p i" "taas faas p i"} {
+ gdb_test $cmd \
+ [multi_line \
+ "" \
+ "Thread 3 ${any}" \
+ "#${number}${ws}${any} in thread2 ${any}" \
+ "\\\$\[0-9]+ = ${number}${any}" \
+ "" \
+ "Thread 2 ${any}" \
+ "#${number}${ws}${any} in thread1 ${any}" \
+ "\\\$\[0-9]+ = ${number}${any}" \
+ ] \
+ "Run a failing command except in one frame of thread 2,3, -s to silently continue"
+ }
+
+ # Verbosity tests
+ gdb_test "thread apply all -sq frame apply all -s p i" \
+ [multi_line \
+ "#${number}${ws}${any} in thread2 ${any}" \
+ "\\\$\[0-9]+ = ${number}${any}" \
+ "#${number}${ws}${any} in thread1 ${any}" \
+ "\\\$\[0-9]+ = ${number}${any}" \
+ ] \
+ "Run a failing command except in one frame of thread 2,3, -s to silently continue. Verbosity 0 threads"
+
+ gdb_test "thread apply all -sq frame apply all -sqq p i" \
+ [multi_line \
+ "\\\$\[0-9]+ = ${number}${any}" \
+ "\\\$\[0-9]+ = ${number}${any}" \
+ ] \
+ "Run a failing command except in one frame of thread 2,3, -s to silently continue. Verbosity 0 threads and frames"
+
+ # check invalid flag combinations
+ gdb_test "thread apply all -cs p 1" \
+ "thread apply all: flags c and s are mutually exclusive" \
+ "Check c and s cannot be used simultaneously"
+
+}
+
if [runto_main] then {
if [test_startup] then {
if [check_control_c] then {
@@ -274,5 +338,6 @@ if [runto_main] then {
return
}
check_backtraces
+ check_vqcs
}
}
--
2.11.0
^ permalink raw reply [flat|nested] 17+ messages in thread* [RFA 1/8] Add helper functions check_for_flags and check_for_flags_vqcs
2018-05-21 11:07 [RFA 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND' Philippe Waroquiers
` (2 preceding siblings ...)
2018-05-21 11:07 ` [RFA 7/8] Modify gdb.threads/threads.exp to test FLAGS vqcs for thread apply Philippe Waroquiers
@ 2018-05-21 11:07 ` Philippe Waroquiers
2018-05-21 11:07 ` [RFA 4/8] Documentation changes for 'frame apply' and 'thread apply' Philippe Waroquiers
` (5 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Philippe Waroquiers @ 2018-05-21 11:07 UTC (permalink / raw)
To: gdb-patches; +Cc: Philippe Waroquiers
Add helper functions check_for_flags and check_for_flags_vqcs
check_for_flags helper function allows to look for a set of flags at
the start of a string.
check_for_flags_vqcs is a specialised helper function to handle
the flags vqcs, that are used in the new command 'frame apply'
and in the command 'thread apply.
Modify number_or_range_parser::get_number to differentiate a
- followed by digits from a - followed by a flag (i.e. an alpha).
That is needed for the addition of the optional -FLAGS... argument to
thread apply ID... [-FLAGS...] COMMAND
The error message for 'thread apply -$unknownconvvar p 1'
is now the more clear:
Convenience variable must have integer value.
Invalid thread ID: -$unknownconvvar p 1
instead of previously:
negative value
---
gdb/cli/cli-utils.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++-
gdb/cli/cli-utils.h | 30 ++++++++++++++
2 files changed, 138 insertions(+), 2 deletions(-)
diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
index c55b5035e4..de2220c21a 100644
--- a/gdb/cli/cli-utils.c
+++ b/gdb/cli/cli-utils.c
@@ -169,7 +169,10 @@ number_or_range_parser::get_number ()
/* Default case: state->m_cur_tok is pointing either to a solo
number, or to the first number of a range. */
m_last_retval = get_number_trailer (&m_cur_tok, '-');
- if (*m_cur_tok == '-')
+ /* If get_number_trailer has found a -, it might be the start
+ of flags. So, do not parse a range if the - is followed by
+ an alpha. */
+ if (*m_cur_tok == '-' && !isalpha (*(m_cur_tok + 1)))
{
const char **temp;
@@ -196,7 +199,17 @@ number_or_range_parser::get_number ()
}
}
else
- error (_("negative value"));
+ {
+ if (isdigit (*(m_cur_tok + 1)))
+ error (_("negative value"));
+ if (*(m_cur_tok + 1) == '$')
+ {
+ // Convenience variable.
+ m_last_retval = ::get_number (&m_cur_tok);
+ if (m_last_retval < 0)
+ error (_("negative value"));
+ }
+ }
m_finished = *m_cur_tok == '\0';
return m_last_retval;
}
@@ -304,3 +317,96 @@ check_for_argument (const char **str, const char *arg, int arg_len)
}
return 0;
}
+
+/* See documentation in cli-utils.h. */
+
+int
+check_for_flags (const char **str, const char *flags,
+ int *flags_counts)
+{
+ const char *p = *str;
+ bool all_valid = true;
+
+ /* First set the flags_counts to 0. */
+ memset (flags_counts, 0, sizeof (flags_counts[0]) * strlen (flags));
+
+ if (*p != '-')
+ return 0;
+
+ p++;
+ /* If - is followed by anything else than an alpha (including \0),
+ then we do not have flags. This also cover the case of a command
+ that accepts optional flags followed by a negative integer.
+ In such a case, we will not handle a negative integer as invalid
+ flags : rather let the caller handler the negative integer. */
+ if (!isalpha (*p))
+ return 0;
+
+ /* We have a set of flags :
+ Scan and validate the flags, and update flags_counts for valid flags. */
+ while (*p != '\0' && !isspace (*p))
+ {
+ const char *f = flags;
+ bool valid = false;
+
+ while (*f != '\0')
+ {
+ valid = *f == *p;
+ if (valid)
+ {
+ flags_counts[f - flags]++;
+ break;
+ }
+ f++;
+ }
+ all_valid = all_valid && valid;
+ p++;
+ }
+
+ /* Skip the space(s). */
+ p = skip_spaces (p);
+
+ if (all_valid)
+ {
+ *str = p;
+ return 1;
+ }
+ else
+ return -1;
+}
+
+/* See documentation in cli-utils.h. */
+
+int
+check_for_flags_vqcs (const char *which_command, const char **str,
+ int *print_what_v, int max_v,
+ bool *cont, bool *silent)
+{
+ const char *flags = "vqcs";
+ int flags_counts[strlen (flags)];
+ int res;
+
+ *cont = false;
+ *silent = false;
+
+ res = check_for_flags (str, flags, flags_counts);
+ if (res == 0)
+ return 0;
+ if (res == -1)
+ error (_("%s only accepts flags %s"), which_command, flags);
+ gdb_assert (res == 1);
+
+ *print_what_v = *print_what_v + flags_counts[0] - flags_counts[1];
+ if (*print_what_v < 0)
+ *print_what_v = 0;
+ if (*print_what_v > max_v)
+ *print_what_v = max_v;
+
+ *cont = flags_counts[2] > 0;
+ *silent = flags_counts[3] > 0;
+ if (*cont && *silent)
+ error (_("%s: flags c and s are mutually exclusive"),
+ which_command);
+
+ return res;
+}
diff --git a/gdb/cli/cli-utils.h b/gdb/cli/cli-utils.h
index e34ee0df37..7b94b82844 100644
--- a/gdb/cli/cli-utils.h
+++ b/gdb/cli/cli-utils.h
@@ -173,4 +173,34 @@ check_for_argument (char **str, const char *arg, int arg_len)
arg, arg_len);
}
+/* A helper function that looks for a set of flags at the start of a
+ string. The possible flags are given as a null terminated string.
+ The flags must also either be at the end of the string,
+ or be followed by whitespace. Returns 1 if it finds only valid flags,
+ 0 if no flags are found, -1 if invalid flags are found.
+ FLAGS_COUNTS must be an int array of length equal to strlen (flags).
+ Sets *FLAGS_COUNTS to the number of times the corresponding flag
+ was found in *STR.
+ If only valid flags are found, it updates *STR. Note that a negative
+ integer (e.g. -123) will not be considered as an invalid set of flags. */
+extern int check_for_flags (const char **str, const char *flags,
+ int *flags_counts);
+
+/* A helper function that uses check_for_flags to handle the flags vqcs :
+ A flag v increases *print_what_v.
+ A flag q decreases *print_what_v.
+ A flag c sets *cont to true, otherwise to false.
+ A flag s sets *silent to true, otherwise to false.
+ The caller must initialise *PRINT_WHAT_V to the default verbosity.
+ MAX_V gives the maximum verbosity : the value returned in *PRINT_WHAT_V
+ will always be >= 0 and <= MAX_V.
+ WHICH_COMMAND is used in the error message in case invalid flags are found.
+
+ Returns 1 and updates *STR if it finds only valid flags.
+ Returns 0 if no flags are found.
+ Raises an error if invalid flags are found.
+*/
+extern int check_for_flags_vqcs (const char *which_command, const char **str,
+ int *print_what_v, int max_v,
+ bool *cont, bool *silent);
#endif /* CLI_UTILS_H */
--
2.11.0
^ permalink raw reply [flat|nested] 17+ messages in thread* [RFA 4/8] Documentation changes for 'frame apply' and 'thread apply'
2018-05-21 11:07 [RFA 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND' Philippe Waroquiers
` (3 preceding siblings ...)
2018-05-21 11:07 ` [RFA 1/8] Add helper functions check_for_flags and check_for_flags_vqcs Philippe Waroquiers
@ 2018-05-21 11:07 ` Philippe Waroquiers
2018-05-21 16:23 ` Eli Zaretskii
2018-05-21 11:07 ` [RFA 8/8] Add a self-test for cli-utils.c Philippe Waroquiers
` (4 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Philippe Waroquiers @ 2018-05-21 11:07 UTC (permalink / raw)
To: gdb-patches; +Cc: Philippe Waroquiers
Documents the new commands
'frame apply'
faas
taas
tfaas
Documents the new argument -FLAGS... added to 'thread apply'
---
gdb/doc/gdb.texinfo | 225 +++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 222 insertions(+), 3 deletions(-)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 28f083f96e..e80f52a66e 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -2933,7 +2933,7 @@ programs:
@item automatic notification of new threads
@item @samp{thread @var{thread-id}}, a command to switch among threads
@item @samp{info threads}, a command to inquire about existing threads
-@item @samp{thread apply [@var{thread-id-list}] [@var{all}] @var{args}},
+@item @samp{thread apply [@var{thread-id-list} | all] @var{args}},
a command to apply a command to a list of threads
@item thread-specific breakpoints
@item @samp{set print thread-events}, which controls printing of
@@ -3170,7 +3170,7 @@ threads.
@kindex thread apply
@cindex apply command to several threads
-@item thread apply [@var{thread-id-list} | all [-ascending]] @var{command}
+@item thread apply [@var{thread-id-list} | all [-ascending]] [@var{-flags@dots{}}] @var{command}
The @code{thread apply} command allows you to apply the named
@var{command} to one or more threads. Specify the threads that you
want affected using the thread ID list syntax (@pxref{thread ID
@@ -3179,6 +3179,63 @@ command to all threads in descending order, type @kbd{thread apply all
@var{command}}. To apply a command to all threads in ascending order,
type @kbd{thread apply all -ascending @var{command}}.
+The @var{flags} control what output to produce and how to handle
+errors raised when applying @var{command} to a thread. @var{flags}
+must start with a @code{-} directly followed by one or more letters in
+@code{vqcs}.
+
+By default, @value{GDBN} displays some thread information before the
+output produced by @var{command}, and an error raised during the
+execution of a @var{command} will abort @code{thread apply}. The
+following flags can be used to fine-tune this behavior:
+
+@table @code
+@item c
+The flag @code{c}, which stands for @samp{continue}, causes any
+errors in @var{command} to be displayed, and the execution of
+@code{thread apply} then continues.
+@item s
+The flag @code{s}, which stands for @samp{silent}, causes any errors
+or empty output produced by a @var{command} to be silently ignored.
+That is, the execution continues, but the thread information and errors
+are not printed.
+@item v
+The flag @code{v} (@samp{verbose}) increases the verbosity.
+@item q
+The flag @code{q} (@samp{quiet}) decreases the verbosity.
+@end table
+
+Flags @code{c} and @code{s} cannot be used together.
+
+The default value of verbosity, 1, prints the per-inferior thread
+number and the target system's thread ID. Under verbosity 0, no
+thread info is printed.
+
+@kindex taas
+@cindex apply command to all threads (ignoring errors and empty output)
+@item taas @var{command}
+Shortcut for @code{thread apply all -s @var{command}}.
+Applies @var{command} on all threads, ignoring errors and empty output.
+
+@kindex tfaas
+@cindex apply a command to all frames of all threads (ignoring errors and empty output)
+@item tfaas @var{command}
+Shortcut for @code{thread apply all -s frame apply all -s @var{command}}.
+Applies @var{command} on all frames of all threads, ignoring errors
+and empty output. Note that the flag @code{-s} is specified twice:
+The first @code{-s} ensures that @code{thread apply} only shows the thread
+information of the threads for which @code{frame apply} produces
+some output. The second @code{-s} is needed to ensure that @code{frame
+apply} shows the frame information of a frame only if the
+@var{command} successfully produced some output.
+
+It can for example be used to print a local variable or a function
+argument without knowing the thread or frame where this variable or argument
+is, using:
+@smallexample
+(@value{GDBP}) tfaas p some_local_var_i_do_not_remember_where_it_is
+@end smallexample
+
@kindex thread name
@cindex name a thread
@@ -7300,6 +7357,7 @@ currently executing frame and describes it briefly, similar to the
* Backtrace:: Backtraces
* Selection:: Selecting a frame
* Frame Info:: Information on a frame
+* Frame Apply:: Applying a command to several frames
* Frame Filter Management:: Managing frame filters
@end menu
@@ -7712,6 +7770,167 @@ accessible at the point of execution of the selected frame.
@end table
+@node Frame Apply
+@section Applying a Command to Several Frames.
+@kindex frame apply
+@cindex apply command to several frames
+@table @code
+@item frame apply [all | @var{count} | @var{-count} ] [@var{-flags@dots{}}] @var{command}
+The @code{frame apply} command allows you to apply the named
+@var{command} to one or more frames.
+
+@table @code
+@item @code{all}
+Specify @code{all} to apply @var{command} to all frames.
+
+@item @var{count}
+Use @var{count} to apply @var{command} to the innermost @var{count}
+frames, where @var{count} is a positive number.
+
+@item @var{-count}
+Use @var{-count} to apply @var{command} to the outermost @var{count}
+frames, where @var{count} is a positive number.
+@end table
+
+@end table
+
+Note that the frames on which @code{frame apply} applies a command are
+also influenced by the @code{set backtrace} settings such as @code{set
+backtrace past-main} and @code{set backtrace limit N}. See
+@xref{Backtrace,,Backtraces}.
+
+The @var{flags} control what output to produce and how to handle
+errors raised when applying @var{command} to a frame. @var{flags}
+must start with a @code{-} directly followed by one or more letters in
+@code{vqcs}.
+
+By default, @value{GDBN} displays some frame information before the
+output produced by @var{command}, and an error raised during the
+execution of a @var{command} will abort @code{frame apply}. The
+following flags can be used to fine-tune this behavior:
+
+@table @code
+@item c
+The flag @code{c}, which stands for @samp{continue}, causes any
+errors in @var{command} to be displayed, and the execution of
+@code{frame apply} then continues.
+@item s
+The flag @code{s}, which stands for @samp{silent}, causes any errors
+or empty output produced by a @var{command} to be silently ignored.
+That is, the execution continues, but the frame information and errors
+are not printed.
+@item v
+The flag @code{v} (@samp{verbose}) increases the verbosity.
+@item q
+The flag @code{q} (@samp{quiet}) decreases the verbosity.
+@end table
+
+The following example shows how the flags @code{c} and @code{s} are
+working when applying the command @code{p j} to all frames, where
+variable @code{j} can only be successfully printed in the outermost
+@code{#1 main} frame.
+
+@smallexample
+@group
+(gdb) frame apply all p j
+#0 some_function (i=5) at fun.c:4
+No symbol "j" in current context.
+(gdb) frame apply all -c p j
+#0 some_function (i=5) at fun.c:4
+No symbol "j" in current context.
+#1 0x565555fb in main (argc=1, argv=0xffffd2c4) at fun.c:11
+$1 = 5
+(gdb) frame apply all -s p j
+#1 0x565555fb in main (argc=1, argv=0xffffd2c4) at fun.c:11
+$2 = 5
+(gdb)
+@end group
+@end smallexample
+
+The default value of verbosity, 2, prints the location before
+the command output:
+
+@smallexample
+@group
+(gdb) frame apply all p $sp
+#0 some_function (i=5) at fun.c:4
+$4 = (void *) 0xffffd1e0
+#1 0x565555fb in main (argc=1, argv=0xffffd2c4) at fun.c:11
+$5 = (void *) 0xffffd1f0
+(gdb)
+@end group
+@end smallexample
+
+The verbosity 3 prints the location and address:
+
+@smallexample
+@group
+(gdb) frame apply all -v p $sp
+#0 0x565555b1 in some_function (i=5) at fun.c:4
+$6 = (void *) 0xffffd1e0
+#1 0x565555fb in main (argc=1, argv=0xffffd2c4) at fun.c:11
+$7 = (void *) 0xffffd1f0
+(gdb)
+@end group
+@end smallexample
+
+The verbosity 4 prints the source line and address:
+@smallexample
+@group
+(gdb) frame apply all -vv p $sp
+#0 some_function (i=5) at fun.c:4
+4 printf ("value of i is %d\n", i);
+$8 = (void *) 0xffffd1e0
+#1 0x565555fb in main (argc=1, argv=0xffffd2c4) at fun.c:11
+11 some_function (j);
+$9 = (void *) 0xffffd1f0
+(gdb)
+@end group
+@end smallexample
+
+At verbosity 1, only the source line is printed:
+
+@smallexample
+@group
+(gdb) frame apply all -q p $sp
+4 printf ("value of i is %d\n", i);
+$10 = (void *) 0xffffd1e0
+0x565555fb 11 some_function (j);
+$11 = (void *) 0xffffd1f0
+(gdb)
+@end group
+@end smallexample
+
+At verbosity 0, no frame information is printed:
+@smallexample
+@group
+(gdb) frame apply all -qq p $sp
+$12 = (void *) 0xffffd1e0
+$13 = (void *) 0xffffd1f0
+(gdb)
+@end group
+@end smallexample
+
+@table @code
+
+@kindex faas
+@cindex apply a command to all frames (ignoring errors and empty output)
+@item faas @var{command}
+Shortcut for @code{frame apply all -s @var{command}}.
+Applies @var{command} on all frames, ignoring errors and empty output.
+
+It can for example be used to print a local variable or a function
+argument without knowing the frame where this variable or argument
+is, using:
+@smallexample
+(@value{GDBP}) faas p some_local_var_i_do_not_remember_where_it_is
+@end smallexample
+
+Note that the command @code{tfaas @var{command}} applies @var{command}
+on all frames of all threads. See @xref{Threads,,Threads}.
+@end table
+
+
@node Frame Filter Management
@section Management of Frame Filters.
@cindex managing frame filters
@@ -11090,7 +11309,7 @@ Visiting node of type NODE_INTEGER
convenience functions.
@table @code
-@item help function
+@item help functionu
@kindex help function
@cindex show all convenience functions
Print a list of all convenience functions.
--
2.11.0
^ permalink raw reply [flat|nested] 17+ messages in thread* [RFA 8/8] Add a self-test for cli-utils.c
2018-05-21 11:07 [RFA 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND' Philippe Waroquiers
` (4 preceding siblings ...)
2018-05-21 11:07 ` [RFA 4/8] Documentation changes for 'frame apply' and 'thread apply' Philippe Waroquiers
@ 2018-05-21 11:07 ` Philippe Waroquiers
2018-05-21 11:54 ` [RFA 6/8] Add a test for 'frame apply' Philippe Waroquiers
` (3 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Philippe Waroquiers @ 2018-05-21 11:07 UTC (permalink / raw)
To: gdb-patches; +Cc: Philippe Waroquiers
tests added for:
* number_or_range_parser
number_or_range_parser is somewhat cumbersome to use and/or does
not work as described in cli-utils.h
In particular, cur_tok () is currently not maintained as described
in cli-utils.h, but changing this is better done in another patch,
as it implies to change callers.
The SELF_CHECK for cumbersome (or unexpected) behaviour are commented
now. Note that the same commented tests are similarly wrong in unpatched gdb.
* check_for_flags_vqcs
* check_for_flags
---
gdb/Makefile.in | 1 +
gdb/unittests/cli-utils-selftests.c | 228 ++++++++++++++++++++++++++++++++++++
2 files changed, 229 insertions(+)
create mode 100644 gdb/unittests/cli-utils-selftests.c
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 09a2ad2ca3..7fdb774036 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -416,6 +416,7 @@ SUBDIR_PYTHON_CFLAGS =
SUBDIR_UNITTESTS_SRCS = \
unittests/array-view-selftests.c \
+ unittests/cli-utils-selftests.c \
unittests/common-utils-selftests.c \
unittests/environ-selftests.c \
unittests/format_pieces-selftests.c \
diff --git a/gdb/unittests/cli-utils-selftests.c b/gdb/unittests/cli-utils-selftests.c
new file mode 100644
index 0000000000..31cede7398
--- /dev/null
+++ b/gdb/unittests/cli-utils-selftests.c
@@ -0,0 +1,228 @@
+/* Unit tests for the cli-utils.c file.
+
+ Copyright (C) 2018 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ 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/>. */
+
+#include "defs.h"
+#include "cli/cli-utils.h"
+#include "selftest.h"
+
+namespace selftests {
+namespace cli_utils {
+
+static void
+test_number_or_range_parser ()
+{
+ number_or_range_parser one ("1");
+
+ SELF_CHECK (one.finished () == false);
+ SELF_CHECK (one.get_number () == 1);
+ SELF_CHECK (one.finished () == true);
+ SELF_CHECK (strcmp (one.cur_tok (), "") == 0);
+
+ number_or_range_parser one_after ("1 after");
+
+ SELF_CHECK (one_after.finished () == false);
+ SELF_CHECK (one_after.get_number () == 1);
+ // SELF_CHECK (one_after.finished () == true);
+ // number_or_range_parser is cumbersome : to have finished == true,
+ // you have to call get_number once more. But then, the cur_tok is wrong.
+ // It should be restructured to have the setup (e.g. discovering if
+ // we have a number or a range or something else) in the init phase.
+ // This allows to indicate finished == true just after init,
+ // in case the init string is not a number or range.
+ SELF_CHECK (strcmp (one_after.cur_tok (), "after") == 0);
+
+ number_or_range_parser one_three ("1-3");
+
+ for (int i = 1; i < 4; i++) {
+ SELF_CHECK (one_three.finished () == false);
+ SELF_CHECK (one_three.get_number () == i);
+ }
+ SELF_CHECK (one_three.finished () == true);
+ SELF_CHECK (strcmp (one_three.cur_tok (), "") == 0);
+
+ number_or_range_parser one_three_after ("1-3 after");
+
+ for (int i = 1; i < 4; i++) {
+ SELF_CHECK (one_three_after.finished () == false);
+ SELF_CHECK (one_three_after.get_number () == i);
+ }
+ // SELF_CHECK (one_three_after.finished () == true);
+ // See above.
+ SELF_CHECK (strcmp (one_three_after.cur_tok (), "after") == 0);
+
+ number_or_range_parser minus_one ("-1");
+
+ SELF_CHECK (minus_one.finished () == false);
+ TRY
+ {
+ minus_one.get_number ();
+ SELF_CHECK (false);
+ }
+ CATCH (ex, RETURN_MASK_ERROR)
+ {
+ SELF_CHECK (ex.reason == RETURN_ERROR);
+ SELF_CHECK (ex.error == GENERIC_ERROR);
+ SELF_CHECK (strcmp (ex.message, "negative value") == 0);
+ SELF_CHECK (strcmp (minus_one.cur_tok (), "-1") == 0);
+ }
+ END_CATCH;
+
+ number_or_range_parser nan ("-whatever");
+
+ // SELF_CHECK (nan.finished () == true);
+ // See above
+ SELF_CHECK (one_three_after.get_number () == 0);
+ SELF_CHECK (strcmp (nan.cur_tok (), "-whatever") == 0);
+}
+
+static void
+test_check_for_flags ()
+{
+ const char *flags = "abc";
+ const char *non_flags_args = "non flags args";
+ int flags_counts[strlen (flags)];
+ int res;
+
+ const char *t1 = "-aa non flags args";
+
+ SELF_CHECK (check_for_flags (&t1, flags, flags_counts) == 1);
+ SELF_CHECK (strcmp (t1, non_flags_args) == 0);
+ SELF_CHECK (flags_counts[0] == 2
+ && flags_counts[1] == 0
+ && flags_counts[2] == 0);
+
+ const char *t2 = "-cbcbc non flags args";
+
+ SELF_CHECK (check_for_flags (&t2, flags, flags_counts) == 1);
+ SELF_CHECK (strcmp (t2, non_flags_args) == 0);
+ SELF_CHECK (flags_counts[0] == 0
+ && flags_counts[1] == 2
+ && flags_counts[2] == 3);
+
+ const char *t3 = non_flags_args;
+
+ SELF_CHECK (check_for_flags (&t3, flags, flags_counts) == 0);
+ SELF_CHECK (strcmp (t3, non_flags_args) == 0);
+ SELF_CHECK (flags_counts[0] == 0
+ && flags_counts[1] == 0
+ && flags_counts[2] == 0);
+
+ const char *t4 = "-cbxyzc";
+ const char *orig_t4 = t4;
+
+ SELF_CHECK (check_for_flags (&t4, flags, flags_counts) == -1);
+ SELF_CHECK (strcmp (t4, orig_t4) == 0);
+ SELF_CHECK (flags_counts[0] == 0
+ && flags_counts[1] == 1
+ && flags_counts[2] == 2);
+
+}
+
+static void
+test_check_for_flags_vqcs ()
+{
+ int verbosity;
+ bool cont;
+ bool silent;
+ const char *non_flags_args = "non flags args";
+
+ const char *t1 = "-vvvqqqs non flags args";
+
+ verbosity = 1;
+ cont = true;
+ silent = false;
+ SELF_CHECK (check_for_flags_vqcs ("test_check_for_flags_vqcs.t1",
+ &t1,
+ &verbosity,
+ 4, &cont, &silent) == 1);
+ SELF_CHECK (verbosity == 1);
+ SELF_CHECK (cont == false && silent == true);
+ SELF_CHECK (strcmp (t1, non_flags_args) == 0);
+
+ const char *t2 = "non flags args";
+
+ verbosity = 2;
+ cont = true;
+ silent = true;
+ SELF_CHECK (check_for_flags_vqcs ("test_check_for_flags_vqcs.t2",
+ &t2,
+ &verbosity,
+ 4, &cont, &silent) == 0);
+ SELF_CHECK (verbosity == 2);
+ SELF_CHECK (cont == false && silent == false);
+ SELF_CHECK (strcmp (t2, non_flags_args) == 0);
+
+ const char *t3 = "-123 non flags args";
+ const char *orig_t3 = t3;
+
+ verbosity = 3;
+ cont = true;
+ silent = true;
+ SELF_CHECK (check_for_flags_vqcs ("test_check_for_flags_vqcs.t3",
+ &t3,
+ &verbosity,
+ 4, &cont, &silent) == 0);
+ SELF_CHECK (verbosity == 3);
+ SELF_CHECK (cont == false && silent == false);
+ SELF_CHECK (strcmp (t3, orig_t3) == 0);
+
+ const char *t4 = "-qqxyz non flags args";
+ const char *orig_t4 = t4;
+ TRY
+ {
+ verbosity = 4;
+ cont = true;
+ silent = true;
+ (void) check_for_flags_vqcs ("test_check_for_flags_vqcs.t4",
+ &t4,
+ &verbosity,
+ 4, &cont, &silent);
+ SELF_CHECK (false);
+ }
+ CATCH (ex, RETURN_MASK_ERROR)
+ {
+ SELF_CHECK (ex.reason == RETURN_ERROR);
+ SELF_CHECK (ex.error == GENERIC_ERROR);
+ SELF_CHECK
+ (strcmp (ex.message,
+ "test_check_for_flags_vqcs.t4 only accepts flags vqcs") == 0);
+ SELF_CHECK (strcmp (t4, orig_t4) == 0);
+ SELF_CHECK (verbosity == 4);
+ }
+ END_CATCH;
+
+}
+
+static void
+test_cli_utils ()
+{
+ selftests::cli_utils::test_number_or_range_parser ();
+ selftests::cli_utils::test_check_for_flags ();
+ selftests::cli_utils::test_check_for_flags_vqcs ();
+}
+
+}
+}
+
+void
+_initialize_cli_utils_selftests ()
+{
+ selftests::register_test ("cli_utils",
+ selftests::cli_utils::test_cli_utils);
+}
--
2.11.0
^ permalink raw reply [flat|nested] 17+ messages in thread* [RFA 6/8] Add a test for 'frame apply'
2018-05-21 11:07 [RFA 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND' Philippe Waroquiers
` (5 preceding siblings ...)
2018-05-21 11:07 ` [RFA 8/8] Add a self-test for cli-utils.c Philippe Waroquiers
@ 2018-05-21 11:54 ` Philippe Waroquiers
2018-05-21 12:16 ` [RFA 2/8] Implement frame apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND Philippe Waroquiers
` (2 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Philippe Waroquiers @ 2018-05-21 11:54 UTC (permalink / raw)
To: gdb-patches; +Cc: Philippe Waroquiers
---
gdb/testsuite/gdb.base/frameapply.c | 71 +++++++++++++++
gdb/testsuite/gdb.base/frameapply.exp | 167 ++++++++++++++++++++++++++++++++++
2 files changed, 238 insertions(+)
create mode 100644 gdb/testsuite/gdb.base/frameapply.c
create mode 100644 gdb/testsuite/gdb.base/frameapply.exp
diff --git a/gdb/testsuite/gdb.base/frameapply.c b/gdb/testsuite/gdb.base/frameapply.c
new file mode 100644
index 0000000000..dccf4031ed
--- /dev/null
+++ b/gdb/testsuite/gdb.base/frameapply.c
@@ -0,0 +1,71 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2018 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/>. */
+
+static void
+setup_done (void)
+{
+}
+
+static int
+f1 (int f1arg)
+{
+ int f1loc;
+
+ f1loc = f1arg - 1;
+
+ setup_done ();
+ return f1loc;
+}
+
+static int
+f2 (int f2arg)
+{
+ int f2loc;
+
+ f2loc = f1 (f2arg - 1);
+
+ return f2loc;
+}
+
+static int
+f3 (int f3arg)
+{
+ int f3loc;
+
+ f3loc = f2 (f3arg - 1);
+
+ return f3loc;
+}
+
+static int
+f4 (int f4arg)
+{
+ int f4loc;
+
+ f4loc = f3 (f4arg - 1);
+
+ return f4loc;
+}
+
+int
+main (void)
+{
+ int result;
+
+ result = f4 (4);
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/frameapply.exp b/gdb/testsuite/gdb.base/frameapply.exp
new file mode 100644
index 0000000000..0257f0214f
--- /dev/null
+++ b/gdb/testsuite/gdb.base/frameapply.exp
@@ -0,0 +1,167 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2018 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 'frame apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND'
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" ${testfile}] } {
+ return -1
+}
+
+clean_restart ${binfile}
+
+
+if ![runto setup_done] then {
+ gdb_suppress_tests
+}
+
+set any "\[^\r\n\]*"
+set ws "\[ \t\]\+"
+set number "\[0-9]\+"
+
+# check all | COUNT | -COUNT with a simple command
+gdb_test "frame apply all p /x 20" \
+ [multi_line \
+ "#0${ws}setup_done ${any}" \
+ "\\\$\[0-9]+ = 0x14${any}" \
+ "#1${ws}${any} f1 ${any}" \
+ "\\\$\[0-9]+ = 0x14${any}" \
+ "#2${ws}${any} f2 ${any}" \
+ "\\\$\[0-9]+ = 0x14${any}" \
+ "#3${ws}${any} f3 ${any}" \
+ "\\\$\[0-9]+ = 0x14${any}" \
+ "#4${ws}${any} f4 ${any}" \
+ "\\\$\[0-9]+ = 0x14${any}" \
+ "#5${ws}${any} main ${any}" \
+ "\\\$\[0-9]+ = 0x14${any}" \
+ ] \
+ "Run a simple command on all frames"
+
+gdb_test "frame apply 3 p /x 20" \
+ [multi_line \
+ "#0${ws}setup_done ${any}" \
+ "\\\$\[0-9]+ = 0x14${any}" \
+ "#1${ws}${any} f1 ${any}" \
+ "\\\$\[0-9]+ = 0x14${any}" \
+ "#2${ws}${any} f2 ${any}" \
+ "\\\$\[0-9]+ = 0x14${any}" \
+ ] \
+ "Run a simple command on the 3 innermost frames"
+
+gdb_test "frame apply -3 p /x 20" \
+ [multi_line \
+ "#3${ws}${any} f3 ${any}" \
+ "\\\$\[0-9]+ = 0x14${any}" \
+ "#4${ws}${any} f4 ${any}" \
+ "\\\$\[0-9]+ = 0x14${any}" \
+ "#5${ws}${any} main ${any}" \
+ "\\\$\[0-9]+ = 0x14${any}" \
+ ] \
+ "Run a simple command on the 3 outermost frames"
+
+# check -c (continue- and -s (silently continue) flags
+gdb_test "frame apply all p f3arg" \
+ [multi_line \
+ "#0${ws}setup_done ${any}" \
+ "No symbol \\\"f3arg\\\" in current context." \
+ ] \
+ "Run a failing command that aborts frame apply"
+
+gdb_test "frame apply all -c p f3arg" \
+ [multi_line \
+ "#0${ws}setup_done ${any}" \
+ "No symbol \\\"f3arg\\\" in current context." \
+ "#1${ws}${any} f1 ${any}" \
+ "No symbol \\\"f3arg\\\" in current context." \
+ "#2${ws}${any} f2 ${any}" \
+ "No symbol \\\"f3arg\\\" in current context." \
+ "#3${ws}${any} f3 ${any}" \
+ "\\\$\[0-9]+ = 3${any}" \
+ "#4${ws}${any} f4 ${any}" \
+ "No symbol \\\"f3arg\\\" in current context." \
+ "#5${ws}${any} main ${any}" \
+ "No symbol \\\"f3arg\\\" in current context." \
+ ] \
+ "Run a command failing in all frames except #3, -c to continue"
+
+foreach cmd {"frame apply all -s p f3arg" "faas p f3arg"} {
+ gdb_test $cmd \
+ [multi_line \
+ "#3${ws}${any} f3 ${any}" \
+ "\\\$\[0-9]+ = 3${any}" \
+ ] \
+ "Run a command failing in all frames except #3, -s to silently continue"
+}
+
+# check verbosity
+gdb_test "frame apply 2 p /x 20" \
+ [multi_line \
+ "#0${ws}setup_done ${any}" \
+ "\\\$\[0-9]+ = 0x14${any}" \
+ "#1${ws}${any} f1 ${any}" \
+ "\\\$\[0-9]+ = 0x14${any}" \
+ ] \
+ "Run a command at default verbosity 2, printing location"
+
+gdb_test "frame apply 2 -q p /x 20" \
+ [multi_line \
+ "${number}${ws}\\\}${any}" \
+ "\\\$\[0-9]+ = 0x14${any}" \
+ "${any}${ws}${number}${ws} setup_done ()${any}" \
+ "\\\$\[0-9]+ = 0x14${any}" \
+ ] \
+ "Run a command at verbosity 1, printing source line"
+
+gdb_test "frame apply 2 -qq p /x 20" \
+ [multi_line \
+ "\\\$\[0-9]+ = 0x14${any}" \
+ "\\\$\[0-9]+ = 0x14${any}" \
+ ] \
+ "Run a command at verbosity 0, printing only command results"
+
+gdb_test "frame apply 2 -v p /x 20" \
+ [multi_line \
+ "#0${ws}0x${any}${ws}in setup_done ${any}" \
+ "\\\$\[0-9]+ = 0x14${any}" \
+ "#1${ws}0x${any} in f1 ${any}" \
+ "\\\$\[0-9]+ = 0x14${any}" \
+ ] \
+ "Run a command at verbosity 3, printing location and address"
+
+gdb_test "frame apply 2 -vv p /x 20" \
+ [multi_line \
+ "#0${ws}setup_done ${any}" \
+ "${number}${ws}\\\}${any}" \
+ "\\\$\[0-9]+ = 0x14${any}" \
+ "#1${ws}${any} f1 ${any}" \
+ "${number}${ws} setup_done ()${any}" \
+ "\\\$\[0-9]+ = 0x14${any}" \
+ ] \
+ "Run a command at verbosity 4, printing location and source line"
+
+# check multiple flags together
+gdb_test "frame apply all -qsq p f3arg" \
+ "\\\$\[0-9]+ = 3${any}" \
+ "Run a command failing in all frames except #3, -s to silently continue, verbosity 0"
+
+# check invalid flag combinations
+gdb_test "frame apply all -cs p f3arg" \
+ "frame apply all: flags c and s are mutually exclusive" \
+ "Check c and s cannot be used simultaneously"
+
--
2.11.0
^ permalink raw reply [flat|nested] 17+ messages in thread* [RFA 2/8] Implement frame apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND.
2018-05-21 11:07 [RFA 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND' Philippe Waroquiers
` (6 preceding siblings ...)
2018-05-21 11:54 ` [RFA 6/8] Add a test for 'frame apply' Philippe Waroquiers
@ 2018-05-21 12:16 ` Philippe Waroquiers
2018-05-28 22:18 ` [PING] [RFA 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND' Philippe Waroquiers
2018-06-01 14:32 ` Pedro Alves
9 siblings, 0 replies; 17+ messages in thread
From: Philippe Waroquiers @ 2018-05-21 12:16 UTC (permalink / raw)
To: gdb-patches; +Cc: Philippe Waroquiers
Implement frame apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND.
Also implement the command 'faas COMMAND', a shortcut for
'frame apply all -s COMMAND'.
Note: the syntax of 'frame apply' to specify some innermost or outermost
frames is similar to 'backtrace' command.
An alternative could be to make 'frame apply' similar to
'thread apply'. This was not chosen as:
* the typical use cases for frame apply are all/some innermost/some outermost
frames.
* a range based syntax would have obliged the user to use absolute numbers
to specify the outermost N frames, which is cumbersone.
Or an syntax different from the thread range would have been needed
(e.g. -0-4 to specify the 5 outermost frames).
So, making frame apply similar to backtrace is found better.
---
gdb/stack.c | 246 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 223 insertions(+), 23 deletions(-)
diff --git a/gdb/stack.c b/gdb/stack.c
index 74c92537da..199e8303fc 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1687,6 +1687,38 @@ info_frame_command (const char *addr_exp, int from_tty)
}
}
+/* trailing_outermost_frame returns the starting frame
+ needed to handle COUNT outermost frames. */
+
+static struct frame_info *
+trailing_outermost_frame (int count)
+{
+ struct frame_info *current;
+ struct frame_info *trailing;
+
+ trailing = get_current_frame ();
+
+ gdb_assert (count > 0);
+
+ current = trailing;
+ while (current != nullptr && count--)
+ {
+ QUIT;
+ current = get_prev_frame (current);
+ }
+
+ /* Will stop when CURRENT reaches the top of the stack.
+ TRAILING will be COUNT below it. */
+ while (current != nullptr)
+ {
+ QUIT;
+ trailing = get_prev_frame (trailing);
+ current = get_prev_frame (current);
+ }
+
+ return trailing;
+}
+
/* Print briefly all stack frames or just the innermost COUNT_EXP
frames. */
@@ -1751,32 +1783,14 @@ backtrace_command_1 (const char *count_exp, frame_filter_flags flags,
variable TRAILING to the frame from which we should start
printing. Second, it must set the variable count to the number
of frames which we should print, or -1 if all of them. */
- trailing = get_current_frame ();
if (count_exp != NULL && count < 0)
{
- struct frame_info *current;
-
- count = -count;
-
- current = trailing;
- while (current && count--)
- {
- QUIT;
- current = get_prev_frame (current);
- }
-
- /* Will stop when CURRENT reaches the top of the stack.
- TRAILING will be COUNT below it. */
- while (current)
- {
- QUIT;
- trailing = get_prev_frame (trailing);
- current = get_prev_frame (current);
- }
-
+ trailing = trailing_outermost_frame (-count);
count = -1;
}
+ else
+ trailing = get_current_frame ();
for (fi = trailing; fi && count--; fi = get_prev_frame (fi))
{
@@ -2505,9 +2519,160 @@ func_command (const char *arg, int from_tty)
select_and_print_frame (frame);
}
+/* Apply a GDB command to a all stack frames, or innermost COUNT frames.
+ With a negative COUNT, apply command on outermost -COUNT frames.
+
+ frame apply 3 info frame Apply 'info frame' to frames 0, 1, 2
+ frame apply -3 info frame Apply 'info frame' to outermost 3 frames.
+ frame apply all x/i $pc Apply 'x/i $pc' cmd to all frames.
+ frame apply all -s p local_var_no_idea_in_which_frame
+ If a frame has a local variable called
+ local_var_no_idea_in_which_frame, print frame
+ and value of local_var_no_idea_in_which_frame.
+ frame apply all -sqq p local_var_no_idea_in_which_frame
+ Same as before, but only print the variable value. */
+
+/* Apply a GDB command to COUNT stack frames, starting at TRAILING.
+ COUNT -1 means all frames starting at TRAILING. WHICH_COMMAND is used
+ for error messages. */
+static void
+frame_apply_command_count (const char* which_command,
+ const char *cmd, int from_tty,
+ struct frame_info *trailing, int count)
+{
+ int print_what_v = 2; /* Corresponding to LOCATION. */
+ enum print_what print_what[5] =
+ {
+ LOC_AND_ADDRESS, /* Should never be used, this is verbosity 0. */
+ SRC_LINE,
+ LOCATION,
+ LOC_AND_ADDRESS,
+ SRC_AND_LOC
+ };
+ bool cont;
+ bool silent;
+ struct frame_info *fi;
+
+ if (cmd != NULL)
+ check_for_flags_vqcs (which_command, &cmd,
+ &print_what_v, 4,
+ &cont, &silent);
+
+ if (cmd == NULL || *cmd == '\0')
+ error (_("Please specify a command to apply on the selected frames"));
+
+ /* The below will restore the current inferior/thread/frame.
+ Usually, only the frame is effectively to be restored.
+ But in case CMD switches of inferior/thread, better restore
+ these also. */
+ scoped_restore_current_thread restore_thread;
+
+ for (fi = trailing; fi && count--; fi = get_prev_frame (fi))
+ {
+ struct frame_id frame_id = get_frame_id (fi);
+
+ QUIT;
+
+ select_frame (fi);
+ TRY
+ {
+ std::string cmd_result;
+ {
+ /* In case CMD switches of inferior/thread/frame, the below
+ restores the inferior/thread. We can then re-initialise
+ FI based on FRAME_ID. */
+ scoped_restore_current_thread restore_fi_current_frame;
+
+ cmd_result = execute_command_to_string (cmd, from_tty);
+ }
+ fi = frame_find_by_id (frame_id);
+ if (fi == NULL)
+ {
+ warning (_("Unable to restore previously selected frame."));
+ break;
+ }
+ if (!silent || cmd_result.length () > 0)
+ {
+ if (print_what_v > 0)
+ print_stack_frame (fi, 1, print_what[print_what_v], 0);
+ printf_filtered ("%s", cmd_result.c_str ());
+ }
+ }
+ CATCH (ex, RETURN_MASK_ERROR)
+ {
+ fi = frame_find_by_id (frame_id);
+ if (fi == NULL)
+ {
+ warning (_("Unable to restore previously selected frame."));
+ break;
+ }
+ if (!silent)
+ {
+ if (print_what_v > 0)
+ print_stack_frame (fi, 1, print_what [print_what_v], 0);
+ if (cont)
+ printf_filtered ("%s\n", ex.message);
+ else
+ throw_exception (ex);
+ }
+ }
+ END_CATCH;
+ }
+}
+
+static void
+frame_apply_all_command (const char *cmd, int from_tty)
+{
+ if (!target_has_stack)
+ error (_("No stack."));
+
+ frame_apply_command_count ("frame apply all", cmd, from_tty,
+ get_current_frame (), INT_MAX);
+}
+
+/* Implementation of the "frame apply" command. */
+
+static void
+frame_apply_command (const char* cmd, int from_tty)
+{
+ int count;
+ struct frame_info *trailing;
+
+ if (!target_has_stack)
+ error (_("No stack."));
+
+ count = get_number_trailer (&cmd, 0);
+
+ if (count < 0)
+ {
+ trailing = trailing_outermost_frame (-count);
+ count = -1;
+ }
+ else
+ trailing = get_current_frame ();
+
+ frame_apply_command_count ("frame apply", cmd, from_tty,
+ trailing, count);
+}
+
+/* Implementation of the "faas" command. */
+
+static void
+faas_command (const char *cmd, int from_tty)
+{
+ std::string expanded = std::string ("frame apply all -s ") + std::string (cmd);
+ execute_command (expanded.c_str (), from_tty);
+}
+
+
+/* Commands with a prefix of `frame'. */
+struct cmd_list_element *frame_cmd_list = NULL;
+
void
_initialize_stack (void)
{
+ static struct cmd_list_element *frame_apply_list = NULL;
+
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\
@@ -2530,14 +2695,49 @@ 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, _("\
+ add_prefix_cmd ("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."));
+It can be a stack frame number or the address of the frame."),
+ &frame_cmd_list, "frame ", 1, &cmdlist);
add_com_alias ("f", "frame", class_stack, 1);
+#define FRAME_APPLY_FLAGS_HELP "\
+FLAGS letters are v(increase verbosity), q(decrease verbosity)\n\
+ c(continue), s(silent).\n\
+Verbosity (default 2) controls what to print for a frame:\n\
+ 0 : no frame info is printed\n\
+ 1 : source line\n\
+ 2 : location\n\
+ 3 : location and address\n\
+ 4 : source line and location\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 [-FLAGS...] COMMAND\n\
+With a negative COUNT argument, applies the command on outermost -COUNT frames.\n"
+FRAME_APPLY_FLAGS_HELP),
+ &frame_apply_list, "frame apply ", 1, &frame_cmd_list);
+
+ add_cmd ("all", class_stack, frame_apply_all_command,
+ _("\
+Apply a command to all frames.\n\
+\n\
+Usage: frame apply all [-FLAGS...] COMMAND\n"
+FRAME_APPLY_FLAGS_HELP),
+ &frame_apply_list);
+
+ 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'"));
+
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\
--
2.11.0
^ permalink raw reply [flat|nested] 17+ messages in thread* [PING] [RFA 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND'
2018-05-21 11:07 [RFA 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND' Philippe Waroquiers
` (7 preceding siblings ...)
2018-05-21 12:16 ` [RFA 2/8] Implement frame apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND Philippe Waroquiers
@ 2018-05-28 22:18 ` Philippe Waroquiers
2018-06-01 14:32 ` Pedro Alves
9 siblings, 0 replies; 17+ messages in thread
From: Philippe Waroquiers @ 2018-05-28 22:18 UTC (permalink / raw)
To: gdb-patches
Ping.
Thanks
Philippe
On Mon, 2018-05-21 at 13:06 +0200, Philippe Waroquiers wrote:
> Implement 'frame apply COMMAND', enhance 'thread apply COMMAND'
>
> Compared to RFC, this handles all comments received from Eli and Simon,
> and completes the changes so that it is (should be) ready for RFA.
>
> This patch series :
> * implements a new command
> 'frame apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND'.
> * enhance 'thread apply COMMAND' by adding a -FLAGS argument
> * adds some shortcuts commands
> * documents the above in gdb.texinfo and NEWS.
> * adds a unit test for cli-utils.c
> * adds test for 'frame apply'
> * modify gdb.threads/pthreads.exp to test 'thread apply' -FLAGS argument
>
> Th new command 'frame apply' allows to apply a COMMAND to a number of frames,
> or to all frames.
> The optional -FLAGS... argument allows to control what output to produce
> and how to handle errors raised when applying COMMAND to a frame.
>
> Some examples usages for this new command:
> frame apply all info frame
> Produce info frame for all frames
> frame apply all p $sp
> For each frame, print the location, followed by the frame sp
> frame apply all -qq p $sp
> Same as before, but -qq flags (q = quiet) indicate to only print
> the frames sp.
> frame apply all -vv p $sp
> Same as before, but -vv flags (v = verbose) indicate to print
> location and source line for each frame.
> frame apply all p some_local_var_somewhere
> Print some_local_var_somewhere in all frames. 'frame apply'
> will abort as soon as the print command fails.
> frame apply all -c p some_local_var_somewhere
> Same as before, but -c flag (c = continue) means to
> print the error and continue applying command in case the
> print command fails.
> frame apply all -s p some_local_var_somewhere
> Same as before, but -s flag (s = silent) means to
> be silent for frames where the print command fails.
> In other words, this allows to 'search' the frame in which
> some_local_var_somewhere can be printed.
>
> 'thread apply' command has been enhanced to also accepts a -FLAGS...
> argument.
>
> Some examples usages for this new argument:
> thread apply all -s frame apply all -s p some_local_var_somewhere
> Prints the thread id, frame location and some_local_var_somewhere
> value in frames of threads that have such local var.
>
> To make the life of the user easier, the most typical use cases
> have shortcuts :
> faas : shortcut for 'frame apply all -s'
> taas : shortcut for 'thread apply all -s'
> tfaas : shortcut for 'thread apply all -s frame apply all -s"
>
> An example usage :
> tfaas p some_local_var_somewhere
> same as the longer:
> 'thread apply all -s frame apply all -s p some_local_var_somewhere'
>
> gdb/ChangeLog
> 2018-05-21 Philippe Waroquiers <philippe.waroquiers@skynet.be>
>
> * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
> unittests/cli-utils-selftests.c
> * NEWS: Mention new commands. Mention change to 'thread apply'.
> * cli-utils.c (number_or_range_parser::get_number): Only handle
> numbers or convenience var as numbers.
> (check_for_flags): New function.
> (check_for_flags_vqcs): New function.
> * cli-utils.h (check_for_flags): New function.
> (check_for_flags_vqcs): New function.
> * stack.c: (trailing_outermost_frame): New function, mostly
> extracted from backtrace_command_1.
> (backtrace_command_1): Update to call trailing_outermost_frame.
> (frame_apply_command_count): New function.
> (frame_apply_all_command): New function.
> (frame_apply_command): New function.
> (faas_command): New function.
> (frame_cmd_list): New variable.
> (_initialize_stack): Update to setup the new commands 'frame apply'
> and 'faas'.
> * thread.c (thr_try_catch_cmd): New function.
> (thread_apply_all_command): Handle vqcs flags.
> (thread_apply_command): Handle vqcs flags.
> (taas_command): New function.
> (tfaas_command): New function.
> (_initialize_thread): Update to setup the new commands 'taas
> and 'tfaas'. Change doc string for 'thread apply'.
> * unittests/cli-utils-selftests.c: New file.
>
> gdb/testsuite/ChangeLog
> 2018-05-21 Philippe Waroquiers <philippe.waroquiers@skynet.be>
>
> * gdb.base/frameapply.c: New file.
> * gdb.base/frameapply.exp: New file.
> * gdb.threads/pthreads/exp: Test vqcs FLAGS.
>
> gdb/doc/ChangeLog
> 2018-05-21 Philippe Waroquiers <philippe.waroquiers@skynet.be>
>
> * gdb.texinfo (Debugging Programs with Multiple Threads):
> Document changes to 'thread apply'. Document 'taas'.
> Document 'tfaas'.
> (Examining the Stack): Document 'frame apply'. Document 'faas'.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [RFA 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND'
2018-05-21 11:07 [RFA 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND' Philippe Waroquiers
` (8 preceding siblings ...)
2018-05-28 22:18 ` [PING] [RFA 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND' Philippe Waroquiers
@ 2018-06-01 14:32 ` Pedro Alves
2018-06-01 19:38 ` Philippe Waroquiers
9 siblings, 1 reply; 17+ messages in thread
From: Pedro Alves @ 2018-06-01 14:32 UTC (permalink / raw)
To: Philippe Waroquiers, gdb-patches
Hi Philippe,
I really wish I had time to play a bit more with the series
(I really like the idea of "frame apply") and do a more in-depth
review today, but I probably won't, so here are some quick comments.
On 05/21/2018 12:06 PM, Philippe Waroquiers wrote:
> Implement 'frame apply COMMAND', enhance 'thread apply COMMAND'
>
> Compared to RFC, this handles all comments received from Eli and Simon,
> and completes the changes so that it is (should be) ready for RFA.
>
> This patch series :
> * implements a new command
> 'frame apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND'.
> * enhance 'thread apply COMMAND' by adding a -FLAGS argument
> * adds some shortcuts commands
> * documents the above in gdb.texinfo and NEWS.
> * adds a unit test for cli-utils.c
> * adds test for 'frame apply'
> * modify gdb.threads/pthreads.exp to test 'thread apply' -FLAGS argument
I'm not sure the idea of using "-" for flags is a good one,
because that conflicts with GDB's usual use of "-" for long
options, which can be abbreviated, and cannot be combined.
For example, "watch -location", "watch -l".
A while ago I was playing with adding a generic framework for
command options, which also handled TAB completion
automatically, and I was thinking about how gdb doesn't use "--"
for long options unlike getopt, and how single-"-" for long options
prevent combining options with a single "-", like you can
with "ls --all --size" -> "ls -as". Then I realized something that I
had haven't seen written down, but I thought made some sense. That
is, that we do have at least one command that allows combining short
options, "x/FMT", and it just uses "/" instead of "-" I.e., we
could make that the way to handle short vs long options throughout.
I.e., comparing gdb's options to getopt-like options yields this:
| getopt | gdb |
long | -- | - |
short | - | / |
So I'm wondering about using / for these new flags too.
Like, long form "-verbose -continue", short form "/vc".
Or you could sidestep the issue by ditching support for
combining flags, i.e., require "-v -v -c" instead of "-vvc".
>
> Th new command 'frame apply' allows to apply a COMMAND to a number of frames,
> or to all frames.
> The optional -FLAGS... argument allows to control what output to produce
> and how to handle errors raised when applying COMMAND to a frame.
>
> Some examples usages for this new command:
> frame apply all info frame
> Produce info frame for all frames
> frame apply all p $sp
> For each frame, print the location, followed by the frame sp
> frame apply all -qq p $sp
> Same as before, but -qq flags (q = quiet) indicate to only print
> the frames sp.
> frame apply all -vv p $sp
> Same as before, but -vv flags (v = verbose) indicate to print
> location and source line for each frame.
> frame apply all p some_local_var_somewhere
> Print some_local_var_somewhere in all frames. 'frame apply'
> will abort as soon as the print command fails.
> frame apply all -c p some_local_var_somewhere
> Same as before, but -c flag (c = continue) means to
> print the error and continue applying command in case the
> print command fails.
> frame apply all -s p some_local_var_somewhere
> Same as before, but -s flag (s = silent) means to
> be silent for frames where the print command fails.
> In other words, this allows to 'search' the frame in which
> some_local_var_somewhere can be printed.
>
> 'thread apply' command has been enhanced to also accepts a -FLAGS...
> argument.
>
> Some examples usages for this new argument:
> thread apply all -s frame apply all -s p some_local_var_somewhere
> Prints the thread id, frame location and some_local_var_somewhere
> value in frames of threads that have such local var.
>
> To make the life of the user easier, the most typical use cases
> have shortcuts :
> faas : shortcut for 'frame apply all -s'
> taas : shortcut for 'thread apply all -s'
> tfaas : shortcut for 'thread apply all -s frame apply all -s"
I'm not particularly sold on adding aliases, since you can
abbreviate and tab-complete. Users are used to "t a a",
for example, so I think "f a a" will come naturally, and
users can add aliases themselves with the "alias" command.
But that may be because I haven't played with the patches much yet.
> An example usage :
> tfaas p some_local_var_somewhere
> same as the longer:
> 'thread apply all -s frame apply all -s p some_local_var_somewhere'
>
> gdb/ChangeLog
> 2018-05-21 Philippe Waroquiers <philippe.waroquiers@skynet.be>
> gdb/testsuite/ChangeLog
> 2018-05-21 Philippe Waroquiers <philippe.waroquiers@skynet.be>
>
> gdb/doc/ChangeLog
> 2018-05-21 Philippe Waroquiers <philippe.waroquiers@skynet.be>
>
Is the idea that the patches should be merged as a single bigger
patch? Otherwise, the ChangeLogs should be split into the
individual patches.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [RFA 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND'
2018-06-01 14:32 ` Pedro Alves
@ 2018-06-01 19:38 ` Philippe Waroquiers
2018-06-04 16:46 ` Pedro Alves
0 siblings, 1 reply; 17+ messages in thread
From: Philippe Waroquiers @ 2018-06-01 19:38 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On Fri, 2018-06-01 at 15:32 +0100, Pedro Alves wrote:
> Hi Philippe,
>
> I really wish I had time to play a bit more with the series
> (I really like the idea of "frame apply") and do a more in-depth
> review today, but I probably won't, so here are some quick comments.
Hello Pedro,
Thanks for these comments.
Note that this work is a re-implementation of an earlier prototype
(the 'block of commands' prototype) that a.o. added some flags and
an optional trailing COMMAND to:
backtrace [QUALIFIERS]... [FLAGS] [COUNT] [COMMAND]
Doug Evans then suggested to rather have a new "frame apply",
which this patch provides. So, the "frame apply" elegance
is "copyrighted" Doug Evans :).
In this earlier prototype, the FLAGS were using /.
Below a tentative explanation (very long, sorry for that) why
this patch rather uses '-' flags.
>
> On 05/21/2018 12:06 PM, Philippe Waroquiers wrote:
> > Implement 'frame apply COMMAND', enhance 'thread apply COMMAND'
> >
> > Compared to RFC, this handles all comments received from Eli and Simon,
> > and completes the changes so that it is (should be) ready for RFA.
> >
> > This patch series :
> > * implements a new command
> > 'frame apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND'.
> > * enhance 'thread apply COMMAND' by adding a -FLAGS argument
> > * adds some shortcuts commands
> > * documents the above in gdb.texinfo and NEWS.
> > * adds a unit test for cli-utils.c
> > * adds test for 'frame apply'
> > * modify gdb.threads/pthreads.exp to test 'thread apply' -FLAGS argument
>
> I'm not sure the idea of using "-" for flags is a good one,
> because that conflicts with GDB's usual use of "-" for long
> options, which can be abbreviated, and cannot be combined.
>
> For example, "watch -location", "watch -l".
>
> A while ago I was playing with adding a generic framework for
> command options, which also handled TAB completion
> automatically, and I was thinking about how gdb doesn't use "--"
> for long options unlike getopt, and how single-"-" for long options
> prevent combining options with a single "-", like you can
> with "ls --all --size" -> "ls -as". Then I realized something that I
> had haven't seen written down, but I thought made some sense. That
> is, that we do have at least one command that allows combining short
> options, "x/FMT", and it just uses "/" instead of "-" I.e., we
> could make that the way to handle short vs long options throughout.
> I.e., comparing gdb's options to getopt-like options yields this:
>
> | getopt | gdb |
> long | -- | - |
> short | - | / |
>
> So I'm wondering about using / for these new flags too.
>
> Like, long form "-verbose -continue", short form "/vc".
>
> Or you could sidestep the issue by ditching support for
> combining flags, i.e., require "-v -v -c" instead of "-vvc".
When re-starting this work, I looked at the various rich different
ways GDB command parses 'flags/options/qualifiers/modifiers/...'
where "various rich different ways" is a politically correct synonym
of "mess" :).
We have at least:
* some commands have options but without -, and these options can be
abbreviated, e.g. backtrace full same as backtrace f
or backtrace fu
(this abbreviation feature seems not documented)
* some commands have options such as -x, and use -- as option terminator
e.g. alias -a -- -myaliasfordownstartswithaminus = down
demangle [-l language] [--] name
* some commands have long option that starts with a -, and cannot be
abbreviated e.g. thread apply all -ascending p 1
* some commands have a short -a option, but accepts whatever after
e.g. (gdb) interrupt -ahboncettecommandeacceptenimportequoi
* some commands have /Â e.g. print,x, disassemble,ptype, ... commands
For print and x, what follows / is a FMT,
while for disassemble, these are more like flag options.
Based on the above, it is not very clear what is the
'less unusual' usual way to specify flags/options.
In parallel, I have started another patch to have e.g.
info var [-t TYPE_REGEXP ] REGEXP
to only show the var having a type matching TYPE_REGEXP
This is showing yet another difficulty: how to put a space
in an argument ?
At that point, it looked like one of my next patch should
be an 'option/argument parser', basically the same as what
you describe above with the 'generic framework for command
options'. This framework should provide option parsing
and argument parsing, and allow optional quoting.
IMO, we better base this as much as possible to be
similar to the usual getopt, with at least the following
differences:
* for backward compatibility, we should support sometimes
alternative backward compatible behaviour, together with
a new 'standard behaviour' e.g.
backtrace [--full | -f | full ] ....
or
thread apply all [--ascending | -a | -ascending ]
* have -- systematically allowing to terminate options
(if we go for /, then should // terminate options then?)
* have a way to (explicitely) quote an argument e.g.
info var -Qt 'long int' regexpofsomevars
where -Q indicates that the next "value argument" is
explicitely quoted.
* allow combining short one letter args such as -cs
but also allow -c -s
* support (at least for disassemble/ptype/..., maybe not
for FMT that would still be only like it is now) the
/x backward compatible form, but provide also new
'standard' options e.g.Â
disassemble [-m | /m | --mixed] [-s | /s | --source] ...
* common behaviour of such option/arg parsing should be centrally
documented, in particular quoting, abbreviation of long options,
terminating options with --, ...
* ... any other idea/needed feature ?
What this patch gives is compatible with this (future generic
framework) option/arg parser to be developed. If we assume that
the 'long single -' option (such as -ascending) cannot be
abbreviated, then I think we can make something backward compatible
but go to a more uniform option/arg parsing (and avoid
'local' re-implementation of option parsing logic such as
skip spaces etc).
Of course, if in this patchn, we mandate -v -v -c -s
instead of -vvcs, that would be equally compatible with the
future option/arg parser
('future' is the politically correct synonym of vapourware :).
So, in summary, from my point of view, in the future,
we better go for something getopt compatible, and have
backward compatibility for the existing flags/modifier,
and so have -FLAGS in this patch rather than /FLAGS.
>
> >
> > Th new command 'frame apply' allows to apply a COMMAND to a number of frames,
> > or to all frames.
> > The optional -FLAGS... argument allows to control what output to produce
> > and how to handle errors raised when applying COMMAND to a frame.
> >
> > Some examples usages for this new command:
> > frame apply all info frame
> > Produce info frame for all frames
> > frame apply all p $sp
> > For each frame, print the location, followed by the frame sp
> > frame apply all -qq p $sp
> > Same as before, but -qq flags (q = quiet) indicate to only print
> > the frames sp.
> > frame apply all -vv p $sp
> > Same as before, but -vv flags (v = verbose) indicate to print
> > location and source line for each frame.
> > frame apply all p some_local_var_somewhere
> > Print some_local_var_somewhere in all frames. 'frame apply'
> > will abort as soon as the print command fails.
> > frame apply all -c p some_local_var_somewhere
> > Same as before, but -c flag (c = continue) means to
> > print the error and continue applying command in case the
> > print command fails.
> > frame apply all -s p some_local_var_somewhere
> > Same as before, but -s flag (s = silent) means to
> > be silent for frames where the print command fails.
> > In other words, this allows to 'search' the frame in which
> > some_local_var_somewhere can be printed.
> >
> > 'thread apply' command has been enhanced to also accepts a -FLAGS...
> > argument.
> >
> > Some examples usages for this new argument:
> > thread apply all -s frame apply all -s p some_local_var_somewhere
> > Prints the thread id, frame location and some_local_var_somewhere
> > value in frames of threads that have such local var.
> >
> > To make the life of the user easier, the most typical use cases
> > have shortcuts :
> > faas : shortcut for 'frame apply all -s'
> > taas : shortcut for 'thread apply all -s'
> > tfaas : shortcut for 'thread apply all -s frame apply all -s"
>
> I'm not particularly sold on adding aliases, since you can
> abbreviate and tab-complete. Users are used to "t a a",
> for example, so I think "f a a" will come naturally, and
> users can add aliases themselves with the "alias" command.
> But that may be because I haven't played with the patches much yet.
I could not make an alias that was specifying -s or any option,
e.g.
(gdb) alias inta = interrupt -a
Invalid command to alias to: interrupt -a
(gdb)
and I found e.g.
t a a -s f a a -sÂ
quite long to type, and so worth the aliases.
Â
>
> > An example usage :
> > tfaas p some_local_var_somewhere
> > same as the longer:
> > 'thread apply all -s frame apply all -s p some_local_var_somewhere'
> >
> > gdb/ChangeLog
> > 2018-05-21 Philippe Waroquiers <philippe.waroquiers@skynet.be>
> > gdb/testsuite/ChangeLog
> > 2018-05-21 Philippe Waroquiers <philippe.waroquiers@skynet.be>
> >
> > gdb/doc/ChangeLog
> > 2018-05-21 Philippe Waroquiers <philippe.waroquiers@skynet.be>
> >
>
> Is the idea that the patches should be merged as a single bigger
> patch? Otherwise, the ChangeLogs should be split into the
> individual patches.
Yes, the idea is to push several commits (like in the RFA).
So, I will split the ChangeLog (I am not yet used to
all the subtilities of the ChangeLog cargo-cult :).
Thanks for the feedback/discussion,
the most central point is for sure the - versus / discussion.
Philippe
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [RFA 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND'
2018-06-01 19:38 ` Philippe Waroquiers
@ 2018-06-04 16:46 ` Pedro Alves
2018-06-04 20:56 ` Philippe Waroquiers
0 siblings, 1 reply; 17+ messages in thread
From: Pedro Alves @ 2018-06-04 16:46 UTC (permalink / raw)
To: Philippe Waroquiers, gdb-patches
On 06/01/2018 08:38 PM, Philippe Waroquiers wrote:
> On Fri, 2018-06-01 at 15:32 +0100, Pedro Alves wrote:
>> Hi Philippe,
>>
>> I really wish I had time to play a bit more with the series
>> (I really like the idea of "frame apply") and do a more in-depth
>> review today, but I probably won't, so here are some quick comments.
> Hello Pedro,
> Thanks for these comments.
> Note that this work is a re-implementation of an earlier prototype
> (the 'block of commands' prototype) that a.o. added some flags and
> an optional trailing COMMAND to:
> backtrace [QUALIFIERS]... [FLAGS] [COUNT] [COMMAND]
> Doug Evans then suggested to rather have a new "frame apply",
> which this patch provides. So, the "frame apply" elegance
> is "copyrighted" Doug Evans :).
>
:-)
> When re-starting this work, I looked at the various rich different
> ways GDB command parses 'flags/options/qualifiers/modifiers/...'
> where "various rich different ways" is a politically correct synonym
> of "mess" :).
:-)
>
> We have at least:
> * some commands have options but without -, and these options can be
> abbreviated, e.g. backtrace full same as backtrace f
> or backtrace fu
> (this abbreviation feature seems not documented)
Yes, these will tend to be very old commands / options.
> * some commands have options such as -x, and use -- as option terminator
> e.g. alias -a -- -myaliasfordownstartswithaminus = down
> demangle [-l language] [--] name
Right, newer commands started doing that. Since we don't have a generic
framework, it's been done on an as-needed, or in-case-we-need-it-in-future
basis. I know I pushed to add it more than once in review.
> * some commands have long option that starts with a -, and cannot be
> abbreviated e.g. thread apply all -ascending p 1
Right. I'd call that a bug.
> * some commands have a short -a option, but accepts whatever after
> e.g. (gdb) interrupt -ahboncettecommandeacceptenimportequoi
LOL
> * some commands have /Â e.g. print,x, disassemble,ptype, ... commands
> For print and x, what follows / is a FMT,
> while for disassemble, these are more like flag options.
>
> Based on the above, it is not very clear what is the
> 'less unusual' usual way to specify flags/options.
It should at least be clear that single-'-' is used throughout,
while double-'-' like gnuopt does (by default) for long options
is never used, except in the '--' terminator.
The explicit location options for example, is a clear
example of more modern GDB options. There, we accept
"break -function", "break -line", etc. and all abbreviations,
like "b -fun", "b -f", etc. And very importantly, we
support TAB completion of command options. I think that's
the killer feature. It helps with the typing _and_ the
discoverability of the user interface. I.e., you can
type "b -TAB" to see all the options you can pass to the
command. I'd like to be able to have that for _all_ commands.
>
> In parallel, I have started another patch to have e.g.
> info var [-t TYPE_REGEXP ] REGEXP
> to only show the var having a type matching TYPE_REGEXP
> This is showing yet another difficulty: how to put a space
> in an argument ?
Unless it's the last/rightmost argument, then I think the
best choice is to have the user quote or escape if
necessary?
Note that "b -function foo (int) -line 10" works, even
though there's a space in "foo (int)", because the explicit
location's parser was taught to skip past function names,
and in that case, we know that ()s are balanced. But it's
tricky code, and the same probably can't be done for
REGEXPs, I think.
>
> At that point, it looked like one of my next patch should
> be an 'option/argument parser', basically the same as what
> you describe above with the 'generic framework for command
> options'. This framework should provide option parsing
> and argument parsing, and allow optional quoting.
> IMO, we better base this as much as possible to be
> similar to the usual getopt, with at least the following
> differences:
> * for backward compatibility, we should support sometimes
> alternative backward compatible behaviour, together with
> a new 'standard behaviour' e.g.
> backtrace [--full | -f | full ] ....
> or
> thread apply all [--ascending | -a | -ascending ]
That's basically unlike any command option in GDB currently.
We've settled on single-'-' a long time ago, so I'm not
seeing how breaking that buys us much other than another
long partial transition, lots of confusion and breaking
scripts and user habits.
Because we currently accept single '-' as long option
specifier, I don't see how we can both keep backward
compatibility with current "b -function" etc, _and_
allow single '-' as specifier for
short-options-that-can-be-combined, because that causes ambiguity
like, what does "(gdb) cmd -fu" mean:
#1 - (gdb) cmd -function
#2 - (gdb) cmd -function -unique
?
I don't think that saying 'it's "-function" if "-unique" doesn't
exist yet', because then we need worry about the fact that adding
any option can break any abbreviated use of existing options.
> * have -- systematically allowing to terminate options
*nod*
> (if we go for /, then should // terminate options then?)
> * have a way to (explicitely) quote an argument e.g.
> info var -Qt 'long int' regexpofsomevars
> where -Q indicates that the next "value argument" is
> explicitely quoted.
Not sure we need that -Q. We can support optional quotes, and
tell users to add quotes if necessary? That's what we've done
since forever in linespecs and expressions, for example.
> * allow combining short one letter args such as -cs
> but also allow -c -s
I think that way lies madness.
> * support (at least for disassemble/ptype/..., maybe not
> for FMT that would still be only like it is now) the
> /x backward compatible form, but provide also new
> 'standard' options e.g.Â
> disassemble [-m | /m | --mixed] [-s | /s | --source] ...
> * common behaviour of such option/arg parsing should be centrally
> documented, in particular quoting, abbreviation of long options,
> terminating options with --, ...
> * ... any other idea/needed feature ?
>
> What this patch gives is compatible with this (future generic
> framework) option/arg parser to be developed.
As mentioned above, I think the combining the short options under
a single '-' is problematic. As such, I'd rather see that removed
from the patch, and the flags implemented as regular
individually-specified options. I.e., have the user type "-s -c"
instead of "-sc". It's really not much of a difference for the user.
> If we assume that
> the 'long single -' option (such as -ascending) cannot be
> abbreviated,
I don't think that's desirable. I think making GDB support
"-ascending" abbreviations would be an obviously desirable patch.
> then I think we can make something backward compatible
> but go to a more uniform option/arg parsing (and avoid
> 'local' re-implementation of option parsing logic such as
> skip spaces etc).
> Of course, if in this patchn, we mandate -v -v -c -s
> instead of -vvcs, that would be equally compatible with the
> future option/arg parser
> ('future' is the politically correct synonym of vapourware :).
I resent the "vapourware" remark. :-P It's real! :-P
See here:
https://github.com/palves/gdb/commits/palves/cmd-options
That started out with some discussions about changing the
defaults of some of the "set print" options, like
"set print static-members" and "set print object".
It occurred to me then that part of the problem of picking
defaults is that it's not possible to override the
"set print" options in individual single-shot "print"
command invocations. If we could do that, then the
defaults are a little bit less important (though still
important).
So that branch wires in all the "set print FOO" toggles
backed by value_print_options in the code as "print" command options,
and along the way introduces a framework to make it happen:
(gdb) set print [TAB]
address max-symbolic-offset static-members
array null-stop symbol
array-indexes object symbol-filename
asm-demangle pascal_static-members symbol-loading
demangle pretty thread-events
elements raw type
entry-values raw-frame-arguments union
frame-arguments repeats vtbl
inferior-events sevenbit-strings
(gdb) set print
(gdb) print -[TAB]
-address -elements -pretty -symbol
-array -null-stop -repeats -union
-array-indexes -object -static-members -vtbl
(gdb) print -
(gdb) p array
$1 = {{elem = 1, static s = 0}, {elem = 1, static s = 0}, {elem = 1, static s = 0}, {elem = 1, static s = 0}, {elem = 1, static s = 0}, {elem = 1, static s = 0}, {
elem = 1, static s = 0}, {elem = 1, static s = 0}, {elem = 1, static s = 0}, {elem = 1, static s = 0}}
(gdb) p -static-members off array
$2 = {{elem = 1}, {elem = 1}, {elem = 1}, {elem = 1}, {elem = 1}, {elem = 1}, {elem = 1}, {elem = 1}, {elem = 1}, {elem = 1}}
(gdb) p -pretty -static-members off -- array
$3 = {{
elem = 1
}, {
elem = 1
}, {
elem = 1
}, {
elem = 1
}, {
elem = 1
}, {
elem = 1
}, {
elem = 1
}, {
elem = 1
}, {
elem = 1
}, {
elem = 1
}}
etc.
I've also hooked it to a few other commands for experimentation:
(gdb) bt -[TAB]
-entry-values -frame-arguments -full -hide -no-filters -raw-frame-arguments
(gdb) info threads [TAB]
-gid THREAD_ID_LIST
(gdb) thread apply all -[TAB]
(gdb) thread apply all -ascending # there are no other options currently
(gdb) help print
Print value of expression EXP.
Usage: print [[OPTION]... --] [EXP]
Variables accessible are those of the lexical environment of the selected
stack frame, plus all those whose scope is global or an entire file.
Options:
-address [on|off]
Set printing of addresses.
-array [on|off]
Set pretty formatting of arrays.
-array-indexes [on|off]
Set printing of array indexes.
-elements NUMBER | unlimited
Set limit on string chars or array elements to print.
"unlimited" causes there to be no limit.
-null-stop [on|off]
Set printing of char arrays to stop at first null char.
-object [on|off]
Set printing of C++ virtual function tables.
-pretty [on|off]
Set pretty formatting of structures.
-repeats NUMBER | unlimited
Set threshold for repeated print elements.
"unlimited" causes all elements to be individually printed.
-static-members [on|off]
Set printing of C++ static members.
-symbol [on|off]
Set printing of symbol names when printing pointers.
-union [on|off]
Set printing of unions interior to structures.
-vtbl [on|off]
Set printing of C++ virtual function tables.
[...]
(gdb)
I rebased that branch this weekend and played with it a bit more,
but there's still a lot to do. In particular, the internal API
grew organically as I threw new commands and use cases at it,
and I wouldn't call it final. To reiterate, a couple design goals
played into its current form:
#1 - desire to have normal parsing and completion take
most of the same code paths, so that we avoid
normal parsing and completion parsing going out
of sync. This is a continuation of the same idea that
I used for the linespec parser rework from last year.
#2 - share options / data structures with "set/show"
command options parsing.
I wrote most of this about 6 months ago, and haven't
context switched it all back in, but I think I wanted to
look to see if we could share more for point #2 above.
I encourage you (and others) to give it a try, particularly
play with TAB completion, see how that makes options more
discoverable and the CLI a better joy. IMO.
(and I think we could do a lot of neat things with
completion, like maybe display the completion matches
in one column, and option usage in another column, all
in the same display. And color!)
>
> So, in summary, from my point of view, in the future,
> we better go for something getopt compatible, and have
> backward compatibility for the existing flags/modifier,
> and so have -FLAGS in this patch rather than /FLAGS.
>
>>> Th new command 'frame apply' allows to apply a COMMAND to a number of frames,
>>> or to all frames.
>>> The optional -FLAGS... argument allows to control what output to produce
>>> and how to handle errors raised when applying COMMAND to a frame.
>>>
>>> Some examples usages for this new command:
>>> frame apply all info frame
>>> Produce info frame for all frames
>>> frame apply all p $sp
>>> For each frame, print the location, followed by the frame sp
>>> frame apply all -qq p $sp
>>> Same as before, but -qq flags (q = quiet) indicate to only print
>>> the frames sp.
>>> frame apply all -vv p $sp
>>> Same as before, but -vv flags (v = verbose) indicate to print
>>> location and source line for each frame.
>>> frame apply all p some_local_var_somewhere
>>> Print some_local_var_somewhere in all frames. 'frame apply'
>>> will abort as soon as the print command fails.
>>> frame apply all -c p some_local_var_somewhere
>>> Same as before, but -c flag (c = continue) means to
>>> print the error and continue applying command in case the
>>> print command fails.
>>> frame apply all -s p some_local_var_somewhere
>>> Same as before, but -s flag (s = silent) means to
>>> be silent for frames where the print command fails.
>>> In other words, this allows to 'search' the frame in which
>>> some_local_var_somewhere can be printed.
>>>
>>> 'thread apply' command has been enhanced to also accepts a -FLAGS...
>>> argument.
>>>
>>> Some examples usages for this new argument:
>>> thread apply all -s frame apply all -s p some_local_var_somewhere
>>> Prints the thread id, frame location and some_local_var_somewhere
>>> value in frames of threads that have such local var.
>>>
>>> To make the life of the user easier, the most typical use cases
>>> have shortcuts :
>>> faas : shortcut for 'frame apply all -s'
>>> taas : shortcut for 'thread apply all -s'
>>> tfaas : shortcut for 'thread apply all -s frame apply all -s"
>> I'm not particularly sold on adding aliases, since you can
>> abbreviate and tab-complete. Users are used to "t a a",
>> for example, so I think "f a a" will come naturally, and
>> users can add aliases themselves with the "alias" command.
>> But that may be because I haven't played with the patches much yet.
> I could not make an alias that was specifying -s or any option,
> e.g.
> (gdb) alias inta = interrupt -a
> Invalid command to alias to: interrupt -a
> (gdb)
That sounds like something we should fix, regardless.
> and I found e.g.
> t a a -s f a a -sÂ
> quite long to type, and so worth the aliases.
I wonder whether that's a real use case in practice though.
What sort of thing does one want to print in all frames
of all threads? Genuinely curious.
> Â
>>> An example usage :
>>> tfaas p some_local_var_somewhere
>>> same as the longer:
>>> 'thread apply all -s frame apply all -s p some_local_var_somewhere'
I could bite that, even though it looks a bit contrived to me.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [RFA 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND'
2018-06-04 16:46 ` Pedro Alves
@ 2018-06-04 20:56 ` Philippe Waroquiers
2018-06-05 16:49 ` Pedro Alves
0 siblings, 1 reply; 17+ messages in thread
From: Philippe Waroquiers @ 2018-06-04 20:56 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On Mon, 2018-06-04 at 17:46 +0100, Pedro Alves wrote:
> On 06/01/2018 08:38 PM, Philippe Waroquiers wrote:
> > Based on the above, it is not very clear what is the
> > 'less unusual' usual way to specify flags/options
>
> It should at least be clear that single-'-' is used throughout,
> while double-'-' like gnuopt does (by default) for long options
> is never used, except in the '--' terminator.
Yes.
>
> The explicit location options for example, is a clear
> example of more modern GDB options. There, we accept
> "break -function", "break -line", etc. and all abbreviations,
> like "b -fun", "b -f", etc.
When I looked, I did not discover this case of
'long options starting with a -, and that can be abbreviated'.
I thought long options were all like 'thread apply -ascending'
and could not be abbreviated.
> And very importantly, we
> support TAB completion of command options. I think that's
> the killer feature. It helps with the typing _and_ the
> discoverability of the user interface. I.e., you can
> type "b -TAB" to see all the options you can pass to the
> command. I'd like to be able to have that for _all_ commands.
Yes, that would be very nice.
>
> >
> > In parallel, I have started another patch to have e.g.
> > info var [-t TYPE_REGEXP ] REGEXP
> > to only show the var having a type matching TYPE_REGEXP
> > This is showing yet another difficulty: how to put a space
> > in an argument ?
>
> Unless it's the last/rightmost argument, then I think the
> best choice is to have the user quote or escape if
> necessary?
Yes, that should work, and is better/more intuitive.
>
> Note that "b -function foo (int) -line 10" works, even
> though there's a space in "foo (int)", because the explicit
> location's parser was taught to skip past function names,
> and in that case, we know that ()s are balanced. But it's
> tricky code, and the same probably can't be done for
> REGEXPs, I think.
Such arg parsing intelligence might make a
generic parser framework somewhat more tricky,
or at least the generic framework will have to "know"
the syntax of all such specially parsed options,
or maybe delegate the parsing of some arguments
to some arg specific code depending on the option type,
which can then make e.g. the -- end of option recognition
to be done at several places.
And effectively, see where a REGEXP terminates and see
where the next REGEXP starts is not doable without
quoting.
>
> >
> > At that point, it looked like one of my next patch should
> > be an 'option/argument parser', basically the same as what
> > you describe above with the 'generic framework for command
> > options'. This framework should provide option parsing
> > and argument parsing, and allow optional quoting.
> > IMO, we better base this as much as possible to be
> > similar to the usual getopt, with at least the following
> > differences:
> > * for backward compatibility, we should support sometimes
> > alternative backward compatible behaviour, together with
> > a new 'standard behaviour' e.g.
> > backtrace [--full | -f | full ] ....
> > or
> > thread apply all [--ascending | -a | -ascending ]
>
> That's basically unlike any command option in GDB currently.
> We've settled on single-'-' a long time ago, so I'm not
> seeing how breaking that buys us much other than another
> long partial transition, lots of confusion and breaking
> scripts and user habits.
The above idea was supposed to be backward compatible,
but is completely broken by the fact that long single - options
can be abbreviated.
>
> Because we currently accept single '-' as long option
> specifier, I don't see how we can both keep backward
> compatibility with current "b -function" etc, _and_
> allow single '-' as specifier for
> short-options-that-can-be-combined, because that causes ambiguity
> like, what does "(gdb) cmd -fu" mean:
>
> #1 - (gdb) cmd -function
> #2 - (gdb) cmd -function -unique
>
> ?
>
> I don't think that saying 'it's "-function" if "-unique" doesn't
> exist yet', because then we need worry about the fact that adding
> any option can break any abbreviated use of existing options.
Agreed.
Note we already have some worry with the current status:
E.g. break -l meaning depends on how many options
starts with -l, and in which order the code checks them.
If break was accepting initially only -line,
and -label is added as a new option : if the code first
checks for -label, then -l will change of semantic
and/or cause an error.
Note that the doc for break says the abbreviation must have
the unique prefix characters, but that of course
depends on the future options being added
(and today, break -l is accepted and interpreted as break -line).
Note that getopt does not accept ambiguity e.g. :
$ ls --a
ls: option '--a' is ambiguous; possibilities: '--all' '--author' '--almost-all'
> > * have a way to (explicitely) quote an argument e.g.
> > info var -Qt 'long int' regexpofsomevars
> > where -Q indicates that the next "value argument" is
> > explicitely quoted.
>
> Not sure we need that -Q. We can support optional quotes, and
> tell users to add quotes if necessary? That's what we've done
> since forever in linespecs and expressions, for example.
Yes I think that should work.
>
> > * allow combining short one letter args such as -cs
> > but also allow -c -s
>
> I think that way lies madness.
Yes, that looks clear to me now that I know about
abbreviated long single - options.
> As mentioned above, I think the combining the short options under
> a single '-' is problematic. As such, I'd rather see that removed
> from the patch, and the flags implemented as regular
> individually-specified options. I.e., have the user type "-s -c"
> instead of "-sc".
Agreed : we cannot have both abbreviated long single - option
and have combined
short options.
>
> > If we assume that
> > the 'long single -' option (such as -ascending) cannot be
> > abbreviated,
>
> I don't think that's desirable. I think making GDB support
> "-ascending" abbreviations would be an obviously desirable patch.
>
> > then I think we can make something backward compatible
> > but go to a more uniform option/arg parsing (and avoid
> > 'local' re-implementation of option parsing logic such as
> > skip spaces etc).
> > Of course, if in this patchn, we mandate -v -v -c -s
> > instead of -vvcs, that would be equally compatible with the
> > future option/arg parser
> > ('future' is the politically correct synonym of vapourware :).
>
> I resent the "vapourware" remark. :-P It's real! :-P
Humph, sorry. I wanted to indicate my own suggestion
(a getopt like parser for --longoptions) was vapourware.
I clearly understood you produced some real code ...
> I wrote most of this about 6 months ago, and haven't
> context switched it all back in, but I think I wanted to
> look to see if we could share more for point #2 above.
>
> I encourage you (and others) to give it a try, particularly
> play with TAB completion, see how that makes options more
> discoverable and the CLI a better joy. IMO.
And the examples you give looks really nice ...
>
> > I could not make an alias that was specifying -s or any option,
> > e.g.
> > (gdb) alias inta = interrupt -a
> > Invalid command to alias to: interrupt -a
> > (gdb)
>
> That sounds like something we should fix, regardless.
Ok. I have added this to my list of things to do one of these
week-ends ...
>
> > and I found e.g.
> > t a a -s f a a -sÂ
> > quite long to type, and so worth the aliases.
>
> I wonder whether that's a real use case in practice though.
> What sort of thing does one want to print in all frames
> of all threads? Genuinely curious.
The idea is to let gdb discover where a certain variable
or argument exists and can be printed.
This helps when things are optimised out in one frame,
but can be printed in upper frames (or upper
scope/nested blocks in Ada).
Another possible use case is something like :
tfaas info local -t sometype
to see all local vars in all frames of all threads
that have the given type. For example, sometype
might be an RAII type that locks whatever :Â
this is then a quick way to discover all active locks.
'info local -t TYPEREGEXP' is a patch I am working on,
but I am waiting an agreement on this patch
to RFC/RFA it ...
More generally, the idea is to let gdb find where a command
can be run successfully.
The plan (almost but not fully vapourware :)) is e.g. to allow the below :
thread apply all -s frame apply all -s if somecondition { some other gdb commands }
or whatever COMMAND that can succeed (or fail) in a frame.
{ and } is the idea of the 'block command' prototype I did a long time ago,
from where this 'apply a command to all frames' originates.
Depending on week-end free time, I will re-start the block of commands
work.
>
> > Â
> > > > An example usage :
> > > > tfaas p some_local_var_somewhere
> > > > same as the longer:
> > > > 'thread apply all -s frame apply all -s p some_local_var_somewhere'
>
> I could bite that, even though it looks a bit contrived to me.
>
> Thanks,
> Pedro Alves
At this point, it looks clear to me we better do not allow
combined short options. If we want to support combined short options
one day, then as you suggested, we need another syntax to combine them:
frame apply all /cvv p somevar
In the meantime, I can rework the patch to (only) accept separately
the -c -s -v -q flags.
Does that sound like a correct plan to you ?
Thanks
Philippe
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [RFA 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND'
2018-06-04 20:56 ` Philippe Waroquiers
@ 2018-06-05 16:49 ` Pedro Alves
0 siblings, 0 replies; 17+ messages in thread
From: Pedro Alves @ 2018-06-05 16:49 UTC (permalink / raw)
To: Philippe Waroquiers, gdb-patches
On 06/04/2018 09:56 PM, Philippe Waroquiers wrote:
> On Mon, 2018-06-04 at 17:46 +0100, Pedro Alves wrote:
>>
>> Note that "b -function foo (int) -line 10" works, even
>> though there's a space in "foo (int)", because the explicit
>> location's parser was taught to skip past function names,
>> and in that case, we know that ()s are balanced. But it's
>> tricky code, and the same probably can't be done for
>> REGEXPs, I think.
> Such arg parsing intelligence might make a
> generic parser framework somewhat more tricky,
> or at least the generic framework will have to "know"
> the syntax of all such specially parsed options,
> or maybe delegate the parsing of some arguments
> to some arg specific code depending on the option type,
> which can then make e.g. the -- end of option recognition
> to be done at several places.
> And effectively, see where a REGEXP terminates and see
> where the next REGEXP starts is not doable without
> quoting.
Yeah. I haven't tried converting the explicit locations
parser to the new framework, because that parser is very
tricky.
> Note we already have some worry with the current status:
> E.g. break -l meaning depends on how many options
> starts with -l, and in which order the code checks them.
> If break was accepting initially only -line,
> and -label is added as a new option : if the code first
> checks for -label, then -l will change of semantic
> and/or cause an error.
Not as bad IMHO, but true regardless.
> Note that the doc for break says the abbreviation must have
> the unique prefix characters, but that of course
> depends on the future options being added
> (and today, break -l is accepted and interpreted as break -line).
>
> Note that getopt does not accept ambiguity e.g. :
> $ ls --a
> ls: option '--a' is ambiguous; possibilities: '--all' '--author' '--almost-all'
Yeah. Traditionally GDB makes it so that old common shorthands continue
working. It's the same with command names too. E.g., "t" for "thread",
even though "t" is ambiguous. So I see the options following the
same principle.
>>> then I think we can make something backward compatible
>>> but go to a more uniform option/arg parsing (and avoid
>>> 'local' re-implementation of option parsing logic such as
>>> skip spaces etc).
>>> Of course, if in this patchn, we mandate -v -v -c -s
>>> instead of -vvcs, that would be equally compatible with the
>>> future option/arg parser
>>> ('future' is the politically correct synonym of vapourware :).
>>
>> I resent the "vapourware" remark. :-P It's real! :-P
> Humph, sorry. I wanted to indicate my own suggestion
> (a getopt like parser for --longoptions) was vapourware.
Ah, the joys of text communication misunderstandings.
No need to apologize, I was just kidding. :-)
>>> and I found e.g.
>>> t a a -s f a a -sÂ
>>> quite long to type, and so worth the aliases.
>>
>> I wonder whether that's a real use case in practice though.
>> What sort of thing does one want to print in all frames
>> of all threads? Genuinely curious.
> The idea is to let gdb discover where a certain variable
> or argument exists and can be printed.
> This helps when things are optimised out in one frame,
> but can be printed in upper frames (or upper
> scope/nested blocks in Ada).
>
> Another possible use case is something like :
> tfaas info local -t sometype
> to see all local vars in all frames of all threads
> that have the given type. For example, sometype
> might be an RAII type that locks whatever :Â
> this is then a quick way to discover all active locks.
Interesting. I wonder whether it would make sense to
give some example like this in the manual.
> At this point, it looks clear to me we better do not allow
> combined short options. If we want to support combined short options
> one day, then as you suggested, we need another syntax to combine them:
> frame apply all /cvv p somevar
>
> In the meantime, I can rework the patch to (only) accept separately
> the -c -s -v -q flags.
>
> Does that sound like a correct plan to you ?
Yes, it does. Thanks for doing all this!
Pedro Alves
^ permalink raw reply [flat|nested] 17+ messages in thread