This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [rfc] Unwind the ARM CPSR


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]