Skip to content
This repository was archived by the owner on Jul 15, 2023. It is now read-only.

Add stacktrace dump and better error messages on EXC_BAD_ACCESS panics #2904

Merged
ramya-rao-a merged 6 commits into from
Mar 20, 2020
Merged

Add stacktrace dump and better error messages on EXC_BAD_ACCESS panics #2904

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c249f49
Merge remote-tracking branch 'upstream/master'
lggomez Apr 21, 2019
7501734
Merge remote-tracking branch 'origin/master'
lggomez May 3, 2019
fe2cb47
Merge remote-tracking branch 'upstream/master'
lggomez Aug 26, 2019
9359e1c
Merge branch 'master' of https://github.com/Microsoft/vscode-go
lggomez Nov 11, 2019
295cb87
Merge branch 'master' of ssh://github.com/microsoft/vscode-go
lggomezml Feb 17, 2020
24e363e
Add stacktrace dump and better error messages on EXC_BAD_ACCESS panics
lggomez Feb 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 100 additions & 19 deletions src/debugAdapter/goDebug.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ interface DebugThread {
id: number;
line: number;
pc: number;
goroutineID: number;
function?: DebugFunction;
}

Expand All @@ -153,6 +154,7 @@ interface DebugFunction {
goType: number;
args: DebugVariable[];
locals: DebugVariable[];
optimized: boolean;
}

interface ListVarsOut {
Expand Down Expand Up @@ -693,7 +695,7 @@ class Delve {
await this.callPromise('Detach', [this.isApiV1 ? true : { Kill: isLocalDebugging }]);
} catch (err) {
log('DetachResponse');
logError(`Failed to detach - ${err.toString() || ''}`);
logError(err, 'Failed to detach');
shouldForceClean = isLocalDebugging;
}
}
Expand Down Expand Up @@ -872,7 +874,7 @@ class GoDebugSession extends LoggingDebugSession {
// We use NonBlocking so the call would return immediately.
this.debugState = await this.delve.getDebugState();
} catch (error) {
logError(`Failed to get state ${String(error)}`);
this.logDelveError(error, 'Failed to get state');
}

if (!this.debugState.Running && !this.continueRequestRunning) {
Expand All @@ -883,15 +885,13 @@ class GoDebugSession extends LoggingDebugSession {
() => {
return this.setBreakPoints(response, args).then(() => {
return this.continue(true).then(null, (err) => {
logError(
`Failed to continue delve after halting it to set breakpoints: "${err.toString()}"`
);
this.logDelveError(err, 'Failed to continue delve after halting it to set breakpoints');
});
});
},
(err) => {
this.skipStopEventOnce = false;
logError(err);
this.logDelveError(err, 'Failed to halt delve before attempting to set breakpoint');
return this.sendErrorResponse(
response,
2008,
Expand Down Expand Up @@ -919,7 +919,7 @@ class GoDebugSession extends LoggingDebugSession {
}

if (err) {
logError('Failed to get threads - ' + err.toString());
this.logDelveError(err, 'Failed to get threads');
return this.sendErrorResponse(response, 2003, 'Unable to display threads: "{e}"', {
e: err.toString()
});
Expand Down Expand Up @@ -961,7 +961,7 @@ class GoDebugSession extends LoggingDebugSession {
[stackTraceIn],
(err, out) => {
if (err) {
logError('Failed to produce stack trace!');
this.logDelveError(err, 'Failed to produce stacktrace');
return this.sendErrorResponse(response, 2004, 'Unable to produce stack trace: "{e}"', {
e: err.toString()
});
Expand Down Expand Up @@ -1002,7 +1002,7 @@ class GoDebugSession extends LoggingDebugSession {
this.delve.isApiV1 ? [listLocalVarsIn] : [{ scope: listLocalVarsIn, cfg: this.delve.loadConfig }],
(err, out) => {
if (err) {
logError('Failed to list local variables - ' + err.toString());
this.logDelveError(err, 'Failed to get list local variables');
return this.sendErrorResponse(response, 2005, 'Unable to list locals: "{e}"', {
e: err.toString()
});
Expand All @@ -1018,7 +1018,7 @@ class GoDebugSession extends LoggingDebugSession {
: [{ scope: listLocalFunctionArgsIn, cfg: this.delve.loadConfig }],
(listFunctionErr, outArgs) => {
if (listFunctionErr) {
logError('Failed to list function args - ' + listFunctionErr.toString());
this.logDelveError(listFunctionErr, 'Failed to list function args');
return this.sendErrorResponse(response, 2006, 'Unable to list args: "{e}"', {
e: listFunctionErr.toString()
});
Expand Down Expand Up @@ -1098,7 +1098,7 @@ class GoDebugSession extends LoggingDebugSession {
this.delve.isApiV1 ? [filter] : [{ filter, cfg: this.delve.loadConfig }],
(listPkgVarsErr, listPkgVarsOut) => {
if (listPkgVarsErr) {
logError('Failed to list global vars - ' + listPkgVarsErr.toString());
this.logDelveError(listPkgVarsErr, 'Failed to list global vars');
return this.sendErrorResponse(
response,
2007,
Expand Down Expand Up @@ -1170,7 +1170,7 @@ class GoDebugSession extends LoggingDebugSession {
const variable = this.delve.isApiV1 ? <DebugVariable>result : (<EvalOut>result).Variable;
v.children = variable.children;
},
(err) => logError('Failed to evaluate expression - ' + err.toString())
(err) => this.logDelveError(err, 'Failed to evaluate expression')
);
}
};
Expand Down Expand Up @@ -1249,7 +1249,7 @@ class GoDebugSession extends LoggingDebugSession {
log('NextRequest');
this.delve.call<DebuggerState | CommandOut>('Command', [{ name: 'next' }], (err, out) => {
if (err) {
logError('Failed to next - ' + err.toString());
this.logDelveError(err, 'Failed to next');
}
const state = this.delve.isApiV1 ? <DebuggerState>out : (<CommandOut>out).State;
log('next state', state);
Expand All @@ -1264,7 +1264,7 @@ class GoDebugSession extends LoggingDebugSession {
log('StepInRequest');
this.delve.call<DebuggerState | CommandOut>('Command', [{ name: 'step' }], (err, out) => {
if (err) {
logError('Failed to step - ' + err.toString());
this.logDelveError(err, 'Failed to step in');
}
const state = this.delve.isApiV1 ? <DebuggerState>out : (<CommandOut>out).State;
log('stop state', state);
Expand All @@ -1279,7 +1279,7 @@ class GoDebugSession extends LoggingDebugSession {
log('StepOutRequest');
this.delve.call<DebuggerState | CommandOut>('Command', [{ name: 'stepOut' }], (err, out) => {
if (err) {
logError('Failed to stepout - ' + err.toString());
this.logDelveError(err, 'Failed to step out');
}
const state = this.delve.isApiV1 ? <DebuggerState>out : (<CommandOut>out).State;
log('stepout state', state);
Expand All @@ -1294,7 +1294,7 @@ class GoDebugSession extends LoggingDebugSession {
log('PauseRequest');
this.delve.call<DebuggerState | CommandOut>('Command', [{ name: 'halt' }], (err, out) => {
if (err) {
logError('Failed to halt - ' + err.toString());
this.logDelveError(err, 'Failed to halt');
return this.sendErrorResponse(response, 2010, 'Unable to halt execution: "{e}"', {
e: err.toString()
});
Expand Down Expand Up @@ -1343,7 +1343,7 @@ class GoDebugSession extends LoggingDebugSession {
this.delve.call(this.delve.isApiV1 ? 'SetSymbol' : 'Set', [setSymbolArgs], (err) => {
if (err) {
const errMessage = `Failed to set variable: ${err.toString()}`;
logError(errMessage);
this.logDelveError(err, 'Failed to set variable');
return this.sendErrorResponse(response, 2010, errMessage);
}
response.body = { value: args.value };
Expand Down Expand Up @@ -1741,7 +1741,7 @@ class GoDebugSession extends LoggingDebugSession {
// [TODO] Can we avoid doing this? https://github.com/Microsoft/vscode/issues/40#issuecomment-161999881
this.delve.call<DebugGoroutine[] | ListGoroutinesOut>('ListGoroutines', [], (err, out) => {
if (err) {
logError('Failed to get threads - ' + err.toString());
this.logDelveError(err, 'Failed to get threads');
}
const goroutines = this.delve.isApiV1 ? <DebugGoroutine[]>out : (<ListGoroutinesOut>out).Goroutines;
this.updateGoroutinesList(goroutines);
Expand Down Expand Up @@ -1781,7 +1781,7 @@ class GoDebugSession extends LoggingDebugSession {
if (!calledWhenSettingBreakpoint) {
errorCallback = (err: any) => {
if (err) {
logError('Failed to continue - ' + err.toString());
this.logDelveError(err, 'Failed to continue');
}
this.handleReenterDebug('breakpoint');
throw err;
Expand Down Expand Up @@ -1838,6 +1838,87 @@ class GoDebugSession extends LoggingDebugSession {
});
});
}

private logDelveError(err: any, message: string) {
if (err === undefined) {
return;
}

let errorMessage = err.toString();
// Handle unpropagated fatalpanic errors with a more user friendly message:
// https://github.com/microsoft/vscode-go/issues/1903#issuecomment-460126884
// https://github.com/go-delve/delve/issues/852
// This affects macOS only although we're agnostic of the OS at this stage, only handle the error
if (errorMessage === 'bad access') {
errorMessage = 'unpropagated fatalpanic: signal SIGSEGV (EXC_BAD_ACCESS). This fatalpanic is not traceable on macOS, see https://github.com/go-delve/delve/issues/852';
}

logError(message + ' - ' + errorMessage);

if (errorMessage === 'bad access') {
logError('WARNING: this stack might not be from the expected active goroutine');
}

this.dumpStacktrace();
}

private async dumpStacktrace() {
// Get current goroutine
// Debugger may be stopped at this point but we still can (and need) to obtain state and stacktrace
let goroutineId = 0;
try {
const stateCallResult = await this.delve.getDebugState();
// In some fault scenarios there may not be a currentGoroutine available from the debugger state
// Use the current thread
if (!stateCallResult.currentGoroutine) {
goroutineId = stateCallResult.currentThread.goroutineID;
} else {
goroutineId = stateCallResult.currentGoroutine.id;
}
} catch (error) {
logError('dumpStacktrace - Failed to get debugger state ' + error);
}

// Get goroutine stacktrace
const stackTraceIn = { id: goroutineId, depth: this.delve.stackTraceDepth };
Copy link
Copy Markdown
Contributor

@ramya-rao-a ramya-rao-a Dec 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goroutineId here will always be 0 as this code path is not inside the callback provided to the delve call a few lines above.

Copy link
Copy Markdown
Contributor

@ramya-rao-a ramya-rao-a Dec 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can try

try {
  const stateCallResult: DebuggerState | CommandOut = await this.delve.callPromise('State', [{ NonBlocking: true }]);
  goroutineId = this.delve.isApiV1 ? (<CommandOut>stateCallResult).State.currentGoroutine.id : (<DebuggerState>stateCallResult).currentGoroutine.id;
} catch (error) {
	// log error?
}

Copy link
Copy Markdown
Contributor Author

@lggomez lggomez Dec 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jhendrixMSFT @ramya-rao-a I think I solved the issue of state returning always (?) zero. The gist of the debugger state is that there seems to be no currentGoroutine in this scenario so we have to default to the currentThread goroutineId:

This is the apicall response:

{
	"Running": false,
	"currentThread": {
		"id": 5142829,
		"pc": 17138024,
		"file": "/Users/lgomez/DEV/go/test_1903/main.go",
		"line": 5,
		"function": {
			"name": "main.oops",
			"value": 17138000,
			"type": 0,
			"goType": 0,
			"optimized": false
		},
		"goroutineID": 1,
		"ReturnValues": null
	},
	"Threads": [
		{
			"id": 5142829,
			"pc": 17138024,
			"file": "/Users/lgomez/DEV/go/test_1903/main.go",
			"line": 5,
			"function": {
				"name": "main.oops",
				"value": 17138000,
				"type": 0,
				"goType": 0,
				"optimized": false
			},
			"goroutineID": 1,
			"ReturnValues": null
		},
		{
			"id": 5142873,
			"pc": 140734907268914,
			"file": "",
			"line": 0,
			"goroutineID": 0,
			"ReturnValues": null
		},
		{
			"id": 5142874,
			"pc": 140734907267178,
			"file": "",
			"line": 0,
			"goroutineID": 0,
			"ReturnValues": null
		},
		{
			"id": 5142875,
			"pc": 140734907267178,
			"file": "",
			"line": 0,
			"goroutineID": 0,
			"ReturnValues": null
		},
		{
			"id": 5142876,
			"pc": 140734907267178,
			"file": "",
			"line": 0,
			"goroutineID": 0,
			"ReturnValues": null
		}
	],
	"NextInProgress": false,
	"exited": false,
	"exitStatus": 0,
	"When": ""
}

And this is the handled panic with the updated behavior:
Screen Shot 2019-12-19 at 17 59 35

This is on mac, I´ll try this at home later to see if the response is different on linux

Copy link
Copy Markdown
Contributor Author

@lggomez lggomez Dec 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not getting the currentGoroutine on linux either, i'm trying other goroutine examples and i'm even reproducing #2563 while at it so I'm willing to say this is our best bet for now. We're supposed to be getting this field but we can follow up on that issue since it is better suited for its root cause

if (!this.delve.isApiV1) {
Object.assign(stackTraceIn, { full: false, cfg: this.delve.loadConfig });
}
this.delve.call<DebugLocation[] | StacktraceOut>(
this.delve.isApiV1 ?
'StacktraceGoroutine' : 'Stacktrace', [stackTraceIn], (err, out) => {
if (err) {
logError('dumpStacktrace: Failed to produce stack trace' + err);
return;
}
const locations = this.delve.isApiV1 ? <DebugLocation[]>out : (<StacktraceOut>out).Locations;
log('locations', locations);
const stackFrames = locations.map((location, frameId) => {
const uniqueStackFrameId = this.stackFrameHandles.create([goroutineId, frameId]);
return new StackFrame(
uniqueStackFrameId,
location.function ? location.function.name : '<unknown>',
location.file === '<autogenerated>' ? null : new Source(
path.basename(location.file),
this.toLocalPath(location.file)
),
location.line,
0
);
});

// Dump stacktrace into error logger
logError(`Last known immediate stacktrace (goroutine id ${goroutineId}):`);
let output = '';
stackFrames.forEach((stackFrame) => {
output = output.concat(`\t${stackFrame.source.path}:${stackFrame.line}\n`);
if (stackFrame.name) {
output = output.concat(`\t\t${stackFrame.name}\n`);
}
});
logError(output);
});
}
}

function random(low: number, high: number): number {
Expand Down