Disassembly address handling improvement (#348)

We observe some issues in while using the disassembly in embedded devices due to sending disassemble request for negative memory regions. We implemented a code-update to limit the lower bound of the query to the address 0x0.

If we still need to fill more instructions depending on the DAP request, you may realise that we used relative addresses in the empty instructions (e.g. (0x00)-2, (0x00)-4 etc). This is arguable and I am happy to hear your thoughts. Simply the VSCode is ignoring these lines and not showing in the disassembly window (maybe because they are not convertable to number or bigint).

It was hard to check this behaviour in unit test, thus I implemented a test for the internal function and checked the behaviour over that internal function.

src/integration-tests/util.spec.ts

@@ -14,11 +14,16 @@ import { expect } from 'chai';
 import * as os from 'os';
 import { calculateMemoryOffset } from '../util/calculateMemoryOffset';
 import { MIDataDisassembleAsmInsn } from '../mi';
+import * as midata from '../mi/data';
 import { DebugProtocol } from '@vscode/debugprotocol';
 import {
     getDisassembledInstruction,
     getEmptyInstructions,
+    getInstructions,
 } from '../util/disassembly';
+import { isHexString } from '../util/isHexString';
+import { afterEach, beforeEach } from 'mocha';
+import * as Sinon from 'sinon';
 
 describe('util', async () => {
     it('compareVersions', async () => {
@@ -358,3 +363,98 @@ describe('getEmptyInstructions', () => {
         });
     });
 });
+
+describe('isHexString', () => {
+    it('should return true on valid values', () => {
+        const validValues = [
+            '0x00',
+            '0x0123456789abcdef',
+            '0x0123456789ABCDEF',
+            '0x0000fefe',
+            '0x10000000000000ff',
+        ];
+
+        for (const value of validValues) {
+            const result = isHexString(value);
+            expect(result).to.equals(true, `isHexString for ${value}`);
+        }
+    });
+
+    it('should return false on invalid values', () => {
+        const validValues = [
+            '(0x00)',
+            '0x00+1',
+            '1x00',
+            'x00',
+            '00h',
+            'main',
+            'main+0x2000',
+            'ABCABC',
+            '0xAZF0',
+        ];
+
+        for (const value of validValues) {
+            const result = isHexString(value);
+            expect(result).to.equals(false, `isHexString for ${value}`);
+        }
+    });
+});
+
+describe('getInstructions', () => {
+    const sandbox = Sinon.createSandbox();
+    const fakeGDBObject = {} as any;
+    let sendDataDisassembleStub: Sinon.SinonStub;
+
+    beforeEach(() => {
+        sendDataDisassembleStub = sandbox.stub(midata, 'sendDataDisassemble');
+        sendDataDisassembleStub.resolves({
+            asm_insns: [
+                {
+                    line: 1,
+                    file: 'test.c',
+                    fullname: '/path/to/test.c',
+                    line_asm_insn: [
+                        {
+                            address: '0x0',
+                            opcodes: 'aa bb',
+                            inst: 'test inst',
+                        },
+                    ],
+                },
+            ],
+        });
+    });
+
+    afterEach(() => {
+        sandbox.reset();
+        sandbox.restore();
+    });
+
+    it('should handle the negative memory areas', async () => {
+        const instructions = await getInstructions(fakeGDBObject, '0x02', -10);
+        Sinon.assert.calledOnceWithExactly(
+            sendDataDisassembleStub,
+            fakeGDBObject,
+            '(0x02)-2',
+            '(0x02)+0'
+        );
+        expect(instructions.length).to.equal(10);
+        for (let i = 0; i < 9; i++) {
+            // Nine of them are invalid with relative addresses
+            expect(instructions[i]).deep.equals({
+                address: `(0x0)-${(9 - i) * 2}`,
+                instruction: 'failed to retrieve instruction',
+                presentationHint: 'invalid',
+            });
+        }
+
+        // Last one is the returned instruction.
+        expect(instructions[9]).deep.equals({
+            address: '0x0',
+            instructionBytes: 'aa bb',
+            instruction: 'test inst',
+            location: { name: 'test.c', path: '/path/to/test.c' },
+            line: 1,
+        });
+    });
+});

src/util/calculateMemoryOffset.ts

@@ -8,6 +8,8 @@
  * SPDX-License-Identifier: EPL-2.0
  *********************************************************************/
 
+import { isHexString } from './isHexString';
+
 /**
  * This method calculates the memory offset arithmetics on string hexadecimal address value
  *
@@ -22,9 +24,12 @@ export const calculateMemoryOffset = (
     address: string,
     offset: string | number | bigint
 ): string => {
-    if (address.startsWith('0x')) {
+    if (isHexString(address)) {
         const addressLength = address.length - 2;
         const newAddress = BigInt(address) + BigInt(offset);
+        if (newAddress < 0) {
+            return `(0x${'0'.padStart(addressLength, '0')})${newAddress}`;
+        }
         return `0x${newAddress.toString(16).padStart(addressLength, '0')}`;
     } else {
         const addrParts = /^([^+-]*)([+-]\d+)?$/g.exec(address);

src/util/disassembly.ts

@@ -11,6 +11,7 @@ import { DebugProtocol } from '@vscode/debugprotocol';
 import { MIDataDisassembleAsmInsn, sendDataDisassemble } from '../mi';
 import { IGDBBackend } from '../types/gdb';
 import { calculateMemoryOffset } from './calculateMemoryOffset';
+import { isHexString } from './isHexString';
 
 /**
  * Converts the MIDataDisassembleAsmInsn object to DebugProtocol.DisassembledInstruction
@@ -121,6 +122,9 @@ export const getInstructions = async (
     };
 
     const sendDataDisassembleWrapper = async (lower: number, upper: number) => {
+        if (lower === upper) {
+            return [];
+        }
         const list: DebugProtocol.DisassembledInstruction[] = [];
 
         const result = await sendDataDisassemble(
@@ -152,6 +156,15 @@ export const getInstructions = async (
         if (isReverseFetch) {
             target.higher = target.lower;
             target.lower += length * meanSizeOfInstruction;
+
+            // Limit the lower bound to not to cross negative memory address area
+            if (
+                isHexString(memoryReference) &&
+                BigInt(memoryReference) + BigInt(target.lower) < 0
+            ) {
+                // Lower and Upper bounds are in number type
+                target.lower = Number(memoryReference) * -1;
+            }
         } else {
             target.lower = target.higher;
             target.higher += length * meanSizeOfInstruction;

src/util/isHexString.ts

@@ -0,0 +1,20 @@
+/*********************************************************************
+ * Copyright (c) 2025 Renesas Electronics Corporation and others
+ *
+ * This program and the accompanying materials are made
+ * available under the terms of the Eclipse Public License 2.0
+ * which is available at https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *********************************************************************/
+
+/**
+ * Checks if the given value is an hex string starting with 0x
+ *
+ * @param value
+ * 		Reference value to check. For example '0x0000FF00', 'main', 'main+200'
+ * @return
+ * 		Returns true if value is an hex string, otherwise returns false.
+ */
+
+export const isHexString = (value: string) => /^0x[\da-f]+$/i.test(value);