public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFA] Fix c++/16253 (tag/variable name collision)
@ 2014-03-21 18:12 Keith Seitz
  2014-03-24 14:15 ` Joel Brobecker
  2014-03-26 13:40 ` Joel Brobecker
  0 siblings, 2 replies; 11+ messages in thread
From: Keith Seitz @ 2014-03-21 18:12 UTC (permalink / raw)
  To: gp >> "gdb-patches@sourceware.org ml"

[-- Attachment #1: Type: text/plain, Size: 5398 bytes --]

Hi,

This bug is easily demonstrated:

enum e { A, B } e;
int main (void) { }
(gdb) p e
Attempt to use a type name as an expression

This is happening because of the following commit:

commit 5eeb2539423ce5e7241bce403f48b8babb3670b0
Author: Aleksandar Ristovski <aristovski@qnx.com>
Date:   Mon May 5 14:37:32 2008 +0000

         * ada-lang.c: Update throughout to use symbol_matches_domain
         instead of matching the symbol domain explictly.
         * dwarf2read.c (add_partial_symbol): Do not add new psym for
         STRUCT_DOMAIN. Make sure you recognize c++ struct and java and ada
         class as typedefs. See lookup_partial_symbol function.
         (new_symbol): Similar to add_partial_symbol, do not create
         symbol for the typedef. See lookup_block_symbol.
         * symtab.c (symbol_matches_domain): New function, takes care
         of dual meaning of STRUCT_DOMAIN symbol for c++, ada and java.
         (lookup_partial_symbol): Use symbol_matches_domain to see if the
         found psym domain matches the given domain.
         (lookup_block_symbol): Likewise.

That change attempted to address languages in which the tag name of an 
elaborated type also defines a type of the same name (e.g., "class A 
{};" -> "A object;".

It did this by introducing a new function, symbol_matches_domain, which 
would allow STRUCT_DOMAIN symbol matches for VAR_DOMAIN searches in C++, 
Java, Ada, and D.

That is the crux of the problem, though.  When lookup_block_symbol 
starts searching for a match, it uses symbol_matches_domain to restrict 
the search to the given domain. As a result, it is a race between the 
VAR_DOMAIN and STRUCT_DOMAIN symbols -- whichever is found in the symbol 
table first is the symbol returned, obscurring the symbol in the other 
domain.

I propose the following patch to address this issue. It involves some 
tweaks to lookup_symbol{,_in_language} and other symbol table API 
functions. The change isn't all that considerable, but this patch 
formalizes exactly what to expect from these functions.

Summary: For those languages which define types of elaborated types 
(C++, Java, Ada, D), a lookup of a VAR_DOMAIN symbol will first search 
for a VAR_DOMAIN symbol. The function will fallback to a STRUCT_DOMAIN 
symbol only if that fails. For other languages, only the given domain 
will be searched.

Additionally, lookup_block_symbol ONLY searches the given domain.

In other words, lookup_symbol does "what you think it should." Or at 
least what *I* think it should.

I have updated the comments/documentation preceding each (exported) 
lookup symbol function to explain exactly what to expect.

I must also give a shout out to Joel -- I've largely avoided hacking 
with/at Ada. In fact, Ada *largely* remains unchanged. However, it now 
must explicitly search STRUCT_DOMAIN in a few places itself (an 
analogous change to the other symbol table API changes I've made). Joel, 
if you could run this through your internal AdaCore test harness, that 
would be most helpful.

I've also had to tweak a few DWARF tests to set the language explicitly. 
This is because those tests are actually testing something c++-ish but 
not specifying the actual language.

There are no regressions on native x86_64 linux and native-gdbserver.

Keith

ChangeLog
2014-03-20  Keith Seitz  <keiths@redhat.com>

	PR c++/16253
	* ada-lang.c (ada_symbol_matches_domain): Moved here and renamed
	from symbol_matches_domain in symtab.c. All local callers
	of symbol_matches_domain updated.
	(standard_lookup): If DOMAIN is VAR_DOMAIN and no symbol is found,
	search STRUCT_DOMAIN.
	(ada_find_any_type_symbol): Do not search STRUCT_DOMAIN
	independently.  standard_lookup will do that automatically.
	* ada-tasks.c (get_tcb_types_info): Search STRUCT_DOMAIN for
	types, not VAR_DOMAIN.
	* cp-namespace.c (cp_lookup_symbol_nonlocal): Explain when/why
	VAR_DOMAIN searches may return a STRUCT_DOMAIN match.
	(cp_lookup_symbol_in_namespace): Likewise.
	If no VAR_DOMAIN symbol is found, search STRUCT_DOMAIN.
	(cp_lookup_symbol_exports): Explain when/why VAR_DOMAIN searches
	may return a STRUCT_DOMAIN match.
	(lookup_symbol_file): Search for the class name in STRUCT_DOMAIN.
	* cp-support.c: Include language.h.
	(inspect_type): Explicitly search STRUCT_DOMAIN before searching
	VAR_DOMAIN.
	* psymtab.c (match_partial_symbol): Compare the requested
	domain with the symbol's domain directly.
	(lookup_partial_symbol): Likewise.
	* symtab.c (lookup_symbol_in_language): Explain when/why
	VAR_DOMAIN searches may return a STRUCT_DOMAIN match.
	If no VAR_DOMAIN symbol is found, search STRUCT_DOMAIN for
	appropriate languages.
	(symbol_matches_domain): Renamed `ada_symbol_matches_domain'
	and moved to ada-lang.c
	(lookup_block_symbol): Explain that this function only returns
	symbol matching the requested DOMAIN.
	Compare the requested domain with the symbol's domain directly.
	(iterate_over_symbols): Compare the requested domain with the
	symbol's domain directly.
	* symtab.h (symbol_matches_domain): Remove.

testsuite/ChangeLog
2014-03-20  Keith Seitz  <keiths@redhat.com>

	PR c++/16253
	* gdb.cp/var-tag.cc: New file.
	* gdb.cp/var-tag.exp: New file.
	* gdb.dwarf2/dw2-ada-ffffffff.exp: Set the language to C++.
	* gdb.dwarf2/dw2-anon-mptr.exp: Likewise.
	* gdb.dwarf2/dw2-double-set-die-type.exp: Likewise.
	* gdb.dwarf2/dw2-inheritance.exp: Likewise.

[-- Attachment #2: 16253.patch --]
[-- Type: text/x-patch, Size: 23707 bytes --]

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 38df182..00728bf 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -4389,6 +4389,20 @@ ada_clear_symbol_cache (void)
   ada_init_symbol_cache (sym_cache);
 }
 
+/* STRUCT_DOMAIN symbols are also typedefs for the type.  This function tests
+   the equivalency of two Ada symbol domain types.  */
+
+static int
+ada_symbol_matches_domain (domain_enum symbol_domain, domain_enum domain)
+{
+  if (symbol_domain == domain
+      || ((domain == VAR_DOMAIN || domain == STRUCT_DOMAIN)
+	  && symbol_domain == STRUCT_DOMAIN))
+    return 1;
+
+  return 0;
+}
+
 /* Search our cache for an entry matching NAME and NAMESPACE.
    Return it if found, or NULL otherwise.  */
 
@@ -4490,6 +4504,13 @@ standard_lookup (const char *name, const struct block *block,
   if (lookup_cached_symbol (name, domain, &sym, NULL))
     return sym;
   sym = lookup_symbol_in_language (name, block, domain, language_c, 0);
+
+  /* STRUCT_DOMAIN symbols also define a typedef for the type.  Lookup
+     a STRUCT_DOMAIN symbol if one is requested for VAR_DOMAIN and not
+     found.  */
+  if (sym == NULL && domain == VAR_DOMAIN)
+    sym = lookup_symbol_in_language (name, block, STRUCT_DOMAIN, language_c, 0);
+
   cache_symbol (name, domain, sym, block_found);
   return sym;
 }
@@ -5845,8 +5866,7 @@ ada_add_block_symbols (struct obstack *obstackp,
       for (sym = block_iter_match_first (block, name, wild_match, &iter);
 	   sym != NULL; sym = block_iter_match_next (name, wild_match, &iter))
       {
-        if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
-                                   SYMBOL_DOMAIN (sym), domain)
+        if (ada_symbol_matches_domain (SYMBOL_DOMAIN (sym), domain)
             && wild_match (SYMBOL_LINKAGE_NAME (sym), name) == 0)
           {
 	    if (SYMBOL_CLASS (sym) == LOC_UNRESOLVED)
@@ -5868,8 +5888,7 @@ ada_add_block_symbols (struct obstack *obstackp,
      for (sym = block_iter_match_first (block, name, full_match, &iter);
 	  sym != NULL; sym = block_iter_match_next (name, full_match, &iter))
       {
-        if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
-                                   SYMBOL_DOMAIN (sym), domain))
+        if (ada_symbol_matches_domain (SYMBOL_DOMAIN (sym), domain))
           {
 	    if (SYMBOL_CLASS (sym) != LOC_UNRESOLVED)
 	      {
@@ -5901,8 +5920,7 @@ ada_add_block_symbols (struct obstack *obstackp,
 
       ALL_BLOCK_SYMBOLS (block, iter, sym)
       {
-        if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
-                                   SYMBOL_DOMAIN (sym), domain))
+        if (ada_symbol_matches_domain (SYMBOL_DOMAIN (sym), domain))
           {
             int cmp;
 
@@ -7486,11 +7504,12 @@ ada_find_any_type_symbol (const char *name)
   struct symbol *sym;
 
   sym = standard_lookup (name, get_selected_block (NULL), VAR_DOMAIN);
-  if (sym != NULL && SYMBOL_CLASS (sym) == LOC_TYPEDEF)
+  if (sym != NULL
+      && (SYMBOL_DOMAIN (sym) != VAR_DOMAIN
+	  || SYMBOL_CLASS (sym) == LOC_TYPEDEF))
     return sym;
 
-  sym = standard_lookup (name, NULL, STRUCT_DOMAIN);
-  return sym;
+  return NULL;
 }
 
 /* Find a type named NAME.  Ignores ambiguity.  This routine will look
diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
index 8b37f51..585c1f6 100644
--- a/gdb/ada-tasks.c
+++ b/gdb/ada-tasks.c
@@ -470,24 +470,24 @@ get_tcb_types_info (void)
      C-like) lookups to get the first match.  */
 
   struct symbol *atcb_sym =
-    lookup_symbol_in_language (atcb_name, NULL, VAR_DOMAIN,
+    lookup_symbol_in_language (atcb_name, NULL, STRUCT_DOMAIN,
 			       language_c, NULL);
   const struct symbol *common_atcb_sym =
-    lookup_symbol_in_language (common_atcb_name, NULL, VAR_DOMAIN,
+    lookup_symbol_in_language (common_atcb_name, NULL, STRUCT_DOMAIN,
 			       language_c, NULL);
   const struct symbol *private_data_sym =
-    lookup_symbol_in_language (private_data_name, NULL, VAR_DOMAIN,
+    lookup_symbol_in_language (private_data_name, NULL, STRUCT_DOMAIN,
 			       language_c, NULL);
   const struct symbol *entry_call_record_sym =
-    lookup_symbol_in_language (entry_call_record_name, NULL, VAR_DOMAIN,
+    lookup_symbol_in_language (entry_call_record_name, NULL, STRUCT_DOMAIN,
 			       language_c, NULL);
 
   if (atcb_sym == NULL || atcb_sym->type == NULL)
     {
       /* In Ravenscar run-time libs, the  ATCB does not have a dynamic
          size, so the symbol name differs.  */
-      atcb_sym = lookup_symbol_in_language (atcb_name_fixed, NULL, VAR_DOMAIN,
-					    language_c, NULL);
+      atcb_sym = lookup_symbol_in_language (atcb_name_fixed, NULL,
+					    STRUCT_DOMAIN, language_c, NULL);
 
       if (atcb_sym == NULL || atcb_sym->type == NULL)
         error (_("Cannot find Ada_Task_Control_Block type. Aborting"));
diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 74ccd45..b5d564b 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -221,7 +221,12 @@ cp_is_anonymous (const char *namespace)
    we're looking for, BLOCK is the block that we're searching within,
    DOMAIN says what kind of symbols we're looking for, and if SYMTAB
    is non-NULL, we should store the symtab where we found the symbol
-   in it.  */
+   in it.
+
+   Class, union, and enum tag names may be used in C++ without specifying
+   class-key or enum.  If searching for a VAR_DOMAIN symbol fails,
+   this function will search STRUCT_DOMAIN.  [This is actually done in
+   cp_lookup_symbol_in_namespace.]  */
 
 struct symbol *
 cp_lookup_symbol_nonlocal (const char *name,
@@ -242,7 +247,10 @@ cp_lookup_symbol_nonlocal (const char *name,
 
 /* Look up NAME in the C++ namespace NAMESPACE.  Other arguments are
    as in cp_lookup_symbol_nonlocal.  If SEARCH is non-zero, search
-   through base classes for a matching symbol.  */
+   through base classes for a matching symbol.
+
+   If DOMAIN is VAR_DOMAIN and no matching symbol exists in that domain,
+   this function will search STRUCT_DOMAIN for a match.  */
 
 static struct symbol *
 cp_lookup_symbol_in_namespace (const char *namespace,
@@ -252,18 +260,30 @@ cp_lookup_symbol_in_namespace (const char *namespace,
 {
   if (namespace[0] == '\0')
     {
-      return lookup_symbol_file (name, block, domain, 0, search);
+      struct symbol *sym = lookup_symbol_file (name, block, domain, 0, search);
+
+      if (sym == NULL && domain == VAR_DOMAIN)
+	sym = lookup_symbol_file (name, block, STRUCT_DOMAIN, 0, search);
+
+      return sym;
     }
   else
     {
+      struct symbol *sym;
       char *concatenated_name = alloca (strlen (namespace) + 2
 					+ strlen (name) + 1);
 
       strcpy (concatenated_name, namespace);
       strcat (concatenated_name, "::");
       strcat (concatenated_name, name);
-      return lookup_symbol_file (concatenated_name, block, domain,
-				 cp_is_anonymous (namespace), search);
+      sym = lookup_symbol_file (concatenated_name, block, domain,
+				cp_is_anonymous (namespace), search);
+
+      if (sym == NULL && domain == VAR_DOMAIN)
+	sym = lookup_symbol_file (concatenated_name, block, STRUCT_DOMAIN,
+				  cp_is_anonymous (namespace), search);
+
+      return sym;
     }
 }
 
@@ -516,7 +536,12 @@ cp_lookup_symbol_imports_or_template (const char *scope,
  /* Searches for NAME in the current namespace, and by applying
     relevant import statements belonging to BLOCK and its parents.
     SCOPE is the namespace scope of the context in which the search is
-    being evaluated.  */
+    being evaluated.
+
+    Class, union, and enum tag names may be used in C++ without specifying
+    class-key or enum.  If searching for a VAR_DOMAIN symbol fails,
+    this function will search STRUCT_DOMAIN.  [This is done in
+    cp_lookup_symbol_in_namespace.]  */
 
 struct symbol*
 cp_lookup_symbol_namespace (const char *scope,
@@ -688,7 +713,7 @@ lookup_symbol_file (const char *name,
 
       /* Lookup a class named KLASS.  If none is found, there is nothing
 	 more that can be done.  */
-      klass_sym = lookup_symbol_global (klass, block, domain);
+      klass_sym = lookup_symbol_global (klass, block, STRUCT_DOMAIN);
       if (klass_sym == NULL)
 	{
 	  do_cleanups (cleanup);
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 2379b54..91533e8 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -35,6 +35,7 @@
 #include "expression.h"
 #include "value.h"
 #include "cp-abi.h"
+#include "language.h"
 
 #include "safe-ctype.h"
 
@@ -177,7 +178,29 @@ inspect_type (struct demangle_parse_info *info,
   sym = NULL;
   TRY_CATCH (except, RETURN_MASK_ALL)
   {
-    sym = lookup_symbol (name, 0, VAR_DOMAIN, 0);
+    /* It is not legal to have a typedef and tag name of the same
+       name in C++.  However, anonymous composite types that are defined
+       with a typedef ["typedef struct {...} anonymous_struct;"] WILL
+       have symbols for a TYPE_CODE_TYPEDEF (in VAR_DOMAIN) and a
+       TYPE_CODE_STRUCT (in STRUCT_DOMAIN).
+
+       If VAR_DOMAIN is searched first, it will return the TYPEDEF symbol,
+       and this function will never output the definition of the typedef,
+       since type_print is called below with SHOW = -1. [The typedef hash
+       is never initialized/used when SHOW <= 0 -- and the finder
+       (find_typedef_for_canonicalize) will always return NULL as a result.]
+
+       Consequently, type_print will eventually keep calling this function
+       to replace the typedef (via
+       print_name_maybe_canonical/cp_canonicalize_full).  This leads to
+       infinite recursion.
+
+       This can all be safely avoid by explicitly searching STRUCT_DOMAIN
+       first to find the structure definition.  */
+    if (current_language->la_language == language_cplus)
+      sym = lookup_symbol (name, 0, STRUCT_DOMAIN, 0);
+    if (sym == NULL)
+      sym = lookup_symbol (name, 0, VAR_DOMAIN, NULL);
   }
 
   if (except.reason >= 0 && sym != NULL)
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index 2787f4c..8f2de2a 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -594,8 +594,7 @@ match_partial_symbol (struct objfile *objfile,
       while (top <= real_top
 	     && match (SYMBOL_SEARCH_NAME (*top), name) == 0)
 	{
-	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*top),
-				     SYMBOL_DOMAIN (*top), domain))
+	  if (SYMBOL_DOMAIN (*top) == domain)
 	    return *top;
 	  top++;
 	}
@@ -608,8 +607,7 @@ match_partial_symbol (struct objfile *objfile,
     {
       for (psym = start; psym < start + length; psym++)
 	{
-	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*psym),
-				     SYMBOL_DOMAIN (*psym), domain)
+	  if (SYMBOL_DOMAIN (*psym) == domain
 	      && match (SYMBOL_SEARCH_NAME (*psym), name) == 0)
 	    return *psym;
 	}
@@ -725,8 +723,7 @@ lookup_partial_symbol (struct objfile *objfile,
 
       while (top <= real_top && SYMBOL_MATCHES_SEARCH_NAME (*top, search_name))
 	{
-	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*top),
-				     SYMBOL_DOMAIN (*top), domain))
+	  if (SYMBOL_DOMAIN (*top) == domain)
 	    {
 	      do_cleanups (cleanup);
 	      return (*top);
@@ -742,8 +739,7 @@ lookup_partial_symbol (struct objfile *objfile,
     {
       for (psym = start; psym < start + length; psym++)
 	{
-	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*psym),
-				     SYMBOL_DOMAIN (*psym), domain)
+	  if (SYMBOL_DOMAIN (*psym) == domain
 	      && SYMBOL_MATCHES_SEARCH_NAME (*psym, search_name))
 	    {
 	      do_cleanups (cleanup);
@@ -1223,8 +1219,7 @@ map_block (const char *name, domain_enum namespace, struct objfile *objfile,
   for (sym = block_iter_match_first (block, name, match, &iter);
        sym != NULL; sym = block_iter_match_next (name, match, &iter))
     {
-      if (symbol_matches_domain (SYMBOL_LANGUAGE (sym), 
-				 SYMBOL_DOMAIN (sym), namespace))
+      if (SYMBOL_DOMAIN (sym) == namespace)
 	{
 	  if (callback (block, sym, data))
 	    return 1;
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 66d1624..15ac3d1 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -1310,7 +1310,11 @@ demangle_for_lookup (const char *name, enum language lang,
    NAME is a field of the current implied argument `this'.  If so set
    *IS_A_FIELD_OF_THIS to 1, otherwise set it to zero.
    BLOCK_FOUND is set to the block in which NAME is found (in the case of
-   a field of `this', value_of_this sets BLOCK_FOUND to the proper value.)  */
+   a field of `this', value_of_this sets BLOCK_FOUND to the proper value.)
+
+   If DOMAIN is VAR_DOMAIN and the language permits using tag names for
+   elaborated types, such as classes in C++, this function will search
+   STRUCT_DOMAIN if no matching is found.  */
 
 /* This function (or rather its subordinates) have a bunch of loops and
    it would seem to be attractive to put in some QUIT's (though I'm not really
@@ -1333,6 +1337,23 @@ lookup_symbol_in_language (const char *name, const struct block *block,
 
   returnval = lookup_symbol_aux (modified_name, block, domain, lang,
 				 is_a_field_of_this);
+  if (returnval == NULL)
+    {
+      if (is_a_field_of_this != NULL
+	  && is_a_field_of_this->type != NULL)
+	return NULL;
+
+      /* Some languages define typedefs of a type equal to its tag name,
+	 e.g., in C++, "struct foo { ... }" also defines a typedef for
+	 "foo".  */
+      if (domain == VAR_DOMAIN
+	  && (lang == language_cplus || lang == language_java
+	      || lang == language_ada || lang == language_d))
+	{
+	  returnval = lookup_symbol_aux (modified_name, block, STRUCT_DOMAIN,
+					 lang, is_a_field_of_this);
+	}
+    }
   do_cleanups (cleanup);
 
   return returnval;
@@ -1907,27 +1928,6 @@ lookup_symbol_global (const char *name,
   return lookup_data.result;
 }
 
-int
-symbol_matches_domain (enum language symbol_language,
-		       domain_enum symbol_domain,
-		       domain_enum domain)
-{
-  /* For C++ "struct foo { ... }" also defines a typedef for "foo".
-     A Java class declaration also defines a typedef for the class.
-     Similarly, any Ada type declaration implicitly defines a typedef.  */
-  if (symbol_language == language_cplus
-      || symbol_language == language_d
-      || symbol_language == language_java
-      || symbol_language == language_ada)
-    {
-      if ((domain == VAR_DOMAIN || domain == STRUCT_DOMAIN)
-	  && symbol_domain == STRUCT_DOMAIN)
-	return 1;
-    }
-  /* For all other languages, strict match is required.  */
-  return (symbol_domain == domain);
-}
-
 /* Look up a type named NAME in the struct_domain.  The type returned
    must not be opaque -- i.e., must have at least one field
    defined.  */
@@ -2050,7 +2050,12 @@ basic_lookup_transparent_type (const char *name)
    binary search terminates, we drop through and do a straight linear
    search on the symbols.  Each symbol which is marked as being a ObjC/C++
    symbol (language_cplus or language_objc set) has both the encoded and
-   non-encoded names tested for a match.  */
+   non-encoded names tested for a match.
+
+   This function specifically disallows domain mismatches.  If a language
+   defines a typedef for an elaborated type, such as classes in C++,
+   then this function will need to be called twice, once to search
+   VAR_DOMAIN and once to search STRUCT_DOMAIN.  */
 
 struct symbol *
 lookup_block_symbol (const struct block *block, const char *name,
@@ -2065,8 +2070,7 @@ lookup_block_symbol (const struct block *block, const char *name,
 	   sym != NULL;
 	   sym = block_iter_name_next (name, &iter))
 	{
-	  if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
-				     SYMBOL_DOMAIN (sym), domain))
+	  if (SYMBOL_DOMAIN (sym) == domain)
 	    return sym;
 	}
       return NULL;
@@ -2085,8 +2089,7 @@ lookup_block_symbol (const struct block *block, const char *name,
 	   sym != NULL;
 	   sym = block_iter_name_next (name, &iter))
 	{
-	  if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
-				     SYMBOL_DOMAIN (sym), domain))
+	  if (SYMBOL_DOMAIN (sym) == domain)
 	    {
 	      sym_found = sym;
 	      if (!SYMBOL_IS_ARGUMENT (sym))
@@ -2120,8 +2123,7 @@ iterate_over_symbols (const struct block *block, const char *name,
        sym != NULL;
        sym = block_iter_name_next (name, &iter))
     {
-      if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
-				 SYMBOL_DOMAIN (sym), domain))
+      if (SYMBOL_DOMAIN (sym) == domain)
 	{
 	  if (!callback (sym, data))
 	    return;
diff --git a/gdb/symtab.h b/gdb/symtab.h
index fbe5868..efc3643 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1019,10 +1019,6 @@ extern const char multiple_symbols_cancel[];
 
 const char *multiple_symbols_select_mode (void);
 
-int symbol_matches_domain (enum language symbol_language, 
-			   domain_enum symbol_domain,
-			   domain_enum domain);
-
 /* lookup a symbol table by source file name.  */
 
 extern struct symtab *lookup_symtab (const char *);
diff --git a/gdb/testsuite/gdb.cp/var-tag.cc b/gdb/testsuite/gdb.cp/var-tag.cc
new file mode 100644
index 0000000..93b9caf
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/var-tag.cc
@@ -0,0 +1,44 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2014 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int global = 3;
+
+class C {
+public:
+  struct C1 {} C1;
+  enum E1 {a1, b1, c1} E1;
+  union U1 {int a1; char b1;} U1;
+
+  C () : E1 (b1) {}
+  void global (void) const {}
+  int f (void) const { global (); return 0; }
+} C;
+
+struct S {} S;
+enum E {a, b, c} E;
+union U {int a; char b;} U;
+
+class CC {} cc;
+struct SS {} ss;
+enum EE {ea, eb, ec} ee;
+union UU {int aa; char bb;} uu;
+
+int
+main (void)
+{
+  return C.f ();
+}
diff --git a/gdb/testsuite/gdb.cp/var-tag.exp b/gdb/testsuite/gdb.cp/var-tag.exp
new file mode 100644
index 0000000..7869fa2
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/var-tag.exp
@@ -0,0 +1,99 @@
+# Copyright 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite
+
+# Test expressions in which variable names shadow tag names.
+
+if {[skip_cplus_tests]} { continue }
+
+standard_testfile .cc
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug c++}]} {
+    return -1
+}
+
+proc do_global_tests {lang} {
+    if {$lang == "c"} {
+	set invalid_print "No symbol \"%s\" in current context."
+	set ptypefmt $invalid_print
+    } else {
+	set invalid_print "Attempt to use a type name as an expression"
+	set ptypefmt "type = (class|enum|union|struct) %s {.*}"
+    }
+
+    with_test_prefix $lang {
+	gdb_test_no_output "set language $lang"
+	gdb_test "ptype C" "type = class C {.*}"
+	gdb_test "print E" "= a"
+	gdb_test "ptype E" "type = enum E {.*}"
+	gdb_test "print S" "= {<No data fields>}"
+	gdb_test "ptype S" "type = struct S {.*}"
+	gdb_test "print U" "= {.*}"
+	gdb_test "ptype U" "type = union U {.*}"
+	gdb_test "print cc" "= {.*}"
+	gdb_test "ptype cc" "type = class CC {.*}"
+	gdb_test "print CC" [format $invalid_print "CC"]
+	gdb_test "ptype CC" [format $ptypefmt "CC"]
+	gdb_test "print ss" "= {<No data fields>}"
+	gdb_test "ptype ss" "type = struct SS {.*}"
+	gdb_test "print SS" [format $invalid_print "SS"]
+	gdb_test "ptype SS" [format $ptypefmt "SS"]
+	gdb_test "print ee" "= .*"
+	gdb_test "ptype ee" "type = enum EE {.*}"
+	gdb_test "print EE" [format $invalid_print "EE"]
+	gdb_test "ptype EE" [format $ptypefmt "EE"]
+	gdb_test "print uu" "= {.*}"
+	gdb_test "ptype uu" "type = union UU {.*}"
+	gdb_test "print UU" [format $invalid_print  "UU"]
+	gdb_test "ptype UU" [format $ptypefmt "UU"]
+    }
+}
+
+# First test expressions when there is no context.
+with_test_prefix "before start" {
+    do_global_tests c++
+    do_global_tests c
+}
+
+# Run to main and test again.
+if {![runto_main]} {
+    perror "couldn't run to main"
+    continue
+}
+
+with_test_prefix "in main" {
+    do_global_tests c++
+    do_global_tests c
+}
+
+# Finally run to C::f and test again
+gdb_breakpoint "C::f"
+gdb_continue_to_breakpoint "continue to C::f"
+with_test_prefix "in C::f" {
+    do_global_tests c++
+    do_global_tests c
+}
+
+# Another hard-to-guess-the-users-intent bug...
+# It would be really nice if we could query the user!
+with_test_prefix "global collision" {
+    gdb_test_no_output "set language c++"
+    setup_kfail "c++/16463" "*-*-*"
+    gdb_test "print global" "= 3"
+
+    # ... with a simple workaround:
+    gdb_test "print ::global" "= 3"
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ada-ffffffff.exp b/gdb/testsuite/gdb.dwarf2/dw2-ada-ffffffff.exp
index 1c1d10f..7936f25 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-ada-ffffffff.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ada-ffffffff.exp
@@ -28,6 +28,10 @@ if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" object {}] != ""
 
 clean_restart $executable
 
+# Force the language to C++, since we want to treat the type
+# defined in the object file like a C++ type, using sizeof.
+gdb_test_no_output "set language c++"
+
 # -1 was produced, it is now caught with the complaint:
 # Suspicious DW_AT_byte_size value treated as zero instead of ...
 gdb_test "p sizeof (t)" " = 0"
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-anon-mptr.exp b/gdb/testsuite/gdb.dwarf2/dw2-anon-mptr.exp
index aef3cb8..c9e59ed 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-anon-mptr.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-anon-mptr.exp
@@ -40,5 +40,7 @@ gdb_test "show cp-abi" {The currently selected C\+\+ ABI is "gnu-v3".*}
 
 gdb_load $binfile
 
+gdb_test_no_output "set language c++"
+
 gdb_test "ptype crash" \
     "type = class crash {\[\r\n \t\]*public:\[\r\n \t\]*crash\\(int \\(class {\\.\\.\\.}::\\*\\)\\(class {\\.\\.\\.} \\* const\\)\\);\[\r\n \t\]*}"
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-double-set-die-type.exp b/gdb/testsuite/gdb.dwarf2/dw2-double-set-die-type.exp
index 7aabcfe..40daed9 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-double-set-die-type.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-double-set-die-type.exp
@@ -30,4 +30,5 @@ if  { [gdb_compile [file join $srcdir $subdir $srcfile] $binfile \
 }
 
 clean_restart $testfile
+gdb_test_no_output "set language c++"
 gdb_test "ptype a" "type = class .*"
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inheritance.exp b/gdb/testsuite/gdb.dwarf2/dw2-inheritance.exp
index 7c954bb..028a1df 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-inheritance.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-inheritance.exp
@@ -31,4 +31,5 @@ if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" $binfile \
 
 clean_restart $testfile
 
+gdb_test_no_output "set language c++"
 gdb_test "ptype inherited" "type = class inherited .*"

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFA] Fix c++/16253 (tag/variable name collision)
  2014-03-21 18:12 [RFA] Fix c++/16253 (tag/variable name collision) Keith Seitz
@ 2014-03-24 14:15 ` Joel Brobecker
  2014-03-25 17:50   ` Joel Brobecker
  2014-03-25 18:11   ` Keith Seitz
  2014-03-26 13:40 ` Joel Brobecker
  1 sibling, 2 replies; 11+ messages in thread
From: Joel Brobecker @ 2014-03-24 14:15 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gp >> "gdb-patches@sourceware.org ml"

[-- Attachment #1: Type: text/plain, Size: 1550 bytes --]

Hey Keith,

> I must also give a shout out to Joel -- I've largely avoided hacking
> with/at Ada. In fact, Ada *largely* remains unchanged. However, it
> now must explicitly search STRUCT_DOMAIN in a few places itself (an
> analogous change to the other symbol table API changes I've made).
> Joel, if you could run this through your internal AdaCore test
> harness, that would be most helpful.

I had a chance to test your patch today, and unfortunately our testsuite
detected some regressions. I think they might all be the same, so I
picked the simplest testcase. I might be running short of time today
to look deeper into this, but I can try scheduling some time for it
tomorrow or Wed.

    % gnatmake -g foo
    % gdb foo
    (gdb) ptype base

It should have been:

    (gdb) ptype base
    type = (first, middle, last)

The debugger finds the type if you start the program, but I think
it's because it finds it via the DIE generated inside the main
subprogram's DIE because of the variable of that type declared
there:

    (gdb) start
    Temporary breakpoint 1 at 0x401d86: file foo.adb, line 4.
    Starting program: /[...]/foo

    Temporary breakpoint 1, foo () at foo.adb:4
    4          B : Base := Base'First;   <<<<<---  the variable of type Base
    (gdb) ptype base
    type = (first, middle, last)

This may not be directly related to your patch. I seem to have seen
some unexplainable behavior in GDB occasionally in the recent past
making me wonder whether there might be something fishy in the symbol
lookup for Ada.

-- 
Joel

[-- Attachment #2: pck.ads --]
[-- Type: text/plain, Size: 169 bytes --]

with System;

package Pck is
   type Base is (First, Middle, Last);
   subtype Enum is Base range First .. Last;

   procedure Do_Nothing (A : System.Address);
end Pck;

[-- Attachment #3: pck.adb --]
[-- Type: text/plain, Size: 119 bytes --]

package body Pck is

   procedure Do_Nothing (A : System.Address) is
   begin
      null;
   end Do_Nothing;

end Pck;

[-- Attachment #4: foo.adb --]
[-- Type: text/plain, Size: 106 bytes --]

with Pck; use Pck;

procedure Foo is
   B : Base := Base'First;
begin
   Do_Nothing (B'Address);
end Foo;

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFA] Fix c++/16253 (tag/variable name collision)
  2014-03-24 14:15 ` Joel Brobecker
@ 2014-03-25 17:50   ` Joel Brobecker
  2014-03-25 18:11   ` Keith Seitz
  1 sibling, 0 replies; 11+ messages in thread
From: Joel Brobecker @ 2014-03-25 17:50 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gp >> "gdb-patches@sourceware.org ml"

Hey Keith,

> > I must also give a shout out to Joel -- I've largely avoided hacking
> > with/at Ada. In fact, Ada *largely* remains unchanged. However, it
> > now must explicitly search STRUCT_DOMAIN in a few places itself (an
> > analogous change to the other symbol table API changes I've made).
> > Joel, if you could run this through your internal AdaCore test
> > harness, that would be most helpful.
> 
> I had a chance to test your patch today, and unfortunately our testsuite
> detected some regressions. I think they might all be the same, so I
> picked the simplest testcase. I might be running short of time today
> to look deeper into this, but I can try scheduling some time for it
> tomorrow or Wed.
> 
>     % gnatmake -g foo
>     % gdb foo
>     (gdb) ptype base

Insert missing "No definition of "base" in current context." as the
output of the "ptype base" command above...

I had a chance to investigate the source of the problem, and I think
I understand it, now. Basically, I think you fixed up the Ada symbol
lookups for local symbols, but did not adjust the non-local lookups.

In our case, our type "base" is defined in unit foo.adb as a static
(non-global) STRUCT_DOMAIN symbol. The symbol lookup is performed
via ada-lang.c::add_nonlocal_symbols which calls
objfile->sf->qf->map_matching_symbols (=map_matching_symbols_psymtab).
And the match now fails since it performs strict domain matching.

I think you actually ended up noticing the problem, since you had to
change ada-tasks to do a STRUCT_DOMAIN search instead of a VAR_DOMAIN
search. I think the ada-tasks change is correct regardless, since
these are all structure types anyways (need to double-check, but
pretty sure), but that was also a clue.

Now, the question becomes: Does Ada need to add some logic to call
objfile->sf->qf->map_matching_symbols a second time with STRUCT_DOMAIN,
or should objfile->sf->qf->map_matching_symbols do it? From the looks
of your patch, it looks like this should be done on the Ada side?

Thanks!
-- 
Joel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFA] Fix c++/16253 (tag/variable name collision)
  2014-03-24 14:15 ` Joel Brobecker
  2014-03-25 17:50   ` Joel Brobecker
@ 2014-03-25 18:11   ` Keith Seitz
  2014-03-26 13:32     ` Joel Brobecker
  1 sibling, 1 reply; 11+ messages in thread
From: Keith Seitz @ 2014-03-25 18:11 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gp >> "gdb-patches@sourceware.org ml"

[-- Attachment #1: Type: text/plain, Size: 765 bytes --]

On 03/24/2014 07:15 AM, Joel Brobecker wrote:
> This may not be directly related to your patch. I seem to have seen
> some unexplainable behavior in GDB occasionally in the recent past
> making me wonder whether there might be something fishy in the symbol
> lookup for Ada.

It is my patch that is causing the problems.

In this case, add_nonlocal_symbols is calling the qf 
map_matching_symbols method which is calling match_partial_symbol. Since 
this method now does strict matches against domain, an explicit check 
for STRUCT_DOMAIN matches must be added.

Give this amendment a shot and see how it goes. I suspect there are 
probably one or two more places where this occurs. [The easiest thing to 
do is audit any function which uses "VAR_DOMAIN".]

Keith


[-- Attachment #2: 16253-amended-1.patch --]
[-- Type: text/x-patch, Size: 1444 bytes --]

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 00728bf..94c6cf8 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -5336,13 +5336,33 @@ add_nonlocal_symbols (struct obstack *obstackp, const char *name,
       data.objfile = objfile;
 
       if (is_wild_match)
-	objfile->sf->qf->map_matching_symbols (objfile, name, domain, global,
-					       aux_add_nonlocal_symbols, &data,
-					       wild_match, NULL);
+	{
+	  objfile->sf->qf->map_matching_symbols (objfile, name, domain, global,
+						 aux_add_nonlocal_symbols,
+						 &data, wild_match, NULL);
+	  if (domain == VAR_DOMAIN)
+	    {
+	      objfile->sf->qf->map_matching_symbols (objfile, name,
+						     STRUCT_DOMAIN, global,
+						     aux_add_nonlocal_symbols,
+						     &data, wild_match, NULL);
+	    }
+	}
       else
-	objfile->sf->qf->map_matching_symbols (objfile, name, domain, global,
-					       aux_add_nonlocal_symbols, &data,
-					       full_match, compare_names);
+	{
+	  objfile->sf->qf->map_matching_symbols (objfile, name, domain, global,
+						 aux_add_nonlocal_symbols,
+						 &data, full_match,
+						 compare_names);
+	  if (domain == VAR_DOMAIN)
+	    {
+	      objfile->sf->qf->map_matching_symbols (objfile, name,
+						     STRUCT_DOMAIN, global,
+						     aux_add_nonlocal_symbols,
+						     &data, full_match,
+						     compare_names);
+	    }
+	}
     }
 
   if (num_defns_collected (obstackp) == 0 && global && !is_wild_match)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFA] Fix c++/16253 (tag/variable name collision)
  2014-03-25 18:11   ` Keith Seitz
@ 2014-03-26 13:32     ` Joel Brobecker
  0 siblings, 0 replies; 11+ messages in thread
From: Joel Brobecker @ 2014-03-26 13:32 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gp >> "gdb-patches@sourceware.org ml"

> In this case, add_nonlocal_symbols is calling the qf
> map_matching_symbols method which is calling match_partial_symbol.
> Since this method now does strict matches against domain, an
> explicit check for STRUCT_DOMAIN matches must be added.
> 
> Give this amendment a shot and see how it goes. I suspect there are
> probably one or two more places where this occurs. [The easiest
> thing to do is audit any function which uses "VAR_DOMAIN".]

I was thinking of the very same patch :), and it tested fine.
Can you remove the curly braces in the second call? They should
be unnecessary in this case, and our CS asks us to avoid them
in that situation (the only exception is when the body also
contains a comment).

Thanks!


> +	{
> +	  objfile->sf->qf->map_matching_symbols (objfile, name, domain, global,
> +						 aux_add_nonlocal_symbols,
> +						 &data, wild_match, NULL);
> +	  if (domain == VAR_DOMAIN)
> +	    {
> +	      objfile->sf->qf->map_matching_symbols (objfile, name,
> +						     STRUCT_DOMAIN, global,
> +						     aux_add_nonlocal_symbols,
> +						     &data, wild_match, NULL);
> +	    }
> +	}
>        else
> -	objfile->sf->qf->map_matching_symbols (objfile, name, domain, global,
> -					       aux_add_nonlocal_symbols, &data,
> -					       full_match, compare_names);
> +	{
> +	  objfile->sf->qf->map_matching_symbols (objfile, name, domain, global,
> +						 aux_add_nonlocal_symbols,
> +						 &data, full_match,
> +						 compare_names);
> +	  if (domain == VAR_DOMAIN)
> +	    {
> +	      objfile->sf->qf->map_matching_symbols (objfile, name,
> +						     STRUCT_DOMAIN, global,
> +						     aux_add_nonlocal_symbols,
> +						     &data, full_match,
> +						     compare_names);
> +	    }
> +	}
>      }
>  
>    if (num_defns_collected (obstackp) == 0 && global && !is_wild_match)


-- 
Joel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFA] Fix c++/16253 (tag/variable name collision)
  2014-03-21 18:12 [RFA] Fix c++/16253 (tag/variable name collision) Keith Seitz
  2014-03-24 14:15 ` Joel Brobecker
@ 2014-03-26 13:40 ` Joel Brobecker
  2014-03-26 15:34   ` Keith Seitz
  2014-04-10 18:54   ` Keith Seitz
  1 sibling, 2 replies; 11+ messages in thread
From: Joel Brobecker @ 2014-03-26 13:40 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gp >> "gdb-patches@sourceware.org ml"

Hey again, Keith,

> ChangeLog
> 2014-03-20  Keith Seitz  <keiths@redhat.com>
> 
> 	PR c++/16253
> 	* ada-lang.c (ada_symbol_matches_domain): Moved here and renamed
> 	from symbol_matches_domain in symtab.c. All local callers
> 	of symbol_matches_domain updated.
> 	(standard_lookup): If DOMAIN is VAR_DOMAIN and no symbol is found,
> 	search STRUCT_DOMAIN.
> 	(ada_find_any_type_symbol): Do not search STRUCT_DOMAIN
> 	independently.  standard_lookup will do that automatically.
> 	* ada-tasks.c (get_tcb_types_info): Search STRUCT_DOMAIN for
> 	types, not VAR_DOMAIN.
> 	* cp-namespace.c (cp_lookup_symbol_nonlocal): Explain when/why
> 	VAR_DOMAIN searches may return a STRUCT_DOMAIN match.
> 	(cp_lookup_symbol_in_namespace): Likewise.
> 	If no VAR_DOMAIN symbol is found, search STRUCT_DOMAIN.
> 	(cp_lookup_symbol_exports): Explain when/why VAR_DOMAIN searches
> 	may return a STRUCT_DOMAIN match.
> 	(lookup_symbol_file): Search for the class name in STRUCT_DOMAIN.
> 	* cp-support.c: Include language.h.
> 	(inspect_type): Explicitly search STRUCT_DOMAIN before searching
> 	VAR_DOMAIN.
> 	* psymtab.c (match_partial_symbol): Compare the requested
> 	domain with the symbol's domain directly.
> 	(lookup_partial_symbol): Likewise.
> 	* symtab.c (lookup_symbol_in_language): Explain when/why
> 	VAR_DOMAIN searches may return a STRUCT_DOMAIN match.
> 	If no VAR_DOMAIN symbol is found, search STRUCT_DOMAIN for
> 	appropriate languages.
> 	(symbol_matches_domain): Renamed `ada_symbol_matches_domain'
> 	and moved to ada-lang.c
> 	(lookup_block_symbol): Explain that this function only returns
> 	symbol matching the requested DOMAIN.
> 	Compare the requested domain with the symbol's domain directly.
> 	(iterate_over_symbols): Compare the requested domain with the
> 	symbol's domain directly.
> 	* symtab.h (symbol_matches_domain): Remove.

Would you mind committing the ada-tasks.c change now, and separately
from the rest? It's an improvement on its own. FTR, I tested this part
of the patch isolated from the rest on x86_64-linux.

Thanks!

-- 
Joel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFA] Fix c++/16253 (tag/variable name collision)
  2014-03-26 13:40 ` Joel Brobecker
@ 2014-03-26 15:34   ` Keith Seitz
  2014-04-10 18:54   ` Keith Seitz
  1 sibling, 0 replies; 11+ messages in thread
From: Keith Seitz @ 2014-03-26 15:34 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gp >> "gdb-patches@sourceware.org ml"

[-- Attachment #1: Type: text/plain, Size: 468 bytes --]

On 03/26/2014 06:40 AM, Joel Brobecker wrote:
> Hey again, Keith,
>
> Would you mind committing the ada-tasks.c change now, and separately
> from the rest? It's an improvement on its own. FTR, I tested this part
> of the patch isolated from the rest on x86_64-linux.

Sure. I've pushed the following patch/ChangeLog.

Keith

ChangeLog
2014-03-26  Keith Seitz  <keiths@redhat.com>

	* ada-tasks.c (get_tcb_types_info): Search STRUCT_DOMAIN for
	types, not VAR_DOMAIN.


[-- Attachment #2: ada-tasks.c-STRUCT_DOMAIN.patch --]
[-- Type: text/x-patch, Size: 1590 bytes --]

diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
index 8b37f51..585c1f6 100644
--- a/gdb/ada-tasks.c
+++ b/gdb/ada-tasks.c
@@ -470,24 +470,24 @@ get_tcb_types_info (void)
      C-like) lookups to get the first match.  */
 
   struct symbol *atcb_sym =
-    lookup_symbol_in_language (atcb_name, NULL, VAR_DOMAIN,
+    lookup_symbol_in_language (atcb_name, NULL, STRUCT_DOMAIN,
 			       language_c, NULL);
   const struct symbol *common_atcb_sym =
-    lookup_symbol_in_language (common_atcb_name, NULL, VAR_DOMAIN,
+    lookup_symbol_in_language (common_atcb_name, NULL, STRUCT_DOMAIN,
 			       language_c, NULL);
   const struct symbol *private_data_sym =
-    lookup_symbol_in_language (private_data_name, NULL, VAR_DOMAIN,
+    lookup_symbol_in_language (private_data_name, NULL, STRUCT_DOMAIN,
 			       language_c, NULL);
   const struct symbol *entry_call_record_sym =
-    lookup_symbol_in_language (entry_call_record_name, NULL, VAR_DOMAIN,
+    lookup_symbol_in_language (entry_call_record_name, NULL, STRUCT_DOMAIN,
 			       language_c, NULL);
 
   if (atcb_sym == NULL || atcb_sym->type == NULL)
     {
       /* In Ravenscar run-time libs, the  ATCB does not have a dynamic
          size, so the symbol name differs.  */
-      atcb_sym = lookup_symbol_in_language (atcb_name_fixed, NULL, VAR_DOMAIN,
-					    language_c, NULL);
+      atcb_sym = lookup_symbol_in_language (atcb_name_fixed, NULL,
+					    STRUCT_DOMAIN, language_c, NULL);
 
       if (atcb_sym == NULL || atcb_sym->type == NULL)
         error (_("Cannot find Ada_Task_Control_Block type. Aborting"));

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFA] Fix c++/16253 (tag/variable name collision)
  2014-03-26 13:40 ` Joel Brobecker
  2014-03-26 15:34   ` Keith Seitz
@ 2014-04-10 18:54   ` Keith Seitz
  2014-04-10 20:29     ` Tom Tromey
  2014-04-17 19:03     ` Regression for gdb.cp/koenig.exp: p entry (c) [Re: [RFA] Fix c++/16253 (tag/variable name collision)] Jan Kratochvil
  1 sibling, 2 replies; 11+ messages in thread
From: Keith Seitz @ 2014-04-10 18:54 UTC (permalink / raw)
  To: gp >> "gdb-patches@sourceware.org ml"

[-- Attachment #1: Type: text/plain, Size: 2539 bytes --]

On 03/26/2014 06:40 AM, Joel Brobecker wrote:
>
> Would you mind committing the ada-tasks.c change now, and separately
> from the rest? It's an improvement on its own. FTR, I tested this part
> of the patch isolated from the rest on x86_64-linux.

For the record, Joel communicated to me privately that he has no further 
comments or regressions with this patch.

I am reposting the entire patch for review. The only difference between 
this and the original version is the exclusion of the patch mentioned above.

Keith

ChangeLog
2014-04-10  Keith Seitz  <keiths@redhat.com>

	PR c++/16253
	* ada-lang.c (ada_symbol_matches_domain): Moved here and renamed
	from symbol_matches_domain in symtab.c. All local callers
	of symbol_matches_domain updated.
	(standard_lookup): If DOMAIN is VAR_DOMAIN and no symbol is found,
	search STRUCT_DOMAIN.
	(ada_find_any_type_symbol): Do not search STRUCT_DOMAIN
	independently.  standard_lookup will do that automatically.
	* cp-namespace.c (cp_lookup_symbol_nonlocal): Explain when/why
	VAR_DOMAIN searches may return a STRUCT_DOMAIN match.
	(cp_lookup_symbol_in_namespace): Likewise.
	If no VAR_DOMAIN symbol is found, search STRUCT_DOMAIN.
	(cp_lookup_symbol_exports): Explain when/why VAR_DOMAIN searches
	may return a STRUCT_DOMAIN match.
	(lookup_symbol_file): Search for the class name in STRUCT_DOMAIN.
	* cp-support.c: Include language.h.
	(inspect_type): Explicitly search STRUCT_DOMAIN before searching
	VAR_DOMAIN.
	* psymtab.c (match_partial_symbol): Compare the requested
	domain with the symbol's domain directly.
	(lookup_partial_symbol): Likewise.
	* symtab.c (lookup_symbol_in_language): Explain when/why
	VAR_DOMAIN searches may return a STRUCT_DOMAIN match.
	If no VAR_DOMAIN symbol is found, search STRUCT_DOMAIN for
	appropriate languages.
	(symbol_matches_domain): Renamed `ada_symbol_matches_domain'
	and moved to ada-lang.c
	(lookup_block_symbol): Explain that this function only returns
	symbol matching the requested DOMAIN.
	Compare the requested domain with the symbol's domain directly.
	(iterate_over_symbols): Compare the requested domain with the
	symbol's domain directly.
	* symtab.h (symbol_matches_domain): Remove.

testsuite/ChangeLog
2014-04-10  Keith Seitz  <keiths@redhat.com>

	PR c++/16253
	* gdb.cp/var-tag.cc: New file.
	* gdb.cp/var-tag.exp: New file.
	* gdb.dwarf2/dw2-ada-ffffffff.exp: Set the language to C++.
	* gdb.dwarf2/dw2-anon-mptr.exp: Likewise.
	* gdb.dwarf2/dw2-double-set-die-type.exp: Likewise.
	* gdb.dwarf2/dw2-inheritance.exp: Likewise.


[-- Attachment #2: 16253-2.patch --]
[-- Type: text/x-patch, Size: 23394 bytes --]

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 71827ae..510c0bf 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -4389,6 +4389,20 @@ ada_clear_symbol_cache (void)
   ada_init_symbol_cache (sym_cache);
 }
 
+/* STRUCT_DOMAIN symbols are also typedefs for the type.  This function tests
+   the equivalency of two Ada symbol domain types.  */
+
+static int
+ada_symbol_matches_domain (domain_enum symbol_domain, domain_enum domain)
+{
+  if (symbol_domain == domain
+      || ((domain == VAR_DOMAIN || domain == STRUCT_DOMAIN)
+	  && symbol_domain == STRUCT_DOMAIN))
+    return 1;
+
+  return 0;
+}
+
 /* Search our cache for an entry matching NAME and NAMESPACE.
    Return it if found, or NULL otherwise.  */
 
@@ -4490,6 +4504,13 @@ standard_lookup (const char *name, const struct block *block,
   if (lookup_cached_symbol (name, domain, &sym, NULL))
     return sym;
   sym = lookup_symbol_in_language (name, block, domain, language_c, 0);
+
+  /* STRUCT_DOMAIN symbols also define a typedef for the type.  Lookup
+     a STRUCT_DOMAIN symbol if one is requested for VAR_DOMAIN and not
+     found.  */
+  if (sym == NULL && domain == VAR_DOMAIN)
+    sym = lookup_symbol_in_language (name, block, STRUCT_DOMAIN, language_c, 0);
+
   cache_symbol (name, domain, sym, block_found);
   return sym;
 }
@@ -5315,13 +5336,29 @@ add_nonlocal_symbols (struct obstack *obstackp, const char *name,
       data.objfile = objfile;
 
       if (is_wild_match)
-	objfile->sf->qf->map_matching_symbols (objfile, name, domain, global,
-					       aux_add_nonlocal_symbols, &data,
-					       wild_match, NULL);
+	{
+	  objfile->sf->qf->map_matching_symbols (objfile, name, domain, global,
+						 aux_add_nonlocal_symbols,
+						 &data, wild_match, NULL);
+	  if (domain == VAR_DOMAIN)
+	    objfile->sf->qf->map_matching_symbols (objfile, name,
+						   STRUCT_DOMAIN, global,
+						   aux_add_nonlocal_symbols,
+						   &data, wild_match, NULL);
+	}
       else
-	objfile->sf->qf->map_matching_symbols (objfile, name, domain, global,
-					       aux_add_nonlocal_symbols, &data,
-					       full_match, compare_names);
+	{
+	  objfile->sf->qf->map_matching_symbols (objfile, name, domain, global,
+						 aux_add_nonlocal_symbols,
+						 &data, full_match,
+						 compare_names);
+	  if (domain == VAR_DOMAIN)
+	    objfile->sf->qf->map_matching_symbols (objfile, name,
+						   STRUCT_DOMAIN, global,
+						   aux_add_nonlocal_symbols,
+						   &data, full_match,
+						   compare_names);
+	}
     }
 
   if (num_defns_collected (obstackp) == 0 && global && !is_wild_match)
@@ -5845,8 +5882,7 @@ ada_add_block_symbols (struct obstack *obstackp,
       for (sym = block_iter_match_first (block, name, wild_match, &iter);
 	   sym != NULL; sym = block_iter_match_next (name, wild_match, &iter))
       {
-        if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
-                                   SYMBOL_DOMAIN (sym), domain)
+        if (ada_symbol_matches_domain (SYMBOL_DOMAIN (sym), domain)
             && wild_match (SYMBOL_LINKAGE_NAME (sym), name) == 0)
           {
 	    if (SYMBOL_CLASS (sym) == LOC_UNRESOLVED)
@@ -5868,8 +5904,7 @@ ada_add_block_symbols (struct obstack *obstackp,
      for (sym = block_iter_match_first (block, name, full_match, &iter);
 	  sym != NULL; sym = block_iter_match_next (name, full_match, &iter))
       {
-        if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
-                                   SYMBOL_DOMAIN (sym), domain))
+        if (ada_symbol_matches_domain (SYMBOL_DOMAIN (sym), domain))
           {
 	    if (SYMBOL_CLASS (sym) != LOC_UNRESOLVED)
 	      {
@@ -5901,8 +5936,7 @@ ada_add_block_symbols (struct obstack *obstackp,
 
       ALL_BLOCK_SYMBOLS (block, iter, sym)
       {
-        if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
-                                   SYMBOL_DOMAIN (sym), domain))
+        if (ada_symbol_matches_domain (SYMBOL_DOMAIN (sym), domain))
           {
             int cmp;
 
@@ -7486,11 +7520,12 @@ ada_find_any_type_symbol (const char *name)
   struct symbol *sym;
 
   sym = standard_lookup (name, get_selected_block (NULL), VAR_DOMAIN);
-  if (sym != NULL && SYMBOL_CLASS (sym) == LOC_TYPEDEF)
+  if (sym != NULL
+      && (SYMBOL_DOMAIN (sym) != VAR_DOMAIN
+	  || SYMBOL_CLASS (sym) == LOC_TYPEDEF))
     return sym;
 
-  sym = standard_lookup (name, NULL, STRUCT_DOMAIN);
-  return sym;
+  return NULL;
 }
 
 /* Find a type named NAME.  Ignores ambiguity.  This routine will look
diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 74ccd45..b5d564b 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -221,7 +221,12 @@ cp_is_anonymous (const char *namespace)
    we're looking for, BLOCK is the block that we're searching within,
    DOMAIN says what kind of symbols we're looking for, and if SYMTAB
    is non-NULL, we should store the symtab where we found the symbol
-   in it.  */
+   in it.
+
+   Class, union, and enum tag names may be used in C++ without specifying
+   class-key or enum.  If searching for a VAR_DOMAIN symbol fails,
+   this function will search STRUCT_DOMAIN.  [This is actually done in
+   cp_lookup_symbol_in_namespace.]  */
 
 struct symbol *
 cp_lookup_symbol_nonlocal (const char *name,
@@ -242,7 +247,10 @@ cp_lookup_symbol_nonlocal (const char *name,
 
 /* Look up NAME in the C++ namespace NAMESPACE.  Other arguments are
    as in cp_lookup_symbol_nonlocal.  If SEARCH is non-zero, search
-   through base classes for a matching symbol.  */
+   through base classes for a matching symbol.
+
+   If DOMAIN is VAR_DOMAIN and no matching symbol exists in that domain,
+   this function will search STRUCT_DOMAIN for a match.  */
 
 static struct symbol *
 cp_lookup_symbol_in_namespace (const char *namespace,
@@ -252,18 +260,30 @@ cp_lookup_symbol_in_namespace (const char *namespace,
 {
   if (namespace[0] == '\0')
     {
-      return lookup_symbol_file (name, block, domain, 0, search);
+      struct symbol *sym = lookup_symbol_file (name, block, domain, 0, search);
+
+      if (sym == NULL && domain == VAR_DOMAIN)
+	sym = lookup_symbol_file (name, block, STRUCT_DOMAIN, 0, search);
+
+      return sym;
     }
   else
     {
+      struct symbol *sym;
       char *concatenated_name = alloca (strlen (namespace) + 2
 					+ strlen (name) + 1);
 
       strcpy (concatenated_name, namespace);
       strcat (concatenated_name, "::");
       strcat (concatenated_name, name);
-      return lookup_symbol_file (concatenated_name, block, domain,
-				 cp_is_anonymous (namespace), search);
+      sym = lookup_symbol_file (concatenated_name, block, domain,
+				cp_is_anonymous (namespace), search);
+
+      if (sym == NULL && domain == VAR_DOMAIN)
+	sym = lookup_symbol_file (concatenated_name, block, STRUCT_DOMAIN,
+				  cp_is_anonymous (namespace), search);
+
+      return sym;
     }
 }
 
@@ -516,7 +536,12 @@ cp_lookup_symbol_imports_or_template (const char *scope,
  /* Searches for NAME in the current namespace, and by applying
     relevant import statements belonging to BLOCK and its parents.
     SCOPE is the namespace scope of the context in which the search is
-    being evaluated.  */
+    being evaluated.
+
+    Class, union, and enum tag names may be used in C++ without specifying
+    class-key or enum.  If searching for a VAR_DOMAIN symbol fails,
+    this function will search STRUCT_DOMAIN.  [This is done in
+    cp_lookup_symbol_in_namespace.]  */
 
 struct symbol*
 cp_lookup_symbol_namespace (const char *scope,
@@ -688,7 +713,7 @@ lookup_symbol_file (const char *name,
 
       /* Lookup a class named KLASS.  If none is found, there is nothing
 	 more that can be done.  */
-      klass_sym = lookup_symbol_global (klass, block, domain);
+      klass_sym = lookup_symbol_global (klass, block, STRUCT_DOMAIN);
       if (klass_sym == NULL)
 	{
 	  do_cleanups (cleanup);
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 2379b54..91533e8 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -35,6 +35,7 @@
 #include "expression.h"
 #include "value.h"
 #include "cp-abi.h"
+#include "language.h"
 
 #include "safe-ctype.h"
 
@@ -177,7 +178,29 @@ inspect_type (struct demangle_parse_info *info,
   sym = NULL;
   TRY_CATCH (except, RETURN_MASK_ALL)
   {
-    sym = lookup_symbol (name, 0, VAR_DOMAIN, 0);
+    /* It is not legal to have a typedef and tag name of the same
+       name in C++.  However, anonymous composite types that are defined
+       with a typedef ["typedef struct {...} anonymous_struct;"] WILL
+       have symbols for a TYPE_CODE_TYPEDEF (in VAR_DOMAIN) and a
+       TYPE_CODE_STRUCT (in STRUCT_DOMAIN).
+
+       If VAR_DOMAIN is searched first, it will return the TYPEDEF symbol,
+       and this function will never output the definition of the typedef,
+       since type_print is called below with SHOW = -1. [The typedef hash
+       is never initialized/used when SHOW <= 0 -- and the finder
+       (find_typedef_for_canonicalize) will always return NULL as a result.]
+
+       Consequently, type_print will eventually keep calling this function
+       to replace the typedef (via
+       print_name_maybe_canonical/cp_canonicalize_full).  This leads to
+       infinite recursion.
+
+       This can all be safely avoid by explicitly searching STRUCT_DOMAIN
+       first to find the structure definition.  */
+    if (current_language->la_language == language_cplus)
+      sym = lookup_symbol (name, 0, STRUCT_DOMAIN, 0);
+    if (sym == NULL)
+      sym = lookup_symbol (name, 0, VAR_DOMAIN, NULL);
   }
 
   if (except.reason >= 0 && sym != NULL)
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index 2787f4c..8f2de2a 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -594,8 +594,7 @@ match_partial_symbol (struct objfile *objfile,
       while (top <= real_top
 	     && match (SYMBOL_SEARCH_NAME (*top), name) == 0)
 	{
-	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*top),
-				     SYMBOL_DOMAIN (*top), domain))
+	  if (SYMBOL_DOMAIN (*top) == domain)
 	    return *top;
 	  top++;
 	}
@@ -608,8 +607,7 @@ match_partial_symbol (struct objfile *objfile,
     {
       for (psym = start; psym < start + length; psym++)
 	{
-	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*psym),
-				     SYMBOL_DOMAIN (*psym), domain)
+	  if (SYMBOL_DOMAIN (*psym) == domain
 	      && match (SYMBOL_SEARCH_NAME (*psym), name) == 0)
 	    return *psym;
 	}
@@ -725,8 +723,7 @@ lookup_partial_symbol (struct objfile *objfile,
 
       while (top <= real_top && SYMBOL_MATCHES_SEARCH_NAME (*top, search_name))
 	{
-	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*top),
-				     SYMBOL_DOMAIN (*top), domain))
+	  if (SYMBOL_DOMAIN (*top) == domain)
 	    {
 	      do_cleanups (cleanup);
 	      return (*top);
@@ -742,8 +739,7 @@ lookup_partial_symbol (struct objfile *objfile,
     {
       for (psym = start; psym < start + length; psym++)
 	{
-	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*psym),
-				     SYMBOL_DOMAIN (*psym), domain)
+	  if (SYMBOL_DOMAIN (*psym) == domain
 	      && SYMBOL_MATCHES_SEARCH_NAME (*psym, search_name))
 	    {
 	      do_cleanups (cleanup);
@@ -1223,8 +1219,7 @@ map_block (const char *name, domain_enum namespace, struct objfile *objfile,
   for (sym = block_iter_match_first (block, name, match, &iter);
        sym != NULL; sym = block_iter_match_next (name, match, &iter))
     {
-      if (symbol_matches_domain (SYMBOL_LANGUAGE (sym), 
-				 SYMBOL_DOMAIN (sym), namespace))
+      if (SYMBOL_DOMAIN (sym) == namespace)
 	{
 	  if (callback (block, sym, data))
 	    return 1;
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 66d1624..15ac3d1 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -1310,7 +1310,11 @@ demangle_for_lookup (const char *name, enum language lang,
    NAME is a field of the current implied argument `this'.  If so set
    *IS_A_FIELD_OF_THIS to 1, otherwise set it to zero.
    BLOCK_FOUND is set to the block in which NAME is found (in the case of
-   a field of `this', value_of_this sets BLOCK_FOUND to the proper value.)  */
+   a field of `this', value_of_this sets BLOCK_FOUND to the proper value.)
+
+   If DOMAIN is VAR_DOMAIN and the language permits using tag names for
+   elaborated types, such as classes in C++, this function will search
+   STRUCT_DOMAIN if no matching is found.  */
 
 /* This function (or rather its subordinates) have a bunch of loops and
    it would seem to be attractive to put in some QUIT's (though I'm not really
@@ -1333,6 +1337,23 @@ lookup_symbol_in_language (const char *name, const struct block *block,
 
   returnval = lookup_symbol_aux (modified_name, block, domain, lang,
 				 is_a_field_of_this);
+  if (returnval == NULL)
+    {
+      if (is_a_field_of_this != NULL
+	  && is_a_field_of_this->type != NULL)
+	return NULL;
+
+      /* Some languages define typedefs of a type equal to its tag name,
+	 e.g., in C++, "struct foo { ... }" also defines a typedef for
+	 "foo".  */
+      if (domain == VAR_DOMAIN
+	  && (lang == language_cplus || lang == language_java
+	      || lang == language_ada || lang == language_d))
+	{
+	  returnval = lookup_symbol_aux (modified_name, block, STRUCT_DOMAIN,
+					 lang, is_a_field_of_this);
+	}
+    }
   do_cleanups (cleanup);
 
   return returnval;
@@ -1907,27 +1928,6 @@ lookup_symbol_global (const char *name,
   return lookup_data.result;
 }
 
-int
-symbol_matches_domain (enum language symbol_language,
-		       domain_enum symbol_domain,
-		       domain_enum domain)
-{
-  /* For C++ "struct foo { ... }" also defines a typedef for "foo".
-     A Java class declaration also defines a typedef for the class.
-     Similarly, any Ada type declaration implicitly defines a typedef.  */
-  if (symbol_language == language_cplus
-      || symbol_language == language_d
-      || symbol_language == language_java
-      || symbol_language == language_ada)
-    {
-      if ((domain == VAR_DOMAIN || domain == STRUCT_DOMAIN)
-	  && symbol_domain == STRUCT_DOMAIN)
-	return 1;
-    }
-  /* For all other languages, strict match is required.  */
-  return (symbol_domain == domain);
-}
-
 /* Look up a type named NAME in the struct_domain.  The type returned
    must not be opaque -- i.e., must have at least one field
    defined.  */
@@ -2050,7 +2050,12 @@ basic_lookup_transparent_type (const char *name)
    binary search terminates, we drop through and do a straight linear
    search on the symbols.  Each symbol which is marked as being a ObjC/C++
    symbol (language_cplus or language_objc set) has both the encoded and
-   non-encoded names tested for a match.  */
+   non-encoded names tested for a match.
+
+   This function specifically disallows domain mismatches.  If a language
+   defines a typedef for an elaborated type, such as classes in C++,
+   then this function will need to be called twice, once to search
+   VAR_DOMAIN and once to search STRUCT_DOMAIN.  */
 
 struct symbol *
 lookup_block_symbol (const struct block *block, const char *name,
@@ -2065,8 +2070,7 @@ lookup_block_symbol (const struct block *block, const char *name,
 	   sym != NULL;
 	   sym = block_iter_name_next (name, &iter))
 	{
-	  if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
-				     SYMBOL_DOMAIN (sym), domain))
+	  if (SYMBOL_DOMAIN (sym) == domain)
 	    return sym;
 	}
       return NULL;
@@ -2085,8 +2089,7 @@ lookup_block_symbol (const struct block *block, const char *name,
 	   sym != NULL;
 	   sym = block_iter_name_next (name, &iter))
 	{
-	  if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
-				     SYMBOL_DOMAIN (sym), domain))
+	  if (SYMBOL_DOMAIN (sym) == domain)
 	    {
 	      sym_found = sym;
 	      if (!SYMBOL_IS_ARGUMENT (sym))
@@ -2120,8 +2123,7 @@ iterate_over_symbols (const struct block *block, const char *name,
        sym != NULL;
        sym = block_iter_name_next (name, &iter))
     {
-      if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
-				 SYMBOL_DOMAIN (sym), domain))
+      if (SYMBOL_DOMAIN (sym) == domain)
 	{
 	  if (!callback (sym, data))
 	    return;
diff --git a/gdb/symtab.h b/gdb/symtab.h
index fbe5868..efc3643 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1019,10 +1019,6 @@ extern const char multiple_symbols_cancel[];
 
 const char *multiple_symbols_select_mode (void);
 
-int symbol_matches_domain (enum language symbol_language, 
-			   domain_enum symbol_domain,
-			   domain_enum domain);
-
 /* lookup a symbol table by source file name.  */
 
 extern struct symtab *lookup_symtab (const char *);
diff --git a/gdb/testsuite/gdb.cp/var-tag.cc b/gdb/testsuite/gdb.cp/var-tag.cc
new file mode 100644
index 0000000..93b9caf
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/var-tag.cc
@@ -0,0 +1,44 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2014 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int global = 3;
+
+class C {
+public:
+  struct C1 {} C1;
+  enum E1 {a1, b1, c1} E1;
+  union U1 {int a1; char b1;} U1;
+
+  C () : E1 (b1) {}
+  void global (void) const {}
+  int f (void) const { global (); return 0; }
+} C;
+
+struct S {} S;
+enum E {a, b, c} E;
+union U {int a; char b;} U;
+
+class CC {} cc;
+struct SS {} ss;
+enum EE {ea, eb, ec} ee;
+union UU {int aa; char bb;} uu;
+
+int
+main (void)
+{
+  return C.f ();
+}
diff --git a/gdb/testsuite/gdb.cp/var-tag.exp b/gdb/testsuite/gdb.cp/var-tag.exp
new file mode 100644
index 0000000..7869fa2
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/var-tag.exp
@@ -0,0 +1,99 @@
+# Copyright 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite
+
+# Test expressions in which variable names shadow tag names.
+
+if {[skip_cplus_tests]} { continue }
+
+standard_testfile .cc
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug c++}]} {
+    return -1
+}
+
+proc do_global_tests {lang} {
+    if {$lang == "c"} {
+	set invalid_print "No symbol \"%s\" in current context."
+	set ptypefmt $invalid_print
+    } else {
+	set invalid_print "Attempt to use a type name as an expression"
+	set ptypefmt "type = (class|enum|union|struct) %s {.*}"
+    }
+
+    with_test_prefix $lang {
+	gdb_test_no_output "set language $lang"
+	gdb_test "ptype C" "type = class C {.*}"
+	gdb_test "print E" "= a"
+	gdb_test "ptype E" "type = enum E {.*}"
+	gdb_test "print S" "= {<No data fields>}"
+	gdb_test "ptype S" "type = struct S {.*}"
+	gdb_test "print U" "= {.*}"
+	gdb_test "ptype U" "type = union U {.*}"
+	gdb_test "print cc" "= {.*}"
+	gdb_test "ptype cc" "type = class CC {.*}"
+	gdb_test "print CC" [format $invalid_print "CC"]
+	gdb_test "ptype CC" [format $ptypefmt "CC"]
+	gdb_test "print ss" "= {<No data fields>}"
+	gdb_test "ptype ss" "type = struct SS {.*}"
+	gdb_test "print SS" [format $invalid_print "SS"]
+	gdb_test "ptype SS" [format $ptypefmt "SS"]
+	gdb_test "print ee" "= .*"
+	gdb_test "ptype ee" "type = enum EE {.*}"
+	gdb_test "print EE" [format $invalid_print "EE"]
+	gdb_test "ptype EE" [format $ptypefmt "EE"]
+	gdb_test "print uu" "= {.*}"
+	gdb_test "ptype uu" "type = union UU {.*}"
+	gdb_test "print UU" [format $invalid_print  "UU"]
+	gdb_test "ptype UU" [format $ptypefmt "UU"]
+    }
+}
+
+# First test expressions when there is no context.
+with_test_prefix "before start" {
+    do_global_tests c++
+    do_global_tests c
+}
+
+# Run to main and test again.
+if {![runto_main]} {
+    perror "couldn't run to main"
+    continue
+}
+
+with_test_prefix "in main" {
+    do_global_tests c++
+    do_global_tests c
+}
+
+# Finally run to C::f and test again
+gdb_breakpoint "C::f"
+gdb_continue_to_breakpoint "continue to C::f"
+with_test_prefix "in C::f" {
+    do_global_tests c++
+    do_global_tests c
+}
+
+# Another hard-to-guess-the-users-intent bug...
+# It would be really nice if we could query the user!
+with_test_prefix "global collision" {
+    gdb_test_no_output "set language c++"
+    setup_kfail "c++/16463" "*-*-*"
+    gdb_test "print global" "= 3"
+
+    # ... with a simple workaround:
+    gdb_test "print ::global" "= 3"
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ada-ffffffff.exp b/gdb/testsuite/gdb.dwarf2/dw2-ada-ffffffff.exp
index 1c1d10f..7936f25 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-ada-ffffffff.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ada-ffffffff.exp
@@ -28,6 +28,10 @@ if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" object {}] != ""
 
 clean_restart $executable
 
+# Force the language to C++, since we want to treat the type
+# defined in the object file like a C++ type, using sizeof.
+gdb_test_no_output "set language c++"
+
 # -1 was produced, it is now caught with the complaint:
 # Suspicious DW_AT_byte_size value treated as zero instead of ...
 gdb_test "p sizeof (t)" " = 0"
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-anon-mptr.exp b/gdb/testsuite/gdb.dwarf2/dw2-anon-mptr.exp
index aef3cb8..c9e59ed 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-anon-mptr.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-anon-mptr.exp
@@ -40,5 +40,7 @@ gdb_test "show cp-abi" {The currently selected C\+\+ ABI is "gnu-v3".*}
 
 gdb_load $binfile
 
+gdb_test_no_output "set language c++"
+
 gdb_test "ptype crash" \
     "type = class crash {\[\r\n \t\]*public:\[\r\n \t\]*crash\\(int \\(class {\\.\\.\\.}::\\*\\)\\(class {\\.\\.\\.} \\* const\\)\\);\[\r\n \t\]*}"
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-double-set-die-type.exp b/gdb/testsuite/gdb.dwarf2/dw2-double-set-die-type.exp
index 7aabcfe..40daed9 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-double-set-die-type.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-double-set-die-type.exp
@@ -30,4 +30,5 @@ if  { [gdb_compile [file join $srcdir $subdir $srcfile] $binfile \
 }
 
 clean_restart $testfile
+gdb_test_no_output "set language c++"
 gdb_test "ptype a" "type = class .*"
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inheritance.exp b/gdb/testsuite/gdb.dwarf2/dw2-inheritance.exp
index 7c954bb..028a1df 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-inheritance.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-inheritance.exp
@@ -31,4 +31,5 @@ if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" $binfile \
 
 clean_restart $testfile
 
+gdb_test_no_output "set language c++"
 gdb_test "ptype inherited" "type = class inherited .*"

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFA] Fix c++/16253 (tag/variable name collision)
  2014-04-10 18:54   ` Keith Seitz
@ 2014-04-10 20:29     ` Tom Tromey
  2014-05-28 17:55       ` Doug Evans
  2014-04-17 19:03     ` Regression for gdb.cp/koenig.exp: p entry (c) [Re: [RFA] Fix c++/16253 (tag/variable name collision)] Jan Kratochvil
  1 sibling, 1 reply; 11+ messages in thread
From: Tom Tromey @ 2014-04-10 20:29 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> I am reposting the entire patch for review. The only difference
Keith> between this and the original version is the exclusion of the patch
Keith> mentioned above.

Thanks Keith.

I like this patch quite a lot.
It gets rid of symbol_matches_domain -- that's fantastic.

Please check it in.

Tom

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Regression for gdb.cp/koenig.exp: p entry (c)  [Re: [RFA] Fix c++/16253 (tag/variable name collision)]
  2014-04-10 18:54   ` Keith Seitz
  2014-04-10 20:29     ` Tom Tromey
@ 2014-04-17 19:03     ` Jan Kratochvil
  1 sibling, 0 replies; 11+ messages in thread
From: Jan Kratochvil @ 2014-04-17 19:03 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches

Hi Keith,

b50c861487bb7d71185777193a9246bac81e4f26 is the first bad commit
commit b50c861487bb7d71185777193a9246bac81e4f26
Author: Keith Seitz <keiths@redhat.com>
Date:   Mon Apr 14 15:47:15 2014 -0700
    Remove symbol_matches_domain. This fixes
    PR c++/16253.

FAIL: gdb.cp/koenig.exp: p entry (c)

p entry (c)
A syntax error in expression, near `c)'.
(gdb) FAIL: gdb.cp/koenig.exp: p entry (c)

Reproducible on Fedora 20 x86_64.


Jan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFA] Fix c++/16253 (tag/variable name collision)
  2014-04-10 20:29     ` Tom Tromey
@ 2014-05-28 17:55       ` Doug Evans
  0 siblings, 0 replies; 11+ messages in thread
From: Doug Evans @ 2014-05-28 17:55 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Keith Seitz, gdb-patches

On Thu, Apr 10, 2014 at 1:29 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:
>
> Keith> I am reposting the entire patch for review. The only difference
> Keith> between this and the original version is the exclusion of the patch
> Keith> mentioned above.
>
> Thanks Keith.
>
> I like this patch quite a lot.
> It gets rid of symbol_matches_domain -- that's fantastic.

Awesome indeed.

Alas it trips over gdb's sucky symbol lookup and introduces a perf regression.
info fun ^foo::(anonymous namespace)
goes from about a minute to a very long time (longer than I wanted to wait :-)).
["foo" is renamed to protect the innocent.  1/2 :-)]

I've reopened 16253 and filed 16994.

One possibility is to revert the patch for 7.8.
I've confirmed doing that removes the perf regression.
I'd like to see if we can take a small step to fixing gdb's symbol
lookup that will fix the perf regression.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2014-05-28 17:55 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-21 18:12 [RFA] Fix c++/16253 (tag/variable name collision) Keith Seitz
2014-03-24 14:15 ` Joel Brobecker
2014-03-25 17:50   ` Joel Brobecker
2014-03-25 18:11   ` Keith Seitz
2014-03-26 13:32     ` Joel Brobecker
2014-03-26 13:40 ` Joel Brobecker
2014-03-26 15:34   ` Keith Seitz
2014-04-10 18:54   ` Keith Seitz
2014-04-10 20:29     ` Tom Tromey
2014-05-28 17:55       ` Doug Evans
2014-04-17 19:03     ` Regression for gdb.cp/koenig.exp: p entry (c) [Re: [RFA] Fix c++/16253 (tag/variable name collision)] Jan Kratochvil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).