From 2cf86cb5c3d387407c7de4fc211794b041d2325e Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Tue, 19 Sep 2023 16:41:08 -0700 Subject: [PATCH 1/2] implement inlined method handling and improve stepping --- package-lock.json | 12 +- package.json | 2 +- src/adapter/dwarf/wasmSymbolProvider.ts | 131 ++++++++-- src/adapter/pause.ts | 41 +++ src/adapter/smartStepping.ts | 2 +- src/adapter/sourceContainer.ts | 15 +- src/adapter/stackTrace.ts | 319 ++++++++++++++++++++---- src/adapter/threads.ts | 80 +++--- src/adapter/variableStore.ts | 70 +++--- src/common/positions.test.ts | 63 +++++ src/common/positions.ts | 42 +++- 11 files changed, 616 insertions(+), 161 deletions(-) create mode 100644 src/adapter/pause.ts create mode 100644 src/common/positions.test.ts diff --git a/package-lock.json b/package-lock.json index 2a1e74d49..13d2cfc86 100644 --- a/package-lock.json +++ b/package-lock.json @@ -78,7 +78,7 @@ "@types/ws": "^8.5.3", "@typescript-eslint/eslint-plugin": "^5.17.0", "@typescript-eslint/parser": "^5.17.0", - "@vscode/dwarf-debugging": "file:../vscode-dwarf-debugging/vscode-dwarf-debugging-0.0.2.tgz", + "@vscode/dwarf-debugging": "^0.0.2", "@vscode/test-electron": "^2.2.3", "chai": "^4.3.6", "chai-as-promised": "^7.1.1", @@ -2283,10 +2283,9 @@ }, "node_modules/@vscode/dwarf-debugging": { "version": "0.0.2", - "resolved": "file:../vscode-dwarf-debugging/vscode-dwarf-debugging-0.0.2.tgz", - "integrity": "sha512-eOipqtiz/bpQsh5Pf4wSXxAwX1w7gkoXMBk5Sivpv4bfLBSu0QdVUgf4G/wx+tuh60tl0V0sY/ByuHe5DAFnwA==", + "resolved": "https://registry.npmjs.org/@vscode/dwarf-debugging/-/dwarf-debugging-0.0.2.tgz", + "integrity": "sha512-u/sQV5SBYOzAFE9Wy0N9oH+FbpZ/KJCl9ESv+3I6G7IAQXvmzFOdkA+BCTFLgZl89viT28SoHmZk4ZPwjQhIkA==", "dev": true, - "license": "MIT", "dependencies": { "ws": "^8.14.1" } @@ -16656,8 +16655,9 @@ } }, "@vscode/dwarf-debugging": { - "version": "file:..\\vscode-dwarf-debugging\\vscode-dwarf-debugging-0.0.2.tgz", - "integrity": "sha512-eOipqtiz/bpQsh5Pf4wSXxAwX1w7gkoXMBk5Sivpv4bfLBSu0QdVUgf4G/wx+tuh60tl0V0sY/ByuHe5DAFnwA==", + "version": "0.0.2", + "resolved": "https://registry.npmjs.org/@vscode/dwarf-debugging/-/dwarf-debugging-0.0.2.tgz", + "integrity": "sha512-u/sQV5SBYOzAFE9Wy0N9oH+FbpZ/KJCl9ESv+3I6G7IAQXvmzFOdkA+BCTFLgZl89viT28SoHmZk4ZPwjQhIkA==", "dev": true, "requires": { "ws": "^8.14.1" diff --git a/package.json b/package.json index 437f92bea..22a0b516f 100644 --- a/package.json +++ b/package.json @@ -125,7 +125,7 @@ "@types/ws": "^8.5.3", "@typescript-eslint/eslint-plugin": "^5.17.0", "@typescript-eslint/parser": "^5.17.0", - "@vscode/dwarf-debugging": "file:../vscode-dwarf-debugging/vscode-dwarf-debugging-0.0.2.tgz", + "@vscode/dwarf-debugging": "^0.0.2", "@vscode/test-electron": "^2.2.3", "chai": "^4.3.6", "chai-as-promised": "^7.1.1", diff --git a/src/adapter/dwarf/wasmSymbolProvider.ts b/src/adapter/dwarf/wasmSymbolProvider.ts index 3fb900ecf..ca2bbfbde 100644 --- a/src/adapter/dwarf/wasmSymbolProvider.ts +++ b/src/adapter/dwarf/wasmSymbolProvider.ts @@ -3,6 +3,7 @@ *--------------------------------------------------------*/ import type { IWasmWorker, MethodReturn } from '@vscode/dwarf-debugging'; +import { Chrome } from '@vscode/dwarf-debugging/chrome-cxx/mnt/extension-api'; import { randomUUID } from 'crypto'; import { inject, injectable } from 'inversify'; import Cdp from '../../cdp/api'; @@ -10,8 +11,9 @@ import { ICdpApi } from '../../cdp/connection'; import { binarySearch } from '../../common/arrayUtils'; import { IDisposable } from '../../common/disposable'; import { ILogger, LogTag } from '../../common/logging'; -import { once } from '../../common/objUtils'; -import { Base0Position, IPosition } from '../../common/positions'; +import { flatten, once } from '../../common/objUtils'; +import { Base0Position, IPosition, Range } from '../../common/positions'; +import { StepDirection } from '../pause'; import { getSourceSuffix } from '../templates'; import { IDwarfModuleProvider } from './dwarfModuleProvider'; @@ -169,8 +171,8 @@ export interface IWasmSymbols extends IDisposable { /** * Gets the source position for the given position in compiled code. * - * Following CDP semantics, it returns a position on line 0 with the column - * offset being the byte offset in webassembly. + * Following CDP semantics, it assumes the column is being the byte offset + * in webassembly. However, we encode the inline frame index in the line. */ originalPositionFor( compiledPosition: IPosition, @@ -182,16 +184,40 @@ export interface IWasmSymbols extends IDisposable { * Following CDP semantics, it assumes the position is line 0 with the column * offset being the byte offset in webassembly. */ - compiledPositionFor(sourceUrl: string, sourcePosition: IPosition): Promise; + compiledPositionFor(sourceUrl: string, sourcePosition: IPosition): Promise; /** * Gets variables in the program scope at the given position. If not * implemented, the variable store should use its default behavior. * - * Following CDP semantics, it assumes the position is line 0 with the column - * offset being the byte offset in webassembly. + * Following CDP semantics, it assumes the column is being the byte offset + * in webassembly. However, we encode the inline frame index in the line. */ getVariablesInScope?(callFrameId: string, position: IPosition): Promise; + + /** + * Gets the stack of WASM functions at the given position. Generally this will + * return an element with a single item containing the function name. However, + * inlined functions may return multiple functions for a position. + * + * It may return an empty array if function information is not available. + * + * @see https://github.com/ChromeDevTools/devtools-frontend/blob/c9f204731633fd2e2b6999a2543e99b7cc489b4b/docs/language_extension_api.md#dealing-with-inlined-functions + */ + getFunctionStack?(position: IPosition): Promise<{ name: string }[]>; + + /** + * Gets ranges that should be stepped for the given step kind and location. + * + * Following CDP semantics, it assumes the column is being the byte offset + * in webassembly. However, we encode the inline frame index in the line. + */ + getStepSkipList?( + direction: StepDirection, + position: IPosition, + sourceUrl?: string, + mappedPosition?: IPosition, + ): Promise; } class DecompiledWasmSymbols implements IWasmSymbols { @@ -245,19 +271,19 @@ class DecompiledWasmSymbols implements IWasmSymbols { public async compiledPositionFor( sourceUrl: string, sourcePosition: IPosition, - ): Promise { + ): Promise { if (sourceUrl !== this.decompiledUrl) { - return undefined; + return []; } const { byteOffsetsOfLines } = await this.doDisassemble(); const { lineNumber } = sourcePosition.base0; if (lineNumber >= byteOffsetsOfLines.length) { - return undefined; + return []; } const columnNumber = byteOffsetsOfLines[sourcePosition.base0.lineNumber]; - return new Base0Position(0, columnNumber); + return [new Base0Position(0, columnNumber)]; } public dispose(): void { @@ -340,7 +366,7 @@ class WasmSymbols extends DecompiledWasmSymbols { ): Promise<{ url: string; position: IPosition } | undefined> { const locations = await this.rpc.sendMessage('rawLocationToSourceLocation', { codeOffset: compiledPosition.base0.columnNumber - this.codeOffset, - inlineFrameIndex: 0, + inlineFrameIndex: compiledPosition.base0.lineNumber, rawModuleId: this.moduleId, }); @@ -358,7 +384,7 @@ class WasmSymbols extends DecompiledWasmSymbols { public override async compiledPositionFor( sourceUrl: string, sourcePosition: IPosition, - ): Promise { + ): Promise { const { lineNumber, columnNumber } = sourcePosition.base0; const locations = await this.rpc.sendMessage('sourceLocationToRawLocation', { lineNumber, @@ -380,8 +406,9 @@ class WasmSymbols extends DecompiledWasmSymbols { } // todo@connor4312: will there ever be a location in another module? - const location = locations.find(l => l.rawModuleId === this.moduleId); - return location && new Base0Position(0, this.codeOffset + locations[0].startOffset); + return locations + .filter(l => l.rawModuleId === this.moduleId) + .map(l => new Base0Position(0, this.codeOffset + l.startOffset)); } /** @inheritdoc */ @@ -396,7 +423,7 @@ class WasmSymbols extends DecompiledWasmSymbols { ): Promise { const location = { codeOffset: position.base0.columnNumber - this.codeOffset, - inlineFrameIndex: 0, + inlineFrameIndex: position.base0.lineNumber, rawModuleId: this.moduleId, }; @@ -415,6 +442,78 @@ class WasmSymbols extends DecompiledWasmSymbols { ); } + /** @inheritdoc */ + public async getFunctionStack(position: IPosition): Promise<{ name: string }[]> { + const info = await this.rpc.sendMessage('getFunctionInfo', { + codeOffset: position.base0.columnNumber - this.codeOffset, + inlineFrameIndex: position.base0.lineNumber, + rawModuleId: this.moduleId, + }); + + return 'frames' in info ? info.frames : []; + } + + /** @inheritdoc */ + public async getStepSkipList( + direction: StepDirection, + position: IPosition, + sourceUrl?: string, + mappedPosition?: IPosition, + ): Promise { + const thisLocation = { + codeOffset: position.base0.columnNumber - this.codeOffset, + inlineFrameIndex: position.base0.lineNumber, + rawModuleId: this.moduleId, + }; + + const getOwnLineRanges = () => { + if (!(mappedPosition && sourceUrl)) { + return []; + } + return this.rpc.sendMessage('sourceLocationToRawLocation', { + lineNumber: mappedPosition.base0.lineNumber, + columnNumber: -1, + rawModuleId: this.moduleId, + sourceFileURL: sourceUrl, + }); + }; + + let rawRanges: Chrome.DevTools.RawLocationRange[]; + switch (direction) { + case StepDirection.Out: { + // Step out should step out of inline functions. + rawRanges = await this.rpc.sendMessage('getInlinedFunctionRanges', thisLocation); + break; + } + case StepDirection.Over: { + // step over should both step over inline functions and any + // intermediary statements on this line, which may exist + // in WAT assembly but not in source code. + const ranges = await Promise.all([ + this.rpc.sendMessage('getInlinedCalleesRanges', thisLocation), + getOwnLineRanges(), + ]); + rawRanges = flatten(ranges); + break; + } + case StepDirection.In: + // Step in should skip over any intermediary statements on this line + rawRanges = await getOwnLineRanges(); + break; + default: + rawRanges = []; + break; + } + + return rawRanges.map( + r => + new Range( + new Base0Position(0, r.startOffset + this.codeOffset), + new Base0Position(0, r.endOffset + this.codeOffset), + ), + ); + } + private getMappedLines(sourceURL: string) { const prev = this.mappedLines.get(sourceURL); if (prev) { diff --git a/src/adapter/pause.ts b/src/adapter/pause.ts new file mode 100644 index 000000000..1eea0b6cf --- /dev/null +++ b/src/adapter/pause.ts @@ -0,0 +1,41 @@ +/*--------------------------------------------------------- + * Copyright (C) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------*/ + +import Cdp from '../cdp/api'; +import { IPossibleBreakLocation } from './breakpoints'; +import { StackTrace } from './stackTrace'; +import { Thread } from './threads'; + +export type PausedReason = + | 'step' + | 'breakpoint' + | 'exception' + | 'pause' + | 'entry' + | 'goto' + | 'function breakpoint' + | 'data breakpoint' + | 'frame_entry'; + +export const enum StepDirection { + In, + Over, + Out, +} + +export type ExpectedPauseReason = + | { reason: Exclude; description?: string } + | { reason: 'step'; description?: string; direction: StepDirection }; + +export interface IPausedDetails { + thread: Thread; + reason: PausedReason; + event: Cdp.Debugger.PausedEvent; + description: string; + stackTrace: StackTrace; + stepInTargets?: IPossibleBreakLocation[]; + hitBreakpoints?: string[]; + text?: string; + exception?: Cdp.Runtime.RemoteObject; +} diff --git a/src/adapter/smartStepping.ts b/src/adapter/smartStepping.ts index b07a07a91..87295c4c2 100644 --- a/src/adapter/smartStepping.ts +++ b/src/adapter/smartStepping.ts @@ -6,10 +6,10 @@ import { inject, injectable } from 'inversify'; import { ILogger, LogTag } from '../common/logging'; import { isInstanceOf } from '../common/objUtils'; import { AnyLaunchConfiguration } from '../configuration'; +import { ExpectedPauseReason, IPausedDetails, PausedReason, StepDirection } from './pause'; import { isSourceWithMap } from './source'; import { UnmappedReason } from './sourceContainer'; import { StackFrame } from './stackTrace'; -import { ExpectedPauseReason, IPausedDetails, PausedReason, StepDirection } from './threads'; export const enum StackFrameStepOverReason { NotStepped, diff --git a/src/adapter/sourceContainer.ts b/src/adapter/sourceContainer.ts index 97d02dd0a..bb158b589 100644 --- a/src/adapter/sourceContainer.ts +++ b/src/adapter/sourceContainer.ts @@ -478,7 +478,7 @@ export class SourceContainer { continue; } - let location: IUiLocation; + let locations: IUiLocation[]; if ('decompiledUrl' in value) { const entry = await value.compiledPositionFor( sourceUrl, @@ -487,8 +487,11 @@ export class SourceContainer { if (!entry) { continue; } - const { lineNumber, columnNumber } = entry.base1; - location = { lineNumber, columnNumber, source: compiled }; + locations = entry.map(l => ({ + lineNumber: l.base1.lineNumber, + columnNumber: l.base1.columnNumber, + source: compiled, + })); } else { const entry = this.sourceMapFactory.guardSourceMapFn( value, @@ -508,10 +511,12 @@ export class SourceContainer { compiled.inlineScriptOffset, ); - location = { lineNumber, columnNumber, source: compiled }; + // recurse for nested sourcemaps: + const location = { lineNumber, columnNumber, source: compiled }; + locations = [location, ...(await this.getCompiledLocations(location))]; } - output = output.concat(location, await this.getCompiledLocations(location)); + output = output.concat(locations); } return output; diff --git a/src/adapter/stackTrace.ts b/src/adapter/stackTrace.ts index b33d6db87..a961eeb17 100644 --- a/src/adapter/stackTrace.ts +++ b/src/adapter/stackTrace.ts @@ -4,13 +4,18 @@ import * as l10n from '@vscode/l10n'; import Cdp from '../cdp/api'; +import { groupBy } from '../common/arrayUtils'; import { once, posInt32Counter, truthy } from '../common/objUtils'; -import { Base0Position } from '../common/positions'; +import { Base0Position, Base1Position, IPosition, Range } from '../common/positions'; import { SourceConstants } from '../common/sourceUtils'; import Dap from '../dap/api'; import { asyncScopesNotAvailable } from '../dap/errors'; import { ProtocolError } from '../dap/protocolError'; +import { WasmScope } from './dwarf/wasmSymbolProvider'; +import { PreviewContextType } from './objectPreview/contexts'; +import { StepDirection } from './pause'; import { StackFrameStepOverReason, shouldStepOverStackFrame } from './smartStepping'; +import { ISourceWithMap, IWasmLocationProvider, SourceFromMap, isSourceWithWasm } from './source'; import { IPreferredUiLocation } from './sourceContainer'; import { getToStringIfCustom } from './templates/getStringyProps'; import { RawLocation, Thread } from './threads'; @@ -18,7 +23,7 @@ import { IExtraProperty, IScopeRef, IVariableContainer } from './variableStore'; export interface IFrameElement { /** DAP stack frame ID */ - frameId: number; + readonly frameId: number; /** Formats the stack element as V8 would format it */ formatAsNative(): Promise; /** Pretty formats the stack element as text */ @@ -27,11 +32,38 @@ export interface IFrameElement { toDap(format?: Dap.StackFrameFormat): Promise; } -type FrameElement = StackFrame | AsyncSeparator; +export interface IStackFrameElement extends IFrameElement { + /** Stack frame that contains this one. Usually == this, except for inline stack frames */ + readonly root: StackFrame; + + /** UI location for the frame. */ + uiLocation(): Promise | IPreferredUiLocation | undefined; + + /** + * Gets variable scopes on this frame. All scope variables should be added + * to the paused {@link VariablesStore} when this resolves. + */ + scopes(): Promise; + + /** + * Gets ranges that should be stepped for the given step kind and location. + */ + getStepSkipList(direction: StepDirection, position: IPosition): Promise; +} + +type FrameElement = StackFrame | InlinedFrame | AsyncSeparator; + +export const isStackFrameElement = (element: IFrameElement): element is IStackFrameElement => + typeof (element as IStackFrameElement).uiLocation === 'function'; export class StackTrace { public readonly frames: FrameElement[] = []; - private _frameById: Map = new Map(); + private _frameById: Map = new Map(); + /** + * Frame index that was last checked for inline expansion. + * @see https://github.com/ChromeDevTools/devtools-frontend/blob/c9f204731633fd2e2b6999a2543e99b7cc489b4b/docs/language_extension_api.md#dealing-with-inlined-functions + */ + private _lastInlineWasmExpanded = Promise.resolve(0); private _asyncStackTraceId?: Cdp.Runtime.StackTraceId; private _lastFrameThread?: Thread; @@ -53,33 +85,6 @@ export class StackTrace { return result; } - public static async fromRuntimeWithPredicate( - thread: Thread, - stack: Cdp.Runtime.StackTrace, - predicate: (frame: StackFrame) => Promise, - frameLimit = Infinity, - ): Promise { - const result = new StackTrace(thread); - for (let frameNo = 0; frameNo < stack.callFrames.length && frameLimit > 0; frameNo++) { - if (!stack.callFrames[frameNo].url.endsWith(SourceConstants.InternalExtension)) { - const frame = StackFrame.fromRuntime(thread, stack.callFrames[frameNo], false); - if (await predicate(frame)) { - result.frames.push(); - frameLimit--; - } - } - } - - if (stack.parentId) { - result._asyncStackTraceId = stack.parentId; - console.assert(!stack.parent); - } else { - result._appendStackTrace(thread, stack.parent); - } - - return result; - } - public static fromDebugger( thread: Thread, frames: Cdp.Debugger.CallFrame[], @@ -97,37 +102,88 @@ export class StackTrace { return result; } - constructor(thread: Thread) { + constructor(private readonly thread: Thread) { this._lastFrameThread = thread; } - async loadFrames(limit: number, noFuncEval?: boolean): Promise { + public async loadFrames(limit: number, noFuncEval?: boolean): Promise { + await this.expandAsyncStack(limit, noFuncEval); + await this.expandWasmFrames(); + return this.frames; + } + + private async expandAsyncStack(limit: number, noFuncEval?: boolean) { while (this.frames.length < limit && this._asyncStackTraceId) { - if (this._asyncStackTraceId.debuggerId) + if (this._asyncStackTraceId.debuggerId) { this._lastFrameThread = Thread.threadForDebuggerId(this._asyncStackTraceId.debuggerId); + } + if (!this._lastFrameThread) { this._asyncStackTraceId = undefined; break; } - if (noFuncEval) + + if (noFuncEval) { this._lastFrameThread .cdp() .DotnetDebugger.setEvaluationOptions({ options: { noFuncEval }, type: 'stackFrame' }); + } const response = await this._lastFrameThread .cdp() .Debugger.getStackTrace({ stackTraceId: this._asyncStackTraceId }); this._asyncStackTraceId = undefined; - if (response) this._appendStackTrace(this._lastFrameThread, response.stackTrace); + if (response) { + this._appendStackTrace(this._lastFrameThread, response.stackTrace); + } } - return this.frames; } - frame(frameId: number): StackFrame | undefined { + private expandWasmFrames() { + return (this._lastInlineWasmExpanded = this._lastInlineWasmExpanded.then(async last => { + for (; last < this.frames.length; last++) { + const frame = this.frames[last]; + if (!(frame instanceof StackFrame)) { + continue; + } + + const source = frame.scriptSource?.resolvedSource; + if (!isSourceWithWasm(source)) { + continue; + } + + const symbols = await source.sourceMap.value.promise; + if (!symbols.getFunctionStack) { + continue; + } + + const stack = await symbols.getFunctionStack(frame.rawPosition); + if (stack.length === 0) { + continue; + } + + for (let i = 0; i < stack.length; i++) { + const inlinedFrame = new InlinedFrame({ + source, + thread: this.thread, + inlineFrameIndex: i, + name: stack[i].name, + root: frame, + }); + + this._appendFrame(inlinedFrame, last++); + } + } + + return last; + })); + } + + public frame(frameId: number): StackFrame | InlinedFrame | undefined { return this._frameById.get(frameId); } - _appendStackTrace(thread: Thread, stackTrace: Cdp.Runtime.StackTrace | undefined) { + private _appendStackTrace(thread: Thread, stackTrace: Cdp.Runtime.StackTrace | undefined) { console.assert(!stackTrace || !this._asyncStackTraceId); while (stackTrace) { @@ -150,18 +206,22 @@ export class StackTrace { } } - _appendFrame(frame: FrameElement) { - this.frames.push(frame); - if (frame instanceof StackFrame) { + private _appendFrame(frame: FrameElement, index?: number) { + if (index !== undefined) { + this.frames.splice(index, 0, frame); + } else { + this.frames.push(frame); + } + if (!(frame instanceof AsyncSeparator)) { this._frameById.set(frame.frameId, frame); } } - async formatAsNative(): Promise { + public async formatAsNative(): Promise { return await this.formatWithMapper(frame => frame.formatAsNative()); } - async format(): Promise { + public async format(): Promise { return await this.formatWithMapper(frame => frame.format()); } @@ -181,7 +241,7 @@ export class StackTrace { return (await Promise.all(promises)).join('\n') + '\n'; } - async toDap(params: Dap.StackTraceParamsExtended): Promise { + public async toDap(params: Dap.StackTraceParamsExtended): Promise { const from = params.startFrame || 0; let to = (params.levels || 50) + from; const frames = await this.loadFrames(to, params.noFuncEval); @@ -288,10 +348,16 @@ function getDefaultName(callFrame: Cdp.Debugger.CallFrame | Cdp.Runtime.CallFram return callFrame.functionName || fallbackName; } -export class StackFrame implements IFrameElement { +export class StackFrame implements IStackFrameElement { public readonly frameId = frameIdCounter(); + /** Override for the `name` in the DAP representation. */ + public overrideName?: string; + /** @inheritdoc */ + public readonly root = this; private _rawLocation: RawLocation; + + /** @inheritdoc */ public readonly uiLocation: () => | Promise | IPreferredUiLocation @@ -339,6 +405,24 @@ export class StackFrame implements IFrameElement { this.isReplEval = script ? script.url.endsWith(SourceConstants.ReplExtension) : false; } + /** + * Gets this frame's script ID. + */ + public get scriptId() { + return 'scriptId' in this.callFrame + ? this.callFrame.scriptId + : this.callFrame.location.scriptId; + } + + /** + * Gets the source associated with the script ID of the stackframe. This may + * not be where the frame is eventually displayed to the user; + * use {@link uiLocation} for that. + */ + public get scriptSource() { + return this._thread._sourceContainer.getScriptById(this.scriptId); + } + /** * Gets whether the runtime explicitly said this frame can be restarted. */ @@ -362,6 +446,7 @@ export class StackFrame implements IFrameElement { return this._scope ? this._scope.callFrameId : undefined; } + /** @inheritdoc */ async scopes(): Promise { const currentScope = this._scope; if (!currentScope) { @@ -447,19 +532,26 @@ export class StackFrame implements IFrameElement { return { scopes: scopes.filter(truthy) }; } + /** @inheritdoc */ + public getStepSkipList(_direction: StepDirection): Promise { + // Normal JS never has any skip lists -- only web assembly does + return Promise.resolve(undefined); + } + private readonly getLocationInfo = once(async () => { const uiLocation = this.uiLocation(); const isSmartStepped = await shouldStepOverStackFrame(this); // only use the relatively expensive custom tostring lookup for frames // that aren't skipped, to avoid unnecessary work e.g. on node_internals const name = - 'this' in this.callFrame + this.overrideName || + ('this' in this.callFrame ? await getEnhancedName( this._thread, this.callFrame, isSmartStepped === StackFrameStepOverReason.NotStepped, ) - : getDefaultName(this.callFrame); + : getDefaultName(this.callFrame)); return { isSmartStepped, name, uiLocation: await uiLocation }; }); @@ -583,3 +675,134 @@ export class StackFrame implements IFrameElement { return ([] as Dap.CompletionItem[]).concat(...completions); }); } + +const EMPTY_SCOPES: Dap.ScopesResult = { scopes: [] }; + +export class InlinedFrame implements IStackFrameElement { + /** @inheritdoc */ + public readonly root: StackFrame; + + /** @inheritdoc */ + public readonly frameId = frameIdCounter(); + + /** @inheritdoc */ + public readonly uiLocation: () => Promise; + + private readonly name: string; + private readonly thread: Thread; + private readonly inlineFrameIndex: number; + private readonly source: ISourceWithMap; + + constructor(opts: { + thread: Thread; + /** Inline frame index in the function info */ + inlineFrameIndex: number; + /** Display name of the call frame */ + name: string; + /** Original WASM source */ + source: ISourceWithMap; + /** Original stack frame this was derived from */ + root: StackFrame; + }) { + this.name = opts.name; + this.root = opts.root; + this.thread = opts.thread; + this.source = opts.source; + this.inlineFrameIndex = opts.inlineFrameIndex; + this.uiLocation = once(() => + opts.thread._sourceContainer.preferredUiLocation({ + columnNumber: opts.root.rawPosition.base1.columnNumber, + lineNumber: opts.inlineFrameIndex + 1, + source: opts.source, + }), + ); + } + + /** @inheritdoc */ + public async formatAsNative(): Promise { + const { columnNumber, lineNumber, source } = await this.uiLocation(); + return ` at ${this.name} (${source.url}:${lineNumber}:${columnNumber})`; + } + + /** @inheritdoc */ + public async format(): Promise { + const { columnNumber, lineNumber, source } = await this.uiLocation(); + const prettyName = (await source.prettyName()) || ''; + return `${this.name} @ ${prettyName}:${lineNumber}:${columnNumber}`; + } + + /** @inheritdoc */ + public async toDap(): Promise { + const { columnNumber, lineNumber, source } = await this.uiLocation(); + return Promise.resolve({ + id: this.frameId, + name: this.name, + column: columnNumber, + line: lineNumber, + source: await source.toDapShallow(), + }); + } + + /** @inheritdoc */ + public async getStepSkipList(direction: StepDirection): Promise { + const sm = this.source.sourceMap.value.settledValue; + if (!sm?.getStepSkipList) { + return; + } + + const uiLocation = await this.uiLocation(); + if (uiLocation) { + return sm.getStepSkipList( + direction, + this.root.rawPosition, + (uiLocation.source as SourceFromMap).compiledToSourceUrl.get(this.source), + new Base1Position(uiLocation.lineNumber, uiLocation.columnNumber), + ); + } else { + return sm.getStepSkipList(direction, this.root.rawPosition); + } + } + + /** @inheritdoc */ + public async scopes(): Promise { + const v = this.source.sourceMap.value.settledValue; + const callFrameId = this.root.callFrameId(); + if (!v || !callFrameId) { + return EMPTY_SCOPES; + } + + const variables = await v.getVariablesInScope?.( + callFrameId, + new Base0Position(this.inlineFrameIndex, this.root.rawPosition.base0.columnNumber), + ); + if (!variables) { + return EMPTY_SCOPES; + } + + const paused = this.thread.pausedVariables(); + if (!paused) { + return EMPTY_SCOPES; + } + + const scopeRef: IScopeRef = { + stackFrame: this.root, + callFrameId, + scopeNumber: 0, // this is only used for setting variables, which wasm doesn't support + }; + + return { + scopes: await Promise.all( + [...groupBy(variables, v => v.scope)].map(([key, vars]) => + paused + .createWasmScope(key as WasmScope, vars, scopeRef) + .toDap(PreviewContextType.PropertyValue) + .then(v => ({ + name: v.name, + variablesReference: v.variablesReference, + expensive: key !== WasmScope.Local, + })), + ), + ), + }; + } +} diff --git a/src/adapter/threads.ts b/src/adapter/threads.ts index 33e0a1408..42e934864 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 } from '../common/positions'; +import { Base1Position, Range } from '../common/positions'; import { IDeferred, delay, getDeferred } from '../common/promiseUtil'; import { IRenameProvider } from '../common/sourceMaps/renameProvider'; import * as sourceUtils from '../common/sourceUtils'; @@ -25,7 +25,7 @@ import { ProtocolError } from '../dap/protocolError'; import { NodeWorkerTarget } from '../targets/node/nodeWorkerTarget'; import { ITarget } from '../targets/targets'; import { IShutdownParticipants } from '../ui/shutdownParticipants'; -import { BreakpointManager, EntryBreakpointMode, IPossibleBreakLocation } from './breakpoints'; +import { BreakpointManager, EntryBreakpointMode } from './breakpoints'; import { UserDefinedBreakpoint } from './breakpoints/userDefinedBreakpoint'; import { ICompletions } from './completions'; import { ExceptionMessage, IConsole, QueryObjectsMessage } from './console'; @@ -34,49 +34,17 @@ import { IEvaluator } from './evaluator'; import { IExceptionPauseService } from './exceptionPauseService'; 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 { IPreferredUiLocation, SourceContainer } from './sourceContainer'; -import { StackFrame, StackTrace } from './stackTrace'; +import { StackFrame, StackTrace, isStackFrameElement } from './stackTrace'; import { serializeForClipboard, serializeForClipboardTmpl, } from './templates/serializeForClipboard'; import { IVariableStoreLocationProvider, VariableStore } from './variableStore'; -export type PausedReason = - | 'step' - | 'breakpoint' - | 'exception' - | 'pause' - | 'entry' - | 'goto' - | 'function breakpoint' - | 'data breakpoint' - | 'frame_entry'; - -export const enum StepDirection { - In, - Over, - Out, -} - -export type ExpectedPauseReason = - | { reason: Exclude; description?: string } - | { reason: 'step'; description?: string; direction: StepDirection }; - -export interface IPausedDetails { - thread: Thread; - reason: PausedReason; - event: Cdp.Debugger.PausedEvent; - description: string; - stackTrace: StackTrace; - stepInTargets?: IPossibleBreakLocation[]; - hitBreakpoints?: string[]; - text?: string; - exception?: Cdp.Runtime.RemoteObject; -} - export class ExecutionContext { public readonly sourceMapLoads = new Map>(); public readonly scripts: Script[] = []; @@ -193,7 +161,7 @@ export class Thread implements IVariableStoreLocationProvider { private _pausedForSourceMapScriptId?: string; private _executionContexts: Map = new Map(); readonly replVariables: VariableStore; - private _sourceContainer: SourceContainer; + readonly _sourceContainer: SourceContainer; private _pauseOnSourceMapBreakpointIds?: Cdp.Debugger.BreakpointId[]; private _selectedContext: ExecutionContext | undefined; static _allThreadsByDebuggerId = new Map(); @@ -301,7 +269,8 @@ export class Thread implements IVariableStoreLocationProvider { public stepOver(): Promise { return this.stateQueue.enqueue('stepOver', async () => { this._expectedPauseReason = { reason: 'step', direction: StepDirection.Over }; - if (await this._cdp.Debugger.stepOver({})) { + const skipList = await this.getCurrentSkipList(StepDirection.Over); + if (await this._cdp.Debugger.stepOver({ skipList })) { return {}; } @@ -324,7 +293,8 @@ export class Thread implements IVariableStoreLocationProvider { return {}; } } else { - if (await this._cdp.Debugger.stepInto({ breakOnAsyncCall: true })) { + const skipList = await this.getCurrentSkipList(StepDirection.In); + if (await this._cdp.Debugger.stepInto({ breakOnAsyncCall: true, skipList })) { return {}; } } @@ -345,6 +315,32 @@ export class Thread implements IVariableStoreLocationProvider { }); } + private async getCurrentSkipList(direction: StepDirection) { + if (!this._pausedDetails) { + return; + } + + const [frame] = await this._pausedDetails.stackTrace.loadFrames(1); + if (!frame || !isStackFrameElement(frame)) { + return undefined; + } + + const list = await frame.getStepSkipList(direction); + if (!list) { + return undefined; + } + + // make sure to simplify the range, as V8 is picky about + // wanting the ranges in order and non-overlapping. + return Range.simplify(list).map( + (r): Cdp.Debugger.LocationRange => ({ + start: r.begin.base0, + end: r.end.base0, + scriptId: frame.root.scriptId, + }), + ); + } + _stackFrameNotFoundError(): Dap.Error { return errors.createSilentError(l10n.t('Stack frame not found')); } @@ -354,7 +350,7 @@ export class Thread implements IVariableStoreLocationProvider { } async restartFrame(params: Dap.RestartFrameParams): Promise { - const stackFrame = this._pausedDetails?.stackTrace.frame(params.frameId); + const stackFrame = this._pausedDetails?.stackTrace.frame(params.frameId)?.root; if (!stackFrame) { return this._stackFrameNotFoundError(); } @@ -432,7 +428,7 @@ export class Thread implements IVariableStoreLocationProvider { let stackFrame: StackFrame | undefined; if (params.frameId !== undefined) { stackFrame = this._pausedDetails - ? this._pausedDetails.stackTrace.frame(params.frameId) + ? this._pausedDetails.stackTrace.frame(params.frameId)?.root : undefined; if (!stackFrame) return this._stackFrameNotFoundError(); if (!stackFrame.callFrameId()) return this._evaluateOnAsyncFrameError(); @@ -471,7 +467,7 @@ export class Thread implements IVariableStoreLocationProvider { let stackFrame: StackFrame | undefined; if (args.frameId !== undefined) { stackFrame = this._pausedDetails - ? this._pausedDetails.stackTrace.frame(args.frameId) + ? this._pausedDetails.stackTrace.frame(args.frameId)?.root : undefined; if (!stackFrame) { throw new ProtocolError(this._stackFrameNotFoundError()); diff --git a/src/adapter/variableStore.ts b/src/adapter/variableStore.ts index d3d54a342..6f73d2a02 100644 --- a/src/adapter/variableStore.ts +++ b/src/adapter/variableStore.ts @@ -7,7 +7,6 @@ import { generate } from 'astring'; import { inject, injectable } from 'inversify'; import Cdp from '../cdp/api'; import { ICdpApi } from '../cdp/connection'; -import { groupBy, iteratorFirst } from '../common/arrayUtils'; import { flatten, isInstanceOf, once } from '../common/objUtils'; import { parseSource, statementsToFunction } from '../common/sourceCodeManipulations'; import { IRenameProvider } from '../common/sourceMaps/renameProvider'; @@ -20,7 +19,6 @@ import { IWasmVariable, IWasmVariableEvaluation, WasmScope } from './dwarf/wasmS import * as objectPreview from './objectPreview'; import { MapPreview, SetPreview } from './objectPreview/betterTypes'; import { PreviewContextType } from './objectPreview/contexts'; -import { SourceFromMap, isSourceWithWasm } from './source'; import { StackFrame, StackTrace } from './stackTrace'; import { RemoteException, RemoteObjectId, getSourceSuffix } from './templates'; import { getArrayProperties } from './templates/getArrayProperties'; @@ -181,9 +179,9 @@ interface IContextSettings { } const wasmScopeNames: { [K in WasmScope]: { name: string; sortOrder: number } } = { - [WasmScope.Parameter]: { name: l10n.t('Native Parameters'), sortOrder: -10 }, - [WasmScope.Local]: { name: l10n.t('Native Locals'), sortOrder: -9 }, - [WasmScope.Global]: { name: l10n.t('Native Globals'), sortOrder: -8 }, + [WasmScope.Parameter]: { name: l10n.t('Parameters'), sortOrder: -10 }, + [WasmScope.Local]: { name: l10n.t('Locals'), sortOrder: -9 }, + [WasmScope.Global]: { name: l10n.t('Globals'), sortOrder: -8 }, }; class VariableContext { @@ -280,21 +278,6 @@ class VariableContext { return this.createVariable(Variable, ctx, object); } - public createVariableForWasm(variables: IWasmVariable[], scopeRef: IScopeRef) { - const groups = groupBy(variables, v => v.scope); - const groupVars: WasmScopeVariable[] = []; - for (const [key, display] of Object.entries(wasmScopeNames)) { - const vars = groups.get(key as WasmScope); - if (vars) { - groupVars.push( - this.createVariable(WasmScopeVariable, display, key as WasmScope, vars, scopeRef), - ); - } - } - - return groupVars; - } - /** * Ensures symbols for custom descriptions are available, must be used * before getStringProps/getToStringIfCustom @@ -1137,7 +1120,7 @@ class WasmScopeVariable implements IVariable { public readonly id = getVariableId(); /** @inheritdoc */ - public readonly sortOrder = wasmScopeNames[this.kind].sortOrder; + public readonly sortOrder = wasmScopeNames[this.kind]?.sortOrder || 0; constructor( private readonly context: VariableContext, @@ -1148,7 +1131,7 @@ class WasmScopeVariable implements IVariable { toDap(): Promise { return Promise.resolve({ - name: wasmScopeNames[this.kind].name, + name: wasmScopeNames[this.kind]?.name || this.kind, value: '', variablesReference: this.id, }); @@ -1195,23 +1178,6 @@ class Scope implements IVariableContainer { } } - // special case: if the stack frame is in a wasm that had a symbolicated - // source at this location, use its symbol mapping instead of native symbols. - if (this.ref.scopeNumber === 0) { - const location = await this.ref.stackFrame.uiLocation(); - if (location?.source instanceof SourceFromMap) { - const c = iteratorFirst(location.source.compiledToSourceUrl.keys()); - if (isSourceWithWasm(c) && c.sourceMap.value.settledValue?.getVariablesInScope) { - const wasmVars = await c.sourceMap.value.settledValue.getVariablesInScope( - this.ref.callFrameId, - this.ref.stackFrame.rawPosition, - ); - const resolved: IVariable[] = this.context.createVariableForWasm(wasmVars, this.ref); - return resolved.concat(variables); - } - } - } - return variables; } @@ -1369,6 +1335,32 @@ export class VariableStore { return scope; } + /** Creates a container for a CDP Scope */ + public createWasmScope( + kind: WasmScope, + variables: readonly IWasmVariable[], + scopeRef: IScopeRef, + ): IVariable { + const scope: WasmScopeVariable = new WasmScopeVariable( + new VariableContext( + this.cdp, + undefined, + { name: '' }, + this.vars, + this.locationProvider, + () => scope, + this.contextSettings, + ), + kind, + variables, + scopeRef, + ); + + this.vars.add(scope); + + return scope; + } + /** Gets whether the variablesReference exists in this store */ public hasVariable(variablesReference: number) { return !!this.vars.get(variablesReference); diff --git a/src/common/positions.test.ts b/src/common/positions.test.ts new file mode 100644 index 000000000..ebd463851 --- /dev/null +++ b/src/common/positions.test.ts @@ -0,0 +1,63 @@ +/*--------------------------------------------------------- + * Copyright (C) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------*/ + +import { expect } from 'chai'; +import { Base0Position, Range } from './positions'; + +describe('Range', () => { + describe('simplify', () => { + it('should merge overlapping ranges', () => { + const range1 = new Range(new Base0Position(0, 0), new Base0Position(0, 5)); + const range2 = new Range(new Base0Position(0, 3), new Base0Position(0, 8)); + const range3 = new Range(new Base0Position(0, 10), new Base0Position(0, 15)); + const range4 = new Range(new Base0Position(0, 12), new Base0Position(0, 20)); + const range5 = new Range(new Base0Position(0, 25), new Base0Position(0, 30)); + const range6 = new Range(new Base0Position(0, 28), new Base0Position(0, 35)); + const mergedRanges = Range.simplify([range1, range2, range3, range4, range5, range6]); + expect(mergedRanges.join(', ')).to.equal( + 'Range[0:0 -> 0:8], Range[0:10 -> 0:20], Range[0:25 -> 0:35]', + ); + }); + + it('should merge adjacent ranges', () => { + const range1 = new Range(new Base0Position(0, 0), new Base0Position(0, 5)); + const range2 = new Range(new Base0Position(0, 5), new Base0Position(0, 8)); + const range3 = new Range(new Base0Position(0, 8), new Base0Position(0, 10)); + const range4 = new Range(new Base0Position(0, 10), new Base0Position(0, 15)); + const range5 = new Range(new Base0Position(0, 15), new Base0Position(0, 20)); + const mergedRanges = Range.simplify([range1, range2, range3, range4, range5]); + expect(mergedRanges.join(', ')).to.equal('Range[0:0 -> 0:20]'); + }); + + it('should not merge non-overlapping ranges', () => { + const range1 = new Range(new Base0Position(0, 0), new Base0Position(0, 5)); + const range2 = new Range(new Base0Position(0, 7), new Base0Position(0, 10)); + const range3 = new Range(new Base0Position(0, 12), new Base0Position(0, 15)); + const mergedRanges = Range.simplify([range1, range2, range3]); + expect(mergedRanges.join(', ')).to.equal( + 'Range[0:0 -> 0:5], Range[0:7 -> 0:10], Range[0:12 -> 0:15]', + ); + }); + + it('should handle empty input', () => { + const mergedRanges = Range.simplify([]); + expect(mergedRanges).to.have.lengthOf(0); + }); + + it('should handle input with a single range', () => { + const range = new Range(new Base0Position(0, 0), new Base0Position(0, 5)); + const mergedRanges = Range.simplify([range]); + expect(mergedRanges.join(', ')).to.equal('Range[0:0 -> 0:5]'); + }); + + it('should handle duplicated range', () => { + const range1 = new Range(new Base0Position(0, 0), new Base0Position(0, 5)); + const range2 = new Range(new Base0Position(0, 0), new Base0Position(0, 5)); + const range3 = new Range(new Base0Position(0, 6), new Base0Position(0, 7)); + const range4 = new Range(new Base0Position(0, 6), new Base0Position(0, 7)); + const mergedRanges = Range.simplify([range1, range2, range3, range4]); + expect(mergedRanges.join(', ')).to.equal('Range[0:0 -> 0:5], Range[0:6 -> 0:7]'); + }); + }); +}); diff --git a/src/common/positions.ts b/src/common/positions.ts index 5c619c70c..bf610001b 100644 --- a/src/common/positions.ts +++ b/src/common/positions.ts @@ -51,7 +51,8 @@ export class Base0Position implements IPosition { } public compare(other: Base0Position) { - return this.lineNumber - other.lineNumber || this.columnNumber - other.columnNumber || 0; + const other0 = other.base0; + return this.lineNumber - other0.lineNumber || this.columnNumber - other0.columnNumber; } } @@ -76,7 +77,8 @@ export class Base1Position implements IPosition { } public compare(other: Base1Position) { - return this.lineNumber - other.lineNumber || this.columnNumber - other.columnNumber || 0; + const other1 = other.base1; + return this.lineNumber - other1.lineNumber || this.columnNumber - other1.columnNumber || 0; } } @@ -101,6 +103,40 @@ export class Base01Position implements IPosition { } public compare(other: Base01Position) { - return this.lineNumber - other.lineNumber || this.columnNumber - other.columnNumber || 0; + const other01 = other.base01; + return this.lineNumber - other01.lineNumber || this.columnNumber - other01.columnNumber; + } +} + +export class Range { + public static simplify(rangeList: readonly Range[]): Range[] { + if (rangeList.length === 0) { + return []; + } + + const sortedRanges = rangeList.slice().sort((a, b) => a.begin.compare(b.begin)); + const mergedRanges: Range[] = []; + + let currentRange = sortedRanges[0]; + for (let i = 1; i < sortedRanges.length; i++) { + const nextRange = sortedRanges[i]; + if (currentRange.end.compare(nextRange.begin) >= 0) { + currentRange = new Range(currentRange.begin, nextRange.end); + } else { + mergedRanges.push(currentRange); + currentRange = nextRange; + } + } + mergedRanges.push(currentRange); + + return mergedRanges; + } + + constructor(public readonly begin: IPosition, public readonly end: IPosition) {} + + public toString() { + const b1 = this.begin.base0; + const e1 = this.end.base0; + return `Range[${b1.lineNumber}:${b1.columnNumber} -> ${e1.lineNumber}:${e1.columnNumber}]`; } } From cdb8ac684c6aaccf3d172a9cc51d48cf1d985755 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Tue, 19 Sep 2023 17:15:47 -0700 Subject: [PATCH 2/2] update tests --- ...with-stop-on-entry-on-ts-file-reports-as-breakpoint.txt | 2 +- ...ces-sourcemap-error-handling-logs-lazy-parse-errors.txt | 7 ------- src/test/sources/sourcesTest.ts | 1 - 3 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/test/breakpoints/breakpoints-user-defined-bp-on-first-line-with-stop-on-entry-on-ts-file-reports-as-breakpoint.txt b/src/test/breakpoints/breakpoints-user-defined-bp-on-first-line-with-stop-on-entry-on-ts-file-reports-as-breakpoint.txt index e382e77bd..43021aad1 100644 --- a/src/test/breakpoints/breakpoints-user-defined-bp-on-first-line-with-stop-on-entry-on-ts-file-reports-as-breakpoint.txt +++ b/src/test/breakpoints/breakpoints-user-defined-bp-on-first-line-with-stop-on-entry-on-ts-file-reports-as-breakpoint.txt @@ -1,7 +1,7 @@ { allThreadsStopped : false description : Paused on breakpoint - reason : entry + reason : breakpoint threadId : } @ ${workspaceFolder}/tsNodeApp/app.ts:1:1 diff --git a/src/test/sources/sources-sourcemap-error-handling-logs-lazy-parse-errors.txt b/src/test/sources/sources-sourcemap-error-handling-logs-lazy-parse-errors.txt index 671ba527c..b9c524945 100644 --- a/src/test/sources/sources-sourcemap-error-handling-logs-lazy-parse-errors.txt +++ b/src/test/sources/sources-sourcemap-error-handling-logs-lazy-parse-errors.txt @@ -1,10 +1,3 @@ Evaluating#1: //# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiZXZhbDEuanMiLCJzb3VyY2VSb290IjoiIiwic291cmNlcyI6WyJldmFsMVNvdXJjZS5qcyJdLCJtYXBwaW5ncyI6IiMsIyMjIzsifQ== stderr> Could not read source map for http://localhost:8001/eval1.js: Error parsing mappings (code 4): invalid base 64 character while parsing a VLQ -{ - allThreadsStopped : false - description : Paused on breakpoint - reason : breakpoint - threadId : -} - @ localhost꞉8001/eval1.js:3:23 diff --git a/src/test/sources/sourcesTest.ts b/src/test/sources/sourcesTest.ts index 8a6384937..481c4b208 100644 --- a/src/test/sources/sourcesTest.ts +++ b/src/test/sources/sourcesTest.ts @@ -262,7 +262,6 @@ describe('sources', () => { `//# sourceMappingURL=data:application/json;charset=utf-8;base64,${contents}\n`, ); await p.logger.logOutput(await output); - await waitForPause(p); await ev; p.assertLog(); });