From bd687bbc40daf47d93f2b4c65b72004eb8f87c83 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Thu, 29 Oct 2026 04:59:04 +0800 Subject: [PATCH] fix(ghidra)!: distinguish unlisted instruction addresses (#7823) * fix(ghidra)!: distinguish unlisted instruction addresses BREAKING CHANGE: Ghidra now returns not-in-listing for in-memory addresses without listed instructions or defined data. Consumers matching the former undecodable value must handle the new status. * fix(ghidra): invalidate snapshots with old instruction status * test(ghidra): update raw profile commitment goldens --- bridge/ghidra/ReaGhidraBridge.java | 3 +- docs/native-investigation.md | 8 +++- scripts/verify-real-ghidra-com.mjs | 50 +++++++++++++++++++++++ src/contracts/officialToolContracts.ts | 2 +- src/domain/native/nativeInstruction.ts | 19 +++++---- src/ghidra/GhidraAnalysisProfile.ts | 1 + src/ghidra/GhidraRawProfileDigest.test.ts | 11 ++--- src/ghidra/GhidraSnapshotCache.test.ts | 5 +++ 8 files changed, 83 insertions(+), 16 deletions(-) diff --git a/bridge/ghidra/ReaGhidraBridge.java b/bridge/ghidra/ReaGhidraBridge.java index d1f8bd141f..953be50b3f 100644 --- a/bridge/ghidra/ReaGhidraBridge.java +++ b/bridge/ghidra/ReaGhidraBridge.java @@ -1313,7 +1313,7 @@ public final class ReaGhidraBridge extends HeadlessScript { flow.addProperty("computed", false); if (instruction == null) { result.addProperty("status", !currentProgram.getMemory().contains(address) ? "outside-memory" : currentProgram.getListing().getInstructionContaining(address) != null - ? "not-instruction-boundary" : currentProgram.getListing().getDefinedDataContaining(address) != null ? "data" : "undecodable"); + ? "not-instruction-boundary" : currentProgram.getListing().getDefinedDataContaining(address) != null ? "data" : "not-in-listing"); for (String key : List.of("bytes", "length", "mnemonic", "raw_disassembly")) result.add(key, JsonNull.INSTANCE); } else { result.addProperty("status", "decoded"); @@ -1385,6 +1385,7 @@ public final class ReaGhidraBridge extends HeadlessScript { result.add("incoming_references", incomingReferences); } List limitations = new ArrayList<>(List.of("Operand components are Ghidra Listing decoder tokens; effective memory base/index/displacement roles and per-instruction context mode are unavailable. The mode field is the program language variant.")); + if (result.get("status").getAsString().equals("not-in-listing")) limitations.add("No listed instruction or defined data covers this in-memory address. REA did not attempt a decode; the memory may be uninitialized or contain valid instructions."); if (flow.get("kind").getAsString().equals("terminal")) limitations.add("No RETURN operation was observed in the provider's instruction p-code; terminal classification remains generic."); result.add("limitations", GSON.toJsonTree(limitations)); return result; diff --git a/docs/native-investigation.md b/docs/native-investigation.md index 9fb4241f51..f15102ab53 100644 --- a/docs/native-investigation.md +++ b/docs/native-investigation.md @@ -127,7 +127,11 @@ rea inspect-native-data-type --address --provider Instruction facts include bytes, length, decoder text, ordered register/scalar/ address tokens, typed references and flow destinations. Mid-instruction, data, -outside-memory and undecodable addresses have separate outcomes. Effective +outside-memory and unlisted addresses have separate outcomes. For Ghidra, +`not-in-listing` means the address is in memory without a listed instruction or +defined data. REA does not attempt a decode there: the memory may be uninitialized +or contain valid instructions. Older Evidence may report `undecodable` for this +same ambiguous case; it does not establish that a decode failed. Effective memory base/index/displacement roles remain unavailable. Ghidra reports its program language variant in `mode`; other providers retain their own mode label. Ghidra MIPS results additionally expose `decoder_context` @@ -138,7 +142,7 @@ that do not expose this facet. The context is an analysis observation, not proof of a runtime processor state. `inspect_native_instruction` also returns `incoming_references` to the exact -queried address, including data, interior-instruction and undecodable addresses. +queried address, including data, interior-instruction and unlisted addresses. Ghidra enumerates its ReferenceManager, excludes synthetic entry-point references, and returns deterministic source-address ordering while retaining distinct operand references from the same source. `[]` means that enumeration diff --git a/scripts/verify-real-ghidra-com.mjs b/scripts/verify-real-ghidra-com.mjs index 80b997f990..2513d493ac 100644 --- a/scripts/verify-real-ghidra-com.mjs +++ b/scripts/verify-real-ghidra-com.mjs @@ -13,8 +13,10 @@ import { fileURLToPath } from "node:url"; import { promisify } from "node:util"; import { Client } from "@modelcontextprotocol/client"; import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; +import { Ajv2020 } from "ajv/dist/2020.js"; import { parseEvidence } from "../dist/domain/evidence.js"; import { functionDossierSchema } from "../dist/domain/native/functionDossier.js"; +import { nativeInstructionSchema } from "../dist/domain/native/nativeInstruction.js"; import { inspectVerifierGhidraInstallation } from "./lib/ghidra-installation-inspection.mjs"; import { buildDosComFixture } from "../tests/conformance/ghidra/dos-com-fixture.mjs"; import { createVerifierRun, completeVerifierRun } from "./lib/verifier-run.mjs"; @@ -158,6 +160,54 @@ try { instruction.bytes.replaceAll(" ", "").toLowerCase(), fixture.entry_hex, ); + // This NOP follows the entry's terminating INT 20h and is not reached by + // either function. It remains valid instruction bytes outside the listing. + const unlistedAddress = "0x1013f"; + const unlistedBytes = await call("read_bytes", { + address: unlistedAddress, + length: 1, + }); + assert.equal(unlistedBytes.bytes_hex, "90"); + const unlistedResponse = await client.callTool({ + name: "inspect_native_instruction", + arguments: { address: unlistedAddress }, + }); + const unlistedEvidence = parseEvidence( + requireMcpOutput(unlistedResponse, "inspect_native_instruction"), + ); + const unlisted = unlistedEvidence.normalized_result; + assert.equal(unlisted.status, "not-in-listing"); + assert.equal(unlisted.bytes, null); + assert.ok( + unlisted.limitations.some((item) => + item.includes("did not attempt a decode"), + ), + ); + const advertised = (await client.listTools()).tools.find( + (tool) => tool.name === "inspect_native_instruction", + ); + assert.ok(advertised?.outputSchema); + const ajv = new Ajv2020({ strict: false, validateFormats: false }); + assert.equal(ajv.validateSchema(advertised.outputSchema), true); + assert.equal( + ajv.validate(advertised.outputSchema, unlistedResponse.structuredContent), + true, + JSON.stringify(ajv.errors), + ); + assert.ok( + nativeInstructionSchema.safeParse({ ...unlisted, status: "undecodable" }) + .success, + ); + assert.equal( + (await call("resolve_native_call_targets", { address: unlistedAddress })) + .status, + "unavailable", + ); + assert.deepEqual( + (await cli("inspect-native-instruction", unlistedAddress)) + .normalized_result, + unlisted, + ); const cliImage = await cli("inspect-native-load-image"); const stable = (x) => ({ ...x, diff --git a/src/contracts/officialToolContracts.ts b/src/contracts/officialToolContracts.ts index 7c4fbde59a..15bbc417d3 100644 --- a/src/contracts/officialToolContracts.ts +++ b/src/contracts/officialToolContracts.ts @@ -66,7 +66,7 @@ export const OFFICIAL_TOOL_CONTRACTS = [ ), official( "inspect_native_instruction", - "Inspect one exact code or data address: decoded bytes, mnemonic, ordered operand tokens, flow, outgoing instruction references and typed incoming references where available. Incoming references preserve source, access kind, operand index and analysis provenance even at data addresses; they do not prove runtime access. Memory addressing decomposition is unavailable when the provider supplies only tokens. Data, interior instruction addresses and undecodable bytes are distinct outcomes.", + "Inspect one exact code or data address: decoded bytes, mnemonic, ordered operand tokens, flow, outgoing instruction references and typed incoming references where available. Incoming references preserve source, access kind, operand index and analysis provenance even at data addresses; they do not prove runtime access. Memory addressing decomposition is unavailable when the provider supplies only tokens. Ghidra reports in-memory addresses without a listed instruction or defined data as not-in-listing, without attempting a decode.", nativeInstructionInputSchema, ), official( diff --git a/src/domain/native/nativeInstruction.ts b/src/domain/native/nativeInstruction.ts index ac2ccfed8b..c5ee4c6219 100644 --- a/src/domain/native/nativeInstruction.ts +++ b/src/domain/native/nativeInstruction.ts @@ -44,13 +44,18 @@ const reference = z.strictObject({ /** Provider-decoded facts for one instruction; operand tokens preserve their native ordering. */ export const nativeInstructionSchema = z.strictObject({ address: z.string(), - status: z.enum([ - "decoded", - "not-instruction-boundary", - "data", - "undecodable", - "outside-memory", - ]), + status: z + .enum([ + "decoded", + "not-instruction-boundary", + "data", + "not-in-listing", + "undecodable", + "outside-memory", + ]) + .describe( + "decoded is an instruction at the address; not-instruction-boundary is inside a listed instruction; data is defined data; outside-memory is outside the program's memory. Ghidra's not-in-listing means the address is in memory with no listed instruction or defined data; no decode was attempted, and memory may be uninitialized. The legacy undecodable value in older Evidence has the same ambiguous meaning and does not prove decoding failed.", + ), procedure: z.string().nullable(), architecture: z.string(), mode: z diff --git a/src/ghidra/GhidraAnalysisProfile.ts b/src/ghidra/GhidraAnalysisProfile.ts index f7dbc65ca8..7860a6c3a6 100644 --- a/src/ghidra/GhidraAnalysisProfile.ts +++ b/src/ghidra/GhidraAnalysisProfile.ts @@ -65,6 +65,7 @@ const sharedAnalysisSemantics = { procedure_file_offset_resolution: "address-first-unique-target-source-mapping-v2", instruction_flow_evidence: "decoded-return-pcode-v1", + instruction_status: "unlisted-address-without-decode-v1", instruction_references: "exact-address-incoming-without-synthetic-entry-v2", function_boundary_observations: "ghidra-function-boundary-limitations-v4", } as const; diff --git a/src/ghidra/GhidraRawProfileDigest.test.ts b/src/ghidra/GhidraRawProfileDigest.test.ts index 836f530253..f9b7ed5551 100644 --- a/src/ghidra/GhidraRawProfileDigest.test.ts +++ b/src/ghidra/GhidraRawProfileDigest.test.ts @@ -6,12 +6,12 @@ import { silentLogger } from "../logger.js"; import { ghidraProfileInstallationHost as host } from "./GhidraInstallation.fixture.js"; import { GhidraProvider } from "./GhidraProvider.js"; -// Digests were computed by the unmodified upstream resolver (3ae7daa10, -// raw branch inline) and must not change when the binary-loader policy is shared. +// Digests commit to the current shared policy, including the instruction status +// revision that prevents reuse of snapshots with the previous ambiguous result. const RAW_DIGESTS = { - "6502": "adbec31711f31471f9cdbdbb2fc743a0ce6b46c9be5568bf90a3f51ef46e376d", + "6502": "8e33776d1cc5c5d77d32923d5978c05ec2d989d00920dd458cc2dafb4c9c0cad", "x86-disabled-analyzer": - "493a02888eca575be286b5a96041af8007444586854edb974d6a0c12fd2768c4", + "d17bddc8ac110c891d30fb989fc7906df421bd9bea5df4aac05a950f50c7bdaf", } as const; const configFor = (environment: NodeJS.ProcessEnv) => { @@ -33,7 +33,7 @@ const raw = (architecture: "6502" | "x86", entryPoint: string): BinaryTarget => }) as unknown as BinaryTarget; describe("raw-binary analysis profile digest", () => { - it("keeps the 6502 raw profile digest from before the shared policy refactor", async () => { + it("commits the 6502 raw profile to the current shared policy", async () => { const ghidra = new GhidraProvider(configFor({}), silentLogger, {}, host); const resolved = await ghidra.resolveAnalysisProfile(raw("6502", "0x8001")); if (!resolved.ok) throw resolved.error; @@ -44,6 +44,7 @@ describe("raw-binary analysis profile digest", () => { language_id: "6502:LE:16:default", language_selection: "caller-supplied-v1", load_image_evidence: "independent-raw-mapping-context-v4", + instruction_status: "unlisted-address-without-decode-v1", analyzer_preset: "ghidra-default", }); }); diff --git a/src/ghidra/GhidraSnapshotCache.test.ts b/src/ghidra/GhidraSnapshotCache.test.ts index dde944b5c2..2c3e207c9c 100644 --- a/src/ghidra/GhidraSnapshotCache.test.ts +++ b/src/ghidra/GhidraSnapshotCache.test.ts @@ -307,6 +307,11 @@ describe("Ghidra relationship coverage replay", () => { describe("Ghidra snapshot profile admission", () => { it.each([ + { + parameter: "instruction_status", + current: "unlisted-address-without-decode-v1", + legacy: undefined, + }, { parameter: "function_boundary_observations", current: "ghidra-function-boundary-limitations-v4",