* [PATCH 0/2] gdb: Allow parenthesis to group arguments to user-defined commands @ 2018-08-15 14:39 Andrew Burgess 2018-08-15 14:39 ` [PATCH 2/2] " Andrew Burgess 2018-08-15 14:39 ` [PATCH 1/2] gdb: Make testnames unique in gdb.base/commands.exp Andrew Burgess 0 siblings, 2 replies; 26+ messages in thread From: Andrew Burgess @ 2018-08-15 14:39 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess The first patch is a small clean up (unique test names), the second patch is the interesting one. --- Andrew Burgess (2): gdb: Make testnames unique in gdb.base/commands.exp gdb: Allow parenthesis to group arguments to user-defined commands gdb/ChangeLog | 5 ++++ gdb/cli/cli-script.c | 8 +++++- gdb/doc/ChangeLog | 5 ++++ gdb/doc/gdb.texinfo | 54 +++++++++++++++++++++++++++++++++---- gdb/testsuite/ChangeLog | 11 ++++++++ gdb/testsuite/gdb.base/commands.exp | 40 +++++++++++++++++++++++++-- gdb/testsuite/gdb.base/run.c | 3 +++ 7 files changed, 118 insertions(+), 8 deletions(-) -- 2.14.4 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 2/2] gdb: Allow parenthesis to group arguments to user-defined commands 2018-08-15 14:39 [PATCH 0/2] gdb: Allow parenthesis to group arguments to user-defined commands Andrew Burgess @ 2018-08-15 14:39 ` Andrew Burgess 2018-08-15 18:24 ` Eli Zaretskii ` (2 more replies) 2018-08-15 14:39 ` [PATCH 1/2] gdb: Make testnames unique in gdb.base/commands.exp Andrew Burgess 1 sibling, 3 replies; 26+ messages in thread From: Andrew Burgess @ 2018-08-15 14:39 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess When calling a user-defined command then currently, arguments are whitespace separated. This means that it is impossible to pass a single argument that contains a whitespace. The exception to the no whitespace rule is strings, a string argument, enclosed in double, or single quotes, can contain whitespace. However, if a user wants to reference, for example, a type name that contains a space, as in these examples: user_command *((unsigned long long *) some_pointer) user_command {unsigned long long}some_pointer then this will not work, as the whitespace between 'unsigned' and 'long', as well as the whitespace between 'long' and 'long', will mean GDB interprets this as many arguments. The solution proposed in this patch is to allow parenthesis to be used to group arguments, so the use could now write: user_command (*((unsigned long long *) some_pointer)) user_command ({unsigned long long}some_pointer) And GDB will interpret these as a single argument. gdb/ChangeLog: * cli/cli-script.c (user_args::user_args): Allow parenthesis to group arguments. gdb/testsuite/ChangeLog: * gdb.base/commands.exp (args_with_whitespace): New proc, which is added to the list of procs to call. * gdb.base/run.c (global_var): Defined global. gdb/doc/ChangeLog: * gdb.texinfo (Define): Additional documentation about argument syntax. --- gdb/ChangeLog | 5 ++++ gdb/cli/cli-script.c | 8 +++++- gdb/doc/ChangeLog | 5 ++++ gdb/doc/gdb.texinfo | 54 +++++++++++++++++++++++++++++++++---- gdb/testsuite/ChangeLog | 6 +++++ gdb/testsuite/gdb.base/commands.exp | 36 +++++++++++++++++++++++++ gdb/testsuite/gdb.base/run.c | 3 +++ 7 files changed, 111 insertions(+), 6 deletions(-) diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c index 6f31a400197..fd80ab9fbcf 100644 --- a/gdb/cli/cli-script.c +++ b/gdb/cli/cli-script.c @@ -758,6 +758,7 @@ user_args::user_args (const char *command_line) int squote = 0; int dquote = 0; int bsquote = 0; + int pdepth = 0; /* Strip whitespace. */ while (*p == ' ' || *p == '\t') @@ -769,7 +770,8 @@ user_args::user_args (const char *command_line) /* Get to the end of this argument. */ while (*p) { - if (((*p == ' ' || *p == '\t')) && !squote && !dquote && !bsquote) + if (((*p == ' ' || *p == '\t')) + && !squote && !dquote && !bsquote && pdepth == 0) break; else { @@ -793,6 +795,10 @@ user_args::user_args (const char *command_line) squote = 1; else if (*p == '"') dquote = 1; + else if (*p == '(') + pdepth++; + else if (*p == ')') + pdepth--; } p++; } diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 433a2698a92..11cbef97b8f 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -25219,14 +25219,58 @@ @noindent This defines the command @code{adder}, which prints the sum of -its three arguments. Note the arguments are text substitutions, so they may -reference variables, use complex expressions, or even perform inferior -functions calls. +its three arguments. + +The arguments to user-defined commands are text substitutions, so they +may reference variables, use complex expressions, or even perform +inferior functions calls. Each argument is separated with whitespace, +so in the previous example three arguments were passed. The following +example also passes three arguments, though the arguments are more +complex: + +@smallexample +adder 10+1 10+2 10+3 +@end smallexample + +@noindent +However, if whitespace were added around the @code{+} characters, then +9 arguments would be passed, @code{adder} only uses the first 3 of +these arguments, and the others would be silently ignored: + +@smallexample +adder 10 + 1 10 + 2 10 + 3 +@end smallexample + +@noindent +Parenthesis can be uses to group complex expressions that include +whitespace into a single argument, so the previous example can be +modified to pass just 3 arguments again, like this: + +@smallexample +adder (10 + 1) (10 + 2) (10 + 3) +@end smallexample + +@noindent +The parenthesis are passed through as part of the argument, so the +previous example causes @value{GDBN} to evaluate: + +@smallexample +print (10 + 1) + (10 + 2) + (10 + 3) +@end smallexample + +@noindent +Nested parenthesis are also allowed within an argument, in the +following example 3 arguments are still passed: + +@smallexample +adder (10 + 1) (10 + (1 + 1)) (10 + (1 + (1 + 1))) +@end smallexample @cindex argument count in user-defined commands @cindex how many arguments (user-defined commands) -In addition, @code{$argc} may be used to find out how many arguments have -been passed. +@noindent +Within a user-defined command @code{$argc} may be used to find out how +many arguments have been passed. @smallexample define adder diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp index 7ce33fdefa9..42ffd6fa0af 100644 --- a/gdb/testsuite/gdb.base/commands.exp +++ b/gdb/testsuite/gdb.base/commands.exp @@ -1125,6 +1125,41 @@ proc_with_prefix backslash_in_multi_line_command_test {} { gdb_test "print 1" "" "run command" } +proc_with_prefix args_with_whitespace {} { + gdb_test_multiple "define show_args" "define show_args" { + -re "End with" { + pass "define show_args" + } + } + + # This test should alternate between 0xdeadbeef and 0xfeedface two times. + gdb_test \ + [multi_line_input \ + {printf "nargs=%d:", $argc} \ + {set $i = 0} \ + {while $i < $argc} \ + {printf " "} \ + {eval "echo '$arg%d'", $i} \ + {set $i = $i + 1} \ + {end} \ + {printf "\n"} \ + {end}] \ + "" \ + "enter commands" + + gdb_test "show_args 1 2 3" \ + "nargs=3: '1' '2' '3'" + + gdb_test "show_args 1 (1 + 1) (1 + (1 + 1))" \ + "nargs=3: '1' '\\(1 \\+ 1\\)' '\\(1 \\+ \\(1 \\+ 1\\)\\)'" + + gdb_test "show_args ({unsigned long long} &global_var)" \ + "nargs=1: '\\({unsigned long long} &global_var\\)'" + + gdb_test "show_args (*((unsigned long long *) &global_var))" \ + "nargs=1: '\\(\\*\\(\\(unsigned long long \\*\\) &global_var\\)\\)'" +} + gdbvar_simple_if_test gdbvar_simple_while_test gdbvar_complex_if_while_test @@ -1154,5 +1189,6 @@ backslash_in_multi_line_command_test define_if_without_arg_test loop_break_test loop_continue_test +args_with_whitespace # This one should come last, as it redefines "backtrace". redefine_backtrace_test diff --git a/gdb/testsuite/gdb.base/run.c b/gdb/testsuite/gdb.base/run.c index 614b018260d..d89bad78bb4 100644 --- a/gdb/testsuite/gdb.base/run.c +++ b/gdb/testsuite/gdb.base/run.c @@ -8,6 +8,9 @@ #include "../lib/unbuffer_output.c" +/* Used by commands.exp test script. */ +volatile unsigned long long global_var = 34; + int factorial (int); int -- 2.14.4 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] gdb: Allow parenthesis to group arguments to user-defined commands 2018-08-15 14:39 ` [PATCH 2/2] " Andrew Burgess @ 2018-08-15 18:24 ` Eli Zaretskii 2018-08-25 19:32 ` Philippe Waroquiers 2018-09-06 23:29 ` [PATCHv2] gdb: Rewrite argument handling for " Andrew Burgess 2 siblings, 0 replies; 26+ messages in thread From: Eli Zaretskii @ 2018-08-15 18:24 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches > From: Andrew Burgess <andrew.burgess@embecosm.com> > Cc: Andrew Burgess <andrew.burgess@embecosm.com> > Date: Wed, 15 Aug 2018 15:39:20 +0100 > > When calling a user-defined command then currently, arguments are > whitespace separated. This means that it is impossible to pass a > single argument that contains a whitespace. > > The exception to the no whitespace rule is strings, a string argument, > enclosed in double, or single quotes, can contain whitespace. > > However, if a user wants to reference, for example, a type name that > contains a space, as in these examples: > > user_command *((unsigned long long *) some_pointer) > > user_command {unsigned long long}some_pointer > > then this will not work, as the whitespace between 'unsigned' and > 'long', as well as the whitespace between 'long' and 'long', will mean > GDB interprets this as many arguments. > > The solution proposed in this patch is to allow parenthesis to be used > to group arguments, so the use could now write: > > user_command (*((unsigned long long *) some_pointer)) > > user_command ({unsigned long long}some_pointer) > > And GDB will interpret these as a single argument. > > gdb/ChangeLog: > > * cli/cli-script.c (user_args::user_args): Allow parenthesis to > group arguments. > > gdb/testsuite/ChangeLog: > > * gdb.base/commands.exp (args_with_whitespace): New proc, which is > added to the list of procs to call. > * gdb.base/run.c (global_var): Defined global. > > gdb/doc/ChangeLog: > > * gdb.texinfo (Define): Additional documentation about argument > syntax. > --- > gdb/ChangeLog | 5 ++++ > gdb/cli/cli-script.c | 8 +++++- > gdb/doc/ChangeLog | 5 ++++ > gdb/doc/gdb.texinfo | 54 +++++++++++++++++++++++++++++++++---- > gdb/testsuite/ChangeLog | 6 +++++ > gdb/testsuite/gdb.base/commands.exp | 36 +++++++++++++++++++++++++ > gdb/testsuite/gdb.base/run.c | 3 +++ > 7 files changed, 111 insertions(+), 6 deletions(-) OK for the documentation parts. Thanks. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] gdb: Allow parenthesis to group arguments to user-defined commands 2018-08-15 14:39 ` [PATCH 2/2] " Andrew Burgess 2018-08-15 18:24 ` Eli Zaretskii @ 2018-08-25 19:32 ` Philippe Waroquiers 2018-08-25 20:53 ` Philippe Waroquiers 2018-09-06 23:29 ` [PATCHv2] gdb: Rewrite argument handling for " Andrew Burgess 2 siblings, 1 reply; 26+ messages in thread From: Philippe Waroquiers @ 2018-08-25 19:32 UTC (permalink / raw) To: Andrew Burgess, gdb-patches Woudn't it be more 'usual' (and maybe simpler) to use a \ for this ? e.g. like the shell: ls ab\ cd ls: cannot access 'ab cd': No such file or directory Philippe On Wed, 2018-08-15 at 15:39 +0100, Andrew Burgess wrote: > When calling a user-defined command then currently, arguments are > whitespace separated. This means that it is impossible to pass a > single argument that contains a whitespace. > > The exception to the no whitespace rule is strings, a string argument, > enclosed in double, or single quotes, can contain whitespace. > > However, if a user wants to reference, for example, a type name that > contains a space, as in these examples: > > user_command *((unsigned long long *) some_pointer) > > user_command {unsigned long long}some_pointer > > then this will not work, as the whitespace between 'unsigned' and > 'long', as well as the whitespace between 'long' and 'long', will mean > GDB interprets this as many arguments. > > The solution proposed in this patch is to allow parenthesis to be used > to group arguments, so the use could now write: > > user_command (*((unsigned long long *) some_pointer)) > > user_command ({unsigned long long}some_pointer) > > And GDB will interpret these as a single argument. > > gdb/ChangeLog: > > * cli/cli-script.c (user_args::user_args): Allow parenthesis to > group arguments. > > gdb/testsuite/ChangeLog: > > * gdb.base/commands.exp (args_with_whitespace): New proc, which is > added to the list of procs to call. > * gdb.base/run.c (global_var): Defined global. > > gdb/doc/ChangeLog: > > * gdb.texinfo (Define): Additional documentation about argument > syntax. > --- > gdb/ChangeLog | 5 ++++ > gdb/cli/cli-script.c | 8 +++++- > gdb/doc/ChangeLog | 5 ++++ > gdb/doc/gdb.texinfo | 54 +++++++++++++++++++++++++++++++++---- > gdb/testsuite/ChangeLog | 6 +++++ > gdb/testsuite/gdb.base/commands.exp | 36 +++++++++++++++++++++++++ > gdb/testsuite/gdb.base/run.c | 3 +++ > 7 files changed, 111 insertions(+), 6 deletions(-) > > diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c > index 6f31a400197..fd80ab9fbcf 100644 > --- a/gdb/cli/cli-script.c > +++ b/gdb/cli/cli-script.c > @@ -758,6 +758,7 @@ user_args::user_args (const char *command_line) > int squote = 0; > int dquote = 0; > int bsquote = 0; > + int pdepth = 0; > > /* Strip whitespace. */ > while (*p == ' ' || *p == '\t') > @@ -769,7 +770,8 @@ user_args::user_args (const char *command_line) > /* Get to the end of this argument. */ > while (*p) > { > - if (((*p == ' ' || *p == '\t')) && !squote && !dquote && !bsquote) > + if (((*p == ' ' || *p == '\t')) > + && !squote && !dquote && !bsquote && pdepth == 0) > break; > else > { > @@ -793,6 +795,10 @@ user_args::user_args (const char *command_line) > squote = 1; > else if (*p == '"') > dquote = 1; > + else if (*p == '(') > + pdepth++; > + else if (*p == ')') > + pdepth--; > } > p++; > } > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index 433a2698a92..11cbef97b8f 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -25219,14 +25219,58 @@ > > @noindent > This defines the command @code{adder}, which prints the sum of > -its three arguments. Note the arguments are text substitutions, so they may > -reference variables, use complex expressions, or even perform inferior > -functions calls. > +its three arguments. > + > +The arguments to user-defined commands are text substitutions, so they > +may reference variables, use complex expressions, or even perform > +inferior functions calls. Each argument is separated with whitespace, > +so in the previous example three arguments were passed. The following > +example also passes three arguments, though the arguments are more > +complex: > + > +@smallexample > +adder 10+1 10+2 10+3 > +@end smallexample > + > +@noindent > +However, if whitespace were added around the @code{+} characters, then > +9 arguments would be passed, @code{adder} only uses the first 3 of > +these arguments, and the others would be silently ignored: > + > +@smallexample > +adder 10 + 1 10 + 2 10 + 3 > +@end smallexample > + > +@noindent > +Parenthesis can be uses to group complex expressions that include > +whitespace into a single argument, so the previous example can be > +modified to pass just 3 arguments again, like this: > + > +@smallexample > +adder (10 + 1) (10 + 2) (10 + 3) > +@end smallexample > + > +@noindent > +The parenthesis are passed through as part of the argument, so the > +previous example causes @value{GDBN} to evaluate: > + > +@smallexample > +print (10 + 1) + (10 + 2) + (10 + 3) > +@end smallexample > + > +@noindent > +Nested parenthesis are also allowed within an argument, in the > +following example 3 arguments are still passed: > + > +@smallexample > +adder (10 + 1) (10 + (1 + 1)) (10 + (1 + (1 + 1))) > +@end smallexample > > @cindex argument count in user-defined commands > @cindex how many arguments (user-defined commands) > -In addition, @code{$argc} may be used to find out how many arguments have > -been passed. > +@noindent > +Within a user-defined command @code{$argc} may be used to find out how > +many arguments have been passed. > > @smallexample > define adder > diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp > index 7ce33fdefa9..42ffd6fa0af 100644 > --- a/gdb/testsuite/gdb.base/commands.exp > +++ b/gdb/testsuite/gdb.base/commands.exp > @@ -1125,6 +1125,41 @@ proc_with_prefix backslash_in_multi_line_command_test {} { > gdb_test "print 1" "" "run command" > } > > +proc_with_prefix args_with_whitespace {} { > + gdb_test_multiple "define show_args" "define show_args" { > + -re "End with" { > + pass "define show_args" > + } > + } > + > + # This test should alternate between 0xdeadbeef and 0xfeedface two times. > + gdb_test \ > + [multi_line_input \ > + {printf "nargs=%d:", $argc} \ > + {set $i = 0} \ > + {while $i < $argc} \ > + {printf " "} \ > + {eval "echo '$arg%d'", $i} \ > + {set $i = $i + 1} \ > + {end} \ > + {printf "\n"} \ > + {end}] \ > + "" \ > + "enter commands" > + > + gdb_test "show_args 1 2 3" \ > + "nargs=3: '1' '2' '3'" > + > + gdb_test "show_args 1 (1 + 1) (1 + (1 + 1))" \ > + "nargs=3: '1' '\\(1 \\+ 1\\)' '\\(1 \\+ \\(1 \\+ 1\\)\\)'" > + > + gdb_test "show_args ({unsigned long long} &global_var)" \ > + "nargs=1: '\\({unsigned long long} &global_var\\)'" > + > + gdb_test "show_args (*((unsigned long long *) &global_var))" \ > + "nargs=1: '\\(\\*\\(\\(unsigned long long \\*\\) &global_var\\)\\)'" > +} > + > gdbvar_simple_if_test > gdbvar_simple_while_test > gdbvar_complex_if_while_test > @@ -1154,5 +1189,6 @@ backslash_in_multi_line_command_test > define_if_without_arg_test > loop_break_test > loop_continue_test > +args_with_whitespace > # This one should come last, as it redefines "backtrace". > redefine_backtrace_test > diff --git a/gdb/testsuite/gdb.base/run.c b/gdb/testsuite/gdb.base/run.c > index 614b018260d..d89bad78bb4 100644 > --- a/gdb/testsuite/gdb.base/run.c > +++ b/gdb/testsuite/gdb.base/run.c > @@ -8,6 +8,9 @@ > > #include "../lib/unbuffer_output.c" > > +/* Used by commands.exp test script. */ > +volatile unsigned long long global_var = 34; > + > int factorial (int); > > int ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] gdb: Allow parenthesis to group arguments to user-defined commands 2018-08-25 19:32 ` Philippe Waroquiers @ 2018-08-25 20:53 ` Philippe Waroquiers 2018-08-25 22:43 ` Andrew Burgess 0 siblings, 1 reply; 26+ messages in thread From: Philippe Waroquiers @ 2018-08-25 20:53 UTC (permalink / raw) To: Andrew Burgess, gdb-patches On Sat, 2018-08-25 at 21:32 +0200, Philippe Waroquiers wrote: > Woudn't it be more 'usual' (and maybe simpler) to use a \ for this ? > e.g. like the shell: > ls ab\ cd > ls: cannot access 'ab cd': No such file or directory > > Philippe See also the RFA (in 'review needed status') https://sourceware.org/ml/gdb-patches/2018-07/msg00131.html where I (inconsistently with my comment above but still consistent with the shell) have used single quotes for such purposes. Like for the level versus number, we should aim at a single approach in gdb for such things ... Philippe > > > On Wed, 2018-08-15 at 15:39 +0100, Andrew Burgess wrote: > > When calling a user-defined command then currently, arguments are > > whitespace separated. This means that it is impossible to pass a > > single argument that contains a whitespace. > > > > The exception to the no whitespace rule is strings, a string argument, > > enclosed in double, or single quotes, can contain whitespace. > > > > However, if a user wants to reference, for example, a type name that > > contains a space, as in these examples: > > > > user_command *((unsigned long long *) some_pointer) > > > > user_command {unsigned long long}some_pointer > > > > then this will not work, as the whitespace between 'unsigned' and > > 'long', as well as the whitespace between 'long' and 'long', will mean > > GDB interprets this as many arguments. > > > > The solution proposed in this patch is to allow parenthesis to be used > > to group arguments, so the use could now write: > > > > user_command (*((unsigned long long *) some_pointer)) > > > > user_command ({unsigned long long}some_pointer) > > > > And GDB will interpret these as a single argument. > > > > gdb/ChangeLog: > > > > * cli/cli-script.c (user_args::user_args): Allow parenthesis to > > group arguments. > > > > gdb/testsuite/ChangeLog: > > > > * gdb.base/commands.exp (args_with_whitespace): New proc, which is > > added to the list of procs to call. > > * gdb.base/run.c (global_var): Defined global. > > > > gdb/doc/ChangeLog: > > > > * gdb.texinfo (Define): Additional documentation about argument > > syntax. > > --- > > gdb/ChangeLog | 5 ++++ > > gdb/cli/cli-script.c | 8 +++++- > > gdb/doc/ChangeLog | 5 ++++ > > gdb/doc/gdb.texinfo | 54 +++++++++++++++++++++++++++++++++---- > > gdb/testsuite/ChangeLog | 6 +++++ > > gdb/testsuite/gdb.base/commands.exp | 36 +++++++++++++++++++++++++ > > gdb/testsuite/gdb.base/run.c | 3 +++ > > 7 files changed, 111 insertions(+), 6 deletions(-) > > > > diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c > > index 6f31a400197..fd80ab9fbcf 100644 > > --- a/gdb/cli/cli-script.c > > +++ b/gdb/cli/cli-script.c > > @@ -758,6 +758,7 @@ user_args::user_args (const char *command_line) > > int squote = 0; > > int dquote = 0; > > int bsquote = 0; > > + int pdepth = 0; > > > > /* Strip whitespace. */ > > while (*p == ' ' || *p == '\t') > > @@ -769,7 +770,8 @@ user_args::user_args (const char *command_line) > > /* Get to the end of this argument. */ > > while (*p) > > { > > - if (((*p == ' ' || *p == '\t')) && !squote && !dquote && !bsquote) > > + if (((*p == ' ' || *p == '\t')) > > + && !squote && !dquote && !bsquote && pdepth == 0) > > break; > > else > > { > > @@ -793,6 +795,10 @@ user_args::user_args (const char *command_line) > > squote = 1; > > else if (*p == '"') > > dquote = 1; > > + else if (*p == '(') > > + pdepth++; > > + else if (*p == ')') > > + pdepth--; > > } > > p++; > > } > > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > > index 433a2698a92..11cbef97b8f 100644 > > --- a/gdb/doc/gdb.texinfo > > +++ b/gdb/doc/gdb.texinfo > > @@ -25219,14 +25219,58 @@ > > > > @noindent > > This defines the command @code{adder}, which prints the sum of > > -its three arguments. Note the arguments are text substitutions, so they may > > -reference variables, use complex expressions, or even perform inferior > > -functions calls. > > +its three arguments. > > + > > +The arguments to user-defined commands are text substitutions, so they > > +may reference variables, use complex expressions, or even perform > > +inferior functions calls. Each argument is separated with whitespace, > > +so in the previous example three arguments were passed. The following > > +example also passes three arguments, though the arguments are more > > +complex: > > + > > +@smallexample > > +adder 10+1 10+2 10+3 > > +@end smallexample > > + > > +@noindent > > +However, if whitespace were added around the @code{+} characters, then > > +9 arguments would be passed, @code{adder} only uses the first 3 of > > +these arguments, and the others would be silently ignored: > > + > > +@smallexample > > +adder 10 + 1 10 + 2 10 + 3 > > +@end smallexample > > + > > +@noindent > > +Parenthesis can be uses to group complex expressions that include > > +whitespace into a single argument, so the previous example can be > > +modified to pass just 3 arguments again, like this: > > + > > +@smallexample > > +adder (10 + 1) (10 + 2) (10 + 3) > > +@end smallexample > > + > > +@noindent > > +The parenthesis are passed through as part of the argument, so the > > +previous example causes @value{GDBN} to evaluate: > > + > > +@smallexample > > +print (10 + 1) + (10 + 2) + (10 + 3) > > +@end smallexample > > + > > +@noindent > > +Nested parenthesis are also allowed within an argument, in the > > +following example 3 arguments are still passed: > > + > > +@smallexample > > +adder (10 + 1) (10 + (1 + 1)) (10 + (1 + (1 + 1))) > > +@end smallexample > > > > @cindex argument count in user-defined commands > > @cindex how many arguments (user-defined commands) > > -In addition, @code{$argc} may be used to find out how many arguments have > > -been passed. > > +@noindent > > +Within a user-defined command @code{$argc} may be used to find out how > > +many arguments have been passed. > > > > @smallexample > > define adder > > diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp > > index 7ce33fdefa9..42ffd6fa0af 100644 > > --- a/gdb/testsuite/gdb.base/commands.exp > > +++ b/gdb/testsuite/gdb.base/commands.exp > > @@ -1125,6 +1125,41 @@ proc_with_prefix backslash_in_multi_line_command_test {} { > > gdb_test "print 1" "" "run command" > > } > > > > +proc_with_prefix args_with_whitespace {} { > > + gdb_test_multiple "define show_args" "define show_args" { > > + -re "End with" { > > + pass "define show_args" > > + } > > + } > > + > > + # This test should alternate between 0xdeadbeef and 0xfeedface two times. > > + gdb_test \ > > + [multi_line_input \ > > + {printf "nargs=%d:", $argc} \ > > + {set $i = 0} \ > > + {while $i < $argc} \ > > + {printf " "} \ > > + {eval "echo '$arg%d'", $i} \ > > + {set $i = $i + 1} \ > > + {end} \ > > + {printf "\n"} \ > > + {end}] \ > > + "" \ > > + "enter commands" > > + > > + gdb_test "show_args 1 2 3" \ > > + "nargs=3: '1' '2' '3'" > > + > > + gdb_test "show_args 1 (1 + 1) (1 + (1 + 1))" \ > > + "nargs=3: '1' '\\(1 \\+ 1\\)' '\\(1 \\+ \\(1 \\+ 1\\)\\)'" > > + > > + gdb_test "show_args ({unsigned long long} &global_var)" \ > > + "nargs=1: '\\({unsigned long long} &global_var\\)'" > > + > > + gdb_test "show_args (*((unsigned long long *) &global_var))" \ > > + "nargs=1: '\\(\\*\\(\\(unsigned long long \\*\\) &global_var\\)\\)'" > > +} > > + > > gdbvar_simple_if_test > > gdbvar_simple_while_test > > gdbvar_complex_if_while_test > > @@ -1154,5 +1189,6 @@ backslash_in_multi_line_command_test > > define_if_without_arg_test > > loop_break_test > > loop_continue_test > > +args_with_whitespace > > # This one should come last, as it redefines "backtrace". > > redefine_backtrace_test > > diff --git a/gdb/testsuite/gdb.base/run.c b/gdb/testsuite/gdb.base/run.c > > index 614b018260d..d89bad78bb4 100644 > > --- a/gdb/testsuite/gdb.base/run.c > > +++ b/gdb/testsuite/gdb.base/run.c > > @@ -8,6 +8,9 @@ > > > > #include "../lib/unbuffer_output.c" > > > > +/* Used by commands.exp test script. */ > > +volatile unsigned long long global_var = 34; > > + > > int factorial (int); > > > > int ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] gdb: Allow parenthesis to group arguments to user-defined commands 2018-08-25 20:53 ` Philippe Waroquiers @ 2018-08-25 22:43 ` Andrew Burgess 2018-08-28 15:54 ` Tom Tromey 0 siblings, 1 reply; 26+ messages in thread From: Andrew Burgess @ 2018-08-25 22:43 UTC (permalink / raw) To: Philippe Waroquiers; +Cc: gdb-patches * Philippe Waroquiers <philippe.waroquiers@skynet.be> [2018-08-25 22:53:23 +0200]: > On Sat, 2018-08-25 at 21:32 +0200, Philippe Waroquiers wrote: > > Woudn't it be more 'usual' (and maybe simpler) to use a \ for this ? > > e.g. like the shell: > > ls ab\ cd > > ls: cannot access 'ab cd': No such file or directory > > > > Philippe > See also the RFA (in 'review needed status') > https://sourceware.org/ml/gdb-patches/2018-07/msg00131.html > where I (inconsistently with my comment above but still consistent with the > shell) have used single quotes for such purposes. > > Like for the level versus number, we should aim at a single approach > in gdb for such things ... The problem here is that user defined commands already have (IMHO) weird handling of single/double quotes. So, my_command 'ab cd' will replace $arg0 with literally, 'ac cd', including the quotes. The same for double quotes. Next, there is some strange handling of backslashes already in place, so, my_command ac\ cd Will replace $arg0 with literally, ac\ cd, including the backslash. Honestly, if there is a logic behind the argument processing, I'm not seeing it, and the docs don't explain it. It feels like an attempt was made to do something like you suggest, except it all went horribly wrong... However, as far as I can tell, the code has been this way for years, a quick look back to revision d318976c46b92e4d in year 2000, showed the same argument processing code unchanged, so my concern is, if we change things too much now we might break existing scripts.... So, my suggestion deliberately avoids using quotes or backslashes as these are bogged down in the existing code. And using (...) is fairly intuitive given GDBs C like expression handling, personally I'd rather write: my_command ({unsigned long long} &global_var) than: my_command {unsigned\ long\ long}\ &global_var though, I think you also suggested quotes as a possibility, so this isn't so bad: my_command '{unsigned long long} &global_var' but the C like expression does feel more natural to me. But I could live with the quotes to get the functionality I need in. Additionally, using (...) for grouping didn't feel like it was likely to break much existing code, I convinced myself that it was unlikely that someone would write: my_command (ab cd) and actually want '(ab' and 'cd)' to be separate arguments... The only saving grace here is that the argument processing for user defined commands is completely separate, and not used anywhere else. We could make the choice that this is just a legacy corner of GDB that has its own rules. In summary I think that the choices are: 1. Break backward compatibility for use of quote and backslashes, and rework argument processing for user defined commands. 2. Stick with (...) 3. Define a new scheme that allows most backward compatibility to be maintained, but extends use backslash and quote to allow for argument grouping. Happy to hear more thoughts on this topic, Thanks, Andrew > > Philippe > > > > > > > On Wed, 2018-08-15 at 15:39 +0100, Andrew Burgess wrote: > > > When calling a user-defined command then currently, arguments are > > > whitespace separated. This means that it is impossible to pass a > > > single argument that contains a whitespace. > > > > > > The exception to the no whitespace rule is strings, a string argument, > > > enclosed in double, or single quotes, can contain whitespace. > > > > > > However, if a user wants to reference, for example, a type name that > > > contains a space, as in these examples: > > > > > > user_command *((unsigned long long *) some_pointer) > > > > > > user_command {unsigned long long}some_pointer > > > > > > then this will not work, as the whitespace between 'unsigned' and > > > 'long', as well as the whitespace between 'long' and 'long', will mean > > > GDB interprets this as many arguments. > > > > > > The solution proposed in this patch is to allow parenthesis to be used > > > to group arguments, so the use could now write: > > > > > > user_command (*((unsigned long long *) some_pointer)) > > > > > > user_command ({unsigned long long}some_pointer) > > > > > > And GDB will interpret these as a single argument. > > > > > > gdb/ChangeLog: > > > > > > * cli/cli-script.c (user_args::user_args): Allow parenthesis to > > > group arguments. > > > > > > gdb/testsuite/ChangeLog: > > > > > > * gdb.base/commands.exp (args_with_whitespace): New proc, which is > > > added to the list of procs to call. > > > * gdb.base/run.c (global_var): Defined global. > > > > > > gdb/doc/ChangeLog: > > > > > > * gdb.texinfo (Define): Additional documentation about argument > > > syntax. > > > --- > > > gdb/ChangeLog | 5 ++++ > > > gdb/cli/cli-script.c | 8 +++++- > > > gdb/doc/ChangeLog | 5 ++++ > > > gdb/doc/gdb.texinfo | 54 +++++++++++++++++++++++++++++++++---- > > > gdb/testsuite/ChangeLog | 6 +++++ > > > gdb/testsuite/gdb.base/commands.exp | 36 +++++++++++++++++++++++++ > > > gdb/testsuite/gdb.base/run.c | 3 +++ > > > 7 files changed, 111 insertions(+), 6 deletions(-) > > > > > > diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c > > > index 6f31a400197..fd80ab9fbcf 100644 > > > --- a/gdb/cli/cli-script.c > > > +++ b/gdb/cli/cli-script.c > > > @@ -758,6 +758,7 @@ user_args::user_args (const char *command_line) > > > int squote = 0; > > > int dquote = 0; > > > int bsquote = 0; > > > + int pdepth = 0; > > > > > > /* Strip whitespace. */ > > > while (*p == ' ' || *p == '\t') > > > @@ -769,7 +770,8 @@ user_args::user_args (const char *command_line) > > > /* Get to the end of this argument. */ > > > while (*p) > > > { > > > - if (((*p == ' ' || *p == '\t')) && !squote && !dquote && !bsquote) > > > + if (((*p == ' ' || *p == '\t')) > > > + && !squote && !dquote && !bsquote && pdepth == 0) > > > break; > > > else > > > { > > > @@ -793,6 +795,10 @@ user_args::user_args (const char *command_line) > > > squote = 1; > > > else if (*p == '"') > > > dquote = 1; > > > + else if (*p == '(') > > > + pdepth++; > > > + else if (*p == ')') > > > + pdepth--; > > > } > > > p++; > > > } > > > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > > > index 433a2698a92..11cbef97b8f 100644 > > > --- a/gdb/doc/gdb.texinfo > > > +++ b/gdb/doc/gdb.texinfo > > > @@ -25219,14 +25219,58 @@ > > > > > > @noindent > > > This defines the command @code{adder}, which prints the sum of > > > -its three arguments. Note the arguments are text substitutions, so they may > > > -reference variables, use complex expressions, or even perform inferior > > > -functions calls. > > > +its three arguments. > > > + > > > +The arguments to user-defined commands are text substitutions, so they > > > +may reference variables, use complex expressions, or even perform > > > +inferior functions calls. Each argument is separated with whitespace, > > > +so in the previous example three arguments were passed. The following > > > +example also passes three arguments, though the arguments are more > > > +complex: > > > + > > > +@smallexample > > > +adder 10+1 10+2 10+3 > > > +@end smallexample > > > + > > > +@noindent > > > +However, if whitespace were added around the @code{+} characters, then > > > +9 arguments would be passed, @code{adder} only uses the first 3 of > > > +these arguments, and the others would be silently ignored: > > > + > > > +@smallexample > > > +adder 10 + 1 10 + 2 10 + 3 > > > +@end smallexample > > > + > > > +@noindent > > > +Parenthesis can be uses to group complex expressions that include > > > +whitespace into a single argument, so the previous example can be > > > +modified to pass just 3 arguments again, like this: > > > + > > > +@smallexample > > > +adder (10 + 1) (10 + 2) (10 + 3) > > > +@end smallexample > > > + > > > +@noindent > > > +The parenthesis are passed through as part of the argument, so the > > > +previous example causes @value{GDBN} to evaluate: > > > + > > > +@smallexample > > > +print (10 + 1) + (10 + 2) + (10 + 3) > > > +@end smallexample > > > + > > > +@noindent > > > +Nested parenthesis are also allowed within an argument, in the > > > +following example 3 arguments are still passed: > > > + > > > +@smallexample > > > +adder (10 + 1) (10 + (1 + 1)) (10 + (1 + (1 + 1))) > > > +@end smallexample > > > > > > @cindex argument count in user-defined commands > > > @cindex how many arguments (user-defined commands) > > > -In addition, @code{$argc} may be used to find out how many arguments have > > > -been passed. > > > +@noindent > > > +Within a user-defined command @code{$argc} may be used to find out how > > > +many arguments have been passed. > > > > > > @smallexample > > > define adder > > > diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp > > > index 7ce33fdefa9..42ffd6fa0af 100644 > > > --- a/gdb/testsuite/gdb.base/commands.exp > > > +++ b/gdb/testsuite/gdb.base/commands.exp > > > @@ -1125,6 +1125,41 @@ proc_with_prefix backslash_in_multi_line_command_test {} { > > > gdb_test "print 1" "" "run command" > > > } > > > > > > +proc_with_prefix args_with_whitespace {} { > > > + gdb_test_multiple "define show_args" "define show_args" { > > > + -re "End with" { > > > + pass "define show_args" > > > + } > > > + } > > > + > > > + # This test should alternate between 0xdeadbeef and 0xfeedface two times. > > > + gdb_test \ > > > + [multi_line_input \ > > > + {printf "nargs=%d:", $argc} \ > > > + {set $i = 0} \ > > > + {while $i < $argc} \ > > > + {printf " "} \ > > > + {eval "echo '$arg%d'", $i} \ > > > + {set $i = $i + 1} \ > > > + {end} \ > > > + {printf "\n"} \ > > > + {end}] \ > > > + "" \ > > > + "enter commands" > > > + > > > + gdb_test "show_args 1 2 3" \ > > > + "nargs=3: '1' '2' '3'" > > > + > > > + gdb_test "show_args 1 (1 + 1) (1 + (1 + 1))" \ > > > + "nargs=3: '1' '\\(1 \\+ 1\\)' '\\(1 \\+ \\(1 \\+ 1\\)\\)'" > > > + > > > + gdb_test "show_args ({unsigned long long} &global_var)" \ > > > + "nargs=1: '\\({unsigned long long} &global_var\\)'" > > > + > > > + gdb_test "show_args (*((unsigned long long *) &global_var))" \ > > > + "nargs=1: '\\(\\*\\(\\(unsigned long long \\*\\) &global_var\\)\\)'" > > > +} > > > + > > > gdbvar_simple_if_test > > > gdbvar_simple_while_test > > > gdbvar_complex_if_while_test > > > @@ -1154,5 +1189,6 @@ backslash_in_multi_line_command_test > > > define_if_without_arg_test > > > loop_break_test > > > loop_continue_test > > > +args_with_whitespace > > > # This one should come last, as it redefines "backtrace". > > > redefine_backtrace_test > > > diff --git a/gdb/testsuite/gdb.base/run.c b/gdb/testsuite/gdb.base/run.c > > > index 614b018260d..d89bad78bb4 100644 > > > --- a/gdb/testsuite/gdb.base/run.c > > > +++ b/gdb/testsuite/gdb.base/run.c > > > @@ -8,6 +8,9 @@ > > > > > > #include "../lib/unbuffer_output.c" > > > > > > +/* Used by commands.exp test script. */ > > > +volatile unsigned long long global_var = 34; > > > + > > > int factorial (int); > > > > > > int ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] gdb: Allow parenthesis to group arguments to user-defined commands 2018-08-25 22:43 ` Andrew Burgess @ 2018-08-28 15:54 ` Tom Tromey 2018-08-28 18:43 ` Andrew Burgess 0 siblings, 1 reply; 26+ messages in thread From: Tom Tromey @ 2018-08-28 15:54 UTC (permalink / raw) To: Andrew Burgess; +Cc: Philippe Waroquiers, gdb-patches >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes: Andrew> So, my suggestion deliberately avoids using quotes or backslashes as Andrew> these are bogged down in the existing code. And using (...) is fairly Andrew> intuitive given GDBs C like expression handling, personally I'd rather Andrew> write: Andrew> my_command ({unsigned long long} &global_var) Andrew> than: Andrew> my_command {unsigned\ long\ long}\ &global_var FWIW I tend to agree with your logic here. User-defined argument parsing is broken (and I think there's at least one bug in bugzilla about this), but at the same time, making breaking changes seems unfriendly. Your approach doesn't seem to be breaking anything that is likely to be actually used. Tom ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] gdb: Allow parenthesis to group arguments to user-defined commands 2018-08-28 15:54 ` Tom Tromey @ 2018-08-28 18:43 ` Andrew Burgess 2018-08-28 20:29 ` Philippe Waroquiers 0 siblings, 1 reply; 26+ messages in thread From: Andrew Burgess @ 2018-08-28 18:43 UTC (permalink / raw) To: Tom Tromey; +Cc: Philippe Waroquiers, gdb-patches * Tom Tromey <tom@tromey.com> [2018-08-28 09:53:52 -0600]: > >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes: > > Andrew> So, my suggestion deliberately avoids using quotes or backslashes as > Andrew> these are bogged down in the existing code. And using (...) is fairly > Andrew> intuitive given GDBs C like expression handling, personally I'd rather > Andrew> write: > Andrew> my_command ({unsigned long long} &global_var) > Andrew> than: > Andrew> my_command {unsigned\ long\ long}\ &global_var > > FWIW I tend to agree with your logic here. > > User-defined argument parsing is broken (and I think there's at least > one bug in bugzilla about this), but at the same time, making breaking > changes seems unfriendly. Your approach doesn't seem to be breaking > anything that is likely to be actually used. Given that the argument passing for user-defined functions is pretty self contained we could, conceivably, implement a whole new system and have a switch to select between them... the existing code does seem rather odd. But ideally, I'd like that to be a project for another day. Thanks, Andrew ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] gdb: Allow parenthesis to group arguments to user-defined commands 2018-08-28 18:43 ` Andrew Burgess @ 2018-08-28 20:29 ` Philippe Waroquiers 2018-08-28 23:29 ` Andrew Burgess 2018-08-30 2:26 ` Tom Tromey 0 siblings, 2 replies; 26+ messages in thread From: Philippe Waroquiers @ 2018-08-28 20:29 UTC (permalink / raw) To: Andrew Burgess, Tom Tromey; +Cc: gdb-patches On Tue, 2018-08-28 at 19:43 +0100, Andrew Burgess wrote: > * Tom Tromey <tom@tromey.com> [2018-08-28 09:53:52 -0600]: > > > > > > > > "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes: > > > > Andrew> So, my suggestion deliberately avoids using quotes or backslashes as > > Andrew> these are bogged down in the existing code. And using (...) is fairly > > Andrew> intuitive given GDBs C like expression handling, personally I'd rather > > Andrew> write: > > Andrew> my_command ({unsigned long long} &global_var) > > Andrew> than: > > Andrew> my_command {unsigned\ long\ long}\ &global_var > > > > FWIW I tend to agree with your logic here. > > > > User-defined argument parsing is broken (and I think there's at least > > one bug in bugzilla about this), but at the same time, making breaking > > changes seems unfriendly. Your approach doesn't seem to be breaking > > anything that is likely to be actually used. I do not think a \ followed by a space will create a lot of incompatibilities. i .e. that someone would type some_user_defined_command a\ b to give 2 different args to the command. For single quotes: it is unlikely that someone types something like some_user_defined_command 'abc def' and was expecting the user defined command to receive two args. What is however a lot more likely is some_user_defined_command 'a' and this command expects to receive a character value. So, yes, single quotes has more potential to create incompatibilities. On the downside, quoting with parenthesis is something very peculiar (is there some other tool/shell/... using this convention ?). And from a previous discussion with Pedro, he indicated that some commands in gdb already are using single quotes. The 'Not sure' below is a quote of Pedro :), replying to a suggestion I gave to let the user tell explicitly if an arg was quoted or not. >   * 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. It is based on this that I used single quotes in the info var/func/arg/local -t TYPEREGEXP NAMEREGEXP patch. > Given that the argument passing for user-defined functions is pretty > self contained we could, conceivably, implement a whole new system and > have a switch to select between them... the existing code does seem > rather odd. > > But ideally, I'd like that to be a project for another day. And we also have the 'generalised arg parser' prototype that Pedro has in a corner. IMO, clearly, what to do is unclear :). Philippe ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] gdb: Allow parenthesis to group arguments to user-defined commands 2018-08-28 20:29 ` Philippe Waroquiers @ 2018-08-28 23:29 ` Andrew Burgess 2018-08-30 2:19 ` Tom Tromey 2018-08-30 21:19 ` Philippe Waroquiers 2018-08-30 2:26 ` Tom Tromey 1 sibling, 2 replies; 26+ messages in thread From: Andrew Burgess @ 2018-08-28 23:29 UTC (permalink / raw) To: Philippe Waroquiers; +Cc: Tom Tromey, gdb-patches * Philippe Waroquiers <philippe.waroquiers@skynet.be> [2018-08-28 22:28:57 +0200]: > On Tue, 2018-08-28 at 19:43 +0100, Andrew Burgess wrote: > > * Tom Tromey <tom@tromey.com> [2018-08-28 09:53:52 -0600]: > > > > > > > > > > "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes: > > > > > > Andrew> So, my suggestion deliberately avoids using quotes or backslashes as > > > Andrew> these are bogged down in the existing code. And using (...) is fairly > > > Andrew> intuitive given GDBs C like expression handling, personally I'd rather > > > Andrew> write: > > > Andrew> my_command ({unsigned long long} &global_var) > > > Andrew> than: > > > Andrew> my_command {unsigned\ long\ long}\ &global_var > > > > > > FWIW I tend to agree with your logic here. > > > > > > User-defined argument parsing is broken (and I think there's at least > > > one bug in bugzilla about this), but at the same time, making breaking > > > changes seems unfriendly. Your approach doesn't seem to be breaking > > > anything that is likely to be actually used. > > I do not think a \ followed by a space will create a lot of incompatibilities. > i > .e. that someone would type > some_user_defined_command a\ b > to give 2 > different args to the command. No, and what they get at the moment is a single argument, which is: a\ b including the backslash and the space. Is this correct, or useful? I suspect not, but my instinct in these cases is not to mess with things without good reason, and I didn't have a good reason. > > For single quotes: it is unlikely that someone types something like > some_user_defined_command 'abc def' > and was expecting the user defined command to receive two args. I agree, and what they get right now is: 'abc def' including the quotes. > What is however a lot more likely is > some_user_defined_command 'a' > and this command expects to receive a character value. Which is more or less what you get: 'a' > So, yes, single quotes has more potential to create > incompatibilities. I guess that depends on what you're proposing the argument should evaluate too. > > On the downside, quoting with parenthesis is something very peculiar > (is there some other tool/shell/... using this convention ?). I guess I never saw it as quoting, just grouping an expression together. I think I see GDB as mostly consuming C like expressions, which is why using () seemed natural enough. Coincidentally the users I'm working with initially tried () too (without prompting from me) so my focus group of 2 all agreed with me ;-) > > And from a previous discussion with Pedro, he indicated that > some commands in gdb already are using single quotes. > The 'Not sure' below is a quote of Pedro :), replying to a suggestion > I gave to let the user tell explicitly if an arg was quoted or not. > > * 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. > > It is based on this that I used single quotes in the > info var/func/arg/local -t TYPEREGEXP NAMEREGEXP > patch. > > > Given that the argument passing for user-defined functions is pretty > > self contained we could, conceivably, implement a whole new system and > > have a switch to select between them... the existing code does seem > > rather odd. > > > > But ideally, I'd like that to be a project for another day. > And we also have the 'generalised arg parser' prototype that Pedro has > in a corner. > > IMO, clearly, what to do is unclear :). I agree, except I need to figure out some clarity in order to resolve the real issue I have :) I think that you're arguing in favour of reworking the argument passing to user-defined commands. I'm not entirely sure exactly what rules you're proposing we adopt though. Maybe you could help me work through a few examples, in the following table I've outlined a few examples, and have the following questions: [1] What argument(s) would be passed under the new scheme? [2] What argument(s) would be passed under the new scheme? [3] How would I write {unsigned long} &var to pass this as a single argument? If with quotes, how does this relate to 1 & 2? | Input | Current Arguments(s) | Proposed Argument(s) | |----------------------+----------------------+----------------------| | a b | a | a | | | b | b | |----------------------+----------------------+----------------------| | "a b" | "a b" | [1] | |----------------------+----------------------+----------------------| | 'a b' | 'a b' | [2] | |----------------------+----------------------+----------------------| | {unsigned long} &var | {unsigned | | | | long} | N/A | | | &var | | |----------------------+----------------------+----------------------| | [3] | | | |----------------------+----------------------+----------------------| In general I'm happy to rework this part of GDB, but ideally I'd like some feedback from a global maintainer that such a change, which might would break backward compatibility, would be acceptable... Thanks, Andrew ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] gdb: Allow parenthesis to group arguments to user-defined commands 2018-08-28 23:29 ` Andrew Burgess @ 2018-08-30 2:19 ` Tom Tromey 2018-08-30 21:19 ` Philippe Waroquiers 1 sibling, 0 replies; 26+ messages in thread From: Tom Tromey @ 2018-08-30 2:19 UTC (permalink / raw) To: Andrew Burgess; +Cc: Philippe Waroquiers, Tom Tromey, gdb-patches >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes: Andrew> In general I'm happy to rework this part of GDB, but ideally I'd like Andrew> some feedback from a global maintainer that such a change, which might Andrew> would break backward compatibility, would be acceptable... My philosophy has generally been to avoid incompatibility when possible and when there seems to be some likelihood that the behavior is actually used. So, for example, I felt ok changing the parsing of "bt" arguments, because I felt the implementation was simply buggy and that nobody would be using its weird corner cases. On the other hand, back in the day I changed the parsing of "disassemble" and heard complaints for quite some time. I guess this informed my view. Sometimes this leads to bad results, like how users should nearly always use "watch -location", but it isn't the default. This is why I think the paren grouping idea is fine: the examples I can think of all seem useless. That said, upthread you mentioned the idea of passing a flag to "define" to change how arguments are parsed. I would be ok with this as well. However I think that in this case it would be good to work out the details beforehand, including looking through bugzilla to see if there are other unmet needs. Tom ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] gdb: Allow parenthesis to group arguments to user-defined commands 2018-08-28 23:29 ` Andrew Burgess 2018-08-30 2:19 ` Tom Tromey @ 2018-08-30 21:19 ` Philippe Waroquiers 2018-08-31 20:59 ` Tom Tromey 1 sibling, 1 reply; 26+ messages in thread From: Philippe Waroquiers @ 2018-08-30 21:19 UTC (permalink / raw) To: Andrew Burgess; +Cc: Tom Tromey, gdb-patches On Wed, 2018-08-29 at 00:29 +0100, Andrew Burgess wrote: > > Maybe you could help me work through a few examples, in the following > table I've outlined a few examples, and have the following questions: > > [1] What argument(s) would be passed under the new scheme? > [2] What argument(s) would be passed under the new scheme? > [3] How would I write {unsigned long} &var to pass this as a single > argument? If with quotes, how does this relate to 1 & 2? > > | Input | Current Arguments(s) | Proposed Argument(s) | > |----------------------+----------------------+----------------------| > | a b | a | a | > | | b | b | > |----------------------+----------------------+----------------------| > | "a b" | "a b" | [1] | > |----------------------+----------------------+----------------------| > | 'a b' | 'a b' | [2] | > |----------------------+----------------------+----------------------| > | {unsigned long} &var | {unsigned | | > | | long} | N/A | > | | &var | | > |----------------------+----------------------+----------------------| > | [3] | | | > |----------------------+----------------------+----------------------| > > In general I'm happy to rework this part of GDB, but ideally I'd like > some feedback from a global maintainer that such a change, which might > would break backward compatibility, would be acceptable... Humph, now I understand that user defined commands already handle specially single and double quotes, as you explained above. This seems not documented in the gdb documentation, which tells that 'Note the arguments are text substitutions, so they may reference variables, use complex expressions, or even perform inferior functions calls.' I have not found a description of this single and double quote behaviour. Maybe the only thing we might need is something like: $unquoted_arg1 or something like that to allow to remove the quotes inside the user defined command, when desired ? Or as you suggested, some new flags to the define command, such as define some_command -unquote_arg3 would indicate that if arg3 is a single quoted argument as in your table above, that it must be given without the quote to some_command. For what concerns the parenthesis based solution, it looks to not work for some cases. E.g. if I want to pass one argument (I am using single quotes to show the args I want in the below examples): 'a b) (c d' then I need to use: some_user_defined (a b) (c d) but that is the same notation as if I want to pass 2 arguments 'a b' and 'c d' So, the parenthesis mechanism would need escaping or something like that, to allow a general grouping. As it stands now, it only works based on the assumption that the content inside the parenthesis Still, it looks strange to me that we introduce another mechanism on top of the single/double quotes to quote. And it would be nice if the mechanism used to quote args would be compatible between user defined commands and native gdb commands. I have not understood the reference given by Tom that expressions are terminated with , and that parenthesis stops this termination. Is that described somewhere in the doc ? The doc (or an example if this is not documented) will help me to understand. Philippe ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] gdb: Allow parenthesis to group arguments to user-defined commands 2018-08-30 21:19 ` Philippe Waroquiers @ 2018-08-31 20:59 ` Tom Tromey 2018-09-01 11:10 ` Philippe Waroquiers 0 siblings, 1 reply; 26+ messages in thread From: Tom Tromey @ 2018-08-31 20:59 UTC (permalink / raw) To: Philippe Waroquiers; +Cc: Andrew Burgess, Tom Tromey, gdb-patches >>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes: Philippe> For what concerns the parenthesis based solution, it looks to not work Philippe> for some cases. Philippe> E.g. if I want to pass one argument (I am using single quotes to show the args Philippe> I want in the below examples): Philippe> 'a b) (c d' Philippe> then I need to use: Philippe> some_user_defined (a b) (c d) Philippe> but that is the same notation as if I want to pass 2 arguments Philippe> 'a b' and 'c d' Can you think of an actual example where this would be useful? My feeling is that there isn't one, though I'd be interested in tricky-but-plausible counter-examples. This feeling is why I'm generally ok with Andrew's idea. Philippe> And it would be nice if the mechanism used to quote args would Philippe> be compatible between user defined commands and native gdb commands. There is no universal quoting in gdb. Instead there are 3 common cases, plus the extras (neglecting MI, which is totally different and not relevant here): 1. Commands that take an expression. These are things like "print". Expressions are passed to the language parser, but language parsers have to follow some gdb rules: optionally terminate parsing at a top-level (unparenthesized) comma, and also terminate parsing at "if" or some other things (there is a mildly odd case for Ada tasks). 2. Commands that take a linespec. These can also take an expression via the "*" linespec, so linespecs end up following some expression rules. Often there are expressions after the linespec too, like break LINESPEC if EXPR or dprintf LINESPEC, "string", EXPR, EXPR... 3. Commands that use gdb_argv (or libiberty's buildargv, which is the same thing). These respect some simple quoting. However, the quoting is very simple, not like what you might expect from the shell, for example I don't think you can quote an embedded quotation mark of the same kind (that is, no "this has \"quotes\""). 4. Everything else. gdb commands are just given a string and some do whatever the author liked. Philippe> I have not understood the reference given by Tom that expressions Philippe> are terminated with , and that parenthesis stops this termination. Philippe> Is that described somewhere in the doc ? Philippe> The doc (or an example if this is not documented) will help Philippe> me to understand. I think it's largely undocumented, since some of these quirks are just constraints arising from gdb's implementation choices. Not having any rhyme or reason to command argument parsing has good and bad facets. The good is that the generally DWIM nature of commands means that usually you don't have to contort yourself to satisfy some parser. Like, you can "break foo.c:93" or "break function" rather than something like the old dbx "stop in" / "stop at" split. The bad of course is that you may sometimes want to quote something and not have any idea of how to do it: because there's no consistency; because the gdb_argv idea was not really thought through (that's my conclusion, I don't know the actual story); or perhaps because you've just tripped across some command that was implemented in a particularly bad way. Now, it would be great to fix this, at least for some subset of things. I find it hard to see a way forward, though. Breaking compatibility (see my post upthread) is unpleasant, unless maybe it is done in a very dramatic way, coupled with a major version bump and some kind of huge warning for users -- but that's also very hard to implement and release. One idea is to add a new standard way to parse arguments, for new commands. But of course then gdb just has 5 ways to do it ... :( Tom ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] gdb: Allow parenthesis to group arguments to user-defined commands 2018-08-31 20:59 ` Tom Tromey @ 2018-09-01 11:10 ` Philippe Waroquiers 2018-09-01 14:20 ` Tom Tromey 0 siblings, 1 reply; 26+ messages in thread From: Philippe Waroquiers @ 2018-09-01 11:10 UTC (permalink / raw) To: Tom Tromey; +Cc: Andrew Burgess, gdb-patches On Fri, 2018-08-31 at 14:59 -0600, Tom Tromey wrote: > > > > > > "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes: > > Philippe> For what concerns the parenthesis based solution, it looks to not work > Philippe> for some cases. > > Philippe> E.g. if I want to pass one argument (I am using single quotes to show the args > Philippe> I want in the below examples): > Philippe> 'a b) (c d' > Philippe> then I need to use: > Philippe> some_user_defined (a b) (c d) > Philippe> but that is the same notation as if I want to pass 2 arguments > Philippe> 'a b' and 'c d' > > Can you think of an actual example where this would be useful? > My feeling is that there isn't one, though I'd be interested in > tricky-but-plausible counter-examples. For example, I want a user defined command that receives a bunch of REGEXPs, and then for each regexp, the user defined command calls info types $arg0 info types $arg1 info types $arg2 More generally, any kind of user defined command might need to receive whatever args. IMO, quoting with quotes is also more usual (at least in shells) and single quotes are already used in gdb (as pointed out by Pedro). See e.g. @cindex quotes in commands @cindex completion of quoted strings @cindex C@t{++} scope resolution @cindex quoting names (and of course, my brand new brilliant patch still to be reviewed :) that implements info [args|functions|locals|variables] [-q] [-t TYPEREGEXP] [NAMEREGEXP] is using optional single quotes for TYPEREGEXP, following Pedro's initial comments). So, it looks strange to me to add a new user defined argument quoting mechanism because the current mechanism (using quotes) has quirks, but the new mechanism itself starts its life with quirks/limitations. What about the alternative solution to allow a user defined command to use $argu0, $argu1, $argu2, ... to do unquoted expansion when needed ? $arguX does unquoted expansion if there are quotes, otherwise does normal expansion. With the patch below, here is the behaviour: define show_args  printf "nargs=%d:", $argc  set $i = 0  while $i < $argc     printf " "     eval "echo [$arg%d] [$argu%d]", $i, $i     set $i = $i + 1  end  printf "\n" end (gdb) show_args 'a b' 'c d' e '   f g   ' nargs=4: ['a b'] [a b] ['c d'] [c d] [e] [e] ['   f g   '] [   f g   ] As far as I can see, the above should allow to solve the original problem of Andrew, but without constraints on what can be quoted. This is backward compatible, unless someone already used somewhere $argu followed by one or more digits in a user defined command (and if we want to reduce further the probability of backward incompatibility, we could use something like: $argunquoted0, $argunquoted1, ... but that seems very long :). Patch: diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c index 8496fb85e6..bfeb92dac6 100644 --- a/gdb/cli/cli-script.c +++ b/gdb/cli/cli-script.c @@ -811,7 +811,9 @@ locate_arg (const char *p)    while ((p = strchr (p, '$')))      {        if (startswith (p, "$arg") -         && (isdigit (p[4]) || p[4] == 'c')) +         && (isdigit (p[4])  /* E.g. $arg34.  */ +             || p[4] == 'c'  /* $argc  */ +             || (p[4] == 'u' && isdigit (p[5])))) /* E.g. $argu34.  */         return p;        p++;      } @@ -854,6 +856,10 @@ user_args::insert_args (const char *line) const         {           char *tmp;           unsigned long i; +         bool unquote = p[4] == 'u'; + +         if (unquote) +           p++;            errno = 0;           i = strtoul (p + 4, &tmp, 10); @@ -863,7 +869,13 @@ user_args::insert_args (const char *line) const             error (_("Missing argument %ld in user function."), i);           else             { -             new_line.append (m_args[i].data (), m_args[i].length ()); +             if (unquote +                 && m_args[i].length () >= 2 +                 && m_args[i].data () [0] == '\'' +                 && m_args[i].data () [m_args[i].length () - 1] == '\'') +               new_line.append (m_args[i].data () + 1, m_args[i].length () - 2); +             else +               new_line.append (m_args[i].data (), m_args[i].length ());               line = tmp;             }         } > > This feeling is why I'm generally ok with Andrew's idea. > > Philippe> And it would be nice if the mechanism used to quote args would > Philippe> be compatible between user defined commands and native gdb commands. > > There is no universal quoting in gdb. Yes, and we might maybe avoid to add a new half working new mechanism :). > Instead there are 3 common cases, > plus the extras (neglecting MI, which is totally different and not > relevant here): > > 1. Commands that take an expression. These are things like "print". > Expressions are passed to the language parser, but language parsers > have to follow some gdb rules: optionally terminate parsing at a > top-level (unparenthesized) comma, and also terminate parsing at "if" > or some other things (there is a mildly odd case for Ada tasks). > > 2. Commands that take a linespec. These can also take an expression via > the "*" linespec, so linespecs end up following some expression > rules. Often there are expressions after the linespec too, like > break LINESPEC if EXPR > or > dprintf LINESPEC, "string", EXPR, EXPR... > > 3. Commands that use gdb_argv (or libiberty's buildargv, which is the > same thing). These respect some simple quoting. However, the > quoting is very simple, not like what you might expect from the > shell, for example I don't think you can quote an embedded quotation > mark of the same kind (that is, no "this has \"quotes\""). As far as I can see, gdb_argv is escaping single and double quotes, e.g. (gdb) handle 10 'ignore this \'bidule' Unrecognized or ambiguous flag word: "ignore this 'bidule". (gdb) > > 4. Everything else. gdb commands are just given a string and some do > whatever the author liked. > > Philippe> I have not understood the reference given by Tom that expressions > Philippe> are terminated with , and that parenthesis stops this termination. > Philippe> Is that described somewhere in the doc ? > Philippe> The doc (or an example if this is not documented) will help > Philippe> me to understand. > > I think it's largely undocumented, since some of these quirks are just > constraints arising from gdb's implementation choices. > > Not having any rhyme or reason to command argument parsing has good and > bad facets. > > The good is that the generally DWIM nature of commands means that > usually you don't have to contort yourself to satisfy some parser. > Like, you can "break foo.c:93" or "break function" rather than something > like the old dbx "stop in" / "stop at" split. > > The bad of course is that you may sometimes want to quote something and > not have any idea of how to do it: because there's no consistency; > because the gdb_argv idea was not really thought through (that's my > conclusion, I don't know the actual story); or perhaps because you've > just tripped across some command that was implemented in a particularly > bad way. > > Now, it would be great to fix this, at least for some subset of things. > I find it hard to see a way forward, though. Breaking compatibility > (see my post upthread) is unpleasant, unless maybe it is done in a very > dramatic way, coupled with a major version bump and some kind of huge > warning for users -- but that's also very hard to implement and release. > > One idea is to add a new standard way to parse arguments, for new > commands. But of course then gdb just has 5 ways to do it ... :( :( But as said above, for user defined command, we might maybe repair what we have now, e.g. using the $argu approach. For sure, the current $arg expansion with quoting is also working (reasonably consistently, except with the fact that you cannot jsut 'pass' a still quoted arg containing a ' to another user defined command ?): (gdb) show_args 'a b' 'c \d e' 'f \\g h' \i \\j 'k \' l' nargs=6: ['a b'] [a b] ['c d e'] [c d e] ['f \g h'] [f \g h] [i] [i] [\j] [\j] ['k ' l'] [k ' l] (so, today, a user defined command can correctly pass a quoted arg to another user defined command, unless this quoted arg initially contained an escaped '). So, in summary, I argue for $argu :). Philippe ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] gdb: Allow parenthesis to group arguments to user-defined commands 2018-09-01 11:10 ` Philippe Waroquiers @ 2018-09-01 14:20 ` Tom Tromey 2018-09-01 15:36 ` Philippe Waroquiers 0 siblings, 1 reply; 26+ messages in thread From: Tom Tromey @ 2018-09-01 14:20 UTC (permalink / raw) To: Philippe Waroquiers; +Cc: Tom Tromey, Andrew Burgess, gdb-patches >>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes: Philippe> What about the alternative solution to allow a user defined Philippe> command to use $argu0, $argu1, $argu2, ... to do unquoted Philippe> expansion when needed ? I think the problem is that people want to write user-defined commands that act like other gdb commands, and in particular writing a user-defined that takes an expression is, by far, the most common case. But, this proposal would mean that "print" would have one style of quoting, while "user-print" would have another, drastically less nice, style. Personally I think people should just write Python, since it is a much better way. But the CLI is more convenient, so here we are. Philippe> As far as I can see, gdb_argv is escaping single and double quotes, e.g. Philippe> (gdb) handle 10 'ignore this \'bidule' Philippe> Unrecognized or ambiguous flag word: "ignore this 'bidule". Philippe> (gdb) Yeah, I misremembered that. Thanks. Tom ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] gdb: Allow parenthesis to group arguments to user-defined commands 2018-09-01 14:20 ` Tom Tromey @ 2018-09-01 15:36 ` Philippe Waroquiers 0 siblings, 0 replies; 26+ messages in thread From: Philippe Waroquiers @ 2018-09-01 15:36 UTC (permalink / raw) To: Tom Tromey; +Cc: Andrew Burgess, gdb-patches On Sat, 2018-09-01 at 08:20 -0600, Tom Tromey wrote: > > > > > > "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes: > > Philippe> What about the alternative solution to allow a user defined > Philippe> command to use $argu0, $argu1, $argu2, ... to do unquoted > Philippe> expansion when needed ? > > I think the problem is that people want to write user-defined commands > that act like other gdb commands, and in particular writing a > user-defined that takes an expression is, by far, the most common case. > > But, this proposal would mean that "print" would have one style of > quoting, while "user-print" would have another, drastically less nice, > style. Then maybe the conclusion is that we really need 2 different styles of quoting: * a quote mechanism to quote expressions, which then must have a 'balanced' nr of open/close parenthesis. * a quote mechanism (based on ' quoting) for non expressions args. Single quotes are already used by several native gdb commands (all gdb_argv based, and some others), but is (currently) poorly supported for user defined commands, which must be able to unquote an arg to pass it to native commands that do not expect quoted args. Â The $argu approach then should allow to write user defined commands that have the same behaviour as the native commands. E.g. I believe the example 'info var for a bunch of REGEXP' is not implementable today, would still not be doable with parenthesis quoting, but is I believe implementable with the $argu approach. > > Personally I think people should just write Python, since it is a much > better way. But the CLI is more convenient, so here we are. > > Philippe> As far as I can see, gdb_argv is escaping single and double quotes, e.g. > Philippe> (gdb) handle 10 'ignore this \'bidule' > Philippe> Unrecognized or ambiguous flag word: "ignore this 'bidule". > Philippe> (gdb)Â > > Yeah, I misremembered that. Thanks. Well, before you pointed me at gdb_argv, I even did not know it was existing :). So thanks to Andrew and you for this discussion, I learned a lot about gdb from it. Philippe ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] gdb: Allow parenthesis to group arguments to user-defined commands 2018-08-28 20:29 ` Philippe Waroquiers 2018-08-28 23:29 ` Andrew Burgess @ 2018-08-30 2:26 ` Tom Tromey 1 sibling, 0 replies; 26+ messages in thread From: Tom Tromey @ 2018-08-30 2:26 UTC (permalink / raw) To: Philippe Waroquiers; +Cc: Andrew Burgess, Tom Tromey, gdb-patches >>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes: Philippe> On the downside, quoting with parenthesis is something very peculiar Philippe> (is there some other tool/shell/... using this convention ?). gdb itself does this in some cases. Often an expression can be terminated with a ",", but parentheses can prevent this termination. I believe this can occur with linespecs as well. Tom ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCHv2] gdb: Rewrite argument handling for user-defined commands 2018-08-15 14:39 ` [PATCH 2/2] " Andrew Burgess 2018-08-15 18:24 ` Eli Zaretskii 2018-08-25 19:32 ` Philippe Waroquiers @ 2018-09-06 23:29 ` Andrew Burgess 2018-09-07 6:31 ` Eli Zaretskii ` (2 more replies) 2 siblings, 3 replies; 26+ messages in thread From: Andrew Burgess @ 2018-09-06 23:29 UTC (permalink / raw) To: gdb-patches; +Cc: Philippe Waroquiers, Tom Tromey, Andrew Burgess Here's a new version of the quoting patch which now uses single and double quotes for quoting arguments. I look forward to any feedback. Eli - I suspect that the documentation changes would need some work, but you should probably wait to review, as I suspect this patch will change again before it can be merged. Thanks, Andrew --- This commit rewrites argument passing for user-defined commands. The rewrite was inspired after this mailing list thread: https://sourceware.org/ml/gdb-patches/2018-08/msg00391.html The summary is that it was felt that in order to pass arguments that include whitespace, then single or double quotes should be used for quoting the argument. The problem is that currently, the quotes are included in the argument that is passed into the user-defined command, so passing the argument "1 + 1" will currently litterally pass "1 + 1" (including the quotes) to GDB, which is no good if what you want to do is to pass an expression. This commit changes how quoting works so that the quotes are NOT now included in the argument passed. If the user wants to include quotes, they would now need to use nested quotes, so "\"abc\"" will pass the argument "abc". It is also possible to use single quotes, so '"abc"' will also pass the argument "abc". As currently there's no documentation for how quoting works in user-defined commands this commit adds documentation for the new behaviour. The big risk with this commit is that this does change how arguments are passed to user-defined commands, and this might causes issues for existing users. gdb/ChangeLog: * cli/cli-script.c (user_args::m_command_line): Remove. (user_args::m_args): Change type. (user_args::user_args): Rewrite to build arguments into std::string. (user_args::insert_args): Update to take account of m_args type change. gdb/doc/ChangeLog: * gdb.texinfo (Define): Additional documentation about argument syntax. gdb/testsuite/ChangeLog: * gdb.base/commands.exp (user_defined_command_arg_quoting): New proc, which is added to the list of procs to call. * gdb.base/run.c (global_var): Defined global. --- gdb/ChangeLog | 9 ++++ gdb/cli/cli-script.c | 88 ++++++++++++++++++------------------- gdb/doc/ChangeLog | 5 +++ gdb/doc/gdb.texinfo | 66 +++++++++++++++++++++++++--- gdb/testsuite/ChangeLog | 6 +++ gdb/testsuite/gdb.base/commands.exp | 45 +++++++++++++++++++ gdb/testsuite/gdb.base/run.c | 3 ++ 7 files changed, 172 insertions(+), 50 deletions(-) diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c index 8496fb85e6f..f2110331765 100644 --- a/gdb/cli/cli-script.c +++ b/gdb/cli/cli-script.c @@ -78,12 +78,8 @@ private: user_args (const user_args &) =delete; user_args &operator= (const user_args &) =delete; - /* It is necessary to store a copy of the command line to ensure - that the arguments are not overwritten before they are used. */ - std::string m_command_line; - - /* The arguments. Each element points inside M_COMMAND_LINE. */ - std::vector<gdb::string_view> m_args; + /* The arguments. Parsed from the LINE passed into the constructor. */ + std::vector<std::string> m_args; }; /* The stack of arguments passed to user defined functions. We need a @@ -749,56 +745,58 @@ user_args::user_args (const char *command_line) if (command_line == NULL) return; - m_command_line = command_line; - p = m_command_line.c_str (); + p = command_line; while (*p) { - const char *start_arg; - int squote = 0; - int dquote = 0; - int bsquote = 0; + std::string arg; + + bool bquote = false; + bool squote = false; + bool dquote = false; /* Strip whitespace. */ - while (*p == ' ' || *p == '\t') + while (isspace (*p)) p++; - /* P now points to an argument. */ - start_arg = p; - /* Get to the end of this argument. */ while (*p) { - if (((*p == ' ' || *p == '\t')) && !squote && !dquote && !bsquote) - break; - else - { - if (bsquote) - bsquote = 0; - else if (*p == '\\') - bsquote = 1; - else if (squote) - { - if (*p == '\'') - squote = 0; - } - else if (dquote) - { - if (*p == '"') - dquote = 0; - } - else - { - if (*p == '\'') - squote = 1; - else if (*p == '"') - dquote = 1; - } - p++; - } + /* If we find whitespace and we're not inside a single or double + quote then we have found the end of this argument. */ + if (isspace (*p) && !(squote || dquote)) + break; + else if (bquote) + bquote = 0; + else + { + /* If we're inside a single quote and we find another single + quote then this is the end of the argument. */ + if (*p == '\'' && !dquote) + { + ++p; + squote = !squote; + continue; + } + + /* If we're inside a double quote and we find another double + quote then this is the end of the argument. */ + if (*p == '"' && !squote) + { + ++p; + dquote = !dquote; + continue; + } + + if (*p == '\\' && !squote) + bquote = true; + } + + arg += *p; + ++p; } - m_args.emplace_back (start_arg, p - start_arg); + m_args.emplace_back (arg); } } @@ -863,7 +861,7 @@ user_args::insert_args (const char *line) const error (_("Missing argument %ld in user function."), i); else { - new_line.append (m_args[i].data (), m_args[i].length ()); + new_line.append (m_args[i]); line = tmp; } } diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index f2d1155b4db..b159df3b217 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -25245,14 +25245,70 @@ @noindent This defines the command @code{adder}, which prints the sum of -its three arguments. Note the arguments are text substitutions, so they may -reference variables, use complex expressions, or even perform inferior -functions calls. +its three arguments. + +The arguments to user-defined commands are text substitutions, so they +may reference variables, use complex expressions, or even perform +inferior functions calls. Each argument is separated with whitespace, +so in the previous example three arguments were passed. The following +example also passes three arguments, though the arguments are more +complex: + +@smallexample +adder 10+1 10+2 10+3 +@end smallexample + +@noindent +However, if whitespace were added around the @code{+} characters, then +9 arguments would be passed, @code{adder} only uses the first 3 of +these arguments, and the others would be silently ignored, for example: + +@smallexample +adder 10 + 1 10 + 2 10 + 3 +@end smallexample + +Causes @value{GDBN} to try and evaluate the following, which is likely +invalid: + +@smallexample +print 10 + + + 1 +@end smallexample + +@noindent +Arguments can be quoted with double (@code{"}) or single (@code{'}) +quotes. These quotes are not passed through as part of the argument, +so the complex arguments from the previous example can be written as: + +@smallexample +adder '10 + 1' '10 + 2' '10 + 3' +@end smallexample + +@noindent +As the quotes are not passed through, then the previous example causes +@value{GDBN} to evaluate: + +@smallexample +print 10 + 1 + 10 + 2 + 10 + 3 +@end smallexample + +@noindent +Outside of quotes, a backslash can be used to pass a quote as part of +an argument, for example, @code{\'} will pass a single quote as an +argument, and @code{\"} passes a double quote as an argument. + +Within double quotes a backslash can be used to pass a literal double +quote, so @code{"\"abc\""} will pass the argument @code{"abc"}. + +Within single quotes a backslash does not escape a single quote, the +next single quote always ends a single quoted argument, and +backslashes within a single quoted argument are passed straight +through, so @code{'abc\'} will pass the argument @code{abc\}. @cindex argument count in user-defined commands @cindex how many arguments (user-defined commands) -In addition, @code{$argc} may be used to find out how many arguments have -been passed. +@noindent +Within a user-defined command @code{$argc} may be used to find out how +many arguments have been passed. @smallexample define adder diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp index 52a22bb5ddc..4e398b84a95 100644 --- a/gdb/testsuite/gdb.base/commands.exp +++ b/gdb/testsuite/gdb.base/commands.exp @@ -1167,6 +1167,50 @@ proc_with_prefix backslash_in_multi_line_command_test {} { gdb_test "print 1" "" "run command" } +proc_with_prefix user_defined_command_arg_quoting {} { + gdb_test_multiple "define show_args" "define show_args" { + -re "End with" { + pass "define show_args" + } + } + + # This test should alternate between 0xdeadbeef and 0xfeedface two times. + gdb_test \ + [multi_line_input \ + {printf "nargs=%d:", $argc} \ + {set $i = 0} \ + {while $i < $argc} \ + {printf " "} \ + {eval "echo '$arg%d'", $i} \ + {set $i = $i + 1} \ + {end} \ + {printf "\n"} \ + {end}] \ + "" \ + "enter commands" + + gdb_test "show_args 1 2 3" \ + "nargs=3: '1' '2' '3'" + + gdb_test "show_args 1 '1 + 1' '1 + (1 + 1)'" \ + "nargs=3: '1' '1 \\+ 1' '1 \\+ \\(1 \\+ 1\\)'" + + gdb_test "show_args '{unsigned long long} &global_var'" \ + "nargs=1: '{unsigned long long} &global_var'" + + gdb_test "show_args '*((unsigned long long *) &global_var)'" \ + "nargs=1: '\\*\\(\\(unsigned long long \\*\\) &global_var\\)'" + + gdb_test "show_args '\"' \"'\" \"''\"" \ + "nargs=3: '\"' ''' ''''" + + gdb_test "show_args \\n a\\'b" \ + "nargs=2: '\r\n' 'a'b'" + + gdb_test "show_args \"This\\nIs\\nA\\nMulti Line\\nMessage" \ + "nargs=1: 'This\r\nIs\r\nA\r\nMulti Line\r\nMessage'" +} + gdbvar_simple_if_test gdbvar_simple_while_test gdbvar_complex_if_while_test @@ -1182,6 +1226,7 @@ user_defined_command_case_sensitivity user_defined_command_args_eval user_defined_command_args_stack_test user_defined_command_manyargs_test +user_defined_command_arg_quoting watchpoint_command_test test_command_prompt_position deprecated_command_test diff --git a/gdb/testsuite/gdb.base/run.c b/gdb/testsuite/gdb.base/run.c index 614b018260d..d89bad78bb4 100644 --- a/gdb/testsuite/gdb.base/run.c +++ b/gdb/testsuite/gdb.base/run.c @@ -8,6 +8,9 @@ #include "../lib/unbuffer_output.c" +/* Used by commands.exp test script. */ +volatile unsigned long long global_var = 34; + int factorial (int); int -- 2.14.4 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCHv2] gdb: Rewrite argument handling for user-defined commands 2018-09-06 23:29 ` [PATCHv2] gdb: Rewrite argument handling for " Andrew Burgess @ 2018-09-07 6:31 ` Eli Zaretskii 2018-09-07 20:36 ` Tom Tromey 2018-09-08 5:35 ` Philippe Waroquiers 2 siblings, 0 replies; 26+ messages in thread From: Eli Zaretskii @ 2018-09-07 6:31 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches, philippe.waroquiers, tom > From: Andrew Burgess <andrew.burgess@embecosm.com> > Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>, Tom Tromey <tom@tromey.com>, Andrew Burgess <andrew.burgess@embecosm.com> > Date: Fri, 7 Sep 2018 00:29:04 +0100 > > Eli - I suspect that the documentation changes would need some work, > but you should probably wait to review, as I suspect this patch will > change again before it can be merged. Fine with me, let me know when it's ready. Thanks. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCHv2] gdb: Rewrite argument handling for user-defined commands 2018-09-06 23:29 ` [PATCHv2] gdb: Rewrite argument handling for " Andrew Burgess 2018-09-07 6:31 ` Eli Zaretskii @ 2018-09-07 20:36 ` Tom Tromey 2018-09-07 22:47 ` Andrew Burgess 2018-09-08 5:35 ` Philippe Waroquiers 2 siblings, 1 reply; 26+ messages in thread From: Tom Tromey @ 2018-09-07 20:36 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches, Philippe Waroquiers, Tom Tromey >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes: Andrew> This commit changes how quoting works so that the quotes are NOT now Andrew> included in the argument passed. If the user wants to include quotes, Andrew> they would now need to use nested quotes, so "\"abc\"" will pass the Andrew> argument "abc". Andrew> It is also possible to use single quotes, so '"abc"' will also pass Andrew> the argument "abc". Andrew> As currently there's no documentation for how quoting works in Andrew> user-defined commands this commit adds documentation for the new Andrew> behaviour. Andrew> The big risk with this commit is that this does change how arguments Andrew> are passed to user-defined commands, and this might causes issues for Andrew> existing users. I think this change goes against the compatibility approach I discussed in that earlier thread -- it changes the syntax of a command in a way that is likely to be used in practice. In my opinion, the documentation issue in cases like this is not a strong argument in favor of allowing a change. That's because users will often resort to trial-and-error to get gdb to do what they want. So unless the documentation is very clear, in practice, and especially over long periods of time, behavior is locked to the implementation. So, my own inclination is to say no to this patch, though I welcome & will listen to other responses. I'd accept a patch adding an option to define, though as I mentioned earlier, in a case like that I think it's better to design something very good rather than to try to patch things piecemeal; the latter being how gdb ended up in this situation in the first place. Tom ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCHv2] gdb: Rewrite argument handling for user-defined commands 2018-09-07 20:36 ` Tom Tromey @ 2018-09-07 22:47 ` Andrew Burgess 2018-09-08 6:27 ` Eli Zaretskii 0 siblings, 1 reply; 26+ messages in thread From: Andrew Burgess @ 2018-09-07 22:47 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches, Philippe Waroquiers * Tom Tromey <tom@tromey.com> [2018-09-07 14:36:04 -0600]: > >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes: > > Andrew> This commit changes how quoting works so that the quotes are NOT now > Andrew> included in the argument passed. If the user wants to include quotes, > Andrew> they would now need to use nested quotes, so "\"abc\"" will pass the > Andrew> argument "abc". > > Andrew> It is also possible to use single quotes, so '"abc"' will also pass > Andrew> the argument "abc". > > Andrew> As currently there's no documentation for how quoting works in > Andrew> user-defined commands this commit adds documentation for the new > Andrew> behaviour. > > Andrew> The big risk with this commit is that this does change how arguments > Andrew> are passed to user-defined commands, and this might causes issues for > Andrew> existing users. > > I think this change goes against the compatibility approach I discussed > in that earlier thread -- it changes the syntax of a command in a way > that is likely to be used in practice. > > In my opinion, the documentation issue in cases like this is not a > strong argument in favor of allowing a change. That's because users > will often resort to trial-and-error to get gdb to do what they want. > So unless the documentation is very clear, in practice, and especially > over long periods of time, behavior is locked to the implementation. > > So, my own inclination is to say no to this patch, though I welcome & > will listen to other responses. > > I'd accept a patch adding an option to define, OK, so we could have a switch to define: (gdb) define --arg-style=v2 my_command ... end Or just a global flag: (gdb) set define-arg-style v1 | v2 I think either is fine, and neither is too hard to add on, so the next hurdle here is: > though as I mentioned > earlier, in a case like that I think it's better to design something > very good rather than to try to patch things piecemeal; the latter being > how gdb ended up in this situation in the first place. Here's my proposal then for how argument quoting might work in a V2 scheme, I'll use '>>' and '<<' to delimit examples where needed: 1. Arguments are separated by whitespace, 2. Everything within matched single or double quotes is a single argument. 3. After a quoted region you still need whitespace to split the arguments, so >>'abc'def<< will be a single argument, >>abcdef<<. 4. Within double quotes, a backslash can be used to escape a double quotes, so >>"\"this is one\""<< will give >>"this is one"<<. 5. Within single quotes backslash does NOT escape a single quote, so a single quote always ends a quoted block, so >>'this is\'<< will pass the argument >>this is\<<. I'm not 100% committed to this idea, and can make this like double quotes if that is preferred. 6. Outside of single or double quotes, a backslash can be used to quote a single or double quotes, so >>ab\'cd<< is >>ab'cd<<. 7. Other than the cases listed in 4, 5, and 6, a backslash is passed through, as is, so >>"\"multi \n line\""<< will pass >>"multi \n line"<<. I'm happy to build or flesh out these rules more if people have feedback. The patch below is just a quick iteration to add a control flag, and to better follow the rules I laid out above. The patch would need tests, and some help text filling in, but I expect I will get feedback and need to adjust things some more yet, so this is still a work in progress. In the patch below I favoured having a global flag to control argument parsing, over requiring the user to flag every use of 'define'. Maybe that's the wrong approach.... again, feedback welcome. Thanks, Andrew --- gdb: Rewrite argument handling for user-defined commands This commit rewrites argument passing for user-defined commands. The rewrite was inspired after this mailing list thread: https://sourceware.org/ml/gdb-patches/2018-08/msg00391.html The summary is that it was felt that in order to pass arguments that include whitespace, then single or double quotes should be used for quoting the argument. The problem is that currently, the quotes are included in the argument that is passed into the user-defined command, so passing the argument "1 + 1" will currently litterally pass "1 + 1" (including the quotes) to GDB, which is no good if what you want to do is to pass an expression. This commit changes how quoting works so that the quotes are NOT now included in the argument passed. If the user wants to include quotes, they would now need to use nested quotes, so "\"abc\"" will pass the argument "abc". It is also possible to use single quotes, so '"abc"' will also pass the argument "abc". As currently there's no documentation for how quoting works in user-defined commands this commit adds documentation for the new behaviour. The big risk with this commit is that this does change how arguments are passed to user-defined commands, and this might causes issues for existing users. gdb/ChangeLog: * cli/cli-script.c (user_defined_command_arg_scheme_v1): New static variable. (user_defined_command_arg_scheme_v2): Likewise. (user_defined_command_arg_scheme_names): Likewise. (user_defined_command_arg_scheme_string): Likewise. (show_user_defined_command_arg_scheme): New static function. (user_args::m_command_line): Remove. (user_args::m_args): Change type. (user_args::parse_command_line_v1): New method, contains the old contents of user_args::user_args. (user_args::parse_command_line_v2): New method, contains code to parse arguments using the new scheme. (user_args::user_args): Call one of the parser methods, the old content has moved into user_args::parse_command_line_v1. (user_args::insert_args): Update to take account of m_args type change. (_initialize_cli_script): Register new set/show variable. gdb/doc/ChangeLog: * gdb.texinfo (Define): Additional documentation about argument syntax. gdb/testsuite/ChangeLog: * gdb.base/commands.exp (user_defined_command_arg_quoting): New proc, which is added to the list of procs to call. * gdb.base/run.c (global_var): Defined global. --- gdb/ChangeLog | 20 ++++++ gdb/cli/cli-script.c | 136 ++++++++++++++++++++++++++++++++---- gdb/doc/ChangeLog | 5 ++ gdb/doc/gdb.texinfo | 66 +++++++++++++++-- gdb/testsuite/ChangeLog | 6 ++ gdb/testsuite/gdb.base/commands.exp | 50 +++++++++++++ gdb/testsuite/gdb.base/run.c | 3 + 7 files changed, 266 insertions(+), 20 deletions(-) diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c index 8496fb85e6f..edfc6586b6d 100644 --- a/gdb/cli/cli-script.c +++ b/gdb/cli/cli-script.c @@ -58,6 +58,31 @@ static int command_nest_depth = 1; /* This is to prevent certain commands being printed twice. */ static int suppress_next_print_command_trace = 0; +/* Constants used in argument parsing control switch. */ +static const char user_defined_command_arg_scheme_v1[] = "v1"; +static const char user_defined_command_arg_scheme_v2[] = "v2"; + +static const char *const user_defined_command_arg_scheme_names[] = { + user_defined_command_arg_scheme_v1, + user_defined_command_arg_scheme_v2, + NULL +}; + +/* The control variable for which argument parsing scheme user defined + commands use. */ +static const char *user_defined_command_arg_scheme_string + = user_defined_command_arg_scheme_v1; + +static void +show_user_defined_command_arg_scheme (struct ui_file *file, int from_tty, + struct cmd_list_element *c, + const char *value) +{ + fprintf_filtered (file, + _("Arguments to user defined commands are parsed " + "using scheme \"%s\".\n"), value); +} + /* Structure for arguments to user defined functions. */ class user_args @@ -78,12 +103,13 @@ private: user_args (const user_args &) =delete; user_args &operator= (const user_args &) =delete; - /* It is necessary to store a copy of the command line to ensure - that the arguments are not overwritten before they are used. */ - std::string m_command_line; + /* We support two schemes for parsing a command line arguments into a + list of arguments. */ + void parse_command_line_v1 (const char *line); + void parse_command_line_v2 (const char *line); - /* The arguments. Each element points inside M_COMMAND_LINE. */ - std::vector<gdb::string_view> m_args; + /* The arguments. Parsed from the LINE passed into the constructor. */ + std::vector<std::string> m_args; }; /* The stack of arguments passed to user defined functions. We need a @@ -744,17 +770,27 @@ if_command (const char *arg, int from_tty) user_args::user_args (const char *command_line) { - const char *p; - if (command_line == NULL) return; - m_command_line = command_line; - p = m_command_line.c_str (); + if (user_defined_command_arg_scheme_string + == user_defined_command_arg_scheme_v1) + parse_command_line_v1 (command_line); + else + parse_command_line_v2 (command_line); +} + +void +user_args::parse_command_line_v1 (const char *command_line) +{ + gdb_assert (command_line != NULL); + + const char *p = command_line; while (*p) { - const char *start_arg; + std::string arg; + int squote = 0; int dquote = 0; int bsquote = 0; @@ -763,9 +799,6 @@ user_args::user_args (const char *command_line) while (*p == ' ' || *p == '\t') p++; - /* P now points to an argument. */ - start_arg = p; - /* Get to the end of this argument. */ while (*p) { @@ -794,11 +827,75 @@ user_args::user_args (const char *command_line) else if (*p == '"') dquote = 1; } + arg += *p; p++; } } - m_args.emplace_back (start_arg, p - start_arg); + m_args.emplace_back (arg); + } +} + +/* The newer v2 scheme for parsing arguments from a command line. This + scheme strips single and double quotes from arguments, thus allowing + quoting of expressions. Within double quotes you can use backslash to + escape nested double quotes. */ + +void +user_args::parse_command_line_v2 (const char *command_line) +{ + gdb_assert (command_line != NULL); + + for (const char *p = command_line; *p != '\0'; ) + { + std::string arg; + bool squote = false; + bool dquote = false; + + /* Strip whitespace. */ + while (isspace (*p)) + p++; + + /* Get to the end of this argument. */ + while (*p) + { + /* If we find whitespace and we're not inside a single or double + quote then we have found the end of this argument. */ + if (isspace (*p) && !(squote || dquote)) + break; + else + { + /* If we're inside a single quote and we find another single + quote then this is the end of the argument. */ + if (*p == '\'' && !dquote) + { + ++p; + squote = !squote; + continue; + } + + /* If we're inside a double quote and we find another double + quote then this is the end of the argument. */ + if (*p == '"' && !squote) + { + ++p; + dquote = !dquote; + continue; + } + + if (*p == '\\' && !squote) + { + if (*(p + 1) == '\'' + || *(p + 1) == '"') + ++p; + } + } + + arg += *p; + ++p; + } + + m_args.emplace_back (arg); } } @@ -863,7 +960,7 @@ user_args::insert_args (const char *line) const error (_("Missing argument %ld in user function."), i); else { - new_line.append (m_args[i].data (), m_args[i].length ()); + new_line.append (m_args[i]); line = tmp; } } @@ -1618,4 +1715,13 @@ The conditional expression must follow the word `if' and must in turn be\n\ followed by a new line. The nested commands must be entered one per line,\n\ and should be terminated by the word 'else' or `end'. If an else clause\n\ is used, the same rules apply to its nested commands as to the first ones.")); + + add_setshow_enum_cmd ("user-defined-command-argument-scheme", class_support, + user_defined_command_arg_scheme_names, + &user_defined_command_arg_scheme_string, _("\ +Set the argument parsing scheme used by user-defined commands."), _("\ +Show the argument parsing scheme used by user-defined commands."), _("\ +Write something here..."), + NULL, show_user_defined_command_arg_scheme, + &setlist, &showlist); } diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index f2d1155b4db..b159df3b217 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -25245,14 +25245,70 @@ @noindent This defines the command @code{adder}, which prints the sum of -its three arguments. Note the arguments are text substitutions, so they may -reference variables, use complex expressions, or even perform inferior -functions calls. +its three arguments. + +The arguments to user-defined commands are text substitutions, so they +may reference variables, use complex expressions, or even perform +inferior functions calls. Each argument is separated with whitespace, +so in the previous example three arguments were passed. The following +example also passes three arguments, though the arguments are more +complex: + +@smallexample +adder 10+1 10+2 10+3 +@end smallexample + +@noindent +However, if whitespace were added around the @code{+} characters, then +9 arguments would be passed, @code{adder} only uses the first 3 of +these arguments, and the others would be silently ignored, for example: + +@smallexample +adder 10 + 1 10 + 2 10 + 3 +@end smallexample + +Causes @value{GDBN} to try and evaluate the following, which is likely +invalid: + +@smallexample +print 10 + + + 1 +@end smallexample + +@noindent +Arguments can be quoted with double (@code{"}) or single (@code{'}) +quotes. These quotes are not passed through as part of the argument, +so the complex arguments from the previous example can be written as: + +@smallexample +adder '10 + 1' '10 + 2' '10 + 3' +@end smallexample + +@noindent +As the quotes are not passed through, then the previous example causes +@value{GDBN} to evaluate: + +@smallexample +print 10 + 1 + 10 + 2 + 10 + 3 +@end smallexample + +@noindent +Outside of quotes, a backslash can be used to pass a quote as part of +an argument, for example, @code{\'} will pass a single quote as an +argument, and @code{\"} passes a double quote as an argument. + +Within double quotes a backslash can be used to pass a literal double +quote, so @code{"\"abc\""} will pass the argument @code{"abc"}. + +Within single quotes a backslash does not escape a single quote, the +next single quote always ends a single quoted argument, and +backslashes within a single quoted argument are passed straight +through, so @code{'abc\'} will pass the argument @code{abc\}. @cindex argument count in user-defined commands @cindex how many arguments (user-defined commands) -In addition, @code{$argc} may be used to find out how many arguments have -been passed. +@noindent +Within a user-defined command @code{$argc} may be used to find out how +many arguments have been passed. @smallexample define adder diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp index 52a22bb5ddc..91023dae7cc 100644 --- a/gdb/testsuite/gdb.base/commands.exp +++ b/gdb/testsuite/gdb.base/commands.exp @@ -1167,6 +1167,55 @@ proc_with_prefix backslash_in_multi_line_command_test {} { gdb_test "print 1" "" "run command" } +proc_with_prefix user_defined_command_arg_quoting {} { + + gdb_test_no_output "set user-defined-command-argument-scheme v2" + + gdb_test_multiple "define show_args" "define show_args" { + -re "End with" { + pass "define show_args" + } + } + + # This test should alternate between 0xdeadbeef and 0xfeedface two times. + gdb_test \ + [multi_line_input \ + {printf "nargs=%d:", $argc} \ + {set $i = 0} \ + {while $i < $argc} \ + {printf " "} \ + {eval "echo '$arg%d'", $i} \ + {set $i = $i + 1} \ + {end} \ + {printf "\n"} \ + {end}] \ + "" \ + "enter commands" + + gdb_test "show_args 1 2 3" \ + "nargs=3: '1' '2' '3'" + + gdb_test "show_args 1 '1 + 1' '1 + (1 + 1)'" \ + "nargs=3: '1' '1 \\+ 1' '1 \\+ \\(1 \\+ 1\\)'" + + gdb_test "show_args '{unsigned long long} &global_var'" \ + "nargs=1: '{unsigned long long} &global_var'" + + gdb_test "show_args '*((unsigned long long *) &global_var)'" \ + "nargs=1: '\\*\\(\\(unsigned long long \\*\\) &global_var\\)'" + + gdb_test "show_args '\"' \"'\" \"''\"" \ + "nargs=3: '\"' ''' ''''" + + gdb_test "show_args \\n a\\'b" \ + "nargs=2: '\r\n' 'a'b'" + + gdb_test "show_args \"This\\nIs\\nA\\nMulti Line\\nMessage" \ + "nargs=1: 'This\r\nIs\r\nA\r\nMulti Line\r\nMessage'" + + gdb_test_no_output "set user-defined-command-argument-scheme v1" +} + gdbvar_simple_if_test gdbvar_simple_while_test gdbvar_complex_if_while_test @@ -1182,6 +1231,7 @@ user_defined_command_case_sensitivity user_defined_command_args_eval user_defined_command_args_stack_test user_defined_command_manyargs_test +user_defined_command_arg_quoting watchpoint_command_test test_command_prompt_position deprecated_command_test diff --git a/gdb/testsuite/gdb.base/run.c b/gdb/testsuite/gdb.base/run.c index 614b018260d..d89bad78bb4 100644 --- a/gdb/testsuite/gdb.base/run.c +++ b/gdb/testsuite/gdb.base/run.c @@ -8,6 +8,9 @@ #include "../lib/unbuffer_output.c" +/* Used by commands.exp test script. */ +volatile unsigned long long global_var = 34; + int factorial (int); int -- 2.14.4 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCHv2] gdb: Rewrite argument handling for user-defined commands 2018-09-07 22:47 ` Andrew Burgess @ 2018-09-08 6:27 ` Eli Zaretskii 0 siblings, 0 replies; 26+ messages in thread From: Eli Zaretskii @ 2018-09-08 6:27 UTC (permalink / raw) To: Andrew Burgess; +Cc: tom, gdb-patches, philippe.waroquiers > Date: Fri, 7 Sep 2018 23:47:21 +0100 > From: Andrew Burgess <andrew.burgess@embecosm.com> > Cc: gdb-patches@sourceware.org, Philippe Waroquiers <philippe.waroquiers@skynet.be> > > 1. Arguments are separated by whitespace, > > 2. Everything within matched single or double quotes is a single > argument. > > 3. After a quoted region you still need whitespace to split the > arguments, so >>'abc'def<< will be a single argument, >>abcdef<<. > > 4. Within double quotes, a backslash can be used to escape a double > quotes, so >>"\"this is one\""<< will give >>"this is one"<<. > > 5. Within single quotes backslash does NOT escape a single quote, so > a single quote always ends a quoted block, so >>'this is\'<< will > pass the argument >>this is\<<. I'm not 100% committed to this idea, > and can make this like double quotes if that is preferred. Why did you decide on this exception for single quotes? Are there any downsides to having \' mean a literal ' inside '..' quoted arguments? If there is no downside, I think it would be a better UX to have both kinds of quotes behave identically wrt escapes. Thanks. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCHv2] gdb: Rewrite argument handling for user-defined commands 2018-09-06 23:29 ` [PATCHv2] gdb: Rewrite argument handling for " Andrew Burgess 2018-09-07 6:31 ` Eli Zaretskii 2018-09-07 20:36 ` Tom Tromey @ 2018-09-08 5:35 ` Philippe Waroquiers 2018-09-08 14:33 ` Andrew Burgess 2 siblings, 1 reply; 26+ messages in thread From: Philippe Waroquiers @ 2018-09-08 5:35 UTC (permalink / raw) To: Andrew Burgess, gdb-patches; +Cc: Tom Tromey On Fri, 2018-09-07 at 00:29 +0100, Andrew Burgess wrote: > Here's a new version of the quoting patch which now uses single and > double quotes for quoting arguments. > > I look forward to any feedback. > > Eli - I suspect that the documentation changes would need some work, > but you should probably wait to review, as I suspect this patch will > change again before it can be merged. > > Thanks, > Andrew > > --- > > This commit rewrites argument passing for user-defined commands. The > rewrite was inspired after this mailing list thread: > > https://sourceware.org/ml/gdb-patches/2018-08/msg00391.html > > The summary is that it was felt that in order to pass arguments that > include whitespace, then single or double quotes should be used for > quoting the argument. Tom felt that we need to support your initial suggestion (parenthesis quoting) for 'balanced expressions', as parenthesis are used in some other commands that are evaluating expressions. I can understand his point of view, see https://sourceware.org/ml/gdb-patches/2018-09/msg00007.html > > The problem is that currently, the quotes are included in the argument > that is passed into the user-defined command, so passing the argument > "1 + 1" will currently litterally pass "1 + 1" (including the quotes) > to GDB, which is no good if what you want to do is to pass an > expression. For this problem, an alternative solution is to have a new way to expand an argument : $argX expands the argument X with the quotes $arguX expands the argument X with the quotes. That allows to pass a quoted argument containing spaces, and use it in the user defined command without quotes where needed, and with quotes where needed : if the user defined command has to call another command (user defined or a native) that itself needs quoting, then use $argX, else use $arguX. In other words, how to handle a quoted arg is decided by the user command 'developer' (similarly to some native GDB commands). So, adder command would become print $argu1 + $argu2 + $argu3 See https://sourceware.org/ml/gdb-patches/2018-09/msg00005.html for a patch (only very limited manual testing done) implementing the arguX approach : (gdb)     define adder Type commands for definition of "adder". End with a line saying just "end". >       print $arg0 + $arg1 + $arg2 >     end (gdb) adder '1 + 5' 2 3 No symbol table is loaded.  Use the "file" command. (gdb) (gdb) define adder Redefine command "adder"? (y or n) y Type commands for definition of "adder". End with a line saying just "end". >print $argu0 + $argu1 + $argu2 >end (gdb) adder '1 + 5' 2 3             $4 = 11 (gdb) > > This commit changes how quoting works so that the quotes are NOT now > included in the argument passed. If the user wants to include quotes, > they would now need to use nested quotes, so "\"abc\"" will pass the > argument "abc". > > It is also possible to use single quotes, so '"abc"' will also pass > the argument "abc". > > As currently there's no documentation for how quoting works in > user-defined commands this commit adds documentation for the new > behaviour. > > The big risk with this commit is that this does change how arguments > are passed to user-defined commands, and this might causes issues for > existing users. Yes, that has the potential to create a lot of backward incompatibility, which is not the case for the $arguX and/or the parenthesis approach you suggested initially. Philippe ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCHv2] gdb: Rewrite argument handling for user-defined commands 2018-09-08 5:35 ` Philippe Waroquiers @ 2018-09-08 14:33 ` Andrew Burgess 0 siblings, 0 replies; 26+ messages in thread From: Andrew Burgess @ 2018-09-08 14:33 UTC (permalink / raw) To: Philippe Waroquiers; +Cc: gdb-patches, Tom Tromey * Philippe Waroquiers <philippe.waroquiers@skynet.be> [2018-09-08 07:35:14 +0200]: > On Fri, 2018-09-07 at 00:29 +0100, Andrew Burgess wrote: > > Here's a new version of the quoting patch which now uses single and > > double quotes for quoting arguments. > > > > I look forward to any feedback. > > > > Eli - I suspect that the documentation changes would need some work, > > but you should probably wait to review, as I suspect this patch will > > change again before it can be merged. > > > > Thanks, > > Andrew > > > > --- > > > > This commit rewrites argument passing for user-defined commands. The > > rewrite was inspired after this mailing list thread: > > > > https://sourceware.org/ml/gdb-patches/2018-08/msg00391.html > > > > The summary is that it was felt that in order to pass arguments that > > include whitespace, then single or double quotes should be used for > > quoting the argument. > Tom felt that we need to support your initial suggestion (parenthesis > quoting) for 'balanced expressions', as parenthesis are used in > some other commands that are evaluating expressions. > I can understand his point of view, see > https://sourceware.org/ml/gdb-patches/2018-09/msg00007.html > > > > > The problem is that currently, the quotes are included in the argument > > that is passed into the user-defined command, so passing the argument > > "1 + 1" will currently litterally pass "1 + 1" (including the quotes) > > to GDB, which is no good if what you want to do is to pass an > > expression. > For this problem, an alternative solution is to have a new > way to expand an argument : > $argX expands the argument X with the quotes > $arguX expands the argument X with the quotes. > > That allows to pass a quoted argument containing spaces, > and use it in the user defined command without quotes where needed, > and with quotes where needed : if the user defined command has to call > another command (user defined or a native) that itself needs quoting, > then use $argX, else use $arguX. > In other words, how to handle a quoted arg is decided by the > user command 'developer' (similarly to some native GDB commands). > > So, adder command would become > print $argu1 + $argu2 + $argu3 > > See https://sourceware.org/ml/gdb-patches/2018-09/msg00005.html > for a patch (only very limited manual testing done) implementing > the arguX approach : Great. OK, lets go with this approach then. Tom, do you feel that we need more is required above and beyond adding argX / arguX as Philippe has suggested? Thanks, Andrew > > (gdb) define adder > Type commands for definition of "adder". > End with a line saying just "end". > > print $arg0 + $arg1 + $arg2 > > end > (gdb) adder '1 + 5' 2 3 > No symbol table is loaded. Use the "file" command. > (gdb) > > (gdb) define adder > Redefine command "adder"? (y or n) y > Type commands for definition of "adder". > End with a line saying just "end". > >print $argu0 + $argu1 + $argu2 > >end > (gdb) adder '1 + 5' 2 3 > $4 = 11 > (gdb) > > > > > > This commit changes how quoting works so that the quotes are NOT now > > included in the argument passed. If the user wants to include quotes, > > they would now need to use nested quotes, so "\"abc\"" will pass the > > argument "abc". > > > > It is also possible to use single quotes, so '"abc"' will also pass > > the argument "abc". > > > > As currently there's no documentation for how quoting works in > > user-defined commands this commit adds documentation for the new > > behaviour. > > > > The big risk with this commit is that this does change how arguments > > are passed to user-defined commands, and this might causes issues for > > existing users. > Yes, that has the potential to create a lot of backward incompatibility, > which is not the case for the $arguX and/or the parenthesis approach > you suggested initially. > > Philippe > > ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/2] gdb: Make testnames unique in gdb.base/commands.exp 2018-08-15 14:39 [PATCH 0/2] gdb: Allow parenthesis to group arguments to user-defined commands Andrew Burgess 2018-08-15 14:39 ` [PATCH 2/2] " Andrew Burgess @ 2018-08-15 14:39 ` Andrew Burgess 2018-08-30 15:26 ` Tom Tromey 1 sibling, 1 reply; 26+ messages in thread From: Andrew Burgess @ 2018-08-15 14:39 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess gdb/testsuite/ChangeLog: * gdb.base/commands.exp (user_defined_command_case_sensitivity): Make test names unique. --- gdb/testsuite/ChangeLog | 5 +++++ gdb/testsuite/gdb.base/commands.exp | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp index 259b89b803d..7ce33fdefa9 100644 --- a/gdb/testsuite/gdb.base/commands.exp +++ b/gdb/testsuite/gdb.base/commands.exp @@ -368,8 +368,8 @@ proc_with_prefix user_defined_command_case_sensitivity {} { gdb_test "print 456\nend" "" "enter commands 2" - gdb_test "Homer-Simpson" " = 123" "execute command" - gdb_test "HomeR-SimpsoN" " = 456" "execute command" + gdb_test "Homer-Simpson" " = 123" "execute first command" + gdb_test "HomeR-SimpsoN" " = 456" "execute second command" gdb_test "HOMER-SIMPSON" "Undefined command.*" "try to call in upper case" gdb_test "homer-simpson" "Undefined command.*" "try to call in lower case" } -- 2.14.4 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] gdb: Make testnames unique in gdb.base/commands.exp 2018-08-15 14:39 ` [PATCH 1/2] gdb: Make testnames unique in gdb.base/commands.exp Andrew Burgess @ 2018-08-30 15:26 ` Tom Tromey 0 siblings, 0 replies; 26+ messages in thread From: Tom Tromey @ 2018-08-30 15:26 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes: Andrew> gdb/testsuite/ChangeLog: Andrew> * gdb.base/commands.exp (user_defined_command_case_sensitivity): Andrew> Make test names unique. This one is obsolete now due to commit ead9aa39bfc80007336bc96c6374df7f79341485 Author: Philippe Waroquiers <philippe.waroquiers@skynet.be> Date: Thu Aug 2 23:15:23 2018 +0200 Modify gdb.base/commands.exp to test multi breakpoints command clearing. Tom ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2018-09-08 14:33 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-08-15 14:39 [PATCH 0/2] gdb: Allow parenthesis to group arguments to user-defined commands Andrew Burgess 2018-08-15 14:39 ` [PATCH 2/2] " Andrew Burgess 2018-08-15 18:24 ` Eli Zaretskii 2018-08-25 19:32 ` Philippe Waroquiers 2018-08-25 20:53 ` Philippe Waroquiers 2018-08-25 22:43 ` Andrew Burgess 2018-08-28 15:54 ` Tom Tromey 2018-08-28 18:43 ` Andrew Burgess 2018-08-28 20:29 ` Philippe Waroquiers 2018-08-28 23:29 ` Andrew Burgess 2018-08-30 2:19 ` Tom Tromey 2018-08-30 21:19 ` Philippe Waroquiers 2018-08-31 20:59 ` Tom Tromey 2018-09-01 11:10 ` Philippe Waroquiers 2018-09-01 14:20 ` Tom Tromey 2018-09-01 15:36 ` Philippe Waroquiers 2018-08-30 2:26 ` Tom Tromey 2018-09-06 23:29 ` [PATCHv2] gdb: Rewrite argument handling for " Andrew Burgess 2018-09-07 6:31 ` Eli Zaretskii 2018-09-07 20:36 ` Tom Tromey 2018-09-07 22:47 ` Andrew Burgess 2018-09-08 6:27 ` Eli Zaretskii 2018-09-08 5:35 ` Philippe Waroquiers 2018-09-08 14:33 ` Andrew Burgess 2018-08-15 14:39 ` [PATCH 1/2] gdb: Make testnames unique in gdb.base/commands.exp Andrew Burgess 2018-08-30 15:26 ` Tom Tromey
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).