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.
dotnet · Mar 26, 2023 · 922c75c · 922c75c
1 parent f1f9fde
commit 922c75c
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 17 deletions.
diff --git 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
@@ -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
@@ -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<DWORDLONG, Agnostic_GetStaticFieldCurrentClass>();
+        GetStaticFieldCurrentClass = new LightWeightMap<DLD, Agnostic_GetStaticFieldCurrentClass>();
 
-    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
@@ -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
@@ -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;
 }