Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Jan Kratochvil wrote:
> I generally considered the overlays to be no longer supported and just still
> not removed from the code.  While reading other GDB code I was feeling overlays
> are no longer supported on multiple places but sure I may be wrong.
> 
> Could you please name which current arch is dependent on overlaying?

As Daniel mentioned, I noticed this on spu-elf, where we do make use of
overlays (at least code overlays -- data overlays are not currently used).

Which parts of GDB do you think do not support overlays?  I'd be interested
in fixing such problems ...

For now, I'm using the patch below that simply falls back to the non-addrmap
case when debugging overlays and the addrmap returned the wrong section.


In testing this, I noticed an independent overlay regression introduced by
a patch of mine:
http://sourceware.org/ml/gdb-patches/2008-05/msg00120.html
which added code to fixup_section to verify the msymbol address.

This patch exposed a pre-existing bug in fixup_section: it uses
  ginfo->value.address
to access the address of a symbol or psymbol.  This is mostly correct,
but in one case it is not: when handling a LOC_BLOCK symbol, in which
case ginfo->value.address is undefined, and the start address needs to
be retrieved from the block at ginfo->value.block.

The following patch fixes this regression as well by having the callers
of fixup_section pass in the proper address.


Tested on spu-elf, powerpc-linux and powerpc64-linux with no regressions;
fixes overlays.exp on spu-elf.

If there is no objection, I'd like to commit this to fix the present
regression until we have a proper implementation of addrmaps for the
overlay case.

Bye,
Ulrich


ChangeLog:

	* symtab.c (fixup_section): Remove prototype.  Add ADDR parameter;
	use it instead of ginfo->value.address.
	(fixup_symbol_section): Pass in correct symbol address.
	(fixup_psymbol_section): Pass in correct symbol address.

	(find_pc_sect_psymtab): Fall back to non-addrmap case when debugging
	overlays and the addrmap returned the wrong section.


diff -urNp gdb-orig/gdb/symtab.c gdb-head/gdb/symtab.c
--- gdb-orig/gdb/symtab.c	2008-05-11 23:41:56.000000000 +0200
+++ gdb-head/gdb/symtab.c	2008-05-13 15:48:55.626975367 +0200
@@ -110,8 +110,6 @@ struct symbol *lookup_symbol_aux_psymtab
 					   const domain_enum domain,
 					   struct symtab **symtab);
 
-static void fixup_section (struct general_symbol_info *, struct objfile *);
-
 static int file_matches (char *, char **, int);
 
 static void print_symbol_info (domain_enum,
@@ -878,6 +876,23 @@ find_pc_sect_psymtab (CORE_ADDR pc, asec
 	pst = addrmap_find (objfile->psymtabs_addrmap, pc);
 	if (pst != NULL)
 	  {
+	    /* FIXME: addrmaps currently do not handle overlayed sections,
+	       so fall back to the non-addrmap case if we're debugging 
+	       overlays and the addrmap returned the wrong section.  */
+	    if (overlay_debugging && msymbol && section)
+	      {
+		struct partial_symbol *p;
+		/* NOTE: This assumes that every psymbol has a
+		   corresponding msymbol, which is not necessarily
+		   true; the debug info might be much richer than the
+		   object's symbol table.  */
+		p = find_pc_sect_psymbol (pst, pc, section);
+		if (!p
+		    || SYMBOL_VALUE_ADDRESS (p)
+		       != SYMBOL_VALUE_ADDRESS (msymbol))
+		  continue;
+	      }
+
 	    /* We do not try to call FIND_PC_SECT_PSYMTAB_CLOSER as
 	       PSYMTABS_ADDRMAP we used has already the best 1-byte
 	       granularity and FIND_PC_SECT_PSYMTAB_CLOSER may mislead us into
@@ -1010,7 +1025,8 @@ find_pc_psymbol (struct partial_symtab *
    out of the minimal symbols and stash that in the debug symbol.  */
 
 static void
-fixup_section (struct general_symbol_info *ginfo, struct objfile *objfile)
+fixup_section (struct general_symbol_info *ginfo, CORE_ADDR addr,
+	       struct objfile *objfile)
 {
   struct minimal_symbol *msym;
   msym = lookup_minimal_symbol (ginfo->name, NULL, objfile);
@@ -1020,8 +1036,7 @@ fixup_section (struct general_symbol_inf
      e.g. on PowerPC64, where the minimal symbol for a function will
      point to the function descriptor, while the debug symbol will
      point to the actual function code.  */
-  if (msym
-      && SYMBOL_VALUE_ADDRESS (msym) == ginfo->value.address)
+  if (msym && SYMBOL_VALUE_ADDRESS (msym) == addr)
     {
       ginfo->bfd_section = SYMBOL_BFD_SECTION (msym);
       ginfo->section = SYMBOL_SECTION (msym);
@@ -1064,11 +1079,8 @@ fixup_section (struct general_symbol_inf
 	 this reason, we still attempt a lookup by name prior to doing
 	 a search of the section table.  */
 	 
-      CORE_ADDR addr;
       struct obj_section *s;
 
-      addr = ginfo->value.address;
-
       ALL_OBJFILE_OSECTIONS (objfile, s)
 	{
 	  int idx = s->the_bfd_section->index;
@@ -1093,7 +1105,22 @@ fixup_symbol_section (struct symbol *sym
   if (SYMBOL_BFD_SECTION (sym))
     return sym;
 
-  fixup_section (&sym->ginfo, objfile);
+  switch (SYMBOL_CLASS (sym))
+    {
+    case LOC_LABEL:
+    case LOC_STATIC:
+    case LOC_INDIRECT:
+      fixup_section (&sym->ginfo, SYMBOL_VALUE_ADDRESS (sym), objfile);
+      break;
+
+    case LOC_BLOCK:
+      fixup_section (&sym->ginfo,
+		     BLOCK_START (SYMBOL_BLOCK_VALUE (sym)), objfile);
+      break;
+
+    default:
+      break;
+    }
 
   return sym;
 }
@@ -1107,7 +1134,18 @@ fixup_psymbol_section (struct partial_sy
   if (SYMBOL_BFD_SECTION (psym))
     return psym;
 
-  fixup_section (&psym->ginfo, objfile);
+  switch (SYMBOL_CLASS (psym))
+    {
+    case LOC_LABEL:
+    case LOC_STATIC:
+    case LOC_INDIRECT:
+    case LOC_BLOCK:
+      fixup_section (&psym->ginfo, SYMBOL_VALUE_ADDRESS (psym), objfile);
+      break;
+
+    default:
+      break;
+    }
 
   return psym;
 }

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com