Re: [RFA] Select a particular mangling of a demangled symbol in lookup_block_symbol

Daniel Jacobowitz
2002-03-19T19:53:45Z

Could someone please review this patch from last month? Thanks.

On Thu, Feb 14, 2002 at 06:55:03PM -0500, Daniel Jacobowitz wrote:
> I just described the problem this patch addresses in my testsuite patch;
> perhaps not the best place :) Here's the relevant bit:
>
> - Multiple symbols with the same demangled name. We can work around this
> for stabs, because we have the physname. We don't have that option for
> DWARF-2, and we shouldn't need to for stabs. I have a patch for the
> workaround. We get the [not-in-charge] constructor by default,
> unfortunately.
>
>
> What this means is that we call lookup_block_symbol on something like:
> _ZN3fooC1ERS_
> but get the information for:
> _ZN3fooC2ERS_
>
> The breakpoint ends up on the base-not-in-charge constructor.
>
> What we really SHOULD do is set it on both constructors silently, without
> even acknowledging that they are different functions, or else offer the user
> the choice. My preference is actually for the former. That requires
> support for a single function existing in multiple places, which will also
> give us nice things like better support for inlined functions with DWARF-2
> (which will always be somewhat shoddy due to the nature of inlining, in that
> it only occurs with lots of other optimization - but we can do much better
> than we do).
>
> Is this patch OK, or is it deemed too gross?
>
> --
> Daniel Jacobowitz Carnegie Mellon University
> MontaVista Software Debian GNU/Linux Developer
>
> 2002-02-14 Daniel Jacobowitz <drow@mvista.com>
>
> * symtab.h (lookup_block_symbol): Add mangled_name argument
> to prototype.
>
> * symmisc.c (maintenance_check_symtabs): Call lookup_block_symbol
> with new mangled_name argument.
> * linespec.c (decode_line_1): Likewise.
> * valops (value_of_this): Likewise.
> * symtab.c (lookup_transparent_type): Likewise.
> (lookup_symbol_aux): Likewise. Accept new mangled_name argument.
> (lookup_symbol): If we are given a mangled name, pass it down
> to lookup_symbol_aux.
> (lookup_block_symbol): If we are given a mangled name to check
> against, only return symbols which match it.
>
> Index: linespec.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/linespec.c,v
> retrieving revision 1.14
> diff -p -u -r1.14 linespec.c
> --- linespec.c 2002/02/02 03:42:58 1.14
> +++ linespec.c 2002/02/14 23:34:24
> @@ -1180,7 +1180,7 @@ symbol_found: /* We also jump here fro
> {
> struct blockvector *bv = BLOCKVECTOR (sym_symtab);
> struct block *b = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
> - if (lookup_block_symbol (b, copy, VAR_NAMESPACE) != NULL)
> + if (lookup_block_symbol (b, copy, NULL, VAR_NAMESPACE) != NULL)
> build_canonical_line_spec (values.sals, copy, canonical);
> }
> return values;
> Index: symmisc.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/symmisc.c,v
> retrieving revision 1.7
> diff -p -u -r1.7 symmisc.c
> --- symmisc.c 2001/12/02 22:38:23 1.7
> +++ symmisc.c 2002/02/14 23:34:24
> @@ -959,7 +959,7 @@ maintenance_check_symtabs (char *ignore,
> while (length--)
> {
> sym = lookup_block_symbol (b, SYMBOL_NAME (*psym),
> - SYMBOL_NAMESPACE (*psym));
> + NULL, SYMBOL_NAMESPACE (*psym));
> if (!sym)
> {
> printf_filtered ("Static symbol `");
> @@ -976,7 +976,7 @@ maintenance_check_symtabs (char *ignore,
> while (length--)
> {
> sym = lookup_block_symbol (b, SYMBOL_NAME (*psym),
> - SYMBOL_NAMESPACE (*psym));
> + NULL, SYMBOL_NAMESPACE (*psym));
> if (!sym)
> {
> printf_filtered ("Global symbol `");
> Index: symtab.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/symtab.c,v
> retrieving revision 1.54
> diff -p -u -r1.54 symtab.c
> --- symtab.c 2002/02/11 03:21:53 1.54
> +++ symtab.c 2002/02/14 23:34:24
> @@ -80,11 +80,12 @@ static struct partial_symbol *lookup_par
> const char *, int,
> namespace_enum);
>
> -static struct symbol *lookup_symbol_aux (const char *name, const
> - struct block *block, const
> - namespace_enum namespace, int
> - *is_a_field_of_this, struct
> - symtab **symtab);
> +static struct symbol *lookup_symbol_aux (const char *name,
> + const char *mangled_name,
> + const struct block *block,
> + const namespace_enum namespace,
> + int *is_a_field_of_this,
> + struct symtab **symtab);
>
>
> static struct symbol *find_active_alias (struct symbol *sym, CORE_ADDR addr);
> @@ -567,6 +568,7 @@ lookup_symbol (const char *name, const s
> {
> char *modified_name = NULL;
> char *modified_name2 = NULL;
> + const char *mangled_name = NULL;
> int needtofreename = 0;
> struct symbol *returnval;
>
> @@ -592,13 +594,14 @@ lookup_symbol (const char *name, const s
> modified_name2 = cplus_demangle (modified_name, DMGL_ANSI | DMGL_PARAMS);
> if (modified_name2)
> {
> + mangled_name = name;
> modified_name = modified_name2;
> needtofreename = 1;
> }
> }
>
> - returnval = lookup_symbol_aux (modified_name, block, namespace,
> - is_a_field_of_this, symtab);
> + returnval = lookup_symbol_aux (modified_name, mangled_name, block,
> + namespace, is_a_field_of_this, symtab);
> if (needtofreename)
> xfree (modified_name2);
>
> @@ -606,9 +609,9 @@ lookup_symbol (const char *name, const s
> }
>
> static struct symbol *
> -lookup_symbol_aux (const char *name, const struct block *block,
> - const namespace_enum namespace, int *is_a_field_of_this,
> - struct symtab **symtab)
> +lookup_symbol_aux (const char *name, const char *mangled_name,
> + const struct block *block, const namespace_enum namespace,
> + int *is_a_field_of_this, struct symtab **symtab)
> {
> register struct symbol *sym;
> register struct symtab *s = NULL;
> @@ -623,7 +626,7 @@ lookup_symbol_aux (const char *name, con
>
> while (block != 0)
> {
> - sym = lookup_block_symbol (block, name, namespace);
> + sym = lookup_block_symbol (block, name, mangled_name, namespace);
> if (sym)
> {
> block_found = block;
> @@ -676,7 +679,7 @@ lookup_symbol_aux (const char *name, con
> if (BLOCK_START (b) <= BLOCK_START (block)
> && BLOCK_END (b) > BLOCK_START (block))
> {
> - sym = lookup_block_symbol (b, name, VAR_NAMESPACE);
> + sym = lookup_block_symbol (b, name, mangled_name, VAR_NAMESPACE);
> if (sym)
> {
> block_found = b;
> @@ -714,7 +717,7 @@ lookup_symbol_aux (const char *name, con
> {
> bv = BLOCKVECTOR (s);
> block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> - sym = lookup_block_symbol (block, name, namespace);
> + sym = lookup_block_symbol (block, name, mangled_name, namespace);
> if (sym)
> {
> block_found = block;
> @@ -743,14 +746,14 @@ lookup_symbol_aux (const char *name, con
> bv = BLOCKVECTOR (s);
> block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
> - namespace);
> + mangled_name, namespace);
> /* We kept static functions in minimal symbol table as well as
> in static scope. We want to find them in the symbol table. */
> if (!sym)
> {
> block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
> sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
> - namespace);
> + mangled_name, namespace);
> }
>
> /* sym == 0 if symbol was found in the minimal symbol table
> @@ -776,7 +779,7 @@ lookup_symbol_aux (const char *name, con
> {
> /* This is a mangled variable, look it up by its
> mangled name. */
> - return lookup_symbol_aux (SYMBOL_NAME (msymbol), block,
> + return lookup_symbol_aux (SYMBOL_NAME (msymbol), mangled_name, block,
> namespace, is_a_field_of_this, symtab);
> }
> /* There are no debug symbols for this file, or we are looking
> @@ -794,7 +797,7 @@ lookup_symbol_aux (const char *name, con
> s = PSYMTAB_TO_SYMTAB (ps);
> bv = BLOCKVECTOR (s);
> block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> - sym = lookup_block_symbol (block, name, namespace);
> + sym = lookup_block_symbol (block, name, mangled_name, namespace);
> if (!sym)
> {
> /* This shouldn't be necessary, but as a last resort
> @@ -803,7 +806,7 @@ lookup_symbol_aux (const char *name, con
> * the psymtab gets it wrong in some cases.
> */
> block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
> - sym = lookup_block_symbol (block, name, namespace);
> + sym = lookup_block_symbol (block, name, mangled_name, namespace);
> if (!sym)
> error ("Internal: global symbol `%s' found in %s psymtab but not in symtab.\n\
> %s may be an inlined function, or may be a template function\n\
> @@ -827,7 +830,7 @@ lookup_symbol_aux (const char *name, con
> {
> bv = BLOCKVECTOR (s);
> block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
> - sym = lookup_block_symbol (block, name, namespace);
> + sym = lookup_block_symbol (block, name, mangled_name, namespace);
> if (sym)
> {
> block_found = block;
> @@ -844,7 +847,7 @@ lookup_symbol_aux (const char *name, con
> s = PSYMTAB_TO_SYMTAB (ps);
> bv = BLOCKVECTOR (s);
> block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
> - sym = lookup_block_symbol (block, name, namespace);
> + sym = lookup_block_symbol (block, name, mangled_name, namespace);
> if (!sym)
> {
> /* This shouldn't be necessary, but as a last resort
> @@ -853,7 +856,7 @@ lookup_symbol_aux (const char *name, con
> * the psymtab gets it wrong in some cases.
> */
> block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> - sym = lookup_block_symbol (block, name, namespace);
> + sym = lookup_block_symbol (block, name, mangled_name, namespace);
> if (!sym)
> error ("Internal: static symbol `%s' found in %s psymtab but not in symtab.\n\
> %s may be an inlined function, or may be a template function\n\
> @@ -910,14 +913,14 @@ lookup_symbol_aux (const char *name, con
> bv = BLOCKVECTOR (s);
> block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
> - namespace);
> + mangled_name, namespace);
> /* We kept static functions in minimal symbol table as well as
> in static scope. We want to find them in the symbol table. */
> if (!sym)
> {
> block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
> sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
> - namespace);
> + mangled_name, namespace);
> }
> /* If we found one, return it */
> if (sym)
> @@ -954,8 +957,9 @@ lookup_symbol_aux (const char *name, con
> && MSYMBOL_TYPE (msymbol) != mst_file_text
> && !STREQ (name, SYMBOL_NAME (msymbol)))
> {
> - return lookup_symbol_aux (SYMBOL_NAME (msymbol), block,
> - namespace, is_a_field_of_this, symtab);
> + return lookup_symbol_aux (SYMBOL_NAME (msymbol), mangled_name,
> + block, namespace, is_a_field_of_this,
> + symtab);
> }
> }
> }
> @@ -1081,7 +1085,7 @@ lookup_transparent_type (const char *nam
> {
> bv = BLOCKVECTOR (s);
> block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> - sym = lookup_block_symbol (block, name, STRUCT_NAMESPACE);
> + sym = lookup_block_symbol (block, name, NULL, STRUCT_NAMESPACE);
> if (sym && !TYPE_IS_OPAQUE (SYMBOL_TYPE (sym)))
> {
> return SYMBOL_TYPE (sym);
> @@ -1095,7 +1099,7 @@ lookup_transparent_type (const char *nam
> s = PSYMTAB_TO_SYMTAB (ps);
> bv = BLOCKVECTOR (s);
> block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> - sym = lookup_block_symbol (block, name, STRUCT_NAMESPACE);
> + sym = lookup_block_symbol (block, name, NULL, STRUCT_NAMESPACE);
> if (!sym)
> {
> /* This shouldn't be necessary, but as a last resort
> @@ -1104,7 +1108,7 @@ lookup_transparent_type (const char *nam
> * the psymtab gets it wrong in some cases.
> */
> block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
> - sym = lookup_block_symbol (block, name, STRUCT_NAMESPACE);
> + sym = lookup_block_symbol (block, name, NULL, STRUCT_NAMESPACE);
> if (!sym)
> error ("Internal: global symbol `%s' found in %s psymtab but not in symtab.\n\
> %s may be an inlined function, or may be a template function\n\
> @@ -1128,7 +1132,7 @@ lookup_transparent_type (const char *nam
> {
> bv = BLOCKVECTOR (s);
> block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
> - sym = lookup_block_symbol (block, name, STRUCT_NAMESPACE);
> + sym = lookup_block_symbol (block, name, NULL, STRUCT_NAMESPACE);
> if (sym && !TYPE_IS_OPAQUE (SYMBOL_TYPE (sym)))
> {
> return SYMBOL_TYPE (sym);
> @@ -1142,7 +1146,7 @@ lookup_transparent_type (const char *nam
> s = PSYMTAB_TO_SYMTAB (ps);
> bv = BLOCKVECTOR (s);
> block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
> - sym = lookup_block_symbol (block, name, STRUCT_NAMESPACE);
> + sym = lookup_block_symbol (block, name, NULL, STRUCT_NAMESPACE);
> if (!sym)
> {
> /* This shouldn't be necessary, but as a last resort
> @@ -1151,7 +1155,7 @@ lookup_transparent_type (const char *nam
> * the psymtab gets it wrong in some cases.
> */
> block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> - sym = lookup_block_symbol (block, name, STRUCT_NAMESPACE);
> + sym = lookup_block_symbol (block, name, NULL, STRUCT_NAMESPACE);
> if (!sym)
> error ("Internal: static symbol `%s' found in %s psymtab but not in symtab.\n\
> %s may be an inlined function, or may be a template function\n\
> @@ -1195,10 +1199,15 @@ find_main_psymtab (void)
> binary search terminates, we drop through and do a straight linear
> search on the symbols. Each symbol which is marked as being a C++
> symbol (language_cplus set) has both the encoded and non-encoded names
> - tested for a match. */
> + tested for a match.
> +
> + If MANGLED_NAME is non-NULL, verify that any symbol we find has this
> + particular mangled name.
> +*/
>
> struct symbol *
> lookup_block_symbol (register const struct block *block, const char *name,
> + const char *mangled_name,
> const namespace_enum namespace)
> {
> register int bot, top, inc;
> @@ -1258,14 +1267,19 @@ lookup_block_symbol (register const stru
> return the first one; I believe it is now impossible for us
> to encounter two symbols with the same name and namespace
> here, because blocks containing argument symbols are no
> - longer sorted. */
> + longer sorted. The exception is for C++, where multiple functions
> + (cloned constructors / destructors, in particular) can have
> + the same demangled name. So if we have a particular
> + mangled name to match, try to do so. */
>
> top = BLOCK_NSYMS (block);
> while (bot < top)
> {
> sym = BLOCK_SYM (block, bot);
> - if (SYMBOL_NAMESPACE (sym) == namespace &&
> - SYMBOL_MATCHES_NAME (sym, name))
> + if (SYMBOL_NAMESPACE (sym) == namespace
> + && (mangled_name
> + ? strcmp (SYMBOL_NAME (sym), mangled_name) == 0
> + : SYMBOL_MATCHES_NAME (sym, name)))
> {
> return sym;
> }
> @@ -1297,8 +1311,10 @@ lookup_block_symbol (register const stru
> while (bot < top)
> {
> sym = BLOCK_SYM (block, bot);
> - if (SYMBOL_NAMESPACE (sym) == namespace &&
> - SYMBOL_MATCHES_NAME (sym, name))
> + if (SYMBOL_NAMESPACE (sym) == namespace
> + && (mangled_name
> + ? strcmp (SYMBOL_NAME (sym), mangled_name) == 0
> + : SYMBOL_MATCHES_NAME (sym, name)))
> {
> /* If SYM has aliases, then use any alias that is active
> at the current PC. If no alias is active at the current
> Index: symtab.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/symtab.h,v
> retrieving revision 1.26
> diff -p -u -r1.26 symtab.h
> --- symtab.h 2001/12/21 22:32:37 1.26
> +++ symtab.h 2002/02/14 23:34:24
> @@ -1095,6 +1095,7 @@ extern struct symbol *lookup_symbol (con
> /* lookup a symbol by name, within a specified block */
>
> extern struct symbol *lookup_block_symbol (const struct block *, const char *,
> + const char *,
> const namespace_enum);
>
> /* lookup a [struct, union, enum] by name, within a specified block */
> Index: valops.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/valops.c,v
> retrieving revision 1.51
> diff -p -u -r1.51 valops.c
> --- valops.c 2002/02/10 05:50:34 1.51
> +++ valops.c 2002/02/14 23:34:25
> @@ -3249,7 +3249,7 @@ value_of_this (int complain)
>
> /* Calling lookup_block_symbol is necessary to get the LOC_REGISTER
> symbol instead of the LOC_ARG one (if both exist). */
> - sym = lookup_block_symbol (b, funny_this, VAR_NAMESPACE);
> + sym = lookup_block_symbol (b, funny_this, NULL, VAR_NAMESPACE);
> if (sym == NULL)
> {
> if (complain)
>

--
Daniel Jacobowitz Carnegie Mellon University
MontaVista Software Debian GNU/Linux Developer