Re: [rfa] delete 'force_return' from lookup_symbol_aux_minsyms

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

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?  'found_misc' also
seems to have gone away; 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:

> +     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.  

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.

David Carlton
carlton@math.stanford.edu