This is the mail archive of the gdb-prs@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

pending/1019: [rfa] more lookup_symbol_aux_minsyms futzing


>Number:         1019
>Category:       pending
>Synopsis:       [rfa] more lookup_symbol_aux_minsyms futzing
>Confidential:   yes
>Severity:       serious
>Priority:       medium
>Responsible:    unassigned
>State:          open
>Class:          change-request
>Submitter-Id:   unknown
>Arrival-Date:   Sun Feb 02 06:28:00 UTC 2003
>Closed-Date:
>Last-Modified:
>Originator:     
>Release:        
>Organization:
>Environment:
>Description:
 Here's another patch involving lookup_symbol_aux_minsyms.  It reverts
 the search_symbols part of my last patch, and deletes the last "else
 if" part of lookup_symbol_aux_minsyms.  (And it deletes the
 'is_a_field_of_this' argument, since that's was only used in that
 "else if" part.)
 
 I'd been ascribing more power to lookup_symbol_aux_minsyms than it
 actually had: I had assumed that it did a reasonable job of finding a
 symbol corresponding to a minimal symbol whenever there was one, but
 now I think that it only does that in the function case.  Furthermore,
 it seems to me that whatever it tries to do in the non-function case
 is more likely to be hazardous than helpful.  (Or at least was more
 likely before 'force_return' got deleted.)
 
 This has two implications:
 
 1) Don't count on lookup_symbol_aux_minsyms doing anything useful in
    the non-function case.  In particular, my search_symbols patch from
    before was bad.
 
 2) Get rid of the non-function case of lookup_symbol_aux_minsyms: it
    just confuses the issue and slows down symbol lookup.
 
 Let me analyze lookup_symbol_aux_minsyms in a bit more detail, to back
 this up.  It looks like this, stripped down:
 
   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)
             {
                /* Find the correct symbol, and return it. */
             }
           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);
             }
         }
     }
 
   return NULL;
 
 Now, find_pc_sect_symtab only works if the symbol in question
 corresponds to a function, loosely speaking: it finds the right
 minimal symbol (which will be 'msymbol' above) and then immediately
 returns NULL if that symbol doesn't have type mst_text or
 mst_file_text.  So the whole bit that I've marked by "Find the correct
 symbol, and return it" only happens if we're looking for a function.
 
 So what happens in the "else if" case?  This checks that we're in the
 non-function case, and that the name we were passed isn't the
 SYMBOL_NAME of msymbol, which means that msymbol has a mangled name
 while we were passed the demangled name.  In this case, it calles
 lookup_symbol_aux with a mangled name as the 'name' argument.  So what
 happens in this nested call to lookup_symbol_aux?
 
 1) lookup_symbol_aux_local.  This doesn't happen because 'block' is
    NULL.
 
 2) The 'is_a_field_of_this' check.  It makes no sense to do this now:
    we already did it once!  So we should have passed NULL in place of
    'is_a_field_of_this': oops.
 
 3) The static block check; again, 'block' is NULL, so this doesn't
    happen.
 
 4) The 'lookup_symbol_aux_symtabs' check.  The symtab search
    eventually comes down to calling lookup_block_symbol a bunch of
    times, with the mangled name as its 'name' argument.  But, as Jim
    Blandy noted in
    <http://sources.redhat.com/ml/gdb-patches/2002-10/msg00040.html>,
    doing that makes no sense: it won't find a match in the hash
    table.  So that's not useful.
 
 5) The 'lookup_symbol_aux_minsyms' check.  That's not going to uncover
    anything the second time that it didn't uncover the first time it
    was called.
 
 6) The 'lookup_symbol_aux_psymtabs' check.  This tries to find the
    right partial symbol table, read in the corresponding symbol table,
    and then search it with lookup_block_symbol.  But, again, calling
    lookup_block_symbol with the mangled name as the 'name' argument
    makes no sense.
 
 So in no case is it correct for lookup_symbol_aux_minsyms to call
 lookup_symbol_aux with the demangled name as the 'name' argument.  So
 I think we should just delete that call.
 
 There is one thing that bothers me: right now, partial symbols don't
 include demangled names.  So that call to lookup_symbol_aux_psymtabs
 with the demangled name might actually cause the correct symtab to be
 read in, even if the lookup_block_symbol fails.  The correct thing to
 do here is to store demangled names in partial symbols, however, not
 to depend on an undocumented side effect of an incorrect psymtab
 search.  I'll submit a patch for that next.
 
 David Carlton
 carlton@math.stanford.edu
 
 2002-12-23  David Carlton  <carlton@math.stanford.edu>
 
 	* symtab.c (search_symbols): Change call to
 	lookup_symbol_aux_minsyms back to cal to lookup_symbol, reverting
 	previous patch.
 	(lookup_symbol_aux_minsyms): Don't search on mangled names; delete
 	'is_a_field_of_this' argument.
 	(lookup_symbol_aux): Don't pass 'is_a_field_of_this' to
 	lookup_symbol_aux_minsyms.
 
 Index: symtab.c
 ===================================================================
 RCS file: /cvs/src/src/gdb/symtab.c,v
 retrieving revision 1.83
 diff -u -p -r1.83 symtab.c
 --- symtab.c	23 Dec 2002 16:43:18 -0000	1.83
 +++ symtab.c	23 Dec 2002 18:03:03 -0000
 @@ -116,7 +116,6 @@ 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);
  
  static struct symbol *find_active_alias (struct symbol *sym, CORE_ADDR addr);
 @@ -923,8 +922,7 @@ lookup_symbol_aux (const char *name, con
       Eventually, all global symbols might be resolved in this way.  */
  
    sym = lookup_symbol_aux_minsyms (name, mangled_name,
 -				   namespace, is_a_field_of_this,
 -				   symtab);
 +				   namespace, symtab);
    
    if (sym != NULL)
      return sym;
 @@ -971,8 +969,7 @@ lookup_symbol_aux (const char *name, con
  
  
    sym = lookup_symbol_aux_minsyms (name, mangled_name,
 -				   namespace, is_a_field_of_this,
 -				   symtab);
 +				   namespace, symtab);
    
    if (sym != NULL)
      return sym;
 @@ -1154,24 +1151,20 @@ lookup_symbol_aux_psymtabs (int block_in
    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.  */
 -
 -/* 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.  */
 +/* Check for the possibility of the symbol being a function that is
 +   stored in one of the minimal symbol tables.  */
 +
 +/* NOTE: carlton/2002-12-23: At one point, when this function was part
 +   of lookup_symbol_aux, its behavior was different, for reasons that
 +   are unclear: it forced a return from lookup_symbol_aux at times
 +   even without checking the partial symbol tables, and it tried to do
 +   something strange involving mangled names in the non-function
 +   case.  */
  
  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)
  {
    struct symbol *sym;
 @@ -1186,20 +1179,15 @@ lookup_symbol_aux_minsyms (const char *n
  
        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.  */
 +	  /* We found a minimal symbol in spite of not finding a
 +	     symbol.  This probably means that the symbol in question
 +	     is in a partial symbol table that hasn't been loaded, but
 +	     it may mean that it's from code compiled without -g.
 +	     Partial symbol table searches are kind of expensive; if
 +	     the symbol corresponds to a function, we can find the
 +	     appropriate symtab more quickly by calling
 +	     find_pc_sect_symtab, so let's try that.  */
 +
  	  s = find_pc_sect_symtab (SYMBOL_VALUE_ADDRESS (msymbol),
  				   SYMBOL_BFD_SECTION (msymbol));
  	  if (s != NULL)
 @@ -1267,16 +1255,6 @@ lookup_symbol_aux_minsyms (const char *n
  		*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);
 -	    }
  	}
      }
  
 @@ -2896,31 +2874,12 @@ search_symbols (char *regexp, namespace_
  	      {
  		if (0 == find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)))
  		  {
 -		    if (kind == FUNCTIONS_NAMESPACE)
 -		      {
 -			found_misc = 1;
 -		      }
 -		    else
 -		      {
 -			struct symbol *sym;
 -
 -			if (SYMBOL_DEMANGLED_NAME (msymbol) != NULL)
 -			  sym
 -			    = lookup_symbol_aux_minsyms (SYMBOL_DEMANGLED_NAME
 -							 (msymbol),
 -							 SYMBOL_NAME (msymbol),
 -							 VAR_NAMESPACE,
 -							 NULL, NULL);
 -			else
 -			  sym
 -			    = lookup_symbol_aux_minsyms (SYMBOL_NAME (msymbol),
 -							 NULL,
 -							 VAR_NAMESPACE,
 -							 NULL, NULL);
 -
 -			if (sym == NULL)
 -			  found_misc = 1;
 -		      }
 +		    if (kind == FUNCTIONS_NAMESPACE
 +			|| lookup_symbol (SYMBOL_NAME (msymbol),
 +					  (struct block *) NULL,
 +					  VAR_NAMESPACE,
 +					0, (struct symtab **) NULL) == NULL)
 +		      found_misc = 1;
  		  }
  	      }
  	  }
 
>How-To-Repeat:
>Fix:
>Release-Note:
>Audit-Trail:
>Unformatted:


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]