Add threadId and FrameId as optional parameters

The GDB MI commands can take a threadId and frameId if specified so add that capability as an optional parameter. Add unit test to verify threadId and frameId changes. Multithread test is needed to showcase this change.
eclipse-cdt-cloud · Aug 23, 2024 · 225072f · 225072f
1 parent cd740cd
commit 225072f
Show file tree

Hide file tree

Showing 6 changed files with 214 additions and 16 deletions.
diff --git a/src/GDBDebugSession.ts b/src/GDBDebugSession.ts
@@ -117,6 +117,11 @@ export interface CDTDisassembleArguments
     endMemoryReference: string;
 }
 
+export interface ThreadContext {
+    threadId: number;
+    frameId: number;
+}
+
 class ThreadWithStatus implements DebugProtocol.Thread {
     id: number;
     name: string;
@@ -279,6 +284,11 @@ export class GDBDebugSession extends LoggingDebugSession {
     ): void {
         if (command === 'cdt-gdb-adapter/Memory') {
             this.memoryRequest(response as MemoryResponse, args);
+        } else if (command === 'cdt-gdb-adapter/readMemoryWithContext') {
+            this.readMemoryWithContextRequest(
+                response as DebugProtocol.ReadMemoryResponse,
+                args
+            );
             // This custom request exists to allow tests in this repository to run arbitrary commands
             // Use at your own risk!
         } else if (command === 'cdt-gdb-tests/executeCommand') {
@@ -1177,6 +1187,7 @@ export class GDBDebugSession extends LoggingDebugSession {
                 args.name.replace(/^\[(\d+)\]/, '$1');
             const stackDepth = await mi.sendStackInfoDepth(this.gdb, {
                 maxDepth: 100,
+                threadId: frame.threadId,
             });
             const depth = parseInt(stackDepth.depth, 10);
             let varobj = this.gdb.varManager.getVar(
@@ -1321,6 +1332,7 @@ export class GDBDebugSession extends LoggingDebugSession {
 
             const stackDepth = await mi.sendStackInfoDepth(this.gdb, {
                 maxDepth: 100,
+                threadId: frame.threadId,
             });
             const depth = parseInt(stackDepth.depth, 10);
             let varobj = this.gdb.varManager.getVar(
@@ -1622,17 +1634,20 @@ export class GDBDebugSession extends LoggingDebugSession {
         }
     }
 
-    protected async readMemoryRequest(
+    protected async readMemoryWithContextRequest(
         response: DebugProtocol.ReadMemoryResponse,
-        args: DebugProtocol.ReadMemoryArguments
+        fullArgs: [DebugProtocol.ReadMemoryArguments, ThreadContext?]
     ): Promise<void> {
+        const [args, context] = fullArgs;
         try {
             if (args.count) {
                 const result = await sendDataReadMemoryBytes(
                     this.gdb,
                     args.memoryReference,
                     args.count,
-                    args.offset
+                    args.offset,
+                    context?.threadId,
+                    context?.frameId
                 );
                 response.body = {
                     data: hexToBase64(result.memory[0].contents),
@@ -1651,6 +1666,13 @@ export class GDBDebugSession extends LoggingDebugSession {
         }
     }
 
+    protected async readMemoryRequest(
+        response: DebugProtocol.ReadMemoryResponse,
+        args: DebugProtocol.ReadMemoryArguments
+    ): Promise<void> {
+        return this.readMemoryWithContextRequest(response, [args, undefined]);
+    }
+
     /**
      * Implement the memoryWrite request.
      */
@@ -1908,6 +1930,7 @@ export class GDBDebugSession extends LoggingDebugSession {
         // stack depth necessary for differentiating between similarly named variables at different stack depths
         const stackDepth = await mi.sendStackInfoDepth(this.gdb, {
             maxDepth: 100,
+            threadId: frame.threadId,
         });
         const depth = parseInt(stackDepth.depth, 10);
 
@@ -2061,6 +2084,7 @@ export class GDBDebugSession extends LoggingDebugSession {
         // fetch stack depth to obtain frameId/threadId/depth tuple
         const stackDepth = await mi.sendStackInfoDepth(this.gdb, {
             maxDepth: 100,
+            threadId: frame.threadId,
         });
         const depth = parseInt(stackDepth.depth, 10);
         // we need to keep track of children and the parent varname in GDB

diff --git a/src/integration-tests/debugClient.ts b/src/integration-tests/debugClient.ts
@@ -14,6 +14,7 @@ import * as path from 'path';
 import { defaultAdapter } from './utils';
 import * as os from 'os';
 import { expect } from 'chai';
+import { ThreadContext } from '../GDBDebugSession';
 
 export type ReverseRequestHandler<
     A = any,
@@ -335,6 +336,12 @@ export class CdtDebugClient extends DebugClient {
         return this.send('readMemory', args);
     }
 
+    public readMemoryWithContextRequest(
+        args: [DebugProtocol.ReadMemoryArguments, ThreadContext?]
+    ): Promise<DebugProtocol.ReadMemoryResponse> {
+        return this.send('cdt-gdb-adapter/readMemoryWithContext', args);
+    }
+
     public writeMemoryRequest(
         args: DebugProtocol.WriteMemoryArguments
     ): Promise<DebugProtocol.WriteMemoryResponse> {

diff --git a/src/integration-tests/multithread.spec.ts b/src/integration-tests/multithread.spec.ts
@@ -20,6 +20,15 @@ import { expect } from 'chai';
 import * as path from 'path';
 import { fail } from 'assert';
 import * as os from 'os';
+import { ThreadContext, base64ToHex } from '../GDBDebugSession';
+import { DebugProtocol } from '@vscode/debugprotocol';
+
+interface VariableContext {
+    name: string;
+    threadId: number;
+    varAddress: number;
+    stackFramePosition: number;
+}
 
 describe('multithread', async function () {
     let dc: CdtDebugClient;
@@ -50,6 +59,29 @@ describe('multithread', async function () {
         await dc.stop();
     });
 
+    /**
+     * Verify that `resp` contains the bytes `expectedBytes` and the
+     * `expectedAddress` start address matches. In this case we know
+     * we're searching for a string so truncate after 0 byte.
+     *
+     */
+    function verifyReadMemoryResponse(
+        resp: DebugProtocol.ReadMemoryResponse,
+        expectedBytes: string,
+        expectedAddress: number
+    ) {
+        const memData = base64ToHex(resp.body?.data ?? '').toString();
+        const memString = Buffer.from(memData.toString(), 'hex').toString();
+        // Only use the data before the 0 byte (truncate after)
+        const simpleString = memString.substring(0, memString.search(/\0/));
+        expect(simpleString).eq(expectedBytes);
+        expect(resp.body?.address).match(/^0x[0-9a-fA-F]+$/);
+        if (resp.body?.address) {
+            const actualAddress = parseInt(resp.body?.address);
+            expect(actualAddress).eq(expectedAddress);
+        }
+    }
+
     it('sees all threads', async function () {
         if (!gdbNonStop && os.platform() === 'win32' && isRemoteTest) {
             // The way thread names are set in remote tests on windows is unsupported
@@ -182,4 +214,85 @@ describe('multithread', async function () {
             }
         }
     });
+
+    it('verify threadId,frameID for multiple threads', async function () {
+        if (!gdbNonStop && os.platform() === 'win32' && isRemoteTest) {
+            // The way thread names are set in remote tests on windows is unsupported
+            this.skip();
+        }
+
+        await dc.hitBreakpoint(
+            fillDefaults(this.test, {
+                program: program,
+            }),
+            {
+                path: source,
+                line: lineTags['LINE_MAIN_ALL_THREADS_STARTED'],
+            }
+        );
+
+        const variableContextArray: VariableContext[] = [];
+        const threads = await dc.threadsRequest();
+        // cycle through the threads and create an index for later
+        for (const threadInfo of threads.body.threads) {
+            // threadId is the id of the thread in DAP
+            const threadId = threadInfo.id;
+            if (threadId === undefined) {
+                // Shouldn't have undefined thread.
+                fail('unreachable');
+            }
+            if (!(threadInfo.name in threadNames)) {
+                continue;
+            }
+
+            if (gdbNonStop) {
+                const waitForStopped = dc.waitForEvent('stopped');
+                const pr = dc.pauseRequest({ threadId });
+                await Promise.all([pr, waitForStopped]);
+            }
+
+            const stack = await dc.stackTraceRequest({ threadId });
+            let nameAddress: number | undefined = undefined;
+            let stackFramePosition = 0;
+            // Frame Reference ID starts at 1000 but actual stack frame # is index.
+            for (const frame of stack.body.stackFrames) {
+                if (frame.name === 'PrintHello') {
+                    // Grab the address for "name" in this thread now because
+                    // gdb-non-stop doesn't have different frame.id's for threads.
+                    const addrOfVariableResp = await dc.evaluateRequest({
+                        expression: 'name',
+                        frameId: frame.id,
+                    });
+                    nameAddress = parseInt(addrOfVariableResp.body.result, 16);
+                    break;
+                }
+                stackFramePosition++;
+            }
+            if (nameAddress === undefined) {
+                fail("Failed to find address of name in 'PrintHello'");
+            }
+
+            variableContextArray.push({
+                name: threadInfo.name.toString(),
+                threadId: threadInfo.id,
+                varAddress: nameAddress,
+                stackFramePosition,
+            });
+        }
+        // cycle through the threads and confirm each thread name (different for each thread)
+        for (const context of variableContextArray) {
+            // Get the address of the variable.
+            const mem = await dc.readMemoryWithContextRequest([
+                {
+                    memoryReference: '0x' + context.varAddress.toString(16),
+                    count: 10,
+                },
+                {
+                    threadId: context.threadId,
+                    frameId: context.stackFramePosition,
+                } as ThreadContext,
+            ]);
+            verifyReadMemoryResponse(mem, context.name, context.varAddress);
+        }
+    });
 });
diff --git a/src/mi/data.ts b/src/mi/data.ts
@@ -54,21 +54,37 @@ export function sendDataReadMemoryBytes(
     gdb: GDBBackend,
     address: string,
     size: number,
-    offset = 0
+    offset = 0,
+    threadId?: number,
+    frameId?: number
 ): Promise<MIDataReadMemoryBytesResponse> {
-    return gdb.sendCommand(
-        `-data-read-memory-bytes -o ${offset} "${address}" ${size}`
-    );
+    let command = `-data-read-memory-bytes`;
+    if (threadId !== undefined) {
+        command += ` --thread ${threadId}`;
+    }
+    if (frameId !== undefined) {
+        command += ` --frame ${frameId}`;
+    }
+    command += ` -o ${offset} "${address}" ${size}`;
+    return gdb.sendCommand(command);
 }
 
 export function sendDataWriteMemoryBytes(
     gdb: GDBBackend,
     memoryReference: string,
-    data: string
+    data: string,
+    threadId?: number,
+    frameId?: number
 ): Promise<void> {
-    return gdb.sendCommand(
-        `-data-write-memory-bytes "${memoryReference}" "${data}"`
-    );
+    let command = `-data-write-memory-bytes`;
+    if (threadId !== undefined) {
+        command += ` --thread ${threadId}`;
+    }
+    if (frameId !== undefined) {
+        command += ` --frame ${frameId}`;
+    }
+    command += ` "${memoryReference}" "${data}"`;
+    return gdb.sendCommand(command);
 }
 
 export function sendDataEvaluateExpression(

diff --git a/src/mi/var.ts b/src/mi/var.ts
@@ -142,9 +142,17 @@ export function sendVarUpdate(
             | MIVarPrintValues.no
             | MIVarPrintValues.all
             | MIVarPrintValues.simple;
+        threadId?: number;
+        frameId?: number;
     }
 ): Promise<MIVarUpdateResponse> {
     let command = '-var-update';
+    if (params.threadId !== undefined) {
+        command += ` --thread ${params.threadId}`;
+    }
+    if (params.frameId !== undefined) {
+        command += ` --frame ${params.frameId}`;
+    }
     if (params.printValues) {
         command += ` ${params.printValues}`;
     } else {
@@ -162,9 +170,18 @@ export function sendVarDelete(
     gdb: GDBBackend,
     params: {
         varname: string;
+        threadId?: number;
+        frameId?: number;
     }
 ): Promise<void> {
-    const command = `-var-delete ${params.varname}`;
+    let command = `-var-delete`;
+    if (params.threadId !== undefined) {
+        command += ` --thread ${params.threadId}`;
+    }
+    if (params.frameId !== undefined) {
+        command += ` --frame ${params.frameId}`;
+    }
+    command += ` ${params.varname}`;
     return gdb.sendCommand(command);
 }
 
@@ -183,9 +200,18 @@ export function sendVarEvaluateExpression(
     gdb: GDBBackend,
     params: {
         varname: string;
+        threadId?: number;
+        frameId?: number;
     }
 ): Promise<MIVarEvalResponse> {
-    const command = `-var-evaluate-expression ${params.varname}`;
+    let command = '-var-evaluate-expression';
+    if (params.threadId !== undefined) {
+        command += ` --thread ${params.threadId}`;
+    }
+    if (params.frameId !== undefined) {
+        command += ` --frame ${params.frameId}`;
+    }
+    command += ` ${params.varname}`;
     return gdb.sendCommand(command);
 }
 

diff --git a/src/varManager.ts b/src/varManager.ts
@@ -124,7 +124,11 @@ export class VarManager {
                 }
             }
             if (deleteme) {
-                await sendVarDelete(this.gdb, { varname: deleteme.varname });
+                await sendVarDelete(this.gdb, {
+                    varname: deleteme.varname,
+                    threadId,
+                    frameId,
+                });
                 vars.splice(vars.indexOf(deleteme), 1);
                 for (const child of deleteme.children) {
                     await this.removeVar(
@@ -145,7 +149,11 @@ export class VarManager {
         varobj: VarObjType
     ): Promise<VarObjType> {
         let returnVar = varobj;
-        const vup = await sendVarUpdate(this.gdb, { name: varobj.varname });
+        const vup = await sendVarUpdate(this.gdb, {
+            name: varobj.varname,
+            threadId,
+            frameId,
+        });
         const update = vup.changelist[0];
         if (update) {
             if (update.in_scope === 'true') {
@@ -155,7 +163,11 @@ export class VarManager {
                 }
             } else {
                 this.removeVar(frameId, threadId, depth, varobj.varname);
-                await sendVarDelete(this.gdb, { varname: varobj.varname });
+                await sendVarDelete(this.gdb, {
+                    varname: varobj.varname,
+                    threadId,
+                    frameId,
+                });
                 const createResponse = await sendVarCreate(this.gdb, {
                     frame: 'current',
                     expression: varobj.expression,