diff --git a/docs/managed-code-analysis.md b/docs/managed-code-analysis.md index e14ff4752a..fa3c965ad7 100644 --- a/docs/managed-code-analysis.md +++ b/docs/managed-code-analysis.md @@ -99,10 +99,13 @@ assembly. Older member Evidence without `selection` remains readable. target. TypeDef and TypeRef names use the namespace-qualified `full_name` form, and nested types keep bare names. A generic-instantiation TypeSpec parent names its generic type, such as ``System.Collections.Generic.List`1``. -Other TypeSpec forms, ModuleRef and MethodDef parents, and unreadable rows stay +Other well-formed TypeSpec forms and ModuleRef or MethodDef parents stay `null`. A `method-spec` call edge reports the name and declaring type of the -generic method it instantiates. Older member Evidence without these fields -reads them as `null`: unknown. +generic method it instantiates. A field access is named only by a field or a +field-signature MemberRef. TypeSpec and MethodSpec blobs are decoded with the +member-signature grammar; malformed rows and blobs stay `null` and make +coverage partial, while a readable MethodSpec method column keeps its name. +Older member Evidence without these fields reads them as `null`: unknown. ## Analysis objective diff --git a/src/dotnet/ManagedMemberInspector.test.ts b/src/dotnet/ManagedMemberInspector.test.ts index 9940b7c21b..840a763e82 100644 --- a/src/dotnet/ManagedMemberInspector.test.ts +++ b/src/dotnet/ManagedMemberInspector.test.ts @@ -212,34 +212,91 @@ describe("managed edge target declaring types", () => { expect(array.coverage).toEqual({ state: "complete", issues: [] }); }); - it("keeps self-referential and out-of-range generic TypeSpec parents unknown", () => { - const cyclic = memberRefOnTypeSpec( - Buffer.from([0x15, 0x12, (1 << 2) | 2, 0x01, 0x0e]), - ); - expect(cyclic.member_refs[0]?.parent_name).toBeNull(); + it.each([ + [ + "names a TypeSpec as its generic type", + [0x15, 0x12, (1 << 2) | 2, 0x01, 0x0e], + "invalid-blob", + "metadata.TypeSpec:0x1b000001", + ], + [ + "has no type arguments", + [0x15, 0x12, (1 << 2) | 1], + "invalid-blob", + "metadata.TypeSpec:0x1b000001", + ], + [ + "names a missing TypeRef", + [0x15, 0x12, (9 << 2) | 1, 0x01, 0x0e], + "invalid-row", + "metadata.TypeRef", + ], + [ + "names a missing TypeDef", + [0x15, 0x12, (9 << 2) | 0, 0x01, 0x0e], + "invalid-row", + "metadata.TypeDef", + ], + ])( + "reports a generic TypeSpec parent that %s and leaves it unknown", + (_case, signature, code, scope) => { + const result = memberRefOnTypeSpec(Buffer.from(signature)); - const missing = memberRefOnTypeSpec( - Buffer.from([0x15, 0x12, (9 << 2) | 1, 0x01, 0x0e]), - ); - expect(missing.member_refs[0]?.parent_name).toBeNull(); - expect(missing.coverage).toMatchObject({ - state: "partial", - issues: [{ code: "invalid-row", scope: "metadata.TypeRef" }], + expect(result.member_refs[0]?.parent_name).toBeNull(); + expect(result.coverage).toMatchObject({ + state: "partial", + issues: [{ code, scope }], + }); + }, + ); + + it("keeps a readable MethodSpec method beside a malformed instantiation", () => { + const result = inspect({ + methodSpecMethods: [(1 << 1) | 1, (9 << 1) | 1], + methodSpecInstantiation: Buffer.from([0x0a, 0x00]), + ilBody: tokenBody([ + [call, 0x2b00_0001], + [call, 0x2b00_0002], + ]), }); + + expect( + result.call_edges.map(({ target_name, target_declaring_type }) => [ + target_name, + target_declaring_type, + ]), + ).toEqual([ + [".ctor", attribute], + [null, null], + ]); + expect(result.coverage.state).toBe("partial"); + expect( + result.coverage.issues.map(({ code, scope }) => [code, scope]), + ).toEqual([ + ["invalid-blob", "metadata.MethodSpec:0x2b000001"], + ["invalid-row", "metadata.MethodSpec:0x2b000002"], + ["invalid-blob", "metadata.MethodSpec:0x2b000002"], + ]); }); - it("names external field accesses and leaves mismatched operand tables unnamed", () => { + it("names field MemberRefs as fields and leaves mismatched member kinds unnamed", () => { const result = inspect({ memberRefName: "Empty", + // FIELD string + memberRefSignature: Buffer.from([0x06, 0x0e]), ilBody: tokenBody([ [ldsfld, 0x0a00_0001], [call, 0x0400_0001], ]), }); + const methodRef = inspect({ ilBody: tokenBody([[ldsfld, 0x0a00_0001]]) }); expect(result.field_accesses).toMatchObject([ { field_name: "Empty", field_declaring_type: attribute }, ]); + expect(methodRef.field_accesses).toMatchObject([ + { field_name: null, field_declaring_type: null }, + ]); expect(result.call_edges).toMatchObject([ { target_kind: "unknown", diff --git a/src/dotnet/ManagedMemberInspectorCore.ts b/src/dotnet/ManagedMemberInspectorCore.ts index f91a24e914..b5a9638305 100644 --- a/src/dotnet/ManagedMemberInspectorCore.ts +++ b/src/dotnet/ManagedMemberInspectorCore.ts @@ -7,6 +7,7 @@ import type { ManagedMemberInspection, ManagedParseIssue, } from "../domain/managed/managedArtifact.js"; +import { managedDecodeBudget } from "./ManagedDecodeBudget.js"; import type { ManagedMetadataLayout, MetadataTableLayout, @@ -57,6 +58,8 @@ export interface MemberCore { readonly token: string; readonly name: string; readonly declaringType: string | null; + /** Fields and methods by table; member references by decoded signature. */ + readonly kind: ManagedSignature["kind"]; } export interface ParsedInstruction { @@ -596,6 +599,61 @@ const callingConvention = (value: number): string => { return flags.length === 0 ? name : `${name} ${flags.join(" ")}`; }; +/** + * Decode a whole TypeSpec blob, or a MethodSpec instantiation (GENERICINST, + * argument count, Types), with the member-signature grammar. Returns the + * rendered types, or null for a well-formed form the grammar does not + * support. A malformed blob throws invalid-blob under the owning row. + */ +export const specSignature = ( + layout: ManagedMetadataLayout, + blob: Buffer, + owner: { + readonly form: "type" | "instantiation"; + readonly scope: string; + readonly offset: number; + }, +): readonly string[] | null => { + const budget = managedDecodeBudget(layout); + // Rendering is transient, so charge it as member signatures are charged. + budget.admitTransient(blob.length * 256, owner.offset); + try { + let cursor = 0; + let count = 1; + if (owner.form === "instantiation") { + if (blob[0] !== 0x0a) + malformedSignature("MethodSpec instantiation is not GENERICINST", 0); + const header = readCompressed(blob, 1); + if (header.value === 0) + malformedSignature("MethodSpec instantiation has no type arguments", 1); + count = header.value; + cursor = header.next; + } + const types: string[] = []; + // Each Type consumes at least one byte, so a false count fails on truncation. + for (let index = 0; index < count; index += 1) { + const type = readTypeSignature(blob, cursor, "type"); + types.push(type.value); + cursor = type.next; + } + if (cursor !== blob.length) + malformedSignature("Trailing signature data", cursor); + return types; + } catch (cause: unknown) { + if (!(cause instanceof ManagedReaderFailure)) throw cause; + if (cause.issue.code === "unsupported-signature") return null; + if (cause.issue.code !== "invalid-blob") throw cause; + throw managedFailure( + "invalid-blob", + owner.scope, + cause.issue.detail, + owner.offset, + ); + } finally { + budget.releaseTransient(); + } +}; + export const signature = ( blob: Buffer, rawSha256?: string, diff --git a/src/dotnet/ManagedMemberRows.ts b/src/dotnet/ManagedMemberRows.ts index 3505b0f49e..6c29d6cd99 100644 --- a/src/dotnet/ManagedMemberRows.ts +++ b/src/dotnet/ManagedMemberRows.ts @@ -9,6 +9,7 @@ import type { ManagedParseIssue } from "../domain/managed/managedArtifact.js"; import type { ManagedMetadataLayout } from "./ManagedMetadataLayout.js"; import { signature, + specSignature, type ManagedCallEdge, type ManagedExportedType, type ManagedField, @@ -80,6 +81,7 @@ export const parseFields = ( token, name, declaringType: declared?.fullName ?? null, + kind: "field", }); if (!isSelected(selected, declared)) return; const sig = readMetadataBlob( @@ -228,15 +230,21 @@ export const parseMemberRefs = ( layout.rowCounts, ); const parentName = typeName(parentToken); + const decoded = budgetedSignature(sig, layout); refs.push({ token, row_offset: cursor.start, parent_token: parentToken, parent_name: parentName, name, - signature: budgetedSignature(sig, layout), + signature: decoded, + }); + core.set(token, { + token, + name, + declaringType: parentName, + kind: decoded.kind, }); - core.set(token, { token, name, declaringType: parentName }); }, issues); } return { refs, core, issues }; @@ -291,6 +299,7 @@ export const parseMethods = ({ token, name, declaringType: declared?.fullName ?? null, + kind: "method", }); if (!isSelected(selected, declared)) return; const sig = readMetadataBlob( @@ -339,34 +348,66 @@ export const edgeTargets = ( } => { const members = new Map(cores.flatMap((core) => [...core])); const issues: ManagedParseIssue[] = []; + const seen = new Set(); for (const method of methods) for (const { operand } of method.body.anchors) { - if (operand?.startsWith("0x2b") !== true || members.has(operand)) - continue; + if (operand?.startsWith("0x2b") !== true || seen.has(operand)) continue; + seen.add(operand); readManagedValue(() => { + const scope = `metadata.MethodSpec:${operand}`; const cursor = metadataRowCursor( bytes, layout, 0x2b, Number.parseInt(operand.slice(4), 16), ); + const methodRaw = cursor.readIndex( + layout.codedIndexSize("MethodDefOrRef"), + ); + const instantiation = cursor.readIndex(layout.blobIndexSize); + const methodReason = metadataCodedTokenInvalidReason( + methodRaw, + 1, + [6, 10], + layout.rowCounts, + ); + if (methodReason !== null || methodRaw === 0) + issues.push({ + code: "invalid-row", + scope, + offset: cursor.start, + detail: + methodReason === null + ? "MethodSpec method coded index 0x0 is null, but the Method column must reference a row" + : `MethodSpec method coded index 0x${methodRaw.toString(16)} is invalid: ${methodReason}`, + }); const generic = metadataCodedToken( - cursor.readIndex(layout.codedIndexSize("MethodDefOrRef")), + methodRaw, 1, [6, 10], layout.rowCounts, ); const target = generic === null ? undefined : members.get(generic); + // A readable Method column stays a fact even if the instantiation is not. if (target !== undefined) members.set(operand, { ...target, token: operand }); + specSignature( + layout, + readMetadataBlob(bytes, layout, instantiation, layout.blob.size), + { form: "instantiation", scope, offset: cursor.start }, + ); }, issues); } return { members, issues }; }; -const TARGET_TABLES = new Map>([ - ["method", new Set(["0x06", "0x0a", "0x2b"])], - ["field", new Set(["0x04", "0x0a"])], +/** + * Member kinds each operand kind may name. A MemberRef whose signature does + * not decode keeps its name on calls, as before, but is never named a field. + */ +const NAMEABLE_KINDS = new Map>([ + ["method", new Set(["method", "unknown"])], + ["field", new Set(["field"])], ]); const targetKind = (token: string): ManagedCallEdge["target_kind"] => @@ -378,6 +419,19 @@ const targetKind = (token: string): ManagedCallEdge["target_kind"] => ? "method-spec" : "unknown"; +/** The member an operand names, if the operand kind can name that member's kind. */ +const namedTarget = ( + members: ReadonlyMap, + operandKind: string, + operand: string, +): MemberCore | undefined => { + const member = members.get(operand); + return member !== undefined && + NAMEABLE_KINDS.get(operandKind)?.has(member.kind) === true + ? member + : undefined; +}; + export const edges = ( methods: readonly ManagedMethod[], members: ReadonlyMap, @@ -402,12 +456,7 @@ export const edges = ( if (anchor.opcode === "ldtoken") continue; const { operand } = anchor; if (operand === null) continue; - // A token from a table the operand kind cannot name stays unnamed. - const target = TARGET_TABLES.get(anchor.operand_kind)?.has( - operand.slice(0, 4), - ) - ? members.get(operand) - : undefined; + const target = namedTarget(members, anchor.operand_kind, operand); if (anchor.operand_kind === "method") { // Each derived record is admitted before the graph grows further. callEdges.push( diff --git a/src/dotnet/ManagedPe.fixture.ts b/src/dotnet/ManagedPe.fixture.ts index ca8d1ba80e..948a66b7be 100644 --- a/src/dotnet/ManagedPe.fixture.ts +++ b/src/dotnet/ManagedPe.fixture.ts @@ -30,8 +30,11 @@ interface ManagedPeFixtureOptions { readonly assemblyRowCount?: number; readonly memberRefParentRaw?: number; readonly memberRefName?: string; + readonly memberRefSignature?: Buffer; /** Raw MethodDefOrRef coded values, one MethodSpec row each. */ readonly methodSpecMethods?: readonly number[]; + /** Instantiation blob shared by every MethodSpec row. */ + readonly methodSpecInstantiation?: Buffer; /** TypeSpec signature blobs, one TypeSpec row each. */ readonly typeSpecSignatures?: readonly Buffer[]; readonly mvid?: Buffer; @@ -836,22 +839,27 @@ const fixtureMetadataRows = ({ return rows; }; -/** Blob indexes for optional TypeSpec and MethodSpec rows; defaults leave the heap unchanged. */ -const specBlobIndexes = ( +/** Blob indexes for the MemberRef and optional spec rows; defaults leave the heap unchanged. */ +const referenceBlobIndexes = ( blobs: BlobHeap, options: ManagedPeFixtureOptions, ): Pick< FixtureMetadataRowContext["indexes"], - "typeSpecSignatures" | "methodSpecInstantiation" + "constructorSignature" | "typeSpecSignatures" | "methodSpecInstantiation" > => ({ + constructorSignature: blobs.add( + options.memberRefSignature ?? Buffer.from([0x20, 0x01, 0x01, 0x0e]), + ), typeSpecSignatures: (options.typeSpecSignatures ?? []).map((signature) => blobs.add(signature), ), - // GENERICINST with one string type argument. + // Defaults to GENERICINST with one string type argument. methodSpecInstantiation: options.methodSpecMethods === undefined ? 0 - : blobs.add(Buffer.from([0x0a, 0x01, 0x0e])), + : blobs.add( + options.methodSpecInstantiation ?? Buffer.from([0x0a, 0x01, 0x0e]), + ), }); const buildManagedFixtureMetadata = ( @@ -899,8 +907,7 @@ const buildManagedFixtureMetadata = ( const methodSignature = blobs.add( options.methodSignature ?? Buffer.from([0x00, 0x00, 0x01]), ); - const constructorSignature = blobs.add(Buffer.from([0x20, 0x01, 0x01, 0x0e])); - const specBlobs = specBlobIndexes(blobs, options); + const referenceBlobs = referenceBlobIndexes(blobs, options); const attributeBlob = blobs.add( fixedStringAttributeBlob( options.targetFramework === null @@ -933,9 +940,8 @@ const buildManagedFixtureMetadata = ( tokenBlob, fieldSignature, methodSignature, - constructorSignature, attributeBlob, - ...specBlobs, + ...referenceBlobs, }, heapSizes: { strings: stringSize, blobs: blobSize, guids: guidSize }, }); diff --git a/src/dotnet/ManagedTypeNames.ts b/src/dotnet/ManagedTypeNames.ts index 998a3d08bc..899c3816cd 100644 --- a/src/dotnet/ManagedTypeNames.ts +++ b/src/dotnet/ManagedTypeNames.ts @@ -2,21 +2,26 @@ import type { ManagedParseIssue } from "../domain/managed/managedArtifact.js"; import { fullName, readCompressed, + specSignature, type TypeRange, } from "./ManagedMemberInspectorCore.js"; -import type { ManagedMetadataLayout } from "./ManagedMetadataLayout.js"; +import { + metadataRowOffset, + type ManagedMetadataLayout, +} from "./ManagedMetadataLayout.js"; import { metadataRowCursor, metadataToken, readMetadataBlob, readMetadataString, } from "./ManagedMetadataHeaps.js"; -import { readManagedValue } from "./ManagedReaderFailure.js"; +import { managedFailure, readManagedValue } from "./ManagedReaderFailure.js"; /** * Name the type a TypeDef, TypeRef, or generic-instantiation TypeSpec token - * denotes, in the namespace-qualified form TypeDef `full_name` uses. Other - * TypeSpec forms and unreadable rows stay null: the type is unknown. + * denotes, in the namespace-qualified form TypeDef `full_name` uses, once per + * token. Other well-formed TypeSpecs stay null: the type is unknown. Malformed + * rows and blobs are reported to `issues` and also stay null. */ export const typeNameLookup = ( bytes: Buffer, @@ -25,19 +30,21 @@ export const typeNameLookup = ( issues: ManagedParseIssue[], ): ((token: string | null) => string | null) => { const names = new Map(); - const resolve = (table: number, row: number): string | null => { - if (table === 2) return ranges[row - 1]?.fullName ?? null; - if (table === 1) { - const cursor = metadataRowCursor(bytes, layout, 1, row); - cursor.readIndex(layout.codedIndexSize("ResolutionScope")); - const name = cursor.readIndex(layout.stringIndexSize); - const namespace = cursor.readIndex(layout.stringIndexSize); - return fullName( - readMetadataString(bytes, layout, namespace, layout.strings.size), - readMetadataString(bytes, layout, name, layout.strings.size), - ); + const definitionName = (table: 1 | 2, row: number): string | null => { + if (table === 2) { + metadataRowOffset(layout, 2, row); + return ranges[row - 1]?.fullName ?? null; } - if (table !== 27) return null; + const cursor = metadataRowCursor(bytes, layout, 1, row); + cursor.readIndex(layout.codedIndexSize("ResolutionScope")); + const name = cursor.readIndex(layout.stringIndexSize); + const namespace = cursor.readIndex(layout.stringIndexSize); + return fullName( + readMetadataString(bytes, layout, namespace, layout.strings.size), + readMetadataString(bytes, layout, name, layout.strings.size), + ); + }; + const specName = (row: number): string | null => { const cursor = metadataRowCursor(bytes, layout, 27, row); const spec = readMetadataBlob( bytes, @@ -45,30 +52,42 @@ export const typeNameLookup = ( cursor.readIndex(layout.blobIndexSize), layout.blob.size, ); - // GENERICINST (CLASS | VALUETYPE) TypeDefOrRefOrSpecEncoded ... - if (spec[0] !== 0x15 || (spec[1] !== 0x12 && spec[1] !== 0x11)) return null; + const owner = { + form: "type", + scope: `metadata.TypeSpec:${metadataToken(27, row)}`, + offset: cursor.start, + } as const; + if (specSignature(layout, spec, owner) === null || spec[0] !== 0x15) + return null; + // GENERICINST (CLASS | VALUETYPE) TypeDefOrRefEncoded: never a TypeSpec, + // so naming never recurses. const encoded = readCompressed(spec, 2).value; - const generic = [2, 1, 27][encoded & 3]; - return generic === undefined - ? null - : lookup(metadataToken(generic, Math.floor(encoded / 4))); + const table = ([2, 1] as const)[encoded & 3]; + if (table === undefined) + throw managedFailure( + "invalid-blob", + owner.scope, + "Generic instantiation does not name a TypeDef or TypeRef", + owner.offset, + ); + return definitionName(table, Math.floor(encoded / 4)); }; - const lookup = (token: string | null): string | null => { + return (token) => { if (token === null) return null; if (names.has(token)) return names.get(token) ?? null; - // Reserve the entry first so a self-referential TypeSpec ends as unknown. - names.set(token, null); + const table = Number.parseInt(token.slice(2, 4), 16); + const row = Number.parseInt(token.slice(4), 16); const name = readManagedValue( () => - resolve( - Number.parseInt(token.slice(2, 4), 16), - Number.parseInt(token.slice(4), 16), - ), + table === 27 + ? specName(row) + : table === 1 || table === 2 + ? definitionName(table, row) + : null, issues, ) ?? null; names.set(token, name); return name; }; - return lookup; };