Re: [PATCH 2/2] gdb/c++: Detect ambiguous variables in imported namespaces

[Andrew Burgess <aburgess@redhat.com>]

Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:

> When running gdb.cp/nsusing.cc and stopping at line 17, we can ask GDB
> to print x and get a compiler-dependent answer. Using gcc 12.2.1, GDB
> will print M::x, and using clang 16.0.0 prints N::x. Not only is this
> behavior confusing to users, it is also not consistent with compiler
> behaviors, which would warn that using x is ambiguous at this point.
>
> This commit makes it so instead of exiting early when finding any symbol
> with the correct name, GDB continues searching through include
> directives until it finds another symbol with the same name but a
> different mangled name - returning an ambiguous variable - or goes
> through all imported namespaces and returns zero or one matching symbols.
>
> The commit also changes gdb.cp/nsusing.exp to test the ambiguous
> detection.
> ---
>  gdb/cp-namespace.c               | 35 ++++++++++++++++++++++++++++----
>  gdb/testsuite/gdb.cp/nsusing.exp | 19 +++++++++++++----
>  2 files changed, 46 insertions(+), 8 deletions(-)
>
> diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
> index c1b6978b7c8..7a07a8dbe75 100644
> --- a/gdb/cp-namespace.c
> +++ b/gdb/cp-namespace.c
> @@ -383,6 +383,8 @@ cp_lookup_symbol_via_imports (const char *scope,
>  {
>    struct using_direct *current;
>    struct block_symbol sym = {};
> +  struct block_symbol saved_sym = {};
> +  const char* sym_name = nullptr;
>    int len;
>    int directive_match;
>  
> @@ -392,7 +394,11 @@ cp_lookup_symbol_via_imports (const char *scope,
>  					 block, domain, 1);
>  
>    if (sym.symbol != NULL)
> -    return sym;
> +    {
> +      saved_sym = sym;
> +      sym_name = saved_sym.symbol->m_name;
> +      sym = {};
> +    }
>  
>    /* Due to a GCC bug, we need to know the boundaries of the current block
>       to know if a certain using directive is valid.  */
> @@ -446,7 +452,16 @@ cp_lookup_symbol_via_imports (const char *scope,
>  	  if (declaration_only || sym.symbol != NULL || current->declaration)
>  	    {
>  	      if (sym.symbol != NULL)
> -		return sym;
> +		{
> +		  if(sym_name == nullptr)
> +		    {
> +		      saved_sym = sym;
> +		      sym_name = saved_sym.symbol->m_name;
> +		      sym = {};
> +		    }
> +		  else if (strcmp(sym_name, sym.symbol->m_name) != 0)
> +		    error (_("reference to \"%s\" is ambiguous"), name);
> +		}
>  
>  	      continue;
>  	    }
> @@ -479,11 +494,23 @@ cp_lookup_symbol_via_imports (const char *scope,
>  	    }
>  
>  	  if (sym.symbol != NULL)
> -	    return sym;
> +	    {
> +	      if(sym_name == nullptr)
> +		{
> +		  saved_sym = sym;
> +		  sym_name = saved_sym.symbol->m_name;
> +		  sym = {};
> +		}
> +	      else if (strcmp(sym_name, sym.symbol->m_name) != 0)
> +		error (_("reference to \"%s\" is ambiguous"), name);
> +	    }
>  	}
>      }
>  
> -  return {};
> +  if (sym_name != nullptr)
> +    return saved_sym;
> +  else
> +    return {};
>  }
>  
>  /* Helper function that searches an array of symbols for one named NAME.  */
> diff --git a/gdb/testsuite/gdb.cp/nsusing.exp b/gdb/testsuite/gdb.cp/nsusing.exp
> index bce6e30adc1..363ae862953 100644
> --- a/gdb/testsuite/gdb.cp/nsusing.exp
> +++ b/gdb/testsuite/gdb.cp/nsusing.exp
> @@ -123,15 +123,26 @@ if { [test_compiler_info {gcc-[0-3]-*}] ||
>      return
>  }
>  
> +    # GCC doesn't properly set the decl_line for namespaces, so GDB believes
> +    # that both using directives are valid as long as we are in this scope.
> +exp_internal 1

I suspect the exp_internal was added for debugging?  Should this, and
the 'exp_internal 0' line below be removed?

Thanks,
Andrew


>  gdb_test_multiple "print x" "print x, before using statement" {
>      -re -wrap "No symbol .x. in current context.*" {
>  	pass $gdb_test_name
>      }
> -    # GCC doesn't properly set the decl_line for namespaces, so GDB believes
> -    # that the "using namespace M" line has already passed at this point.
> -    -re -wrap "= 911.*" {
> +    -re -wrap "reference to .x. is ambiguous.*" {
>  	xfail $gdb_test_name
>      }
>  }
> +exp_internal 0
>  gdb_test "next" ".*" "using namespace M"
> -gdb_test "print x" "= 911" "print x, only using M"
> +gdb_test_multiple "print x" "print x, only using M" {
> +    -re -wrap "= 911.*" {
> +	pass $gdb_test_name
> +    }
> +    -re -wrap "reference to .x. is ambiguous.*" {
> +	xfail $gdb_test_name
> +    }
> +}
> +gdb_test "next" ".*" "using namespace N"
> +gdb_test "print x"  "reference to .x. is ambiguous" "print x, with M and N"
> -- 
> 2.37.3