From 83dc6b406d20871ce80ed2326fee0bb79b35d185 Mon Sep 17 00:00:00 2001 From: Jackson Schuster <36744439+jtschuster@users.noreply.github.com> Date: Fri, 5 Jun 2026 11:05:52 -0700 Subject: [PATCH] Fix composite R2R token corruption for devirtualized async-variant callees In composite ReadyToRun, a runtime-async caller that awaits a devirtualized call to a non-runtime-async virtual method reaches token resolution with the callee's compiler-synthesized async-variant MethodDesc. GetTypicalMethodDefinition() on that variant returns the variant itself rather than the underlying EcmaMethod, so _HandleToModuleToken skipped the methoddef fast path and fell through to the faux-IL token path, indexing the thunk's small synthetic token table with the callee's real methoddef token. This corrupts token resolution (out-of-bounds for larger rows, mis-resolution for low rows) and aborts crossgen2. Insert GetPrimaryMethodDesc() before GetTypicalMethodDefinition() so synthetic wrappers unwrap to the underlying EcmaMethod, matching the idiom already used at the sibling resolver sites. The token and module are then sourced consistently from that EcmaMethod. Add regression tests in ILCompiler.ReadyToRun.Tests (composite, two-assembly) and src/tests/async (single-assembly via [RuntimeAsyncMethodGeneration(false)]). Both abort crossgen2 without the fix and pass with it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../TestCases/R2RTestSuites.cs | 48 +++++++++++++++ .../CompositeAsyncDevirtNonAsyncCalleeMain.cs | 20 +++++++ .../AsyncDevirtNonAsyncCalleeLib.cs | 23 ++++++++ .../JitInterface/CorInfoImpl.ReadyToRun.cs | 6 +- .../devirtualize-inherited-nonasync.cs | 58 +++++++++++++++++++ .../devirtualize-inherited-nonasync.csproj | 5 ++ 6 files changed, 159 insertions(+), 1 deletion(-) create mode 100644 src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/RuntimeAsync/CompositeAsyncDevirtNonAsyncCalleeMain.cs create mode 100644 src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/RuntimeAsync/Dependencies/AsyncDevirtNonAsyncCalleeLib.cs create mode 100644 src/tests/async/devirtualize-inherited-nonasync/devirtualize-inherited-nonasync.cs create mode 100644 src/tests/async/devirtualize-inherited-nonasync/devirtualize-inherited-nonasync.csproj diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/R2RTestSuites.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/R2RTestSuites.cs index 2dd6625bdd286f..601a680f874af0 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/R2RTestSuites.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/R2RTestSuites.cs @@ -1039,6 +1039,54 @@ static void Validate(ReadyToRunReader reader) } } + /// + /// Composite + runtime-async caller awaiting a NON-runtime-async virtual callee that the JIT + /// devirtualizes to a sealed receiver. Resolving the callee's async-variant thunk must unwrap it + /// to the underlying EcmaMethod. + /// + [Fact] + public void CompositeAsyncDevirtNonAsyncCallee() + { + // Compiled WITHOUT runtime-async so the awaited virtuals get synthesized async-variant thunks. + var nonAsyncCalleeLib = new CompiledAssembly + { + AssemblyName = "AsyncDevirtNonAsyncCalleeLib", + SourceResourceNames = ["RuntimeAsync/Dependencies/AsyncDevirtNonAsyncCalleeLib.cs"], + }; + var main = new CompiledAssembly + { + AssemblyName = "CompositeAsyncDevirtNonAsyncCalleeMain", + SourceResourceNames = ["RuntimeAsync/CompositeAsyncDevirtNonAsyncCalleeMain.cs"], + Features = { RuntimeAsyncFeature }, + References = [nonAsyncCalleeLib], + }; + + new R2RTestRunner(_output).Run(new R2RTestCase( + nameof(CompositeAsyncDevirtNonAsyncCallee), + [ + new(nameof(CompositeAsyncDevirtNonAsyncCallee), + [ + new CrossgenAssembly(nonAsyncCalleeLib), + new CrossgenAssembly(main), + ]) + { + Options = [Crossgen2Option.Composite, Crossgen2Option.Optimize], + Validate = Validate, + }, + ])); + + static void Validate(ReadyToRunReader reader) + { + string diag; + Assert.True(R2RAssert.HasManifestRef(reader, "AsyncDevirtNonAsyncCalleeLib", out diag), diag); + + Assert.True(R2RAssert.HasAsyncVariant(reader, "WriterBase.CompleteValueTaskAsync(", out diag), diag); + Assert.True(R2RAssert.HasAsyncVariant(reader, "WriterBase.CompleteTaskAsync(", out diag), diag); + Assert.True(R2RAssert.HasAsyncVariant(reader, "AwaitInheritedValueTask(", out diag), diag); + Assert.True(R2RAssert.HasAsyncVariant(reader, "AwaitInheritedTask(", out diag), diag); + } + } + /// /// Composite with 3 assemblies in A→B→C transitive chain. /// Validates manifest refs for all three and transitive inlining. diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/RuntimeAsync/CompositeAsyncDevirtNonAsyncCalleeMain.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/RuntimeAsync/CompositeAsyncDevirtNonAsyncCalleeMain.cs new file mode 100644 index 00000000000000..bbfd8311e09d92 --- /dev/null +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/RuntimeAsync/CompositeAsyncDevirtNonAsyncCalleeMain.cs @@ -0,0 +1,20 @@ +// Runtime-async caller for the composite devirt regression test. Awaits non-runtime-async virtuals +// (in AsyncDevirtNonAsyncCalleeLib) that the JIT devirtualizes to the sealed receiver, requesting the +// callee's synthesized async-variant thunk. +using System.Runtime.CompilerServices; +using System.Threading.Tasks; + +public static class CompositeAsyncDevirtNonAsyncCallee +{ + [MethodImpl(MethodImplOptions.NoInlining)] + public static async Task AwaitInheritedValueTask(Holder h) + { + await h.Writer.CompleteValueTaskAsync(); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static async Task AwaitInheritedTask(Holder h) + { + await h.Writer.CompleteTaskAsync(); + } +} diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/RuntimeAsync/Dependencies/AsyncDevirtNonAsyncCalleeLib.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/RuntimeAsync/Dependencies/AsyncDevirtNonAsyncCalleeLib.cs new file mode 100644 index 00000000000000..65000d7a5121a7 --- /dev/null +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/RuntimeAsync/Dependencies/AsyncDevirtNonAsyncCalleeLib.cs @@ -0,0 +1,23 @@ +// Helper for the composite runtime-async devirt regression test. Compiled WITHOUT runtime-async, so +// WriterBase's virtuals are classic methods that get an "async variant" thunk when a runtime-async +// caller in another module awaits them. ConcreteWriter is sealed and doesn't override, and Holder +// exposes it base-typed, so the caller late-devirtualizes to the inherited base method. +using System.Threading.Tasks; + +public class WriterBase +{ + public virtual ValueTask CompleteValueTaskAsync() => default; + + public virtual Task CompleteTaskAsync() => Task.CompletedTask; +} + +public sealed class ConcreteWriter : WriterBase +{ +} + +public sealed class Holder +{ + private readonly ConcreteWriter _writer = new ConcreteWriter(); + + public WriterBase Writer => _writer; +} diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index 8c4af968d66747..9bf0b49c148b8b 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -1427,8 +1427,12 @@ ModuleToken _HandleToModuleToken(ref CORINFO_RESOLVED_TOKEN pResolvedToken, Meth || (pResolvedToken.tokenType == CorInfoTokenKind.CORINFO_TOKENKIND_DevirtualizedMethod) || methodDesc.IsPInvoke)) { + // Unwrap synthetic MethodDesc wrappers (e.g. async-variant thunks) to the + // underlying metadata method before resolving its token. For a devirtualized + // callee, resolveVirtualMethod guarantees a real methoddef token in that + // method's own EcmaModule, so token and module are sourced consistently here. if ((CorTokenType)(unchecked((uint)pResolvedToken.token) & 0xFF000000u) == CorTokenType.mdtMethodDef && - methodDesc?.GetTypicalMethodDefinition() is EcmaMethod ecmaMethod) + methodDesc?.GetPrimaryMethodDesc().GetTypicalMethodDefinition() is EcmaMethod ecmaMethod) { mdToken token = (mdToken)MetadataTokens.GetToken(ecmaMethod.Handle); diff --git a/src/tests/async/devirtualize-inherited-nonasync/devirtualize-inherited-nonasync.cs b/src/tests/async/devirtualize-inherited-nonasync/devirtualize-inherited-nonasync.cs new file mode 100644 index 00000000000000..ae9971c5141a8c --- /dev/null +++ b/src/tests/async/devirtualize-inherited-nonasync/devirtualize-inherited-nonasync.cs @@ -0,0 +1,58 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.CompilerServices; +using System.Threading.Tasks; +using Xunit; + +// A runtime-async caller awaits a devirtualized call to a NON-runtime-async virtual method that a +// sealed receiver inherits without overriding. Resolving the callee's synthesized async-variant thunk +// must unwrap it to the underlying method instead of indexing the thunk's small token table with the +// callee's real methoddef token; otherwise composite ReadyToRun compilation corrupts token resolution +// and crossgen2 aborts. +public class Async2DevirtualizeInheritedNonAsync +{ + public class WriterBase + { + // Non-runtime-async virtuals with bodies: awaited from a runtime-async caller via a thunk. + [RuntimeAsyncMethodGeneration(false)] + public virtual ValueTask CompleteValueTaskAsync() => default; + + [RuntimeAsyncMethodGeneration(false)] + public virtual Task CompleteTaskAsync() => Task.CompletedTask; + } + + // Sealed and does NOT override: late devirtualization resolves to the inherited base methods. + public sealed class ConcreteWriter : WriterBase + { + } + + public sealed class Holder + { + private readonly ConcreteWriter _writer = new ConcreteWriter(); + + // Base-typed accessor over the sealed concrete type, so the exact receiver is only known via + // late devirtualization. + public WriterBase Writer => _writer; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static async Task AwaitInheritedValueTask(Holder h) + { + await h.Writer.CompleteValueTaskAsync(); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static async Task AwaitInheritedTask(Holder h) + { + await h.Writer.CompleteTaskAsync(); + } + + [Fact] + public static void TestEntryPoint() + { + var h = new Holder(); + AwaitInheritedValueTask(h).GetAwaiter().GetResult(); + AwaitInheritedTask(h).GetAwaiter().GetResult(); + } +} diff --git a/src/tests/async/devirtualize-inherited-nonasync/devirtualize-inherited-nonasync.csproj b/src/tests/async/devirtualize-inherited-nonasync/devirtualize-inherited-nonasync.csproj new file mode 100644 index 00000000000000..197767e2c4e249 --- /dev/null +++ b/src/tests/async/devirtualize-inherited-nonasync/devirtualize-inherited-nonasync.csproj @@ -0,0 +1,5 @@ + + + + +