[RuntimeAsync] A few fixes for issues found when enabling rt async in Libraries.

[VSadov]

A few simple/trivial fixes for issues found in #119432

• Sometimes GS cookie check uses the same register (rcx) as the continuation return and overwrites it. GS check does not need to use rcx though, some other volatile register like REG_ARG_1 would work too.
  Fixes: [RuntimeAsync] A few Libraries tests end up crashing with AccessViolationException in FinalizeTaskReturningThunk if runtime async is enabled #119622

• Async thunks need some specialcasing for ValueTask. For example we missed the part that ValueTask is a struct and when invoking methods on it (like GetAwaiter), we need to do stloc;ldloca first.
  Fixes: [RuntimeAsync] Async thunks need special support for ValueTask-returning methods. #119613

• SaveAsyncContexts may split the current block if await is in a try, make sure that the splitted off tail block is still visited as it may contain another await.
  Fixes: [RuntimeAsync] Two awaits in a Try may not be properly handled. #119611

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

[dotnet-policy-service]

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull Request Overview

This PR addresses several issues found when enabling runtime async functionality in the .NET Libraries. The changes fix problems related to GS cookie checks, ValueTask async thunks, and async context handling.

• Fixes GS cookie check register conflicts with continuation returns by using a different register
• Adds proper ValueTask support to async thunks including struct-specific handling for method invocation
• Ensures split blocks from SaveAsyncContexts are properly visited for additional awaits

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/coreclr/vm/method.hpp	Adds declaration for IsValueTaskAsyncThunk method
src/coreclr/vm/corelib.h	Defines ValueTask and ValueTaskAwaiter method bindings for async support
src/coreclr/vm/asyncthunks.cpp	Implements ValueTask detection and specialized handling in async thunks
src/coreclr/jit/codegenxarch.cpp	Fixes GS cookie check register selection to avoid conflicts
src/coreclr/jit/async.cpp	Ensures proper block traversal after context save splits and improves assertions

[VSadov]

There is a failure on x86 and seems related to the change.
The scratch registers on x86 are only RBM_EAX|RBM_ECX|RBM_EDX, and all can be used to return something. In a worst case EAX is LNGARG_LO EDX is LNGARG_HI and ECX is CONTINUATION_RET.

So, if we also need a GS check in the epilog, we are in trouble.

[jakobbotsch]

There is a failure on x86 and seems related to the change. The scratch registers on x86 are only RBM_EAX|RBM_ECX|RBM_EDX, and all can be used to return something. In a worst case EAX is LNGRET_LO EDX is LNGRET_HI and ECX is CONTINUATION_RET.

So, if we also need a GS check in the epilog, we are in trouble.

I think we can detect and use a callee save in this case. Just need to take care to specially save it. Similar to this logic:

runtime/src/coreclr/jit/codegencommon.cpp

Lines 4560 to 4579 in 2db45f1

	// Parameter homing may need an additional register to handle conflicts if
	// all callee trash registers are used by parameters.
	regMaskTP homingCandidates = genGetParameterHomingTempRegisterCandidates();
	if (((homingCandidates & ~intRegState.rsCalleeRegArgMaskLiveIn) & RBM_ALLINT) == RBM_NONE)
	{
	regMaskTP extraRegMask = RBM_ALLINT & ~homingCandidates & ~regSet.rsMaskResvd;
	assert(extraRegMask != RBM_NONE);
	regNumber extraReg = genFirstRegNumFromMask(extraRegMask);
	JITDUMP("No temporary registers are available for integer parameter homing. Adding %s\n", getRegName(extraReg));
	regSet.rsSetRegsModified(genRegMask(extraReg));
	}
	if (((homingCandidates & ~floatRegState.rsCalleeRegArgMaskLiveIn) & RBM_ALLFLOAT) == RBM_NONE)
	{
	regMaskTP extraRegMask = RBM_ALLFLOAT & ~homingCandidates & ~regSet.rsMaskResvd;
	assert(extraRegMask != RBM_NONE);
	regNumber extraReg = genFirstRegNumFromMask(extraRegMask);
	JITDUMP("No temporary registers are available for float parameter homing. Adding %s\n", getRegName(extraReg));
	regSet.rsSetRegsModified(genRegMask(extraReg));
	}

[VSadov]

There is a failure on x86 and seems related to the change. The scratch registers on x86 are only RBM_EAX|RBM_ECX|RBM_EDX, and all can be used to return something. In a worst case EAX is LNGARG_LO EDX is LNGARG_HI and ECX is CONTINUATION_RET.
So, if we also need a GS check in the epilog, we are in trouble.

I think we can detect and use a callee save in this case. Just need to take care to specially save it. Similar to this logic:

runtime/src/coreclr/jit/codegencommon.cpp

Lines 4560 to 4579 in 2db45f1

	// Parameter homing may need an additional register to handle conflicts if
	// all callee trash registers are used by parameters.
	regMaskTP homingCandidates = genGetParameterHomingTempRegisterCandidates();
	if (((homingCandidates & ~intRegState.rsCalleeRegArgMaskLiveIn) & RBM_ALLINT) == RBM_NONE)
	{
	regMaskTP extraRegMask = RBM_ALLINT & ~homingCandidates & ~regSet.rsMaskResvd;
	assert(extraRegMask != RBM_NONE);
	regNumber extraReg = genFirstRegNumFromMask(extraRegMask);
	JITDUMP("No temporary registers are available for integer parameter homing. Adding %s\n", getRegName(extraReg));
	regSet.rsSetRegsModified(genRegMask(extraReg));
	}
	if (((homingCandidates & ~floatRegState.rsCalleeRegArgMaskLiveIn) & RBM_ALLFLOAT) == RBM_NONE)
	{
	regMaskTP extraRegMask = RBM_ALLFLOAT & ~homingCandidates & ~regSet.rsMaskResvd;
	assert(extraRegMask != RBM_NONE);
	regNumber extraReg = genFirstRegNumFromMask(extraRegMask);
	JITDUMP("No temporary registers are available for float parameter homing. Adding %s\n", getRegName(extraReg));
	regSet.rsSetRegsModified(genRegMask(extraReg));
	}

My plan is to use REG_ECX on x86 for GS check, and in async methods save/restore around the check. I think that should work.

[jakobbotsch]

There is a failure on x86 and seems related to the change. The scratch registers on x86 are only RBM_EAX|RBM_ECX|RBM_EDX, and all can be used to return something. In a worst case EAX is LNGARG_LO EDX is LNGARG_HI and ECX is CONTINUATION_RET.
So, if we also need a GS check in the epilog, we are in trouble.

I think we can detect and use a callee save in this case. Just need to take care to specially save it. Similar to this logic:

runtime/src/coreclr/jit/codegencommon.cpp

Lines 4560 to 4579 in 2db45f1

	// Parameter homing may need an additional register to handle conflicts if
	// all callee trash registers are used by parameters.
	regMaskTP homingCandidates = genGetParameterHomingTempRegisterCandidates();
	if (((homingCandidates & ~intRegState.rsCalleeRegArgMaskLiveIn) & RBM_ALLINT) == RBM_NONE)
	{
	regMaskTP extraRegMask = RBM_ALLINT & ~homingCandidates & ~regSet.rsMaskResvd;
	assert(extraRegMask != RBM_NONE);
	regNumber extraReg = genFirstRegNumFromMask(extraRegMask);
	JITDUMP("No temporary registers are available for integer parameter homing. Adding %s\n", getRegName(extraReg));
	regSet.rsSetRegsModified(genRegMask(extraReg));
	}
	if (((homingCandidates & ~floatRegState.rsCalleeRegArgMaskLiveIn) & RBM_ALLFLOAT) == RBM_NONE)
	{
	regMaskTP extraRegMask = RBM_ALLFLOAT & ~homingCandidates & ~regSet.rsMaskResvd;
	assert(extraRegMask != RBM_NONE);
	regNumber extraReg = genFirstRegNumFromMask(extraRegMask);
	JITDUMP("No temporary registers are available for float parameter homing. Adding %s\n", getRegName(extraReg));
	regSet.rsSetRegsModified(genRegMask(extraReg));
	}

My plan is to use REG_ECX on x86 for GS check, and in async methods save/restore around the check. I think that should work.

That might work too, assuming we can push registers at that point (don't recall if the cookie check is in the formal epilog or not).

[VSadov]

I do not think GS check is in epilog, in no-gc sense.

So another plan: x86 uses the same code as before this PR, but if we have both async method and long return, use REG_ARG_1 (EDX) and push/pop it around the check.

[VSadov]

/azp run runtime-coreclr gcstress-extra, runtime-coreclr gcstress0x3-gcstress0xc

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[VSadov]

All tests pass in normal mode with async enabled, but under GC stress there are a few failures, which all also happen in a parallel no-op PR, thus are not regressions.

[VSadov]

/backport to release/10.0

[github-actions]

Started backporting to release/10.0: https://github.com/dotnet/runtime/actions/runs/17920889413