[Lldb-commits] [PATCH]: Fix inline stepping test case (and possibly improve remote step performance)

Malea, Daniel daniel.malea at intel.com
Mon May 13 15:56:39 PDT 2013

Oops, the one I attached last was incomplete. Here's the complete patch..

On 2013-05-13 6:53 PM, "Malea, Daniel" <daniel.malea at intel.com> wrote:

>Here's a patch that implements the last solution I proposed. It still
>fixes the problem with TestInlineStepping, and I think it's a little more
>general than my initial approach, and should be extendable to handle the
>case you mentioned with continue.
>On 2013-05-13 6:38 PM, "Malea, Daniel" <daniel.malea at intel.com> wrote:
>>So it looks like ThreadPlanStepInRange already has an m_virtual_step
>>member that keeps track of this pretend-stepping sitaution already. I was
>>hesitant to expose this internal flag because I couldn't think of what
>>use/meaning that member would have for other thread plans, but based on
>>the step-out use case you mentioned, maybe it should be exposed?
>>Perhaps something like:
>>ThreadPlan::IsVirtualStep() { return false; }
>>With an override in ThreadPlanStepInRange that returns m_virtual_step.
>>Once we handle the step-out case you mentioned ThreadPlanStepOut (and any
>>other plans which need to do some virtual steps) can also override that
>>What do you think?
>>Just to confirm, there's no existing test case for the use-case you
>>mentioned, right?
>>On 2013-05-13 6:24 PM, "jingham at apple.com" <jingham at apple.com> wrote:
>>>I don't think we handle it yet, but we also should do the same thing on
>>>the way out if you are stepping out of a bunch of inlined functions
>>>returns all have the same PC.  Just something to keep in mind.
>>>And to complete the picture, the other major bit we don't do yet is for
>>>instance if you have:
>>>0x0000    ContainingFunction
>>>0x0004    InlinedFunction1
>>>0x0004    InlinedFunction2
>>>0x0004    InlinedFunction3
>>>and somebody does:
>>>(lldb) break set -n InlinedFunction2
>>>and then runs, we don't properly set the "inlined stack depth" to 1 when
>>>we hit the breakpoint.  So this area still needs a little bit of work.
>>>On May 13, 2013, at 3:19 PM, jingham at apple.com wrote:
>>>> The general idea seems fine.  The thread requests a "virtual step" in
>>>>DoWillResume, maybe you can just keep track of that?  That's more
>>>>declarative, and if we come up with some other reason to "pretend to
>>>>step without stepping" it would get that too.  OTOH, you'd have to
>>>>manage setting it in the right place, and remember to clear it in
>>>>WillResume, so maybe that is a pain?
>>>> Otherwise, if you look in StackFrameList.cpp, you'll see a couple of
>>>>functions that deal with the current inlined depth
>>>>(GetCurrentInlinedDepth, IncrementCurrentInlinedDepth.)  I am pretty
>>>>sure (haven't looked at this code in a while) that you should just be
>>>>able to ask whether the current inlined depth is > 0.  When the PC
>>>>away from the stack of same-pc inlined functions, the InlinedDepth goes
>>>>back to 0 till you get to another similar set.  But just take a look at
>>>>that code and it should be clear.
>>>> Jim
>>>> On May 13, 2013, at 3:07 PM, "Malea, Daniel" <daniel.malea at intel.com>
>>>>> Ah, I see; I didn't think of that case. Thanks for catching it!
>>>>> Do you agree with the basic approach of the patch though? If so,
>>>>> the best way to check for a 'virtual inline step'. Would it be to
>>>>> that frame 0 and frame 1 have the same PC, or is there something else
>>>>> would be better?
>>>>> Dan
>>>>> On 2013-05-13 6:04 PM, "jingham at apple.com" <jingham at apple.com> wrote:
>>>>>> Daniel,
>>>>>> I think your test in IsVirtualInlineStep is too broad.
>>>>>> The virtual inlined step is the one where you have several stack
>>>>>> with the SAME PC, and you are logically at a higher one of these,
>>>>>> then the user says "step in".  That means just decrement the virtual
>>>>>> frame counter, so it looks like the next frame in the "contiguous
>>>>>> frame stack" is the current one.  But you don't have to run the
>>>>>>target to
>>>>>> do this.
>>>>>> So a step is not a "virtual inlined step" just because the stack
>>>>>>frame at
>>>>>> index 0 is inlined.  After all, you could be in the middle of an
>>>>>> function, and do a step that is just an ordinary straight-up step,
>>>>>> some code with no debug info, etc.  In the course of doing that, you
>>>>>> could hit a breakpoint in inlined code somewhere deeper on the
>>>>>> if I read it aright, your patch would preserve the stop info from
>>>>>> step in, which would be incorrect.
>>>>>> Jim
>>>>>> On May 13, 2013, at 2:18 PM, "Malea, Daniel"
>>>>>><daniel.malea at intel.com>
>>>>>> wrote:
>>>>>>> Hi all,
>>>>>>> Please review the attached patch that fixes the TestInlineStep case
>>>>>>> Linux which was broken after r181501. The fix involves tweaking
>>>>>>> Thread::GetPrivateStopReason() to not reset the m_stop_info_sp in
>>>>>>> case of a "virtual" inline step-in (that is, a step that does not
>>>>>>> the inferior process to resume, but rather just pretends to step
>>>>>>>into an
>>>>>>> inlined function call).
>>>>>>> I also removed a double-initialization and check for a Process at
>>>>>>> start of the function. In addition, with my change, I expect that
>>>>>>> remotely stepping into an inlined function call, LLDB will avoid
>>>>>>> needless call into ThreadGDBRemote::CalculateStopReason, which if I
>>>>>>> it correctly, causes some packets to be sent over the wire.
>>>>>>> An alternative way to implement what I'm doing here would be to
>>>>>>> introduce a new stop reason as per the comment in
>>>>>>> ThreadPlanStepInRange.cpp:458 but I expect that will require a lot
>>>>>>> changes to accomplish essentially the same thing.
>>>>>>> Reviews are welcome,
>>>>>>> Dan
>>>>>>> ________
>>>>>>> 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
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>>lldb-commits mailing list
>>lldb-commits at cs.uiuc.edu

-------------- next part --------------
A non-text attachment was scrubbed...
Name: handle_virtual_inline_step_v3.patch
Type: application/octet-stream
Size: 3175 bytes
Desc: handle_virtual_inline_step_v3.patch
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20130513/659ef5a6/attachment.obj>

More information about the lldb-commits mailing list