From 52e76c5d39b58d1685bb5e22bd40d19524dfa2cb Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 12 Oct 2026 04:33:25 +0800 Subject: [PATCH] fix(managed): share first-match member ownership indexes Resolve fields, methods, and attribute constructors through one budgeted index per layout. Preserve malformed-list first-match ownership without per-member TypeDef scans; remove the superseded lookup implementations. Validate pointer identity and malformed ranges through production parsers, including a 40,000-type/method case and allocation admission. Managed source tests and CLI/MCP pointer acceptance pass. Refs #1953. --- src/dotnet/ManagedMemberInspectorCore.test.ts | 96 ------------ src/dotnet/ManagedMemberInspectorCore.ts | 50 ------ src/dotnet/ManagedMemberOwnership.test.ts | 146 ++++++++++++++++++ src/dotnet/ManagedMemberOwnership.ts | 90 +++++++++++ src/dotnet/ManagedMemberRows.ts | 33 ++-- src/dotnet/ManagedMetadataInventoryRows.ts | 104 +------------ src/dotnet/ManagedPe.fixture.ts | 7 +- 7 files changed, 259 insertions(+), 267 deletions(-) delete mode 100644 src/dotnet/ManagedMemberInspectorCore.test.ts create mode 100644 src/dotnet/ManagedMemberOwnership.test.ts create mode 100644 src/dotnet/ManagedMemberOwnership.ts diff --git a/src/dotnet/ManagedMemberInspectorCore.test.ts b/src/dotnet/ManagedMemberInspectorCore.test.ts deleted file mode 100644 index 2067e614b..000000000 --- a/src/dotnet/ManagedMemberInspectorCore.test.ts +++ /dev/null @@ -1,96 +0,0 @@ -import { fc, it } from "@fast-check/vitest"; -import { describe, expect } from "vitest"; - -import { - createDeclaringTypeLookup, - declaringType, - type TypeRange, -} from "./ManagedMemberInspectorCore.js"; - -const range = ( - name: string, - fieldStart: number, - fieldEnd: number, - methodStart = fieldStart, - methodEnd = fieldEnd, -): TypeRange => ({ - token: name, - fullName: name, - fieldStart, - fieldEnd, - methodStart, - methodEnd, -}); - -describe("managed declaring type ownership", () => { - it("resolves field and method boundaries independently, including empty types and gaps", () => { - const ranges = [ - range("first", 1, 3, 1, 1), - range("empty", 3, 3, 1, 1), - range("last", 5, 7, 1, 4), - ]; - const field = createDeclaringTypeLookup(ranges, "field"); - const method = createDeclaringTypeLookup(ranges, "method"); - expect([0, 1, 2, 3, 4, 5, 6, 7].map(field)).toEqual([ - null, - { token: "first", fullName: "first" }, - { token: "first", fullName: "first" }, - null, - null, - { token: "last", fullName: "last" }, - { token: "last", fullName: "last" }, - null, - ]); - expect([0, 1, 2, 3, 4].map(method)).toEqual([ - null, - { token: "last", fullName: "last" }, - { token: "last", fullName: "last" }, - { token: "last", fullName: "last" }, - null, - ]); - }); - - it("retains the first observed owner for overlapping, reversed, or out-of-order ranges", () => { - const ranges = [ - range("first", 5, 9), - range("overlap", 1, 7), - range("reversed", 10, 2), - ]; - const lookup = createDeclaringTypeLookup(ranges, "method"); - expect(lookup(6)).toEqual({ token: "first", fullName: "first" }); - expect(lookup(2)).toEqual({ token: "overlap", fullName: "overlap" }); - expect(lookup(10)).toBeNull(); - expect(createDeclaringTypeLookup([], "method")(1)).toBeNull(); - }); - - it.prop([ - fc.array( - fc.record({ - fieldStart: fc.integer({ min: 0, max: 30 }), - fieldEnd: fc.integer({ min: 0, max: 30 }), - methodStart: fc.integer({ min: 0, max: 30 }), - methodEnd: fc.integer({ min: 0, max: 30 }), - }), - { maxLength: 20 }, - ), - ])( - "agrees with observed first-match ownership for arbitrary metadata ranges", - (raw) => { - const ranges = raw.map((value, index) => ({ - ...value, - token: String(index), - fullName: `Type${String(index)}`, - })); - for (const table of ["field", "method"] as const) { - const lookup = createDeclaringTypeLookup(ranges, table); - for (const row of [ - NaN, - -Infinity, - Infinity, - ...Array.from({ length: 32 }, (_, i) => i), - ]) - expect(lookup(row)).toEqual(declaringType(ranges, table, row)); - } - }, - ); -}); diff --git a/src/dotnet/ManagedMemberInspectorCore.ts b/src/dotnet/ManagedMemberInspectorCore.ts index 54dd49976..f54a11769 100644 --- a/src/dotnet/ManagedMemberInspectorCore.ts +++ b/src/dotnet/ManagedMemberInspectorCore.ts @@ -154,56 +154,6 @@ export const typeRanges = ( return ranges; }; -export const declaringType = ( - ranges: readonly TypeRange[], - table: "field" | "method", - row: number, -): { readonly token: string; readonly fullName: string | null } | null => { - for (const range of ranges) { - const start = table === "field" ? range.fieldStart : range.methodStart; - const end = table === "field" ? range.fieldEnd : range.methodEnd; - if (row >= start && row < end) - return { token: range.token, fullName: range.fullName }; - } - return null; -}; - -/** Resolve member ownership without rescanning ordered type ranges per row. */ -export const createDeclaringTypeLookup = ( - ranges: readonly TypeRange[], - table: "field" | "method", -): ((row: number) => ReturnType) => { - const entries = ranges.map((range) => ({ - range, - start: table === "field" ? range.fieldStart : range.methodStart, - end: table === "field" ? range.fieldEnd : range.methodEnd, - })); - const ordered = entries.every((entry, index) => { - const previous = entries[index - 1]; - return ( - entry.start <= entry.end && - (previous === undefined || previous.end <= entry.start) - ); - }); - // Malformed metadata can overlap or reverse ranges. Retain the original - // first-match interpretation rather than guessing a different owner. - if (!ordered) return (row) => declaringType(ranges, table, row); - return (row) => { - let first = 0; - let end = entries.length; - while (first < end) { - const middle = first + Math.floor((end - first) / 2); - const entry = entries[middle]; - if (entry === undefined) return null; - if (row >= entry.start && row < entry.end) - return { token: entry.range.token, fullName: entry.range.fullName }; - if (row < entry.start) end = middle; - else first = middle + 1; - } - return null; - }; -}; - export const parseTypes = ( bytes: Buffer, layout: ManagedMetadataLayout, diff --git a/src/dotnet/ManagedMemberOwnership.test.ts b/src/dotnet/ManagedMemberOwnership.test.ts new file mode 100644 index 000000000..c734aa05c --- /dev/null +++ b/src/dotnet/ManagedMemberOwnership.test.ts @@ -0,0 +1,146 @@ +import { fc, it } from "@fast-check/vitest"; +import { describe, expect, vi } from "vitest"; + +import { inspectManagedArtifactBytes } from "./ManagedArtifactInspector.js"; +import { inspectManagedMembersBytes } from "./ManagedMemberInspector.js"; +import { managedDecodeBudget } from "./ManagedDecodeBudget.js"; +import { memberDeclaringTypeRowLookup } from "./ManagedMemberOwnership.js"; +import { memberListIndexSize } from "./ManagedMemberPointers.js"; +import { readManagedMetadataLayout } from "./ManagedMetadataLayout.js"; +import { + buildManagedMemberPointerFixture, + buildManagedPeFixture, + managedPeFixtureTarget, +} from "./ManagedPe.fixture.js"; +import { readManagedPeLayout } from "./ManagedPeReader.js"; +import { ManagedReaderFailure } from "./ManagedReaderFailure.js"; + +const metadataLayout = (bytes: Buffer) => { + const pe = readManagedPeLayout(bytes); + if (pe.cli === null) throw new Error("Fixture omitted CLI metadata"); + return readManagedMetadataLayout( + bytes, + pe.rvaToOffset(pe.cli.metadata.rva, pe.cli.metadata.size, "cli.metadata"), + pe.cli.metadata.size, + ); +}; + +describe("managed declaring type ownership", () => { + it.prop([ + fc.tuple( + fc.tuple( + ...Array.from({ length: 3 }, () => fc.integer({ min: 0, max: 6 })), + ), + fc.tuple( + ...Array.from({ length: 3 }, () => fc.integer({ min: 0, max: 6 })), + ), + ), + ])( + "retains first-match field and method owners for arbitrary metadata lists", + ([fields, methods]) => { + const bytes = buildManagedMemberPointerFixture(); + const layout = metadataLayout(bytes); + const types = layout.table(2); + if (types === undefined) throw new Error("Fixture omitted TypeDef"); + for (let row = 0; row < 3; row++) { + const fieldOffset = + types.offset + + row * types.rowSize + + 4 + + 2 * layout.stringIndexSize + + layout.codedIndexSize("TypeDefOrRef"); + bytes.writeUInt16LE(fields[row] ?? 0, fieldOffset); + bytes.writeUInt16LE( + methods[row] ?? 0, + fieldOffset + memberListIndexSize(layout, 4), + ); + } + const target = managedPeFixtureTarget(bytes); + const members = inspectManagedMembersBytes(bytes, target); + const artifact = inspectManagedArtifactBytes(bytes, target); + for (const [table, starts] of [ + ["fields", fields], + ["methods", methods], + ] as const) { + // The fixture's pointers reorder physical rows 1,2 to list positions 2,1. + const expected = [2, 1].map((position) => { + const owner = starts.findIndex( + (start, index) => + position >= start && position < (starts[index + 1] ?? 3), + ); + return owner < 0 ? null : members.types[owner]?.full_name; + }); + expect(members[table].map((member) => member.declaring_type)).toEqual( + expected, + ); + } + expect(artifact.attributes[0]?.type_name).toBe( + members.methods[0]?.declaring_type, + ); + }, + ); + + it("indexes large unordered member lists once per layout", () => { + const count = 40_000; + const bytes = buildManagedPeFixture({ + additionalEmptyTypes: count - 1, + methodRowCount: count, + }); + const layout = metadataLayout(bytes); + const types = layout.table(2); + if (types === undefined) throw new Error("Fixture omitted TypeDef"); + const methodOffset = + types.offset + + 4 + + 2 * layout.stringIndexSize + + layout.codedIndexSize("TypeDefOrRef") + + memberListIndexSize(layout, 4); + bytes.writeUInt16LE(2, methodOffset); + const lookup = memberDeclaringTypeRowLookup(bytes, layout, 6); + expect( + Array.from({ length: count }, (_, row) => lookup(row + 1)).every( + (owner) => owner === count, + ), + ).toBe(true); + expect(memberDeclaringTypeRowLookup(bytes, layout, 6)).toBe(lookup); + expect([lookup(0), lookup(count + 1)]).toEqual([0, 0]); + }); + + it("admits ownership storage before allocation and retains failed admission", () => { + const bytes = buildManagedPeFixture(); + const layout = metadataLayout(bytes); + const budget = managedDecodeBudget(layout); + const capacityFailure = new ManagedReaderFailure({ + code: "resource-limit", + scope: "metadata.representation", + offset: layout.table(2)?.offset ?? null, + detail: "No remaining ownership index capacity", + }); + const admission = vi.spyOn(budget, "reserve").mockImplementation(() => { + throw capacityFailure; + }); + let allocations = 0; + vi.stubGlobal( + "Uint32Array", + new Proxy(Uint32Array, { + construct(target, args, newTarget) { + allocations++; + return Reflect.construct(target, args, newTarget); + }, + }), + ); + try { + expect(() => memberDeclaringTypeRowLookup(bytes, layout, 6)).toThrow( + capacityFailure, + ); + expect(() => memberDeclaringTypeRowLookup(bytes, layout, 6)).toThrow( + capacityFailure, + ); + expect(admission).toHaveBeenCalledTimes(1); + expect(allocations).toBe(0); + } finally { + vi.unstubAllGlobals(); + admission.mockRestore(); + } + }); +}); diff --git a/src/dotnet/ManagedMemberOwnership.ts b/src/dotnet/ManagedMemberOwnership.ts new file mode 100644 index 000000000..0292f5e0a --- /dev/null +++ b/src/dotnet/ManagedMemberOwnership.ts @@ -0,0 +1,90 @@ +import { managedDecodeBudget } from "./ManagedDecodeBudget.js"; +import { metadataRowCursor } from "./ManagedMetadataHeaps.js"; +import type { ManagedMetadataLayout } from "./ManagedMetadataLayout.js"; +import { + memberListIndexSize, + memberListRowLookup, + memberListTable, +} from "./ManagedMemberPointers.js"; +import { ManagedReaderFailure } from "./ManagedReaderFailure.js"; + +type MemberTable = 4 | 6; +type OwnerLookup = (definitionRow: number) => number; +const lookups = new WeakMap< + ManagedMetadataLayout, + Map +>(); + +/** One first-match TypeDef owner per physical member, shared by every inspection facet. */ +export const memberDeclaringTypeRowLookup = ( + bytes: Buffer, + layout: ManagedMetadataLayout, + table: MemberTable, +): OwnerLookup => { + let byTable = lookups.get(layout); + const cached = byTable?.get(table); + if (cached instanceof ManagedReaderFailure) throw cached; + if (cached !== undefined) return cached; + if (byTable === undefined) { + byTable = new Map(); + lookups.set(layout, byTable); + } + try { + const positionOf = memberListRowLookup(bytes, layout, table); + const positions = memberListTable(layout, table)?.rowCount ?? 0; + const types = layout.table(2)?.rowCount ?? 0; + managedDecodeBudget(layout).reserve( + (2 * positions + 3) * Uint32Array.BYTES_PER_ELEMENT + 256, + layout.table(2)?.offset ?? null, + ); + const owners = new Uint32Array(positions + 1); + // Skip already-owned positions so overlapping malformed lists remain near-linear. + const next = Uint32Array.from({ length: positions + 2 }, (_, p) => p); + const unowned = (position: number): number => { + let root = position; + while (next[root] !== root) root = next[root] ?? root; + for (let p = position; p !== root;) { + const following = next[p] ?? root; + next[p] = root; + p = following; + } + return root; + }; + const startForType = (row: number): number => { + const cursor = metadataRowCursor(bytes, layout, 2, row); + cursor.readUInt32(); + cursor.readIndex(layout.stringIndexSize); + cursor.readIndex(layout.stringIndexSize); + cursor.readIndex(layout.codedIndexSize("TypeDefOrRef")); + const fieldStart = cursor.readIndex(memberListIndexSize(layout, 4)); + return table === 4 + ? fieldStart + : cursor.readIndex(memberListIndexSize(layout, 6)); + }; + let start = types === 0 ? 0 : startForType(1); + for (let row = 1; row <= types; row += 1) { + const end = row === types ? positions + 1 : startForType(row + 1); + const limit = Math.min(end, positions + 1); + for ( + let p = unowned(Math.min(Math.max(start, 1), positions + 1)); + p < limit; + p = unowned(p + 1) + ) { + owners[p] = row; + next[p] = p + 1; + } + start = end; + } + const definitions = layout.table(table)?.rowCount ?? 0; + const lookup: OwnerLookup = (definition) => + definition < 1 || definition > definitions + ? 0 + : (owners[positionOf(definition)] ?? 0); + byTable.set(table, lookup); + return lookup; + } catch (cause: unknown) { + // Replaying a malformed layout must neither rebuild nor charge the index again. + if (cause instanceof ManagedReaderFailure) byTable.set(table, cause); + throw cause; + } +}; diff --git a/src/dotnet/ManagedMemberRows.ts b/src/dotnet/ManagedMemberRows.ts index bc982d81f..559201f13 100644 --- a/src/dotnet/ManagedMemberRows.ts +++ b/src/dotnet/ManagedMemberRows.ts @@ -8,7 +8,6 @@ import type { ManagedPeLayout } from "./ManagedPeReader.js"; import type { ManagedParseIssue } from "../domain/managed/managedArtifact.js"; import type { ManagedMetadataLayout } from "./ManagedMetadataLayout.js"; import { - createDeclaringTypeLookup, signature, type FieldCore, type ManagedCallEdge, @@ -31,7 +30,7 @@ import { } from "./ManagedMetadataHeaps.js"; import { readManagedValue } from "./ManagedReaderFailure.js"; import { methodBody } from "./ManagedMethodBodyReader.js"; -import { memberListRowLookup } from "./ManagedMemberPointers.js"; +import { memberDeclaringTypeRowLookup } from "./ManagedMemberOwnership.js"; export const parseFields = ( bytes: Buffer, @@ -46,9 +45,8 @@ export const parseFields = ( const core = new Map(); const issues: ManagedParseIssue[] = []; const table = layout.table(4); - const declaringType = createDeclaringTypeLookup(ranges, "field"); - const listRow = readManagedValue( - () => memberListRowLookup(bytes, layout, 4), + const ownerRow = readManagedValue( + () => memberDeclaringTypeRowLookup(bytes, layout, 4), issues, ); for (let row = 1; row <= (table?.rowCount ?? 0); row += 1) { @@ -67,11 +65,11 @@ export const parseFields = ( cursor.readIndex(layout.blobIndexSize), layout.blob.size, ); - const position = - listRow === undefined - ? undefined - : readManagedValue(() => listRow(row), issues); - const declared = position === undefined ? null : declaringType(position); + const owner = + ownerRow === undefined + ? 0 + : (readManagedValue(() => ownerRow(row), issues) ?? 0); + const declared = ranges[owner - 1]; const token = metadataToken(4, row); fields.push({ token, @@ -245,9 +243,8 @@ export const parseMethods = ({ const core = new Map(); const issues: ManagedParseIssue[] = []; const table = layout.table(6); - const declaringType = createDeclaringTypeLookup(ranges, "method"); - const listRow = readManagedValue( - () => memberListRowLookup(bytes, layout, 6), + const ownerRow = readManagedValue( + () => memberDeclaringTypeRowLookup(bytes, layout, 6), issues, ); for (let row = 1; row <= (table?.rowCount ?? 0); row += 1) { @@ -269,11 +266,11 @@ export const parseMethods = ({ layout.blob.size, ); cursor.readIndex(layout.tableIndexSize(8)); - const position = - listRow === undefined - ? undefined - : readManagedValue(() => listRow(row), issues); - const declared = position === undefined ? null : declaringType(position); + const owner = + ownerRow === undefined + ? 0 + : (readManagedValue(() => ownerRow(row), issues) ?? 0); + const declared = ranges[owner - 1]; const token = metadataToken(6, row); methods.push({ token, diff --git a/src/dotnet/ManagedMetadataInventoryRows.ts b/src/dotnet/ManagedMetadataInventoryRows.ts index ebe034f7d..4689b72eb 100644 --- a/src/dotnet/ManagedMetadataInventoryRows.ts +++ b/src/dotnet/ManagedMetadataInventoryRows.ts @@ -1,9 +1,5 @@ import { managedDecodeBudget } from "./ManagedDecodeBudget.js"; -import { - memberListIndexSize, - memberListRowLookup, - memberListTable, -} from "./ManagedMemberPointers.js"; +import { memberDeclaringTypeRowLookup } from "./ManagedMemberOwnership.js"; import type { ManagedArtifactInspection, @@ -22,11 +18,7 @@ import { readMetadataGuid, readMetadataString, } from "./ManagedMetadataHeaps.js"; -import { - managedFailure, - ManagedReaderFailure, - readManagedValue, -} from "./ManagedReaderFailure.js"; +import { managedFailure, readManagedValue } from "./ManagedReaderFailure.js"; import type { ManagedResourceDirectory } from "./ManagedMetadataInventory.js"; type ModuleIdentity = NonNullable; @@ -244,105 +236,17 @@ const readTypeName = ({ return namespace.length === 0 ? name : `${namespace}.${name}`; }; -const methodOwners = new WeakMap< - ManagedMetadataLayout, - Uint32Array | ManagedReaderFailure ->(); - -/** - * Index the first TypeDef whose method list contains each list position, once - * per layout. Malformed lists may overlap; skipping already-owned positions - * keeps first-match ownership without rescanning TypeDef rows per attribute. - */ -const methodOwnerRows = ( - bytes: Buffer, - layout: ManagedMetadataLayout, - types: number, - positions: number, -): Uint32Array => { - const cached = methodOwners.get(layout); - if (cached instanceof ManagedReaderFailure) throw cached; - if (cached !== undefined) return cached; - try { - managedDecodeBudget(layout).reserve( - (2 * positions + 3) * Uint32Array.BYTES_PER_ELEMENT + 256, - layout.table(2)?.offset ?? null, - ); - const owners = new Uint32Array(positions + 1); - // next[p] leads to the first unowned position at or after p. - const next = Uint32Array.from({ length: positions + 2 }, (_, p) => p); - const unowned = (position: number): number => { - let root = position; - while (next[root] !== root) root = next[root] ?? root; - for (let p = position; p !== root;) { - const following = next[p] ?? root; - next[p] = root; - p = following; - } - return root; - }; - let start = types === 0 ? 0 : methodListForType(bytes, layout, 1); - for (let row = 1; row <= types; row += 1) { - const end = - row === types - ? positions + 1 - : methodListForType(bytes, layout, row + 1); - const limit = Math.min(end, positions + 1); - for ( - let p = unowned(Math.min(Math.max(start, 1), positions + 1)); - p < limit; - p = unowned(p + 1) - ) { - owners[p] = row; - next[p] = p + 1; - } - start = end; - } - methodOwners.set(layout, owners); - return owners; - } catch (cause: unknown) { - // Invalid metadata must not rebuild the index for every attribute. - if (cause instanceof ManagedReaderFailure) methodOwners.set(layout, cause); - throw cause; - } -}; - const declaringTypeForMethod = ( bytes: Buffer, layout: ManagedMetadataLayout, methodRow: number, heapExtent: number, ): string | null => { - const types = layout.table(2); - const methods = memberListTable(layout, 6); - if (types === undefined || methods === undefined) return null; - if (methodRow < 1 || methodRow > (layout.table(6)?.rowCount ?? 0)) - return null; - const position = memberListRowLookup(bytes, layout, 6)(methodRow); - const owner = methodOwnerRows( - bytes, - layout, - types.rowCount, - methods.rowCount, - )[position]; - if (owner === undefined || owner === 0) return null; + const owner = memberDeclaringTypeRowLookup(bytes, layout, 6)(methodRow); + if (owner === 0) return null; return readTypeName({ bytes, layout, table: 2, row: owner, heapExtent }); }; -const methodListForType = ( - bytes: Buffer, - layout: ManagedMetadataLayout, - row: number, -): number => { - const cursor = metadataRowCursor(bytes, layout, 2, row); - cursor.readUInt32(); - cursor.readIndex(layout.stringIndexSize); - cursor.readIndex(layout.stringIndexSize); - cursor.readIndex(layout.codedIndexSize("TypeDefOrRef")); - cursor.readIndex(memberListIndexSize(layout, 4)); - return cursor.readIndex(memberListIndexSize(layout, 6)); -}; - const attributeTypeName = ( bytes: Buffer, layout: ManagedMetadataLayout, diff --git a/src/dotnet/ManagedPe.fixture.ts b/src/dotnet/ManagedPe.fixture.ts index 8f8ae95fb..25f819925 100644 --- a/src/dotnet/ManagedPe.fixture.ts +++ b/src/dotnet/ManagedPe.fixture.ts @@ -22,6 +22,7 @@ interface ManagedPeFixtureOptions { readonly malformedCustomAttributeRows?: readonly number[]; readonly metadataExtraData?: number; readonly metadataValidMaskExtra?: bigint; + readonly methodRowCount?: number; readonly methodName?: string; readonly methodFlags?: number; readonly methodSignature?: Buffer; @@ -604,7 +605,7 @@ const fixtureIndexWidths = ({ [1, 1], [2, indexes.additionalTypeNames.length + 1], [4, 1], - [6, 1], + [6, options.methodRowCount ?? 1], [8, 0], [10, 1], [ @@ -707,7 +708,7 @@ const fixtureMetadataRows = ({ ], [ 6, - [ + repeatedRows(options.methodRowCount ?? 1, () => methodDefRow({ name: indexes.methodName, signature: indexes.methodSignature, @@ -719,7 +720,7 @@ const fixtureMetadataRows = ({ options.methodFlags ?? (options.pinvoke === undefined ? 0x0016 : 0x2016), }), - ], + ), ], [ 10,