Skip to content

Commit b5c07d4

Browse files
janvorlijkotas
andauthored
Fix few startup path issues with interpreter (#118448)
* Fix few startup path issues with interpreter This change fixes the following issues: * Reverse pinvoke to method marked by UnmanagedCallersOnly attribute. The InterpExecMethod is entered in GC preemptive mode in that case, but it is expected to run in GC cooperative mode. * Pinvoke to method marked by SuppressGCTransition attribute . The pinvoke is entered in GC preemtive mode, but it needs to be called in GC cooperative mode. * Calling a delegate or a similar case when IL stub is used. The interpreter code is set only on the IL stub MethodDesc, but it needs to be set on both the IL stub and the original MethodDesc. * Check for the SuppressGCTransition in interpreter at compile time * PR feedback * Update src/coreclr/vm/prestub.cpp Co-authored-by: Jan Kotas <[email protected]> * Update src/coreclr/vm/interpexec.cpp Co-authored-by: Jan Kotas <[email protected]> * Undo INTOP_CALL_PINVOKE opcode size bump No longer needed after the previous commit * Fix build break --------- Co-authored-by: Jan Kotas <[email protected]>
1 parent eace168 commit b5c07d4

File tree

4 files changed

+41
-6
lines changed

4 files changed

+41
-6
lines changed

src/coreclr/interpreter/compiler.cpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1215,8 +1215,13 @@ InterpMethod* InterpCompiler::CreateInterpMethod()
12151215
pDataItems[i] = m_dataItems.Get(i);
12161216

12171217
bool initLocals = (m_methodInfo->options & CORINFO_OPT_INIT_LOCALS) != 0;
1218+
CORJIT_FLAGS corJitFlags;
1219+
DWORD jitFlagsSize = m_compHnd->getJitFlags(&corJitFlags, sizeof(corJitFlags));
1220+
assert(jitFlagsSize == sizeof(corJitFlags));
12181221

1219-
InterpMethod *pMethod = new InterpMethod(m_methodHnd, m_totalVarsStackSize, pDataItems, initLocals);
1222+
bool unmanagedCallersOnly = corJitFlags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_REVERSE_PINVOKE);
1223+
1224+
InterpMethod *pMethod = new InterpMethod(m_methodHnd, m_totalVarsStackSize, pDataItems, initLocals, unmanagedCallersOnly);
12201225

12211226
return pMethod;
12221227
}
@@ -3038,9 +3043,13 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* pConstrainedToken, bool re
30383043
CORINFO_CONST_LOOKUP lookup;
30393044
m_compHnd->getAddressOfPInvokeTarget(callInfo.hMethod, &lookup);
30403045
m_pLastNewIns->data[1] = GetDataItemIndex(lookup.addr);
3041-
m_pLastNewIns->data[2] = lookup.accessType == IAT_PVALUE;
30423046
if (lookup.accessType == IAT_PPVALUE)
30433047
NO_WAY("IAT_PPVALUE pinvokes not implemented in interpreter");
3048+
bool suppressGCTransition = false;
3049+
m_compHnd->getUnmanagedCallConv(callInfo.hMethod, nullptr, &suppressGCTransition);
3050+
m_pLastNewIns->data[2] =
3051+
((lookup.accessType == IAT_PVALUE) ? (int32_t)PInvokeCallFlags::Indirect : 0) |
3052+
(suppressGCTransition ? (int32_t)PInvokeCallFlags::SuppressGCTransition : 0);
30443053
}
30453054
}
30463055
break;

src/coreclr/interpreter/interpretershared.h

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,9 @@ struct InterpMethod
3535
// This stub is used for calling the interpreted method from JITted/AOTed code
3636
CallStubHeader *pCallStub;
3737
bool initLocals;
38+
bool unmanagedCallersOnly;
3839

39-
InterpMethod(CORINFO_METHOD_HANDLE methodHnd, int32_t allocaSize, void** pDataItems, bool initLocals)
40+
InterpMethod(CORINFO_METHOD_HANDLE methodHnd, int32_t allocaSize, void** pDataItems, bool initLocals, bool unmanagedCallersOnly)
4041
{
4142
#if DEBUG
4243
this->self = this;
@@ -45,6 +46,7 @@ struct InterpMethod
4546
this->allocaSize = allocaSize;
4647
this->pDataItems = pDataItems;
4748
this->initLocals = initLocals;
49+
this->unmanagedCallersOnly = unmanagedCallersOnly;
4850
pCallStub = NULL;
4951
}
5052

@@ -157,4 +159,11 @@ struct InterpGenericLookup
157159
uint16_t offsets[InterpGenericLookup_MaxIndirections];
158160
};
159161

162+
enum class PInvokeCallFlags : int32_t
163+
{
164+
None = 0,
165+
Indirect = 1 << 0, // The call target address is indirect
166+
SuppressGCTransition = 1 << 1, // The pinvoke is marked by the SuppressGCTransition attribute
167+
};
168+
160169
#endif

src/coreclr/vm/interpexec.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1855,11 +1855,11 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr
18551855
callArgsOffset = ip[2];
18561856
methodSlot = ip[3];
18571857
int32_t targetAddrSlot = ip[4];
1858-
int32_t indirectFlag = ip[5];
1858+
int32_t flags = ip[5];
18591859

18601860
ip += 6;
18611861
targetMethod = (MethodDesc*)pMethod->pDataItems[methodSlot];
1862-
PCODE callTarget = indirectFlag
1862+
PCODE callTarget = (flags & (int32_t)PInvokeCallFlags::Indirect)
18631863
? *(PCODE *)pMethod->pDataItems[targetAddrSlot]
18641864
: (PCODE)pMethod->pDataItems[targetAddrSlot];
18651865

@@ -1872,7 +1872,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr
18721872
inlinedCallFrame.Push();
18731873

18741874
{
1875-
GCX_PREEMP();
1875+
GCX_MAYBE_PREEMP(!(flags & (int32_t)PInvokeCallFlags::SuppressGCTransition));
18761876
InvokeCompiledMethod(targetMethod, stack + callArgsOffset, stack + returnOffset, callTarget);
18771877
}
18781878

src/coreclr/vm/prestub.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2001,13 +2001,30 @@ static InterpThreadContext* GetInterpThreadContext()
20012001
return threadContext;
20022002
}
20032003

2004+
EXTERN_C void STDCALL ReversePInvokeBadTransition();
2005+
20042006
extern "C" void* STDCALL ExecuteInterpretedMethod(TransitionBlock* pTransitionBlock, TADDR byteCodeAddr, void* retBuff)
20052007
{
20062008
// Argument registers are in the TransitionBlock
20072009
// The stack arguments are right after the pTransitionBlock
20082010
InterpThreadContext *threadContext = GetInterpThreadContext();
20092011
int8_t *sp = threadContext->pStackPointer;
20102012

2013+
InterpByteCodeStart* pInterpreterCode = dac_cast<PTR_InterpByteCodeStart>(byteCodeAddr);
2014+
2015+
if (pInterpreterCode->Method->unmanagedCallersOnly)
2016+
{
2017+
Thread* thread = GetThreadNULLOk();
2018+
if (thread == NULL)
2019+
CREATETHREAD_IF_NULL_FAILFAST(thread, W("Failed to setup new thread during reverse P/Invoke"));
2020+
2021+
// Verify the current thread isn't in COOP mode.
2022+
if (thread->PreemptiveGCDisabled())
2023+
ReversePInvokeBadTransition();
2024+
}
2025+
2026+
GCX_MAYBE_COOP(pInterpreterCode->Method->unmanagedCallersOnly);
2027+
20112028
// This construct ensures that the InterpreterFrame is always stored at a higher address than the
20122029
// InterpMethodContextFrame. This is important for the stack walking code.
20132030
struct Frames

0 commit comments

Comments
 (0)