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

[Andrew Cagney <ac131313@redhat.com>]

> 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:

Nope (it pays to look at the archives).

>   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.

The complete code body is:

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

   if (frame_id_p (step_frame_id)
       && !IN_SOLIB_DYNSYM_RESOLVE_CODE (sr_sal.pc))
     step_resume_breakpoint->frame_id = step_frame_id;

while the original code looks like:

   struct symtab_and_line sr_sal;

   init_sal (&sr_sal);		/* initialize to zeros */
   sr_sal.pc = ADDR_BITS_REMOVE (SAVED_PC_AFTER_CALL (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_current_frame (), 
bp_step_resume);

   if (step_frame_address && !IN_SOLIB_DYNSYM_RESOLVE_CODE (sr_sal.pc))
     step_resume_breakpoint->frame = step_frame_address;

   if (breakpoints_inserted)
     insert_breakpoints ();

It would appear that the get_frame_id() call has been wrong for a long 
long time but, at a guess, was worked around by picking up step_frame_id 
/ step_frame_address.

Andrew