Re: [RFC] Revisit PR 16253 ("Attempt to use a type name...")

[Doug Evans <dje at google dot com>]

Keith Seitz writes:
 > On 06/17/2015 08:50 AM, Keith Seitz wrote:
 > > On 06/17/2015 05:33 AM, Jan Kratochvil wrote:
 > >> On Thu, 11 Jun 2015 20:57:18 +0200, Keith Seitz wrote:
 > >>> 	PR 16253
 > >>> 	* block.c (block_lookup_symbol_primary): If a symbol is found
 > >>> 	which does not exactly match the requested domain, keep searching
 > >>> 	for an exact match.  Otherwise, return the previously found "best"
 > >>> 	symbol.
 > >>
 > >> Is there a reason why you haven't patched also block_lookup_symbol?
 > >
 > > Two reasons:
 > >
 > > 1) I posted this RFC to raise discussion whether this was worth
 > > pursuing. No need to expend any energy on something that has zero  
chance
 > > of being accepted by maintainers.
 > >
 > > 2) More important I have not discovered/attempted to coverage test
 > > lookup_block_symbol to trigger this bug.
 > >
 > > I'll be attempting to do that today.
 >
 > And I failed. I've not found the "magic" combination of buttons to make
 > any difference in block_lookup_symbol.

A variation of PR 18150?
You need to put the symbols in an unexpanded symtab,
the catch being that gdb expands the symtab with main() at startup.

Lookup in expanded symtabs, which is done before looking up in
unexpanded symtabs, is done with block_lookup_symbol_primary.
Lookup in unexpanded symtabs is done with block_lookup_symbol.

 > Nonetheless it is not quite so straight-forward in the BLOCK_FUNCTION
 > case where we have to decide what is a "better" match:
 >
 >   SYMBOL_DOMAIN == domain && SYMBOL_IS_ARGUMENT
 >
 > or
 >   SYMBOL_DOMAIN != domain (but symbol_matches_domain returns 1) &&
 > !SYMBOL_IS_ARGUMENT

I'm not sure either. I'm not sure the BLOCK_FUNCTION case
can even exercise this bug.

 > In that case, I cannot say which is more correct. Moreover I have been
 > unable to figure out how to test this. I worry that I would simply be
 > introducing a regression. IMO this is getting into "risky" territory.
 > [But then my philosophy is "if it ain't broke, don't fix it." As far as
 > I can tell, block_lookup_symbol is not "broke."]
 >
 > So what do maintainers want me to do?

How about this?

diff --git a/gdb/block.c b/gdb/block.c
index 79a8f19..93ed6e7 100644
--- a/gdb/block.c
+++ b/gdb/block.c
@@ -739,13 +739,21 @@ block_lookup_symbol (const struct block *block, const  
char *name,

   if (!BLOCK_FUNCTION (block))
     {
+      struct symbol *other = NULL;
+
       ALL_BLOCK_SYMBOLS_WITH_NAME (block, name, iter, sym)
 	{
+	  if (SYMBOL_DOMAIN (sym) == domain)
+	    return sym;
+	  /* This is a bit of a hack, but symbol_matches_domain might ignore
+	     STRUCT vs VAR domain symbols.  So if a matching symbol is found,
+	     make sure there is no "better" matching symbol, i.e., one with
+	     exactly the same domain.  PR 16253.  */
 	  if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
 				     SYMBOL_DOMAIN (sym), domain))
-	    return sym;
+	    other = sym;
 	}
-      return NULL;
+      return other;
     }
   else
     {
@@ -753,7 +761,10 @@ block_lookup_symbol (const struct block *block, const  
char *name,
 	 list; this loop makes sure to take anything else other than
 	 parameter symbols first; it only uses parameter symbols as a
 	 last resort.  Note that this only takes up extra computation
-	 time on a match.  */
+	 time on a match.
+	 It's hard to define types in the parameter list (at least in
+	 C/C++) so we don't do the same PR 16253 hack here that is done
+	 for the !BLOCK_FUNCTION case.  */

       struct symbol *sym_found = NULL;

@@ -779,23 +790,33 @@ struct symbol *
 block_lookup_symbol_primary (const struct block *block, const char *name,
 			     const domain_enum domain)
 {
-  struct symbol *sym;
+  struct symbol *sym, *other;
   struct dict_iterator dict_iter;

   /* Verify BLOCK is STATIC_BLOCK or GLOBAL_BLOCK.  */
   gdb_assert (BLOCK_SUPERBLOCK (block) == NULL
 	      || BLOCK_SUPERBLOCK (BLOCK_SUPERBLOCK (block)) == NULL);

+  other = NULL;
   for (sym = dict_iter_name_first (block->dict, name, &dict_iter);
        sym != NULL;
        sym = dict_iter_name_next (name, &dict_iter))
     {
+      if (SYMBOL_DOMAIN (sym) == domain)
+	return sym;
+
+      /* This is a bit of a hack, but symbol_matches_domain might ignore
+	 STRUCT vs VAR domain symbols.  So if a matching symbol is found,
+	 make sure there is no "better" matching symbol, i.e., one with
+	 exactly the same domain.  PR 16253.  */
       if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
 				 SYMBOL_DOMAIN (sym), domain))
-	return sym;
+	{
+	  other = sym;
+	}
     }

-  return NULL;
+  return other;
 }

 /* See block.h.  */
diff --git a/gdb/testsuite/gdb.cp/var-tag-2.cc  
b/gdb/testsuite/gdb.cp/var-tag-2.cc
new file mode 100644
index 0000000..7733473
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/var-tag-2.cc
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 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 object is in a separate file so that its debug info is not
+   expanded at startup.  Once debug info is expanded we are no longer
+   exercising block_lookup_symbol, and instead are exercising
+   block_lookup_symbol_primary.  */
+enum E2 {a2, b2, c2} E2;
diff --git a/gdb/testsuite/gdb.cp/var-tag-3.cc  
b/gdb/testsuite/gdb.cp/var-tag-3.cc
new file mode 100644
index 0000000..7f2133f
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/var-tag-3.cc
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 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 object is in a separate file so that its debug info is not
+   expanded at startup.  Once debug info is expanded we are no longer
+   exercising block_lookup_symbol, and instead are exercising
+   block_lookup_symbol_primary.  */
+struct S2 {} S2;
diff --git a/gdb/testsuite/gdb.cp/var-tag-4.cc  
b/gdb/testsuite/gdb.cp/var-tag-4.cc
new file mode 100644
index 0000000..162541c
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/var-tag-4.cc
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 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 object is in a separate file so that its debug info is not
+   expanded at startup.  Once debug info is expanded we are no longer
+   exercising block_lookup_symbol, and instead are exercising
+   block_lookup_symbol_primary.  */
+union U2 {int a; char b;} U2;
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..9854c31
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/var-tag.exp
@@ -0,0 +1,105 @@
+# Copyright 2014, 2015 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 var-tag.cc var-tag-2.cc var-tag-3.cc var-tag-4.cc
+
+if {[prepare_for_testing $testfile.exp $testfile \
+	[list $srcfile $srcfile2 $srcfile3 $srcfile4] {debug c++}]} {
+    return -1
+}
+
+proc do_global_tests {lang} {
+    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"]
+
+	# These tests exercise lookup of symbols using the "quick fns" API.
+	# Each of them is in a separate CU as once its CU is expanded,
+	# we're no longer using the quick fns API.
+	gdb_test "print E2" "= a2"
+	gdb_test "ptype E2" "type = enum E2 {.*}"
+	gdb_test "print S2" "= {<No data fields>}"
+	gdb_test "ptype S2" "type = struct S2 {.*}"
+	gdb_test "print U2" "= {.*}"
+	gdb_test "ptype U2" "type = union U2 {.*}"
+    }
+}
+
+# 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
+}
+
+# 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"
+}