Bigger stepping granularity in debugging

Bigger stepping granularity in debugging

- Single step now steps to a safe point on the next line.
- Step into no longer steps into library code.
- Each library has a flag that specifies whether one
  can step into its code.

Later I will add an API to set the per-library flag. Currently
it's set to true for the top-level library and to false for
all other libraries.
Committed: https://code.google.com/p/dart/source/detail?r=8173

[hausner, 2012-05-31 15:04:25]

Ping.

[siva, 2012-05-31 19:11:15]

LGTM

http://codereview.chromium.org/10442088/diff/2003/runtime/vm/debugger.cc
File runtime/vm/debugger.cc (right):

http://codereview.chromium.org/10442088/diff/2003/runtime/vm/debugger.cc#newc...
runtime/vm/debugger.cc:1045: func_to_instrument = bpt->function();
This will be an issue if we have resursive calls right.

http://codereview.chromium.org/10442088/diff/2003/runtime/vm/debugger.cc#newc...
runtime/vm/debugger.cc:1094: func_to_instrument =
caller_frame->DartFunction().raw();
How does this work if we have recursive calls i.e we are trying to stepout from
the innermost invocation.

http://codereview.chromium.org/10442088/diff/2003/runtime/vm/debugger.cc#newc...
runtime/vm/debugger.cc:1205: // returns from the user-defined breakpoint
callback.
Can you add a comment stating that breakpoints can only be set and deleted when
we are in the breakpoint or ctrl-C handler code and hence we are guaranteed to
run RemoveInternalBreakpoints.

http://codereview.chromium.org/10442088/diff/2003/runtime/vm/debugger.cc#newc...
runtime/vm/debugger.cc:1211: curr_bpt->set_src_bpt(NULL);
can you break out from here instead of looping till the end?

The looping is needed if there could be multiple breakpoints which match src_bpt
but looks like in SetBreakpoints you eliminate duplicates.

[hausner, 2012-05-31 20:04:14]

Thanks.

http://codereview.chromium.org/10442088/diff/2003/runtime/vm/debugger.cc
File runtime/vm/debugger.cc (right):

http://codereview.chromium.org/10442088/diff/2003/runtime/vm/debugger.cc#newc...
runtime/vm/debugger.cc:1045: func_to_instrument = bpt->function();
On 2012/05/31 19:11:15, asiva wrote:
> This will be an issue if we have resursive calls right.

We don't have recursive calls. Breakpoints are ignored when we're currently
handling another breakpoint.

http://codereview.chromium.org/10442088/diff/2003/runtime/vm/debugger.cc#newc...
runtime/vm/debugger.cc:1094: func_to_instrument =
caller_frame->DartFunction().raw();
On 2012/05/31 19:11:15, asiva wrote:
> How does this work if we have recursive calls i.e we are trying to stepout
from
> the innermost invocation.
Recursive functions are not properly treated yet. We step out to the previous
activation frame of the same function.

http://codereview.chromium.org/10442088/diff/2003/runtime/vm/debugger.cc#newc...
runtime/vm/debugger.cc:1205: // returns from the user-defined breakpoint
callback.
On 2012/05/31 19:11:15, asiva wrote:
> Can you add a comment stating that breakpoints can only be set and deleted
when
> we are in the breakpoint or ctrl-C handler code and hence we are guaranteed to
> run RemoveInternalBreakpoints.

Done.

http://codereview.chromium.org/10442088/diff/2003/runtime/vm/debugger.cc#newc...
runtime/vm/debugger.cc:1211: curr_bpt->set_src_bpt(NULL);
There can be multiple code breakpoints pointing to the same source breakpoint.
For example, we can have multiple code objects for the same source location if
the function is used as a closure and is called in its non-closureized version.
So, we need to go through the whole list of code breakpoints.
On 2012/05/31 19:11:15, asiva wrote:
> can you break out from here instead of looping till the end?
> 
> The looping is needed if there could be multiple breakpoints which match
src_bpt
> but looks like in SetBreakpoints you eliminate duplicates.

[siva, 2012-05-31 20:26:56]

http://codereview.chromium.org/10442088/diff/2003/runtime/vm/debugger.cc
File runtime/vm/debugger.cc (right):

http://codereview.chromium.org/10442088/diff/2003/runtime/vm/debugger.cc#newc...
runtime/vm/debugger.cc:1045: func_to_instrument = bpt->function();
I didn't mean recursive calls to the breakpoint handler, I meant recursive dart
calls. We are trying to stepover a recursive call.

On 2012/05/31 20:04:14, hausner wrote:
> On 2012/05/31 19:11:15, asiva wrote:
> > This will be an issue if we have resursive calls right.
> 
> We don't have recursive calls. Breakpoints are ignored when we're currently
> handling another breakpoint.

http://codereview.chromium.org/10442088/diff/2003/runtime/vm/debugger.cc#newc...
runtime/vm/debugger.cc:1094: func_to_instrument =
caller_frame->DartFunction().raw();
Actually we won't step out to the previous activation frame, we will remain in
the current activation frame.

On 2012/05/31 20:04:14, hausner wrote:
> On 2012/05/31 19:11:15, asiva wrote:
> > How does this work if we have recursive calls i.e we are trying to stepout
> from
> > the innermost invocation.
> Recursive functions are not properly treated yet. We step out to the previous
> activation frame of the same function.

[hausner, 2012-05-31 20:32:27]

http://codereview.chromium.org/10442088/diff/2003/runtime/vm/debugger.cc
File runtime/vm/debugger.cc (right):

http://codereview.chromium.org/10442088/diff/2003/runtime/vm/debugger.cc#newc...
runtime/vm/debugger.cc:1094: func_to_instrument =
caller_frame->DartFunction().raw();
Ah, you are right. I will put this on my todo list.

On 2012/05/31 20:26:56, asiva wrote:
> Actually we won't step out to the previous activation frame, we will remain in
> the current activation frame.
> 
> On 2012/05/31 20:04:14, hausner wrote:
> > On 2012/05/31 19:11:15, asiva wrote:
> > > How does this work if we have recursive calls i.e we are trying to stepout
> > from
> > > the innermost invocation.
> > Recursive functions are not properly treated yet. We step out to the
previous
> > activation frame of the same function.
>