Prologue analysis interface

Prologue analysis interface

[Jim Blandy]

Daniel, you said that you felt that the interface in the
prologue-value.h that I posted wasn't at the right level, and
suggested that, instead of providing types and some operations on
those types, it should instead provide the interpretation loop and
call back into tdep code to interpret specific instructions.

Could you sketch that interface for me?  I'm concerned that it will be
difficult to allow the tdep code the flexibility it needs to model the
processor without a lot of bureaucracy, but I'm willing to be
impressed.

Re: Prologue analysis interface

[Daniel Jacobowitz]

On Tue, Nov 22, 2005 at 11:24:06AM -0800, Jim Blandy wrote:
> Daniel, you said that you felt that the interface in the
> prologue-value.h that I posted wasn't at the right level, and
> suggested that, instead of providing types and some operations on
> those types, it should instead provide the interpretation loop and
> call back into tdep code to interpret specific instructions.
> 
> Could you sketch that interface for me?  I'm concerned that it will be
> difficult to allow the tdep code the flexibility it needs to model the
> processor without a lot of bureaucracy, but I'm willing to be
> impressed.

What I don't know is how much flexibility is needed.  Maybe it's more
than I thought.

I'm penciling in reading the existing s390 analyzer and posted m32c
analyzers for tonight's schedule, since this keeps coming up in
conversation.  I promise I'll get back to you about this soon.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: Prologue analysis interface

[Jim Blandy]

On 11/22/05, Daniel Jacobowitz <drow@false.org> wrote:
> What I don't know is how much flexibility is needed.  Maybe it's more
> than I thought.

Could be less than I think.  I'm interested to see what you come up with.

> I'm penciling in reading the existing s390 analyzer and posted m32c
> analyzers for tonight's schedule, since this keeps coming up in
> conversation.  I promise I'll get back to you about this soon.

Thanks.  The m32c port is blocked on this; I'd like to get it out of the way.

Re: Prologue analysis interface

[Daniel Jacobowitz]

On Tue, Nov 22, 2005 at 03:49:59PM -0800, Jim Blandy wrote:
> On 11/22/05, Daniel Jacobowitz <drow@false.org> wrote:
> > What I don't know is how much flexibility is needed.  Maybe it's more
> > than I thought.
> 
> Could be less than I think.  I'm interested to see what you come up with.
> 
> > I'm penciling in reading the existing s390 analyzer and posted m32c
> > analyzers for tonight's schedule, since this keeps coming up in
> > conversation.  I promise I'll get back to you about this soon.
> 
> Thanks.  The m32c port is blocked on this; I'd like to get it out of the way.

OK, here's what I've got from reading the m32c port.  I'm afraid that
I don't have a whole lot of time to build such an interface myself,
just now; what GDB time I have for the next couple of weeks is going to
go to implementing my "target properties query" interface from earlier
this year.  But I do still feel that this code should call into the
tdep file rather than the other way around.

Almost everything the analyzer does for m32c is perfectly explainable
as a generic concept.  80% of that is decoding what an instruction
accomplishes.  The rest seems to break down into:

- Setting an initial state, by modeling the call instruction.

- Recognizing "frame-related" stores to the stack, to find the real end
of the prologue.

- Recognizing whether a frame pointer has been set up, and if so, what
it is and its relation to the CFA.

- Finding saved registers in the stack after analyzing.

- All of the infrastructure for a frame unwinder based on this data.

The frame pointer bits do involve some target heuristics.  I think we
could do away with that, if we had reason to (i.e. an interesting
target which didn't use a fixed register for this).  But I'm not
suggesting it until we have such a target, hopefully not for a long
time.

The simplest form of a common-code-driven interface would just call
target supplied routines to initialize state, decode and process an
instruction, and finalize any use of a frame pointer.  The common code
would subsume m32c_skip_prologue through m32c_frame_sniffer. 
m32c_analyze_prologue would be broken up into the init/decode/fini
hooks, or even (for now this seems just as good) left as containing the
whole loop, but the pv_area_scan call at the end could live in
target-independent code.

I did have a more sophisticated approach in mind but I think it's not
justified yet.  I spoke with Alan briefly about supplying some of the
decoding information from the disassembler, which would help, but it's
a big project.

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.  This seems to me to defeat the
appeal of a code-driven analyzer somewhat; it'll break if the compiler
uses "mov" instead of "push" to save a register.  I think all stores of
the original value of registers (or part of that value), to the stack,
are effectively frame related; whether they're arg registers or not is
irrelevant.  At that point it's pretty much a target-independent
concept.

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.

If time proves me wrong on this, we can migrate checks into targets as
necessary.  I'm trying to minimize the amount of "boilerplate" code a
target needs, so that we can (A) make targets easier and smaller,
and (B) improve the template bits and see benefit uniformly.

The S/390 code is a lot less clear-cut than the m32c, not surprisingly.
Most of the tangly bits are related to frameless functions.  Some of
this could be further commonized too, but for now my suggested plan
above should work for this also.

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.

So, for now, how about that?  Just provide something along the lines of
the tramp-frame interface that does the busywork.  We can adjust later
what is busywork and what isn't.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: Prologue analysis interface

[Joel Brobecker]

> The frame pointer bits do involve some target heuristics.  I think we
> could do away with that, if we had reason to (i.e. an interesting
> target which didn't use a fixed register for this).  But I'm not
> suggesting it until we have such a target, hopefully not for a long
> time.

Not sure if this is relevant, but the HPPA ABI says that when there
is a frame pointer, it is either in r3 or r4, depending on the frame
size (IIRC). GCC doesn't follow the ABI, and always uses r3, but I am
pretty sure that the HP C compiler does follow that convention...

-- 
Joel

Re: Prologue analysis interface

[Jim Blandy]

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.

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

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

> So, for now, how about that?  Just provide something along the lines of
> the tramp-frame interface that does the busywork.  We can adjust later
> what is busywork and what isn't.

Having voiced my misgivings above, I'll give it a shot.

Re: Prologue analysis interface

[Jim Blandy]

On 11/22/05, Joel Brobecker <brobecker@adacore.com> wrote:
> > The frame pointer bits do involve some target heuristics.  I think we
> > could do away with that, if we had reason to (i.e. an interesting
> > target which didn't use a fixed register for this).  But I'm not
> > suggesting it until we have such a target, hopefully not for a long
> > time.
>
> Not sure if this is relevant, but the HPPA ABI says that when there
> is a frame pointer, it is either in r3 or r4, depending on the frame
> size (IIRC). GCC doesn't follow the ABI, and always uses r3, but I am
> pretty sure that the HP C compiler does follow that convention...

The MIPS (if I recall, the first architecture to give us real
headaches in prologue analysis, before it became popular, things were
pretty simple and the current approach worked pretty well) doesn't
designate a particular register for use as a frame pointer, either.

I think all we really need to know is how to find the original SP (for
use as the CFA).  Having a designated frame pointer narrows the set of
places we need to look, but we could search all the registers for
something with the right value.

Re: Prologue analysis interface

[Daniel Jacobowitz]

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

Re: Prologue analysis interface

[Ulrich Weigand]

Daniel Jacobowitz wrote:

> The S/390 code is a lot less clear-cut than the m32c, not surprisingly.
> Most of the tangly bits are related to frameless functions.  Some of
> this could be further commonized too, but for now my suggested plan
> above should work for this also.

Note that some of the heuristics is actually wrong.  I hope to be able
to update the s390 prolog analyzer soon to simplify it a bit and fix
some of the issues I've found.  Unfortunately I cannot guarantee I'll
find time to do so right now ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  Linux on zSeries Development
  Ulrich.Weigand@de.ibm.com

Re: Prologue analysis interface

[Ulrich Weigand]

Jim Blandy wrote:

> I think all we really need to know is how to find the original SP (for
> use as the CFA).

Just one comment on this:  the CFA is not necessarily the original SP
(on s390 we have a constant offset).  We'd have to leave it to platform
code to actually determine the CFA itself.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  Linux on zSeries Development
  Ulrich.Weigand@de.ibm.com

Re: Prologue analysis interface

[Joel Brobecker]

> > I think all we really need to know is how to find the original SP (for
> > use as the CFA).
> 
> Just one comment on this:  the CFA is not necessarily the original SP
> (on s390 we have a constant offset).  We'd have to leave it to platform
> code to actually determine the CFA itself.

If I understand the new approach correctly, we really don't need to know
what the CFA is. All you need to know is where things have been saved.
Things like : r3 is saved at original SP + offset, or r4 saved at r30 +
offset. Right?

-- 
Joel

Re: Prologue analysis interface

[Jim Blandy]

On 11/23/05, Joel Brobecker <brobecker@adacore.com> wrote:
> If I understand the new approach correctly, we really don't need to know
> what the CFA is. All you need to know is where things have been saved.
> Things like : r3 is saved at original SP + offset, or r4 saved at r30 +
> offset. Right?

We need it for use in frame ID's.

Re: Prologue analysis interface

[Jim Blandy]

On 11/23/05, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> Just one comment on this:  the CFA is not necessarily the original SP
> (on s390 we have a constant offset).  We'd have to leave it to platform
> code to actually determine the CFA itself.

Why does your CFA need to be an offset from the original SP?  Aside
from some more code needing to add an offset itself, what would break?

Re: Prologue analysis interface

[Ulrich Weigand]

Jim Blandy wrote:
> On 11/23/05, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> > Just one comment on this:  the CFA is not necessarily the original SP
> > (on s390 we have a constant offset).  We'd have to leave it to platform
> > code to actually determine the CFA itself.
> 
> Why does your CFA need to be an offset from the original SP?  Aside
> from some more code needing to add an offset itself, what would break?

Well, as CFA is just convention, it does *need* an offset as such.
However, fact is that this is how CFA is currently registered in
DWARF-2 CFI generated by GCC (due to an historical accident ...).

Changing this is difficult: if you were to mix object files built with
different CFA base locations, ordering of CFA values along the stack
frame chain would no longer be guaranteed.  (The same would hold if
GDB internally used different CFA conventions for DWARF-2 detected
frames and prolog-parser detected frames ...)

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  Linux on zSeries Development
  Ulrich.Weigand@de.ibm.com

Re: Prologue analysis interface

[Jim Blandy]

On 11/24/05, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> Changing this is difficult: if you were to mix object files built with
> different CFA base locations, ordering of CFA values along the stack
> frame chain would no longer be guaranteed.  (The same would hold if
> GDB internally used different CFA conventions for DWARF-2 detected
> frames and prolog-parser detected frames ...)

Yes --- certainly the prologue CFA and Dwarf CFA need to match.

Re: Prologue analysis interface

[Daniel Jacobowitz]

On Tue, Nov 22, 2005 at 11:24:06AM -0800, Jim Blandy wrote:
> Daniel, you said that you felt that the interface in the
> prologue-value.h that I posted wasn't at the right level, and
> suggested that, instead of providing types and some operations on
> those types, it should instead provide the interpretation loop and
> call back into tdep code to interpret specific instructions.
> 
> Could you sketch that interface for me?  I'm concerned that it will be
> difficult to allow the tdep code the flexibility it needs to model the
> processor without a lot of bureaucracy, but I'm willing to be
> impressed.

Hi Jim,

I'm assuming you haven't had any time for this since we last talked
about it.  If that's true, how do you feel about reposting the current
version of the code for review anyway?

I just needed to fix a prologue analysis bug for Thumb.  It took me
about six hours to rewrite the prologue analyzer from scratch based
on prologue-value.[ch], and it has equivalent testsuite results to
the DWARF-2 unwinder; one more pass than the existing unwinder with
DWARF-2 disabled; and handles the somewhat odd case that I was trying
to fix without impairing anything else.  It's an obvious step forwards.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Prologue analysis interface

[Jim Blandy]

On 1/27/06, Daniel Jacobowitz <drow@false.org> wrote:
> I'm assuming you haven't had any time for this since we last talked
> about it.  If that's true, how do you feel about reposting the current
> version of the code for review anyway?

Yes, I haven't had time for it.  I'll re-post.

> I just needed to fix a prologue analysis bug for Thumb.  It took me
> about six hours to rewrite the prologue analyzer from scratch based
> on prologue-value.[ch], and it has equivalent testsuite results to
> the DWARF-2 unwinder; one more pass than the existing unwinder with
> DWARF-2 disabled; and handles the somewhat odd case that I was trying
> to fix without impairing anything else.  It's an obvious step forwards.

Yay! :)

Re: Prologue analysis interface

[Jim Blandy]

[-- Attachment #1: Type: text/plain, Size: 224 bytes --]

I merged some recent changes from our internal tree, and there's some
bitrot; I haven't looked at what's wrong yet.  But if anyone is
interested, here's the patch as it stands.

This patch includes prologue-value.[ch].

[-- Attachment #2: jimb.gdb-contribute-m32c.patch.gz --]
[-- Type: application/x-gzip, Size: 30436 bytes --]