Skip to content

Commit 6cf5b32

Browse files
asimgunesasimgunes
committed
Improving the DisassembleRequest logic
1 parent b3a5eb6 commit 6cf5b32

8 files changed

+605
-140
lines changed

‎package.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@
6363
},
6464
"homepage": "https://github.com/eclipse-cdt-cloud/cdt-gdb-adapter#readme",
6565
"dependencies": {
66-
"@vscode/debugadapter": "^1.59.0",
67-
"@vscode/debugprotocol": "^1.59.0",
66+
"@vscode/debugadapter": "^1.68.0",
67+
"@vscode/debugprotocol": "^1.68.0",
6868
"node-addon-api": "^4.3.0",
6969
"serialport": "11.0.0",
7070
"utf8": "^3.0.0"

‎src/gdb/GDBDebugSessionBase.ts

+32-96
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ import {
3939
CDTDisassembleArguments,
4040
} from '../types/session';
4141
import { IGDBBackend, IGDBBackendFactory } from '../types/gdb';
42+
import { getInstructions } from '../util/disassembly';
43+
import { calculateMemoryOffset } from '../util/calculateMemoryOffset';
4244

4345
class ThreadWithStatus implements DebugProtocol.Thread {
4446
id: number;
@@ -1344,108 +1346,42 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession {
13441346
args: CDTDisassembleArguments
13451347
) {
13461348
try {
1347-
const meanSizeOfInstruction = 4;
1348-
let startOffset = 0;
1349-
let lastStartOffset = -1;
1349+
const instructionStartOffset = args.instructionOffset ?? 0;
1350+
const instructionEndOffset =
1351+
args.instructionCount + instructionStartOffset;
13501352
const instructions: DebugProtocol.DisassembledInstruction[] = [];
1351-
let oneIterationOnly = false;
1352-
outer_loop: while (
1353-
instructions.length < args.instructionCount &&
1354-
!oneIterationOnly
1355-
) {
1356-
if (startOffset === lastStartOffset) {
1357-
// We have stopped getting new instructions, give up
1358-
break outer_loop;
1359-
}
1360-
lastStartOffset = startOffset;
1361-
1362-
const fetchSize =
1363-
(args.instructionCount - instructions.length) *
1364-
meanSizeOfInstruction;
1365-
1366-
// args.memoryReference is an arbitrary expression, so let GDB do the
1367-
// math on resolving value rather than doing the addition in the adapter
1368-
try {
1369-
const stepStartAddress = `(${args.memoryReference})+${startOffset}`;
1370-
let stepEndAddress = `(${args.memoryReference})+${startOffset}+${fetchSize}`;
1371-
if (args.endMemoryReference && instructions.length === 0) {
1372-
// On the first call, if we have an end memory address use it instead of
1373-
// the approx size
1374-
stepEndAddress = args.endMemoryReference;
1375-
oneIterationOnly = true;
1376-
}
1377-
const result = await mi.sendDataDisassemble(
1378-
this.gdb,
1379-
stepStartAddress,
1380-
stepEndAddress
1381-
);
1382-
for (const asmInsn of result.asm_insns) {
1383-
const line: number | undefined = asmInsn.line
1384-
? parseInt(asmInsn.line, 10)
1385-
: undefined;
1386-
const source = {
1387-
name: asmInsn.file,
1388-
path: asmInsn.fullname,
1389-
} as DebugProtocol.Source;
1390-
for (const asmLine of asmInsn.line_asm_insn) {
1391-
let funcAndOffset: string | undefined;
1392-
if (asmLine['func-name'] && asmLine.offset) {
1393-
funcAndOffset = `${asmLine['func-name']}+${asmLine.offset}`;
1394-
} else if (asmLine['func-name']) {
1395-
funcAndOffset = asmLine['func-name'];
1396-
} else {
1397-
funcAndOffset = undefined;
1398-
}
1399-
const disInsn = {
1400-
address: asmLine.address,
1401-
instructionBytes: asmLine.opcodes,
1402-
instruction: asmLine.inst,
1403-
symbol: funcAndOffset,
1404-
location: source,
1405-
line,
1406-
} as DebugProtocol.DisassembledInstruction;
1407-
instructions.push(disInsn);
1408-
if (instructions.length === args.instructionCount) {
1409-
break outer_loop;
1410-
}
1353+
const memoryReference =
1354+
args.offset === undefined
1355+
? args.memoryReference
1356+
: calculateMemoryOffset(args.memoryReference, args.offset);
1357+
1358+
if (instructionStartOffset < 0) {
1359+
// Getting lower memory area
1360+
const list = await getInstructions(
1361+
this.gdb,
1362+
memoryReference,
1363+
instructionStartOffset
1364+
);
14111365

1412-
const bytes = asmLine.opcodes.replace(/\s/g, '');
1413-
startOffset += bytes.length;
1414-
}
1415-
}
1416-
} catch (err) {
1417-
// Failed to read instruction -- what best to do here?
1418-
// in other words, whose responsibility (adapter or client)
1419-
// to reissue reads in smaller chunks to find good memory
1420-
while (instructions.length < args.instructionCount) {
1421-
const badDisInsn = {
1422-
// TODO this should start at byte after last retrieved address
1423-
address: `0x${startOffset.toString(16)}`,
1424-
instruction:
1425-
err instanceof Error
1426-
? err.message
1427-
: String(err),
1428-
} as DebugProtocol.DisassembledInstruction;
1429-
instructions.push(badDisInsn);
1430-
startOffset += 2;
1431-
}
1432-
break outer_loop;
1433-
}
1366+
// Add them to instruction list
1367+
instructions.push(...list);
14341368
}
14351369

1436-
if (!args.endMemoryReference) {
1437-
while (instructions.length < args.instructionCount) {
1438-
const badDisInsn = {
1439-
// TODO this should start at byte after last retrieved address
1440-
address: `0x${startOffset.toString(16)}`,
1441-
instruction: 'failed to retrieve instruction',
1442-
} as DebugProtocol.DisassembledInstruction;
1443-
instructions.push(badDisInsn);
1444-
startOffset += 2;
1445-
}
1370+
if (instructionEndOffset > 0) {
1371+
// Getting higher memory area
1372+
const list = await getInstructions(
1373+
this.gdb,
1374+
memoryReference,
1375+
instructionEndOffset
1376+
);
1377+
1378+
// Add them to instruction list
1379+
instructions.push(...list);
14461380
}
14471381

1448-
response.body = { instructions };
1382+
response.body = {
1383+
instructions,
1384+
};
14491385
this.sendResponse(response);
14501386
} catch (err) {
14511387
this.sendErrorResponse(

‎src/integration-tests/diassemble.spec.ts

+106-33
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,36 @@ import * as path from 'path';
1313
import { DebugProtocol } from '@vscode/debugprotocol/lib/debugProtocol';
1414
import { CdtDebugClient } from './debugClient';
1515
import { fillDefaults, standardBeforeEach, testProgramsDir } from './utils';
16+
import { calculateMemoryOffset } from '../util/calculateMemoryOffset';
1617

1718
describe('Disassembly Test Suite', function () {
1819
let dc: CdtDebugClient;
1920
let frame: DebugProtocol.StackFrame;
2021
const disProgram = path.join(testProgramsDir, 'disassemble');
2122
const disSrc = path.join(testProgramsDir, 'disassemble.c');
2223

24+
const expectsGeneralDisassemble = (
25+
disassemble: DebugProtocol.DisassembleResponse,
26+
length: number,
27+
ignoreEmptyInstructions?: boolean
28+
) => {
29+
expect(disassemble).not.eq(undefined);
30+
expect(disassemble.body).not.eq(undefined);
31+
if (disassemble.body) {
32+
const instructions = disassemble.body.instructions;
33+
expect(instructions).to.have.lengthOf(length);
34+
// the contents of the instructions are platform dependent, so instead
35+
// make sure we have read fully
36+
for (const i of instructions) {
37+
expect(i.address).to.have.lengthOf.greaterThan(0);
38+
expect(i.instruction).to.have.lengthOf.greaterThan(0);
39+
if (!ignoreEmptyInstructions) {
40+
expect(i.instructionBytes).to.have.lengthOf.greaterThan(0);
41+
}
42+
}
43+
}
44+
};
45+
2346
beforeEach(async function () {
2447
dc = await standardBeforeEach();
2548

@@ -60,19 +83,8 @@ describe('Disassembly Test Suite', function () {
6083
memoryReference: 'main',
6184
instructionCount: 100,
6285
})) as DebugProtocol.DisassembleResponse;
63-
expect(disassemble).not.eq(undefined);
64-
expect(disassemble.body).not.eq(undefined);
65-
if (disassemble.body) {
66-
const instructions = disassemble.body.instructions;
67-
expect(instructions).to.have.lengthOf(100);
68-
// the contents of the instructions are platform dependent, so instead
69-
// make sure we have read fully
70-
for (const i of instructions) {
71-
expect(i.address).to.have.lengthOf.greaterThan(0);
72-
expect(i.instructionBytes).to.have.lengthOf.greaterThan(0);
73-
expect(i.instruction).to.have.lengthOf.greaterThan(0);
74-
}
75-
}
86+
87+
expectsGeneralDisassemble(disassemble, 100);
7688
});
7789

7890
it('can disassemble with no source references', async function () {
@@ -82,38 +94,99 @@ describe('Disassembly Test Suite', function () {
8294
memoryReference: 'main+1000',
8395
instructionCount: 100,
8496
})) as DebugProtocol.DisassembleResponse;
85-
expect(disassemble).not.eq(undefined);
86-
expect(disassemble.body).not.eq(undefined);
87-
if (disassemble.body) {
88-
const instructions = disassemble.body.instructions;
89-
expect(instructions).to.have.lengthOf(100);
90-
// the contents of the instructions are platform dependent, so instead
91-
// make sure we have read fully
92-
for (const i of instructions) {
93-
expect(i.address).to.have.lengthOf.greaterThan(0);
94-
expect(i.instructionBytes).to.have.lengthOf.greaterThan(0);
95-
expect(i.instruction).to.have.lengthOf.greaterThan(0);
96-
}
97-
}
97+
98+
expectsGeneralDisassemble(disassemble, 100);
99+
});
100+
101+
it('can disassemble with negative offsets', async function () {
102+
// In this case we attempt to read from where there is no source,
103+
// GDB returns data in a different format in that case
104+
const disassemble = (await dc.send('disassemble', {
105+
memoryReference: 'main',
106+
instructionOffset: -20,
107+
instructionCount: 20,
108+
} as DebugProtocol.DisassembleArguments)) as DebugProtocol.DisassembleResponse;
109+
110+
expectsGeneralDisassemble(disassemble, 20, true);
111+
});
112+
113+
it('can disassemble with correct boundries', async function () {
114+
const get = (
115+
disassemble: DebugProtocol.DisassembleResponse,
116+
offset: number
117+
) => {
118+
const instruction = disassemble.body?.instructions[offset];
119+
expect(instruction).not.eq(undefined);
120+
// Instruction undefined already checked.
121+
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
122+
return instruction!;
123+
};
124+
125+
const expectsInstructionEquals = (
126+
instruction1: DebugProtocol.DisassembledInstruction,
127+
instruction2: DebugProtocol.DisassembledInstruction,
128+
message?: string
129+
) => {
130+
expect(instruction1.address).to.eq(instruction2.address, message);
131+
};
132+
133+
// In this case we attempt to read from where there is no source,
134+
// GDB returns data in a different format in that case
135+
const disassembleLower = (await dc.send('disassemble', {
136+
memoryReference: 'main',
137+
instructionOffset: -20,
138+
instructionCount: 20,
139+
} as DebugProtocol.DisassembleArguments)) as DebugProtocol.DisassembleResponse;
140+
const disassembleMiddle = (await dc.send('disassemble', {
141+
memoryReference: 'main',
142+
instructionOffset: -10,
143+
instructionCount: 20,
144+
} as DebugProtocol.DisassembleArguments)) as DebugProtocol.DisassembleResponse;
145+
const disassembleHigher = (await dc.send('disassemble', {
146+
memoryReference: 'main',
147+
instructionOffset: 0,
148+
instructionCount: 20,
149+
} as DebugProtocol.DisassembleArguments)) as DebugProtocol.DisassembleResponse;
150+
151+
expectsGeneralDisassemble(disassembleLower, 20, true);
152+
expectsGeneralDisassemble(disassembleMiddle, 20, true);
153+
expectsGeneralDisassemble(disassembleHigher, 20, true);
154+
155+
expectsInstructionEquals(
156+
get(disassembleLower, 15),
157+
get(disassembleMiddle, 5),
158+
'lower[15] should be same with middle[5]'
159+
);
160+
161+
expectsInstructionEquals(
162+
get(disassembleMiddle, 15),
163+
get(disassembleHigher, 5),
164+
'middle[15] should be same with higher[5]'
165+
);
98166
});
99167

100168
it('can handle disassemble at bad address', async function () {
101169
const disassemble = (await dc.send('disassemble', {
102170
memoryReference: '0x0',
103171
instructionCount: 10,
104172
})) as DebugProtocol.DisassembleResponse;
173+
105174
expect(disassemble).not.eq(undefined);
106175
expect(disassemble.body).not.eq(undefined);
107176
if (disassemble.body) {
108177
const instructions = disassemble.body.instructions;
109178
expect(instructions).to.have.lengthOf(10);
110-
// the contens of the instructions are platform dependent, so instead
111-
// make sure we have read fully
112-
for (const i of instructions) {
113-
expect(i.address).to.have.lengthOf.greaterThan(0);
114-
expect(i.instruction).to.have.lengthOf.greaterThan(0);
115-
expect(i.instructionBytes).eq(undefined);
116-
}
179+
// Checking the invalid instructions content
180+
instructions.forEach((inst, ix) => {
181+
expect(inst.address).to.eq(
182+
calculateMemoryOffset('0x0', ix * 2)
183+
);
184+
expect(inst.address).to.have.lengthOf.greaterThan(0);
185+
expect(inst.instruction).to.eq(
186+
'failed to retrieve instruction'
187+
);
188+
expect(inst.presentationHint).to.eq('invalid');
189+
});
117190
}
118191
});
119192
});

0 commit comments

Comments
 (0)