From 77e88ca8ee73b91744b1208ac7cb37a82568ba59 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Tue, 26 Sep 2023 16:09:39 -0700 Subject: [PATCH] fix: delegate wasm evaluation with dwarf Fixes https://github.com/microsoft/vscode/issues/194232 --- src/adapter/dwarf/wasmSymbolProvider.ts | 41 ++++++ src/adapter/stackTrace.ts | 15 ++- src/adapter/threads.ts | 126 +++++++++++++----- src/test/wasm/wasm.test.ts | 16 +++ ...webassembly-dwarf-does-lldb-evaluation.txt | 9 ++ 5 files changed, 165 insertions(+), 42 deletions(-) create mode 100644 src/test/wasm/webassembly-dwarf-does-lldb-evaluation.txt diff --git a/src/adapter/dwarf/wasmSymbolProvider.ts b/src/adapter/dwarf/wasmSymbolProvider.ts index 638c7552f..f7f348da4 100644 --- a/src/adapter/dwarf/wasmSymbolProvider.ts +++ b/src/adapter/dwarf/wasmSymbolProvider.ts @@ -14,6 +14,8 @@ import { ILogger, LogTag } from '../../common/logging'; import { flatten, once } from '../../common/objUtils'; import { Base0Position, IPosition, Range } from '../../common/positions'; import { AnyLaunchConfiguration } from '../../configuration'; +import * as errors from '../../dap/errors'; +import { ProtocolError } from '../../dap/protocolError'; import { StepDirection } from '../pause'; import { getSourceSuffix } from '../templates'; import { IDwarfModuleProvider } from './dwarfModuleProvider'; @@ -226,6 +228,18 @@ export interface IWasmSymbols extends IDisposable { */ getFunctionStack?(position: IPosition): Promise<{ name: string }[]>; + /** + * Evaluates the expression at a position. + * + * Following CDP semantics, it assumes the column is being the byte offset + * in webassembly. However, we encode the inline frame index in the line. + */ + evaluate?( + callFrameId: string, + position: IPosition, + expression: string, + ): Promise; + /** * Gets ranges that should be stepped for the given step kind and location. * @@ -549,6 +563,33 @@ class WasmSymbols extends DecompiledWasmSymbols { ); } + /** @inheritdoc */ + public async evaluate( + callFrameId: string, + position: IPosition, + expression: string, + ): Promise { + try { + const r = await this.rpc.sendMessage( + 'evaluate', + expression, + { + codeOffset: position.base0.columnNumber - this.codeOffset, + inlineFrameIndex: position.base0.lineNumber, + rawModuleId: this.moduleId, + }, + callFrameId, + ); + + // cast since types in dwarf-debugging are slightly different than generated cdp API + return (r as Cdp.Runtime.RemoteObject) ?? undefined; + } catch (e) { + // errors are expected here if the user tries to evaluate expressions + // the simple lldb-eval can't handle. + throw new ProtocolError(errors.createSilentError(e.message)); + } + } + private getMappedLines(sourceURL: string) { const prev = this.mappedLines.get(sourceURL); if (prev) { diff --git a/src/adapter/stackTrace.ts b/src/adapter/stackTrace.ts index fcefdcbdf..4e083aca9 100644 --- a/src/adapter/stackTrace.ts +++ b/src/adapter/stackTrace.ts @@ -694,9 +694,11 @@ export class InlinedFrame implements IStackFrameElement { /** @inheritdoc */ public readonly uiLocation: () => Promise; + public readonly inlineFrameIndex: number; + + private readonly wasmPosition: Base0Position; private readonly name: string; private readonly thread: Thread; - private readonly inlineFrameIndex: number; private readonly source: ISourceWithMap; constructor(opts: { @@ -715,6 +717,10 @@ export class InlinedFrame implements IStackFrameElement { this.thread = opts.thread; this.source = opts.source; this.inlineFrameIndex = opts.inlineFrameIndex; + this.wasmPosition = new Base0Position( + this.inlineFrameIndex, + this.root.rawPosition.base0.columnNumber, + ); this.uiLocation = once(() => opts.thread._sourceContainer.preferredUiLocation({ columnNumber: opts.root.rawPosition.base1.columnNumber, @@ -760,7 +766,7 @@ export class InlinedFrame implements IStackFrameElement { if (uiLocation) { return sm.getStepSkipList( direction, - this.root.rawPosition, + this.wasmPosition, (uiLocation.source as SourceFromMap).compiledToSourceUrl.get(this.source), new Base1Position(uiLocation.lineNumber, uiLocation.columnNumber), ); @@ -777,10 +783,7 @@ export class InlinedFrame implements IStackFrameElement { return EMPTY_SCOPES; } - const variables = await v.getVariablesInScope?.( - callFrameId, - new Base0Position(this.inlineFrameIndex, this.root.rawPosition.base0.columnNumber), - ); + const variables = await v.getVariablesInScope?.(callFrameId, this.wasmPosition); if (!variables) { return EMPTY_SCOPES; } diff --git a/src/adapter/threads.ts b/src/adapter/threads.ts index 42e934864..08d0f2fdd 100644 --- a/src/adapter/threads.ts +++ b/src/adapter/threads.ts @@ -10,7 +10,7 @@ import { EventEmitter } from '../common/events'; import { HrTime } from '../common/hrnow'; import { ILogger, LogTag } from '../common/logging'; import { isInstanceOf, truthy } from '../common/objUtils'; -import { Base1Position, Range } from '../common/positions'; +import { Base0Position, Base1Position, Range } from '../common/positions'; import { IDeferred, delay, getDeferred } from '../common/promiseUtil'; import { IRenameProvider } from '../common/sourceMaps/renameProvider'; import * as sourceUtils from '../common/sourceUtils'; @@ -36,9 +36,9 @@ import * as objectPreview from './objectPreview'; import { PreviewContextType, getContextForType } from './objectPreview/contexts'; import { ExpectedPauseReason, IPausedDetails, StepDirection } from './pause'; import { SmartStepper } from './smartStepping'; -import { ISourceWithMap, IUiLocation, Source, base1To0 } from './source'; +import { ISourceWithMap, IUiLocation, Source, base1To0, isSourceWithWasm } from './source'; import { IPreferredUiLocation, SourceContainer } from './sourceContainer'; -import { StackFrame, StackTrace, isStackFrameElement } from './stackTrace'; +import { InlinedFrame, StackFrame, StackTrace, isStackFrameElement } from './stackTrace'; import { serializeForClipboard, serializeForClipboardTmpl, @@ -462,36 +462,53 @@ export class Thread implements IVariableStoreLocationProvider { .map(label => ({ label, start: 0, length: params.text.length })); } - async evaluate(args: Dap.EvaluateParamsExtended): Promise { - let callFrameId: Cdp.Debugger.CallFrameId | undefined; - let stackFrame: StackFrame | undefined; - if (args.frameId !== undefined) { - stackFrame = this._pausedDetails - ? this._pausedDetails.stackTrace.frame(args.frameId)?.root - : undefined; - if (!stackFrame) { - throw new ProtocolError(this._stackFrameNotFoundError()); - } + /** + * Evaluates the expression on the stackframe if it's a WASM stackframe with + * evaluation information. Returns undefined otherwise. + */ + private async _evaluteWasm( + stackFrame: StackFrame | InlinedFrame | undefined, + args: Dap.EvaluateParamsExtended, + variables: VariableStore, + ): Promise { + if (!stackFrame) { + return; + } - callFrameId = stackFrame.callFrameId(); - if (!callFrameId) { - throw new ProtocolError(this._evaluateOnAsyncFrameError()); - } + const root = stackFrame.root; + const cfId = root.callFrameId(); + const source = await root.scriptSource?.source; + if (!isSourceWithWasm(source) || !cfId) { + return; } - if (args.context === 'repl' && args.expression.startsWith('cd ')) { - const contextName = args.expression.substring('cd '.length).trim(); - for (const ec of this._executionContexts.values()) { - if (this.target.executionContextName(ec.description) === contextName) { - this._selectedContext = ec; - return { - result: `[${contextName}]`, - variablesReference: 0, - }; - } - } + const symbols = await source.sourceMap.value.promise; + if (!symbols.evaluate) { + return; + } + + const position = new Base0Position( + stackFrame instanceof InlinedFrame ? stackFrame.inlineFrameIndex : 0, + root.rawPosition.base0.columnNumber, + ); + + const result = await symbols.evaluate(cfId, position, args.expression); + if (result) { + return await variables + .createFloatingVariable(args.expression, result) + .toDap(args.context as PreviewContextType, args.format); } + } + /** + * Evaluates the expression on the stackframe. + */ + private async _evaluteJs( + stackFrame: StackFrame | InlinedFrame | undefined, + args: Dap.EvaluateParamsExtended, + variables: VariableStore, + ): Promise { + const callFrameId = stackFrame?.root.callFrameId(); // For clipboard evaluations, return a safe JSON-stringified string. const params: Cdp.Runtime.EvaluateParams = args.context === 'clipboard' @@ -534,7 +551,7 @@ export class Thread implements IVariableStoreLocationProvider { ...params, contextId: this._selectedContext ? this._selectedContext.description.id : undefined, }, - { isInternalScript: false, stackFrame }, + { isInternalScript: false, stackFrame: stackFrame?.root }, ); // Report result for repl immediately so that the user could see the expression they entered. @@ -543,6 +560,7 @@ export class Thread implements IVariableStoreLocationProvider { } const response = await responsePromise; + if (!response) throw new ProtocolError(errors.createSilentError(l10n.t('Unable to evaluate'))); if (response.exceptionDetails) { let text = response.exceptionDetails.exception @@ -552,17 +570,53 @@ export class Thread implements IVariableStoreLocationProvider { throw new ProtocolError(errors.createSilentError(text)); } + return variables + .createFloatingVariable(params.expression, response.result) + .toDap(args.context as PreviewContextType, args.format); + } + + public async evaluate(args: Dap.EvaluateParamsExtended): Promise { + let callFrameId: Cdp.Debugger.CallFrameId | undefined; + let stackFrame: StackFrame | InlinedFrame | undefined; + if (args.frameId !== undefined) { + stackFrame = this._pausedDetails + ? this._pausedDetails.stackTrace.frame(args.frameId) + : undefined; + + if (!stackFrame) { + throw new ProtocolError(this._stackFrameNotFoundError()); + } + + callFrameId = stackFrame.root.callFrameId(); + if (!callFrameId) { + throw new ProtocolError(this._evaluateOnAsyncFrameError()); + } + } + + if (args.context === 'repl' && args.expression.startsWith('cd ')) { + const contextName = args.expression.substring('cd '.length).trim(); + for (const ec of this._executionContexts.values()) { + if (this.target.executionContextName(ec.description) === contextName) { + this._selectedContext = ec; + return { + result: `[${contextName}]`, + variablesReference: 0, + }; + } + } + } + const variableStore = callFrameId ? this._pausedVariables : this.replVariables; if (!variableStore) { throw new ProtocolError(errors.createSilentError(l10n.t('Unable to evaluate'))); } - const variable = await variableStore - .createFloatingVariable(params.expression, response.result) - .toDap(args.context as PreviewContextType, args.format); + const variable = + (await this._evaluteWasm(stackFrame, args, variableStore)) ?? + (await this._evaluteJs(stackFrame, args, variableStore)); return { - type: response.result.type, + type: variable.type, result: variable.value, variablesReference: variable.variablesReference, namedVariables: variable.namedVariables, @@ -588,9 +642,9 @@ export class Thread implements IVariableStoreLocationProvider { | Promise | Promise, format: Dap.ValueFormat | undefined, - ): Promise { + ): Promise { const response = await responsePromise; - if (!response) return { result: '', variablesReference: 0 }; + if (!response) return { name: originalCall.expression, value: '', variablesReference: 0 }; if (response.exceptionDetails) { const formattedException = await new ExceptionMessage(response.exceptionDetails).toDap(this); @@ -623,7 +677,7 @@ export class Thread implements IVariableStoreLocationProvider { ); } - return { ...resultVar, result: `${contextName}${resultVar.value}` }; + return { ...resultVar, value: `${contextName}${resultVar.value}` }; } private _initialize() { diff --git a/src/test/wasm/wasm.test.ts b/src/test/wasm/wasm.test.ts index a84e4771f..32229dc87 100644 --- a/src/test/wasm/wasm.test.ts +++ b/src/test/wasm/wasm.test.ts @@ -195,5 +195,21 @@ describe('webassembly', () => { r.assertLog(); }); + + itIntegrates('does lldb evaluation', async ({ r, context }) => { + const p = await prepare(r, context, 'fibonacci', { + source: { path: 'web/dwarf/fibonacci.c' }, + breakpoints: [{ line: 6 }], + }); + + const { threadId } = p.log(await p.dap.once('stopped')); + const { id: frameId } = (await p.dap.stackTrace({ threadId })).stackFrames[0]; + + await p.logger.evaluateAndLog(`n`, { params: { frameId } }); + await p.logger.evaluateAndLog(`a`, { params: { frameId } }); + await p.logger.evaluateAndLog(`a + n * 2`, { params: { frameId } }); + + r.assertLog(); + }); }); }); diff --git a/src/test/wasm/webassembly-dwarf-does-lldb-evaluation.txt b/src/test/wasm/webassembly-dwarf-does-lldb-evaluation.txt new file mode 100644 index 000000000..8dc6bf1b8 --- /dev/null +++ b/src/test/wasm/webassembly-dwarf-does-lldb-evaluation.txt @@ -0,0 +1,9 @@ +{ + allThreadsStopped : false + description : Paused on breakpoint + reason : breakpoint + threadId : +} +result: 9 +result: 0 +result: 18