From: Mark Kettenis <mark.kettenis@xs4all.nl>
To: brobecker@adacore.com
Cc: gdb-patches@sourceware.org, jimb@codesourcery.com
Subject: Re: [rfc] Unwind the ARM CPSR
Date: Tue, 16 Oct 2007 22:09:00 -0000 [thread overview]
Message-ID: <200710162201.l9GM13MR006755@brahms.sibelius.xs4all.nl> (raw)
In-Reply-To: <20071012035336.GA4038@adacore.com> (message from Joel Brobecker on Thu, 11 Oct 2007 20:53:36 -0700)
> Date: Thu, 11 Oct 2007 20:53:36 -0700
> From: Joel Brobecker <brobecker@adacore.com>
>
> > So why shouldn't the argument be the CURRENT (i.e. THIS) frame
> > instead? Then we can call frame_register (CURRENT) instead of
> > frame_unwind_register (NEXT) to get the same result, plus we'll
> > have the option of calling frame_unwind_register (CURRENT) when
> > we need it.
And risking infinite recursion? I'm fairly sure it is a mistake to
call frame_register(CURRENT) at this place.
> I can see why you'd like some feedback. This (amazing piece of) code
> has always been a bit difficult for me to grasp in its entirety...
> I hope others will take some time to think about it too, because
> I don't feel sufficiently proficient to provide a confident answer.
>
> FWIW, I don't see how we could end up breaking something with your
> approach. As it turns out, there is a comment that says about
> the prev_register method:
>
> Why not pass in THIS_FRAME? By passing in NEXT frame and THIS
> cache, the supplied parameters are consistent with the sibling
> function THIS_ID.
>
> So it sounds like the author wasn't seeing any issue with that
> either.
I wouldn't be so sure. Andrew Cagney put quite a bit of thought in
the interface, and while it sometimes appears to be strange on first
sight, it has always proven to be the right choice.
> I personally think that passing the current frame will make
> this function a little easier to understand too, no? (I find
> the twist of computing the caller's registers of this frame
> using the next frame a constant mental exercise)
I agree it is a mental excercise, but it prevents you from doing
stupid things.
> > Of course this would be a pain to change all at once since there are
> > so many unwinders. I'd introduce a new method instead
> > (unwind->prev_register_this?). What do you think?
>
> Sounds like a possible plan to me.
I think it is a very bad idea. It will take ages before all targets
are converted, and having two functions to do the same thing will be
even more confusing.
next prev parent reply other threads:[~2007-10-16 22:04 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-10-11 14:59 Daniel Jacobowitz
2007-10-11 15:52 ` Daniel Jacobowitz
2007-10-12 8:08 ` Joel Brobecker
2007-10-16 22:09 ` Mark Kettenis [this message]
2007-10-17 0:36 ` Daniel Jacobowitz
2007-10-17 6:05 ` Daniel Jacobowitz
2007-10-17 13:18 ` Joel Brobecker
2007-10-19 11:54 ` Ulrich Weigand
2007-10-17 0:30 ` Mark Kettenis
2008-05-01 18:31 ` Daniel Jacobowitz
2008-05-01 18:35 ` Daniel Jacobowitz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200710162201.l9GM13MR006755@brahms.sibelius.xs4all.nl \
--to=mark.kettenis@xs4all.nl \
--cc=brobecker@adacore.com \
--cc=gdb-patches@sourceware.org \
--cc=jimb@codesourcery.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox