This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [rfa] search for fields of this correctly, PR gdb/804
- From: Elena Zannoni <ezannoni at redhat dot com>
- To: David Carlton <carlton at math dot stanford dot edu>
- Cc: gdb-patches at sources dot redhat dot com, Elena Zannoni <ezannoni at redhat dot com>, Jim Blandy <jimb at redhat dot com>, Fernando Nasser <fnasser at redhat dot com>
- Date: Wed, 4 Dec 2002 22:20:37 -0500
- Subject: Re: [rfa] search for fields of this correctly, PR gdb/804
- References: <ro1y97xuxof.fsf@jackfruit.Stanford.EDU>
David Carlton writes:
> 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.
>
I see, the search stops as soon as you find something, so doing it in
that order would find the 'outer' first.
> 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'.
>
ah, I see what you did. It took me a while, must confess. You changed
the order from:
lookup blocks inner->outer included global and static.
test for is_a_field_of_this
lookup in symtabs and psymtabs
to:
lookup blocks inner->outer excluded global and static.
test for is_a_field_of_this
lookup in static block
lookup in symtabs and psymtabs
makes sense. But would the initial version of lookup_symbol_aux_local
look also in global block? Hmm,...I understand your doubts below. I
think you are right, the looking in the global block symtabs that
comes right after seems to take care of that.
> * 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.
>
Since this bit is just moving code around, would you mind it checking
it in first, by itself? Then check in the rest.
> * 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.
The only thing I wonder is if there's going to be a performance
penalty for this. Can you mention in the comment that there was old
code that was looking for symbols in the current file's global block?
>
> 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.
>
looks reasonable.
> This patch is orthogonal to my lookup_symbol_aux_minsym patch that is
> currently under review: they touch different parts of
> lookup_symbol_aux.
>
Ok, go ahead.
It would be really nice if that 'goto' would disappear....hint, hint.
Elena
> 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;
> }