Re: [rfa] delete 'force_return' from lookup_symbol_aux_minsyms

[Elena Zannoni <ezannoni at redhat dot com>]

David Carlton writes:
 > This patch is a short one: it deletes the 'force_return' argument of
 > lookup_symbol_aux_minsyms.  That argument was there because, when the
 > code in lookup_symbol_aux_minsyms was part of lookup_symbol_aux, the
 > return statements would sometimes cause lookup_symbol_aux to return a
 > NULL value without checking the psymtabs first.


I am tempted to accept it, but could you first look at the
archeological diggings below?

 > 
 > I don't think that behavior was either intentional or desirable.
 > There's not much rhyme or reason to when this happens: in particular,
 > it doesn't happen every time lookup_symbol_aux_minsyms finds a minsym.
 > Instead, it is necessary, in addition, either for there to be a symtab at
 > the appropriate address or for the symtab to be NULL but for the
 > following test to hold:
 > 
 >           else if (MSYMBOL_TYPE (msymbol) != mst_text
 >                    && MSYMBOL_TYPE (msymbol) != mst_file_text
 >                    && !STREQ (name, SYMBOL_NAME (msymbol)))
 > 
 > (I actually experimented with trying to have lookup_symbol _always_
 > return NULL if a minsym was found without a corresponding symbol, and
 > that breaks GDB.)  I can't imagine that there are callers of
 > lookup_symbol that depend on having it return NULL in these particular
 > circumstances; it's certainly not documented anywhere.  And I suspect

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.

Don't know if it helps.

Elena


 > that this issue is what caused the #ifdef HPUXHPPA to be added to
 > lookup_symbol_aux: as a relevant comment says,
 > 
 >   For HP-generated symbol tables, this check was causing a premature
 >   exit from lookup_symbol with NULL return, and thus messing up symbol
 >   lookups of things like "c::f".
 > 
 > When I asked about this issue earlier, Jim Blandy came to more or less
 > the same conclusion, I think: see
 > <http://sources.redhat.com/ml/gdb/2002-11/msg00045.html>.
 > 
 > David Carlton
 > carlton@math.stanford.edu
 > 
 > 2002-12-05  David Carlton  <carlton@math.stanford.edu>
 > 
 > 	* symtab.c (lookup_symbol_aux): Delete 'force_return' variable.
 > 	(lookup_symbol_aux_minsyms): Delete 'force_return' argument.
 > 
 > Index: symtab.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/symtab.c,v
 > retrieving revision 1.81
 > diff -u -p -r1.81 symtab.c
 > --- symtab.c	5 Dec 2002 21:26:57 -0000	1.81
 > +++ symtab.c	5 Dec 2002 22:23:55 -0000
 > @@ -117,8 +117,7 @@ struct symbol *lookup_symbol_aux_minsyms
 >  					  const char *mangled_name,
 >  					  const namespace_enum namespace,
 >  					  int *is_a_field_of_this,
 > -					  struct symtab **symtab,
 > -					  int *force_return);
 > +					  struct symtab **symtab);
 >  
 >  static struct symbol *find_active_alias (struct symbol *sym, CORE_ADDR addr);
 >  
 > @@ -805,14 +804,6 @@ lookup_symbol_aux (const char *name, con
 >    struct symbol *sym;
 >    const struct block *static_block;
 >  
 > -  /* 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.  Don't search
 >       STATIC_BLOCK or GLOBAL_BLOCK.  */
 >  
 > @@ -931,13 +922,11 @@ 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.  */
 >  
 > -  force_return = 0;
 > -
 >    sym = lookup_symbol_aux_minsyms (name, mangled_name,
 >  				   namespace, is_a_field_of_this,
 > -				   symtab, &force_return);
 > +				   symtab);
 >    
 > -  if (sym != NULL || force_return == 1)
 > +  if (sym != NULL)
 >      return sym;
 >  
 >  #endif
 > @@ -981,13 +970,11 @@ lookup_symbol_aux (const char *name, con
 >     */
 >  
 >  
 > -  force_return = 0;
 > -
 >    sym = lookup_symbol_aux_minsyms (name, mangled_name,
 >  				   namespace, is_a_field_of_this,
 > -				   symtab, &force_return);
 > +				   symtab);
 >    
 > -  if (sym != NULL || force_return == 1)
 > +  if (sym != NULL)
 >      return sym;
 >  
 >  #endif
 > @@ -1172,13 +1159,20 @@ lookup_symbol_aux_psymtabs (int block_in
 >     tables.  Eventually, all global symbols might be resolved in this
 >     way.  */
 >  
 > +/* NOTE: carlton/2002-12-05: At one point, this function was part of
 > +   lookup_symbol_aux, and what are now 'return' statements within
 > +   lookup_symbol_aux_minsyms returned from lookup_symbol_aux, even if
 > +   sym was NULL.  As far as I can tell, this was basically accidental;
 > +   it didn't happen every time that msymbol was non-NULL, but only if
 > +   some additional conditions held as well, and it caused problems
 > +   with HP-generated symbol tables.  */
 > +
 >  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 symtab **symtab)
 >  {
 >    struct symbol *sym;
 >    struct blockvector *bv;
 > @@ -1271,7 +1265,6 @@ lookup_symbol_aux_minsyms (const char *n
 >  
 >  	      if (symtab != NULL && sym != NULL)
 >  		*symtab = s;
 > -	      *force_return = 1;
 >  	      return fixup_symbol_section (sym, s->objfile);
 >  	    }
 >  	  else if (MSYMBOL_TYPE (msymbol) != mst_text
 > @@ -1280,7 +1273,6 @@ lookup_symbol_aux_minsyms (const char *n
 >  	    {
 >  	      /* 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);