diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index 28ef2438eb1d4b..02ac3c0c730654 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -4870,21 +4870,9 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* pConstrainedToken, bool re if (!m_isSynchronized && !m_isAsyncMethodWithContextSaveRestore) NO_WAY("INTERP_LOAD_RETURN_VALUE_FOR_SYNCHRONIZED_OR_ASYNC used in a non-synchronized or async method"); - if (m_synchronizedOrAsyncRetValVarIndex == -1) - { - // Code inside for-loops, for example, is not emitted in the first pass, so we can reach the async - // epilog with m_synchronizedOrAsyncRetValVarIndex uninitialized. - CORINFO_SIG_INFO sig = m_methodInfo->args; - InterpType retType = GetInterpType(sig.retType); - if (retType != InterpTypeVoid) - { - CORINFO_CLASS_HANDLE retClsHnd = (retType == InterpTypeVT) ? sig.retTypeClass : NULL; - PushInterpType(retType, retClsHnd); - m_synchronizedOrAsyncRetValVarIndex = m_pStackPointer[-1].var; - m_pStackPointer--; - INTERP_DUMP("Created ret val var V%d (pre-created in epilog)\n", m_synchronizedOrAsyncRetValVarIndex); - } - } + // Code inside for-loops, for example, is not emitted in the first pass, so we can reach the async + // epilog with m_synchronizedOrAsyncRetValVarIndex uninitialized. + CreateSynchronizedRetValVar(); INTERP_DUMP("INTERP_LOAD_RETURN_VALUE_FOR_SYNCHRONIZED_OR_ASYNC with ret val var V%d\n", (int)m_synchronizedOrAsyncRetValVarIndex); if (m_synchronizedOrAsyncRetValVarIndex != -1) @@ -5762,7 +5750,9 @@ void InterpCompiler::EmitRet(CORINFO_METHOD_INFO* methodInfo) CORINFO_SIG_INFO sig = methodInfo->args; InterpType retType = GetInterpType(sig.retType); - if (m_isAsyncVersionOfSyncMethod) + // Wrap top of stack in await. For async versions that are synchronized, + // only wrap the outer most return in await. + if (m_isAsyncVersionOfSyncMethod && (!m_isSynchronized || m_currentILOffset >= m_ILCodeSizeFromILHeader)) { if (m_asyncVersionIsTailCalling) { @@ -5798,16 +5788,11 @@ void InterpCompiler::EmitRet(CORINFO_METHOD_INFO* methodInfo) int32_t ilOffset = (int32_t)(m_ip - m_pILCode); int32_t target = m_synchronizedOrAsyncPostFinallyOffset; - if (retType != InterpTypeVoid) + CreateSynchronizedRetValVar(); + + if ((retType != InterpTypeVoid) || m_isAsyncVersionOfSyncMethod) { CheckStackExact(1); - if (m_synchronizedOrAsyncRetValVarIndex == -1) - { - PushInterpType(retType, m_pVars[m_pStackPointer[-1].var].clsHnd); - m_synchronizedOrAsyncRetValVarIndex = m_pStackPointer[-1].var; - m_pStackPointer--; - INTERP_DUMP("Created ret val var V%d\n", m_synchronizedOrAsyncRetValVarIndex); - } INTERP_DUMP("Store to ret val var V%d\n", m_synchronizedOrAsyncRetValVarIndex); EmitStoreVar(m_synchronizedOrAsyncRetValVarIndex); } @@ -7591,7 +7576,7 @@ bool InterpCompiler::IsRuntimeAsyncCallConfigureAwaitValueTask(const uint8_t* ip bool InterpCompiler::IsRuntimeAsyncCallRetOrJmpInAsyncVersion(const uint8_t* ip, OpcodePeepElement* pattern, void** ppComputedInfo) { - if (!m_isAsyncVersionOfSyncMethod) + if (!m_isAsyncVersionOfSyncMethod || m_isSynchronized) { return false; } @@ -8320,6 +8305,47 @@ int InterpCompiler::ApplyTypeValueTypePeep(const uint8_t* ip, OpcodePeepElement* return -1; } +void InterpCompiler::CreateSynchronizedRetValVar() +{ + if (m_synchronizedOrAsyncRetValVarIndex != -1) + { + return; + } + + if (m_methodInfo->args.retType == CORINFO_TYPE_VOID && !m_isAsyncVersionOfSyncMethod) + { + return; + } + + InterpType retType; + CORINFO_CLASS_HANDLE retClsHnd = NULL; + if (m_isAsyncVersionOfSyncMethod) + { + // The actual type of the return value will be the Task or + // ValueTask type, so get it from the await call signature + CORINFO_LOOKUP instArg; + CORINFO_METHOD_HANDLE awaitCall = m_compHnd->getAwaitReturnCall(m_methodHnd, &instArg); + CORINFO_SIG_INFO awaitCallSig; + m_compHnd->getMethodSig(awaitCall, &awaitCallSig); + + CORINFO_CLASS_HANDLE vcTypeRet; + CorInfoType paramType = strip(m_compHnd->getArgType(&awaitCallSig, awaitCallSig.args, &vcTypeRet)); + + retType = GetInterpType(paramType); + retClsHnd = (retType == InterpTypeVT) ? vcTypeRet : NULL; + } + else + { + retType = GetInterpType(m_methodInfo->args.retType); + retClsHnd = (retType == InterpTypeVT) ? m_methodInfo->args.retTypeClass : NULL; + } + + PushInterpType(retType, retClsHnd); + m_synchronizedOrAsyncRetValVarIndex = m_pStackPointer[-1].var; + m_pStackPointer--; + INTERP_DUMP("Created ret val var V%d\n", m_synchronizedOrAsyncRetValVarIndex); +} + void InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) { bool readonly = false; diff --git a/src/coreclr/interpreter/compiler.h b/src/coreclr/interpreter/compiler.h index dad66ff5e2e111..a459090f1df827 100644 --- a/src/coreclr/interpreter/compiler.h +++ b/src/coreclr/interpreter/compiler.h @@ -760,6 +760,8 @@ class InterpCompiler int32_t GetMethodDataItemIndex(CORINFO_METHOD_HANDLE mHandle); int32_t GetDataForHelperFtn(CorInfoHelpFunc ftn); + void CreateSynchronizedRetValVar(); + void GenerateCode(CORINFO_METHOD_INFO* methodInfo); InterpBasicBlock* GenerateCodeForLeaveChainIslands(InterpBasicBlock *pNewBB, InterpBasicBlock *pPrevBB); void PatchInitLocals(CORINFO_METHOD_INFO* methodInfo); diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 69e10441a264d6..984022eeb991a4 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -1614,9 +1614,11 @@ void Compiler::fgAddSyncMethodEnterExit() // For EnC this is part of the frame header. Furthermore, this is allocated above PSP on ARM64. // To avoid complicated reasoning about alignment we always allocate a full pointer sized slot for this. var_types typeMonAcquired = TYP_I_IMPL; - this->lvaMonAcquired = lvaGrabTemp(true DEBUGARG("Synchronized method monitor acquired boolean")); - - lvaTable[lvaMonAcquired].lvType = typeMonAcquired; + if (lvaMonAcquired == BAD_VAR_NUM) + { + lvaMonAcquired = lvaGrabTemp(true DEBUGARG("Synchronized method monitor acquired boolean")); + lvaTable[lvaMonAcquired].lvType = typeMonAcquired; + } if (opts.IsOSR()) { @@ -1670,11 +1672,15 @@ void Compiler::fgAddSyncMethodEnterExit() false /*exit*/); // non-exceptional cases - for (BasicBlock* const block : Blocks()) + // For async versions we created these directly in import + if (!compIsAsyncVersion()) { - if (block->KindIs(BBJ_RETURN)) + for (BasicBlock* const block : Blocks()) { - fgCreateMonitorTree(lvaMonAcquired, info.compThisArg, block, false /*exit*/); + if (block->KindIs(BBJ_RETURN)) + { + fgCreateMonitorTree(lvaMonAcquired, info.compThisArg, block, false /*exit*/); + } } } } diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 1b811f91084775..9b6e1636dcd11e 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -9052,7 +9052,8 @@ void Compiler::impImportBlockCode(BasicBlock* block) if (compIsAsyncVersion()) { - if ((codeAddr + sz < codeEndp) && (getU1LittleEndian(codeAddr + sz) == CEE_RET)) + if ((codeAddr + sz < codeEndp) && (getU1LittleEndian(codeAddr + sz) == CEE_RET) && + ((info.compFlags & CORINFO_FLG_SYNCH) == 0)) { JITDUMP("\nRecognized tail-call in async version\n"); awaitOffset = (IL_OFFSET)(codeAddr - 1 - info.compCode); @@ -11706,6 +11707,23 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode) // bool Compiler::impWrapTopOfStackInAwait() { + if ((info.compFlags & CORINFO_FLG_SYNCH) != 0) + { + assert(!compIsForInlining()); + + if (lvaMonAcquired == BAD_VAR_NUM) + { + lvaMonAcquired = lvaGrabTemp(true DEBUGARG("Synchronized method monitor acquired boolean")); + lvaGetDesc(lvaMonAcquired)->lvType = TYP_I_IMPL; + } + + GenTree* varAddrNode = gtNewLclVarAddrNode(lvaMonAcquired); + GenTree* lockObject = + info.compIsStatic ? fgGetCritSectOfStaticMethod() : gtNewLclvNode(info.compThisArg, TYP_REF); + GenTree* exitMon = gtNewHelperCallNode(CORINFO_HELP_MON_EXIT, TYP_VOID, lockObject, varAddrNode); + impAppendTree(exitMon, CHECK_SPILL_ALL, impCurStmtDI); + } + if (impFoldAwaitedTopOfStack()) { return true; diff --git a/src/tests/async/regression/synchronized-async-version.cs b/src/tests/async/regression/synchronized-async-version.cs new file mode 100644 index 00000000000000..b032dd17eaef89 --- /dev/null +++ b/src/tests/async/regression/synchronized-async-version.cs @@ -0,0 +1,68 @@ +// 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; +using System.Threading.Tasks; +using Xunit; + +public class Async2Synchronized +{ + [Fact] + public static void TestEntryPoint() + { + TestEntryPointAsync().GetAwaiter().GetResult(); + } + + private static async Task TestEntryPointAsync() + { + Async2Synchronized p = new(); + TaskCompletionSource tcs = new(); + Task t = p.Foo(tcs.Task); + // Returning the task from the [MethodImpl(MethodImplOptions.Synchronized)] + // method Bar must release the lock it acquired, so it should not be held here. + Assert.False(Monitor.IsEntered(p)); + tcs.SetResult(); + await t; + } + + private async Task Foo(Task task) + { + await Bar(task); + } + + [MethodImpl(MethodImplOptions.Synchronized)] + private Task Bar(Task t) + { + return t; + } + + [Fact] + public static void TestEntryPointValueTask() + { + TestEntryPointValueTaskAsync().GetAwaiter().GetResult(); + } + + private static async ValueTask TestEntryPointValueTaskAsync() + { + Async2Synchronized p = new(); + TaskCompletionSource tcs = new(); + ValueTask t = p.FooValueTask(new ValueTask(tcs.Task)); + // Returning the ValueTask from the [MethodImpl(MethodImplOptions.Synchronized)] + // method Bar must release the lock it acquired, so it should not be held here. + Assert.False(Monitor.IsEntered(p)); + tcs.SetResult(); + await t; + } + + private async ValueTask FooValueTask(ValueTask task) + { + await Bar(task); + } + + [MethodImpl(MethodImplOptions.Synchronized)] + private ValueTask Bar(ValueTask t) + { + return t; + } +} diff --git a/src/tests/async/regression/synchronized-async-version.csproj b/src/tests/async/regression/synchronized-async-version.csproj new file mode 100644 index 00000000000000..197767e2c4e249 --- /dev/null +++ b/src/tests/async/regression/synchronized-async-version.csproj @@ -0,0 +1,5 @@ + + + + +