RFA symtab: Fix for PR c++/1267 ("next" and shared libraries)

[Daniel Jacobowitz <drow@mvista.com>]

This patch fixes c++/1267, a bug where stepping over a function call that
went through the PLT (as happens when a -fPIC function makes a call to a
globally visible symbol) would lose control of the inferior.  I'll spare you
the complete debugging session, as it really doesn't make much sense.  But
here's the root of the problem:

When we called frame_pc_unwind on the sentinel frame, we got an address in
the PLT.  But when we called frame_func_unwind, we got "_init", in ".init",
which is generally located right before the PLT.  Then, we'd run the
new-and-improved prologue unwinder on _init, and get some completely bogus
information, since things weren't actually saved on the stack where it
thought they were.  This led to the unwound stack pointer being wrong for
the step_resume breakpoint, so when we hit the step_resume breakpoint we
kept going.

I fixed this by changing lookup_minimal_symbol_pc_section to be paranoid
about returning a minsym in the same section as the PC.  Technically, at
least on ELF targets, that doesn't _have_ to be true.  I've never
encountered an exception or a good reason for one, though.  Does anyone see
any pitfalls for this change?  Symtab maintainers, is this patch OK?

I believe this patch should also fix shlibs/1237, and may also fix
shlibs/1280.  Adam, could you check those?





By the way, I'm convinced that all is not well in step_over_function.  This
comment,

  /* NOTE: cagney/2003-04-06:

     The intent of DEPRECATED_SAVED_PC_AFTER_CALL was to:

     - provide a very light weight equivalent to frame_unwind_pc()
     (nee FRAME_SAVED_PC) that avoids the prologue analyzer

     - avoid handling the case where the PC hasn't been saved in the
     prologue analyzer

     Unfortunatly, not five lines further down, is a call to
     get_frame_id() and that is guarenteed to trigger the prologue
     analyzer.

is either incorrect or has gotten out of sync with the code:

  if (DEPRECATED_SAVED_PC_AFTER_CALL_P ())
    sr_sal.pc = ADDR_BITS_REMOVE (DEPRECATED_SAVED_PC_AFTER_CALL (get_current_frame ()));
  else
    sr_sal.pc = ADDR_BITS_REMOVE (frame_pc_unwind (get_current_frame ()));
  sr_sal.section = find_pc_overlay (sr_sal.pc);

  check_for_old_step_resume_breakpoint ();
  step_resume_breakpoint =
    set_momentary_breakpoint (sr_sal, get_frame_id (get_current_frame ()),
                              bp_step_resume);


Note that get_frame_id unwinds from the NEXT frame, and
frame_pc_unwind/DEPRECATED_SAVED_PC_AFTER_CALL unwind from THIS frame.
This throws me a loop every time I have to work in this function.  Also, I
have the nagging feeling we're saving the wrong frame.  I have an old MIPS
patch where I needed to use get_prev_frame in step_over_function.  As soon
as I have time to revisit that patch I'll be back to clean this up some
more.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2003-07-19  Daniel Jacobowitz  <drow@mvista.com>

	PR c++/1267
	* minsyms.c (lookup_minimal_symbol_by_pc_section): If SECTION is
	NULL, default to the section containing PC.

Index: minsyms.c
===================================================================
RCS file: /cvs/src/src/gdb/minsyms.c,v
retrieving revision 1.31
diff -u -p -r1.31 minsyms.c
--- minsyms.c	15 May 2003 22:23:24 -0000	1.31
+++ minsyms.c	19 Jul 2003 18:03:08 -0000
@@ -403,12 +403,22 @@ lookup_minimal_symbol_by_pc_section (COR
   struct objfile *objfile;
   struct minimal_symbol *msymbol;
   struct minimal_symbol *best_symbol = NULL;
+  struct obj_section *pc_section;
 
   /* 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.  */
-  if (find_pc_section (pc) == NULL)
+  pc_section = find_pc_section (pc);
+  if (pc_section == NULL)
     return NULL;
+
+  /* If no section was specified, then just make sure that the PC is in
+     the same section as the minimal symbol we find.  */
+  if (section == NULL)
+    section = pc_section->the_bfd_section;
+
+  /* FIXME drow/2003-07-19: Should we also check that PC is in SECTION
+     if we were passed a non-NULL SECTION argument?  */
 
   for (objfile = object_files;
        objfile != NULL;