Re: [rfa] delete 'force_return' from lookup_symbol_aux_minsyms

Elena Zannoni
2002-12-19T16:46:27Z

David Carlton writes:
> On Mon, 9 Dec 2002 16:10:46 -0500, Elena Zannoni <ezannoni@redhat.com> said:
>
> > I am tempted to accept it, but could you first look at the
> > archeological diggings below?
>
> Interesting. How did you track that down? I couldn't get CVS to go
> back that far when I just tried it.
>
> > I see that this bit:
>
> > + 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 (SYMBOL_NAME (msymbol), block,
> > + 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. */
>
> > and this bit:
>
> > @@ -2629,13 +2684,20 @@ list_symbols (regexp, class, bpt, from_t
> > }
> > }
>
> > - /* Here, we search through the minimal symbol tables for functions that
> > - match, and call find_pc_symtab on them to force their symbols to
> > - be read. The symbol will then be found during the scan of symtabs
> > - below. If find_pc_symtab fails, set found_misc so that we will
> > - rescan to print any matching symbols without debug info. */
> > + /* Here, we search through the minimal symbol tables for functions
> > + and variables that match, and force their symbols to be read.
> > + This is in particular necessary for demangled variable names,
> > + which are no longer put into the partial symbol tables.
> > + The symbol will then be found during the scan of symtabs below.
> > +
> > + For functions, find_pc_symtab should succeed if we have debug info
> > + for the function, for variables we have to call lookup_symbol
> > + to determine if the variable has debug info.
> > + If the lookup fails, set found_misc so that we will rescan to print
> > + any matching symbols without debug info.
> > + */
>
> > - if (class == 1)
> > + if (class == 0 || class == 1)
> > {
> > ALL_MSYMBOLS (objfile, msymbol)
> > {
> > @@ -2648,7 +2710,12 @@ list_symbols (regexp, class, bpt, from_t
> > {
> > if (0 == find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)))
> > {
> > - found_misc = 1;
> > + if (class == 1
> > + || lookup_symbol (SYMBOL_NAME (msymbol),
> > + (struct block *) NULL,
> > + VAR_NAMESPACE,
> > + 0, (struct symtab **) NULL) == NULL)
> > + found_misc = 1;
> > }
> > }
> > }
>
> > were added together in 94. the changelog was:
>
> > date: 1994/10/08 11:54:20; author: schauer; state: Exp; lines: +87 -20
> > Speed up GDB startup time by not demangling partial symbols.
> > * symfile.h (ADD_PSYMBOL_VT_TO_LIST),
> > symfile.c (add_psymbol_to_list, add_psymbol_addr_to_list):
> > No longer demangle partial symbols.
> > * symtab.c (lookup_symbol, list_symbols): Handle mangled
> > variables, e.g. C++ static members, via the minimal symbols.
>
> Hmm. What's 'class'? Does that refer to a variable/function
> distinction, or a mangled/demangled distinction?

Class was a parameter for the function list_symbols which now is
called search_symbols, it has been renamed to 'kind' and cleaned up:

If CLASS is zero, list all symbols except functions, type names, and
constants (enums).
If CLASS is 1, list only functions.
If CLASS is 2, list only type names.
If CLASS is 3, list only method names.

> 'found_misc' also seems to have gone away;

Actually it is still there, the code I posted is for list/search_symbols.

> maybe that was a hint that callers should
> follow up a failed call to lookup_symbol by one to
> lookup_minimal_symbol, whereas we now always expect callers to do so
> if appropriate, even without the hint. At any rate, I'm particularly
> interested by this part of the comment:

I forgot to post the bit of code that used found_misc: This is the end
of that function, and it is still pretty much the same today.

if (found_misc || class != 1)
{
found_in_file = 0;
ALL_MSYMBOLS (objfile, msymbol)
{
if (MSYMBOL_TYPE (msymbol) == ourtype ||
MSYMBOL_TYPE (msymbol) == ourtype2 ||
MSYMBOL_TYPE (msymbol) == ourtype3 ||
MSYMBOL_TYPE (msymbol) == ourtype4)
{
if (regexp == NULL || SYMBOL_MATCHES_REGEXP (msymbol))
{
/* Functions: Look up by address. */
if (class != 1 ||
(0 == find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol))))
{
/* Variables/Absolutes: Look up by name */
if (lookup_symbol (SYMBOL_NAME (msymbol),
(struct block *) NULL, VAR_NAMESPACE,
0, (struct symtab **) NULL) == NULL)
{
if (!found_in_file)
{
printf_filtered ("\nNon-debugging symbols:\n");
found_in_file = 1;
}
printf_filtered (" %08lx %s\n",
(unsigned long) SYMBOL_VALUE_ADDRESS (msymbol),
SYMBOL_SOURCE_NAME (msymbol));
}}}}}

>
> > + For functions, find_pc_symtab should succeed if we have debug info
> > + for the function, for variables we have to call lookup_symbol
> > + to determine if the variable has debug info.
> > + If the lookup fails, set found_misc so that we will rescan to print
> > + any matching symbols without debug info.
>
> So, at some point, this code had explicit assumptions about when there
> should be a symbol corresponding to a minimal symbol, and set
> found_misc appropriately. But then, at some point, 'found_misc' went
> away; it left, as a legacy, a somewhat strange flow of control that
> wasn't at all explicit until I had to introduce 'force_return' to
> mimic it.

Found_misc is set if lookup_symbol returns null, which then leads to
looking through the minsyms. The assumption is that found_misc is set
if there is no symbol/debuginfo for a name. With your force_return
elimination, is this still the case? I see that it is for the HPPA
case, but for the other. You wouldn't return NULL, right there, but
would continue searching. Would this matter? I am not sure. Probably
you would end up returning NULL anyway later?

>
> Also, the line
>
> > + The symbol will then be found during the scan of symtabs below.
>
> makes me wonder if, at this time, the minimal symbol search happened
> before the symtab search; I'm not sure how relevant that is.
>
> At any rate, this was certainly interesting in terms of giving an idea
> as to how the current flow of control might have arisen. I still
> think my patch is okay, though.
>

See above for why I am not truely convinced yet.

Does it matter that we keep going with the search instead of stopping?

Elena

> David Carlton
> carlton@math.stanford.edu