[rfa] search for fields of this correctly, PR gdb/804

[David Carlton <carlton at math dot stanford dot edu>]

In lookup_symbol_aux, the 'field_of_this' search happens at the wrong
time: it happens after all of the superblocks of 'block' have been
searched, whereas fields of this should shadow file-level static and
global variables.

Here's a patch to fix this, together with a check for it in the
testsuite.  The idea is as follows.

* Modify lookup_symbol_aux_local to stop once it hits a static block.
  It also stores the address of that static block in a new argument to
  it, 'static_block'.

* Pull out the code from lookup_symbol_aux_local that sets 'symtab'
  correctly and that does the fixup_symbol stuff into a new function,
  lookup_symbol_aux_block.

* In lookup_symbol_aux, after doing the field_of_this stuff, call
  lookup_symbol_aux_block to search in the static block.

I waffled a bit as to whether or not we should preserve the current
behavior of searching the current file's global block before searching
all of the global blocks; I decided that doing so was redundant and a
bit messy, so I omitted that but left in a comment mentioning the
issue.

I think the testsuite stuff is pretty straightforward; the current
version of m-data.exp was doing breakpoints on line numbers, which is
really fragile, so I modified that to look for a comment, like I did
when I changed m-static.exp a couple of months ago.

This patch is orthogonal to my lookup_symbol_aux_minsym patch that is
currently under review: they touch different parts of
lookup_symbol_aux.

David Carlton
carlton@math.stanford.edu

2002-11-13  David Carlton  <carlton@math.stanford.edu>

	* symtab.c (lookup_symbol_aux_block): New function.
	(lookup_symbol_aux_local): Move code into lookup_symbol_aux_block;
	add 'static_block' argument.
	(lookup_symbol_aux): Do the 'field_of_this' check before checking
	the static block.  See PR gdb/804.

2002-11-13  David Carlton  <carlton@math.stanford.edu>

	* gdb.c++/m-data.exp: Add test for members that shadow global
	variables: see PR gdb/804.
	* gdb.c++/m-data.cc: Ditto.

Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.78
diff -u -p -r1.78 symtab.c
--- symtab.c	6 Nov 2002 23:27:58 -0000	1.78
+++ symtab.c	13 Nov 2002 18:17:00 -0000
@@ -83,11 +83,20 @@ static struct symbol *lookup_symbol_aux 
 					 int *is_a_field_of_this,
 					 struct symtab **symtab);
 
-static struct symbol *lookup_symbol_aux_local (const char *name,
-					       const char *mangled_name,
-					       const struct block *block,
-					       const namespace_enum namespace,
-					       struct symtab **symtab);
+static
+struct symbol *lookup_symbol_aux_local (const char *name,
+					const char *mangled_name,
+					const struct block *block,
+					const namespace_enum namespace,
+					struct symtab **symtab,
+					const struct block **static_block);
+
+static
+struct symbol *lookup_symbol_aux_block (const char *name,
+					const char *mangled_name,
+					const struct block *block,
+					const namespace_enum namespace,
+					struct symtab **symtab);
 
 static
 struct symbol *lookup_symbol_aux_symtabs (int block_index,
@@ -789,11 +798,13 @@ lookup_symbol_aux (const char *name, con
   struct symtab *s = NULL;
   struct blockvector *bv;
   struct minimal_symbol *msymbol;
+  const struct block *static_block;
 
-  /* Search specified block and its superiors.  */
+  /* Search specified block and its superiors.  Don't search
+     STATIC_BLOCK or GLOBAL_BLOCK.  */
 
   sym = lookup_symbol_aux_local (name, mangled_name, block, namespace,
-				 symtab);
+				 symtab, &static_block);
   if (sym != NULL)
     return sym;
 
@@ -859,6 +870,28 @@ lookup_symbol_aux (const char *name, con
 	}
     }
 
+  /* If there's a static block to search, search it next.  */
+
+  /* NOTE: carlton/2002-11-13: There is a question as to whether or
+     not it would be appropriate to search the current global block
+     here as well.  On the one hand, it's redundant with the
+     lookup_symbol_aux_symtabs search that happens next.  On the other
+     hand, if decode_line_1 is passed an argument like filename:var,
+     then the user presumably wants 'var' to be searched for in
+     filename.  On the third hand, there shouldn't be multiple global
+     variables all of which are named 'var', and it's not like
+     decode_line_1 has ever restricted its search to only global
+     variables in a single filename.  All in all, only searching the
+     static block here seems best: it's correct and it's cleanest.  */
+
+  if (static_block != NULL)
+    {
+      sym = lookup_symbol_aux_block (name, mangled_name, static_block,
+				     namespace, symtab);
+      if (sym != NULL)
+	return sym;
+    }
+
   /* Now search all global blocks.  Do the symtab's first, then
      check the psymtab's. If a psymtab indicates the existence
      of the desired name as a global, then do psymtab-to-symtab
@@ -1064,13 +1097,49 @@ lookup_symbol_aux (const char *name, con
   return NULL;
 }
 
-/* Check to see if the symbol is defined in BLOCK or its
-   superiors.  */
+/* Check to see if the symbol is defined in BLOCK or its superiors.
+   Don't search STATIC_BLOCK or GLOBAL_BLOCK.  If we don't find a
+   match, store the address of STATIC_BLOCK in static_block.  */
 
 static struct symbol *
 lookup_symbol_aux_local (const char *name, const char *mangled_name,
 			 const struct block *block,
 			 const namespace_enum namespace,
+			 struct symtab **symtab,
+			 const struct block **static_block)
+{
+  struct symbol *sym;
+  
+  /* Check if either no block is specified or it's a global block.  */
+
+  if (block == NULL || BLOCK_SUPERBLOCK (block) == NULL)
+    {
+      *static_block = NULL;
+      return NULL;
+    }
+
+  while (BLOCK_SUPERBLOCK (BLOCK_SUPERBLOCK (block)) != NULL)
+    {
+      sym = lookup_symbol_aux_block (name, mangled_name, block, namespace,
+				     symtab);
+      if (sym != NULL)
+	return sym;
+      block = BLOCK_SUPERBLOCK (block);
+    }
+
+  /* We've reached the static block.  */
+
+  *static_block = block;
+  return NULL;
+}
+
+/* Look up a symbol in a block; if found, locate its symtab, fixup the
+   symbol, and set block_found appropriately.  */
+
+static struct symbol *
+lookup_symbol_aux_block (const char *name, const char *mangled_name,
+			 const struct block *block,
+			 const namespace_enum namespace,
 			 struct symtab **symtab)
 {
   struct symbol *sym;
@@ -1078,32 +1147,28 @@ lookup_symbol_aux_local (const char *nam
   struct blockvector *bv;
   struct block *b;
   struct symtab *s = NULL;
-  
-  while (block != 0)
+
+  sym = lookup_block_symbol (block, name, mangled_name, namespace);
+  if (sym)
     {
-      sym = lookup_block_symbol (block, name, mangled_name, namespace);
-      if (sym)
+      block_found = block;
+      if (symtab != NULL)
 	{
-	  block_found = block;
-	  if (symtab != NULL)
+	  /* Search the list of symtabs for one which contains the
+	     address of the start of this block.  */
+	  ALL_SYMTABS (objfile, s)
 	    {
-	      /* Search the list of symtabs for one which contains the
-	         address of the start of this block.  */
-	      ALL_SYMTABS (objfile, s)
-	      {
-		bv = BLOCKVECTOR (s);
-		b = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
-		if (BLOCK_START (b) <= BLOCK_START (block)
-		    && BLOCK_END (b) > BLOCK_START (block))
-		  goto found;
-	      }
-	    found:
-	      *symtab = s;
+	      bv = BLOCKVECTOR (s);
+	      b = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
+	      if (BLOCK_START (b) <= BLOCK_START (block)
+		  && BLOCK_END (b) > BLOCK_START (block))
+		goto found;
 	    }
-
-	  return fixup_symbol_section (sym, objfile);
+	found:
+	  *symtab = s;
 	}
-      block = BLOCK_SUPERBLOCK (block);
+      
+      return fixup_symbol_section (sym, objfile);
     }
 
   return NULL;
Index: m-data.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.c++/m-data.exp,v
retrieving revision 1.1
diff -u -p -r1.1 m-data.exp
--- m-data.exp	27 May 2002 21:46:52 -0000	1.1
+++ m-data.exp	13 Nov 2002 17:44:41 -0000
@@ -54,9 +54,12 @@ if ![runto_main] then {
     continue
 }
 
+# First, run to after we've constructed all the gnu_obj_N's:
+
+gdb_breakpoint [gdb_get_line_number "first-constructs-done"]
+gdb_continue_to_breakpoint "end of first constructors"
+
 # One.
-gdb_test "break 50" "Breakpoint \[0-9\]*.*line 50\\."
-gdb_test "continue" "Continuing\\.\r\n\r\nBreakpoint.*at.*m-data\\.cc:50\r\n.*" "continue to 50"
 
 # simple object, const bool
 gdb_test "print test1.test" "\\$\[0-9\]* = true" "simple object, const bool"
@@ -71,8 +74,6 @@ gdb_test "print test1.key2" "\\$\[0-9\]*
 gdb_test "print test1.value" "\\$\[0-9\]* = egyptian" "simple object, enum"
 
 # Two.
-gdb_test "break 51" "Breakpoint \[0-9\]*.*line 51\\."
-gdb_test "continue" "Continuing\\.\r\n\r\nBreakpoint.*at.*m-data\\.cc:51\r\n.*" "continue to 51"
 
 # derived template object, base const bool
 gdb_test "print test2.test" "\\$\[0-9\]* = true" "derived template object, base const bool"
@@ -90,8 +91,6 @@ gdb_test "print test2.value" "\\$\[0-9\]
 gdb_test "print test2.value_derived" "\\$\[0-9\]* = roman" "derived template object, derived enum"
 
 # Three.
-gdb_test "break 52" "Breakpoint \[0-9\]*.*line 52\\."
-gdb_test "continue" "Continuing\\.\r\n\r\nBreakpoint.*at.*m-data\\.cc:52\r\n.*" "continue to 52"
 
 # template object, derived template data member's base const bool
 gdb_test "print test3.data.test" "\\$\[0-9\]* = true" "template object, const bool"
@@ -107,6 +106,14 @@ gdb_test "print test3.data.value" "\\$\[
 
 # template object, derived template data member's enum
 gdb_test "print test3.data.value_derived" "\\$\[0-9]\* = etruscan" "template object, derived enum"
+
+# Now some tests for shadowing (see PR gdb/804):
+
+gdb_breakpoint "C::marker"
+gdb_continue_to_breakpoint "continue to shadow breakpoint"
+
+gdb_test "print shadow" "\\$\[0-9]\* = 1" "shadowing member"
+gdb_test "print ::shadow" "\\$\[0-9]\* = 0" "shadowed global variable"
 
 gdb_exit
 return 0
Index: m-data.cc
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.c++/m-data.cc,v
retrieving revision 1.1
diff -u -p -r1.1 m-data.cc
--- m-data.cc	27 May 2002 21:46:52 -0000	1.1
+++ m-data.cc	13 Nov 2002 17:44:37 -0000
@@ -41,7 +41,18 @@ namespace __gnu_test
     public:
       gnu_obj_3(antiquities b): data(etruscan) { }
     }; 
-} 
+}
+
+int shadow = 0;
+
+class C
+{
+public:
+  C (int x) : shadow (x) {}
+  void marker () {}
+private:
+  int shadow;
+};
 
 int main()
 {
@@ -49,5 +60,9 @@ int main()
   gnu_obj_1		test1(egyptian, 4589);
   gnu_obj_2<long>	test2(roman);
   gnu_obj_3<long>	test3(greek);
+
+  C theC (1);				// breakpoint: first-constructs-done
+  theC.marker ();
+  
   return 0;
 }