Skip to content

Commit

Permalink
Introduce CallConvSuppressGCTransition type to enable suppressing the…
Browse files Browse the repository at this point in the history
… transition frame for unmanaged function pointer calls (#46343)

* Add public CallConvSuppressGCTransition type

* Fix parsing CallConv modopts out of an IL stub.

* Insert SuppressGCTransition modopt into ilstub target sig. Use modopts for IL stub callconv when SuppressGCTransition in use.

* Recognize CallConvSuppressGCTransition as supressing the GC transition for modopts (Coreclr and crossgen1).

* Support specifying CallConvSuppressGCTransition on both inlinable and non-inlinable calli.

* Enable SuppressGCTransition on calli signatures in crossgen2.

* Fix callconv in il

* PR feedback

* Add docs.

* Use STANDARD_VM_CONTRACT and fix clang build.

* Update contracts in jitinterface.

* Exclude test on mono since support isn't implemented.

* Fix contract.

* PR feedback.

* Add flags back to cache.
  • Loading branch information
jkoritzinsky committed Jan 7, 2021
1 parent 1d0f67f commit 92870e5
Show file tree
Hide file tree
Showing 22 changed files with 377 additions and 146 deletions.
1 change: 1 addition & 0 deletions src/coreclr/inc/corhdr.h
Original file line number Diff line number Diff line change
Expand Up @@ -1873,6 +1873,7 @@ typedef enum LoadHintEnum
#define CMOD_CALLCONV_NAME_STDCALL "CallConvStdcall"
#define CMOD_CALLCONV_NAME_THISCALL "CallConvThiscall"
#define CMOD_CALLCONV_NAME_FASTCALL "CallConvFastcall"
#define CMOD_CALLCONV_NAME_SUPPRESSGCTRANSITION "CallConvSuppressGCTransition"

#endif // MACROS_NOT_SUPPORTED

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/inc/corinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -1065,7 +1065,7 @@ enum CorInfoSigInfoFlags
{
CORINFO_SIGFLAG_IS_LOCAL_SIG = 0x01,
CORINFO_SIGFLAG_IL_STUB = 0x02,
CORINFO_SIGFLAG_SUPPRESS_GC_TRANSITION = 0x04,
// unused = 0x04,
CORINFO_SIGFLAG_FAT_CALL = 0x08,
};

Expand Down
25 changes: 9 additions & 16 deletions src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -562,8 +562,9 @@ private CorInfoCallConvExtension GetUnmanagedCallingConventionFromAttribute(Cust
return callConv;
}

private bool TryGetUnmanagedCallingConventionFromModOpt(MethodSignature signature, out CorInfoCallConvExtension callConv)
private bool TryGetUnmanagedCallingConventionFromModOpt(MethodSignature signature, out CorInfoCallConvExtension callConv, out bool suppressGCTransition)
{
suppressGCTransition = false;
callConv = CorInfoCallConvExtension.Managed;
if (!signature.HasEmbeddedSignatureData || signature.GetEmbeddedSignatureData() == null)
return false;
Expand All @@ -585,6 +586,12 @@ private bool TryGetUnmanagedCallingConventionFromModOpt(MethodSignature signatur
if (defType.Namespace != "System.Runtime.CompilerServices")
continue;

if (defType.Name == "CallConvSuppressGCTransition")
{
suppressGCTransition = true;
continue;
}

CorInfoCallConvExtension? callConvLocal = GetCallingConventionForCallConvType(defType);

if (callConvLocal.HasValue)
Expand Down Expand Up @@ -1070,10 +1077,6 @@ private CorInfoCallConvExtension getUnmanagedCallConv(CORINFO_METHOD_STRUCT_* me
Debug.Assert(sig != null);

CorInfoCallConvExtension callConv = GetUnmanagedCallConv((MethodSignature)HandleToObject((IntPtr)sig->pSig), out pSuppressGCTransition);
if (sig->flags.HasFlag(CorInfoSigInfoFlags.CORINFO_SIGFLAG_SUPPRESS_GC_TRANSITION))
{
pSuppressGCTransition = true;
}
return callConv;
}
}
Expand Down Expand Up @@ -1123,7 +1126,7 @@ private CorInfoCallConvExtension GetUnmanagedCallConv(MethodSignature signature,
case MethodSignatureFlags.UnmanagedCallingConventionThisCall:
return CorInfoCallConvExtension.Thiscall;
case MethodSignatureFlags.UnmanagedCallingConvention:
if (TryGetUnmanagedCallingConventionFromModOpt(signature, out CorInfoCallConvExtension callConvMaybe))
if (TryGetUnmanagedCallingConventionFromModOpt(signature, out CorInfoCallConvExtension callConvMaybe, out suppressGCTransition))
{
return callConvMaybe;
}
Expand Down Expand Up @@ -1384,16 +1387,6 @@ private void findSig(CORINFO_MODULE_STRUCT_* module, uint sigTOK, CORINFO_CONTEX

Get_CORINFO_SIG_INFO(methodSig, sig);

// TODO: Replace this with a public mechanism to mark calli with SuppressGCTransition once it becomes available.
if (methodIL is PInvokeILStubMethodIL stubIL)
{
var method = stubIL.OwningMethod;
if (method.IsPInvoke && method.IsSuppressGCTransition())
{
sig->flags |= CorInfoSigInfoFlags.CORINFO_SIGFLAG_SUPPRESS_GC_TRANSITION;
}
}

#if !READYTORUN
// Check whether we need to report this as a fat pointer call
if (_compilation.IsFatPointerCandidate(methodIL.OwningMethod, methodSig))
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ public enum CorInfoSigInfoFlags : byte
{
CORINFO_SIGFLAG_IS_LOCAL_SIG = 0x01,
CORINFO_SIGFLAG_IL_STUB = 0x02,
CORINFO_SIGFLAG_SUPPRESS_GC_TRANSITION = 0x04,
// unused = 0x04,
CORINFO_SIGFLAG_FAT_CALL = 0x08,
};

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/array.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ MethodTable* Module::CreateArrayMethodTable(TypeHandle elemTypeHnd, CorElementTy

pMT->SetParentMethodTable(pParentClass);

// Method tables for arrays of generic type parameters are needed for type analysis.
// Method tables for arrays of generic type parameters are needed for type analysis.
// No instances will be created, so we can use 0 as element size.
DWORD dwComponentSize = CorTypeInfo::IsGenericVariable(elemType) ?
0 :
Expand Down Expand Up @@ -751,7 +751,7 @@ class ArrayOpLinker : public ILStubLinker

public:
ArrayOpLinker(ArrayMethodDesc * pMD)
: ILStubLinker(pMD->GetModule(), pMD->GetSignature(), &m_emptyContext, pMD, TRUE, TRUE, FALSE)
: ILStubLinker(pMD->GetModule(), pMD->GetSignature(), &m_emptyContext, pMD, (ILStubLinkerFlags)(ILSTUB_LINKER_FLAG_STUB_HAS_THIS | ILSTUB_LINKER_FLAG_TARGET_HAS_THIS))
{
m_pCode = NewCodeStream(kDispatch);
m_pMD = pMD;
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/comdelegate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2190,7 +2190,7 @@ FCIMPL1(PCODE, COMDelegate::GetMulticastInvoke, Object* refThisIn)
BOOL fReturnVal = !sig.IsReturnTypeVoid();

SigTypeContext emptyContext;
ILStubLinker sl(pMD->GetModule(), pMD->GetSignature(), &emptyContext, pMD, TRUE, TRUE, FALSE);
ILStubLinker sl(pMD->GetModule(), pMD->GetSignature(), &emptyContext, pMD, (ILStubLinkerFlags)(ILSTUB_LINKER_FLAG_STUB_HAS_THIS | ILSTUB_LINKER_FLAG_TARGET_HAS_THIS));

ILCodeStream *pCode = sl.NewCodeStream(ILStubLinker::kDispatch);

Expand Down Expand Up @@ -2374,7 +2374,7 @@ PCODE COMDelegate::GetWrapperInvoke(MethodDesc* pMD)
BOOL fReturnVal = !sig.IsReturnTypeVoid();

SigTypeContext emptyContext;
ILStubLinker sl(pMD->GetModule(), pMD->GetSignature(), &emptyContext, pMD, TRUE, TRUE, FALSE);
ILStubLinker sl(pMD->GetModule(), pMD->GetSignature(), &emptyContext, pMD, (ILStubLinkerFlags)(ILSTUB_LINKER_FLAG_STUB_HAS_THIS | ILSTUB_LINKER_FLAG_TARGET_HAS_THIS));

ILCodeStream *pCode = sl.NewCodeStream(ILStubLinker::kDispatch);

Expand Down
6 changes: 6 additions & 0 deletions src/coreclr/vm/corelib.h
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,12 @@ DEFINE_CLASS(RUNTIME_WRAPPED_EXCEPTION, CompilerServices, RuntimeWrappedExcept
DEFINE_METHOD(RUNTIME_WRAPPED_EXCEPTION, OBJ_CTOR, .ctor, IM_Obj_RetVoid)
DEFINE_FIELD(RUNTIME_WRAPPED_EXCEPTION, WRAPPED_EXCEPTION, _wrappedException)

DEFINE_CLASS(CALLCONV_CDECL, CompilerServices, CallConvCdecl)
DEFINE_CLASS(CALLCONV_STDCALL, CompilerServices, CallConvStdcall)
DEFINE_CLASS(CALLCONV_THISCALL, CompilerServices, CallConvThiscall)
DEFINE_CLASS(CALLCONV_FASTCALL, CompilerServices, CallConvFastcall)
DEFINE_CLASS(CALLCONV_SUPPRESSGCTRANSITION, CompilerServices, CallConvSuppressGCTransition)

DEFINE_CLASS_U(Interop, SafeHandle, SafeHandle)
DEFINE_FIELD_U(handle, SafeHandle, m_handle)
DEFINE_FIELD_U(_state, SafeHandle, m_state)
Expand Down
121 changes: 69 additions & 52 deletions src/coreclr/vm/dllimport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -256,12 +256,10 @@ class ILStubState : public StubState
Module* pStubModule,
const Signature &signature,
SigTypeContext* pTypeContext,
BOOL fTargetHasThis,
BOOL fStubHasThis,
DWORD dwStubFlags,
int iLCIDParamIdx,
MethodDesc* pTargetMD)
: m_slIL(dwStubFlags, pStubModule, signature, pTypeContext, pTargetMD, iLCIDParamIdx, fTargetHasThis, fStubHasThis)
: m_slIL(dwStubFlags, pStubModule, signature, pTypeContext, pTargetMD, iLCIDParamIdx)
{
STANDARD_VM_CONTRACT;

Expand Down Expand Up @@ -1221,8 +1219,6 @@ class StructMarshal_ILStubState : public ILStubState
pMT->GetModule(),
signature,
pTypeContext,
FALSE,
FALSE,
dwStubFlags,
-1 /* We have no LCID parameter */,
nullptr),
Expand Down Expand Up @@ -1349,9 +1345,7 @@ class PInvoke_ILStubState : public ILStubState
pStubModule,
signature,
pTypeContext,
TargetHasThis(dwStubFlags),
StubHasThis(dwStubFlags),
dwStubFlags,
UpdateStubFlags(dwStubFlags, pTargetMD),
iLCIDParamIdx,
pTargetMD)
{
Expand All @@ -1371,6 +1365,23 @@ class PInvoke_ILStubState : public ILStubState
}

private:
static DWORD UpdateStubFlags(DWORD dwStubFlags, MethodDesc* pTargetMD)
{
if (TargetHasThis(dwStubFlags))
{
dwStubFlags |= NDIRECTSTUB_FL_TARGET_HAS_THIS;
}
if (StubHasThis(dwStubFlags))
{
dwStubFlags |= NDIRECTSTUB_FL_STUB_HAS_THIS;
}
if (TargetSuppressGCTransition(dwStubFlags, pTargetMD))
{
dwStubFlags |= NDIRECTSTUB_FL_SUPPRESSGCTRANSITION;
}
return dwStubFlags;
}

static BOOL TargetHasThis(DWORD dwStubFlags)
{
//
Expand All @@ -1389,6 +1400,11 @@ class PInvoke_ILStubState : public ILStubState
//
return SF_IsForwardDelegateStub(dwStubFlags);
}

static BOOL TargetSuppressGCTransition(DWORD dwStubFlags, MethodDesc* pTargetMD)
{
return SF_IsForwardStub(dwStubFlags) && pTargetMD && pTargetMD->ShouldSuppressGCTransition();
}
};

#ifdef FEATURE_COMINTEROP
Expand All @@ -1402,9 +1418,7 @@ class CLRToCOM_ILStubState : public ILStubState
pStubModule,
signature,
pTypeContext,
TRUE,
TRUE,
dwStubFlags,
dwStubFlags | NDIRECTSTUB_FL_STUB_HAS_THIS | NDIRECTSTUB_FL_TARGET_HAS_THIS,
iLCIDParamIdx,
pTargetMD)
{
Expand Down Expand Up @@ -1470,9 +1484,7 @@ class COMToCLR_ILStubState : public ILStubState
pStubModule,
signature,
pTypeContext,
TRUE,
TRUE,
dwStubFlags,
dwStubFlags | NDIRECTSTUB_FL_STUB_HAS_THIS | NDIRECTSTUB_FL_TARGET_HAS_THIS,
iLCIDParamIdx,
pTargetMD)
{
Expand Down Expand Up @@ -1531,24 +1543,47 @@ class COMToCLRFieldAccess_ILStubState : public COMToCLR_ILStubState
};
#endif // FEATURE_COMINTEROP

ILStubLinkerFlags GetILStubLinkerFlagsForNDirectStubFlags(NDirectStubFlags flags)
{
DWORD result = ILSTUB_LINKER_FLAG_NONE;
if (!SF_IsCOMStub(flags))
{
result |= ILSTUB_LINKER_FLAG_NDIRECT;
}
if (SF_IsReverseStub(flags))
{
result |= ILSTUB_LINKER_FLAG_REVERSE;
}
if (flags & NDIRECTSTUB_FL_SUPPRESSGCTRANSITION)
{
result |= ILSTUB_LINKER_FLAG_SUPPRESSGCTRANSITION;
}
if (flags & NDIRECTSTUB_FL_STUB_HAS_THIS)
{
result |= ILSTUB_LINKER_FLAG_STUB_HAS_THIS;
}
if (flags & NDIRECTSTUB_FL_TARGET_HAS_THIS)
{
result |= ILSTUB_LINKER_FLAG_TARGET_HAS_THIS;
}
return (ILStubLinkerFlags)result;
}

NDirectStubLinker::NDirectStubLinker(
DWORD dwStubFlags,
Module* pModule,
const Signature &signature,
SigTypeContext *pTypeContext,
MethodDesc* pTargetMD,
int iLCIDParamIdx,
BOOL fTargetHasThis,
BOOL fStubHasThis)
: ILStubLinker(pModule, signature, pTypeContext, pTargetMD, fTargetHasThis, fStubHasThis, !SF_IsCOMStub(dwStubFlags), SF_IsReverseStub(dwStubFlags)),
int iLCIDParamIdx)
: ILStubLinker(pModule, signature, pTypeContext, pTargetMD, GetILStubLinkerFlagsForNDirectStubFlags((NDirectStubFlags)dwStubFlags)),
m_pCleanupFinallyBeginLabel(NULL),
m_pCleanupFinallyEndLabel(NULL),
m_pSkipExceptionCleanupLabel(NULL),
m_fHasCleanupCode(FALSE),
m_fHasExceptionCleanupCode(FALSE),
m_fCleanupWorkListIsSetup(FALSE),
m_targetHasThis(fTargetHasThis),
m_targetHasThis((dwStubFlags & NDIRECTSTUB_FL_TARGET_HAS_THIS) != 0),
m_dwThreadLocalNum(-1),
m_dwCleanupWorkListLocalNum(-1),
m_dwRetValLocalNum(-1),
Expand Down Expand Up @@ -2548,13 +2583,7 @@ void PInvokeStaticSigInfo::PreInit(MethodDesc* pMD)
PInvokeStaticSigInfo::PInvokeStaticSigInfo(
MethodDesc* pMD, LPCUTF8 *pLibName, LPCUTF8 *pEntryPointName)
{
CONTRACTL
{
THROWS;
GC_NOTRIGGER;
MODE_ANY;
}
CONTRACTL_END;
STANDARD_VM_CONTRACT;

DllImportInit(pMD, pLibName, pEntryPointName);

Expand All @@ -2565,10 +2594,7 @@ PInvokeStaticSigInfo::PInvokeStaticSigInfo(MethodDesc* pMD, ThrowOnError throwOn
{
CONTRACTL
{
THROWS;
GC_NOTRIGGER;
MODE_ANY;

STANDARD_VM_CHECK;
PRECONDITION(CheckPointer(pMD));
}
CONTRACTL_END;
Expand Down Expand Up @@ -2666,9 +2692,7 @@ PInvokeStaticSigInfo::PInvokeStaticSigInfo(
{
CONTRACTL
{
THROWS;
GC_NOTRIGGER;
MODE_ANY;
STANDARD_VM_CHECK;

PRECONDITION(CheckPointer(pModule));
}
Expand All @@ -2686,9 +2710,7 @@ void PInvokeStaticSigInfo::DllImportInit(MethodDesc* pMD, LPCUTF8 *ppLibName, LP
{
CONTRACTL
{
THROWS;
GC_NOTRIGGER;
MODE_ANY;
STANDARD_VM_CHECK;

PRECONDITION(CheckPointer(pMD));

Expand Down Expand Up @@ -2968,16 +2990,11 @@ namespace
_Out_ CorPinvokeMap *pPinvokeMapOut,
_Out_ UINT *errorResID)
{
CONTRACTL
{
NOTHROW;
GC_NOTRIGGER;
MODE_ANY;
}
CONTRACTL_END
STANDARD_VM_CONTRACT;

CorUnmanagedCallingConvention callConvMaybe;
HRESULT hr = MetaSig::TryGetUnmanagedCallingConventionFromModOpt(pModule, pSig, cSig, &callConvMaybe, errorResID);
bool suppressGCTransition;
HRESULT hr = MetaSig::TryGetUnmanagedCallingConventionFromModOpt(GetScopeHandle(pModule), pSig, cSig, &callConvMaybe, &suppressGCTransition, errorResID);
if (hr != S_OK)
return hr;

Expand All @@ -2990,13 +3007,7 @@ namespace

void PInvokeStaticSigInfo::InitCallConv(CorPinvokeMap callConv, BOOL bIsVarArg)
{
CONTRACTL
{
NOTHROW;
GC_NOTRIGGER;
MODE_ANY;
}
CONTRACTL_END
STANDARD_VM_CONTRACT;

// Convert WinAPI methods to either StdCall or CDecl based on if they are varargs or not.
if (callConv == pmCallConvWinapi)
Expand Down Expand Up @@ -6714,7 +6725,8 @@ PCODE GetILStubForCalli(VASigCookie *pVASigCookie, MethodDesc *pMD)
{
CorUnmanagedCallingConvention callConvMaybe;
UINT errorResID;
HRESULT hr = MetaSig::TryGetUnmanagedCallingConventionFromModOpt(pVASigCookie->pModule, signature.GetRawSig(), signature.GetRawSigLen(), &callConvMaybe, &errorResID);
bool suppressGCTransition = false;
HRESULT hr = MetaSig::TryGetUnmanagedCallingConventionFromModOpt(GetScopeHandle(pVASigCookie->pModule), signature.GetRawSig(), signature.GetRawSigLen(), &callConvMaybe, &suppressGCTransition, &errorResID);
if (FAILED(hr))
COMPlusThrowHR(hr, errorResID);

Expand All @@ -6726,6 +6738,11 @@ PCODE GetILStubForCalli(VASigCookie *pVASigCookie, MethodDesc *pMD)
{
callConv = MetaSig::GetDefaultUnmanagedCallingConvention();
}

if (suppressGCTransition)
{
dwStubFlags |= NDIRECTSTUB_FL_SUPPRESSGCTRANSITION;
}
}

if (!TryConvertCallConvValueToPInvokeCallConv(callConv, &unmgdCallConv))
Expand Down
Loading

0 comments on commit 92870e5

Please sign in to comment.