Skip pausing target at startup if request has no breakpoints (#467)

In configuration phase, without this commit, breakpoint requests
are still sent to gdb even when there is no breakpoints to set.
When client does any of the various set breakpoint requests,
interrupt and continue command could be sent to gdb and
those commands could change the state of target unexpectedly.
Therefore this change makes it so breakpoint requests don't
pause the target during configuration phase if the request
has no breakpoints.

Co-authored-by: Dat Phan <[email protected]>

Author: datphan9798

src/gdb/GDBDebugSessionBase.ts

@@ -124,6 +124,11 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession {
     protected gdb!: IGDBBackend;
     protected auxGdb?: IGDBBackend;
 
+    protected isBreakpointSent = false;
+    protected isDataBreakpointSent = false;
+    protected isFunctionBreakpointSent = false;
+    protected isInstructionBreakpointSent = false;
+
     protected isAttach = false;
     /**
      * True if GDB is using the "remote" or "extended-remote" target. In
@@ -717,6 +722,19 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession {
         response: DebugProtocol.SetDataBreakpointsResponse,
         args: DebugProtocol.SetDataBreakpointsArguments
     ): Promise<void> {
+        // If this is the first time sending this breakpoint and there are no breakpoints to set,
+        // do not change state of the target by avoiding an unnecessary pause
+        if (!this.isDataBreakpointSent) {
+            if (args.breakpoints.length === 0) {
+                response.body = {
+                    breakpoints: [],
+                };
+                this.sendResponse(response);
+                return;
+            }
+            this.isDataBreakpointSent = true;
+        }
+
         await this.pauseIfNeeded();
         // Get existing GDB watchpoints
         let existingGDBWatchpointsList = await this.getWatchpointList();
@@ -799,6 +817,19 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession {
         response: DebugProtocol.SetInstructionBreakpointsResponse,
         args: DebugProtocol.SetInstructionBreakpointsArguments
     ): Promise<void> {
+        // If this is the first time sending this breakpoint and there are no breakpoints to set,
+        // do not change state of the target by avoiding an unnecessary pause
+        if (!this.isInstructionBreakpointSent) {
+            if (args.breakpoints.length === 0) {
+                response.body = {
+                    breakpoints: [],
+                };
+                this.sendResponse(response);
+                return;
+            }
+            this.isInstructionBreakpointSent = true;
+        }
+
         await this.pauseIfNeeded();
         // Get a list of existing instruction breakpoints
         const existingInstBreakpointsList =
@@ -894,6 +925,22 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession {
         response: DebugProtocol.SetBreakpointsResponse,
         args: DebugProtocol.SetBreakpointsArguments
     ): Promise<void> {
+        // If this is the first time sending this breakpoint and there are no breakpoints to set,
+        // do not change state of the target by avoiding an unnecessary pause
+        if (!this.isBreakpointSent) {
+            if (
+                args.breakpoints !== undefined &&
+                args.breakpoints.length === 0
+            ) {
+                response.body = {
+                    breakpoints: [],
+                };
+                this.sendResponse(response);
+                return;
+            }
+            this.isBreakpointSent = true;
+        }
+
         await this.pauseIfNeeded();
 
         try {
@@ -1115,6 +1162,19 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession {
         response: DebugProtocol.SetFunctionBreakpointsResponse,
         args: DebugProtocol.SetFunctionBreakpointsArguments
     ) {
+        // If this is the first time sending this breakpoint and there are no breakpoints to set,
+        // do not change state of the target by avoiding an unnecessary pause
+        if (!this.isFunctionBreakpointSent) {
+            if (args.breakpoints.length === 0) {
+                response.body = {
+                    breakpoints: [],
+                };
+                this.sendResponse(response);
+                return;
+            }
+            this.isFunctionBreakpointSent = true;
+        }
+
         await this.pauseIfNeeded();
 
         try {

src/integration-tests/breakpoints.spec.ts

@@ -795,4 +795,38 @@ describe('breakpoints', async function () {
         });
         expect(response.body.breakpoints.length).to.eq(1);
     });
+
+    it('empty breakpoint request if no breakpoint exists', async () => {
+        const response = await dc.setBreakpointsRequest({
+            source: {
+                name: 'count.c',
+                path: path.join(testProgramsDir, 'count.c'),
+            },
+            breakpoints: [],
+        });
+        expect(response.body.breakpoints.length).eq(0);
+    });
+
+    it('empty breakpoint request if a breakpoint exists', async () => {
+        await dc.setBreakpointsRequest({
+            source: {
+                name: 'count.c',
+                path: path.join(testProgramsDir, 'count.c'),
+            },
+            breakpoints: [
+                {
+                    column: 1,
+                    line: 4,
+                },
+            ],
+        });
+        const response = await dc.setBreakpointsRequest({
+            source: {
+                name: 'count.c',
+                path: path.join(testProgramsDir, 'count.c'),
+            },
+            breakpoints: [],
+        });
+        expect(response.body.breakpoints.length).eq(0);
+    });
 });

src/integration-tests/functionBreakpoints.spec.ts

@@ -291,4 +291,25 @@ describe('function breakpoints', async function () {
         await dc.configurationDoneRequest();
         await dc.assertStoppedLocation('function breakpoint', { line: 10 });
     });
+
+    it('empty function breakpoint request if no breakpoint exists', async () => {
+        const bpResp = await dc.setFunctionBreakpointsRequest({
+            breakpoints: [],
+        });
+        expect(bpResp.body.breakpoints.length).eq(0);
+    });
+
+    it('empty function breakpoint request if a breakpoint exists', async () => {
+        await dc.setFunctionBreakpointsRequest({
+            breakpoints: [
+                {
+                    name: 'main',
+                },
+            ],
+        });
+        const bpResp = await dc.setFunctionBreakpointsRequest({
+            breakpoints: [],
+        });
+        expect(bpResp.body.breakpoints.length).eq(0);
+    });
 });