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

Debugger: On stopping event, Code does display any source location if top stack frame does not have source information. #4027

Closed
chuckries opened this issue Mar 11, 2016 · 16 comments
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues verified Verification succeeded
Milestone

Comments

@chuckries
Copy link
Member

The issue: If the top stack frame does not have source information, the debugger fails to show a current location (even if another stack frame does have source information).

I came across this issue while using the new C# debugger for .NET Core: https://marketplace.visualstudio.com/items?itemName=ms-vscode.csharp

In C#, when stopping on an unhandled exception, the top stack frame is often in "non-user code" and the frame is either "[external code]" or a frame in OS code that the user will not have source info for. Example:
image

In this file, I found that when the debugger receives a stopping event, it is hard coded to set the 0th frame as the current frame:

this.setFocusedStackFrameAndEvaluate(callStack[0]);
this.openOrRevealEditor(callStack[0].source, callStack[0].lineNumber, false, false).done(null, errors.onUnexpectedError);

Prior to calling setFocusedStackFrameAndEvaluate or openOrRevealEditor, the debugService should walk it's callstack looking for a frame that has valid source information. The first frame it finds should be passed to setFocusedStackFrameAndEvaluate and openOrRevealEditor.

Additionally: I found that if a debugAdapter does not provide source information for a frame, VS Code creates a default Source object for the stack frame:

return new StackFrame(data.threadId, rsf.id, rsf.source ? new Source(rsf.source) : new Source({ name: 'unknown' }), rsf.name, rsf.line, rsf.column);

This issue is that a default Source objects sets source available to true:

    constructor(public raw: DebugProtocol.Source) {
        this.uri = raw.path ? uri.file(raw.path) : uri.parse(Source.INTERNAL_URI_PREFIX + raw.name);
        this.available = true;
    }

I believe that if a debugAdapter does not provide source for a frame, VS Code should not mark a stack frame as having source available.

@chuckries
Copy link
Member Author

@weinand

@weinand weinand added bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues labels Mar 11, 2016
@weinand weinand added this to the March 2016 milestone Mar 11, 2016
@weinand weinand assigned weinand and isidorn and unassigned isidorn and weinand Mar 11, 2016
@weinand weinand removed the bug Issue identified by VS Code Team member as probable bug label Mar 11, 2016
@weinand
Copy link
Contributor

weinand commented Mar 11, 2016

@chuckries If a stack frame does not have source information, the debugger UI cannot "infer" a location from some other frame by "walking its callstack looking for a frame that has valid source information". That would be too much guessing in a language agnostic UI.
So if you need this behaviour, just implement it in the debug adapter.

From your screenshot I do not really understand what is the problem because I only see the name of the frame?

@isidorn could you please look into the issue of "default Source objects sets source available to true"

@chuckries
Copy link
Member Author

The issue is that when the exception goes unhandled and the debugger stops, it does not set the current location.

In this image I have just stopped but I do not know where I am.
image

After selecting the ConsoleApplication.... frame it looks like this:
image

I believe that the location seen in the second screenshot should be the behavior when the debugger stops on the exception. The debug adapter should not control this sort of UI feature. It simply hands back a callstack. It is up to the UI to determine from those stack frames which should be set as the active frame. If there is no source info availalbe in frame[0], but there is source info available in frame[1], why would the debugger not set frame[1] as the active frame with the green arrow? The green arrow signifies that this is the current source location for the active frame, but it is not the current IP.

@chuckries chuckries changed the title Debugger: On stopping event, Code does not display current location if top stack frame does not have source information. Debugger: On stopping event, Code does not any location if top stack frame does not have source information. Mar 11, 2016
@chuckries chuckries changed the title Debugger: On stopping event, Code does not any location if top stack frame does not have source information. Debugger: On stopping event, Code does display any source location if top stack frame does not have source information. Mar 11, 2016
@weinand
Copy link
Contributor

weinand commented Mar 11, 2016

I still cannot see the relevant information in your stackframes.
Could you try to make the Call Stack viewlet wider so that we can see the right hand side of it.
Like this:

picture

“The debug adapter should not control this sort of UI feature.”

This is not really true.
The debug adapter is an adapter between a debugger API and the VS Code debugger UI.
The VS Code debug protocol is a protocol for serving the VS Code debugger UI (and nothing else). It is not a protocol for serving any other tool.
The VS Code debugger UI is debugger and language agnostic, that means that the debug adapter has to control how types, values and the structure of variables is presented in the UI. The same is true for other areas of the protocol.

@weinand
Copy link
Contributor

weinand commented Mar 11, 2016

@chuckries after having had dinner, I think I understand now what the problem is. You are not talking about the CALL STACK viewlet (as your first screenshot suggested), but you are referring to the stack frame indication in the editor. You are missing the yellow (or green) line where the exception occurs. Yes, that's a bug.
I thought you were missing the source and line information from the stack frames in the CALL STACK viewlet.

@chuckries
Copy link
Member Author

I apologize for not being clear. It seems like you understand my issue now. Thanks!

I simply dug into the code and tried to figure out why the stack frame indication was not being set correctly, and it is from the hard coding of 0 in

this.setFocusedStackFrameAndEvaluate(callStack[0]);
this.openOrRevealEditor(callStack[0].source, callStack[0].lineNumber, false, false).done(null, errors.onUnexpectedError);

This is where the "call stack walk" must be done. The frame object simply need to be iterated over to find one that has source. I believe this should set the stack frame indication correctly.

@isidorn isidorn assigned weinand and unassigned isidorn Mar 16, 2016
@isidorn
Copy link
Contributor

isidorn commented Mar 16, 2016

@chuckries nice catch and a good investigation - thanks! Also when you figure out everything on your own like now feel free to submit a PR.

@chuckries
Copy link
Member Author

Excellent! Has the behavior changes regarding a debug adapter providing a stack frame with a null Source field?

@isidorn
Copy link
Contributor

isidorn commented Mar 16, 2016

Yes, if the top stack frame has a null Source field we will focus the first stack frame with a proper Source field.

@chuckries
Copy link
Member Author

I understand that, but I discovered a secondary issue while investigating this that StackFrame's are never created with null Source fields. Here you can see that if a null Source is provided by the debug adapter, the StackFrame will be created with a default Source object that has its name field set to uknown. I believe this was done so that the callstack viewlet willl display the string "unkown" when a stackframe does not have have a source field, but this will interfere with setting the current stack frame.

@chuckries
Copy link
Member Author

I can confirm that even with this change I am not seeing the current stack frame get set correctly because my debug adapter returns a null Source for the stackframe and code is creating non null default source object. I am having trouble debugging code with code however using current master.

@chuckries
Copy link
Member Author

@isidorn Can this be reopened or should I file another issue?

@weinand weinand reopened this Mar 16, 2016
@isidorn
Copy link
Contributor

isidorn commented Mar 16, 2016

@chuckries good catch, I will fix this tomorrow, unless it is urgent then let me know and I can look right away

@chuckries
Copy link
Member Author

any time before next release is fine, but tomorrow is great! thanks!

@isidorn isidorn self-assigned this Mar 17, 2016
@isidorn
Copy link
Contributor

isidorn commented Mar 17, 2016

Giving to @weinand to verify, also @chuckries let us know if it still does not work for you

@isidorn isidorn assigned weinand and unassigned isidorn Mar 17, 2016
@chuckries
Copy link
Member Author

Looks great! Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

3 participants