Re: Prologue analysis interface

[Daniel Jacobowitz <drow at false dot org>]

On Tue, Nov 22, 2005 at 07:09:44PM -0800, Jim Blandy wrote:
> On 11/22/05, Daniel Jacobowitz <drow@false.org> wrote:
> > One thing that jumped out at me in the m32c analyzer.  You record
> > pushes of registers as frame related, and you record spills of argument
> > registers using "mov" as frame related.
> 
> You mean "not frame related" right?  Yes, it does undermine the
> general principle.  I don't really see why stores of original values
> to the stack would be any worse.

Eh, no, I mean frame related:

              if (m32c_is_arg_spill (&st, dest, src_value))
                after_last_frame_related_insn = st.next_addr;

> > The question in my mind is whether the target-independently-describable
> > tweaks we need for this are good for all targets.  And as unlikely as
> > it may seem, I think it's true.  For instance, S/390 has a bit to use
> > only the stack slot with the highest address if it sees multiple saves.
> > We could easily enough handle that in the target-dependent part in the
> > model I described above, but we don't have to (with an appropriate
> > check for the direction of the stack).  It's probably good just about
> > everywhere.
> 
> I see how the m32c and the S/390 could both be accomodated by a single
> piece of code.  But I think there's something to watch out for here. 
> We don't want the generic prologue analyzer driver to become a
> somewhat complex piece of code with a lot of little behaviors
> carefully chosen to make things work nicely on particular targets ---
> exactly what you're suggesting.  Think about (if I'm remembering
> correctly) monitor_fetch_register.  It's a generic function, meant to
> be shared amongst all sorts of monitors, that does a decent job
> groveling through the output of a ROM monitor's "show me the
> registers" commands and finding the actual values.  When it works,
> it's great, but when it isn't working right for your monitor, you just
> have to custom-code your own register parser.  You can't change the
> generic function's behavior in the slightest, because you don't know
> what behaviors all the other monitors (which you can't test) are
> depending on.  Which puts you in the ridiculous situation of not being
> able to change monitor_fetch_register even if *nobody* wants the
> current behavior --- because you can't discover that that's the case. 
> It's an ossified function.  It will never change.  That's not the end
> of the world --- you just copy it, install it as a custom function in
> the monitor_ops structure, and go on your way.  But you lose the
> benefits you'd like to get from common code.

Point taken.  But please take another look at what I've suggested - in
the end, I ended up leaving almost all of the likely-fiddly-bits to the
target.

My message was a bit confusing to read, since that's not where I
_wanted_ to end up.

> > Both the S/390 and m32c use heuristics to terminate the frame chain.
> > I agree that these are necessary, but I've also come to see a bit of
> > the counterargument that they're in the wrong place.  I think what we
> > need may be an additional method for frames to say "this same unwinder
> > thinks the stack is finished here".  Pathological examples: calling
> > thread_start() or an optimized noreturn function from gdb.
> 
> Why is it better to have a separate method than to have 'this_id'
> leave the id unchanged?

My thinking was: because !frame_id_p (get_frame_id (next_frame)) means
"there can be no frames after me" and "frame_should_unwind_past
(next_frame)" means "it looks like I've come to the end of the stack,
here, I don't know how to go on".  Maybe someone else does, e.g. to
find a dummy frame.  But really the value of this seems minimal.  I
just keep coming back to the nagging feeling that the way we find the
last frame is just not right.

-- 
Daniel Jacobowitz
CodeSourcery, LLC