[PATCH] gdb: improve command completion for 'print', 'x', and 'display'
Simon Marchi
simark@simark.ca
Fri Nov 27 14:04:23 GMT 2020
On 2020-11-27 6:13 a.m., Andrew Burgess wrote:
> * Simon Marchi <simark@simark.ca> [2020-11-26 14:12:34 -0500]:
>
>> On 2020-11-16 10:42 a.m., Andrew Burgess wrote:
>>> The /FMT specification on the print command currently breaks command
>>> completion, so:
>>>
>>> (gdb) p var.<TAB><TAB>
>>> .... list of fields in var .....
>>>
>>> But,
>>>
>>> (gdb) p/d var.<TAB><TAB>
>>> ..... list of all symbols .....
>>>
>>> After this commit this issue is now resolved.
>>>
>>> There are some other details around tab-completion and /FMT which
>>> hopefully this commit improves. So, before:
>>>
>>> (gdb) p/<TAB><TAB>
>>> .... lists all symbols .....
>>>
>>> After:
>>>
>>> (gdb) p/<TAB><TAB> # Nothing changes...
>>>
>>> The thinking here is that after a / the user must type a FMT, but we
>>> don't offer tab completion on FMT characters. Placing a symbol
>>> directly after a / will not do what the user expects, so offering that
>>> seems wrong.
>>>
>>> Similarly, before we had:
>>>
>>> (gdb) p/d<TAB><TAB>
>>> ... lists all symbols starting with 'd' ....
>>>
>>> But afterwards:
>>>
>>> (gdb) p/d<TAB><TAB> # Adds a single space, so we get:
>>> (gdb) p/d <CURSOR>
>>>
>>> As before, typing a symbol where FMT is expected will not do what the
>>> user expects. If the user has added a FMT string then upon tab
>>> completion GDB assumes the FMT string is complete and prepares the
>>> user to type an expression.
>>>
>>> In this commit I have also added completion functions for the 'x' and
>>> 'display' commands. These commands also support /FMT specifiers and
>>> so share some code with 'print'.
>>>
>>> gdb/ChangeLog:
>>>
>>> * printcmd.c: Include 'safe-ctype.c'.
>>> (skip_over_slash_fmt): New function.
>>> (print_command_completer): Call skip_over_slash_fmt.
>>> (display_and_x_command_completer): New function.
>>> (_initialize_printcmd): Add command completion for 'x' and
>>> 'display'.
>>>
>>> gdb/testsuite/ChangeLog:
>>>
>>> * gdb.base/completion.exp: Add new tests.
>>> ---
>>> gdb/ChangeLog | 9 +++
>>> gdb/printcmd.c | 81 ++++++++++++++++++++++++++-
>>> gdb/testsuite/ChangeLog | 4 ++
>>> gdb/testsuite/gdb.base/completion.exp | 26 +++++++++
>>> 4 files changed, 118 insertions(+), 2 deletions(-)
>>
>> I noticed this warning in the code added by this patch, I think it's not a false positive:
>>
>> /home/simark/src/binutils-gdb/gdb/printcmd.c: In function ‘bool skip_over_slash_fmt(completion_tracker&, const char**)’:
>> /home/simark/src/binutils-gdb/gdb/printcmd.c:1285:14: warning: ‘in_fmt’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>> 1285 | return in_fmt;
>> | ^~~~~~
>
> Thanks for bringing this to my attention. I plan to push the patch
> below to resolve this issue.
>
> Thanks,
> Andrew
>
> ---
>
> commit 7ab525f25d2918c6e4073b346107b8730f07b0b9
> Author: Andrew Burgess <andrew.burgess@embecosm.com>
> Date: Fri Nov 27 10:46:07 2020 +0000
>
> gdb: fix potentially uninitialised variable
>
> In commit:
>
> commit 037d7135de575c9e0c20e9158c105979bfee339c
> Date: Mon Nov 16 11:36:56 2020 +0000
>
> gdb: improve command completion for 'print', 'x', and 'display'
>
> A potential use of an uninitialised variable was introduced. This is
> fixed in this commit.
>
> Previously when analysing /FMT strings for tab completion we
> considered two possibilities, either the user has typed '/', or the
> user has typed '/' followed by an alpha-numeric character, as these
> are the only valid FMT string characters.
>
> This meant that if the user type, for example '/@' and then tried to
> tab complete gdb would use an uninitialised variable.
>
> Currently only the first character after the '/' is checked to see if
> it is alpha-numeric, so if a user typed '/x@@' then gdb would be happy
> to treat this as a FMT string.
>
> Given the goal of this change was primarily to allow tab completion of
> symbols later in the command when a /FMT was used then I decided to
> just make the /FMT skipping less smart. Now any characters after the
> '/' up to the first white space, will be treated as a FMT string.
>
> gdb/ChangeLog:
>
> * printcmd.c (skip_over_slash_fmt): Reorder code to ensure in_fmt
> is always initialized.
>
> diff --git a/gdb/printcmd.c b/gdb/printcmd.c
> index a9c64b97c81..e95b8802950 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -1258,27 +1258,38 @@ skip_over_slash_fmt (completion_tracker &tracker, const char **args)
> bool in_fmt;
> tracker.set_use_custom_word_point (true);
>
> - if (ISALNUM (text[1]) || ISSPACE (text[1]))
> + if (text[1] == '\0')
> {
> - /* Skip over the actual format specification. */
> + /* The user tried to complete after typing just the '/' character
> + of the /FMT string. Step the completer past the '/', but we
> + don't offer any completions. */
> + in_fmt = true;
> + ++text;
> + }
> + else
> + {
> + /* The user has typed some characters after the '/', we assume
> + this is a complete /FMT string, first skip over it. */
> text = skip_to_space (text);
>
> if (*text == '\0')
> {
> + /* We're at the end of the input string. The user has typed
> + '/FMT' and asked for a completion. Push an empty
> + completion string, this will cause readline to insert a
> + space so the user now has '/FMT '. */
> in_fmt = true;
> tracker.add_completion (make_unique_xstrdup (text));
> }
> else
> {
> + /* The user has already typed things after the /FMT, skip the
> + whitespace and return false. Whoever called this function
> + should then try to complete what comes next. */
> in_fmt = false;
> text = skip_spaces (text);
> }
> }
> - else if (text[1] == '\0')
> - {
> - in_fmt = true;
> - ++text;
> - }
>
> tracker.advance_custom_word_point_by (text - *args);
> *args = text;
>
I don't really know about how completion work, but based on the (very
useful) comments you put, it makes sense.
Thanks!
Simon
More information about the Gdb-patches
mailing list