Conversation
- Add UseRuntimeAsync property for source projects - Add TestRuntimeAsync property for test projects - Enable RuntimeAsync with crossgen2 - Add pipeline leg for runtimeasync tests with crossgen2
|
/azp run runtime-coreclr crossgen2 |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This PR adds initial support for emitting and consuming runtime-async-related methods in crossgen2 ReadyToRun (R2R) images, including new signature flags and runtime-side lookup/GC-walk plumbing for async variants and resumption stubs.
Changes:
- Enables
runtime-async=onfeature flagging for selected source and test projects, and adds a dedicated CI leg to run R2R + runtime-async library tests. - Extends R2R method signature encoding/decoding to represent async variants and resumption stubs, and updates crossgen2 tooling/readers to surface these modifiers.
- Adds runtime support for locating and registering resumption stub entrypoints so stack walking/GC can associate R2R resumption stubs with a
MethodDesc.
Reviewed changes
Copilot reviewed 35 out of 36 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/Directory.Build.targets | Enables preview features + runtime-async=on for source projects when UseRuntimeAsync=true. |
| eng/testing/tests.targets | Adjusts runtime-async enablement for tests and adds TestRuntimeAsync override knob. |
| eng/pipelines/coreclr/crossgen2.yml | Adds a new CI test matrix leg for R2R + runtime-async libraries testing. |
| src/coreclr/inc/corcompile.h | Adds ENCODE_METHOD_SIG_ResumptionStub flag for method signature encoding. |
| src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs | Adds managed enum flag READYTORUN_METHOD_SIG_ResumptionStub. |
| src/coreclr/vm/zapsig.cpp | Reads new ResumptionStub flag during method sig decode (currently unused in logic). |
| src/coreclr/vm/stackwalk.cpp | Relaxes a debug assert for unwind table registration to accommodate async R2R scenarios. |
| src/coreclr/vm/readytoruninfo.h | Declares runtime API to look up resumption stub entrypoints for async variants. |
| src/coreclr/vm/readytoruninfo.cpp | Implements resumption stub lookup and registers R2R-backed stub MethodDesc for GC stack walks. |
| src/coreclr/vm/method.hpp | Extends async lookup enum and dynamic IL stub types to represent R2R resumption stubs. |
| src/coreclr/vm/methodtable.cpp | Adds an AsyncResumptionStub lookup path (currently duplicative of the existing slow path). |
| src/coreclr/vm/ilstubcache.h | Declares helper to create a DynamicMethodDesc wrapper around precompiled (R2R) stub code. |
| src/coreclr/vm/ilstubcache.cpp | Implements creation of an R2R-backed IL-stub MethodDesc with native entrypoint set directly. |
| src/coreclr/vm/jitinterface.cpp | Tweaks READYTORUN_HELPER handling (includes an unexpected printf). |
| src/coreclr/inc/readytorunhelpers.h | Adds mapping for READYTORUN_HELPER_ThrowExact. |
| src/coreclr/inc/readytorun.h | Clarifies formatting/commenting for async continuation helpers. |
| src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/ReadyToRunSignature.cs | Shows [RESUME] in method display and improves BadImageFormatException message. |
| src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/ReadyToRunReader.cs | Tracks async/resume modifiers when parsing instance method + PGO sections. |
| src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/ReadyToRunMethod.cs | Stores/display method modifiers (async/resume) in signature string. |
| src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/PgoInfoKey.cs | Includes modifiers in PGO key signature string generation. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj | Adds AsyncMethodVariant.Mangling.cs to the build. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/IL/ReadyToRunILProvider.cs | Broadens IL provisioning to handle async variants and resumption stubs. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunTableManager.cs | Categorizes async variants/resumption stubs with instantiated methods for table emission. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs | Avoids inlining async call/thunk methods and force-adds required async metadata references once. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunSymbolNodeFactory.cs | Minor whitespace/style fix. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/SignatureBuilder.cs | Emits ResumptionStub flag and hashes resumption stubs with their async variant method signature. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ModuleTokenResolver.cs | Adjusts method token resolution and adds field token resolution helper. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs | Ensures async variants/resumption stubs aren’t incorrectly optimized as ordinary defs. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/InstanceEntryPointTableNode.cs | Handles async variants/resumption stubs in instantiated entrypoint table emission. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/InliningInfoNode.cs | Skips emitting inlining info for async thunks and avoids work for methods with no inlinees. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ExceptionInfoLookupTableNode.cs | Skips EH-info table processing for resumption stubs. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs | Adjusts GC map encoding size computation (currently incorrect for non-64-bit pointer sizes). |
| src/coreclr/tools/Common/TypeSystem/IL/Stubs/AsyncResumptionStub.cs | Updates diagnostic naming and hashing; marks token generation on emit. |
| src/coreclr/tools/Common/TypeSystem/IL/InstantiatedMethodIL.cs | Relaxes a Debug.Assert to accommodate non-standard owning method relationships. |
| src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs | Enables READYTORUN path to provide an async resumption stub and relaxes a debug assert for async variants. |
| src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.Async.cs | Tracks continuation types as valid types. |
| src/coreclr/tools/Common/Compiler/AsyncMethodVariant.cs | Treats resumption stubs as async thunks for compilation decisions. |
Comments suppressed due to low confidence (1)
src/coreclr/vm/jitinterface.cpp:14153
- Avoid calling
printffrom the VM here. This will write to stdout in production and can interfere with host output; it also bypasses existing logging/diagnostics patterns already present in this block (STRESS_LOG+_ASSERTE). Please remove theprintfand rely on the existing logging/assertion mechanisms (or route through the runtime logging infrastructure if an additional message is needed).
result = (size_t)GetEEFuncEntryPoint(DelayLoad_Helper_Obj);
break;
case READYTORUN_HELPER_DelayLoad_Helper_ObjObj:
...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
|
/azp run runtime-coreclr crossgen2 |
|
Azure Pipelines successfully started running 1 pipeline(s). |
The R2R async resumption stub calls the target async variant method through a standard delay-load import cell (MethodEntry fixup). When the async variant method gets tiered up, the import cell or its underlying precode gets backpatched to the Tier1 code. But continuations created by R2R code have a register layout specific to R2R's compilation, and resuming them through Tier1 code (which expects a different layout) causes a SIGSEGV. Fix this by emitting a direct call from the resumption stub to the compiled method body (MethodWithGCInfo) instead of going through an import cell. Since MethodWithGCInfo.RepresentsIndirectionCell is false, the JIT emits a direct call relocation to the R2R method body that cannot be backpatched by tiering. This matches the JIT's approach where each code version gets its own pinned m_finalCodeAddressSlot for the resume stub to call through. Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/coreclr/vm/readytoruninfo.cpp:21
sigbuilder.his included here but doesn't appear to be used anywhere in this translation unit (the resumption stub signature is a static byte array). Consider removing the unused include to avoid unnecessary dependencies/build overhead.
| public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false) | ||
| { | ||
| ObjectDataSignatureBuilder builder = new ObjectDataSignatureBuilder(factory, relocsOnly); | ||
| builder.AddSymbol(this); | ||
|
|
||
| if (!relocsOnly) | ||
| { | ||
| builder.EmitByte((byte)ReadyToRunFixupKind.ResumptionStubEntryPoint); | ||
| } | ||
|
|
||
| // Emit a relocation to the resumption stub code; at link time this becomes the RVA. | ||
| builder.EmitReloc(_resumptionStub, RelocType.IMAGE_REL_BASED_ADDR32NB, delta: factory.Target.CodeDelta); | ||
|
|
There was a problem hiding this comment.
ResumptionStubEntryPointSignature.GetData emits the fixup-kind byte only when relocsOnly == false, but still emits the relocation unconditionally. In relocsOnly mode this shifts the relocation offset by 1 byte vs the real signature layout, which can produce an invalid signature blob / bad RVA at runtime. Emit a placeholder byte even in relocsOnly (or always emit the kind byte) so relocation offsets match the final data layout.
There was a problem hiding this comment.
This feedback is wrong because the non-reloc data doesn't matter in relocsOnly mode.
But the if is probably a premature optimization. We condition things on relocsOnly if computing the non-reloc data is expensive, or impossible during relocsOnly mode. Emitting a byte probably amounts to noise in the grand scale of things, I'd not try to optimize this.
(relocsOnly means we're in the dependency graph expansion phase, not output writing yet and data is not looked at, only relocs.)
| // Use the entry point hashtable to check if another thread already registered a MethodDesc | ||
| if (m_pCompositeInfo->GetMethodDescForEntryPointInNativeImage(stubEntryPoint) != NULL) | ||
| return; | ||
|
|
||
| AllocMemTracker amTracker; | ||
| ILStubCache *pStubCache = m_pModule->GetILStubCache(); | ||
| MethodTable* pStubMT = pStubCache->GetOrCreateStubMethodTable(m_pModule); | ||
|
|
||
| // Resumption stub signature: object(object, ref byte) | ||
| // This matches BuildResumptionStubSignature in jitinterface.cpp | ||
| static const BYTE s_resumptionStubSig[] = { | ||
| IMAGE_CEE_CS_CALLCONV_DEFAULT, // regular calling convention - continuations are explicitly passed and returned | ||
| 2, // 2 arguments | ||
| ELEMENT_TYPE_OBJECT, // return type: object (continuation) | ||
| ELEMENT_TYPE_OBJECT, // arg0: object (continuation) | ||
| ELEMENT_TYPE_BYREF, // arg1: ref byte (result location) | ||
| ELEMENT_TYPE_U1 | ||
| }; | ||
|
|
||
| MethodDesc* pStubMD = pStubCache->CreateR2RBackedILStub( | ||
| m_pModule->GetLoaderAllocator(), | ||
| pStubMT, | ||
| stubEntryPoint, | ||
| DynamicMethodDesc::StubAsyncResume, | ||
| (PCCOR_SIGNATURE)s_resumptionStubSig, | ||
| sizeof(s_resumptionStubSig), | ||
| &amTracker); | ||
|
|
||
| amTracker.SuppressRelease(); | ||
|
|
||
| // Register the stub's entry point so GC can find it during stack walks. | ||
| // SetMethodDescForEntryPointInNativeImage handles the race - if another thread | ||
| // already registered a MethodDesc for this entry point, ours is simply discarded. | ||
| m_pCompositeInfo->SetMethodDescForEntryPointInNativeImage(stubEntryPoint, pStubMD); |
There was a problem hiding this comment.
RegisterResumptionStub calls amTracker.SuppressRelease() unconditionally. If another thread wins the race and SetMethodDescForEntryPointInNativeImage does not insert (it only inserts when missing), this MethodDesc chunk allocation will not be rolled back and becomes permanently unreachable. Consider re-checking under the same lock (or changing SetMethodDescForEntryPointInNativeImage to report whether it inserted) and only suppressing the AllocMemTracker when the entry is successfully registered.
The JIT's async transformation creates CT_USER_FUNC calls for async context helper methods (CaptureContexts, RestoreContexts, etc.) without populating gtEntryPoint. In R2R mode, this causes the lowering to create a plain indirect call (ldr/blr) instead of the ARM64 x11 indirection cell pattern required by DelayLoad_MethodCall thunks. On ARM64, the delay-load thunk expects x11 to hold the import cell address so ExternalMethodFixupWorker can resolve the fixup. Without setEntryPoint, IsR2RRelativeIndir() returns false, the JIT emits 'ldr xN, [cell]; blr xN' without setting x11, and the thunk reads a stale x11 value, causing an assert failure: pImportSection == pModule->GetImportSectionForRVA(rva) x64 is unaffected because ExternalMethodFixupWorker recovers the cell address from the RIP-relative displacement encoded in the call instruction itself. The fix calls getFunctionEntryPoint + setEntryPoint on each async helper call so the JIT knows it's an R2R indirection cell and emits the correct x11 pattern. This is guarded with #ifdef FEATURE_READYTORUN and IsAot() following existing JIT conventions. Co-authored-by: Copilot <[email protected]>
The genFnEpilog code for GT_JMP with IAT_PVALUE on ARM64 loaded the import cell address into x16 (REG_INDIRECT_CALL_TARGET_REG) and used x16 for both the cell address and the indirect branch target. The R2R DelayLoad_MethodCall thunk expects x11 (REG_R2R_INDIRECT_PARAM) to hold the import cell address so ExternalMethodFixupWorker can resolve the fixup. The fix loads the cell address into x11 first, then loads the target from x11 into x16 for the branch. This matches the pattern used by regular R2R calls. Co-authored-by: Copilot <[email protected]>
| // Use the entry point hashtable to check if another thread already registered a MethodDesc | ||
| if (m_pCompositeInfo->GetMethodDescForEntryPointInNativeImage(stubEntryPoint) != NULL) | ||
| return; | ||
|
|
||
| AllocMemTracker amTracker; | ||
| ILStubCache *pStubCache = m_pModule->GetILStubCache(); | ||
| MethodTable* pStubMT = pStubCache->GetOrCreateStubMethodTable(m_pModule); | ||
|
|
||
| // Resumption stub signature: object(object, ref byte) | ||
| // This matches BuildResumptionStubSignature in jitinterface.cpp | ||
| static const BYTE s_resumptionStubSig[] = { | ||
| IMAGE_CEE_CS_CALLCONV_DEFAULT, // regular calling convention - continuations are explicitly passed and returned | ||
| 2, // 2 arguments | ||
| ELEMENT_TYPE_OBJECT, // return type: object (continuation) | ||
| ELEMENT_TYPE_OBJECT, // arg0: object (continuation) | ||
| ELEMENT_TYPE_BYREF, // arg1: ref byte (result location) | ||
| ELEMENT_TYPE_U1 | ||
| }; | ||
|
|
||
| MethodDesc* pStubMD = pStubCache->CreateR2RBackedILStub( | ||
| m_pModule->GetLoaderAllocator(), | ||
| pStubMT, | ||
| stubEntryPoint, | ||
| DynamicMethodDesc::StubAsyncResume, | ||
| (PCCOR_SIGNATURE)s_resumptionStubSig, | ||
| sizeof(s_resumptionStubSig), | ||
| &amTracker); | ||
|
|
||
| amTracker.SuppressRelease(); | ||
|
|
||
| // Register the stub's entry point so GC can find it during stack walks. | ||
| // SetMethodDescForEntryPointInNativeImage handles the race - if another thread | ||
| // already registered a MethodDesc for this entry point, ours is simply discarded. | ||
| m_pCompositeInfo->SetMethodDescForEntryPointInNativeImage(stubEntryPoint, pStubMD); | ||
| } |
There was a problem hiding this comment.
In RegisterResumptionStub, AllocMemTracker is suppressed unconditionally before calling SetMethodDescForEntryPointInNativeImage. If another thread registers a MethodDesc for the same entry point between the initial lookup and SetMethodDescForEntryPointInNativeImage, the insert will be skipped and this thread’s allocations will be unnecessarily kept (AllocMemTracker can no longer roll them back). Consider suppressing only if we actually won the race (e.g., call SetMethodDescForEntryPointInNativeImage first, then check whether GetMethodDescForEntryPointInNativeImage(entryPoint) == pStubMD before calling SuppressRelease).
| ReadyToRunMethod stubMethod = new ReadyToRunMethod( | ||
| this, | ||
| method.ComponentReader, | ||
| method.MethodHandle, | ||
| index, | ||
| owningType: null, | ||
| constrainedType: null, | ||
| instanceArgs: method.InstanceArgs, | ||
| signaturePrefixes: ["[RESUME]"], | ||
| fixupOffset: null); | ||
| _instanceMethods.Add(new InstanceMethod(0, stubMethod)); |
There was a problem hiding this comment.
MarkResumptionStubEntryPoints appends synthetic resumption stub methods into _instanceMethods (with bucket 0). InstanceMethods is documented/used as the parsed InstanceMethodEntryPoints table (e.g., r2rdump prints it), so mixing in methods that are not actually present in that section will make tooling output/diffs inaccurate. Consider keeping resumption stubs in a separate collection (and only including them via Methods enumeration), or otherwise ensure InstanceMethods continues to reflect only the InstanceMethodEntryPoints section contents.
| ReadyToRunMethod stubMethod = new ReadyToRunMethod( | |
| this, | |
| method.ComponentReader, | |
| method.MethodHandle, | |
| index, | |
| owningType: null, | |
| constrainedType: null, | |
| instanceArgs: method.InstanceArgs, | |
| signaturePrefixes: ["[RESUME]"], | |
| fixupOffset: null); | |
| _instanceMethods.Add(new InstanceMethod(0, stubMethod)); |
Co-authored-by: Copilot <[email protected]>
…ectWriter The ADRP (PAGEBASE_REL21) and ADD/LDR (PAGEOFFSET_12A/12L) relocation cases rejected non-zero addends with NotSupportedException. This was a defensive guard added in PR dotnet#120454 since no code path generated these at the time. On ARM64, async R2R compilation emits ADRP+ADD/LDR sequences that reference fields at various offsets within the async continuation data layout node (e.g. __readwritedata_..._AsyncCallable_...). This node is a single symbol, so accessing individual fields within it requires non-zero addends on the page-relative relocations. Include the addend in the page and offset calculations so these relocations resolve correctly, matching how the ELF and Mach-O writers already handle this case. The fix follows the standard AArch64 ELF ABI relocation formulas: ADRP: Page(S + A) - Page(P) ADD/LDR: (S + A) & 0xFFF Co-authored-by: Copilot <[email protected]>
Clear CORJIT_FLAG_PROCSPLIT in getJitFlags when the method being compiled is async. The runtime's stack walker and EH dispatch assume funclets are not hot/cold split (codeman.cpp IsFilterFunclet: 'This assumes no hot/cold splitting for funclets'). Async methods have continuation entry points that behave like funclets, so hot/cold splitting them produces code the runtime cannot walk correctly. Also add a JIT assert to catch cases where both ASYNC and PROCSPLIT flags are set simultaneously. Co-authored-by: Copilot <[email protected]>
|
/azp run runtime-coreclr crossgen2 |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Emit async methods, and their resumption stubs into ReadyToRun images. Also compiles and emits async thunks for task-returning methods.
Signatures for async methods are emitted in the InstanceMethod table, with an additional ENCODE_METHOD_SIG_AsyncVariant (0x100) flag.
Resumption stubs are encoded as precode fixups for the async method with the "signature" being the RVA of the start point of the code. When the fixups are resolved, a DynamicMethodDesc / ILStub is created to represent the resumption and enable GC and unwind info to be resolved. Async methods which do not await and do not need resumption stubs won't have a resumption stub fixup.
The resumption stub MethodDescs are created following the existing pattern for ILStubs, but set the code to the R2R code rather than a precode thunk.