[Lldb-commits] [lldb][PATCH] Clear thread unwinder when stack is reset

Kaylor, Andrew andrew.kaylor at intel.com
Mon Oct 22 16:42:26 PDT 2012

Hi Jason,

Have you had a chance to take a closer look at this yet?


-----Original Message-----
From: lldb-commits-bounces at cs.uiuc.edu [mailto:lldb-commits-bounces at cs.uiuc.edu] On Behalf Of Kaylor, Andrew
Sent: Tuesday, October 02, 2012 9:27 AM
To: Jason Molenda
Cc: lldb-commits at cs.uiuc.edu
Subject: Re: [Lldb-commits] [lldb][PATCH] Clear thread unwinder when stack is reset

Hi Jason,

Thanks for your looking at this.

This is indeed complicated code!  I was a bit perplexed as I worked through this by the number of different places that stack frames and register contexts were being tracked.  I'll admit that my own understanding of it all is vague at best.

In the version of the patch I submitted that calls Unwind::Clear from Thread::ResetFrameZeroRegisters that call would happen immediately following a call to Thread::ClearStackFrames, and of course my other candidate for clearing was within ClearStackFrames itself, so unless I'm misunderstanding something (most certainly possible) there shouldn't be any problems with regard to saved frames.

In the particular case I was working with, a call to InferiorCallMmap had hijacked a thread in the inferior for a call to mmap and it needed to put things back the way they were when that call was done.  A private breakpoint at the mmap return address had caused the Thread::Unwind object to store an essentially bogus stack, and so when Process::RunThreadPlan was trying to restore the selected frame after its work was done, it couldn't find the frame it was looking for.  (BTW, the code at this point is a being worrisome in that it does nothing if it can't find the old selected frame.  It seems like it might be best to raise an error, or at least log something, there.)  The consequence was that subsequent lldb commands were confused as to what the current frame was.  For instance, 'expression argc' failed because it didn't recognize that 'argc' was a parameter to the current function.

In this case, at least, clearing the unwinder's internal state sometime after the ThreadPlanCallFunction has cleaned up the actual frame is necessary.  I'm reasonably confident that the correct place to do this is from within the ThreadPlanCallFunction::DoTakedown call to m_thread.RestoreThreadStateFromCheckpoint.  I'm less confident about the details of precisely how it should happen, so I appreciate your input in that regard.


-----Original Message-----
From: Jason Molenda [mailto:jmolenda at apple.com] 
Sent: Monday, October 01, 2012 11:39 PM
To: Kaylor, Andrew
Cc: lldb-commits at cs.uiuc.edu
Subject: Re: [Lldb-commits] [lldb][PATCH] Clear thread unwinder when stack is reset

Thanks for submitting the patches Andy.  This part of the unwinder is a little fragile and hard to see the dependencies correctly - I'm not yet sure what I think here.

Normally a thread has an Unwind object (usually an UnwindLLDB object).  The UnwindLLDB object provides the Thread with RegisterContexts up the stack -- it provides register values for all the stack frames on a thread.  So if you want to get rip on frame 4, the Unwind will give you a RegisterContext for frame 4 that provides the value, assuming it is retrievable (and it will usually do this by seeing where frame 3 stored the value, going down the stack frames until it finds someone who saved it.)

The problem is this:  The Thread has an Unwind object in the m_unwinder_ap, and the Unwind object has an array of RegisterContexts that it is responsible for.  But IIRC the StackFrames also have pointers into these RegisterContexts.  If we're throwing away all known StackFrames for a Thread, it should be safe to reset the Unwind object.  But if we're *restoring* stack frames that were saved aside earlier, I worry that they may have pointers to RegisterContexts that have since been freed or something.

(and to further complicate things, I think the RegisterContextLLBB's refer to their "owning" UnwindLLDB object to retrieve values).

This is probably all done with refcounted shared pointers and will "just work" but it's a complicated enough interdependency that I need to look at it a bit more closely before I'm confident about what the right answer is here.  I haven't looked at this part of the code in probably a year or so.


On Oct 1, 2012, at 1:42 PM, "Kaylor, Andrew" <andrew.kaylor at intel.com> wrote:

> In debugging a problem with expression evaluation after an inferior function call, I tracked it back to the Thread object's m_unwinder_ap member not being cleared when the Thread state was restored from a checkpoint.  I was able to fix the problem by inserting a call to Unwind::Clear in the Thread::ResetFrameZeroRegisters function.  I thought that it might make sense to clear the unwinder every time the thread's stack was cleared (i.e. from Thread::ClearStackFrames), but I wasn't sure how that is used and whether it always indicates an actual stack change that the unwinder needs to know about.
> The problem in question showed up in the BasicExprCommandsTestCase.test_many_expr_commands test on x86_64 Linux.
> I'm attaching two patches, one with each implementation, though only one is needed.  Can someone offer some guidance as to which approach is preferred?
> Thanks,
> Andy
> <unwinder-reset.patch><reset-frame-zero-unwind.patch>_______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

lldb-commits mailing list
lldb-commits at cs.uiuc.edu

More information about the lldb-commits mailing list