Re: [PATCH v3 2/3] gdb: Make the parser recognize the [[N]] syntax for variables

[Guinevere Larsen <guinevere@redhat.com>]

On 6/20/25 3:57 AM, Eli Zaretskii wrote:
>> From: Guinevere Larsen <guinevere@redhat.com>
>> Cc: Guinevere Larsen <guinevere@redhat.com>
>> Date: Thu, 19 Jun 2025 15:05:39 -0300
>>
>> This commit adds a couple new rules to the YACC parser, that will allow
>> GDB to recognize expressions like [[N]]::var.
>>
>> This new syntax re-uses the existing rules for searching for "block"s,
>> because it allows, with minimal changes to the .y file as a whole, while
>> also allowing [[N]]::function::symbol work. Notably, [[N]]::'file.c'::var
>> does not work yet, as this code change is also involved and would pollute
>> the current commit, so will be done in a separate one in the same series.
>>
>> To restrict the search to a desired namespace, a new function is
>> introduced,  lookup_symbol_in_linker_namespace.  This function will only
>> perform the symbol lookup in the objfiles that belong to the requested
>> namespace.
>>
>> This commit also changes the error message when "print var" if "var" is
>> only present in namespaces other than the current one. Before this commit,
>> lookup_minimal_symbol would find them as "external" symbols with
>> incomplete information - outputting "<symbol> has incomplete
>> information, cast it to it's defined type." - which could be confusing for
>> the users. This commit changes the parser rule "variable: name_not_typename"
>> so that, if multiple namespaces are active, we'll look for minimal symbols
>> only in the current namespace.
>>
>> Known limitations:
>> * When using [[N]]::foo::var, if that specific foo version is not on the
>>    stack, the error will be unable to include the linker namespace
>>    requested.  So if a user sees "foo" in the stack this could be
>>    confusing, but it should be mitigated when a future patch adds the
>>    namespace ID to the backtrace where applicable.
>> * Completion is not supported.  That's because the function used to
>>    complete symbols expects only text in the form "file.c:symbol", so
>>    can't handle the double colon.  This isn't unique to linker
>>    namespaces, though, since func::var will also not complete, it is just
>>    a quirk of how :: completion work at this point.
>> * The expression that parses [[N]] only takes an integer.  That's
>>    because there aren't many expressions that would make sense in there,
>>    and the work of accepting an arbitrary expression felt too much for
>>    little gain.
>> ---
>>   gdb/NEWS                                     |   6 +
>>   gdb/c-exp.y                                  |  72 ++++++++---
>>   gdb/doc/gdb.texinfo                          |   6 +
>>   gdb/minsyms.c                                | 124 ++++++++++++-------
>>   gdb/minsyms.h                                |   8 ++
>>   gdb/parser-defs.h                            |   7 ++
>>   gdb/symtab.c                                 |  37 ++++++
>>   gdb/symtab.h                                 |   8 ++
>>   gdb/testsuite/gdb.base/dlmopen-ns-ids-lib.c  |  14 +++
>>   gdb/testsuite/gdb.base/dlmopen-ns-ids-main.c |   3 +
>>   gdb/testsuite/gdb.base/dlmopen-ns-ids.exp    |  47 +++++++
>>   11 files changed, 268 insertions(+), 64 deletions(-)
> Thanks.
>
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -102,6 +102,12 @@ info threads [-gid] [-stopped] [-running] [ID]...
>>     These new flags can be useful to get a reduced list when there is a
>>     large number of threads.
>>   
>> +print
>> +  The print command now accepts the following syntax to print values
>> +  from a specific linker namespace: `[[N]]::foo'.  N is the namespace
>> +  identifier as understood by GDB.
> This part is okay.
>
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -10565,6 +10565,12 @@ you can choose a different format by specifying @samp{/@var{f}}, where
>>   @var{f} is a letter specifying the format; see @ref{Output Formats,,Output
>>   Formats}.
>>   
>> +Additionally, if the inferior supports linker namespaces, @value{GDBN}
>> +has implemented the following syntax to allow users to resolve symbols
>> +to a specific namespace: @code{[[@var{n}]]::@var{symbol}}.  Here @var{n}
>> +is @value{GDBN}'s namespace identifier, which may be different to the
>> +inferior's identifier.
> This part is also okay, but please say "different from" in the last
> sentence.
>
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
>
Thanks for the review (and sorry for the delay). I fixed this locally

-- 
Cheers,
Guinevere Larsen
She/Her/Hers