Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to set breakpoints in parts of a library #29988

Closed
bergwerf opened this issue Jun 22, 2017 · 13 comments
Closed

Unable to set breakpoints in parts of a library #29988

bergwerf opened this issue Jun 22, 2017 · 13 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. vm-debugger

Comments

@bergwerf
Copy link

I know this sounds dramatic, but somehow it seems to be the case.
Dart version 1.25.0-dev.1.0, downgrading to 1.24.1-1 fixed the issue. I use Dart debugging with VS code, not sure how to reproduce it on the command line. Can be reproduced with https://github.com/hermanbergwerf/euclid by setting a breakpoint at https://github.com/hermanbergwerf/euclid/blob/e12b0cae7af7482ef56da9b971ff2bcaf8f6a63c/lib/src/algorithms/conics_intersection.dart#L37.

@rmacnak-google
Copy link
Contributor

return result.map((v) => v.xy / v.z).toList();

This line has code that is in intersectConics and code in the anonymous closure. If the breakpoint is resolved a position in the anonymous closure, then the debugger won't pause on that line when the code in intersectConics runs.

I think @sivachandra made changes recently to how breakpoints are resolved, which might explain a difference in behavior between 1.24 and 1.25.

The deeper issue here is line-based breakpoint are ambiguous, and VS should be using precise source locations.

@rmacnak-google rmacnak-google added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. vm-debugger labels Jun 22, 2017
@bergwerf
Copy link
Author

Right. I don't know how this is implemented, or how it should be implemented. But I guess this means https://github.com/Dart-Code/Dart-Code (Dart plugin for VS code) must be patched. I opened an issue there: Dart-Code/Dart-Code#334.

@sivachandra sivachandra self-assigned this Jun 22, 2017
@DanTup
Copy link
Collaborator

DanTup commented Jun 22, 2017

The deeper issue here is line-based breakpoint are ambiguous, and VS should be using precise source locations.

I've got a case open to support this; I don't think it was originally supported by Code:

Dart-Code/Dart-Code#260

If the breakpoint is resolved a position in the anonymous closure, then the debugger won't pause on that line when the code in intersectConics runs.

I'm slightly confused by this - where the debugger resolves the breakpoint to and chooses to stop is surely out of my (Dart Code's) hands? If I'm not currently providing a column number for the breakpoint I would expect it to resolve to the start of the line, but even if it resolved inside the anonymous function surely the debugger would break on that line in either case? (unless of course the function never executes, but looking at the code it doesn't seem like that's a possibility)?

@DanTup
Copy link
Collaborator

DanTup commented Jun 23, 2017

I don't believe this is related to column numbers, because in the repro @hermanbergwerf provided in Dart-Code/Dart-Code#334 the breakpoint is on a line that looks like this:

  print('Break!');

Here's my notes and repro from Dart-Code/Dart-Code#334. If this isn't a bug in 1.25 then I'll need some info on what I'm doing wrong and how I can fix it.


Ok, I ran this same test in 1.24 and 1.25 and found this..

We currently send breakpoints twice because we previously found that depending on how a file was imported, the debugger would not break on it (some discussion in Dart-Code/Dart-Code#134). So the request to add breakpoints looks like this:

==> {"id":"5","method":"addBreakpointWithScriptUri","params":{"isolateId":"isolates/11111111","scriptUri":"file:///c:/Users/danny/Desktop/DartDebugSample/lib/src/test.dart","line":4}}
==> {"id":"6","method":"addBreakpointWithScriptUri","params":{"isolateId":"isolates/11111111","scriptUri":"package:yourpackage/src/test.dart","line":4}}

In 1.24, both of these breakpoints are accepted:

<== {"jsonrpc":"2.0", "result":{"type":"Breakpoint","fixedId":true,"id":"breakpoints\/1","breakpointNumber":1,"resolved":false,"location":{"type":"UnresolvedSourceLocation","scriptUri":"file:\/\/\/c:\/Users\/danny\/Desktop\/DartDebugSample\/lib\/src\/test.dart","line":4}},"id":"5"}
<== {"jsonrpc":"2.0", "result":{"type":"Breakpoint","fixedId":true,"id":"breakpoints\/2","breakpointNumber":2,"resolved":false,"location":{"type":"UnresolvedSourceLocation","script":{"type":"@Script","fixedId":true,"id":"libraries\/@17134453\/scripts\/package%3Ayourpackage%2Fsrc%2Ftest.dart\/15cd52126c7","uri":"package:yourpackage\/src\/test.dart","_kind":"source"},"line":4}},"id":"6"}

and when we go on to execute, we stop at the second breakpoint - the "package" version.

However, here's what happens in 1.25. We send the same requests:

==> {"id":"5","method":"addBreakpointWithScriptUri","params":{"isolateId":"isolates/11111111","scriptUri":"file:///c:/Users/danny/Desktop/DartDebugSample/lib/src/test.dart","line":4}}
==> {"id":"6","method":"addBreakpointWithScriptUri","params":{"isolateId":"isolates/11111111","scriptUri":"package:yourpackage/src/test.dart","line":4}}

But here's the response:

<== {"jsonrpc":"2.0", "result":{"type":"Breakpoint","fixedId":true,"id":"breakpoints\/1","breakpointNumber":1,"resolved":false,"location":{"type":"UnresolvedSourceLocation","scriptUri":"file:\/\/\/c:\/Users\/danny\/Desktop\/DartDebugSample\/lib\/src\/test.dart","line":4}},"id":"5"}
<== {"jsonrpc":"2.0", "error":{"code":102,"message":"Cannot add breakpoint","data":{"request":{"method":"addBreakpointWithScriptUri","params":{"isolateId":"isolates\/11111111","scriptUri":"package:yourpackage\/src\/test.dart","line":"4"}},"details":"addBreakpointWithScriptUri: Cannot add breakpoint at line '4'"}},"id":"6"}

Note that the second breakpoint (which is the one that breaks in 1.24) is the one being rejected.

So, I have no idea how to fix this - the breakpoint we can't set is the one that would actually work.

DartDebugSample.zip

@sivachandra
Copy link
Contributor

I will review the different scenarios and see if we can do better here. If yes, will follow up with a fix as well.

@sivachandra sivachandra changed the title Debugger does not pause at all breakpoints Unable to set breakpoints in parts of a library Jun 30, 2017
@sivachandra
Copy link
Contributor

I changed the title of this issue and have a change under review: https://codereview.chromium.org/2965673004/

sivachandra pushed a commit that referenced this issue Jul 11, 2017
This fixes the regression caused by
1eaa3dd and reported in #29988.

When picking a class to set a breakpoint, instead of rejecting classes
not defined in the script, we now reject classes which do not belong to
the library to which the script belongs.

[email protected]

Review-Url: https://codereview.chromium.org/2965673004 .
@sivachandra
Copy link
Contributor

@hermanbergwerf, @DanTup: The fix has landed. Could you test and see if the problem is fixed for you?

@DanTup
Copy link
Collaborator

DanTup commented Jul 11, 2017

By landed - do you mean it's in the latest dev release?

If so, I'll give it a test on Thursday when next working on Dart Code.

@sivachandra
Copy link
Contributor

sivachandra commented Jul 11, 2017

Ah, I thought you could use ToT. If not, you will have to wait for it to be picked up by a dev release.

@DanTup
Copy link
Collaborator

DanTup commented Jul 11, 2017

What's ToT?

I do hope to get set up so I can build the SDK stuff at some point; but it hasn't happened yet ;(

@sivachandra
Copy link
Contributor

ToT == Top of Trunk
Anyway, 1.25.0-dev.6.0 has my change now.

@DanTup
Copy link
Collaborator

DanTup commented Jul 13, 2017

Breakpoint in the sample I had seems to work correctly in dev.6.0 for me!

@sivachandra
Copy link
Contributor

Closing. If required, please reopen or file another bug as suitable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. vm-debugger
Projects
None yet
Development

No branches or pull requests

4 participants