Write custom modifiers in PersistedAssemblyBuilder's order in CoreCLR's AssemblyBuilder by caraioniurie47 · Pull Request #136699 · dotnet/runtime

caraioniurie47 · 2026-10-30T12:16:28Z

As asked in #136450 (r4414670923, r4418172093): CoreCLR's AssemblyBuilder and PersistedAssemblyBuilder wrote custom modifiers in different orders. With required [IsConst, IsVolatile] and optional [IsLong, IsBoxed] passed to DefineField or DefineMethod on .NET 10.0.14:

AssemblyBuilder:           modopt(IsLong) modopt(IsBoxed) modreq(IsConst) modreq(IsVolatile) int32
PersistedAssemblyBuilder:  modreq(IsVolatile) modreq(IsConst) modopt(IsBoxed) modopt(IsLong) int32

PersistedAssemblyBuilder takes each array in the order GetRequiredCustomModifiers and GetOptionalCustomModifiers return it, the reverse of signature order, and writes the required modifiers first. CoreCLR's SignatureHelper took each array in signature order and wrote the optional modifiers first.

SignatureHelper now writes PersistedAssemblyBuilder's order. That includes member references to a loaded target in another module, built from its reflected modifiers. DispatchProxy and MEF's MetadataViewGenerator reversed the arrays they read from reflection to fit the old order; they no longer do, except that MEF's net10.0 and net11.0 builds still reverse them when running on .NET 10 or 11. ComImportInteropInterfaceDetailsStrategy already passed the arrays it reads from reflection unreversed, so its forwarders now write custom modifiers in the same order as DispatchProxy; that path isn't tested here.

This is a breaking change for callers relying on the old modifier order. Member references built from reflected modifiers now match only signatures listing required modifiers first. On CoreCLR, for targets whose parameter is modopt(IsLong) modreq(IsVolatile) int32, calls to a generic method, a vararg method, or a method or constructor of a generic type, and ldftn to a generic method, bound on .NET 10.0.14 and now throw MissingMethodException, and MEF's net12.0 build can't create a metadata view whose property has that type. As r4418172093 notes, the C# compiler does not generate signatures that would be affected. This needs the breaking-change label.

Mono's AssemblyBuilder is a separate implementation that this change does not touch, so on Mono .NET 12, DispatchProxy and MEF now pass it two modifiers of one kind in the reverse of their old order. As r4407294021 suggests for Mono, the DispatchProxy tests this affects are marked SkipOnMono. So are the two signature tests in TypeBuilderCustomModifierOrder.cs: Mono's builder writes each array in the order given, and its Module.ResolveSignature resolves only standalone signature tokens.

New tests: TypeBuilderCustomModifierOrder.cs (both builders write PersistedAssemblyBuilder's signature above, and AssemblyBuilder calls a generic method and a method of a generic type written by PersistedAssemblyBuilder), DispatchProxyTests.cs and MetadataViewProviderTests.cs (a proxy and a metadata view for interfaces written by PersistedAssemblyBuilder). On CoreCLR, they fail without the change, except PersistedAssemblyBuilder_CustomModifierOrder. SignatureHelperGetPropertySigHelper.cs changes because required modifiers are now validated first.

Note

AI-generated, written at my direction and reviewed by me before posting.
Tests ran on Windows 11 x64, Release; the signatures above are from .NET 10.0.14 on Windows.

CoreCLR's RuntimeAssemblyBuilder took each custom modifier array in signature order and wrote the optional modifiers first. PersistedAssemblyBuilder takes each array in the order GetRequiredCustomModifiers and GetOptionalCustomModifiers return it, the reverse of signature order, and writes the required modifiers first. CoreCLR's SignatureHelper now writes PersistedAssemblyBuilder's order, for module signatures and for DynamicMethod signatures alike. DispatchProxy and MEF's MetadataViewGenerator no longer reverse the arrays they read from reflection. Mono's AssemblyBuilder is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

azure-pipelines · 2026-10-30T12:17:14Z

Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run.

dotnet-policy-service · 2026-10-30T12:17:51Z

Tagging subscribers to this area: @steveisok, @dotnet/area-system-reflection-emit
See info in area-owners.md if you want to be subscribed.

Name the flag that selects required or optional custom modifiers isOptional, as CustomModifiersEncoder.AddModifier does: in the SignatureHelper writers, in ModifiedType.GetCustomModifiers on each runtime, and in CoreCLR's Signature.GetCustomModifiers. The Signature_GetCustomModifiersAtOffset QCall keeps its required flag, which it compares with the required byte of internal modifiers. State the modifier order in the writers' loops, and inline the property's custom modifiers in MetadataViewGenerator. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

caraioniurie47 · 2026-10-31T11:22:19Z

Mono's ILGenerator doesn't implement EmitCalli(Type), so the two new EmitCalli tests in Emit4Tests.cs now use SkipOnMono instead of ActiveIssue for #124149, which is closed.

jkotas

Thanks

Copilot

🟡 Changes recommended

Unconditionally removing modifier reversals breaks metadata-view creation on older targets such as .NET 10 by causing getter signatures to mismatch their interfaces.

1 open issue

Preserve modifier reversals for older runtime emitters · New

What changed in this PR

This PR aligns CoreCLR’s custom-modifier emission with PersistedAssemblyBuilder, so reflection-provided modifier arrays can be passed directly to the emitter.

Changes:

Emit required modifiers first and reverse each modifier array in regular and dynamic signatures.
Remove modifier-array reversals from DispatchProxy and MEF, and standardize internal boolean parameters as isOptional.
Add ordering and integration tests, update validation expectations, and mark affected tests as skipped on Mono.

File	Description
src/mono/System.Private.CoreLib/src/System/Reflection/ModifiedType.Mono.cs	Adopt `isOptional` semantics while preserving modifier selection.
src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderCustomModifierOrder.cs	Test runtime and persisted signature ordering.
src/libraries/System.Reflection.Emit/tests/System.Reflection.Emit.Tests.csproj	Include the new ordering tests.
src/libraries/System.Reflection.Emit.ILGeneration/tests/SignatureHelper/SignatureHelperGetPropertySigHelper.cs	Expect required-modifier validation first.
src/libraries/System.Reflection.Emit.ILGeneration/tests/ILGenerator/Emit4Tests.cs	Test dynamic-signature modifier validation order.
src/libraries/System.Reflection.DispatchProxy/tests/DispatchProxyTests.cs	Add persisted-interface coverage and share assertions.
src/libraries/System.Reflection.DispatchProxy/src/System/Reflection/DispatchProxyGenerator.cs	Pass reflected modifier arrays without reversal.
src/libraries/System.Private.CoreLib/src/System/Reflection/ModifiedType.cs	Use named `isOptional` arguments.
src/libraries/System.ComponentModel.Composition/tests/System/ComponentModel/Composition/MetadataViewProviderTests.cs	Test metadata views with persisted custom modifiers.
src/libraries/System.ComponentModel.Composition/src/System/ComponentModel/Composition/MetadataViewGenerator.cs	Remove getter modifier-array reversals.
src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs	Translate `isOptional` to the native required-modifier flag.
src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs	Update property modifier-selection calls.
src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeParameterInfo.CoreCLR.cs	Update parameter modifier-selection calls.
src/coreclr/System.Private.CoreLib/src/System/Reflection/RtFieldInfo.cs	Update field modifier-selection calls.
src/coreclr/System.Private.CoreLib/src/System/Reflection/ModifiedType.CoreCLR.cs	Forward `isOptional` through signature lookup.
src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/SignatureHelper.cs	Implement the new ordering in both emission paths.
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/ModifiedType.NativeAot.cs	Preserve modifier filtering with `isOptional` semantics.

_{Review effort: Balanced}

MEF also ships net10.0 and net11.0 builds, which run on runtimes whose AssemblyBuilder still takes custom modifiers of one kind in the reverse of the order reflection returns them in. MetadataViewGenerator reverses them again when Environment.Version is below 12; the runtime, not the target framework, decides, since those builds also run on .NET 12. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Environment.Version.Major < 12 is always false where the net12.0 build runs, so put the field and its use under !NET12_0_OR_GREATER. The reversal compiles out of net12.0 once the SDK defines that symbol, and can be deleted once net12.0 is the oldest target framework. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

pcshrosbree

This PR also changes the custom-modifier order in the MemberRefs that RuntimeModuleBuilder emits for calls to generic methods and to methods of generic types. I have a test request and a breaking-change case inline.

On CoreCLR, a call to a loaded generic method, or to a method of a loaded generic type, in another module gets a member reference whose signature the runtime builder writes from the target's reflected custom modifiers. With the old order, such a call to a PersistedAssemblyBuilder method with two required modifiers threw MissingMethodException. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

jkotas

Thanks

dotnet-policy-service · 2026-11-04T03:58:42Z

Added needs-breaking-change-doc-created label because this PR has the breaking-change label.

When you commit this breaking change:

Create and link to this PR and the issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.
Ask a committer to mail the .NET Breaking Change Notification DL.

Tagging @dotnet/compat for awareness of the breaking change.

jkotas · 2026-11-04T04:41:09Z

Breaking change doc created dotnet/docs#56739

Conflict in SignatureHelper.cs with dotnet#136699, resolved by keeping its AddCustomModifiers and dropping this PR's AddReflectedCustomModifiers and AddCustomModifier, which did the same. A modified type's own modifiers now go through AddCustomModifiers, whose messages name the modifier and whose paramName may be null for them. The comments in the resolved code no longer give PersistedAssemblyBuilder as the reason. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

DefineMethod_ModifiedParameterTypes_CopiesSignature defines its source method with modifier arrays. Since dotnet#136699, DefineMethod takes each array in the order reflection returns it, so the test now expects the source's and the copy's modifiers in the arrays' order. A modified List<int> from Type.MakeModifiedSignatureType was written without its modifiers on .NET 11 RC1. A test of its own now checks that its modifier comes back, and fails without this PR. The constructed-generic ldtoken test now says what its first-byte check shows. The comment on the worker call now says that the other overload would also write modifiers an array replaced, and a comment sentence that restated the code is gone. The worker calls name lastWasGenericInst, and the module check uses "is not null", as the repository's C# guidelines ask. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

github-actions Bot added the area-System.Reflection.Emit label Oct 30, 2026

dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Oct 30, 2026

caraioniurie47 mentioned this pull request Oct 30, 2026

Encode modified types in TypeBuilder signatures #136450

Open

build-analysis Bot mentioned this pull request Oct 30, 2026

Crash: access violation in coreclr!PgoManager::getPgoInstrumentationResults during tier-1 background JIT (Windows x86) #134695

Open

jkotas reviewed Oct 30, 2026

View reviewed changes

caraioniurie47 requested review from MichalStrehovsky, steveisok and vitek-karas as code owners October 31, 2026 11:17

jkotas approved these changes Nov 1, 2026

View reviewed changes

jkotas mentioned this pull request Nov 1, 2026

System.Xml.CharCheckingReaderTests.CharCheckingReaderTest fails with OutOfMemoryException #136776

Open

jkotas requested a balanced review from Copilot November 1, 2026 07:21

Copilot started reviewing on behalf of jkotas November 1, 2026 07:22

View session

Copilot AI reviewed Nov 1, 2026

View reviewed changes

Comment thread ...em.ComponentModel.Composition/src/System/ComponentModel/Composition/MetadataViewGenerator.cs Resolved

jkotas reviewed Nov 1, 2026

View reviewed changes

Comment thread src/libraries/System.Reflection.Emit.ILGeneration/tests/ILGenerator/Emit4Tests.cs Resolved

jkotas reviewed Nov 1, 2026

View reviewed changes

Comment thread ...em.ComponentModel.Composition/src/System/ComponentModel/Composition/MetadataViewGenerator.cs Resolved

build-analysis Bot mentioned this pull request Nov 1, 2026

[ci-scan] Test failure: System.Net.Http.Functional.Tests.HttpClientHandlerTest_Http3.SendAsync_RequestRejected_ClientRetries #133059

Open

pcshrosbree reviewed Nov 2, 2026

View reviewed changes

Comment thread src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/SignatureHelper.cs Resolved

build-analysis Bot mentioned this pull request Nov 2, 2026

Test failure: IdsConversionsWithRegionTest America/Vancouver America/Los_Angeles mismatch #136758

Closed

jkotas reviewed Nov 3, 2026

View reviewed changes

Comment thread src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderCustomModifierOrder.cs Resolved

Drop CoreCLR from a test comment

e92df7d

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

build-analysis Bot mentioned this pull request Nov 3, 2026

slow macOS - "##[error]The job running on agent Azure Pipelines 9 ran longer than the maximum time of 60 minutes." dotnet/dnceng#1883

Open

3 tasks

jkotas approved these changes Nov 4, 2026

View reviewed changes

jkotas merged commit f55fa83 into dotnet:main Nov 4, 2026
136 of 139 checks passed

jkotas added the breaking-change Issue or PR that represents a breaking API or functional change over a previous release. label Nov 4, 2026

dotnet-policy-service Bot added the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Nov 4, 2026

dotnet-policy-service Bot deployed to copilot-pat-pool November 4, 2026 03:58 Active

dotnet-policy-service Bot deployed to copilot-pat-pool November 4, 2026 03:59 Active

jkotas removed the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Nov 4, 2026

dotnet-maestro Bot mentioned this pull request Nov 4, 2026

[main] Source code updates from dotnet/runtime dotnet/dotnet#10605

Merged

dotnet-milestone-bot Bot added this to the 12.0-preview1 milestone Nov 4, 2026

Conversation

caraioniurie47 commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azure-pipelines Bot commented Oct 30, 2026

Uh oh!

dotnet-policy-service Bot commented Oct 30, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

caraioniurie47 commented Oct 31, 2026

Uh oh!

jkotas left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pcshrosbree left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jkotas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dotnet-policy-service Bot commented Nov 4, 2026

Uh oh!

jkotas commented Nov 4, 2026

Uh oh!

This branch was successfully deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

caraioniurie47 commented Oct 30, 2026 •

edited

Loading