[PATCH RFA] dbxread.c, partial-stab.h patch

[Kevin Buettner <kevinb at cygnus dot com>]

The patch below fixes the problem reported by H.J. Lu in:

    http://sources.redhat.com/ml/gdb/2001-08/msg00222.html

using the example from Mike Krogh:

    http://sources.redhat.com/ml/gdb/2001-08/msg00220.html

I was able to reproduce the behavior reported by H.J. on a stock Red
Hat 7.1 system.

GDB loads libdl.so.2 in the course of execution and this file
has the following entry as reported by ``objdump -G'':

1327   FUN    0      36     00000000 31372  __strtol_internal:F(0,3)

Note that n_value is 00000000 for this entry.

The fact that this value is zero means that the
SOFUN_ADDRESS_MAYBE_MISSING code gets triggered (in both
partial-stab.h and dbxread.c) in which the symbol's adjusted value is
equal to the start of the .text segment.  When this happens, GDB
attempts to work around the fact that the address may be missing for
the N_FUN symbol.  It does this by calling find_stab_function_addr()
which looks for the symbol in the minimal symbol table.

BUT, if the symbol is not found in the symbol table,
find_stab_function_addr() will simply return 0 which indicates that
the function is not found.  (Alternately, it could return 0 if that's
really its address too, but I'll have more to say about that case in a
moment.)  Anyway, the callers of find_stab_function_addr() are
certainly not handling the case where the symbol is not found in the
minimal symbol table.  If that should happen, the 0 value is used even
when it makes no sense.  (I.e, suppose that .text doesn't start at 0.)
One of the other side effects of this bug is that the ``textlow''
member will sometimes get set (in partial-stab.h) to this 0 value.

This is precisely what was happening in Mike Krogh's example.
GDB was loading the symbols for libdl.so.2 before the dlopen'd object
file.  It was setting the textlow field for one of the psymtabs
to 0.  When the dlopen'd object file was loaded, it ended up having
starting and ending addresses which were (erroneously) encompassed by
the range of one of the files from libdl.so.2.  As a consequence, GDB
would get confused by the following code in find_pc_sect_symtab():

	/* For an objfile that has its functions reordered,
	   find_pc_psymtab will find the proper partial symbol table
	   and we simply return its corresponding symtab.  */
	/* In order to better support objfiles that contain both
	   stabs and coff debugging info, we continue on if a psymtab
	   can't be found. */
	if ((objfile->flags & OBJF_REORDERED) && objfile->psymtabs)
	  {
	    ps = find_pc_sect_psymtab (pc, section);
	    if (ps)
	      return PSYMTAB_TO_SYMTAB (ps);
	  }

The objfile in this case *was* reordered, so it simply took the result
as computed by PSYMTAB_TO_SYMTAB (find_pc_sect_psymtab (pc, section)). 
If it wouldn't have been reordered, find_pc_sect_symtab() would have
continued its search and correctly found the "best" (and in this case
correct) symtab.  I'm not convinced that the code for handling
reordered files in symtab.c is correct and I think it'd be a good idea
for the symtab maintainers to review this code.

But, regardless, the reason why GDB got confused in find_pc_sect_psymtab()
is because GDB had first read in the symtabs incorrectly by getting
the ``textlow'' value wrong.  The patch below sets this right.

Now, with regard to 0 also being a valid address... If it should happen
that we end up calling find_stab_function_addr() on a symbol with
a legitimately valid 0 address, this will have happened because the
symbol's adjusted value from the stabs is equal to the start of the
.text section.  In order for this to happen though, the start of the
.text section will have to have been 0.  If that's the case, the
code that I've added will simply set the symbol's value back to
its original value.  (Which must have been 0 since it was the same as
the start of the .text section.) In other words, not being able to
distinguish between a real value 0 and a missing symbol is not that
great of a liability for these purposes since we "accidentally" get
the right answer anyway.  (I'm certainly willing to resubmit the patch
though which uses an improved version of find_stab_function_addr()
which makes the success/failure explicit.)

Okay to commit?

(I think this ought to go on the 5.1 branch too.)

	* dbxread.c (process_one_symbol): Don't use error result from
	find_stab_function_addr().
	* partial-stab.h (case 'F'): Likewise.

Index: dbxread.c
===================================================================
RCS file: /cvs/src/src/gdb/dbxread.c,v
retrieving revision 1.20
diff -u -p -r1.20 dbxread.c
--- dbxread.c	2001/08/15 05:02:28	1.20
+++ dbxread.c	2001/09/03 21:18:38
@@ -2270,8 +2270,18 @@ process_one_symbol (int type, int desc, 
 	         from N_FUN symbols.  */
 	      if (type == N_FUN
 		  && valu == ANOFFSET (section_offsets, SECT_OFF_TEXT (objfile)))
-		valu = 
-		  find_stab_function_addr (name, last_source_file, objfile);
+		{
+		  CORE_ADDR minsym_valu = 
+		    find_stab_function_addr (name, last_source_file, objfile);
+
+		  /* find_stab_function_addr will return 0 if the minimal
+		     symbol wasn't found.  (Unfortunately, this might also
+		     be a valid address.)  Anyway, if it *does* return 0,
+		     it is likely that the value was set correctly to begin
+		     with... */
+		  if (minsym_valu != 0)
+		    valu = minsym_valu;
+		}
 #endif
 
 #ifdef SUN_FIXED_LBRAC_BUG
Index: partial-stab.h
===================================================================
RCS file: /cvs/src/src/gdb/partial-stab.h,v
retrieving revision 1.12
diff -u -p -r1.12 partial-stab.h
--- partial-stab.h	2001/08/15 05:02:28	1.12
+++ partial-stab.h	2001/09/03 21:18:39
@@ -652,8 +652,17 @@ switch (CUR_SYMBOL_TYPE)
 	   value for the bottom of the text seg in those cases. */
 	if (CUR_SYMBOL_VALUE == ANOFFSET (objfile->section_offsets, 
 	                                  SECT_OFF_TEXT (objfile)))
-	  CUR_SYMBOL_VALUE = 
-	    find_stab_function_addr (namestring, pst->filename, objfile);
+	  {
+	    CORE_ADDR minsym_valu = 
+	      find_stab_function_addr (namestring, pst->filename, objfile);
+	    /* find_stab_function_addr will return 0 if the minimal
+	       symbol wasn't found.  (Unfortunately, this might also
+	       be a valid address.)  Anyway, if it *does* return 0,
+	       it is likely that the value was set correctly to begin
+	       with... */
+	    if (minsym_valu != 0)
+	      CUR_SYMBOL_VALUE = minsym_valu;
+	  }
 	if (pst && textlow_not_set)
 	  {
 	    pst->textlow = CUR_SYMBOL_VALUE;