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

[Guinevere Larsen <guinevere@redhat.com>]

On 4/20/26 10:59 AM, Andrew Burgess wrote:
> Guinevere Larsen <guinevere@redhat.com> writes:
>
>> This commit adds a couple new rules to the YACC parser, that will allow
>> GDB to recognize expressions like [[N]]::var.
>>
>> This new syntax reuses 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
> typo: "cast it to its defined type".
>
> I went to check if the original error message contained the same error.
> It doesn't, but not does it match what you quote.  I think this is the
> error you are referencing, from typeprint.c:
>
>    void
>    error_unknown_type (const char *sym_print_name)
>    {
>      error (_("'%s' has unknown type; cast it to its declared type"),
>    	 sym_print_name);
>    }
Fixed.
>> 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.
>>
>> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
>> ---
>>   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    |  53 ++++++++
>>   11 files changed, 274 insertions(+), 64 deletions(-)
>>
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index 097d8da350b..273bd400f1b 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -227,6 +227,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.
>> +
>> +
> Double blank line.
fixed
>
>>   * GDB-internal Thread Local Storage (TLS) support
>>   
>>     ** Linux targets for the x86_64, aarch64, ppc64, s390x, and riscv
>> diff --git a/gdb/c-exp.y b/gdb/c-exp.y
>> index 9f1cb1b2e2a..cb70bf3c863 100644
>> --- a/gdb/c-exp.y
>> +++ b/gdb/c-exp.y
>> @@ -173,7 +173,7 @@ using namespace expr;
>>       struct ttype tsym;
>>       struct symtoken ssym;
>>       int voidval;
>> -    const struct block *bval;
>> +    struct bval_type bval;
>>       enum exp_opcode opcode;
>>   
>>       struct stoken_vector svec;
>> @@ -261,6 +261,7 @@ static void c_print_token (FILE *file, int type, YYSTYPE value);
>>   %token TYPEOF
>>   %token DECLTYPE
>>   %token TYPEID
>> +%token DOUBLESQUAREOPEN DOUBLESQUARECLOSE
>>   
>>   /* Special type cases, put in to allow the parser to distinguish different
>>      legal basetypes.  */
>> @@ -1056,30 +1057,42 @@ exp     :       FALSEKEYWORD
>>   
>>   block	:	BLOCKNAME
>>   			{
>> -			  if ($1.sym.symbol)
>> -			    $$ = $1.sym.symbol->value_block ();
>> -			  else
>> +			  if ($1.sym.symbol == nullptr)
>>   			    error (_("No file or function \"%s\"."),
>>   				   copy_name ($1.stoken).c_str ());
>> +			  $$.search_namespace = false;
>> +			  $$.b_val = $1.sym.symbol->value_block ();
>>   			}
>>   	|	FILENAME
>>   			{
>>   			  $$ = $1;
>>   			}
>> +	|	DOUBLESQUAREOPEN INT DOUBLESQUARECLOSE
>> +			{
>> +			    $$.search_namespace = true;
>> +			    $$.namespace_val = $2.val;
> Totally trivial issue, but the indentation here is different to at least
> everything in this 'block' rule, you use TABS + 4 whitespace, while
> everything else uses TABS + 2 whitespace.  Would be nice to be
> consistent.
fixed
>
>> +			}
>>   	;
>>   
>>   block	:	block COLONCOLON name
>>   			{
>>   			  std::string copy = copy_name ($3);
>> -			  struct symbol *tem
>> -			    = lookup_symbol (copy.c_str (), $1,
>> -					     SEARCH_FUNCTION_DOMAIN,
>> -					     nullptr).symbol;
>> +			  struct block_symbol tem;
>>   
>> -			  if (tem == nullptr)
>> +			  if ($1.search_namespace)
>> +			    tem = lookup_symbol_in_linker_namespace
>> +				(copy.c_str (), $1.namespace_val, SEARCH_FUNCTION_DOMAIN);
> lookup_symbol_in_linker_namespace takes an 'int' for the namespace-id,
> but namespace_val is LONGEST.  I suspect namespace_val needs to be
> LONGEST as it gets it value from an INT token, whose value is a LONGEST.
>
> But we should really have a check prior to the
> lookup_symbol_in_linker_namespace call that we're not narrowing the
> value in a meaningful way.
>
> Or just change the namespace-id argument to LONGEST throughout maybe?
>
> Either will work.
I changed the argument to be LONGEST instead, I don't have a good reason 
to keep it as an int anyway.
>
>
>> +			  else
>> +			    tem = lookup_symbol (copy.c_str (), $1.b_val,
>> +						 SEARCH_FUNCTION_DOMAIN,
>> +						 nullptr);
>> +
>> +			  if (tem.symbol == nullptr)
>>   			    error (_("No function \"%s\" in specified context."),
>>   				   copy.c_str ());
>> -			  $$ = tem->value_block (); }
>> +			  $$.b_val = tem.symbol->value_block ();
>> +			  $$.search_namespace = false;
>> +			}
>>   	;
>>   
>>   variable:	name_not_typename ENTRY
>> @@ -1098,9 +1111,15 @@ variable:	name_not_typename ENTRY
>>   variable:	block COLONCOLON name
>>   			{
>>   			  std::string copy = copy_name ($3);
>> -			  struct block_symbol sym
>> -			    = lookup_symbol (copy.c_str (), $1,
>> -					     SEARCH_VFT, NULL);
>> +			  struct block_symbol sym;
>> +
>> +			  if ($1.search_namespace)
>> +			    sym = lookup_symbol_in_linker_namespace
>> +				    (copy.c_str (), $1.namespace_val,
>> +				     SEARCH_VFT);
>> +			  else
>> +			    sym = lookup_symbol (copy.c_str (), $1.b_val,
>> +						 SEARCH_VFT, NULL);
>>   
>>   			  if (sym.symbol == 0)
>>   			    error (_("No symbol \"%s\" in specified context."),
>> @@ -1196,13 +1215,25 @@ variable:	name_not_typename
>>   			    {
>>   			      std::string arg = copy_name ($1.stoken);
>>   
>> -			      bound_minimal_symbol msymbol
>> -				= lookup_minimal_symbol (current_program_space, arg.c_str ());
>> +			      int active_linker_nss
>> +				= solib_linker_namespace_count
>> +				  (current_program_space);
>> +			      bound_minimal_symbol msymbol;
>> +
>> +			      if (active_linker_nss > 1)
>> +				msymbol = lookup_minimal_symbol_in_linker_namespace
>> +				  (current_program_space, arg.c_str ());
>> +			      else
>> +				msymbol = lookup_minimal_symbol
>> +				  (current_program_space, arg.c_str ());
>>   			      if (msymbol.minsym == NULL)
>>   				{
>>   				  if (!have_full_symbols (current_program_space)
>>   				      && !have_partial_symbols (current_program_space))
>>   				    error (_("No symbol table is loaded.  Use the \"file\" command."));
>> +				  else if (active_linker_nss > 1)
>> +				    error (_("No symbol \"%s\" in the current linker namespace."),
>> +					   arg.c_str ());
>>   				  else
>>   				    error (_("No symbol \"%s\" in current context."),
>>   					   arg.c_str ());
>> @@ -2540,7 +2571,9 @@ static const struct c_token tokentab2[] =
>>       {"!=", NOTEQUAL, OP_NULL, 0},
>>       {"<=", LEQ, OP_NULL, 0},
>>       {">=", GEQ, OP_NULL, 0},
>> -    {".*", DOT_STAR, OP_NULL, FLAG_CXX}
>> +    {".*", DOT_STAR, OP_NULL, FLAG_CXX},
>> +    {"[[", DOUBLESQUAREOPEN, OP_NULL, 0},
>> +    {"]]", DOUBLESQUARECLOSE, OP_NULL, 0},
>>     };
>>   
>>   /* Identifier-like tokens.  Only type-specifiers than can appear in
>> @@ -3143,8 +3176,9 @@ classify_name (struct parser_state *par_state, const struct block *block,
>>   	  if (auto symtab = lookup_symtab (current_program_space, copy.c_str ());
>>   	      symtab != nullptr)
>>   	    {
>> -	      yylval.bval
>> +	      yylval.bval.b_val
>>   		= symtab->compunit ()->blockvector ()->static_block ();
>> +	      yylval.bval.search_namespace = false;
>>   
>>   	      return FILENAME;
>>   	    }
>> @@ -3394,7 +3428,7 @@ yylex (void)
>>     name_obstack.clear ();
>>     checkpoint = 0;
>>     if (current.token == FILENAME)
>> -    search_block = current.value.bval;
>> +    search_block = current.value.bval.b_val;
>>     else if (current.token == COLONCOLON)
>>       search_block = NULL;
>>     else
>> @@ -3577,7 +3611,7 @@ c_print_token (FILE *file, int type, YYSTYPE value)
>>         break;
>>   
>>       case FILENAME:
>> -      parser_fprintf (file, "bval<%s>", host_address_to_string (value.bval));
>> +      parser_fprintf (file, "bval<%s>", host_address_to_string (value.bval.b_val));
>>         break;
>>       }
>>   }
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index 821f3ed4b05..b48f70d2250 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -10686,6 +10686,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 from the
>> +inferior's identifier.
> Instead of @code{...} I think a better choice would be @samp{...} here.
> My understanding is that @code would be for actual programming language
> code.
Fixed.
>> +
>>   @anchor{print options}
>>   The @code{print} command supports a number of options that allow
>>   overriding relevant global print settings as set by @code{set print}
>> diff --git a/gdb/minsyms.c b/gdb/minsyms.c
>> index 7e4f47b8c3f..a73d4f21858 100644
>> --- a/gdb/minsyms.c
>> +++ b/gdb/minsyms.c
>> @@ -335,7 +335,10 @@ lookup_minimal_symbol_demangled (const lookup_name_info &lookup_name,
>>       }
>>   }
>>   
>> -/* Look through all the current minimal symbol tables and find the
>> +/* Main implementation for the lookup_minimal_symbol function.
>> +   Receives a list of objfiles in which to look for the symbol.
>> +
>> +   Look through all the current minimal symbol tables and find the
>>      first minimal symbol that matches NAME.  If OBJF is non-NULL, limit
>>      the search to that objfile.  If SFILE is non-NULL, the only file-scope
>>      symbols considered will be from that source file (global symbols are
>> @@ -355,9 +358,9 @@ lookup_minimal_symbol_demangled (const lookup_name_info &lookup_name,
>>      Obviously, there must be distinct mangled names for each of these,
>>      but the demangled names are all the same: S::S or S::~S.  */
>>   
>> -bound_minimal_symbol
>> -lookup_minimal_symbol (program_space *pspace, const char *name, objfile *objf,
>> -		       const char *sfile)
>> +static bound_minimal_symbol
>> +lookup_minimal_symbol_in_objfiles (std::vector<objfile *> objfile_list,
>> +				   const char *name, const char *sfile)
>>   {
>>     found_minimal_symbols found;
>>   
>> @@ -373,53 +376,48 @@ lookup_minimal_symbol (program_space *pspace, const char *name, objfile *objf,
>>   
>>     lookup_name_info lookup_name (name, symbol_name_match_type::FULL);
>>   
>> -  for (objfile &objfile : pspace->objfiles ())
>> +  for (objfile *objfile : objfile_list)
>>       {
>>         if (found.external_symbol.minsym != NULL)
>>   	break;
>>   
>> -      if (objf == NULL || objf == &objfile
>> -	  || objf == objfile.separate_debug_objfile_backlink)
>> +      symbol_lookup_debug_printf ("lookup_minimal_symbol (%s, %s, %s)",
>> +				  name, sfile != NULL ? sfile : "NULL",
>> +				  objfile_debug_name (objfile));
>> +
>> +      /* Do two passes: the first over the ordinary hash table,
>> +	 and the second over the demangled hash table.  */
>> +      lookup_minimal_symbol_mangled (name, sfile, objfile,
>> +				     objfile->per_bfd->msymbol_hash,
>> +				     mangled_hash, mangled_cmp, found);
>> +
>> +      /* If not found, try the demangled hash table.  */
>> +      if (found.external_symbol.minsym == NULL)
>>   	{
>> -	  symbol_lookup_debug_printf ("lookup_minimal_symbol (%s, %s, %s, %s)",
>> -				      host_address_to_string (pspace),
>> -				      name, sfile != NULL ? sfile : "NULL",
>> -				      objfile_debug_name (&objfile));
>> -
>> -	  /* Do two passes: the first over the ordinary hash table,
>> -	     and the second over the demangled hash table.  */
>> -	  lookup_minimal_symbol_mangled (name, sfile, &objfile,
>> -					 objfile.per_bfd->msymbol_hash,
>> -					 mangled_hash, mangled_cmp, found);
>> -
>> -	  /* If not found, try the demangled hash table.  */
>> -	  if (found.external_symbol.minsym == NULL)
>> +	  /* Once for each language in the demangled hash names
>> +	     table (usually just zero or one languages).  */
>> +	  for (unsigned iter = 0; iter < nr_languages; ++iter)
>>   	    {
>> -	      /* Once for each language in the demangled hash names
>> -		 table (usually just zero or one languages).  */
>> -	      for (unsigned iter = 0; iter < nr_languages; ++iter)
>> -		{
>> -		  if (!objfile.per_bfd->demangled_hash_languages.test (iter))
>> -		    continue;
>> -		  enum language lang = (enum language) iter;
>> -
>> -		  unsigned int hash
>> -		    = (lookup_name.search_name_hash (lang)
>> -		       % MINIMAL_SYMBOL_HASH_SIZE);
>> -
>> -		  symbol_name_matcher_ftype *match
>> -		    = language_def (lang)->get_symbol_name_matcher
>> -							(lookup_name);
>> -		  struct minimal_symbol **msymbol_demangled_hash
>> -		    = objfile.per_bfd->msymbol_demangled_hash;
>> -
>> -		  lookup_minimal_symbol_demangled (lookup_name, sfile, &objfile,
>> -						   msymbol_demangled_hash,
>> -						   hash, match, found);
>> -
>> -		  if (found.external_symbol.minsym != NULL)
>> -		    break;
>> -		}
>> +	      if (!objfile->per_bfd->demangled_hash_languages.test (iter))
>> +		continue;
>> +	      enum language lang = (enum language) iter;
>> +
>> +	      unsigned int hash
>> +		= (lookup_name.search_name_hash (lang)
>> +		   % MINIMAL_SYMBOL_HASH_SIZE);
>> +
>> +	      symbol_name_matcher_ftype *match
>> +		= language_def (lang)->get_symbol_name_matcher
>> +						    (lookup_name);
>> +	      struct minimal_symbol **msymbol_demangled_hash
>> +		= objfile->per_bfd->msymbol_demangled_hash;
>> +
>> +	      lookup_minimal_symbol_demangled (lookup_name, sfile, objfile,
>> +					       msymbol_demangled_hash,
>> +					       hash, match, found);
>> +
>> +	      if (found.external_symbol.minsym != NULL)
>> +		break;
>>   	    }
>>   	}
>>       }
>> @@ -472,6 +470,42 @@ lookup_minimal_symbol (program_space *pspace, const char *name, objfile *objf,
>>     return {};
>>   }
>>   
>> +/* See minsyms.h.  */
>> +
>> +bound_minimal_symbol
>> +lookup_minimal_symbol (program_space *pspace, const char *name, objfile *objf,
>> +		       const char *sfile)
>> +{
>> +  std::vector<objfile *> search_objfiles;
>> +  if (objf != nullptr)
>> +    {
>> +      search_objfiles.push_back (objf);
>> +      if (objf->separate_debug_objfile_backlink != nullptr)
>> +	search_objfiles.push_back (objf->separate_debug_objfile_backlink);
> There's a change in behaviour here.  If we imagine that objfile A has a
> separate_debug_objfile of B.  The previous code looped over every
> objfile and did:
>
>        if (objf == NULL || objf == &objfile
> 	  || objf == objfile.separate_debug_objfile_backlink)
>          // Include this objfile.
>
> If OBJF was 'A' then we would include both 'A' and 'B' as B's backlink
> points at 'A'.
>
> However, the new code does:
>
>        search_objfiles.push_back (objf);
>        if (objf->separate_debug_objfile_backlink != nullptr)
> 	search_objfiles.push_back (objf->separate_debug_objfile_backlink);
>
> So if we pass 'A' then we include 'A', but A's backlink will be NULL, so
> we'll not search 'B'.
>
> Due to this flipping of the logic, then if we pass 'B' then under the
> old scheme we'd only include 'B', but under the new scheme we include
> both 'A' and 'B'.
>
> This change of behaviour wasn't called out in the commit message, so I'm
> assuming this might be a mistake.

So, the thinking here might be incorrect, but the idea was: My 
understanding was that this function is mostly called when GDB is 
iterating over many objfiles, and most likely we'll call this with both 
A and B anyway, so it wouldn't make much of a difference, and then this 
change make the code more readable in my opinion. So while there is a 
behavior change in the function itself, I didn't think there would be an 
actual change in the macro scale.

If my understanding is mistaken, please let me know and I'll revert to 
the previous behavior.

>
>> +    }
>> +  else
>> +    {
>> +      for (objfile &objfile : pspace->objfiles ())
>> +	search_objfiles.push_back (&objfile);
>> +    }
>> +
>> +  return lookup_minimal_symbol_in_objfiles (search_objfiles, name, sfile);
>> +}
>> +
>> +/* See minsyms.h.  */
>> +
>> +bound_minimal_symbol
>> +lookup_minimal_symbol_in_linker_namespace (program_space *pspace,
>> +					   const char *name)
>> +{
>> +  LONGEST curr_namespace;
>> +  get_internalvar_integer (lookup_internalvar ("_linker_namespace"),
>> +			   &curr_namespace);
> Rather than reading the internal variable, would it not be better to
> have a helper function in solib.{c,h} which returns the current
> namespace-id, then have the internal variable and this code just call
> that?
I didn't think it was too much worse, but now that you mention it, it 
would be much more readable for future maintainers, I'll do that, but in 
a separate commit so it's easier to bisect stuff :)
>
>> +  return lookup_minimal_symbol_in_objfiles
>> +      (get_objfiles_in_linker_namespace (curr_namespace, pspace),
>> +       name, nullptr);
>> +}
>> +
>>   /* See gdbsupport/symbol.h.  */
>>   
>>   int
>> diff --git a/gdb/minsyms.h b/gdb/minsyms.h
>> index 4aa8a428470..f2ba15d9318 100644
>> --- a/gdb/minsyms.h
>> +++ b/gdb/minsyms.h
>> @@ -211,6 +211,14 @@ bound_minimal_symbol lookup_minimal_symbol (program_space *pspace,
>>   					    objfile *obj = nullptr,
>>   					    const char *sfile = nullptr);
>>   
>> +/* Same as above, but only look for a minimal symbol in the objfiles that
>> +   belong to the current linker namespace.  This function is only meant to
>> +   be called when attempting to resolve a symbol from a user command, and
>> +   there are multiple linker namespaces available to resolve it in.  */
>> +
>> +bound_minimal_symbol lookup_minimal_symbol_in_linker_namespace
>> +  (program_space *pspace, const char *name);
>> +
>>   /* Look through all the minimal symbol tables in PSPACE and find the
>>      first minimal symbol that matches NAME and has text type.  If OBJF
>>      is non-NULL, limit the search to that objfile.  Returns a bound
>> diff --git a/gdb/parser-defs.h b/gdb/parser-defs.h
>> index f5618f3a9ce..ff4ab6a09de 100644
>> --- a/gdb/parser-defs.h
>> +++ b/gdb/parser-defs.h
>> @@ -358,6 +358,13 @@ struct ttype
>>       struct type *type;
>>     };
>>   
>> +struct bval_type
>> +  {
>> +    bool search_namespace;
>> +    const struct block *b_val;
>> +    LONGEST namespace_val;
>> +  };
> I know there are plenty of (older) structures in here that are missing
> comments, but there are also some, e.g. stoken, typed_stoken, that do
> have comments.  I think it would be better if new code did comment the
> structures that are added.
Added comments to the structure and fields.
>
>> +
>>   struct symtoken
>>     {
>>       struct stoken stoken;
>> diff --git a/gdb/symtab.c b/gdb/symtab.c
>> index 786dd9f56ba..d237a5cc5d2 100644
>> --- a/gdb/symtab.c
>> +++ b/gdb/symtab.c
>> @@ -2558,6 +2558,43 @@ lookup_static_symbol (const char *name, const domain_search_flags domain)
>>   
>>   /* See symtab.h.  */
>>   
>> +struct block_symbol
>> +lookup_symbol_in_linker_namespace (const char *name, int nsid,
>> +				   const domain_search_flags domain)
>> +{
>> +  if (!current_program_space->solib_ops ()->supports_namespaces ())
>> +    error (_("Linker namespaces are not supported by the inferior."));
>> +
>> +  std::vector<objfile *> objfiles_in_namespace
>> +    = get_objfiles_in_linker_namespace (nsid, current_program_space);
>> +
>> +  if (objfiles_in_namespace.size() == 0)
> Missing space before '()'.
Fixed.
>
>> +    error (_("Namespace [[%d]] is inactive"), nsid);
>> +
>> +  symbol_lookup_debug_printf ("lookup_symbol_in_linker_namespace (%d, %s, %s)",
>> +			      nsid, name, domain_name (domain).c_str ());
>> +
>> +  /* We look for both global and static symbols in here.  There is no reason
>> +     to pick one over the other to my knowledge, so we go alphabetical.  */
> I find the whole alphabetical thing a bit weird.  I'd just say:
>
>    /* We look for both global and static symbols in here, the order is
>       arbitrary.  */
>
> But if you prefer your wording then that's fine too I guess.
Changed to use your wording.
>> +  for (objfile *objf : objfiles_in_namespace)
>> +    {
>> +      struct block_symbol bsym
>> +	= lookup_global_symbol_from_objfile (objf, GLOBAL_BLOCK,
>> +					     name, domain);
>> +      if (bsym.symbol != nullptr)
>> +	return bsym;
>> +
>> +      bsym = lookup_global_symbol_from_objfile (objf, STATIC_BLOCK,
>> +						name, domain);
>> +      if (bsym.symbol != nullptr)
>> +	return bsym;
>> +    }
> When looking at this function, I started looking in c-exp.y, and in here
> we see that the two paths are lookup_symbol_in_linker_namespace or
> lookup_symbol, so when I reviewed this function I was comparing against
> lookup_symbol.
>
> lookup_symbol calls lookup_symbol_in_language, which uses
> demangle_for_lookup to demangle and canonicalize the string being
> searched for.  But lookup_symbol_in_linker_namespace doesn't call this
> function.  Isn't that going to be a problem?
Probably would be a problem, if the user used a mangled name. I'll add a 
call to it, it wasn't omitted on purpose.
> After demangle_for_lookup lookup_symbol_in_language calls
> lookup_symbol_aux to do the actual lookup.  This function does a bunch
> of local symbol lookup, which we probably don't need for the namespace
> searching path, but then does:
>
>    /* Now do whatever is appropriate for LANGUAGE to look
>       up static and global variables.  */
>
>    result = langdef->lookup_symbol_nonlocal (name, block, domain);
>
> to lookup the global/static symbols, this does a bunch of language
> specific stuff, especially for e.g. C++.  Yet
> lookup_symbol_in_linker_namespace doesn't call this, it just goes direct
> to the global/static block and looks for an exact match.
>
> Are there issues here?  Or is there something that I'm missing.  If this
> is a limitation then maybe this should be documented somewhere?

I thought that the language-specific stuff was to look for built-in 
types and other kinds of things that wouldn't be necessary if we are 
looking at something that is specific to a namespace.

I don't know if this will cause a problem or not, I can add it if I'm 
mistaken in what the langdef search does, just let me know.

>
>> +
>> +  return {};
>> +}
>> +
>> +/* See symtab.h.  */
>> +
>>   struct block_symbol
>>   lookup_global_symbol (const char *name,
>>   		      const struct block *block,
>> diff --git a/gdb/symtab.h b/gdb/symtab.h
>> index e47033efd01..fe2304acb4f 100644
>> --- a/gdb/symtab.h
>> +++ b/gdb/symtab.h
>> @@ -2737,6 +2737,14 @@ extern struct block_symbol
>>   				     const char *name,
>>   				     const domain_search_flags domain);
>>   
>> +/* Lookup symbol NAME from DOMAIN in the linker namespace NSID.
>> +   This generates a list of all objfiles in NSID, then searches
>> +   those objfiles for the given symbol.  Searches for both global or
>> +   static symbols.  */
>> +extern struct block_symbol
>> +  lookup_symbol_in_linker_namespace (const char *name, int nsid,
>> +				     const domain_search_flags domain);
>> +
>>   extern unsigned int symtab_create_debug;
>>   
>>   /* Print a "symtab-create" debug statement.  */
>> diff --git a/gdb/testsuite/gdb.base/dlmopen-ns-ids-lib.c b/gdb/testsuite/gdb.base/dlmopen-ns-ids-lib.c
>> index 86cbb0f4e38..cc35ce0080e 100644
>> --- a/gdb/testsuite/gdb.base/dlmopen-ns-ids-lib.c
>> +++ b/gdb/testsuite/gdb.base/dlmopen-ns-ids-lib.c
>> @@ -26,3 +26,17 @@ inc (int n)
>>     int amount = gdb_dlmopen_glob;
>>     return n + amount;  /* bp.inc.  */
>>   }
>> +
>> +static int
>> +change_global (int n)
>> +{
>> +  gdb_dlmopen_glob += n;
>> +  return n;
>> +}
>> +
>> +__attribute__((visibility ("default")))
>> +int
>> +func_with_other_call (int n)
>> +{
>> +  return change_global (n + 1);
>> +}
>> diff --git a/gdb/testsuite/gdb.base/dlmopen-ns-ids-main.c b/gdb/testsuite/gdb.base/dlmopen-ns-ids-main.c
>> index c7c038a08d1..459ae547842 100644
>> --- a/gdb/testsuite/gdb.base/dlmopen-ns-ids-main.c
>> +++ b/gdb/testsuite/gdb.base/dlmopen-ns-ids-main.c
>> @@ -47,6 +47,9 @@ main (void)
>>         fun (dl);
>>       }
>>   
>> +  fun = dlsym (handle[0], "func_with_other_call");
>> +  fun (0);
>> +
>>     dlclose (handle[0]); /* TAG: first dlclose */
>>     dlclose (handle[1]); /* TAG: second dlclose */
>>     dlclose (handle[2]); /* TAG: third dlclose */
>> diff --git a/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp b/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp
>> index f3bdfb054d9..fec7adb5317 100644
>> --- a/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp
>> +++ b/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp
>> @@ -123,6 +123,10 @@ proc get_first_so_ns {} {
>>       return $ns
>>   }
>>   
>> +proc ns_id_for_command {ns} {
>> +    return "\[\[$ns\]\]"
>> +}
>> +
>>   # Run the tests relating to the command "info sharedlibrary", to
>>   # verify that the namespace ID is consistent.
>>   proc test_info_shared {} {
>> @@ -315,6 +319,55 @@ proc test_info_linker_namespaces {} {
>>   		    ".*" ] "print namespaces with no argument"
>>   }
>>   
>> +# Test that we can use the [[N]]::symbol syntax, and it's variations
>> +# like [[N]]::func::symbol.
> typo: "and its variations like..."
Fixed.

-- 
Cheers,
Guinevere Larsen
It/she

> Thanks,
> Andrew
>
>> +proc_with_prefix test_print_namespace_symbol {} {
>> +    clean_restart
>> +    gdb_load $::binfile
>> +
>> +    set ns1 [ns_id_for_command 1]
>> +    set ns2 [ns_id_for_command 2]
>> +
>> +    if { ![runto_main] } {
>> +	return
>> +    }
>> +
>> +    gdb_breakpoint [gdb_get_line_number "TAG: first dlclose"]
>> +    gdb_breakpoint "change_global" allow-pending
>> +
>> +    # Test printing variables for non-existent namespace.
>> +    gdb_test "print ${ns1}::gdb_dlmopen_glob" "Namespace ..1.. is inactive" \
>> +	"Before loading namespaces"
>> +
>> +    gdb_continue_to_breakpoint "change_global"
>> +
>> +    # Test finding location of variables.
>> +    gdb_test "print ${ns1}::gdb_dlmopen_glob" ".* = 0" "before changing"
>> +    gdb_test_no_output "set ${ns1}::gdb_dlmopen_glob = 1"
>> +    gdb_test "print ${ns1}::gdb_dlmopen_glob" ".* = 1" "after changing"
>> +    gdb_test "print ${ns2}::gdb_dlmopen_glob" ".* = 0" "other namespace"
>> +
>> +    gdb_test "print gdb_dlmopen_glob" ".* = 1" "changed the right variable"
>> +
>> +    # Test finding functions in specific namespaces.
>> +    gdb_test "print ${ns1}::inc (2)" ".* = 3"
>> +    gdb_test "print ${ns2}::inc (2)" ".* = 2"
>> +
>> +    # Test finding a specific block in the backtrace.
>> +    gdb_test "print n" ".* = 1"
>> +    gdb_test "print ${ns1}::func_with_other_call::n" ".* = 0"
>> +    gdb_test "print ${ns2}::func_with_other_call::n" \
>> +	"No frame is currently executing in block func_with_other_call."
>> +
>> +    # Leave to default namespace.
>> +    gdb_continue_to_breakpoint "TAG: first dlclose"
>> +    # This global doesn't exist in the default namespace.  Rather than
>> +    # 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."
>> +}
>> +
>>   test_info_shared
>>   test_conv_vars
>>   test_info_linker_namespaces
>> +test_print_namespace_symbol
>> -- 
>> 2.51.0