From 922c75c837fc95092e1d9e433a087ac62082953d Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Sun, 26 Mar 2023 15:21:26 +0200 Subject: [PATCH] SPMI: Fix recGetStaticFieldCurrentClass (#83843) getStaticFieldCurrentClass has very different behavior depending on whether pIsSpeculative is passed or not, and we need to record the resulting class handle in both cases. This fixes a case of replays not working after recording that I hit while tracking down a separate issue. --- src/coreclr/inc/jiteeversionguid.h | 10 ++++---- .../tools/superpmi/superpmi-shared/lwmlist.h | 2 +- .../superpmi-shared/methodcontext.cpp | 23 ++++++++++++------- .../superpmi/superpmi-shared/methodcontext.h | 4 ++-- .../superpmi-shim-collector/icorjitinfo.cpp | 2 +- 5 files changed, 24 insertions(+), 17 deletions(-) diff --git a/src/coreclr/inc/jiteeversionguid.h b/src/coreclr/inc/jiteeversionguid.h index 27d8a3350c1ad..627f976b8774b 100644 --- a/src/coreclr/inc/jiteeversionguid.h +++ b/src/coreclr/inc/jiteeversionguid.h @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID; #define GUID_DEFINED #endif // !GUID_DEFINED -constexpr GUID JITEEVersionIdentifier = { /* 3a8a07e7-928e-4281-ab68-cd4017c1141b */ - 0x3a8a07e7, - 0x928e, - 0x4281, - {0xab, 0x68, 0xcd, 0x40, 0x17, 0xc1, 0x14, 0x1b} +constexpr GUID JITEEVersionIdentifier = { /* 95c688c7-28cf-4b1f-922a-11bf3947e56f */ + 0x95c688c7, + 0x28cf, + 0x4b1f, + {0x92, 0x2a, 0x11, 0xbf, 0x39, 0x47, 0xe5, 0x6f} }; ////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/coreclr/tools/superpmi/superpmi-shared/lwmlist.h b/src/coreclr/tools/superpmi/superpmi-shared/lwmlist.h index e602fba183951..9c43e34dba40c 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/lwmlist.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/lwmlist.h @@ -78,7 +78,7 @@ LWM(GetDelegateCtor, Agnostic_GetDelegateCtorIn, Agnostic_GetDelegateCtorOut) LWM(GetEEInfo, DWORD, Agnostic_CORINFO_EE_INFO) LWM(GetEHinfo, DLD, Agnostic_CORINFO_EH_CLAUSE) LWM(GetReadonlyStaticFieldValue, DLDDD, DD) -LWM(GetStaticFieldCurrentClass, DWORDLONG, Agnostic_GetStaticFieldCurrentClass) +LWM(GetStaticFieldCurrentClass, DLD, Agnostic_GetStaticFieldCurrentClass) LWM(GetFieldClass, DWORDLONG, DWORDLONG) LWM(GetFieldInClass, DLD, DWORDLONG) LWM(GetFieldInfo, Agnostic_GetFieldInfo, Agnostic_CORINFO_FIELD_INFO) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp index 0384f01cf39b1..c7ac369a1dbaf 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp @@ -3710,29 +3710,36 @@ bool MethodContext::repGetReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, u } void MethodContext::recGetStaticFieldCurrentClass(CORINFO_FIELD_HANDLE field, - bool isSpeculative, + bool* pIsSpeculative, CORINFO_CLASS_HANDLE result) { if (GetStaticFieldCurrentClass == nullptr) - GetStaticFieldCurrentClass = new LightWeightMap(); + GetStaticFieldCurrentClass = new LightWeightMap(); - Agnostic_GetStaticFieldCurrentClass value; + DLD key; + ZeroMemory(&key, sizeof(key)); + key.A = CastHandle(field); + key.B = pIsSpeculative != nullptr ? 1 : 0; + Agnostic_GetStaticFieldCurrentClass value; value.classHandle = CastHandle(result); - value.isSpeculative = isSpeculative; + value.isSpeculative = pIsSpeculative != nullptr ? *pIsSpeculative : false; - DWORDLONG key = CastHandle(field); GetStaticFieldCurrentClass->Add(key, value); DEBUG_REC(dmpGetStaticFieldCurrentClass(key, value)); } -void MethodContext::dmpGetStaticFieldCurrentClass(DWORDLONG key, const Agnostic_GetStaticFieldCurrentClass& value) +void MethodContext::dmpGetStaticFieldCurrentClass(DLD key, const Agnostic_GetStaticFieldCurrentClass& value) { - printf("GetStaticFieldCurrentClass key fld-%016" PRIX64 ", value clsHnd-%016" PRIX64 " isSpeculative-%u", key, value.classHandle, + printf("GetStaticFieldCurrentClass key fld-%016" PRIX64 ", value clsHnd-%016" PRIX64 " isSpeculative-%u", key.A, value.classHandle, value.isSpeculative); } CORINFO_CLASS_HANDLE MethodContext::repGetStaticFieldCurrentClass(CORINFO_FIELD_HANDLE field, bool* pIsSpeculative) { - DWORDLONG key = CastHandle(field); + DLD key; + ZeroMemory(&key, sizeof(key)); + key.A = CastHandle(field); + key.B = pIsSpeculative != nullptr ? 1 : 0; + Agnostic_GetStaticFieldCurrentClass value = LookupByKeyOrMiss(GetStaticFieldCurrentClass, key, ": key %016" PRIX64 "", key); DEBUG_REP(dmpGetStaticFieldCurrentClass(key, value)); diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h index 920bdbe7a2e65..04acac3103ff8 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h @@ -493,8 +493,8 @@ class MethodContext void dmpGetReadonlyStaticFieldValue(DLDDD key, DD value); bool repGetReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, int valueOffset, bool ignoreMovableObjects); - void recGetStaticFieldCurrentClass(CORINFO_FIELD_HANDLE field, bool isSpeculative, CORINFO_CLASS_HANDLE result); - void dmpGetStaticFieldCurrentClass(DWORDLONG key, const Agnostic_GetStaticFieldCurrentClass& value); + void recGetStaticFieldCurrentClass(CORINFO_FIELD_HANDLE field, bool* pIsSpeculative, CORINFO_CLASS_HANDLE result); + void dmpGetStaticFieldCurrentClass(DLD key, const Agnostic_GetStaticFieldCurrentClass& value); CORINFO_CLASS_HANDLE repGetStaticFieldCurrentClass(CORINFO_FIELD_HANDLE field, bool* pIsSpeculative); void recGetClassGClayout(CORINFO_CLASS_HANDLE cls, BYTE* gcPtrs, unsigned len, unsigned result); diff --git a/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitinfo.cpp b/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitinfo.cpp index 02f70a5ed9861..7b85b45dbf695 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitinfo.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitinfo.cpp @@ -1713,7 +1713,7 @@ CORINFO_CLASS_HANDLE interceptor_ICJI::getStaticFieldCurrentClass(CORINFO_FIELD_ { mc->cr->AddCall("getStaticFieldCurrentClass"); CORINFO_CLASS_HANDLE result = original_ICorJitInfo->getStaticFieldCurrentClass(field, pIsSpeculative); - mc->recGetStaticFieldCurrentClass(field, (pIsSpeculative == nullptr) ? false : *pIsSpeculative, result); + mc->recGetStaticFieldCurrentClass(field, pIsSpeculative, result); return result; }