diff --git a/src/gdb/GDBDebugSessionBase.ts b/src/gdb/GDBDebugSessionBase.ts index 36807eac..64b64a24 100644 --- a/src/gdb/GDBDebugSessionBase.ts +++ b/src/gdb/GDBDebugSessionBase.ts @@ -42,6 +42,7 @@ import { ObjectVariableReference, MemoryRequestArguments, CDTDisassembleArguments, + RequestArgRun, } from '../types/session'; import { IGDBBackend, IGDBBackendFactory } from '../types/gdb'; import { getInstructions } from '../util/disassembly'; @@ -154,7 +155,9 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession { * (GDBTargetDebugSession) and never for type="gdb" (GDBDebugSession). */ protected isRemote = false; - // isRunning === true means there are no threads stopped. + // isRunning === true means there are threads and none are stopped. + // When uncertain because we have not received the status of the newest + // threads yet, this is the last certain value. protected isRunning = false; protected supportsRunInTerminalRequest = false; @@ -175,6 +178,7 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession { protected updateThreadInfo: 'missing' | 'when-requested' | 'never' = 'missing'; + protected runAfterConfiguration: RequestArgRun = RequestArgRun.ALWAYS; /** * State variables for pauseIfNeeded/continueIfNeeded logic, mostly used for @@ -208,7 +212,9 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession { // keeps track of where in the configuration phase (between initialize event // and configurationDone response) we are protected configuringState: ConfiguringState = ConfiguringState.INITIAL; - protected isInitialized = false; + protected isInitialized = false; // unused here but kept for compatibility + protected deferredStopEvents: any[] = []; + protected firstContinueIsRun = false; /** * customResetCommands from launch.json @@ -348,6 +354,16 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession { "Valid values are: 'missing', 'when-requested', or 'never'." ); } + if ( + args.run !== undefined && + args.run !== RequestArgRun.ALWAYS && + args.run !== RequestArgRun.PRESERVE + ) { + throw new Error( + `Invalid value for 'run': '${args.run}'. ` + + "Valid values are: 'always', 'preserve'." + ); + } } /** @@ -361,6 +377,7 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession { this.steppingResponseTimeout = args.steppingResponseTimeout ?? DEFAULT_STEPPING_RESPONSE_TIMEOUT; this.updateThreadInfo = args.updateThreadInfo ?? 'missing'; + this.runAfterConfiguration = args.run ?? RequestArgRun.ALWAYS; } /** @@ -758,17 +775,35 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession { this.waitPaused = undefined; } + protected setConfiguringStateDone(): void { + this.configuringState = ConfiguringState.DONE; + for (const resultData of this.deferredStopEvents) { + this.handleGDBStopped(resultData); + } + this.deferredStopEvents.splice(0); + } + protected async continueIfNeeded(): Promise { if (this.pauseCount > 0) { this.pauseCount--; if (this.pauseCount === 0) { if (this.configuringState === ConfiguringState.FINISHING) { - this.configuringState = ConfiguringState.DONE; - if (this.isAttach) { - await mi.sendExecContinue(this.gdb); - } else { - await mi.sendExecRun(this.gdb); + if ( + this.runAfterConfiguration == RequestArgRun.ALWAYS || + this.waitPausedNeeded + ) { + if (this.isAttach) { + await mi.sendExecContinue(this.gdb); + } else { + await mi.sendExecRun(this.gdb); + } + } else if (!this.isAttach) { + // We would have sent a 'run' rather than a 'continue', + // but since the user didn't want us to, let them do it + // manually using a continue request. + this.firstContinueIsRun = true; } + this.setConfiguringStateDone(); } else if (this.waitPausedNeeded) { if (this.gdb.isNonStopMode()) { await mi.sendExecContinue( @@ -1706,7 +1741,7 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession { this.configuringState = ConfiguringState.FINISHING; await this.continueIfNeeded(); } else { - this.configuringState = ConfiguringState.DONE; + this.setConfiguringStateDone(); } this.sendResponse(response); } catch (err) { @@ -1725,7 +1760,14 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession { name += ` (${thread.details})`; } - const running = thread.state === 'running'; + // 'thread-created' notifications don't include a thread state, it comes + // later via a 'stopped' or 'running' notification. + const running = + thread.state === 'running' + ? true + : thread.state === 'stopped' + ? false + : undefined; return new ThreadWithStatus(parseInt(thread.id, 10), name, running); } @@ -2014,7 +2056,12 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession { } try { - await mi.sendExecContinue(this.gdb, args.threadId); + if (this.firstContinueIsRun) { + this.firstContinueIsRun = false; + await mi.sendExecRun(this.gdb); + } else { + await mi.sendExecContinue(this.gdb, args.threadId); + } let isAllThreadsContinued; if (this.gdb.isNonStopMode()) { isAllThreadsContinued = args.threadId ? false : true; @@ -2338,15 +2385,16 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession { /** * Send command to backend. * @param expression Command to be executed + * @returns Promise for: - MI response as JS object if MI command, - `undefined` if CLI command */ protected async sendCommandToGdb( gdb: IGDBBackend, expression: string, frameRef: FrameReference | undefined - ): Promise { + ): Promise { if (expression.startsWith('-')) { // GDB/MI command - await gdb.sendCommand(expression); + return await gdb.sendCommand(expression); } else { // GDB CLI command await mi.sendInterpreterExecConsole(gdb, { @@ -2397,9 +2445,13 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession { frameRef: FrameReference | undefined ): Promise { const trimmedExpression = expression.trim(); - await this.sendCommandToGdb(this.gdb, trimmedExpression, frameRef); + const result = await this.sendCommandToGdb( + this.gdb, + trimmedExpression, + frameRef + ); response.body = { - result: '\r', + result: result ? JSON.stringify(result) : '\r', variablesReference: 0, }; await this.sendCommandToOtherGdbs( @@ -2455,7 +2507,7 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession { response: DebugProtocol.EvaluateResponse, args: DebugProtocol.EvaluateArguments ): Promise { - return this.doEvaluateRequest(response, args, false); + return this.doEvaluateRequest(response, args, true); } private extractExpressionFormat( @@ -2499,7 +2551,7 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession { protected async doEvaluateRequest( response: DebugProtocol.EvaluateResponse, args: DebugProtocol.EvaluateArguments, - alwaysAllowCliCommand: boolean // if true, allows evaluation of expression without a frameId + alwaysAllowCliCommand: boolean // if true, allows evaluation of expression without a frameId (kept for compatibility) ): Promise { response.body = { result: 'Error: could not evaluate expression', @@ -3148,15 +3200,19 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession { return requestsToResolve.length > 0; } + private updateIsRunning() { + const newIsRunning = this.threads.some((t) => t.running === false) // have stopped + ? false + : this.threads.some((t) => t.running === undefined) // have unknown + ? undefined + : this.threads.some((t) => t.running === true); // have running + if (newIsRunning !== undefined) { + this.isRunning = newIsRunning; + } + // else leave this.isRunning at its previous known value + } + protected handleGDBAsync(resultClass: string, resultData: any) { - const updateIsRunning = () => { - this.isRunning = this.threads.length ? true : false; - for (const thread of this.threads) { - if (!thread.running) { - this.isRunning = false; - } - } - }; switch (resultClass) { case 'running': if (this.gdb.isNonStopMode()) { @@ -3172,13 +3228,28 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession { thread.running = true; } } - updateIsRunning(); - if (this.isInitialized) { + this.updateIsRunning(); + if (this.configuringState == ConfiguringState.DONE) { this.handleGDBResume(resultData); + } else { + // while we are deferring events, cancel previous stop events for the same thread + if (this.deferredStopEvents.length > 0) { + if (resultData['thread-id'] === 'all') { + this.deferredStopEvents.splice(0); + } else if (resultData['thread-id'] !== undefined) { + this.deferredStopEvents = + this.deferredStopEvents.filter( + (stopResultData) => + stopResultData['thread-id'] !== + resultData['thread-id'] + ); + } + } } break; case 'stopped': { let suppressHandleGDBStopped = false; + let newThreadsConfirmed = false; if (this.gdb.isNonStopMode()) { const id = parseInt(resultData['thread-id'], 10); for (const thread of this.threads) { @@ -3202,6 +3273,9 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession { } } else { for (const thread of this.threads) { + if (thread.running === undefined) { + newThreadsConfirmed = true; + } thread.running = false; thread.lastRunToken = undefined; } @@ -3227,14 +3301,17 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession { } const wasRunning = this.isRunning; - updateIsRunning(); + this.updateIsRunning(); if ( !suppressHandleGDBStopped && (this.gdb.isNonStopMode() || + newThreadsConfirmed || (wasRunning && !this.isRunning)) ) { - if (this.isInitialized) { + if (this.configuringState == ConfiguringState.DONE) { this.handleGDBStopped(resultData); + } else { + this.deferredStopEvents.push(resultData); } } break; @@ -3267,6 +3344,7 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession { switch (notifyClass) { case 'thread-created': this.threads.push(this.convertThread(notifyData)); + this.updateIsRunning(); this.missingThreadNames = true; break; case 'thread-exited': { diff --git a/src/gdb/common.ts b/src/gdb/common.ts index c6d5d734..adcf8337 100644 --- a/src/gdb/common.ts +++ b/src/gdb/common.ts @@ -12,9 +12,9 @@ import { DebugProtocol } from '@vscode/debugprotocol'; export class ThreadWithStatus implements DebugProtocol.Thread { id: number; name: string; - running: boolean; + running?: boolean; lastRunToken: string | undefined; - constructor(id: number, name: string, running: boolean) { + constructor(id: number, name: string, running?: boolean) { this.id = id; this.name = name; this.running = running; diff --git a/src/integration-tests/attach.spec.ts b/src/integration-tests/attach.spec.ts index 2cc353bb..8c85778a 100644 --- a/src/integration-tests/attach.spec.ts +++ b/src/integration-tests/attach.spec.ts @@ -17,6 +17,7 @@ import { fillDefaults, gdbAsync, gdbNonStop, + gdbVersionAtLeast, isRemoteTest, standardBeforeEach, testProgramsDir, @@ -56,6 +57,94 @@ describe('attach', function () { expect(await dc.evaluate('argv[1]')).to.contain('running-from-spawn'); }); + function makeRunArgTest(runArg: string) { + return async function (this: Mocha.Context) { + if (isRemoteTest) { + // attachRemote.spec.ts is the test for when isRemoteTest + this.skip(); + } + const isAsync = + gdbAsync && + (os.platform() !== 'win32' || + (await gdbVersionAtLeast('13.0'))); + if (!isAsync && runArg === 'always') { + // in sync mode when all threads are running we can't ask '-thread-info' + this.skip(); + } + + const eventCounter = { stopped: 0, continued: 0 }; + dc.on('stopped', () => { + eventCounter.stopped++; + }); + dc.on('continued', () => { + eventCounter.continued++; + }); + + const attachArgs = fillDefaults(this.test, { + program: program, + processId: `${inferior.pid}`, + run: runArg, + } as AttachRequestArguments); + + await Promise.all([ + dc + .waitForEvent('initialized') + .then(() => dc.configurationDoneRequest()), + dc.initializeRequest().then(() => dc.attachRequest(attachArgs)), + ]); + + const threadInfo = JSON.parse( + ( + await dc.evaluateRequest({ + expression: '>-thread-info', + context: 'repl', + }) + ).body.result + ); + const threadStates = threadInfo.threads.map((t: any) => t.state); + if (runArg === 'always') { + expect(threadStates).to.contain('running'); + expect(threadStates).not.to.contain('stopped'); + } else if (runArg === 'preserve') { + expect(threadStates).to.contain('stopped'); + } else { + expect(runArg).to.be.oneOf(['always', 'preserve']); + } + + expect(eventCounter).to.deep.equal({ + stopped: runArg === 'always' ? 0 : 1, + continued: 0, + }); + + // check that continue sends the right GDB commands + // (always -exec-continue for attach; + // first -exec-run, then -exec-continue for launch) + if (runArg === 'preserve') { + await dc.setBreakpointsRequest({ + source: { path: src }, + breakpoints: [{ line: 25 }], + }); + await dc.continue({ threadId: -1 }, 'breakpoint', { line: 25 }); + await dc.setBreakpointsRequest({ + source: { path: src }, + breakpoints: [{ line: 26 }], + }); + await dc.continue({ threadId: -1 }, 'breakpoint', { line: 26 }); + + expect(eventCounter).to.deep.equal({ + stopped: 3, + continued: 2, + }); + } + }; + } + + it('can attach and continue stopped threads', makeRunArgTest('always')); + it( + 'can attach without continuing stopped threads', + makeRunArgTest('preserve') + ); + it('can attach and hit a breakpoint with no program specified', async function () { if (isRemoteTest) { // attachRemote.spec.ts is the test for when isRemoteTest diff --git a/src/integration-tests/attachRemote.spec.ts b/src/integration-tests/attachRemote.spec.ts index 0d26f76c..b788b080 100644 --- a/src/integration-tests/attachRemote.spec.ts +++ b/src/integration-tests/attachRemote.spec.ts @@ -23,6 +23,7 @@ import { gdbAsync, fillDefaults, gdbNonStop, + isRemoteTest, } from './utils'; import { expect } from 'chai'; import { DebugProtocol } from '@vscode/debugprotocol'; @@ -81,6 +82,93 @@ describe('attach remote', function () { expect(await dc.evaluate('argv[1]')).to.contain('running-from-spawn'); }); + function makeRunArgTest(runArg: string) { + return async function (this: Mocha.Context) { + if (!isRemoteTest) { + // attach.spec.ts is the test for when !isRemoteTest + this.skip(); + } + if ((!gdbAsync || !gdbNonStop) && runArg === 'always') { + // in sync mode when all threads are running we can't ask '-thread-info' + this.skip(); + } + + const eventCounter = { stopped: 0, continued: 0 }; + dc.on('stopped', () => { + eventCounter.stopped++; + }); + dc.on('continued', () => { + eventCounter.continued++; + }); + + const attachArgs = fillDefaults(this.test, { + program: program, + target: { + type: 'remote', + parameters: [`localhost:${port}`], + } as TargetAttachArguments, + run: runArg, + } as TargetAttachRequestArguments); + + await Promise.all([ + dc + .waitForEvent('initialized') + .then(() => dc.configurationDoneRequest()), + dc.initializeRequest().then(() => dc.attachRequest(attachArgs)), + ]); + + const threadInfo = JSON.parse( + ( + await dc.evaluateRequest({ + expression: '>-thread-info', + context: 'repl', + }) + ).body.result + ); + const threadStates = threadInfo.threads.map((t: any) => t.state); + if (runArg === 'always') { + expect(threadStates).to.contain('running'); + expect(threadStates).not.to.contain('stopped'); + } else if (runArg === 'preserve') { + expect(threadStates).to.contain('stopped'); + } else { + expect(runArg).to.be.oneOf(['always', 'preserve']); + } + + expect(eventCounter).to.deep.equal({ + stopped: runArg === 'always' ? 0 : 1, + continued: 0, + }); + + // check that continue sends the right GDB commands + // (always -exec-continue for attach; + // first -exec-run, then -exec-continue for launch) + if (runArg === 'preserve') { + await dc.setBreakpointsRequest({ + source: { path: src }, + breakpoints: [{ line: 25 }], + }); + await dc.continue({ threadId: -1 }, 'breakpoint', { line: 25 }); + await dc.setBreakpointsRequest({ + source: { path: src }, + breakpoints: [{ line: 26 }], + }); + await dc.continue({ threadId: -1 }, 'breakpoint', { line: 26 }); + + expect(eventCounter).to.deep.equal({ + stopped: 3, + continued: 2, + }); + } + }; + } + + it('can attach and continue stopped threads', makeRunArgTest('always')); + it( + 'can attach without continuing stopped threads', + makeRunArgTest('preserve') + ); + it('can attach remote and hit a breakpoint without a program', async function () { if (os.platform() === 'win32') { // win32 host does support this use case diff --git a/src/integration-tests/evaluate.spec.ts b/src/integration-tests/evaluate.spec.ts index 097d6ade..cb20324f 100644 --- a/src/integration-tests/evaluate.spec.ts +++ b/src/integration-tests/evaluate.spec.ts @@ -180,7 +180,7 @@ describe('evaluate request', function () { frameId: scope.frame.id, }); - expect(res2.body.result).eq('\r'); + expect(res2.body.result).matches(/^\{.*\}$/); }); it('should reject entering an invalid MI command', async function () { diff --git a/src/integration-tests/launch.spec.ts b/src/integration-tests/launch.spec.ts index 9f90afa8..4c6de449 100644 --- a/src/integration-tests/launch.spec.ts +++ b/src/integration-tests/launch.spec.ts @@ -18,7 +18,9 @@ import { import { CdtDebugClient } from './debugClient'; import { fillDefaults, + gdbAsync, gdbNonStop, + gdbVersionAtLeast, isRemoteTest, standardBeforeEach, testProgramsDir, @@ -33,6 +35,8 @@ describe('launch', function () { const unicodeProgram = path.join(testProgramsDir, 'bug275-测试'); // the name of this file is short enough to work around https://sourceware.org/bugzilla/show_bug.cgi?id=30618 const unicodeSrc = path.join(testProgramsDir, 'bug275-测试.c'); + const loopForeverProgram = path.join(testProgramsDir, 'loopforever'); + const loopForeverSrc = path.join(testProgramsDir, 'loopforever.c'); beforeEach(async function () { dc = await standardBeforeEach(); @@ -54,6 +58,102 @@ describe('launch', function () { ); }); + function makeRunArgTest(runArg: string) { + return async function (this: Mocha.Context) { + // This tests both local and remote cases and does not need to be + // duplicated in launchRemote.spec.ts (beforeEach and afterEach are + // similar enough here and there). + const isAsync = + gdbAsync && + (os.platform() !== 'win32' || + isRemoteTest || + (await gdbVersionAtLeast('13.0'))); + if ( + (!isAsync || (isRemoteTest && !gdbNonStop)) && + runArg === 'always' + ) { + // in sync mode when all threads are running we can't ask '-thread-info' + // (remote needs non-stop to be really async) + this.skip(); + } + + const eventCounter = { stopped: 0, continued: 0 }; + dc.on('stopped', () => { + eventCounter.stopped++; + }); + dc.on('continued', () => { + eventCounter.continued++; + }); + + const launchArgs = fillDefaults(this.test, { + program: loopForeverProgram, + run: runArg, + } as LaunchRequestArguments); + + await Promise.all([ + dc + .waitForEvent('initialized') + .then(() => dc.configurationDoneRequest()), + dc.initializeRequest().then(() => dc.launchRequest(launchArgs)), + ]); + + const threadInfo = JSON.parse( + ( + await dc.evaluateRequest({ + expression: '>-thread-info', + context: 'repl', + }) + ).body.result + ); + const threadStates = threadInfo.threads.map((t: any) => t.state); + if (runArg === 'always') { + expect(threadStates).to.contain('running'); + expect(threadStates).not.to.contain('stopped'); + } else if (runArg === 'preserve') { + if (isRemoteTest) { + // GDBTargetDebugSession interprets "launch" as "launch + // gdbserver", not "launch the program", so this case actually + // behaves like an attach, not like a launch. + expect(threadStates).to.contain('stopped'); + } else { + // GDBDebugSessionBase implements "launch" as expected. + expect(threadStates).to.be.an('array').that.is.empty; + } + } else { + expect(runArg).to.be.oneOf(['always', 'preserve']); + } + + expect(eventCounter).to.deep.equal({ + stopped: isRemoteTest ? (runArg === 'always' ? 0 : 1) : 0, + continued: 0, + }); + + // check that continue sends the right GDB commands + // (always -exec-continue for attach; + // first -exec-run, then -exec-continue for launch) + if (runArg === 'preserve') { + await dc.setBreakpointsRequest({ + source: { path: loopForeverSrc }, + breakpoints: [{ line: 25 }], + }); + await dc.continue({ threadId: -1 }, 'breakpoint', { line: 25 }); + await dc.setBreakpointsRequest({ + source: { path: loopForeverSrc }, + breakpoints: [{ line: 26 }], + }); + await dc.continue({ threadId: -1 }, 'breakpoint', { line: 26 }); + + expect(eventCounter).to.deep.equal({ + stopped: (isRemoteTest ? 1 : 0) + 2, + continued: 2, + }); + } + }; + } + + it('can launch and run', makeRunArgTest('always')); + it('can launch without running', makeRunArgTest('preserve')); + it('receives an error when no port is provided nor a suitable regex', async function () { if (!isRemoteTest) { this.skip(); diff --git a/src/types/session.ts b/src/types/session.ts index a106e744..caa1a916 100644 --- a/src/types/session.ts +++ b/src/types/session.ts @@ -10,6 +10,11 @@ import { Response } from '@vscode/debugadapter'; import { DebugProtocol } from '@vscode/debugprotocol'; +export const enum RequestArgRun { + ALWAYS = 'always', + PRESERVE = 'preserve', +} + export interface RequestArguments extends DebugProtocol.LaunchRequestArguments { gdb?: string; gdbArguments?: string[]; @@ -30,6 +35,7 @@ export interface RequestArguments extends DebugProtocol.LaunchRequestArguments { customResetCommands?: string[]; steppingResponseTimeout?: number; updateThreadInfo?: 'missing' | 'when-requested' | 'never'; + run?: RequestArgRun; } export interface LaunchRequestArguments extends RequestArguments {