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

[jingham at apple.com]

Daniel,

I think your test in IsVirtualInlineStep is too broad.

The virtual inlined step is the one where you have several stack frames with the SAME PC, and you are logically at a higher one of these, and then the user says "step in".  That means just decrement the virtual frame counter, so it looks like the next frame in the "contiguous inlined 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 inlined function, and do a step that is just an ordinary straight-up step, over 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 stack, and if I read it aright, your patch would preserve the stop info from the 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 on Linux which was broken after r181501. The fix involves tweaking Thread::GetPrivateStopReason() to not reset the m_stop_info_sp in the case of a "virtual" inline step-in (that is, a step that does not cause 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 the start of the function. In addition, with my change, I expect that when remotely stepping into an inlined function call, LLDB will avoid the needless call into ThreadGDBRemote::CalculateStopReason, which if I read 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 more changes to accomplish essentially the same thing.
> 
> Reviews are welcome,
> Dan
> <handle_virtual_inline_step.patch>_______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits