From 5ecd7deea7bb3f86f4f78072e4f275b0abe927a8 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 24 Oct 2026 03:01:11 +0800 Subject: [PATCH] fix(native): preserve procedure selector coordinate intent --- bridge/hopper_bridge.py | 24 ++++++++--- docs/native-investigation.md | 7 ++++ src/ida/IdaOperationRunner.test.ts | 38 ++++++++++++++++++ src/ida/IdaOperationRunner.ts | 13 ++++++ .../hopper/hopperProcedureSelection.test.ts | 40 +++++++++++++++++++ .../fixtures/hopperProcedureSelectionProbe.py | 40 +++++++++++++++++++ 6 files changed, 157 insertions(+), 5 deletions(-) create mode 100644 src/ida/IdaOperationRunner.test.ts create mode 100644 tests/boundary/providers/hopper/hopperProcedureSelection.test.ts create mode 100644 tests/fixtures/hopperProcedureSelectionProbe.py diff --git a/bridge/hopper_bridge.py b/bridge/hopper_bridge.py index dd10204103..13c6c9c554 100644 --- a/bridge/hopper_bridge.py +++ b/bridge/hopper_bridge.py @@ -337,11 +337,14 @@ def _bookmark_name(document, address): return _bookmark_api(document, "getBookmarkName")(address) -def _address(document, value=None): - """Keep explicit hexadecimal coordinates distinct from symbol names.""" +def _address_selection(document, value=None): + """Own the coordinate and whether it was selected by symbol or address.""" if value is None: - return document.getCurrentAddress() + return document.getCurrentAddress(), "cursor" _api_text(value, "Address") + if value.startswith("file-offset:"): + raise InvalidRequestError("Hopper does not support file-offset: selectors; select Ghidra for source-file coordinates or use a Hopper analysis address") + source = "address" if value.lower().startswith("0x"): try: address = int(value, 16) @@ -352,6 +355,8 @@ def _address(document, value=None): # Prefer an observed symbol, retaining bare-hex compatibility only # when Hopper has no symbol with the supplied name. address = document.getAddressForName(value) + if address not in BAD_ADDRESSES: + source = "symbol" if address in BAD_ADDRESSES: try: address = int(value, 16) @@ -359,7 +364,11 @@ def _address(document, value=None): raise InvalidRequestError("Unknown Hopper address or name: %s" % value) from error if not 0 <= address < 0xFFFFFFFFFFFFFFFF: raise InvalidRequestError("Address is outside Hopper's usable unsigned 64-bit range: %s" % value) - return address + return address, source + + +def _address(document, value=None): + return _address_selection(document, value)[0] def _segment(document, address): @@ -370,10 +379,15 @@ def _segment(document, address): def _procedure(document, value=None): - address = document.getCurrentAddress() if value is None else _address(document, value) + address, source = _address_selection(document, value) result = _segment(document, address).getProcedureAtAddress(address) if result is None: raise InvalidRequestError("No procedure exists at the requested address") + if source == "symbol" and address != result.getEntryPoint(): + raise InvalidRequestError( + "Symbol %s at %s is inside procedure %s at %s; select its entry name/address or use resolve_containing_procedure for explicit containment" + % (value, _hex(address), _procedure_name(result), _hex(result.getEntryPoint())) + ) return result diff --git a/docs/native-investigation.md b/docs/native-investigation.md index fa8c14aeac..6bf57c07a5 100644 --- a/docs/native-investigation.md +++ b/docs/native-investigation.md @@ -982,3 +982,10 @@ semantics. This query never executes or modifies target bytes. Exact-name xref lookup reports distinct candidate addresses as an input ambiguity; inventory order cannot select one silently. Duplicate rows at one address remain a singular match, and a missing name remains an unresolved observation. + +Hopper procedure names must identify a procedure entry. A non-entry symbol returns +an input diagnostic with its coordinate and containing procedure; an explicit +numeric address still selects its containing procedure. Use +`resolve_containing_procedure` to request containment directly. IDA and Hopper +reject reserved `file-offset:` selectors before name lookup; source-file offset +selection belongs to the Ghidra import mapping. diff --git a/src/ida/IdaOperationRunner.test.ts b/src/ida/IdaOperationRunner.test.ts new file mode 100644 index 0000000000..6475eda2fa --- /dev/null +++ b/src/ida/IdaOperationRunner.test.ts @@ -0,0 +1,38 @@ +import { rm } from "node:fs/promises"; +import { expect, it } from "vitest"; +import { + createIdaTarget, + RecordingIdaMcp, +} from "../../tests/fixtures/idaMcp.js"; +import { IdaOperationRunner } from "./IdaOperationRunner.js"; + +it.each(["legacy", "modern", "native"] as const)( + "%s procedure resolution rejects file coordinates before any symbol lookup", + async (protocol) => { + const { root, target } = await createIdaTarget(); + const producer = new RecordingIdaMcp( + target, + protocol === "legacy" ? "attached" : "headless", + ); + const runner = new IdaOperationRunner(producer, protocol); + try { + await expect( + runner.resolve("file-offset:0x10", "procedure_pseudo_code"), + ).rejects.toMatchObject({ + _tag: "AnalysisInputError", + operation: "procedure_pseudo_code", + issues: [ + { + path: ["procedure"], + message: expect.stringContaining("source-file coordinates"), + }, + ], + }); + expect(producer.calls).toEqual([]); + expect((await runner.resolve("main")).name).toBe("main"); + } finally { + await producer.close(); + await rm(root, { recursive: true, force: true }); + } + }, +); diff --git a/src/ida/IdaOperationRunner.ts b/src/ida/IdaOperationRunner.ts index a5121b1a9f..0d773d93a8 100644 --- a/src/ida/IdaOperationRunner.ts +++ b/src/ida/IdaOperationRunner.ts @@ -166,6 +166,19 @@ export class IdaOperationRunner { * addresses were reported by IDA itself. */ async resolve(query: string, requestedBy?: string): Promise { + if (query.startsWith("file-offset:")) + throw new AnalysisInputError( + requestedBy ?? "procedure_address", + undefined, + [ + { + path: ["procedure"], + reason: "invalid_value", + message: + "IDA does not support file-offset: procedure selectors. Select Ghidra for source-file coordinates or supply an IDA analysis address or exact procedure name.", + }, + ], + ); if (this.protocol !== "legacy") { const item = modernLookupSchema.parse( await this.call("lookup_funcs", { queries: [query] }), diff --git a/tests/boundary/providers/hopper/hopperProcedureSelection.test.ts b/tests/boundary/providers/hopper/hopperProcedureSelection.test.ts new file mode 100644 index 0000000000..fe34f2d34f --- /dev/null +++ b/tests/boundary/providers/hopper/hopperProcedureSelection.test.ts @@ -0,0 +1,40 @@ +import { execFile } from "node:child_process"; +import { fileURLToPath } from "node:url"; +import { promisify } from "node:util"; +import { expect, it } from "vitest"; + +it("Hopper preserves named-entry selection and explicit address containment", async () => { + const fixture = fileURLToPath( + new URL( + "../../../fixtures/hopperProcedureSelectionProbe.py", + import.meta.url, + ), + ); + const bridge = fileURLToPath( + new URL("../../../../bridge/hopper_bridge.py", import.meta.url), + ); + const { stdout } = await promisify(execFile)( + "python3", + ["-I", "-S", fixture, bridge], + { timeout: 3_000 }, + ); + const observed = JSON.parse(stdout); + for (const selector of ["entry", "alias_entry", "0x401004", "401004"]) + expect(observed.results[selector]).toEqual({ entry: "0x401000" }); + expect(observed.results.inner).toMatchObject({ + diagnostic_type: "invalid_request", + message: expect.stringContaining( + "inner at 0x401004 is inside procedure entry at 0x401000", + ), + }); + expect(observed.results["file-offset:0x4"]).toMatchObject({ + diagnostic_type: "invalid_request", + message: expect.stringContaining("source-file coordinates"), + }); + expect(observed.name_lookups).toEqual([ + "entry", + "alias_entry", + "inner", + "401004", + ]); +}); diff --git a/tests/fixtures/hopperProcedureSelectionProbe.py b/tests/fixtures/hopperProcedureSelectionProbe.py new file mode 100644 index 0000000000..35055e38b4 --- /dev/null +++ b/tests/fixtures/hopperProcedureSelectionProbe.py @@ -0,0 +1,40 @@ +"""Exercise named-entry and explicit-coordinate selection at the Hopper boundary.""" +import json +from pathlib import Path +import sys + +bridge_path = sys.argv[1] +bridge = {"__file__": bridge_path, "__name__": "rea_hopper_bridge"} +exec(compile(Path(bridge_path).read_text(encoding="utf-8"), bridge_path, "exec"), bridge) + +class Procedure: + def getEntryPoint(self): + return 0x401000 + def getSegment(self): + return segment +class Segment: + def getNameAtAddress(self, address): + return "entry" if address == 0x401000 else "inner" + def getProcedureAtAddress(self, address): + return procedure +class Document: + def __init__(self): + self.names = [] + def getCurrentAddress(self): + return 0x401004 + def getAddressForName(self, name): + self.names.append(name) + return {"entry": 0x401000, "alias_entry": 0x401000, "inner": 0x401004}.get(name) + def getSegmentAtAddress(self, address): + return segment +segment = Segment() +procedure = Procedure() +document = Document() +results = {} +for selector in ["entry", "alias_entry", "inner", "0x401004", "401004", "file-offset:0x4"]: + try: + selected = bridge["_procedure"](document, selector) + results[selector] = {"entry": bridge["_hex"](selected.getEntryPoint())} + except Exception as error: + results[selector] = {"diagnostic_type": bridge["_diagnostic_type"](error), "message": str(error)} +print(json.dumps({"results": results, "name_lookups": document.names}))