This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [rfa] lookup_symbol_aux_minsym
- 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>
- Date: Mon, 11 Nov 2002 13:18:44 -0500
- Subject: Re: [rfa] lookup_symbol_aux_minsym
- References: <ro1adkna8ox.fsf@jackfruit.Stanford.EDU>
David Carlton writes:
> Here's the part of my previous lookup_symbol_aux patch where I combine
> the two versions of the minsym code into a single function
> lookup_symbol_aux_minsym. I've tried to mimic the current flow of
> control as exactly as possible: in particular, this code is run at a
> different time in HPUXHPPA situations than in other situations, and I
> preserved the weird circumstances in which the minsym check might lead
> to a NULL return. (I'm not sure either of those is a good idea, but
> that's a matter for a separate patch.)
>
> Here are the differences between the two versions of the minsym code
> in the current version of lookup_symbol_aux.
>
> * The comments differ, to some extent: I picked whichever version of
> the comment I liked more.
>
> * The non-HP code has
>
> s = find_pc_sect_symtab (SYMBOL_VALUE_ADDRESS (msymbol),
> SYMBOL_BFD_SECTION (msymbol));
>
> where the HP code has
>
> s = find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol));
>
> Both seem fine to me; I picked the former, but I don't have a strong
> feeling about this.
>
Some random comments....
I don't know about this. find_pc_symtab calls find_pc_sect_symtab with
NULL as section parameter, unless there are overlays involved. I
don't know if it would make a difference, but it seems that it would
in case of overlays, because finding the section is a bit more involved.
Maybe we should adopt the other approach? I.e. use the
s = find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol));
What do you think?
> * The non-HP code has
>
> if (symtab != NULL)
> *symtab = s;
> return fixup_symbol_section (sym, s->objfile);
>
> where the non-HP code has
>
> if (sym)
> {
> if (symtab != NULL)
> *symtab = s;
> return sym;
> }
>
> There are a few differences here. The non-HP code does a
> fixup_symbol_section, which seems like a good idea. The HP code
> sets *symtab to s only if both symtab and sym are non-NULL: that
> sounds like a good idea.
The cases seem to be as follow (I am not trying to be pedantic, I am
having a hard time myself understanding this code):
s == null && sym == null
non-HP--> fall through
HP--> fall through
s == null && sym != null
Can't happen. sym must be null as well because the previous searches have
failed. (actually sym is uninitialized).
s != null && sym != null
non-HP--> search finished: *symtab set to s + returns fixed up sym
HP--> search finished: *symtab set to s + returns sym
s != null && sym == null
non-HP--> search finished: *symtab set to s + returns null sym
HP--> fall through. This means that, because of the position
of the ifdef, *symtab set to null + returns null sym.
So, the behaviors differ for 3 and 4. I think that for 3, we could
call fixup_section, which is called for all the non-hp cases. And you
did that. For 4, I am not sure what behavior is more correct, set the
symtab to null or not? What do the callers expect? Hmm, sadly enough,
almost none of the callers to lookup_symbol (which initiates all these
calls) seems to care about the symtab parameter at all. Almost all the
calls use NULL, and those that don't use a placeholder parameter. But
maybe I missed a few. Bottom line, I am not sure what is best to do.
> Finally, the HP code only returns sym if
> sym is non-NULL; that turns out not to make a difference since, if
> we're in the HP branch, this is right next to the end of
> lookup_symbol_aux, so we'll return NULL immediately anyways.
>
> It seems to me that none of these differences should stand in the way
> of merging the two code paths into a single function; here's a patch
> which does that.
>
> Joel has kindly tested this patch on an HP/UX machine, so I don't
> think I screwed up anything in that case.
I don't particularly like the force_return thing. May I suggest to change
the signature of the new function to this:
static enum return_code (or similar)
lookup_symbol_aux_minsyms (const char *name,
const char *mangled_name,
const namespace_enum namespace,
int *is_a_field_of_this,
struct symtab **symtab,
struct symbol **return_symbol)
and
*force_return = 1;
return fixup_symbol_section (sym, s->objfile);
into
*return_symbol = fixup_symbol_section (sym, s->objfile);
return RETURN_NOW; (or something like that)
Elena
>
> David Carlton
> carlton@math.stanford.edu
>
> 2002-11-05 David Carlton <carlton@math.stanford.edu>
>
> * symtab.c (lookup_symbol_aux): Move minsym code into a separate
> function.
> (lookup_symbol_aux_minsyms): New function.
>
> Index: symtab.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/symtab.c,v
> retrieving revision 1.77
> diff -u -p -r1.77 symtab.c
> --- symtab.c 5 Nov 2002 20:33:01 -0000 1.77
> +++ symtab.c 5 Nov 2002 22:43:38 -0000
> @@ -103,6 +103,14 @@ struct symbol *lookup_symbol_aux_psymtab
> const namespace_enum namespace,
> struct symtab **symtab);
>
> +static
> +struct symbol *lookup_symbol_aux_minsyms (const char *name,
> + const char *mangled_name,
> + const namespace_enum namespace,
> + int *is_a_field_of_this,
> + struct symtab **symtab,
> + int *force_return);
> +
> static struct symbol *find_active_alias (struct symbol *sym, CORE_ADDR addr);
>
> /* This flag is used in hppa-tdep.c, and set in hp-symtab-read.c */
> @@ -786,9 +794,14 @@ lookup_symbol_aux (const char *name, con
> int *is_a_field_of_this, struct symtab **symtab)
> {
> struct symbol *sym;
> - struct symtab *s = NULL;
> - struct blockvector *bv;
> - struct minimal_symbol *msymbol;
> +
> + /* FIXME: carlton/2002-11-05: This variable is here so that
> + lookup_symbol_aux will sometimes return NULL after receiving a
> + NULL return value from lookup_symbol_aux_minsyms, without
> + proceeding on to the partial symtab and static variable tests. I
> + suspect that that's a bad idea. */
> +
> + int force_return;
>
> /* Search specified block and its superiors. */
>
> @@ -875,65 +888,14 @@ lookup_symbol_aux (const char *name, con
> a mangled variable that is stored in one of the minimal symbol tables.
> Eventually, all global symbols might be resolved in this way. */
>
> - if (namespace == VAR_NAMESPACE)
> - {
> - msymbol = lookup_minimal_symbol (name, NULL, NULL);
> - if (msymbol != NULL)
> - {
> - s = find_pc_sect_symtab (SYMBOL_VALUE_ADDRESS (msymbol),
> - SYMBOL_BFD_SECTION (msymbol));
> - if (s != NULL)
> - {
> - /* This is a function which has a symtab for its address. */
> - bv = BLOCKVECTOR (s);
> - block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> -
> - /* This call used to pass `SYMBOL_NAME (msymbol)' as the
> - `name' argument to lookup_block_symbol. But the name
> - of a minimal symbol is always mangled, so that seems
> - to be clearly the wrong thing to pass as the
> - unmangled name. */
> - sym = lookup_block_symbol (block, name, 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, name,
> - mangled_name, namespace);
> - }
> -
> - /* sym == 0 if symbol was found in the minimal symbol table
> - but not in the symtab.
> - Return 0 to use the msymbol definition of "foo_".
> -
> - This happens for Fortran "foo_" symbols,
> - which are "foo" in the symtab.
> -
> - This can also happen if "asm" is used to make a
> - regular symbol but not a debugging symbol, e.g.
> - asm(".globl _main");
> - asm("_main:");
> - */
> + force_return = 0;
>
> - if (symtab != NULL)
> - *symtab = s;
> - return fixup_symbol_section (sym, s->objfile);
> - }
> - else if (MSYMBOL_TYPE (msymbol) != mst_text
> - && MSYMBOL_TYPE (msymbol) != mst_file_text
> - && !STREQ (name, SYMBOL_NAME (msymbol)))
> - {
> - /* This is a mangled variable, look it up by its
> - mangled name. */
> - return lookup_symbol_aux (SYMBOL_NAME (msymbol), mangled_name, NULL,
> - namespace, is_a_field_of_this, symtab);
> - }
> - /* There are no debug symbols for this file, or we are looking
> - for an unmangled variable.
> - Try to find a matching static symbol below. */
> - }
> - }
> + sym = lookup_symbol_aux_minsyms (name, mangled_name,
> + namespace, is_a_field_of_this,
> + symtab, &force_return);
> +
> + if (sym != NULL || force_return == 1)
> + return sym;
>
> #endif
>
> @@ -975,87 +937,15 @@ lookup_symbol_aux (const char *name, con
> the static check in this case?
> */
>
> - if (namespace == VAR_NAMESPACE)
> - {
> - msymbol = lookup_minimal_symbol (name, NULL, NULL);
> - if (msymbol != NULL)
> - {
> - /* OK, we found a minimal symbol in spite of not
> - * finding any symbol. There are various possible
> - * explanations for this. One possibility is the symbol
> - * exists in code not compiled -g. Another possibility
> - * is that the 'psymtab' isn't doing its job.
> - * A third possibility, related to #2, is that we were confused
> - * by name-mangling. For instance, maybe the psymtab isn't
> - * doing its job because it only know about demangled
> - * names, but we were given a mangled name...
> - */
> -
> - /* We first use the address in the msymbol to try to
> - * locate the appropriate symtab. Note that find_pc_symtab()
> - * has a side-effect of doing psymtab-to-symtab expansion,
> - * for the found symtab.
> - */
> - s = find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol));
> - if (s != NULL)
> - {
> - bv = BLOCKVECTOR (s);
> - block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> - /* This call used to pass `SYMBOL_NAME (msymbol)' as the
> - `name' argument to lookup_block_symbol. But the name
> - of a minimal symbol is always mangled, so that seems
> - to be clearly the wrong thing to pass as the
> - unmangled name. */
> - sym = lookup_block_symbol (block, name, 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, name,
> - mangled_name, namespace);
> - }
> - /* If we found one, return it */
> - if (sym)
> - {
> - if (symtab != NULL)
> - *symtab = s;
> - return sym;
> - }
> -
> - /* If we get here with sym == 0, the symbol was
> - found in the minimal symbol table
> - but not in the symtab.
> - Fall through and return 0 to use the msymbol
> - definition of "foo_".
> - (Note that outer code generally follows up a call
> - to this routine with a call to lookup_minimal_symbol(),
> - so a 0 return means we'll just flow into that other routine).
>
> - This happens for Fortran "foo_" symbols,
> - which are "foo" in the symtab.
> -
> - This can also happen if "asm" is used to make a
> - regular symbol but not a debugging symbol, e.g.
> - asm(".globl _main");
> - asm("_main:");
> - */
> - }
> + force_return = 0;
>
> - /* If the lookup-by-address fails, try repeating the
> - * entire lookup process with the symbol name from
> - * the msymbol (if different from the original symbol name).
> - */
> - else if (MSYMBOL_TYPE (msymbol) != mst_text
> - && MSYMBOL_TYPE (msymbol) != mst_file_text
> - && !STREQ (name, SYMBOL_NAME (msymbol)))
> - {
> - return lookup_symbol_aux (SYMBOL_NAME (msymbol), mangled_name,
> - NULL, namespace, is_a_field_of_this,
> - symtab);
> - }
> - }
> - }
> + sym = lookup_symbol_aux_minsyms (name, mangled_name,
> + namespace, is_a_field_of_this,
> + symtab, &force_return);
> +
> + if (sym != NULL || force_return == 1)
> + return sym;
>
> #endif
>
> @@ -1198,6 +1088,104 @@ lookup_symbol_aux_psymtabs (int block_in
> return fixup_symbol_section (sym, objfile);
> }
> }
> +
> + return NULL;
> +}
> +
> +/* Check for the possibility of the symbol being a function or a
> + mangled variable that is stored in one of the minimal symbol
> + tables. Eventually, all global symbols might be resolved in this
> + way. */
> +
> +static struct symbol *
> +lookup_symbol_aux_minsyms (const char *name,
> + const char *mangled_name,
> + const namespace_enum namespace,
> + int *is_a_field_of_this,
> + struct symtab **symtab,
> + int *force_return)
> +{
> + struct symbol *sym;
> + struct blockvector *bv;
> + const struct block *block;
> + struct minimal_symbol *msymbol;
> + struct symtab *s;
> +
> + if (namespace == VAR_NAMESPACE)
> + {
> + msymbol = lookup_minimal_symbol (name, NULL, NULL);
> +
> + if (msymbol != NULL)
> + {
> + /* OK, we found a minimal symbol in spite of not finding any
> + symbol. There are various possible explanations for
> + this. One possibility is the symbol exists in code not
> + compiled -g. Another possibility is that the 'psymtab'
> + isn't doing its job. A third possibility, related to #2,
> + is that we were confused by name-mangling. For instance,
> + maybe the psymtab isn't doing its job because it only
> + know about demangled names, but we were given a mangled
> + name... */
> +
> + /* We first use the address in the msymbol to try to locate
> + the appropriate symtab. Note that find_pc_sect_symtab()
> + has a side-effect of doing psymtab-to-symtab expansion,
> + for the found symtab. */
> + s = find_pc_sect_symtab (SYMBOL_VALUE_ADDRESS (msymbol),
> + SYMBOL_BFD_SECTION (msymbol));
> + if (s != NULL)
> + {
> + /* This is a function which has a symtab for its address. */
> + bv = BLOCKVECTOR (s);
> + block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> +
> + /* This call used to pass `SYMBOL_NAME (msymbol)' as the
> + `name' argument to lookup_block_symbol. But the name
> + of a minimal symbol is always mangled, so that seems
> + to be clearly the wrong thing to pass as the
> + unmangled name. */
> + sym =
> + lookup_block_symbol (block, name, 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, name,
> + mangled_name, namespace);
> + }
> +
> + /* sym == 0 if symbol was found in the minimal symbol table
> + but not in the symtab.
> + Return 0 to use the msymbol definition of "foo_".
> +
> + This happens for Fortran "foo_" symbols,
> + which are "foo" in the symtab.
> +
> + This can also happen if "asm" is used to make a
> + regular symbol but not a debugging symbol, e.g.
> + asm(".globl _main");
> + asm("_main:");
> + */
> +
> + if (symtab != NULL && sym != NULL)
> + *symtab = s;
> + *force_return = 1;
> + return fixup_symbol_section (sym, s->objfile);
> + }
> + else if (MSYMBOL_TYPE (msymbol) != mst_text
> + && MSYMBOL_TYPE (msymbol) != mst_file_text
> + && !STREQ (name, SYMBOL_NAME (msymbol)))
> + {
> + /* This is a mangled variable, look it up by its
> + mangled name. */
> + *force_return = 1;
> + return lookup_symbol_aux (SYMBOL_NAME (msymbol), mangled_name,
> + NULL, namespace, is_a_field_of_this,
> + symtab);
> + }
> + }
> + }
>
> return NULL;
> }