[rfa] more lookup_symbol_aux_minsyms futzing

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

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;
 		  }
 	      }
 	  }