Re: [PATCH v6 3/3] gdb: extend the [[N]]::foo syntax for files

[Guinevere Larsen <guinevere@redhat.com>]

On 4/20/26 12:13 PM, Andrew Burgess wrote:
> Guinevere Larsen <guinevere@redhat.com> writes:
>
>> This commit implements the missing support for [[N]]::'file.c'::var
>> syntax that was skipped on the previous commit.
>>
>> This is done by adding a new value to the global parser_state, so that
>> the classify_name function can restrict its search for file names to the
>> specified linker namespace.  It had to be done this way because if the
>> logic was contained on the newly added "block: block COLONCOLON
>> FILENAME" rule, we would not have the name to rerun the search.
> Did you consider adding the filename to the <whatever> type such that it
> was available within the rule to allow for the filename to be re-looked
> up?
>
> I'd be interested to know if this was tried, why this was worse than
> pushing parser state back to the lexer, which I always thought was not a
> great design.
I can't remember having tried it, so I'll explore this option and report 
back.
>>                                                                   Because
>> of that, the new rule only exists to make sure this is only used in the
>> intended syntax.  This commit also adds a link to the solib_ops relevant
>> to the expression being parsed, as a way to identify if linker
>> namespaces are supported; this could be achieved by guarding the search
>> functions instead, but I think that this is more reliable.
>>
>> The new rule uses "block" rather than a linker namespace specific token,
>> because if we allowed that token to also reduce into block, the new rule
>> would never be triggered as the simple reduction would be preferred. If
>> we didn't allow the linker namespace to reduce into block, the rest of
>> the syntaxes for [[N]]::foo would be more involved, so the option in
>> this commit seemed like the simplest solution.
>>
>> The rule "block: block COLONCOLON name" also had to be updated because,
>> if a user tried to search for a file not present in the linker
>> namespace, GDB would miscategorize it to 'name' instead of 'filename',
>> and if no function of the same name was found, the error would only say
>> "No function \"foo\" (...)" which could confuse a user. The new error
>> says "Nothing named \"foo\" (...)" instead since it is impossible for us
>> to disambiguate between filename or function name, and both would be
>> valid in this code path.
>>
>> The parser_state changes were not used for the previous commit because
>> the lookup_symbol calls would lead to an even more intrusive change for
>> no real gain, since the search would be rerun any way. There is an
>> argument to be made that the results could be different if only
>> namespace had a type and another had a variable, but a situation where
>> that significantly changes the result is quite unlikely, I think.
>> ---
>>   gdb/c-exp.y                               | 18 +++++++++++--
>>   gdb/linespec.c                            |  4 +--
>>   gdb/parse.c                               |  3 ++-
>>   gdb/parser-defs.h                         | 32 +++++++++++++++++++++--
>>   gdb/rust-parse.c                          |  3 ++-
>>   gdb/symtab.c                              | 26 +++++++++++++-----
>>   gdb/symtab.h                              |  7 ++++-
>>   gdb/testsuite/gdb.base/dlmopen-ns-ids.exp | 12 +++++++++
>>   8 files changed, 90 insertions(+), 15 deletions(-)
>>
>> diff --git a/gdb/c-exp.y b/gdb/c-exp.y
>> index cb70bf3c863..f59aca327ed 100644
>> --- a/gdb/c-exp.y
>> +++ b/gdb/c-exp.y
>> @@ -1071,6 +1071,7 @@ block	:	BLOCKNAME
>>   			{
>>   			    $$.search_namespace = true;
>>   			    $$.namespace_val = $2.val;
>> +			    pstate->set_linker_namespace ($2.val);
>>   			}
>>   	;
>>   
>> @@ -1087,12 +1088,24 @@ block	:	block COLONCOLON name
>>   						 SEARCH_FUNCTION_DOMAIN,
>>   						 nullptr);
>>   
>> -			  if (tem.symbol == nullptr)
>> +			  if (tem.symbol == nullptr && !$$.search_namespace)
>>   			    error (_("No function \"%s\" in specified context."),
>>   				   copy.c_str ());
>> +			  else if (tem.symbol == nullptr && $$.search_namespace)
> My yacc foo is weak, but isn't '$$' the rule's output?  Which is only
> being set below.  Shouldn't this be $1 maybe?  If there's not some
> reason why search_namespace is being set to the correct value (that I'm
> missing) then this would seem to indicate that there's a gap in the
> testing here.
You're right, this must ave slipped past me at some point, thanks!
>
>> +			    /* COPY can be a function or a file.  There is no way
>> +			       to identify which the user intended, so emit a
>> +			       generic warning instead.  */
>> +			    error (_("Nothing named \"%s\" in specified context."),
>> +				   copy.c_str ());
>>   			  $$.b_val = tem.symbol->value_block ();
>>   			  $$.search_namespace = false;
>>   			}
>> +	|	block COLONCOLON FILENAME
>> +			{
>> +			    if (!$1.search_namespace)
>> +				error (_("Filename must be the first part of the expression"));
>> +			    $$ = $3;
>> +			}
>>   	;
>>   
>>   variable:	name_not_typename ENTRY
>> @@ -3173,7 +3186,8 @@ classify_name (struct parser_state *par_state, const struct block *block,
>>   	  || is_quoted_name)
>>   	{
>>   	  /* See if it's a file name. */
>> -	  if (auto symtab = lookup_symtab (current_program_space, copy.c_str ());
>> +	  if (auto symtab = lookup_symtab (current_program_space, copy.c_str (),
>> +					   par_state->get_linker_namespace ());
>>   	      symtab != nullptr)
>>   	    {
>>   	      yylval.bval.b_val
>> diff --git a/gdb/linespec.c b/gdb/linespec.c
>> index 08f7fdd0daa..ec90ad4826a 100644
>> --- a/gdb/linespec.c
>> +++ b/gdb/linespec.c
>> @@ -3633,11 +3633,11 @@ collect_symtabs_from_filename (const char *file,
>>   	  if (pspace->executing_startup)
>>   	    continue;
>>   
>> -	  iterate_over_symtabs (pspace, file, collector);
>> +	  iterate_over_symtabs (pspace, file, -1, collector);
>>   	}
>>       }
>>     else
>> -    iterate_over_symtabs (search_pspace, file, collector);
>> +    iterate_over_symtabs (search_pspace, file, -1, collector);
>>   
>>     /* It is tempting to use the unordered_dense 'extract' method here,
>>        and remove the separate vector -- but it's unclear if ordering
>> diff --git a/gdb/parse.c b/gdb/parse.c
>> index d76768f41d7..7358c369d52 100644
>> --- a/gdb/parse.c
>> +++ b/gdb/parse.c
>> @@ -420,7 +420,8 @@ parse_exp_in_context (const char **stringptr, CORE_ADDR pc,
>>   
>>     parser_state ps (lang, get_current_arch (), expression_context_block,
>>   		   expression_context_pc, flags, *stringptr,
>> -		   completer != nullptr, tracker);
>> +		   completer != nullptr, tracker,
>> +		   current_program_space->solib_ops ());
>>   
>>     scoped_restore_current_language lang_saver (lang->la_language);
>>   
>> diff --git a/gdb/parser-defs.h b/gdb/parser-defs.h
>> index ff4ab6a09de..c0be1083f66 100644
>> --- a/gdb/parser-defs.h
>> +++ b/gdb/parser-defs.h
>> @@ -26,6 +26,7 @@
>>   #include "expression.h"
>>   #include "symtab.h"
>>   #include "expop.h"
>> +#include "solib.h"
>>   
>>   struct block;
>>   struct language_defn;
>> @@ -147,7 +148,8 @@ struct parser_state : public expr_builder
>>   		parser_flags flags,
>>   		const char *input,
>>   		bool completion,
>> -		innermost_block_tracker *tracker)
>> +		innermost_block_tracker *tracker,
>> +		const solib_ops *ops)
>>       : expr_builder (lang, gdbarch),
>>         expression_context_block (context_block),
>>         expression_context_pc (context_pc),
>> @@ -157,7 +159,8 @@ struct parser_state : public expr_builder
>>         comma_terminates ((flags & PARSER_COMMA_TERMINATES) != 0),
>>         parse_completion (completion),
>>         void_context_p ((flags & PARSER_VOID_CONTEXT) != 0),
>> -      debug ((flags & PARSER_DEBUG) != 0)
>> +      debug ((flags & PARSER_DEBUG) != 0),
>> +      m_solib_ops (ops)
>>     {
>>     }
>>   
>> @@ -263,6 +266,21 @@ struct parser_state : public expr_builder
>>       push (expr::make_operation<T> (std::move (lhs), std::move (rhs)));
>>     }
>>   
>> +  void set_linker_namespace (LONGEST ns_id)
>> +  {
>> +    if (m_solib_ops == nullptr)
>> +      error (_("Linker namespaces require an active inferior"));
>> +    if (m_solib_ops->supports_namespaces ())
>> +      linker_namespace = ns_id;
>> +    else
>> +      error (_("Linker namespaces are not supported"));
>> +  }
>> +
>> +  LONGEST get_linker_namespace ()
> This can be const.
Fixed, just in case I still need to use the parser state in the lexer.
>
>> +  {
>> +    return linker_namespace;
>> +  }
>> +
>>     /* Function called from the various parsers' yyerror functions to throw
>>        an error.  The error will include a message identifying the location
>>        of the error within the current expression.  */
>> @@ -323,6 +341,16 @@ struct parser_state : public expr_builder
>>   
>>     /* Stack of operations.  */
>>     std::vector<expr::operation_up> m_operations;
>> +
>> +  /* If the expression is being restricted to a specific namespace, this is
>> +     where that information is stored for the block lookup.  It should be
>> +     accessed through setter/getters to ensure that the linker namespace is
>> +     only set when the gdbarch supports it.  */
>> +  LONGEST linker_namespace = -1;
> This should be renamed m_linker_namespace as it is private.
>
> Also, I couldn't find anything that would reset the linker namepsace
> back to -1 when parsing a complex expression.  For example, something
> like:
>
>    print [[1]]::'foo.c'::var_a + 'bar.c'::var_b
>
> After parsing the [[1]] linker_namespace will be set to '1', but is it
> ever reset?  Will we not end up parsing the "'bar.c'::var_b" as if the
> lookup had to be in namespace 1?

You're right, I think I missed it as a possibility.

Hopefully I'll be able to move away from state in the lexer, and that 
won't be a concern.

>
> Thanks,
> Andrew
>
>> +
>> +  /* Used to figure out if an inferior is capable of handling linker
>> +     namespaces at all.  */
>> +  const solib_ops *m_solib_ops;
>>   };
>>   
>>   /* A string token, either a char-string or bit-string.  Char-strings are
>> diff --git a/gdb/rust-parse.c b/gdb/rust-parse.c
>> index b740d5a7b16..27f03b23e6a 100644
>> --- a/gdb/rust-parse.c
>> +++ b/gdb/rust-parse.c
>> @@ -2315,7 +2315,8 @@ rust_lex_tests (void)
>>   {
>>     /* Set up dummy "parser", so that rust_type works.  */
>>     parser_state ps (language_def (language_rust), current_inferior ()->arch (),
>> -		   nullptr, 0, 0, nullptr, 0, nullptr);
>> +		   nullptr, 0, 0, nullptr, 0, nullptr,
>> +		   current_program_space->solib_ops ());
>>     rust_parser parser (&ps);
>>   
>>     rust_lex_test_one (&parser, "", 0);
>> diff --git a/gdb/symtab.c b/gdb/symtab.c
>> index d237a5cc5d2..bbaf94e51d0 100644
>> --- a/gdb/symtab.c
>> +++ b/gdb/symtab.c
>> @@ -618,6 +618,7 @@ compare_filenames_for_search (const char *filename, const char *search_name)
>>   
>>   void
>>   iterate_over_symtabs (program_space *pspace, const char *name,
>> +		      LONGEST linker_ns,
>>   		      gdb::function_view<bool (symtab *)> callback)
>>   {
>>     gdb::unique_xmalloc_ptr<char> real_path;
>> @@ -630,20 +631,33 @@ iterate_over_symtabs (program_space *pspace, const char *name,
>>         gdb_assert (IS_ABSOLUTE_PATH (real_path.get ()));
>>       }
>>   
>> -  for (objfile &objfile : pspace->objfiles ())
>> -    if (objfile.map_symtabs_matching_filename (name, real_path.get (),
>> -					       callback))
>> +  std::vector<objfile *> objfiles_to_search;
>> +  if (linker_ns >= 0)
>> +    {
>> +      gdb_assert (pspace->solib_ops ()->supports_namespaces ());
>> +      objfiles_to_search
>> +	= get_objfiles_in_linker_namespace (linker_ns, pspace);
>> +    }
>> +  else
>> +    {
>> +      for (objfile &objf : pspace->objfiles ())
>> +	objfiles_to_search.push_back (&objf);
>> +    }
>> +
>> +  for (objfile *objfile : objfiles_to_search)
>> +    if (objfile->map_symtabs_matching_filename (name, real_path.get (),
>> +						callback))
>>         return;
>>   }
>>   
>>   /* See symtab.h.  */
>>   
>>   symtab *
>> -lookup_symtab (program_space *pspace, const char *name)
>> +lookup_symtab (program_space *pspace, const char *name, LONGEST linker_ns)
>>   {
>>     struct symtab *result = NULL;
>>   
>> -  iterate_over_symtabs (pspace, name, [&] (symtab *symtab)
>> +  iterate_over_symtabs (pspace, name, linker_ns, [&] (symtab *symtab)
>>       {
>>         result = symtab;
>>         return true;
>> @@ -6158,7 +6172,7 @@ collect_file_symbol_completion_matches (completion_tracker &tracker,
>>   
>>     /* Go through symtabs for SRCFILE and check the externs and statics
>>        for symbols which match.  */
>> -  iterate_over_symtabs (current_program_space, srcfile, [&] (symtab *s)
>> +  iterate_over_symtabs (current_program_space, srcfile, -1, [&] (symtab *s)
>>       {
>>         add_symtab_completions (s->compunit (),
>>   			      tracker, mode, lookup_name,
>> diff --git a/gdb/symtab.h b/gdb/symtab.h
>> index fe2304acb4f..bda6e82da02 100644
>> --- a/gdb/symtab.h
>> +++ b/gdb/symtab.h
>> @@ -2079,7 +2079,8 @@ const char *multiple_symbols_select_mode (void);
>>   
>>   /* Lookup a symbol table in PSPACE by source file name.  */
>>   
>> -extern symtab *lookup_symtab (program_space *pspace, const char *name);
>> +extern symtab *lookup_symtab (program_space *pspace, const char *name,
>> +			      LONGEST linker_ns = -1);
>>   
>>   /* An object of this type is passed as the 'is_a_field_of_this'
>>      argument to lookup_symbol and lookup_symbol_in_language.  */
>> @@ -2805,10 +2806,14 @@ bool compare_glob_filenames_for_search (const char *filename,
>>      psymtabs.  *If* there is no '/' in the name, a match after a '/' in the
>>      symtab filename will also work.
>>   
>> +   If LINKER_NS is 0 or greater, only the objfiles in the provided linker
>> +   namespace will be iterated over.
>> +
>>      Call CALLBACK with each symtab that is found.  If CALLBACK returns
>>      true, the search stops.  */
>>   
>>   void iterate_over_symtabs (program_space *pspace, const char *name,
>> +			   LONGEST linker_ns,
>>   			   gdb::function_view<bool (symtab *)> callback);
>>   
>>   std::vector<const linetable_entry *> find_linetable_entries_for_symtab_line
>> diff --git a/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp b/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp
>> index fec7adb5317..a1e18fc39d3 100644
>> --- a/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp
>> +++ b/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp
>> @@ -325,9 +325,15 @@ proc_with_prefix test_print_namespace_symbol {} {
>>       clean_restart
>>       gdb_load $::binfile
>>   
>> +    set ns0 [ns_id_for_command 0]
>>       set ns1 [ns_id_for_command 1]
>>       set ns2 [ns_id_for_command 2]
>>   
>> +    # Test printing variables before starting the inferior
>> +    gdb_test "print ${ns1}::gdb_dlmopen_glob" \
>> +	"Linker namespaces require an active inferior" \
>> +	"Before starting inferior"
>> +
>>       if { ![runto_main] } {
>>   	return
>>       }
>> @@ -365,6 +371,12 @@ proc_with_prefix test_print_namespace_symbol {} {
>>       # returning a random one, we just say we didn't find one.
>>       gdb_test "print gdb_dlmopen_glob" \
>>   	"No symbol .gdb_dlmopen_glob. in the current linker namespace."
>> +
>> +    # Minimal testing for finding files in namespaces.
>> +    gdb_test "print ${ns1}::'${::srcfile_lib}'::gdb_dlmopen_glob" \
>> +	".* = 2"
>> +    gdb_test "print ${ns0}::'${::srcfile_lib}'::gdb_dlmopen_glob" \
>> +	"Nothing named .${::srcfile_lib}. in specified context."
>>   }
>>   
>>   test_info_shared
>> -- 
>> 2.51.0


-- 
Cheers,
Guinevere Larsen
It/she