Skip to content

Commit

Permalink
Instead of carrying around a token map, introduce a new kind of "inte…
Browse files Browse the repository at this point in the history 
…rnal modifier" type for modifiers in internal signatures.
  • Loading branch information
@jkoritzinsky
jkoritzinsky committed Aug 16, 2024
1 parent 4371de2 commit bb9fe1a
Show file tree
Hide file tree
Showing 22 changed files with 484 additions and 553 deletions.
3 changes: 3 additions & 0 deletions src/coreclr/ildasm/dasm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2088,6 +2088,9 @@ BYTE* PrettyPrintCABlobValue(PCCOR_SIGNATURE &typePtr,

#ifdef LOGGING
case ELEMENT_TYPE_INTERNAL :
case ELEMENT_TYPE_CMOD_INTERNAL :
typePtr += 1;
Reiterate = TRUE;
#endif // LOGGING
return NULL;

Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/inc/corhdr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -911,9 +911,10 @@ typedef enum CorElementType

// This is for signatures generated internally (which will not be persisted in any way).
ELEMENT_TYPE_INTERNAL = 0x21, // INTERNAL <typehandle>
ELEMENT_TYPE_CMOD_INTERNAL = 0x22, // CMOD_INTERNAL <reqd> <typehandle>

// Note that this is the max of base type excluding modifiers
ELEMENT_TYPE_MAX = 0x22, // first invalid element type
ELEMENT_TYPE_MAX = 0x23, // first invalid element type


ELEMENT_TYPE_MODIFIER = 0x40,
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/inc/cortypeinfo.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,4 @@ TYPEINFO(ELEMENT_TYPE_MVAR, NULL, NULL, TARGET_POINTER_SI
TYPEINFO(ELEMENT_TYPE_CMOD_REQD, NULL, NULL, 0, TYPE_GC_NONE, false, false, false, false, false) // 0x1f
TYPEINFO(ELEMENT_TYPE_CMOD_OPT, NULL, NULL, 0, TYPE_GC_NONE, false, false, false, false, false) // 0x20
TYPEINFO(ELEMENT_TYPE_INTERNAL, NULL, NULL, 0, TYPE_GC_OTHER, false, false, false, false, false) // 0x21
TYPEINFO(ELEMENT_TYPE_CMOD_INTERNAL,NULL, NULL, 0, TYPE_GC_NONE, false, false, false, false, false) // 0x22
38 changes: 38 additions & 0 deletions src/coreclr/inc/formattype.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,44 @@ PCCOR_SIGNATURE PrettyPrintType(
appendStr(out, sz);
break;
}
case ELEMENT_TYPE_CMOD_INTERNAL :
{
// ELEMENT_TYPE_CMOD_INTERNAL <required> <TypeHandle>
bool required = *typePtr++ != 0;
_ASSERTE(sizeof(TypeHandle) == sizeof(void *));
TypeHandle typeHandle;
typePtr += CorSigUncompressPointer(typePtr, (void **)&typeHandle);

MethodTable *pMT = NULL;
if (typeHandle.IsTypeDesc())
{
pMT = typeHandle.AsTypeDesc()->GetMethodTable();
if (pMT)
{
PrettyPrintClass(out, pMT->GetCl(), pMT->GetMDImport());

// It could be a "native version" of the managed type used in interop
if (typeHandle.AsTypeDesc()->IsNativeValueType())
appendStr(out, "_NativeValueType");
}
else
appendStr(out, "(null)");
}
else
{
pMT = typeHandle.AsMethodTable();
if (pMT)
PrettyPrintClass(out, pMT->GetCl(), pMT->GetMDImport());
else
appendStr(out, "(null)");
}

const char fmt[] = " mod%s(/* MT: %p */)";
char sz[Max64BitHexString + ARRAY_SIZE(fmt) + ARRAY_SIZE("req")];
sprintf_s(sz, ARRAY_SIZE(sz), fmt, required ? "req" : "opt", pMT);
appendStr(out, sz);
break;
}
#endif


Expand Down
58 changes: 42 additions & 16 deletions src/coreclr/inc/sigparser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -582,19 +582,32 @@ class SigParser
return hr;

while ((ELEMENT_TYPE_CMOD_REQD == bElementType) ||
(ELEMENT_TYPE_CMOD_OPT == bElementType))
(ELEMENT_TYPE_CMOD_OPT == bElementType) ||
(ELEMENT_TYPE_CMOD_INTERNAL == bElementType))
{
sigTemp.SkipBytes(1);
if (ELEMENT_TYPE_CMOD_INTERNAL == bElementType)
{
void * pMT;
uint8_t required;
if (FAILED(hr = sigTemp.GetByte(&required)))
return hr;

if (FAILED(hr = sigTemp.GetPointer(&pMT)))
return hr;

}
else
{
mdToken token;

mdToken token;

hr = sigTemp.GetToken(&token);
hr = sigTemp.GetToken(&token);

if (FAILED(hr))
return hr;
if (FAILED(hr))
return hr;
}

hr = sigTemp.PeekByte(&bElementType);
if (FAILED(hr))
if (FAILED(hr = sigTemp.PeekByte(&bElementType)))
return hr;
}

Expand Down Expand Up @@ -643,19 +656,32 @@ class SigParser
while (ELEMENT_TYPE_CMOD_REQD == bElementType ||
ELEMENT_TYPE_CMOD_OPT == bElementType ||
ELEMENT_TYPE_MODIFIER == bElementType ||
ELEMENT_TYPE_PINNED == bElementType)
ELEMENT_TYPE_PINNED == bElementType ||
ELEMENT_TYPE_CMOD_INTERNAL == bElementType)
{
sigTemp.SkipBytes(1);
if (ELEMENT_TYPE_CMOD_INTERNAL == bElementType)
{
void * pMT;
uint8_t required;
if (FAILED(hr = sigTemp.GetByte(&required)))
return hr;

if (FAILED(hr = sigTemp.GetPointer(&pMT)))
return hr;

}
else
{
mdToken token;

mdToken token;

hr = sigTemp.GetToken(&token);
hr = sigTemp.GetToken(&token);

if (FAILED(hr))
return hr;
if (FAILED(hr))
return hr;
}

hr = sigTemp.PeekByte(&bElementType);
if (FAILED(hr))
if (FAILED(hr = sigTemp.PeekByte(&bElementType)))
return hr;
}

Expand Down
12 changes: 12 additions & 0 deletions src/coreclr/utilcode/prettyprintsig.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,18 @@ static HRESULT PrettyPrintTypeA(
sprintf_s(tempBuffer, 64, "pMT: %p", pMT);
IfFailGo(appendStrA(out, tempBuffer));
break;

case ELEMENT_TYPE_CMOD_INTERNAL:
{
bool required = *typePtr++ != 0;
void* pMT;
pMT = *((void* UNALIGNED *)typePtr);
typePtr += sizeof(void*);
CHAR tempBuffer[64];
sprintf_s(tempBuffer, 64, "pMT: %p", pMT);
IfFailGo(appendStrA(out, tempBuffer));
break;
}

case ELEMENT_TYPE_VALUETYPE:
str = "value class ";
Expand Down
50 changes: 37 additions & 13 deletions src/coreclr/vm/callconvbuilder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,25 +358,49 @@ HRESULT CallConv::TryGetUnmanagedCallingConventionFromModOpt(
_ASSERTE(pWalk <= pSig + cSig);

CallConvBuilder& callConvBuilder = *builder;
while ((pWalk < (pSig + cSig)) && ((*pWalk == ELEMENT_TYPE_CMOD_OPT) || (*pWalk == ELEMENT_TYPE_CMOD_REQD)))
while ((pWalk < (pSig + cSig)) && ((*pWalk == ELEMENT_TYPE_CMOD_OPT) || (*pWalk == ELEMENT_TYPE_CMOD_REQD) || (*pWalk == ELEMENT_TYPE_CMOD_INTERNAL)))
{
BOOL fIsOptional = (*pWalk == ELEMENT_TYPE_CMOD_OPT);

pWalk++;
if (pWalk + CorSigUncompressedDataSize(pWalk) > pSig + cSig)
CORINFO_MODULE_HANDLE tokenLookupModule = pModule;
mdToken tk;
LPCSTR typeNamespace;
LPCSTR typeName;
if (*pWalk == ELEMENT_TYPE_CMOD_INTERNAL)
{
*errorResID = BFA_BAD_SIGNATURE;
return COR_E_BADIMAGEFORMAT; // Bad formatting
// Skip internal modifiers
pWalk++;
if (pWalk + 1 + sizeof(void*) > pSig + cSig)
{
*errorResID = BFA_BAD_SIGNATURE;
return COR_E_BADIMAGEFORMAT; // Bad formatting
}

BOOL required = *pWalk++ != 0;
void* pType;
pWalk += CorSigUncompressPointer(pWalk, &pType);
TypeHandle type = TypeHandle::FromPtr(pType);

if (!required)
continue;

tokenLookupModule = GetScopeHandle(type.GetModule());
tk = type.GetCl();
}
else
{
BOOL fIsOptional = (*pWalk == ELEMENT_TYPE_CMOD_OPT);

mdToken tk;
pWalk += CorSigUncompressToken(pWalk, &tk);
pWalk++;
if (pWalk + CorSigUncompressedDataSize(pWalk) > pSig + cSig)
{
*errorResID = BFA_BAD_SIGNATURE;
return COR_E_BADIMAGEFORMAT; // Bad formatting
}

if (!fIsOptional)
continue;
pWalk += CorSigUncompressToken(pWalk, &tk);

LPCSTR typeNamespace;
LPCSTR typeName;
if (!fIsOptional)
continue;
}

// Check for CallConv types specified in modopt
if (FAILED(GetNameOfTypeRefOrDef(pModule, tk, &typeNamespace, &typeName)))
Expand Down
44 changes: 8 additions & 36 deletions src/coreclr/vm/dllimport.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,19 +407,9 @@ class ILStubState : public StubState
SigBuilder sigBuilder;

{
if (pStubMD->IsNoMetadata() && pStubMD->AsDynamicMethodDesc()->HasFlags(DynamicMethodDesc::FlagIndependentSig))
{
// We already have a module-independent signature, but it is based on the current resolver in the stub method desc.
// We need to convert it based on our token lookup map.
SigPointer sigPtr(pStubMD->GetSig());
sigPtr.ConvertToInternalSignature(pStubMD->AsDynamicMethodDesc()->GetResolver(), pStubMD->GetModule(), NULL, &sigBuilder, GetTokenLookupMap());
}
else
{
// Convert to a module independent signature
SigPointer sigPtr(pStubMD->GetSig());
sigPtr.ConvertToInternalSignature(pStubMD->GetModule(), NULL, &sigBuilder, GetTokenLookupMap());
}
// Convert to a module independent signature
SigPointer sigPtr(pStubMD->GetSig());
sigPtr.ConvertToInternalSignature(pStubMD->GetModule(), NULL, &sigBuilder, /* bSkipCustomModifier */ FALSE);
}

//
Expand All @@ -440,7 +430,6 @@ class ILStubState : public StubState
memcpyNoGCRefs((void *)pNewSig, GetStubTargetMethodSig(), cbNewSig);

pStubMD->AsDynamicMethodDesc()->SetStoredMethodSig(pNewSig, cbNewSig);
pStubMD->AsDynamicMethodDesc()->SetFlags(DynamicMethodDesc::FlagIndependentSig);

SigPointer sigPtr(pNewSig, cbNewSig);
uint32_t callConvInfo;
Expand Down Expand Up @@ -480,20 +469,8 @@ class ILStubState : public StubState
{
SigBuilder sigBuilder;
_ASSERTE(pStubMD->IsNoMetadata());
DynamicMethodDesc* pDMD = pStubMD->AsDynamicMethodDesc();
if (pDMD->HasFlags(DynamicMethodDesc::FlagIndependentSig))
{
// We already have a module-independent signature, but it is based on the current resolver in the stub method desc.
// We need to convert it based on our token lookup map.
SigPointer sigPtr(pStubMD->GetSig());
sigPtr.ConvertToInternalSignature(pDMD->GetResolver(), pStubMD->GetModule(), NULL, &sigBuilder, GetTokenLookupMap());
}
else
{
SigPointer sigPtr(pStubMD->GetSig());
sigPtr.ConvertToInternalSignature(pStubMD->GetModule(), NULL, &sigBuilder, GetTokenLookupMap());
}

SigPointer sigPtr(pStubMD->GetSig());
sigPtr.ConvertToInternalSignature(pStubMD->GetModule(), NULL, &sigBuilder, /* bSkipCustomModifier */ FALSE);

//
// make a domain-local copy of the sig so that this state can outlive the
Expand All @@ -505,8 +482,7 @@ class ILStubState : public StubState

memcpyNoGCRefs((void *)pNewSig, pNewSigBuffer, cbNewSig);

pDMD->SetStoredMethodSig(pNewSig, cbNewSig);
pDMD->SetFlags(DynamicMethodDesc::FlagIndependentSig);
pStubMD->AsDynamicMethodDesc()->SetStoredMethodSig(pNewSig, cbNewSig);
}

void EmitInvokeTarget(MethodDesc *pStubMD)
Expand Down Expand Up @@ -4022,7 +3998,6 @@ namespace
DWORD m_StubFlags;

INT32 m_iLCIDArg;
INT32 m_tokenMapHash;
INT32 m_nParams;
BYTE m_rgbSigAndParamData[1];
// (dwParamAttr, cbNativeType) // length: number of parameters
Expand Down Expand Up @@ -4078,11 +4053,9 @@ namespace

// note that ConvertToInternalSignature also resolves generics so different instantiations will get different
// hash blobs for methods that have generic parameters in their signature
// The signature may have custom modifiers, so provide a token lookup map to resolve them.
// We'll include a hash of the token lookup map in the hash blob.
TokenLookupMap tokenLookupMap;
// The signature may have custom modifiers that influence behavior, so don't skip them
SigBuilder sigBuilder;
sigPtr.ConvertToInternalSignature(pParams->m_pModule, pParams->m_pTypeContext, &sigBuilder, &tokenLookupMap);
sigPtr.ConvertToInternalSignature(pParams->m_pModule, pParams->m_pTypeContext, &sigBuilder, /* bSkipCustomModifier */ FALSE);

DWORD cbSig;
PVOID pSig = sigBuilder.GetSignature(&cbSig);
Expand Down Expand Up @@ -4117,7 +4090,6 @@ namespace
pBlob->m_nlFlags = static_cast<BYTE>(pParams->m_nlFlags & ~nlfNoMangle); // this flag does not affect the stub
pBlob->m_iLCIDArg = pParams->m_iLCIDArg;

pBlob->m_tokenMapHash = tokenLookupMap.GetHashValue();
pBlob->m_StubFlags = pParams->m_dwStubFlags;
pBlob->m_nParams = pParams->m_nParamTokens;

Expand Down
1 change: 1 addition & 0 deletions src/coreclr/vm/gdbjit.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ static constexpr const int CorElementTypeToDWEncoding[] =
/* ELEMENT_TYPE_CMOD_REQD */ DW_ATE_address,
/* ELEMENT_TYPE_CMOD_OPT */ DW_ATE_address,
/* ELEMENT_TYPE_INTERNAL */ DW_ATE_address,
/* ELEMENT_TYPE_CMOD_INTERNAL */DW_ATE_address,
/* ELEMENT_TYPE_MAX */ DW_ATE_address,
};

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/ilmarshalers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3465,7 +3465,7 @@ MarshalerOverrideStatus ILBlittableValueClassWithCopyCtorMarshaler::ArgumentOver
// but on other platforms it comes by-reference
#ifdef TARGET_X86
LocalDesc locDesc(pargs->mm.m_pMT);
locDesc.AddModifier(true, pslIL->GetToken(pargs->mm.m_pSigMod));
locDesc.AddModifier(true, pargs->mm.m_pSigMod);
pslIL->SetStubTargetArgType(&locDesc);

pslILDispatch->EmitLDARGA(argidx);
Expand Down
Loading

0 comments on commit bb9fe1a

Please sign in to comment.