Skip to content

Commit 83dc6b4

Browse files
jtschusterCopilot
andcommitted
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>
1 parent 4c16f5f commit 83dc6b4

6 files changed

Lines changed: 159 additions & 1 deletion

File tree

src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/R2RTestSuites.cs

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1039,6 +1039,54 @@ static void Validate(ReadyToRunReader reader)
10391039
}
10401040
}
10411041

1042+
/// <summary>
1043+
/// Composite + runtime-async caller awaiting a NON-runtime-async virtual callee that the JIT
1044+
/// devirtualizes to a sealed receiver. Resolving the callee's async-variant thunk must unwrap it
1045+
/// to the underlying EcmaMethod.
1046+
/// </summary>
1047+
[Fact]
1048+
public void CompositeAsyncDevirtNonAsyncCallee()
1049+
{
1050+
// Compiled WITHOUT runtime-async so the awaited virtuals get synthesized async-variant thunks.
1051+
var nonAsyncCalleeLib = new CompiledAssembly
1052+
{
1053+
AssemblyName = "AsyncDevirtNonAsyncCalleeLib",
1054+
SourceResourceNames = ["RuntimeAsync/Dependencies/AsyncDevirtNonAsyncCalleeLib.cs"],
1055+
};
1056+
var main = new CompiledAssembly
1057+
{
1058+
AssemblyName = "CompositeAsyncDevirtNonAsyncCalleeMain",
1059+
SourceResourceNames = ["RuntimeAsync/CompositeAsyncDevirtNonAsyncCalleeMain.cs"],
1060+
Features = { RuntimeAsyncFeature },
1061+
References = [nonAsyncCalleeLib],
1062+
};
1063+
1064+
new R2RTestRunner(_output).Run(new R2RTestCase(
1065+
nameof(CompositeAsyncDevirtNonAsyncCallee),
1066+
[
1067+
new(nameof(CompositeAsyncDevirtNonAsyncCallee),
1068+
[
1069+
new CrossgenAssembly(nonAsyncCalleeLib),
1070+
new CrossgenAssembly(main),
1071+
])
1072+
{
1073+
Options = [Crossgen2Option.Composite, Crossgen2Option.Optimize],
1074+
Validate = Validate,
1075+
},
1076+
]));
1077+
1078+
static void Validate(ReadyToRunReader reader)
1079+
{
1080+
string diag;
1081+
Assert.True(R2RAssert.HasManifestRef(reader, "AsyncDevirtNonAsyncCalleeLib", out diag), diag);
1082+
1083+
Assert.True(R2RAssert.HasAsyncVariant(reader, "WriterBase.CompleteValueTaskAsync(", out diag), diag);
1084+
Assert.True(R2RAssert.HasAsyncVariant(reader, "WriterBase.CompleteTaskAsync(", out diag), diag);
1085+
Assert.True(R2RAssert.HasAsyncVariant(reader, "AwaitInheritedValueTask(", out diag), diag);
1086+
Assert.True(R2RAssert.HasAsyncVariant(reader, "AwaitInheritedTask(", out diag), diag);
1087+
}
1088+
}
1089+
10421090
/// <summary>
10431091
/// Composite with 3 assemblies in A→B→C transitive chain.
10441092
/// Validates manifest refs for all three and transitive inlining.
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Runtime-async caller for the composite devirt regression test. Awaits non-runtime-async virtuals
2+
// (in AsyncDevirtNonAsyncCalleeLib) that the JIT devirtualizes to the sealed receiver, requesting the
3+
// callee's synthesized async-variant thunk.
4+
using System.Runtime.CompilerServices;
5+
using System.Threading.Tasks;
6+
7+
public static class CompositeAsyncDevirtNonAsyncCallee
8+
{
9+
[MethodImpl(MethodImplOptions.NoInlining)]
10+
public static async Task AwaitInheritedValueTask(Holder h)
11+
{
12+
await h.Writer.CompleteValueTaskAsync();
13+
}
14+
15+
[MethodImpl(MethodImplOptions.NoInlining)]
16+
public static async Task AwaitInheritedTask(Holder h)
17+
{
18+
await h.Writer.CompleteTaskAsync();
19+
}
20+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Helper for the composite runtime-async devirt regression test. Compiled WITHOUT runtime-async, so
2+
// WriterBase's virtuals are classic methods that get an "async variant" thunk when a runtime-async
3+
// caller in another module awaits them. ConcreteWriter is sealed and doesn't override, and Holder
4+
// exposes it base-typed, so the caller late-devirtualizes to the inherited base method.
5+
using System.Threading.Tasks;
6+
7+
public class WriterBase
8+
{
9+
public virtual ValueTask CompleteValueTaskAsync() => default;
10+
11+
public virtual Task CompleteTaskAsync() => Task.CompletedTask;
12+
}
13+
14+
public sealed class ConcreteWriter : WriterBase
15+
{
16+
}
17+
18+
public sealed class Holder
19+
{
20+
private readonly ConcreteWriter _writer = new ConcreteWriter();
21+
22+
public WriterBase Writer => _writer;
23+
}

src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1427,8 +1427,12 @@ ModuleToken _HandleToModuleToken(ref CORINFO_RESOLVED_TOKEN pResolvedToken, Meth
14271427
|| (pResolvedToken.tokenType == CorInfoTokenKind.CORINFO_TOKENKIND_DevirtualizedMethod)
14281428
|| methodDesc.IsPInvoke))
14291429
{
1430+
// Unwrap synthetic MethodDesc wrappers (e.g. async-variant thunks) to the
1431+
// underlying metadata method before resolving its token. For a devirtualized
1432+
// callee, resolveVirtualMethod guarantees a real methoddef token in that
1433+
// method's own EcmaModule, so token and module are sourced consistently here.
14301434
if ((CorTokenType)(unchecked((uint)pResolvedToken.token) & 0xFF000000u) == CorTokenType.mdtMethodDef &&
1431-
methodDesc?.GetTypicalMethodDefinition() is EcmaMethod ecmaMethod)
1435+
methodDesc?.GetPrimaryMethodDesc().GetTypicalMethodDefinition() is EcmaMethod ecmaMethod)
14321436
{
14331437
mdToken token = (mdToken)MetadataTokens.GetToken(ecmaMethod.Handle);
14341438

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Runtime.CompilerServices;
5+
using System.Threading.Tasks;
6+
using Xunit;
7+
8+
// A runtime-async caller awaits a devirtualized call to a NON-runtime-async virtual method that a
9+
// sealed receiver inherits without overriding. Resolving the callee's synthesized async-variant thunk
10+
// must unwrap it to the underlying method instead of indexing the thunk's small token table with the
11+
// callee's real methoddef token; otherwise composite ReadyToRun compilation corrupts token resolution
12+
// and crossgen2 aborts.
13+
public class Async2DevirtualizeInheritedNonAsync
14+
{
15+
public class WriterBase
16+
{
17+
// Non-runtime-async virtuals with bodies: awaited from a runtime-async caller via a thunk.
18+
[RuntimeAsyncMethodGeneration(false)]
19+
public virtual ValueTask CompleteValueTaskAsync() => default;
20+
21+
[RuntimeAsyncMethodGeneration(false)]
22+
public virtual Task CompleteTaskAsync() => Task.CompletedTask;
23+
}
24+
25+
// Sealed and does NOT override: late devirtualization resolves to the inherited base methods.
26+
public sealed class ConcreteWriter : WriterBase
27+
{
28+
}
29+
30+
public sealed class Holder
31+
{
32+
private readonly ConcreteWriter _writer = new ConcreteWriter();
33+
34+
// Base-typed accessor over the sealed concrete type, so the exact receiver is only known via
35+
// late devirtualization.
36+
public WriterBase Writer => _writer;
37+
}
38+
39+
[MethodImpl(MethodImplOptions.NoInlining)]
40+
private static async Task AwaitInheritedValueTask(Holder h)
41+
{
42+
await h.Writer.CompleteValueTaskAsync();
43+
}
44+
45+
[MethodImpl(MethodImplOptions.NoInlining)]
46+
private static async Task AwaitInheritedTask(Holder h)
47+
{
48+
await h.Writer.CompleteTaskAsync();
49+
}
50+
51+
[Fact]
52+
public static void TestEntryPoint()
53+
{
54+
var h = new Holder();
55+
AwaitInheritedValueTask(h).GetAwaiter().GetResult();
56+
AwaitInheritedTask(h).GetAwaiter().GetResult();
57+
}
58+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
<ItemGroup>
3+
<Compile Include="$(MSBuildProjectName).cs" />
4+
</ItemGroup>
5+
</Project>

0 commit comments

Comments
 (0)