diff --git a/docs/managed-code-analysis.md b/docs/managed-code-analysis.md index f4a2adf065..c340d38ebc 100644 --- a/docs/managed-code-analysis.md +++ b/docs/managed-code-analysis.md @@ -739,6 +739,10 @@ refer to the pointer table when present rather than necessarily contiguous definition rows. Invalid or nonmonotonic list coordinates and malformed or ambiguous pointers leave affected owners unknown with partial coverage while preserving readable member and attribute facts. +For each list column, ordering checks compare consecutive valid coordinates +even across invalid rows. A provable decrease leaves the affected ownership +unknown. Retained declaring-type filters count these candidates in +`unresolved_filter_rows`; omit that filter to retrieve their known-name rows. The retained inverse indexes are admitted against the existing representation budget before allocation. diff --git a/src/dotnet/ManagedMemberInspector.test.ts b/src/dotnet/ManagedMemberInspector.test.ts index ca7a9a9f17..5801de7ee5 100644 --- a/src/dotnet/ManagedMemberInspector.test.ts +++ b/src/dotnet/ManagedMemberInspector.test.ts @@ -12,6 +12,7 @@ import { type ManagedMetadataLayout, } from "./ManagedMetadataLayout.js"; import { readManagedPeLayout } from "./ManagedPeReader.js"; +import { managedMemberListStartsFixture } from "../../tests/fixtures/managedMemberListStarts.js"; import { memberListIndexSize } from "./ManagedMemberPointers.js"; import { edges } from "./ManagedMemberRows.js"; import { parseUserStrings } from "./ManagedUserStrings.js"; @@ -2212,6 +2213,127 @@ describe("managed TypeDef list ownership bounds", () => { }); }); +describe("managed TypeDef ordering across invalid coordinates", () => { + it.each([ + { + label: "both lists decrease", + fields: [2, 0, 1], + methods: [2, 0, 1], + fieldOwner: null, + methodOwner: null, + issues: 4, + decreases: [ + ["FieldList", 3], + ["MethodList", 3], + ], + }, + { + label: "out-of-range nonzero coordinates are skipped", + fields: [2, 65535, 1], + methods: [2, 65535, 1], + fieldOwner: null, + methodOwner: null, + issues: 4, + decreases: [ + ["FieldList", 3], + ["MethodList", 3], + ], + }, + { + label: "known coordinates stay ordered", + fields: [1, 0, 1], + methods: [1, 0, 1], + fieldOwner: "0x02000003", + methodOwner: "0x02000003", + issues: 2, + decreases: [], + }, + { + label: "only fields decrease", + fields: [2, 0, 1], + methods: [1, 1, 1], + fieldOwner: null, + methodOwner: "0x02000003", + issues: 2, + decreases: [["FieldList", 3]], + }, + { + label: "only methods decrease", + fields: [1, 1, 1], + methods: [2, 0, 1], + fieldOwner: "0x02000003", + methodOwner: null, + issues: 2, + decreases: [["MethodList", 3]], + }, + { + label: "multiple invalid rows", + fields: [2, 0, 0, 1], + methods: [2, 0, 0, 1], + fieldOwner: null, + methodOwner: null, + issues: 6, + decreases: [ + ["FieldList", 4], + ["MethodList", 4], + ], + }, + { + label: "one invalid column does not hide another column or a valid tail", + fields: [1, 0, 1], + methods: [2, 1, 1], + fieldOwner: "0x02000003", + methodOwner: "0x02000003", + issues: 2, + decreases: [["MethodList", 2]], + }, + ] as const)( + "checks independent ordering across invalid starts: $label", + (test) => { + const bytes = managedMemberListStartsFixture(test.fields, test.methods); + const result = inspectManagedMembersBytes( + bytes, + managedPeFixtureTarget(bytes), + ); + expect(result.fields).toMatchObject([ + { + token: "0x04000001", + declaring_type: test.fieldOwner === null ? null : "Fixture.Program", + declaring_type_token: test.fieldOwner, + signature: { parse_status: "decoded" }, + }, + ]); + expect(result.methods).toMatchObject([ + { + token: "0x06000001", + declaring_type: test.methodOwner === null ? null : "Fixture.Program", + declaring_type_token: test.methodOwner, + signature: { parse_status: "decoded" }, + }, + ]); + expect(result.types.at(-1)).toMatchObject({ + field_list: { count: test.fieldOwner === null ? 0 : 1 }, + method_list: { count: test.methodOwner === null ? 0 : 1 }, + }); + expect(result.coverage.state).toBe("partial"); + expect(result.coverage.issues).toHaveLength(test.issues); + expect( + result.coverage.issues.filter(({ detail }) => + detail.includes("decreases"), + ), + ).toMatchObject( + test.decreases.map(([list, row]) => ({ + code: "invalid-row", + scope: `metadata.TypeDef:0x0200000${String(row)}`, + detail: expect.stringContaining( + `${list} start 1 decreases from preceding start 2`, + ), + })), + ); + }, + ); +}); + describe("managed TypeDef list ordering", () => { it("leaves adjacent ownership unknown when valid list starts decrease", () => { const bytes = buildManagedPeFixture({ diff --git a/src/dotnet/ManagedTypeMemberLists.ts b/src/dotnet/ManagedTypeMemberLists.ts index e66ea37a2f..977c97c3cd 100644 --- a/src/dotnet/ManagedTypeMemberLists.ts +++ b/src/dotnet/ManagedTypeMemberLists.ts @@ -94,29 +94,35 @@ export const readTypeMemberListFacts = ( } const nonmonotonicFields = new Set(); const nonmonotonicMethods = new Set(); - for (let index = 0; index + 1 < rows.length; index += 1) { - const current = rows[index]; - const next = rows[index + 1]; - if (current === undefined || next === undefined) continue; - for (const [name, before, after, affected] of [ - ["FieldList", current.field, next.field, nonmonotonicFields], - ["MethodList", current.method, next.method, nonmonotonicMethods], - ] as const) + const previousKnown = { field: -1, method: -1 }; + for (const [index, entry] of rows.entries()) { + for (const [name, member, affected] of [ + ["FieldList", "field", nonmonotonicFields], + ["MethodList", "method", nonmonotonicMethods], + ] as const) { + const after = entry[member]; + // An invalid coordinate cannot establish a range endpoint, but it + // cannot hide a decrease between the surrounding valid coordinates. + if (after.value === null) continue; + const preceding = previousKnown[member]; + const before = rows[preceding]?.[member]; + previousKnown[member] = index; if ( + before !== undefined && before.value !== null && - after.value !== null && after.value < before.value ) { managedDecodeBudget(layout).reserve(512, after.offset); + affected.add(preceding); affected.add(index); - affected.add(index + 1); issues.push({ code: "invalid-row", - scope: `metadata.TypeDef:${metadataToken(2, index + 2)}`, + scope: `metadata.TypeDef:${metadataToken(2, index + 1)}`, offset: after.offset, - detail: `TypeDef ${name} start ${String(after.raw)} decreases from preceding start ${String(before.raw)}; adjacent member ranges are unknown`, + detail: `TypeDef ${name} start ${String(after.raw)} decreases from preceding start ${String(before.raw)}; affected member ranges are unknown`, }); } + } } const entries: TypeMemberListEntry[] = rows.map((entry, index) => { const next = rows[index + 1]; diff --git a/tests/boundary/cli/managedMemberListOrdering.test.ts b/tests/boundary/cli/managedMemberListOrdering.test.ts new file mode 100644 index 0000000000..6c731d003e --- /dev/null +++ b/tests/boundary/cli/managedMemberListOrdering.test.ts @@ -0,0 +1,120 @@ +import { createHash } from "node:crypto"; +import { readFile, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { expect } from "vitest"; + +import { analysisViewResultSchema } from "../../../src/domain/analysisView/analysisViewResult.js"; +import { parseEvidence } from "../../../src/domain/evidence.js"; +import { parseEvidenceBundle } from "../../../src/domain/evidenceBundle.js"; +import { managedMemberInspectionSchema } from "../../../src/domain/managed/managedArtifact.js"; +import { managedMemberListStartsFixture } from "../../fixtures/managedMemberListStarts.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest } from "../../support/cli/cliFixture.js"; + +cliTest.for(["decreasing", "ordered"] as const)( + "preserves managed ownership and unknown filter rows over CLI/MCP: %s", + async (kind, { cli }) => { + const root = await createTestTempDirectory("rea-managed-list-order-"); + const path = join(root, "fixture.dll"); + const starts = kind === "decreasing" ? [2, 0, 1] : [1, 0, 1]; + const bytes = managedMemberListStartsFixture(starts, starts); + await writeFile(path, bytes); + const sha256 = createHash("sha256").update(bytes).digest("hex"); + const destination = join(root, "members.json"); + const cliReply = await cli.run({ + arguments: [ + "inspect-managed-members", + path, + "--detail", + "summary", + "--evidence-bundle", + destination, + "--json", + ], + }); + expect(cliReply.exitCode, cliReply.stderr).toBe(0); + const bundle = parseEvidenceBundle( + JSON.parse(await readFile(destination, "utf8")), + ); + const parent = bundle.records.find( + ({ operation }) => operation === "inspect_managed_members", + ); + if (parent === undefined) + throw new Error("Missing complete member Evidence"); + expect(parent.subject?.digest.sha256).toBe(sha256); + const report = managedMemberInspectionSchema.parse( + parent.normalized_result, + ); + expect(report.coverage.state).toBe("partial"); + expect( + report.coverage.issues.filter(({ detail }) => + detail.includes("decreases"), + ), + ).toHaveLength(kind === "decreasing" ? 2 : 0); + const owner = kind === "decreasing" ? null : "0x02000003"; + const field = report.fields.find(({ token }) => token === "0x04000001"); + const method = report.methods.find(({ token }) => token === "0x06000001"); + if (field?.name == null || method?.name == null) + throw new Error("Readable fixture members were lost"); + for (const row of [field, method]) { + expect(row.declaring_type_token).toBe(owner); + expect(row.signature.parse_status).toBe("decoded"); + } + + const call = await cli.sharedMcpCall!(); + const reply = await call("inspect_managed_members", { + path, + detail: "summary", + }); + expect(reply.isError, JSON.stringify(reply)).not.toBe(true); + const summary = analysisViewResultSchema.parse( + parseEvidence(reply.structuredContent).normalized_result, + ); + if (summary.kind !== "summary") throw new Error("Expected member summary"); + for (const [collection, member] of [ + ["fields", field], + ["methods", method], + ] as const) { + for (const typed of [false, true]) { + const result = await call("inspect_analysis_view", { + source: { + kind: "retained-evidence", + evidence_id: summary.parent_evidence_id, + }, + view: { + kind: "managed-members", + collection, + names: [member.name], + fields: ["name", "declaring_type"], + offset: 0, + limit: 8, + ...(typed ? { declaring_types: ["Fixture.Program"] } : {}), + }, + }); + expect(result.isError, JSON.stringify(result)).not.toBe(true); + const view = analysisViewResultSchema.parse( + parseEvidence(result.structuredContent).normalized_result, + ); + if (view.kind !== "managed-members") + throw new Error("Expected retained members"); + expect(view.page.collection).toBe(collection); + expect(view.source_coverage.state).toBe("partial"); + if (typed && kind === "decreasing") { + expect(view.page.rows).toEqual([]); + expect(view.page.unresolved_filter_rows).toBe(1); + } else { + expect(view.page.rows).toMatchObject([ + { + token: member.token, + name: member.name, + declaring_type_token: owner, + }, + ]); + expect(view.page.rows[0]).not.toHaveProperty("body"); + expect(view.page.rows[0]).not.toHaveProperty("signature"); + } + } + } + expect((await readFile(path)).equals(bytes)).toBe(true); + }, +); diff --git a/tests/fixtures/managedMemberListStarts.ts b/tests/fixtures/managedMemberListStarts.ts new file mode 100644 index 0000000000..465e407e18 --- /dev/null +++ b/tests/fixtures/managedMemberListStarts.ts @@ -0,0 +1,30 @@ +import { readManagedMetadataLayout } from "../../src/dotnet/ManagedMetadataLayout.js"; +import { buildManagedPeFixture } from "../../src/dotnet/ManagedPe.fixture.js"; +import { readManagedPeLayout } from "../../src/dotnet/ManagedPeReader.js"; +import { readTypeDefinitionRowFacts } from "../../src/dotnet/ManagedTypeDefinitionRows.js"; + +/** Select TypeDef list coordinates while retaining the fixture's readable member rows. */ +export const managedMemberListStartsFixture = ( + fields: readonly number[], + methods: readonly number[], +): Buffer => { + if (fields.length === 0 || fields.length !== methods.length) + throw new Error("Each fixture TypeDef needs both list coordinates"); + const bytes = buildManagedPeFixture({ + additionalEmptyTypes: fields.length - 1, + }); + const pe = readManagedPeLayout(bytes); + if (pe.cli === null) throw new Error("Fixture omitted CLI metadata"); + const { rva, size } = pe.cli.metadata; + const layout = readManagedMetadataLayout( + bytes, + pe.rvaToOffset(rva, size, "cli.metadata"), + size, + ); + for (const [index, field] of fields.entries()) { + const row = readTypeDefinitionRowFacts(bytes, layout, index + 1); + bytes.writeUInt16LE(field, row.fieldListOffset); + bytes.writeUInt16LE(methods[index]!, row.methodListOffset); + } + return bytes; +};