Re: [patch] Speed up find_pc_section

["Ulrich Weigand" <uweigand@de.ibm.com>]

Paul Pluzhnikov wrote:

> I believe the crash at load should be gone now, and some debugging will
> be possible, though find_pc_section will fail to find any overlay section
> which find_pc_mapped_section doesn't find. I think that's still better
> than what GDB was doing before under these conditions: finding a "random"
> overlapping overlay section.

With this patch, GDB no longer crashes, but some overlay tests fail anyway.
The problem is related to resolving an address in an overlay back to a
symbol via lookup_minimal_symbol_by_pc_section_1.  The caller passes the
correct PC (VMA value) and overlay section to this routine, but the
first thing the routine does is:

  /* PC has to be in a known section.  This ensures that anything
     beyond the end of the last segment doesn't appear to be part of
     the last function in the last segment.  */
  pc_section = find_pc_section (pc);
  if (pc_section == NULL)
    return NULL;

The new find_pc_section logic now returns NULL as the PC is in an
overlay section, which is not in the section map.

I think this routine needs to be fixed to take into account the section
it is passed as argument, probably along the following lines:

Index: gdb/minsyms.c
===================================================================
RCS file: /cvs/src/src/gdb/minsyms.c,v
retrieving revision 1.66
diff -u -p -r1.66 minsyms.c
--- gdb/minsyms.c	28 Jun 2009 00:20:22 -0000	1.66
+++ gdb/minsyms.c	21 Aug 2009 11:16:26 -0000
@@ -457,7 +457,7 @@ lookup_minimal_symbol_by_pc_section_1 (C
   struct objfile *objfile;
   struct minimal_symbol *msymbol;
   struct minimal_symbol *best_symbol = NULL;
-  struct obj_section *pc_section;
+  struct obj_section *pc_section = section;
   enum minimal_symbol_type want_type, other_type;
 
   want_type = want_trampoline ? mst_solib_trampoline : mst_text;
@@ -466,7 +466,8 @@ lookup_minimal_symbol_by_pc_section_1 (C
   /* PC has to be in a known section.  This ensures that anything
      beyond the end of the last segment doesn't appear to be part of
      the last function in the last segment.  */
-  pc_section = find_pc_section (pc);
+  if (pc_section == NULL)
+    pc_section = find_pc_section (pc);
   if (pc_section == NULL)
     return NULL;

(A cleaner fix might be to fix all callers to never pass in a NULL
section argument --most already don't-- and simply rely on it.)
 
With this patch in addition to yours, all overlay tests pass again
on spu-elf.


I'm still not completely happy about the assertions you added.  An
assertion failure is supposed to be an indication of a bug in GDB
-- it should never be possible to trigger the assertion just by
providing particular user input (this includes the binary file).
If for whatever reason the user provides a binary that has overlapping
section that are not recognized as overlays by your logic, you'll
still run into the failure ...

In other places where we detect weird contents of the binary (e.g.
in the DWARF data), we issue a "complaint" and then try to continue.
Maybe you should do the same here: if you find overlapping sections,
issue a complaint and then continue, while ignoring those sections.

Bye,
Ulrich

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