Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Handle 'unmanaged' calling convention value #39030

Merged
merged 4 commits into from
Jul 15, 2020
Merged

Handle 'unmanaged' calling convention value #39030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6947f5b
Handle 'unmanaged' calling convention value
elinor-fung Jul 9, 2020
990ad1d
Update managed type system
elinor-fung Jul 14, 2020
69e5f5e
Fix checking of modifiers to only look on return type
elinor-fung Jul 14, 2020
fb786e7
Update comments
elinor-fung Jul 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions src/coreclr/src/tools/Common/JitInterface/CorInfoImpl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,49 @@ private void Get_CORINFO_SIG_INFO(MethodDesc method, CORINFO_SIG_INFO* sig, bool
}
}

private bool TryGetUnmanagedCallingConventionFromModOpt(MethodSignature signature, out CorInfoCallConv callConv)
{
callConv = CorInfoCallConv.CORINFO_CALLCONV_UNMANAGED;
if (!signature.HasEmbeddedSignatureData || signature.GetEmbeddedSignatureData() == null)
return false;

foreach (EmbeddedSignatureData data in signature.GetEmbeddedSignatureData())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be looking at the custom modifier on the return parameter only.

If someone places a modifier on one of the parameters, it should be ignored. I think this can actually be hit with C# - a method signature that has a function pointer as one of the parameters would have multiple modifiers in it.

I don't know how to extract that information from EmbeddedSignatureData. It's a ridiculously bad API and it should have never been merged into the type system. I get unreasonably angry whenever I see it. @davidwrighton might have advice.

Copy link
Member Author

@elinor-fung elinor-fung Jul 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Boo, totally missed that. Could the EmbeddedSignatureData.index string be an indicator? From looking through how it is set, it looks like the modifiers for the return type should have the 'lowest' index, but I'm not entirely sure what exactly that is representing / how to parse that? It seems like the lowest possible is 0.1.1.1 from what I can tell?

@davidwrighton - would appreciate any guidance. I'd like to get this change done before the P8 snap tomorrow.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... yes, the api is the worst. (but it does ... work... blech.) It represents the hierarchy of parsing the signature, and it actually allows for substitution inside of the signature without breaking stuff (which I must say is somewhat miraculous).

However, the goal was to require the modopt for these to be at the very beginning of the modifier stream before any non mod opts. Fortunately, that will result in the index following a consistent pattern. I need to do some research to understand the actual pattern you need to actually work with. I hope I can get back to you today.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so, you'll want to only examine those modopts which have exactly the index of 0.1.1.1 I've just filed #39329 which adds a test to ensure that this behavior remains consistent.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might also consider adding an constant string to the MethodSignature type named IndexOfCustomModifiersOnReturnType and put the magic string in there. I don't like the idea of blindly writing the literal string 0.1.1.1 here.

{
if (data.kind != EmbeddedSignatureDataKind.OptionalCustomModifier)
continue;

// We only care about the modifiers for the return type. These will be at the start of
// the signature, so will be first in the array of embedded signature data.
if (data.index != MethodSignature.IndexOfCustomModifiersOnReturnType)
break;

if (!(data.type is DefType defType))
continue;

if (defType.Namespace != "System.Runtime.CompilerServices")
continue;

// Take the first recognized calling convention in metadata.
switch (defType.Name)
{
case "CallConvCdecl":
AaronRobinsonMSFT marked this conversation as resolved.
Show resolved Hide resolved
callConv = CorInfoCallConv.CORINFO_CALLCONV_C;
return true;
case "CallConvStdcall":
callConv = CorInfoCallConv.CORINFO_CALLCONV_STDCALL;
return true;
case "CallConvFastcall":
callConv = CorInfoCallConv.CORINFO_CALLCONV_FASTCALL;
return true;
case "CallConvThiscall":
callConv = CorInfoCallConv.CORINFO_CALLCONV_THISCALL;
return true;
}
}

return false;
}

private void Get_CORINFO_SIG_INFO(MethodSignature signature, CORINFO_SIG_INFO* sig)
{
sig->callConv = (CorInfoCallConv)(signature.Flags & MethodSignatureFlags.UnmanagedCallingConventionMask);
Expand All @@ -520,6 +563,22 @@ private void Get_CORINFO_SIG_INFO(MethodSignature signature, CORINFO_SIG_INFO* s

if (!signature.IsStatic) sig->callConv |= CorInfoCallConv.CORINFO_CALLCONV_HASTHIS;

// Unmanaged calling convention indicates modopt should be read
if (sig->callConv == CorInfoCallConv.CORINFO_CALLCONV_UNMANAGED)
{
if (TryGetUnmanagedCallingConventionFromModOpt(signature, out CorInfoCallConv callConvMaybe))
{
sig->callConv = callConvMaybe;
}
else
{
// Use platform default
sig->callConv = _compilation.TypeSystemContext.Target.IsWindows
? CorInfoCallConv.CORINFO_CALLCONV_STDCALL
: CorInfoCallConv.CORINFO_CALLCONV_C;
}
}

TypeDesc returnType = signature.ReturnType;

CorInfoType corInfoRetType = asCorInfoType(signature.ReturnType, &sig->retTypeClass);
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/src/tools/Common/TypeSystem/Common/MethodDesc.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ public sealed partial class MethodSignature : TypeSystemEntity
internal TypeDesc[] _parameters;
internal EmbeddedSignatureData[] _embeddedSignatureData;

// Value of <see cref="EmbeddedSignatureData.index" /> for any custom modifiers on the return type
public const string IndexOfCustomModifiersOnReturnType = "0.1.1.1";

public MethodSignature(MethodSignatureFlags flags, int genericParameterCount, TypeDesc returnType, TypeDesc[] parameters, EmbeddedSignatureData[] embeddedSignatureData = null)
{
_flags = flags;
Expand Down
89 changes: 69 additions & 20 deletions src/coreclr/src/vm/dllimport.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2923,6 +2923,57 @@ inline CorPinvokeMap GetDefaultCallConv(BOOL bIsVarArg)
#endif // !TARGET_UNIX
}

namespace
{
bool TryConvertCallConvValueToPInvokeCallConv(_In_ BYTE callConv, _Out_ CorPinvokeMap *pPinvokeMapOut)
{
elinor-fung marked this conversation as resolved.
Show resolved Hide resolved
LIMITED_METHOD_CONTRACT;

switch (callConv)
{
case IMAGE_CEE_CS_CALLCONV_C:
*pPinvokeMapOut = pmCallConvCdecl;
return true;
case IMAGE_CEE_CS_CALLCONV_STDCALL:
*pPinvokeMapOut = pmCallConvStdcall;
return true;
case IMAGE_CEE_CS_CALLCONV_THISCALL:
*pPinvokeMapOut = pmCallConvThiscall;
return true;
case IMAGE_CEE_CS_CALLCONV_FASTCALL:
*pPinvokeMapOut = pmCallConvFastcall;
return true;
}

return false;
}

HRESULT GetUnmanagedPInvokeCallingConvention(
_In_ Module *pModule,
_In_ PCCOR_SIGNATURE pSig,
_In_ ULONG cSig,
_Out_ CorPinvokeMap *pPinvokeMapOut)
{
elinor-fung marked this conversation as resolved.
Show resolved Hide resolved
CONTRACTL
{
NOTHROW;
GC_NOTRIGGER;
MODE_ANY;
}
CONTRACTL_END

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

if (!TryConvertCallConvValueToPInvokeCallConv(callConvMaybe, pPinvokeMapOut))
return S_FALSE;

return hr;
}
}

void PInvokeStaticSigInfo::InitCallConv(CorPinvokeMap callConv, BOOL bIsVarArg)
{
CONTRACTL
Expand All @@ -2938,9 +2989,8 @@ void PInvokeStaticSigInfo::InitCallConv(CorPinvokeMap callConv, BOOL bIsVarArg)
callConv = GetDefaultCallConv(bIsVarArg);

CorPinvokeMap sigCallConv = (CorPinvokeMap)0;
BOOL fSuccess = MetaSig::GetUnmanagedCallingConvention(m_pModule, m_sig.GetRawSig(), m_sig.GetRawSigLen(), &sigCallConv);

if (!fSuccess)
HRESULT hr = GetUnmanagedPInvokeCallingConvention(m_pModule, m_sig.GetRawSig(), m_sig.GetRawSigLen(), &sigCallConv);
if (FAILED(hr))
{
SetError(IDS_EE_NDIRECT_BADNATL); //Bad metadata format
}
Expand Down Expand Up @@ -6778,26 +6828,25 @@ PCODE GetILStubForCalli(VASigCookie *pVASigCookie, MethodDesc *pMD)
dwStubFlags |= NDIRECTSTUB_FL_UNMANAGED_CALLI;

// need to convert the CALLI signature to stub signature with managed calling convention
switch (MetaSig::GetCallingConvention(pVASigCookie->pModule, pVASigCookie->signature))
BYTE callConv = MetaSig::GetCallingConvention(pVASigCookie->pModule, signature);

// Unmanaged calling convention indicates modopt should be read
if (callConv == IMAGE_CEE_CS_CALLCONV_UNMANAGED)
{
case IMAGE_CEE_CS_CALLCONV_C:
unmgdCallConv = pmCallConvCdecl;
break;
case IMAGE_CEE_CS_CALLCONV_STDCALL:
unmgdCallConv = pmCallConvStdcall;
break;
case IMAGE_CEE_CS_CALLCONV_THISCALL:
unmgdCallConv = pmCallConvThiscall;
break;
case IMAGE_CEE_CS_CALLCONV_FASTCALL:
unmgdCallConv = pmCallConvFastcall;
break;
case IMAGE_CEE_CS_CALLCONV_UNMANAGED:
COMPlusThrow(kNotImplementedException);
default:
COMPlusThrow(kTypeLoadException, IDS_INVALID_PINVOKE_CALLCONV);
CorUnmanagedCallingConvention callConvMaybe;
if (S_OK == MetaSig::TryGetUnmanagedCallingConventionFromModOpt(pVASigCookie->pModule, signature.GetRawSig(), signature.GetRawSigLen(), &callConvMaybe))
{
callConv = callConvMaybe;
}
else
{
callConv = MetaSig::GetDefaultUnmanagedCallingConvention();
}
}

if (!TryConvertCallConvValueToPInvokeCallConv(callConv, &unmgdCallConv))
COMPlusThrow(kTypeLoadException, IDS_INVALID_PINVOKE_CALLCONV);

LoaderHeap *pHeap = pVASigCookie->pModule->GetLoaderAllocator()->GetHighFrequencyHeap();
PCOR_SIGNATURE new_sig = (PCOR_SIGNATURE)(void *)pHeap->AllocMem(S_SIZE_T(signature.GetRawSigLen()));
CopyMemory(new_sig, signature.GetRawSig(), signature.GetRawSigLen());
Expand Down
32 changes: 24 additions & 8 deletions src/coreclr/src/vm/jitinterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,10 +465,10 @@ CEEInfo::ConvToJitSig(
SigTypeContext::InitTypeContext(contextType, &typeContext);
}

_ASSERTE(CORINFO_CALLCONV_DEFAULT == (CorInfoCallConv) IMAGE_CEE_CS_CALLCONV_DEFAULT);
_ASSERTE(CORINFO_CALLCONV_VARARG == (CorInfoCallConv) IMAGE_CEE_CS_CALLCONV_VARARG);
_ASSERTE(CORINFO_CALLCONV_MASK == (CorInfoCallConv) IMAGE_CEE_CS_CALLCONV_MASK);
_ASSERTE(CORINFO_CALLCONV_HASTHIS == (CorInfoCallConv) IMAGE_CEE_CS_CALLCONV_HASTHIS);
static_assert_no_msg(CORINFO_CALLCONV_DEFAULT == (CorInfoCallConv) IMAGE_CEE_CS_CALLCONV_DEFAULT);
static_assert_no_msg(CORINFO_CALLCONV_VARARG == (CorInfoCallConv) IMAGE_CEE_CS_CALLCONV_VARARG);
static_assert_no_msg(CORINFO_CALLCONV_MASK == (CorInfoCallConv) IMAGE_CEE_CS_CALLCONV_MASK);
static_assert_no_msg(CORINFO_CALLCONV_HASTHIS == (CorInfoCallConv) IMAGE_CEE_CS_CALLCONV_HASTHIS);

TypeHandle typeHnd = TypeHandle();

Expand Down Expand Up @@ -506,6 +506,25 @@ CEEInfo::ConvToJitSig(
}
#endif // defined(TARGET_UNIX) || defined(TARGET_ARM)

// Unmanaged calling convention indicates modopt should be read
if (sigRet->callConv == CORINFO_CALLCONV_UNMANAGED)
{
static_assert_no_msg(CORINFO_CALLCONV_C == (CorInfoCallConv)IMAGE_CEE_UNMANAGED_CALLCONV_C);
static_assert_no_msg(CORINFO_CALLCONV_STDCALL == (CorInfoCallConv)IMAGE_CEE_UNMANAGED_CALLCONV_STDCALL);
static_assert_no_msg(CORINFO_CALLCONV_THISCALL == (CorInfoCallConv)IMAGE_CEE_UNMANAGED_CALLCONV_THISCALL);
static_assert_no_msg(CORINFO_CALLCONV_FASTCALL == (CorInfoCallConv)IMAGE_CEE_UNMANAGED_CALLCONV_FASTCALL);

CorUnmanagedCallingConvention callConvMaybe;
if (S_OK == MetaSig::TryGetUnmanagedCallingConventionFromModOpt(module, pSig, cbSig, &callConvMaybe))
{
sigRet->callConv = (CorInfoCallConv)callConvMaybe;
}
else
{
sigRet->callConv = (CorInfoCallConv)MetaSig::GetDefaultUnmanagedCallingConvention();
}
}

// Skip number of type arguments
if (sigRet->callConv & IMAGE_CEE_CS_CALLCONV_GENERIC)
IfFailThrow(sig.GetData(NULL));
Expand Down Expand Up @@ -566,10 +585,7 @@ CEEInfo::ConvToJitSig(
sigRet->args = (CORINFO_ARG_LIST_HANDLE)sig.GetPtr();
}

if (sigRet->callConv == CORINFO_CALLCONV_UNMANAGED)
{
COMPlusThrowHR(E_NOTIMPL);
}
_ASSERTE(sigRet->callConv != CORINFO_CALLCONV_UNMANAGED);

// Set computed flags
sigRet->flags = sigRetFlags;
Expand Down
109 changes: 78 additions & 31 deletions src/coreclr/src/vm/siginfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5219,70 +5219,117 @@ BOOL MetaSig::IsReturnTypeVoid() const

#ifndef DACCESS_COMPILE

namespace
{
HRESULT GetNameOfTypeRefOrDef(
_In_ const Module *pModule,
_In_ mdToken token,
_Out_ LPCSTR *namespaceOut,
_Out_ LPCSTR *nameOut)
{
CONTRACTL
{
NOTHROW;
GC_NOTRIGGER;
FORBID_FAULT;
MODE_ANY;
}
CONTRACTL_END

IMDInternalImport *pInternalImport = pModule->GetMDImport();
if (TypeFromToken(token) == mdtTypeDef)
{
HRESULT hr = pInternalImport->GetNameOfTypeDef(token, nameOut, namespaceOut);
if (FAILED(hr))
return hr;
}
else if (TypeFromToken(token) == mdtTypeRef)
{
HRESULT hr = pInternalImport->GetNameOfTypeRef(token, namespaceOut, nameOut);
if (FAILED(hr))
return hr;
}
else
{
return E_INVALIDARG;
}

return S_OK;
}
}

//----------------------------------------------------------
// Returns the unmanaged calling convention.
//----------------------------------------------------------
/*static*/
BOOL
MetaSig::GetUnmanagedCallingConvention(
Module * pModule,
PCCOR_SIGNATURE pSig,
ULONG cSig,
CorPinvokeMap * pPinvokeMapOut)
HRESULT
MetaSig::TryGetUnmanagedCallingConventionFromModOpt(
_In_ Module *pModule,
_In_ PCCOR_SIGNATURE pSig,
_In_ ULONG cSig,
_Out_ CorUnmanagedCallingConvention *callConvOut)
{
CONTRACTL
{
NOTHROW;
GC_NOTRIGGER;
FORBID_FAULT;
MODE_ANY;
PRECONDITION(callConvOut != NULL);
}
CONTRACTL_END


// Instantiations aren't relevant here
MetaSig msig(pSig, cSig, pModule, NULL);
PCCOR_SIGNATURE pWalk = msig.m_pRetType.GetPtr();
_ASSERTE(pWalk <= pSig + cSig);

*callConvOut = (CorUnmanagedCallingConvention)0;
while ((pWalk < (pSig + cSig)) && ((*pWalk == ELEMENT_TYPE_CMOD_OPT) || (*pWalk == ELEMENT_TYPE_CMOD_REQD)))
{
BOOL fIsOptional = (*pWalk == ELEMENT_TYPE_CMOD_OPT);

pWalk++;
if (pWalk + CorSigUncompressedDataSize(pWalk) > pSig + cSig)
{
return FALSE; // Bad formatting
}
return E_FAIL; // Bad formatting

mdToken tk;
pWalk += CorSigUncompressToken(pWalk, &tk);

if (fIsOptional)
if (!fIsOptional)
continue;

LPCSTR typeNamespace;
LPCSTR typeName;

// Check for CallConv types specified in modopt
if (FAILED(GetNameOfTypeRefOrDef(pModule, tk, &typeNamespace, &typeName)))
continue;

if (::strcmp(typeNamespace, CMOD_CALLCONV_NAMESPACE) != 0)
continue;

const struct {
LPCSTR name;
CorUnmanagedCallingConvention value;
} knownCallConvs[] = {
{ CMOD_CALLCONV_NAME_CDECL, IMAGE_CEE_UNMANAGED_CALLCONV_C },
{ CMOD_CALLCONV_NAME_STDCALL, IMAGE_CEE_UNMANAGED_CALLCONV_STDCALL },
{ CMOD_CALLCONV_NAME_THISCALL, IMAGE_CEE_UNMANAGED_CALLCONV_THISCALL },
{ CMOD_CALLCONV_NAME_FASTCALL, IMAGE_CEE_UNMANAGED_CALLCONV_FASTCALL } };

for (const auto &callConv : knownCallConvs)
{
if (IsTypeRefOrDef("System.Runtime.CompilerServices.CallConvCdecl", pModule, tk))
// Take the first recognized calling convention in metadata.
if (::strcmp(typeName, callConv.name) == 0)
{
*pPinvokeMapOut = pmCallConvCdecl;
return TRUE;
}
else if (IsTypeRefOrDef("System.Runtime.CompilerServices.CallConvStdcall", pModule, tk))
{
*pPinvokeMapOut = pmCallConvStdcall;
return TRUE;
}
else if (IsTypeRefOrDef("System.Runtime.CompilerServices.CallConvThiscall", pModule, tk))
{
*pPinvokeMapOut = pmCallConvThiscall;
return TRUE;
}
else if (IsTypeRefOrDef("System.Runtime.CompilerServices.CallConvFastcall", pModule, tk))
{
*pPinvokeMapOut = pmCallConvFastcall;
return TRUE;
*callConvOut = callConv.value;
return S_OK;
}
}
}

*pPinvokeMapOut = (CorPinvokeMap)0;
return TRUE;
return S_FALSE;
}

//---------------------------------------------------------------------------------------
Expand Down
Loading