Skip to content

AssemblyIdentityReader: FindDeclaringType always returns the first TypeDef row #72

Description

@gfraiteur

Summary

AssemblyIdentityReader.FindDeclaringType resolves the type that declares the constructor of a custom attribute by walking the MethodList run lists of the TypeDef table. The walk returns the first row that satisfies its predicate instead of the last, and the first row always satisfies it, so the method returns <Module> for every input.

As a consequence, a custom attribute whose constructor is a MethodDef — that is, an attribute whose type is defined by the assembly that applies it — is never recognized.

Diagnosis

for ( MetadataRow row = tables.TypeDefTable.GetRow( 0 ); !row.IsNull; row = tables.TypeDefTable.GetNextRow( row ) )
{
    if ( tables.TypeDefMethodListColumn.GetValue( row ).Index <= constructorToken.Index )
        return tables.TypeDefTable.GetToken( row );
}

Row 0 is <Module>, whose MethodList is 1, i.e. index 0. The condition 0 <= constructorToken.Index holds for every token, so the loop returns on its first iteration and never inspects another row. A run list designates the beginning of a run, so the run that contains a given token is the one of the last row whose value is smaller or equal to it, not the first.

MatchesCustomAttribute therefore compares the requested namespace and name against <Module>, which matches nothing, and returns false.

Consequence

ReadAssemblyIdentity uses this to detect exactly two attributes:

  • System.Runtime.Versioning.TargetFrameworkAttribute
  • System.Runtime.CompilerServices.ReferenceAssemblyAttribute

Both are defined in the very assemblies that apply them: mscorlib, System.Runtime, netstandard, and the reference assemblies. For those assemblies the constructor is a MethodDef, so neither attribute is ever found and the identity is read with TargetFramework == null and IsReferenceAssembly left to whatever the processor architecture implies.

The extent of the user-visible impact has not been measured. The comment above the call site notes that the same algorithm is implemented in the AssemblyEnvelope constructor and in RuntimeAssemblyLocator.ProbeCurrentAppDomain, so a correct value may be obtained through another path, and processorArchitecture == NoPlatform already forces IsReferenceAssembly for many reference assemblies.

Second defect on the same line

MetadataToken.Index throws InvalidOperationException when the token is null, in release builds as well as in debug ones: AssertValidOperation is not compiled out.

GetValue returns a null token when the raw column value is zero, which is exactly what a MethodDef table of exactly 65535 rows produces (see #71: the end-of-run sentinel does not fit in the 2-byte column that ECMA-335 II.24.2.6 mandates, and is written truncated to zero).

Today this is unreachable, because the loop returns at row 0 before reaching a trailing row. Fixing the walk without also handling the null token makes it reachable, and it then throws while reading assembly identity — that is, during binding, before the module is loaded, which is earlier and harder to diagnose than the crash reported in #71. The two defects have to be fixed together.

Affected source

  • Core/PostSharp.Compiler.Engine/Sdk/CodeModel/Binding/AssemblyIdentityReader.csFindDeclaringType

Reproduction

Emit a module whose TypeDef table is, in order: <Module>, a type owning a few methods, and System.Runtime.Versioning.TargetFrameworkAttribute owning a .ctor(string); then add a CustomAttribute row whose parent is the Assembly row and whose type is that constructor. Read it with AssemblyIdentityReader.GetAssemblyBindingIdentity. The returned identity has TargetFramework == null.

Repeating this with a MethodDef table of exactly 65535 rows and a trailing type owning no method covers the null-token dereference described above.

Proposed fix

  1. Walk the run lists for the row whose run contains the token — the last row whose value is smaller or equal to it — or, better, resolve the range through MetadataColumnRowIndex.GetValueRange, which already handles the boundary and the clamping. The // TODO: Add binary search comment on the method suggests a containment search was the intent.
  2. Never call MetadataToken.Index on the raw column value without testing IsNull first.

Environment

  • Present on release/2024.0 and unchanged on release/2026.0.

-- Claude for Gael Fraiteur

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions