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

[Andrew Cagney <ac131313 at redhat dot 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