From 10acbeeb439a6d4b874eeba133e48bcfe9afe619 Mon Sep 17 00:00:00 2001 From: Bruce Forstall Date: Tue, 2 Feb 2021 02:04:26 -0800 Subject: [PATCH 1/2] Fix SuperPMI collection with crossgen2 Crossgen2 abuses the `CORINFO_SIG_INFO.pSig` field by treating it as an opaque handle/pointer, and not a pointer to an array of signature bytes. Work around this by detecting this case and storing/restoring the pointer instead of a signature byte array. Also, add and improve some dump functions. Mostly fixes https://github.com/dotnet/runtime/issues/47540 (Fixes it to about the same level of failures in crossgen1 compilations.) --- .../superpmi/superpmi-shared/agnostic.h | 6 + .../superpmi-shared/methodcontext.cpp | 204 ++++++++++++++---- .../superpmi/superpmi-shared/spmidumphelper.h | 70 ++++-- .../superpmi-shared/spmirecordhelper.h | 18 +- src/coreclr/inc/jiteeversionguid.h | 10 +- 5 files changed, 240 insertions(+), 68 deletions(-) diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shared/agnostic.h b/src/coreclr/ToolBox/superpmi/superpmi-shared/agnostic.h index b5e4cc43c2d236..b00b3fe907d88b 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shared/agnostic.h +++ b/src/coreclr/ToolBox/superpmi/superpmi-shared/agnostic.h @@ -27,6 +27,7 @@ struct Agnostic_CORINFO_SIG_INFO DWORDLONG args; DWORD pSig_Index; DWORD cbSig; + DWORDLONG sigHandle; // used instead of cbSig/pSig_Index on crossgen2 if (cbSig == 0) && (pSig != nullptr) DWORDLONG scope; DWORD token; }; @@ -128,13 +129,17 @@ struct Agnostic_CORINFO_RESOLVED_TOKENout struct Agnostic_GetArgType_Key { + // Partial CORINFO_SIG_INFO data DWORD flags; DWORD numArgs; DWORD sigInst_classInstCount; DWORD sigInst_classInst_Index; DWORD sigInst_methInstCount; DWORD sigInst_methInst_Index; + DWORDLONG sigHandle; // used instead of cbSig/pSig_Index on crossgen2 if (cbSig == 0) && (pSig != nullptr) DWORDLONG scope; + + // Other getArgType() arguments DWORDLONG args; }; @@ -146,6 +151,7 @@ struct Agnostic_GetArgClass_Key DWORD sigInst_methInst_Index; DWORDLONG scope; DWORDLONG args; + DWORDLONG sigHandle; // used instead of cbSig/pSig_Index on crossgen2 if (cbSig == 0) && (pSig != nullptr) }; struct Agnostic_GetBoundaries diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp index 30cef07453ac16..12d267b5e83295 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp @@ -512,6 +512,42 @@ void MethodContext::dumpToConsole(int mcNumber, bool simple) #include "crlwmlist.h" } +const char* toString(InfoAccessType iat) +{ + switch (iat) + { + case IAT_VALUE: + return "VALUE"; + case IAT_PVALUE: + return "PVALUE"; + case IAT_PPVALUE: + return "PPVALUE"; + case IAT_RELPVALUE: + return "RELPVALUE"; + default: + return "UNKNOWN"; + } +}; + +const char* toString(CORINFO_CALL_KIND cick) +{ + switch (cick) + { + case CORINFO_CALL: + return "CALL"; + case CORINFO_CALL_CODE_POINTER: + return "CALL_CODE_POINTER"; + case CORINFO_VIRTUALCALL_STUB: + return "VIRTUALCALL_STUB"; + case CORINFO_VIRTUALCALL_LDVIRTFTN: + return "VIRTUALCALL_LDVIRTFTN"; + case CORINFO_VIRTUALCALL_VTABLE: + return "VIRTUALCALL_VTABLE"; + default: + return "UNKNOWN"; + } +}; + const char* toString(CorInfoType cit) { switch (cit) @@ -567,6 +603,19 @@ const char* toString(CorInfoType cit) } } +const char* toString(CorInfoTypeWithMod cit) +{ + switch (cit) + { + case CORINFO_TYPE_BYREF | CORINFO_TYPE_MOD_PINNED: + return "pinned byref"; + case CORINFO_TYPE_CLASS | CORINFO_TYPE_MOD_PINNED: + return "pinned class"; + default: + return toString((CorInfoType)(cit & CORINFO_TYPE_MASK)); + } +} + unsigned int toCorInfoSize(CorInfoType cit) { switch (cit) @@ -1342,34 +1391,65 @@ void MethodContext::recGetCallInfo(CORINFO_RESOLVED_TOKEN* pResolvedToken, value.instParamLookup.accessType = (DWORD)pResult->instParamLookup.accessType; value.instParamLookup.handle = CastHandle(pResult->instParamLookup.handle); - value.wrapperDelegateInvoke = (DWORD)pResult->wrapperDelegateInvoke; + value.wrapperDelegateInvoke = (DWORD)pResult->wrapperDelegateInvoke; } - else - ZeroMemory(&value, sizeof(Agnostic_CORINFO_CALL_INFO)); value.exceptionCode = (DWORD)exceptionCode; GetCallInfo->Add(key, value); DEBUG_REC(dmpGetCallInfo(key, value)); } + void MethodContext::dmpGetCallInfo(const Agnostic_GetCallInfo& key, const Agnostic_CORINFO_CALL_INFO& value) { printf("GetCallInfo key rt{%s} crt{%s} ch-%016llX flg-%08X" - ", value mth-%016llX, mf-%08X cf-%08X" - " sig-%s" - " vsig-%s" - " ipl{at-%08X hnd-%016llX}" - " sdi-%08X" - " excp-%08X" - " stubLookup{%s}", - SpmiDumpHelper::DumpAgnostic_CORINFO_RESOLVED_TOKEN(key.ResolvedToken).c_str(), - SpmiDumpHelper::DumpAgnostic_CORINFO_RESOLVED_TOKEN(key.ConstrainedResolvedToken).c_str(), - key.callerHandle, - key.flags, - value.hMethod, value.methodFlags, value.classFlags, - SpmiDumpHelper::DumpAgnostic_CORINFO_SIG_INFO(value.sig, GetCallInfo, SigInstHandleMap).c_str(), - SpmiDumpHelper::DumpAgnostic_CORINFO_SIG_INFO(value.verSig, GetCallInfo, SigInstHandleMap).c_str(), value.instParamLookup.accessType, - value.instParamLookup.handle, value.wrapperDelegateInvoke, value.exceptionCode, - SpmiDumpHelper::DumpAgnostic_CORINFO_LOOKUP(value.stubLookup).c_str()); + ", value mth-%016llX, mf-%08X (%s) cf-%08X (%s)" + " sig-%s" + " vmf-%08X (%s)" + " vsig-%s" + " aa-%u" + " cch{hn-%u na-%u (TODO: dump callsiteCalloutHelper.args)}" + " tt-%u" + " k-%u (%s)" + " nic-%u (%s)" + " ch-%016llX" + " ecnrl-%u (%s)" + " stubLookup{%s}" + " ipl{at-%08X (%s) hnd-%016llX}" + " wdi-%u (%s)" + " excp-%08X", + // input + SpmiDumpHelper::DumpAgnostic_CORINFO_RESOLVED_TOKEN(key.ResolvedToken).c_str(), + SpmiDumpHelper::DumpAgnostic_CORINFO_RESOLVED_TOKEN(key.ConstrainedResolvedToken).c_str(), + key.callerHandle, + key.flags, + // output + value.hMethod, + value.methodFlags, + SpmiDumpHelper::DumpCorInfoFlag((CorInfoFlag)value.methodFlags).c_str(), + value.classFlags, + SpmiDumpHelper::DumpCorInfoFlag((CorInfoFlag)value.classFlags).c_str(), + SpmiDumpHelper::DumpAgnostic_CORINFO_SIG_INFO(value.sig, GetCallInfo, SigInstHandleMap).c_str(), + value.verMethodFlags, + SpmiDumpHelper::DumpCorInfoFlag((CorInfoFlag)value.verMethodFlags).c_str(), + SpmiDumpHelper::DumpAgnostic_CORINFO_SIG_INFO(value.verSig, GetCallInfo, SigInstHandleMap).c_str(), + value.accessAllowed, + value.callsiteCalloutHelper.helperNum, + value.callsiteCalloutHelper.numArgs, + value.thisTransform, + value.kind, + toString((CORINFO_CALL_KIND)value.kind), + value.nullInstanceCheck, + (bool)value.nullInstanceCheck ? "true" : "false", + value.contextHandle, + value.exactContextNeedsRuntimeLookup, + (bool)value.exactContextNeedsRuntimeLookup ? "true" : "false", + SpmiDumpHelper::DumpAgnostic_CORINFO_LOOKUP(value.stubLookup).c_str(), + value.instParamLookup.accessType, + toString((InfoAccessType)value.instParamLookup.accessType), + value.instParamLookup.handle, + value.wrapperDelegateInvoke, + (bool)value.wrapperDelegateInvoke ? "true" : "false", + value.exceptionCode); } void MethodContext::repGetCallInfo(CORINFO_RESOLVED_TOKEN* pResolvedToken, CORINFO_RESOLVED_TOKEN* pConstrainedResolvedToken, @@ -1418,9 +1498,9 @@ void MethodContext::repGetCallInfo(CORINFO_RESOLVED_TOKEN* pResolvedToken, } pResult->thisTransform = (CORINFO_THIS_TRANSFORM)value.thisTransform; pResult->kind = (CORINFO_CALL_KIND)value.kind; - pResult->nullInstanceCheck = (BOOL)value.nullInstanceCheck; + pResult->nullInstanceCheck = (bool)value.nullInstanceCheck; pResult->contextHandle = (CORINFO_CONTEXT_HANDLE)value.contextHandle; - pResult->exactContextNeedsRuntimeLookup = (BOOL)value.exactContextNeedsRuntimeLookup; + pResult->exactContextNeedsRuntimeLookup = (bool)value.exactContextNeedsRuntimeLookup; pResult->stubLookup.lookupKind.needsRuntimeLookup = value.stubLookup.lookupKind.needsRuntimeLookup != 0; pResult->stubLookup.lookupKind.runtimeLookupKind = (CORINFO_RUNTIME_LOOKUP_KIND)value.stubLookup.lookupKind.runtimeLookupKind; @@ -1440,7 +1520,7 @@ void MethodContext::repGetCallInfo(CORINFO_RESOLVED_TOKEN* pResolvedToken, } pResult->instParamLookup.accessType = (InfoAccessType)value.instParamLookup.accessType; pResult->instParamLookup.handle = (CORINFO_GENERIC_HANDLE)value.instParamLookup.handle; - pResult->wrapperDelegateInvoke = (BOOL)value.wrapperDelegateInvoke; + pResult->wrapperDelegateInvoke = (bool)value.wrapperDelegateInvoke; *exceptionCode = (DWORD)value.exceptionCode; DEBUG_REP(dmpGetCallInfo(key, value)); @@ -1582,7 +1662,9 @@ void MethodContext::dmpAsCorInfoType(DWORDLONG key, DWORD value) } CorInfoType MethodContext::repAsCorInfoType(CORINFO_CLASS_HANDLE cls) { - AssertCodeMsg((AsCorInfoType != nullptr) && (AsCorInfoType->GetIndex(CastHandle(cls)) != -1), EXCEPTIONCODE_MC, + AssertCodeMsg(AsCorInfoType != nullptr, EXCEPTIONCODE_MC, + "Didn't find %016llX. Probable cached value in JIT issue", CastHandle(cls)); + AssertCodeMsg(AsCorInfoType->GetIndex(CastHandle(cls)) != -1, EXCEPTIONCODE_MC, "Didn't find %016llX. Probable cached value in JIT issue", CastHandle(cls)); CorInfoType result = (CorInfoType)AsCorInfoType->Get(CastHandle(cls)); DEBUG_REP(dmpAsCorInfoType(CastHandle(cls), (DWORD)result)); @@ -2440,8 +2522,18 @@ void MethodContext::recGetArgType(CORINFO_SIG_INFO* sig, // Only setting values for CORINFO_SIG_INFO things the EE seems to pay attention to... this is necessary since some of the values // are unset and fail our precise comparisons ... - key.flags = (DWORD)sig->flags; - key.numArgs = (DWORD)sig->numArgs; + // TODO: verify that the above comment is still true (that some of the fields of incoming argument `sig` contain garbage), or + // can we store the whole thing and use StoreAgnostic_CORINFO_SIG_INFO()? + + key.flags = (DWORD)sig->flags; + key.numArgs = (DWORD)sig->numArgs; + + if ((sig->cbSig == 0) && (sig->pSig != nullptr)) + { + // In this case, assume it is crossgen2 and pSig is actually a "handle" (some kind of pointer + // to an object), not a pointer to an array of signature bytes. Store the handle itself. + key.sigHandle = CastHandle((void*)sig->pSig); + } SpmiRecordsHelper::StoreAgnostic_CORINFO_SIG_INST_HandleArray(sig->sigInst.classInstCount, sig->sigInst.classInst, SigInstHandleMap, &key.sigInst_classInstCount, &key.sigInst_classInst_Index); SpmiRecordsHelper::StoreAgnostic_CORINFO_SIG_INST_HandleArray(sig->sigInst.methInstCount, sig->sigInst.methInst, SigInstHandleMap, &key.sigInst_methInstCount, &key.sigInst_methInst_Index); @@ -2459,26 +2551,36 @@ void MethodContext::recGetArgType(CORINFO_SIG_INFO* sig, } void MethodContext::dmpGetArgType(const Agnostic_GetArgType_Key& key, const Agnostic_GetArgType_Value& value) { - printf("GetArgType key flg-%08X na-%u %s %s scp-%016llX arg-%016llX", key.flags, key.numArgs, + printf("GetArgType key flg-%08X na-%u sigHnd-%016llX %s %s scp-%016llX arg-%016llX", + key.flags, key.numArgs, + key.sigHandle, SpmiDumpHelper::DumpAgnostic_CORINFO_SIG_INST_Element("", "cc", "ci", key.sigInst_classInstCount, key.sigInst_classInst_Index, SigInstHandleMap).c_str(), SpmiDumpHelper::DumpAgnostic_CORINFO_SIG_INST_Element("", "mc", "mi", key.sigInst_methInstCount, key.sigInst_methInst_Index, SigInstHandleMap).c_str(), key.scope, key.args); - printf(", value rt-%016llX cit-%u excp-%08X", value.vcTypeRet, value.result, value.exceptionCode); + printf(", value result(cit)-%u(%s) vcType-%016llX excp-%08X", value.result, toString((CorInfoTypeWithMod)value.result), value.vcTypeRet, value.exceptionCode); } CorInfoTypeWithMod MethodContext::repGetArgType(CORINFO_SIG_INFO* sig, CORINFO_ARG_LIST_HANDLE args, CORINFO_CLASS_HANDLE* vcTypeRet, DWORD* exceptionCode) { + AssertCodeMsg(GetArgType != nullptr, EXCEPTIONCODE_MC, + "Didn't find %016llx, %016llx. probably a missing exception in getArgType", CastHandle(sig->scope), CastHandle(args)); + Agnostic_GetArgType_Key key; ZeroMemory(&key, sizeof(Agnostic_GetArgType_Key)); // We use the input structs as a key and use memcmp to compare.. so - // we need to zero out padding too - - AssertCodeMsg(GetArgType != nullptr, EXCEPTIONCODE_MC, - "Didn't find %016llx, %016llx. probably a missing exception in getArgType", key.scope, key.args); + // we need to zero out padding too key.flags = (DWORD)sig->flags; key.numArgs = (DWORD)sig->numArgs; + + if ((sig->cbSig == 0) && (sig->pSig != nullptr)) + { + // In this case, assume it is crossgen2 and pSig is actually a "handle" (some kind of pointer + // to an object), not a pointer to an array of signature bytes. Store the handle itself. + key.sigHandle = CastHandle((void*)sig->pSig); + } + key.sigInst_classInstCount = (DWORD)sig->sigInst.classInstCount; key.sigInst_methInstCount = (DWORD)sig->sigInst.methInstCount; key.scope = CastHandle(sig->scope); @@ -2576,6 +2678,13 @@ void MethodContext::recGetArgClass(CORINFO_SIG_INFO* sig, key.scope = CastHandle(sig->scope); key.args = CastHandle(args); + if ((sig->cbSig == 0) && (sig->pSig != nullptr)) + { + // In this case, assume it is crossgen2 and pSig is actually a "handle" (some kind of pointer + // to an object), not a pointer to an array of signature bytes. Store the handle itself. + key.sigHandle = CastHandle((void*)sig->pSig); + } + Agnostic_GetArgClass_Value value; value.result = CastHandle(result); value.exceptionCode = exceptionCode; @@ -2585,27 +2694,35 @@ void MethodContext::recGetArgClass(CORINFO_SIG_INFO* sig, } void MethodContext::dmpGetArgClass(const Agnostic_GetArgClass_Key& key, const Agnostic_GetArgClass_Value& value) { - printf("GetArgClass key %s %s scp-%016llX args-%016llX", + printf("GetArgClass key %s %s scp-%016llX args-%016llX sigHnd-%016llX", SpmiDumpHelper::DumpAgnostic_CORINFO_SIG_INST_Element("", "cc", "ci", key.sigInst_classInstCount, key.sigInst_classInst_Index, SigInstHandleMap).c_str(), SpmiDumpHelper::DumpAgnostic_CORINFO_SIG_INST_Element("", "mc", "mi", key.sigInst_methInstCount, key.sigInst_methInst_Index, SigInstHandleMap).c_str(), - key.scope, key.args); + key.scope, key.args, key.sigHandle); printf(", value %016llX excp-%08X", value.result, value.exceptionCode); } CORINFO_CLASS_HANDLE MethodContext::repGetArgClass(CORINFO_SIG_INFO* sig, CORINFO_ARG_LIST_HANDLE args, DWORD* exceptionCode) { + AssertCodeMsg(GetArgClass != nullptr, EXCEPTIONCODE_MC, + "Didn't find %016llx, %016llx. probably a missing exception in getArgClass", CastHandle(sig->scope), CastHandle(args)); + Agnostic_GetArgClass_Key key; ZeroMemory(&key, sizeof(Agnostic_GetArgClass_Key)); // We use the input structs as a key and use memcmp to compare.. so // we need to zero out padding too - AssertCodeMsg(GetArgClass != nullptr, EXCEPTIONCODE_MC, - "Didn't find %016llx, %016llx. probably a missing exception in getArgClass", key.scope, key.args); key.sigInst_classInstCount = (DWORD)sig->sigInst.classInstCount; key.sigInst_methInstCount = (DWORD)sig->sigInst.methInstCount; key.scope = CastHandle(sig->scope); key.args = CastHandle(args); + if ((sig->cbSig == 0) && (sig->pSig != nullptr)) + { + // In this case, assume it is crossgen2 and pSig is actually a "handle" (some kind of pointer + // to an object), not a pointer to an array of signature bytes. Store the handle itself. + key.sigHandle = CastHandle((void*)sig->pSig); + } + key.sigInst_classInst_Index = SpmiRecordsHelper::ContainsHandleMap(sig->sigInst.classInstCount, sig->sigInst.classInst, SigInstHandleMap); key.sigInst_methInst_Index = SpmiRecordsHelper::ContainsHandleMap(sig->sigInst.methInstCount, sig->sigInst.methInst, SigInstHandleMap); @@ -3631,8 +3748,10 @@ void MethodContext::recPInvokeMarshalingRequired(CORINFO_METHOD_HANDLE method, } void MethodContext::dmpPInvokeMarshalingRequired(const MethodOrSigInfoValue& key, DWORD value) { - printf("PInvokeMarshalingRequired key mth-%016llX scp-%016llX sig-%u, value res-%u", key.method, key.scope, - key.pSig_Index, value); + printf("PInvokeMarshalingRequired key mth-%016llX scp-%016llX sig-%s, value res-%u", + key.method, key.scope, + SpmiDumpHelper::DumpPSig(key.pSig_Index, key.cbSig, PInvokeMarshalingRequired).c_str(), + value); } // Note the jit interface implementation seems to only care about scope and pSig from callSiteSig bool MethodContext::repPInvokeMarshalingRequired(CORINFO_METHOD_HANDLE method, CORINFO_SIG_INFO* callSiteSig) @@ -3689,8 +3808,10 @@ void MethodContext::recGetUnmanagedCallConv(CORINFO_METHOD_HANDLE method, } void MethodContext::dmpGetUnmanagedCallConv(const MethodOrSigInfoValue& key, DD value) { - printf("GetUnmanagedCallConv key mth-%016llX scp-%016llX sig-%u, value res-%u,%u", key.method, key.scope, - key.pSig_Index, value.A, value.B); + printf("GetUnmanagedCallConv key mth-%016llX scp-%016llX sig-%s, value res-%u,%u", + key.method, key.scope, + SpmiDumpHelper::DumpPSig(key.pSig_Index, key.cbSig, GetUnmanagedCallConv).c_str(), + value.A, value.B); } // Note the jit interface implementation seems to only care about scope and pSig from callSiteSig CorInfoCallConvExtension MethodContext::repGetUnmanagedCallConv(CORINFO_METHOD_HANDLE method, CORINFO_SIG_INFO* callSiteSig, bool* pSuppressGCTransition) @@ -4998,8 +5119,9 @@ void MethodContext::recGetVarArgsHandle(CORINFO_SIG_INFO* pSig, void** ppIndirec } void MethodContext::dmpGetVarArgsHandle(const GetVarArgsHandleValue& key, DLDL value) { - printf("GetVarArgsHandle key cbSig-%08X pSig_Index-%08X scope-%016llX token-%08X", key.cbSig, key.pSig_Index, - key.scope, key.token); + printf("GetVarArgsHandle key sig-%s scope-%016llX token-%08X", + SpmiDumpHelper::DumpPSig(key.pSig_Index, key.cbSig, GetVarArgsHandle).c_str(), + key.scope, key.token); printf(", value ppIndirection-%016llX result-%016llX", value.A, value.B); } CORINFO_VARARGS_HANDLE MethodContext::repGetVarArgsHandle(CORINFO_SIG_INFO* pSig, void** ppIndirection) diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shared/spmidumphelper.h b/src/coreclr/ToolBox/superpmi/superpmi-shared/spmidumphelper.h index dea7acf1145cc4..8908a8b74a64b8 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shared/spmidumphelper.h +++ b/src/coreclr/ToolBox/superpmi/superpmi-shared/spmidumphelper.h @@ -27,6 +27,12 @@ class SpmiDumpHelper const Agnostic_CORINFO_RUNTIME_LOOKUP& lookup); static std::string DumpAgnostic_CORINFO_LOOKUP(const Agnostic_CORINFO_LOOKUP& lookup); + template + static std::string DumpPSig( + DWORD pSig_Index, + DWORD cbSig, + LightWeightMap* buffers); + template static std::string DumpAgnostic_CORINFO_SIG_INFO( const Agnostic_CORINFO_SIG_INFO& sigInfo, @@ -61,26 +67,18 @@ class SpmiDumpHelper }; template -inline std::string SpmiDumpHelper::DumpAgnostic_CORINFO_SIG_INFO( - const Agnostic_CORINFO_SIG_INFO& sigInfo, - LightWeightMap* buffers, - const DenseLightWeightMap* handleMap) +inline std::string SpmiDumpHelper::DumpPSig( + DWORD pSig_Index, + DWORD cbSig, + LightWeightMap* buffers) { char buffer[MAX_BUFFER_SIZE]; char* pbuf = buffer; int sizeOfBuffer = sizeof(buffer); int cch; - cch = sprintf_s(pbuf, sizeOfBuffer, "{callConv-%08X retTypeClass-%016llX retTypeSigClass-%016llX retType-%08X(%s) flg-%08X na-%u", - sigInfo.callConv, sigInfo.retTypeClass, sigInfo.retTypeSigClass, sigInfo.retType, toString((CorInfoType)sigInfo.retType), sigInfo.flags, sigInfo.numArgs); - pbuf += cch; - sizeOfBuffer -= cch; - - FormatAgnostic_CORINFO_SIG_INST_Element(pbuf, sizeOfBuffer, " ", "cc", "ci", sigInfo.sigInst_classInstCount, sigInfo.sigInst_classInst_Index, handleMap); - FormatAgnostic_CORINFO_SIG_INST_Element(pbuf, sizeOfBuffer, " ", "mc", "mi", sigInfo.sigInst_methInstCount, sigInfo.sigInst_methInst_Index, handleMap); - - cch = sprintf_s(pbuf, sizeOfBuffer, " args-%016llX si-%08X, cbSig-%08X", - sigInfo.args, sigInfo.pSig_Index, sigInfo.cbSig); + cch = sprintf_s(pbuf, sizeOfBuffer, "cbSig-%u, si-%08X", + cbSig, pSig_Index); pbuf += cch; sizeOfBuffer -= cch; @@ -90,7 +88,7 @@ inline std::string SpmiDumpHelper::DumpAgnostic_CORINFO_SIG_INFO( // from an API of `false`). So check that cbSig > 0 before calling GetBuffer(), just to be sure // there is some data to find. - if (sigInfo.cbSig == 0) + if (cbSig == 0) { cch = sprintf_s(pbuf, sizeOfBuffer, " (SIG SIZE ZERO)"); pbuf += cch; @@ -98,7 +96,7 @@ inline std::string SpmiDumpHelper::DumpAgnostic_CORINFO_SIG_INFO( } else { - PCCOR_SIGNATURE pSig = buffers->GetBuffer(sigInfo.pSig_Index); + PCCOR_SIGNATURE pSig = buffers->GetBuffer(pSig_Index); if (pSig == nullptr) { cch = sprintf_s(pbuf, sizeOfBuffer, " (NO SIG FOUND)"); @@ -112,16 +110,24 @@ inline std::string SpmiDumpHelper::DumpAgnostic_CORINFO_SIG_INFO( sizeOfBuffer -= cch; const unsigned int maxSigDisplayBytes = 25; // Don't display more than this. - const unsigned int sigDisplayBytes = min(maxSigDisplayBytes, sigInfo.cbSig); + const unsigned int sigDisplayBytes = min(maxSigDisplayBytes, cbSig); + + // TODO: display character representation of the types? for (DWORD i = 0; i < sigDisplayBytes; i++) { + if (i > 0) + { + cch = sprintf_s(pbuf, sizeOfBuffer, " "); + pbuf += cch; + sizeOfBuffer -= cch; + } cch = sprintf_s(pbuf, sizeOfBuffer, "%02X", pSig[i]); pbuf += cch; sizeOfBuffer -= cch; } - if (sigDisplayBytes < sigInfo.cbSig) + if (sigDisplayBytes < cbSig) { cch = sprintf_s(pbuf, sizeOfBuffer, "..."); pbuf += cch; @@ -134,8 +140,32 @@ inline std::string SpmiDumpHelper::DumpAgnostic_CORINFO_SIG_INFO( } } - cch = sprintf_s(pbuf, sizeOfBuffer, " scp-%016llX tok-%08X}", - sigInfo.scope, sigInfo.token); + return std::string(buffer); +} + +template +inline std::string SpmiDumpHelper::DumpAgnostic_CORINFO_SIG_INFO( + const Agnostic_CORINFO_SIG_INFO& sigInfo, + LightWeightMap* buffers, + const DenseLightWeightMap* handleMap) +{ + char buffer[MAX_BUFFER_SIZE]; + char* pbuf = buffer; + int sizeOfBuffer = sizeof(buffer); + int cch; + + cch = sprintf_s(pbuf, sizeOfBuffer, "{callConv-%08X retTypeClass-%016llX retTypeSigClass-%016llX retType-%08X(%s) flg-%08X na-%u", + sigInfo.callConv, sigInfo.retTypeClass, sigInfo.retTypeSigClass, sigInfo.retType, toString((CorInfoType)sigInfo.retType), sigInfo.flags, sigInfo.numArgs); + pbuf += cch; + sizeOfBuffer -= cch; + + FormatAgnostic_CORINFO_SIG_INST_Element(pbuf, sizeOfBuffer, " ", "cc", "ci", sigInfo.sigInst_classInstCount, sigInfo.sigInst_classInst_Index, handleMap); + FormatAgnostic_CORINFO_SIG_INST_Element(pbuf, sizeOfBuffer, " ", "mc", "mi", sigInfo.sigInst_methInstCount, sigInfo.sigInst_methInst_Index, handleMap); + + cch = sprintf_s(pbuf, sizeOfBuffer, " args-%016llX sig-%s sigHnd-%016llX scp-%016llX tok-%08X}", + sigInfo.args, DumpPSig(sigInfo.pSig_Index, sigInfo.cbSig, buffers).c_str(), sigInfo.sigHandle, sigInfo.scope, sigInfo.token); + pbuf += cch; + sizeOfBuffer -= cch; return std::string(buffer); } diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shared/spmirecordhelper.h b/src/coreclr/ToolBox/superpmi/superpmi-shared/spmirecordhelper.h index 932e192186c364..6b996f9fc87156 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shared/spmirecordhelper.h +++ b/src/coreclr/ToolBox/superpmi/superpmi-shared/spmirecordhelper.h @@ -218,6 +218,12 @@ inline Agnostic_CORINFO_SIG_INFO SpmiRecordsHelper::CreateAgnostic_CORINFO_SIG_I sig.sigInst_methInstCount = (DWORD)sigInfo.sigInst.methInstCount; sig.args = CastHandle(sigInfo.args); sig.cbSig = (DWORD)sigInfo.cbSig; + if ((sigInfo.cbSig == 0) && (sigInfo.pSig != nullptr)) + { + // In this case, assume it is crossgen2 and pSig is actually a "handle" (some kind of pointer + // to an object), not a pointer to an array of signature bytes. Store the handle itself. + sig.sigHandle = CastHandle((void*)sigInfo.pSig); + } sig.scope = CastHandle(sigInfo.scope); sig.token = (DWORD)sigInfo.token; return sig; @@ -385,8 +391,16 @@ inline CORINFO_SIG_INFO SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(const Agnost sig.flags = (unsigned)sigInfo.flags; sig.numArgs = (unsigned)sigInfo.numArgs; sig.args = (CORINFO_ARG_LIST_HANDLE)sigInfo.args; - sig.cbSig = (unsigned int)sigInfo.cbSig; - sig.pSig = (PCCOR_SIGNATURE)buffers->GetBuffer(sigInfo.pSig_Index); + if (sigInfo.sigHandle != 0) + { + sig.cbSig = 0; + sig.pSig = (PCCOR_SIGNATURE)sigInfo.sigHandle; + } + else + { + sig.cbSig = (unsigned int)sigInfo.cbSig; + sig.pSig = (PCCOR_SIGNATURE)buffers->GetBuffer(sigInfo.pSig_Index); + } sig.scope = (CORINFO_MODULE_HANDLE)sigInfo.scope; sig.token = (mdToken)sigInfo.token; diff --git a/src/coreclr/inc/jiteeversionguid.h b/src/coreclr/inc/jiteeversionguid.h index 452e508ddd8626..5383dd1d8efe24 100644 --- a/src/coreclr/inc/jiteeversionguid.h +++ b/src/coreclr/inc/jiteeversionguid.h @@ -32,11 +32,11 @@ ////////////////////////////////////////////////////////////////////////////////////////////////////////// // -constexpr GUID JITEEVersionIdentifier = { /* 960894e2-ec41-4088-82bb-bdcbac4ac2d3 */ - 0x960894e2, - 0xec41, - 0x4088, - {0x82, 0xbb, 0xbd, 0xcb, 0xac, 0x4a, 0xc2, 0xd3} +constexpr GUID JITEEVersionIdentifier = { /* a904114d-6fe1-4884-82ab-47849d22d882 */ + 0xa904114d, + 0x6fe1, + 0x4884, + {0x82, 0xab, 0x47, 0x84, 0x9d, 0x22, 0xd8, 0x82} }; ////////////////////////////////////////////////////////////////////////////////////////////////////////// From 620b5d9ae4e3706dc3ba85565607fb67cb0346dc Mon Sep 17 00:00:00 2001 From: Bruce Forstall Date: Tue, 2 Feb 2021 16:09:55 -0800 Subject: [PATCH 2/2] Fix clang build break Add logging variables, but leave them under #if 0 Fix handling of getCallInfo throwing an exception. --- .../superpmi-shared/methodcontext.cpp | 113 +++++++++--------- .../superpmi/superpmi-shared/methodcontext.h | 23 ++++ .../superpmi-shim-collector.cpp | 25 ++++ 3 files changed, 104 insertions(+), 57 deletions(-) diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp index 12d267b5e83295..7f44c9d6ae73c2 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp @@ -20,21 +20,8 @@ #define sparseMC // Support filling in details where guesses are okay and will still generate good code. (i.e. helper // function addresses) -#if 0 -// Enable these to get verbose logging during record or playback. -#define DEBUG_REC(x) \ - printf("rec"); \ - x; \ - printf("\n"); - -#define DEBUG_REP(x) \ - printf("rep"); \ - x; \ - printf("\n"); -#else -#define DEBUG_REC(x) -#define DEBUG_REP(x) -#endif +bool g_debugRec = false; +bool g_debugRep = false; // static variable initialization Hash MethodContext::m_hash; @@ -605,11 +592,13 @@ const char* toString(CorInfoType cit) const char* toString(CorInfoTypeWithMod cit) { - switch (cit) + // Need to cast `cit` to numeric type to avoid clang compiler warnings + // "case value not in enumerated type 'CorInfoTypeWithMod'". + switch ((unsigned)cit) { - case CORINFO_TYPE_BYREF | CORINFO_TYPE_MOD_PINNED: + case (unsigned)(CORINFO_TYPE_BYREF | CORINFO_TYPE_MOD_PINNED): return "pinned byref"; - case CORINFO_TYPE_CLASS | CORINFO_TYPE_MOD_PINNED: + case (unsigned)(CORINFO_TYPE_CLASS | CORINFO_TYPE_MOD_PINNED): return "pinned class"; default: return toString((CorInfoType)(cit & CORINFO_TYPE_MASK)); @@ -1393,6 +1382,7 @@ void MethodContext::recGetCallInfo(CORINFO_RESOLVED_TOKEN* pResolvedToken, value.instParamLookup.handle = CastHandle(pResult->instParamLookup.handle); value.wrapperDelegateInvoke = (DWORD)pResult->wrapperDelegateInvoke; } + value.exceptionCode = (DWORD)exceptionCode; GetCallInfo->Add(key, value); @@ -1478,50 +1468,59 @@ void MethodContext::repGetCallInfo(CORINFO_RESOLVED_TOKEN* pResolvedToken, value = GetCallInfo->Get(key); - pResult->hMethod = (CORINFO_METHOD_HANDLE)value.hMethod; - pResult->methodFlags = (unsigned)value.methodFlags; - pResult->classFlags = (unsigned)value.classFlags; - pResult->sig = SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(value.sig, GetCallInfo, SigInstHandleMap); - if (flags & CORINFO_CALLINFO_VERIFICATION) + if (value.exceptionCode != 0) { - pResult->verMethodFlags = (unsigned)value.verMethodFlags; - pResult->verSig = SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(value.verSig, GetCallInfo, SigInstHandleMap); - } - pResult->accessAllowed = (CorInfoIsAccessAllowedResult)value.accessAllowed; - pResult->callsiteCalloutHelper.helperNum = (CorInfoHelpFunc)value.callsiteCalloutHelper.helperNum; - pResult->callsiteCalloutHelper.numArgs = (unsigned)value.callsiteCalloutHelper.numArgs; - for (int i = 0; i < CORINFO_ACCESS_ALLOWED_MAX_ARGS; i++) - { - pResult->callsiteCalloutHelper.args[i].constant = (size_t)value.callsiteCalloutHelper.args[i].constant; - pResult->callsiteCalloutHelper.args[i].argType = - (CorInfoAccessAllowedHelperArgType)value.callsiteCalloutHelper.args[i].argType; - } - pResult->thisTransform = (CORINFO_THIS_TRANSFORM)value.thisTransform; - pResult->kind = (CORINFO_CALL_KIND)value.kind; - pResult->nullInstanceCheck = (bool)value.nullInstanceCheck; - pResult->contextHandle = (CORINFO_CONTEXT_HANDLE)value.contextHandle; - pResult->exactContextNeedsRuntimeLookup = (bool)value.exactContextNeedsRuntimeLookup; - pResult->stubLookup.lookupKind.needsRuntimeLookup = value.stubLookup.lookupKind.needsRuntimeLookup != 0; - pResult->stubLookup.lookupKind.runtimeLookupKind = - (CORINFO_RUNTIME_LOOKUP_KIND)value.stubLookup.lookupKind.runtimeLookupKind; - if (pResult->stubLookup.lookupKind.needsRuntimeLookup) - { - pResult->stubLookup.runtimeLookup = SpmiRecordsHelper::RestoreCORINFO_RUNTIME_LOOKUP(value.stubLookup.runtimeLookup); + // If there was an exception, the stored data is ignored. + ZeroMemory(pResult, sizeof(*pResult)); } else { - pResult->stubLookup.constLookup.accessType = (InfoAccessType)value.stubLookup.constLookup.accessType; - pResult->stubLookup.constLookup.handle = (CORINFO_GENERIC_HANDLE)value.stubLookup.constLookup.handle; - } - if (pResult->kind == CORINFO_VIRTUALCALL_STUB) - { - cr->CallTargetTypes->Add(CastPointer(pResult->codePointerLookup.constLookup.addr), - (DWORD)CORINFO_VIRTUALCALL_STUB); + pResult->hMethod = (CORINFO_METHOD_HANDLE)value.hMethod; + pResult->methodFlags = (unsigned)value.methodFlags; + pResult->classFlags = (unsigned)value.classFlags; + pResult->sig = SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(value.sig, GetCallInfo, SigInstHandleMap); + if (flags & CORINFO_CALLINFO_VERIFICATION) + { + pResult->verMethodFlags = (unsigned)value.verMethodFlags; + pResult->verSig = SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(value.verSig, GetCallInfo, SigInstHandleMap); + } + pResult->accessAllowed = (CorInfoIsAccessAllowedResult)value.accessAllowed; + pResult->callsiteCalloutHelper.helperNum = (CorInfoHelpFunc)value.callsiteCalloutHelper.helperNum; + pResult->callsiteCalloutHelper.numArgs = (unsigned)value.callsiteCalloutHelper.numArgs; + for (int i = 0; i < CORINFO_ACCESS_ALLOWED_MAX_ARGS; i++) + { + pResult->callsiteCalloutHelper.args[i].constant = (size_t)value.callsiteCalloutHelper.args[i].constant; + pResult->callsiteCalloutHelper.args[i].argType = + (CorInfoAccessAllowedHelperArgType)value.callsiteCalloutHelper.args[i].argType; + } + pResult->thisTransform = (CORINFO_THIS_TRANSFORM)value.thisTransform; + pResult->kind = (CORINFO_CALL_KIND)value.kind; + pResult->nullInstanceCheck = (bool)value.nullInstanceCheck; + pResult->contextHandle = (CORINFO_CONTEXT_HANDLE)value.contextHandle; + pResult->exactContextNeedsRuntimeLookup = (bool)value.exactContextNeedsRuntimeLookup; + pResult->stubLookup.lookupKind.needsRuntimeLookup = value.stubLookup.lookupKind.needsRuntimeLookup != 0; + pResult->stubLookup.lookupKind.runtimeLookupKind = + (CORINFO_RUNTIME_LOOKUP_KIND)value.stubLookup.lookupKind.runtimeLookupKind; + if (pResult->stubLookup.lookupKind.needsRuntimeLookup) + { + pResult->stubLookup.runtimeLookup = SpmiRecordsHelper::RestoreCORINFO_RUNTIME_LOOKUP(value.stubLookup.runtimeLookup); + } + else + { + pResult->stubLookup.constLookup.accessType = (InfoAccessType)value.stubLookup.constLookup.accessType; + pResult->stubLookup.constLookup.handle = (CORINFO_GENERIC_HANDLE)value.stubLookup.constLookup.handle; + } + if (pResult->kind == CORINFO_VIRTUALCALL_STUB) + { + cr->CallTargetTypes->Add(CastPointer(pResult->codePointerLookup.constLookup.addr), + (DWORD)CORINFO_VIRTUALCALL_STUB); + } + pResult->instParamLookup.accessType = (InfoAccessType)value.instParamLookup.accessType; + pResult->instParamLookup.handle = (CORINFO_GENERIC_HANDLE)value.instParamLookup.handle; + pResult->wrapperDelegateInvoke = (bool)value.wrapperDelegateInvoke; } - pResult->instParamLookup.accessType = (InfoAccessType)value.instParamLookup.accessType; - pResult->instParamLookup.handle = (CORINFO_GENERIC_HANDLE)value.instParamLookup.handle; - pResult->wrapperDelegateInvoke = (bool)value.wrapperDelegateInvoke; - *exceptionCode = (DWORD)value.exceptionCode; + + *exceptionCode = (DWORD)value.exceptionCode; DEBUG_REP(dmpGetCallInfo(key, value)); } diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.h b/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.h index e3796060ed3443..0bf0cfaa8ef28d 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.h +++ b/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.h @@ -17,6 +17,29 @@ #include "hash.h" #include "agnostic.h" +extern bool g_debugRec; +extern bool g_debugRep; + +#if 0 +// Enable these to get verbose logging during record or playback. +#define DEBUG_REC(x) \ + if (g_debugRec) { \ + printf("rec"); \ + x; \ + printf("\n"); \ + } + +#define DEBUG_REP(x) \ + if (g_debugRep) { \ + printf("rep"); \ + x; \ + printf("\n"); \ + } +#else +#define DEBUG_REC(x) +#define DEBUG_REP(x) +#endif + // Helper function for dumping. const char* toString(CorInfoType cit); diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shim-collector/superpmi-shim-collector.cpp b/src/coreclr/ToolBox/superpmi/superpmi-shim-collector/superpmi-shim-collector.cpp index bb39b86d6d1a0a..995da3f2009fae 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shim-collector/superpmi-shim-collector.cpp +++ b/src/coreclr/ToolBox/superpmi/superpmi-shim-collector/superpmi-shim-collector.cpp @@ -27,6 +27,8 @@ char* g_logFilePath = nullptr; // We *don't* leak this, hooray! WCHAR* g_HomeDirectory = nullptr; WCHAR* g_DefaultRealJitPath = nullptr; MethodContext* g_globalContext = nullptr; +WCHAR* g_debugRecStr = nullptr; +WCHAR* g_debugRepStr = nullptr; void SetDefaultPaths() { @@ -81,6 +83,27 @@ void SetLogFilePath() } } +void SetDebugVariables() +{ + if (g_debugRecStr == nullptr) + { + g_debugRecStr = GetEnvironmentVariableWithDefaultW(W("SuperPMIShimDebugRec"), W("0")); + } + if (g_debugRepStr == nullptr) + { + g_debugRepStr = GetEnvironmentVariableWithDefaultW(W("SuperPMIShimDebugRep"), W("0")); + } + + if (0 == wcscmp(g_debugRecStr, W("1"))) + { + g_debugRec = true; + } + if (0 == wcscmp(g_debugRepStr, W("1"))) + { + g_debugRep = true; + } +} + extern "C" #ifdef HOST_UNIX DLLEXPORT // For Win32 PAL LoadLibrary emulation @@ -123,6 +146,7 @@ extern "C" DLLEXPORT void __stdcall jitStartup(ICorJitHost* host) { SetDefaultPaths(); SetLibName(); + SetDebugVariables(); if (!LoadRealJitLib(g_hRealJit, g_realJitPath)) { @@ -153,6 +177,7 @@ extern "C" DLLEXPORT ICorJitCompiler* __stdcall getJit() SetLibName(); SetLogPath(); SetLogPathName(); + SetDebugVariables(); if (!LoadRealJitLib(g_hRealJit, g_realJitPath)) {