From 511b4e9eba6bcc49147664d664777371dbb52c76 Mon Sep 17 00:00:00 2001 From: WyoTwT Date: Tue, 5 Dec 2023 23:38:35 +0100 Subject: [PATCH] Remove parenthesis cleanup & extra getFullPathExpression() calls The total removal of parenthesis removes casting parenthesis and breaks potential watch expressions so this cleanup should be removed. Instead of calling getFullPathExpression() multiple times, build the fullPath from the topLevelExpression variable. In the case of Expressions, add encapsulating parenthesis to handle type casting properly. Update variable casting tests accordingly. --- src/GDBDebugSession.ts | 35 ++++---- src/integration-tests/test-programs/vars.c | 1 + src/integration-tests/var.spec.ts | 93 +++++++++++++++++++--- 3 files changed, 102 insertions(+), 27 deletions(-) diff --git a/src/GDBDebugSession.ts b/src/GDBDebugSession.ts index 5967869f..d144e489 100644 --- a/src/GDBDebugSession.ts +++ b/src/GDBDebugSession.ts @@ -2088,10 +2088,15 @@ export class GDBDebugSession extends LoggingDebugSession { }); } // Grab the full path of parent. - const topLevelPathExpression = + const tempTopLevelPathExpression = varobj?.expression ?? (await this.getFullPathExpression(parentVarname)); + // In the case of Expressions, there might be casting, so add outer () + const topLevelPathExpression = varobj?.isVar + ? tempTopLevelPathExpression + : `(${tempTopLevelPathExpression})`; + // iterate through the children for (const child of children.children) { // check if we're dealing with a C++ object. If we are, we need to fetch the grandchildren instead. @@ -2123,8 +2128,9 @@ export class GDBDebugSession extends LoggingDebugSession { } } else { // check if we're dealing with an array - let name = `${ref.varobjName}.${child.exp}`; - let varobjName = name; + let variableName = child.exp; + let varobjName = `${ref.varobjName}.${child.exp}`; + let fullPath = `${topLevelPathExpression}.${variableName}`; let value = child.value ? child.value : child.type; const isArrayParent = arrayRegex.test(child.type); const isArrayChild = @@ -2132,16 +2138,14 @@ export class GDBDebugSession extends LoggingDebugSession { ? arrayRegex.test(varobj.type) && arrayChildRegex.test(child.exp) : false; - if (isArrayChild) { - // update the display name for array elements to have square brackets - name = `[${child.exp}]`; - } if (isArrayParent || isArrayChild) { // can't use a relative varname (eg. var1.a.b.c) to create/update a new var so fetch and track these // vars by evaluating their path expression from GDB - const fullPath = await this.getFullPathExpression( - child.name - ); + if (isArrayChild) { + // update the display name for array elements to have square brackets + variableName = `[${child.exp}]`; + fullPath = `(${topLevelPathExpression})${variableName}`; + } // create or update the var in GDB let arrobj = this.gdb.varManager.getVar( frame.frameId, @@ -2182,14 +2186,9 @@ export class GDBDebugSession extends LoggingDebugSession { arrobj.isChild = true; varobjName = arrobj.varname; } - const variableName = isArrayChild ? name : child.exp; - const evaluateName = - isArrayParent || isArrayChild - ? await this.getFullPathExpression(child.name) - : `${topLevelPathExpression}.${child.exp}`; variables.push({ name: variableName, - evaluateName, + evaluateName: fullPath, value, type: child.type, variablesReference: @@ -2212,8 +2211,8 @@ export class GDBDebugSession extends LoggingDebugSession { this.gdb, inputVarName ); - // result from GDB looks like (parentName).field so remove (). - return exprResponse.path_expr.replace(/[()]/g, ''); + // GDB result => (parentName).field + return exprResponse.path_expr; } // Register view diff --git a/src/integration-tests/test-programs/vars.c b/src/integration-tests/test-programs/vars.c index a3168d67..075c8a9b 100644 --- a/src/integration-tests/test-programs/vars.c +++ b/src/integration-tests/test-programs/vars.c @@ -32,5 +32,6 @@ int main() int rax = 1; const unsigned char h[] = {0x01, 0x10, 0x20}; const unsigned char k[] = "hello"; // char string setup + r.z.a = 10; // update embedded int return 0; } diff --git a/src/integration-tests/var.spec.ts b/src/integration-tests/var.spec.ts index 0c4fe302..90747e36 100644 --- a/src/integration-tests/var.spec.ts +++ b/src/integration-tests/var.spec.ts @@ -12,7 +12,6 @@ import { expect } from 'chai'; import * as path from 'path'; import { CdtDebugClient } from './debugClient'; import { - expectRejection, getScopes, resolveLineTagLocations, Scope, @@ -37,6 +36,7 @@ describe('Variables Test Suite', function () { 'STOP HERE': 0, 'After array init': 0, 'char string setup': 0, + 'update embedded int': 0, }; const hexValueRegex = /^0x[\da-fA-F]+$/; @@ -557,7 +557,7 @@ describe('Variables Test Suite', function () { ); }); - it('catch error from casting of struct to a character array', async function () { + it('support watch a character array', async function () { // skip ahead to array initialization const br = await dc.setBreakpointsRequest({ source: { path: varsSrc }, @@ -586,13 +586,88 @@ describe('Variables Test Suite', function () { }); expect(res2.body.result).eq('[6]'); // Call handleVariableRequestObject() on above using returned variablesReference. - const error = await expectRejection( - dc.variablesRequest({ - variablesReference: res2.body.variablesReference, - }) - ); - expect(error.message).contains( - '-var-create: unable to create variable object' + const vars = await dc.variablesRequest({ + variablesReference: res2.body.variablesReference, + }); + expect( + vars.body.variables.length, + 'There is a different number of variables than expected' + ).to.equal(6); + verifyVariable(vars.body.variables[0], '[0]', 'char', "104 'h'", { + hasMemoryReference: false, + }); + verifyVariable(vars.body.variables[1], '[1]', 'char', "101 'e'", { + hasMemoryReference: false, + }); + }); + + it.only('support watch of char cast of an int in complex structure', async function () { + // skip ahead to array initialization + const br = await dc.setBreakpointsRequest({ + source: { path: varsSrc }, + breakpoints: [{ line: lineTags['update embedded int'] }], + }); + expect(br.success).to.equal(true); + await dc.continue({ threadId: scope.thread.id }, 'breakpoint', { + line: lineTags['update embedded int'], + path: varsSrc, + }); + scope = await getScopes(dc); + expect( + scope.scopes.body.scopes.length, + 'Unexpected number of scopes returned' + ).to.equal(2); + // Setup the new variable to be evaluated. + const res = await dc.evaluateRequest({ + context: 'watch', + expression: '(char[])r.z.a', + frameId: scope.frame.id, + }); + expect(res.body.result).eq('[4]'); + // Call handleVariableRequestObject() on above using returned variablesReference. + let vars = await dc.variablesRequest({ + variablesReference: res.body.variablesReference, + }); + expect( + vars.body.variables.length, + 'There is a different number of variables than expected' + ).to.equal(4); + verifyVariable(vars.body.variables[0], '[0]', 'char', "3 '\\003'", { + hasMemoryReference: false, + }); + verifyVariable(vars.body.variables[1], '[1]', 'char', "0 '\\000'", { + hasMemoryReference: false, + }); + // step the program and see that the values were passed to the program and evaluated. + await dc.next( + { threadId: scope.thread.id }, + { path: varsSrc, line: lineTags['update embedded int'] + 1 } ); + scope = await getScopes(dc); + expect( + scope.scopes.body.scopes.length, + 'Unexpected number of scopes returned' + ).to.equal(2); + // Setup the updated embedded variable to be evaluated. + const res2 = await dc.evaluateRequest({ + context: 'watch', + expression: '(char[])r.z.a', + frameId: scope.frame.id, + }); + expect(res2.body.result).eq('[4]'); + // Call handleVariableRequestObject() on above using returned variablesReference. + vars = await dc.variablesRequest({ + variablesReference: res2.body.variablesReference, + }); + expect( + vars.body.variables.length, + 'There is a different number of variables than expected' + ).to.equal(4); + verifyVariable(vars.body.variables[0], '[0]', 'char', "10 '\\n'", { + hasMemoryReference: false, + }); + verifyVariable(vars.body.variables[1], '[1]', 'char', "0 '\\000'", { + hasMemoryReference: false, + }); }); });