Conversation
When a runtime-async variant of a cross-module generic method is compiled into an image with --opt-cross-module and inlines another method, CompileMethod records a Check_IL_Body fixup for the AsyncMethodVariant. InliningInfoNode collapsed every inliner to its EcmaMethod and looked up that method's Check_IL_Body import instead, which was never marked, so its index was unset and the checked cast threw OverflowException (or asserted in debug builds). Keep the identity whose IL body fixup was recorded for cross-module inliners while still deduplicating by EcmaMethod, and use the primary EcmaMethod when encoding RIDs. Fixes #134015 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Member
Author
|
cc @richlander |
|
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. |
Contributor
|
Tagging subscribers to this area: @dotnet/crossgen-contrib |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
One or more issues must be addressed before approval.
Review effort: Lite
Findings: 1
Open (2)
What changed in this PR
Fixes crossgen2 overflow/assert failures when emitting ReadyToRun cross-module inlining information for runtime-async generic methods.
Changes:
- Preserves the
AsyncMethodVariantidentity used byCheck_IL_Bodyfixups. - Retains metadata-based RID encoding for stable image format compatibility.
- Adds a regression test covering cross-module generic async inlining.
| File | Description |
|---|---|
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/InliningInfoNode.cs | Updated as part of this pull request. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/R2RTestSuites.cs | Updated as part of this pull request. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/CrossModuleInlining/Dependencies/AsyncCrossModuleGenericLib.cs | Updated as part of this pull request. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/CrossModuleInlining/AsyncGenericInlinerConsumer.cs | Updated as part of this pull request. |
Compiler-generated async thunks cannot carry a Check_IL_Body fixup, so don't report them as cross-module inliners. Extend the regression test so both the task-returning method and its async variant inline the same cross-module inlinee. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…foNode Only report a cross-module inliner when it has its own Check_IL_Body fixup, using the same identity CompileMethod creates the fixup for. Return-dropping thunks, resumption stubs, and other wrappers that unwrap to an EcmaMethod no longer borrow that method's import. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With cross-module inlining enabled (
--opt-cross-module), crossgen2 can fail while emitting the cross-module inlining info table:Debug builds assert
_index != InvalidOffsetat the same place.Root cause
When the runtime-async variant of a cross-module generic method is compiled into the image and inlines another method,
CompileMethodrecords aCheck_IL_Bodyfixup for the method's typical definition, which is theAsyncMethodVariant.InliningInfoNodereduced every inliner to its primaryEcmaMethodand looked up that method'sCheck_IL_Bodyimport instead. Nothing ever marked that import, so it was never placed, its index stayed atint.MinValue, and the checked(uint)cast threw. For anasyncmethod theEcmaMethodis the compiler-generated thunk, so debug builds instead assert while creating the signature.In the reported app, the failing inliner was the async variant of
Task<Completion<...>>.WaitAsync(CancellationToken)(a CoreLib generic over an app type) inliningTimeProvider.get_System().The cause is the async inlining change in #125472. #133146 probably just changed codegen enough to produce this case in the reported app. The bug isn't Wasm-specific: the new test reproduces it on osx-arm64. Browser CoreCLR R2R hits it first because its targets pass
--opt-cross-module:*by default.Fix
CompileMethodandInliningInfoNodenow shareILBodyFixupSignature.GetSignatureMethodForCompiledMethod, which returns the typicalEcmaMethodorAsyncMethodVariantthat gets a compiled method's ownCheck_IL_Bodyfixup, or null if it can't have one.InliningInfoNodekeeps one inliner entry perEcmaMethod, but stores that identity for cross-module inliners. Cross-module inliners with no such identity (async resumption stubs, return-dropping thunks, and other wrappers) or whose identity is a compiler-generated async thunk are not reported, since noCheck_IL_Bodyimport describes them. Inliners inside the version bubble are still encoded byEcmaMethodRID. When both variants inline the same method, the entry chosen doesn't depend on enumeration order. The runtime reader (inlinetracking.cpp,GetILBodyTokenInfo) uses only the module and token from these imports, so the output means the same thing. The image format is unchanged.Validation
AsyncCrossModuleGenericInlinerto the R2R test suite. It fails without the fix (on osx-arm64) and passes with it. It also covers a method whose task-returning and async variants both inline the same cross-module inlinee. The fullILCompiler.ReadyToRun.Testssuite passes locally: 35 passed, 13 skipped for platform.dotnet-inspectbrowser CoreCLR R2R publish (12.0.100-alpha.1.26472.116) with a Debug crossgen2 built from this branch. It now emitsDotnetInspect.Web.Core.dllwithout asserts.Resolves #134015
Note
This PR description was generated with GitHub Copilot.