Better single stepping in VM debugger

Better single stepping in VM debugger

Single stepping now steps into the next dart code that the
user is interested in, including from one asynchronous task
to the next.

R=asiva@google.com

Committed: https://code.google.com/p/dart/source/detail?r=24632

[hausner, 2013-06-28 18:31:06]

Ok, this is ready now for a review.

[siva, 2013-07-01 17:04:48]

LGTM.

https://codereview.chromium.org/17846009/diff/25002/runtime/vm/debugger.cc
File runtime/vm/debugger.cc (right):

https://codereview.chromium.org/17846009/diff/25002/runtime/vm/debugger.cc#ne...
runtime/vm/debugger.cc:1599: }
I might have asked this question before, how does this work when one is
currently in a function and is trying to step out or step over a recursive
invocation of the same function that we are currently stopped in.

https://codereview.chromium.org/17846009/diff/25002/runtime/vm/debugger.h
File runtime/vm/debugger.h (right):

https://codereview.chromium.org/17846009/diff/25002/runtime/vm/debugger.h#new...
runtime/vm/debugger.h:272: bool IsStepping() { return resume_action_ !=
kContinue; }
const

https://codereview.chromium.org/17846009/diff/25002/runtime/vm/debugger.h#new...
runtime/vm/debugger.h:348: ActivationFrame* TopDartFrame();
const

https://codereview.chromium.org/17846009/diff/25002/runtime/vm/stub_code_arm.cc
File runtime/vm/stub_code_arm.cc (right):

https://codereview.chromium.org/17846009/diff/25002/runtime/vm/stub_code_arm....
runtime/vm/stub_code_arm.cc:1641: __ Bind(&not_stepping);
why not abstract this into a private function
StubCode::CheckForSingleStepping(Register tmp, bool preserve_ic_data);
Would avoid code duplication.

Here and in the other targets too.

https://codereview.chromium.org/17846009/diff/25002/tests/standalone/standalo...
File tests/standalone/standalone.status (right):

https://codereview.chromium.org/17846009/diff/25002/tests/standalone/standalo...
tests/standalone/standalone.status:157: 
Why is  out_of_memory_test related to this change.

https://codereview.chromium.org/17846009/diff/25002/tests/standalone/standalo...
tests/standalone/standalone.status:165: 
What is this extra line for?

[hausner, 2013-07-01 17:29:29]

Committed patchset #5 manually as r24632 (presubmit successful).

[hausner, 2013-08-09 09:59:02]

Ahem.

https://codereview.chromium.org/17846009/diff/25002/runtime/vm/debugger.cc
File runtime/vm/debugger.cc (right):

https://codereview.chromium.org/17846009/diff/25002/runtime/vm/debugger.cc#ne...
runtime/vm/debugger.cc:1599: }
On 2013/07/01 17:04:48, siva wrote:
> I might have asked this question before, how does this work when one is
> currently in a function and is trying to step out or step over a recursive
> invocation of the same function that we are currently stopped in.

Stepping out of a function works by setting temporary breakpoints in the caller,
then resume. In the case of recursive calls, this is indeed not a very good
solution since the function that gets instrumented with temp breakpoints is the
same function we're currently stopped in.

I could walk the stack and instrument the first function that is different from
the current function. But that would not allow the user to step out one
recursive activation frame at a time.

Maybe we can discuss this offline again.

https://codereview.chromium.org/17846009/diff/25002/runtime/vm/debugger.h
File runtime/vm/debugger.h (right):

https://codereview.chromium.org/17846009/diff/25002/runtime/vm/debugger.h#new...
runtime/vm/debugger.h:272: bool IsStepping() { return resume_action_ !=
kContinue; }
On 2013/07/01 17:04:48, siva wrote:
> const

Done.

https://codereview.chromium.org/17846009/diff/25002/runtime/vm/debugger.h#new...
runtime/vm/debugger.h:348: ActivationFrame* TopDartFrame();
On 2013/07/01 17:04:48, siva wrote:
> const

Done. A number of debugger methods modify the debugger object by caching
something as a side effect, so I didn't care much about 'constness'.

https://codereview.chromium.org/17846009/diff/25002/runtime/vm/stub_code_arm.cc
File runtime/vm/stub_code_arm.cc (right):

https://codereview.chromium.org/17846009/diff/25002/runtime/vm/stub_code_arm....
runtime/vm/stub_code_arm.cc:1641: __ Bind(&not_stepping);
On 2013/07/01 17:04:48, siva wrote:
> why not abstract this into a private function
> StubCode::CheckForSingleStepping(Register tmp, bool preserve_ic_data);
> Would avoid code duplication.
> 
> Here and in the other targets too.

I'll do that in a separate checkin once I add more places where we do this
check. Good idea.

https://codereview.chromium.org/17846009/diff/25002/tests/standalone/standalo...
File tests/standalone/standalone.status (right):

https://codereview.chromium.org/17846009/diff/25002/tests/standalone/standalo...
tests/standalone/standalone.status:157: 
On 2013/07/01 17:04:48, siva wrote:
> Why is  out_of_memory_test related to this change.

I can't explain that :) I just found that these tests are no longer crashing on
my system.

https://codereview.chromium.org/17846009/diff/25002/tests/standalone/standalo...
tests/standalone/standalone.status:165: 
On 2013/07/01 17:04:48, siva wrote:
> What is this extra line for?

I get paid by the number of lines added :)

Removed.