Re: [offbyone RFC] Merge i386newframe

[Mark Kettenis <kettenis@chello.nl>]

   Date: Sun, 06 Apr 2003 13:10:40 -0400
   From: Andrew Cagney <ac131313@redhat.com>

   [picking up old thread]

   >   The need for the above suggests code trying to walk up the frame chain 
   >    when it shouldn't need to.  Do you have more details?
   > 
   >    >  static CORE_ADDR
   >    >  i386_saved_pc_after_call (struct frame_info *frame)
   >    >  {
   >    > -  if (get_frame_type (frame) == SIGTRAMP_FRAME)
   >    > -    return i386_sigtramp_saved_pc (frame);
   >    > +  char buf[4];
   >    >  
   >    > -  return read_memory_unsigned_integer (read_register (SP_REGNUM), 4);
   >    > +  /* Our frame unwinder handles this just fine.  */
   >    > +  frame_unwind_register (frame, PC_REGNUM, buf);
   >    > +  return extract_address (buf, 4);
   >    >  }
   > 
   >    Idea's for what to do with this architecture method welcome.
   > 
   >    I believe the intent is for this method to have relatively low overhead 
   >    (when measured by the number of target interactions).  Hence, it should 
   >    avoid doing prologue analysis (which frame_unwind_register() will trigger).

   If that was the intent, then it no longer applies.  The call site looks 
   like:

      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_frame_id (get_current_frame ()),
				  bp_step_resume);

   Not five lines after the SAVED_PC_AFTER_CALL call is a call to 
   get_frame_id() and that is going to trigger the prologue analyser.  Kind 
   of makes avoiding prologue analysis futile.

Indeed.

   > Hmm.  I was under the impression that we have this function because on
   > some targets (the i386 is one of them) the frame hasn't been setup yet
   > when we've stopped on the first instruction of a function.

   I think the prologue analyzer needs to handle this case regardless.  It 
   is just an edge case of the more general problem of determing the frame 
   ID when still part way through the prologue.  The d10v handles this by 
   bailing out of the prologue analysis when it reaches the current 
   instruction.

I totally agree with you here.

   >    Perhaphs it should be superseeded by a method that takes a regcache 
   >    instead of a frame (making the non-analysis of the prologue clearer)?
   > 
   > I think that would be a good idea.

   On second thoughts, I'm back to thinking that deprecating it is the 
   right thing to do.  Architectures need to fix their prologue analyzer.

Please do so.  Make things as simple as possible now, and let's
optimize *after* the new frame code has stabilized, if the need
arises.  If SAVED_PC_AFTER_CALL was an optimization, chances are it
isn't anymore with the new code.

Mark