Mop-up changes per Jakob's PR suggestions (#70180)

trylek · web-flow · commit 86a59cd7c4df · 2022-06-07T18:25:23.000+02:00
1) Rename CORINFO_HELP_STATIC_VIRTUAL_AMBIGUOUS_RESOLUTION to CORINFO_HELP_THROW_AMBIGUOUS_RESOLUTION_EXCEPTION and put it next to the other CORINFO_HELP_THROW methods; 2) Add the new helper to CorInfoHelpFunc.cs; 3) Remove the jitinterface member doesFieldBelongToClass; 4) Update the JIT EE GUID. Thanks Tomas Fixes: #69900
diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h
@@ -625,6 +625,7 @@ enum CorInfoHelpFunc
     CORINFO_HELP_THROW_NOT_IMPLEMENTED,             // throw NotImplementedException
     CORINFO_HELP_THROW_PLATFORM_NOT_SUPPORTED,      // throw PlatformNotSupportedException
     CORINFO_HELP_THROW_TYPE_NOT_SUPPORTED,          // throw TypeNotSupportedException
+    CORINFO_HELP_THROW_AMBIGUOUS_RESOLUTION_EXCEPTION, // throw AmbiguousResolutionException for failed static virtual method resolution
 
     CORINFO_HELP_JIT_PINVOKE_BEGIN, // Transition to preemptive mode before a P/Invoke, frame is the first argument
     CORINFO_HELP_JIT_PINVOKE_END,   // Transition to cooperative mode after a P/Invoke, frame is the first argument
@@ -646,8 +647,6 @@ enum CorInfoHelpFunc
     CORINFO_HELP_VALIDATE_INDIRECT_CALL,    // CFG: Validate function pointer
     CORINFO_HELP_DISPATCH_INDIRECT_CALL,    // CFG: Validate and dispatch to pointer
 
-    CORINFO_HELP_STATIC_VIRTUAL_AMBIGUOUS_RESOLUTION, // Throw AmbiguousResolutionException for failed static virtual method resolution
-
     CORINFO_HELP_COUNT,
 };
 
diff --git a/src/coreclr/inc/corjit.h b/src/coreclr/inc/corjit.h
@@ -495,12 +495,6 @@ class ICorJitInfo : public ICorDynamicInfo
         uint32_t        sizeInBytes   /* IN: The size of the buffer. Note that this is effectively a
                                           version number for the CORJIT_FLAGS value. */
         ) = 0;
-
-    // Checks if a field belongs to a given class.
-    virtual bool doesFieldBelongToClass(
-        CORINFO_FIELD_HANDLE fldHnd, /* IN: the field that we are checking */
-        CORINFO_CLASS_HANDLE cls     /* IN: the class that we are checking */
-        ) = 0;
 };
 
 /**********************************************************************************/
diff --git a/src/coreclr/inc/icorjitinfoimpl_generated.h b/src/coreclr/inc/icorjitinfoimpl_generated.h
@@ -710,10 +710,6 @@ uint32_t getJitFlags(
           CORJIT_FLAGS* flags,
           uint32_t sizeInBytes) override;
 
-bool doesFieldBelongToClass(
-          CORINFO_FIELD_HANDLE fldHnd,
-          CORINFO_CLASS_HANDLE cls) override;
-
 /**********************************************************************************/
 // clang-format on
 /**********************************************************************************/
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 = { /* f2a217c4-2a69-4308-99ce-8292c6763776 */
-    0xf2a217c4,
-    0x2a69,
-    0x4308,
-    {0x99, 0xce, 0x82, 0x92, 0xc6, 0x76, 0x37, 0x76}
+constexpr GUID JITEEVersionIdentifier = { /* 5868685e-b877-4ef5-83f0-73d601e50013 */
+    0x5868685e,
+    0xb877,
+    0x4ef5,
+    {0x83, 0xf0, 0x73, 0xd6, 0x01, 0xe5, 0x00, 0x13}
   };
 
 //////////////////////////////////////////////////////////////////////////////////////////////////////////
diff --git a/src/coreclr/inc/jithelpers.h b/src/coreclr/inc/jithelpers.h
@@ -309,6 +309,7 @@
     JITHELPER(CORINFO_HELP_THROW_NOT_IMPLEMENTED,             JIT_ThrowNotImplementedException,       CORINFO_HELP_SIG_REG_ONLY)
     JITHELPER(CORINFO_HELP_THROW_PLATFORM_NOT_SUPPORTED,      JIT_ThrowPlatformNotSupportedException, CORINFO_HELP_SIG_REG_ONLY)
     JITHELPER(CORINFO_HELP_THROW_TYPE_NOT_SUPPORTED,          JIT_ThrowTypeNotSupportedException,     CORINFO_HELP_SIG_REG_ONLY)
+    JITHELPER(CORINFO_HELP_THROW_AMBIGUOUS_RESOLUTION_EXCEPTION, JIT_ThrowAmbiguousResolutionException, CORINFO_HELP_SIG_REG_ONLY)
 
     JITHELPER(CORINFO_HELP_JIT_PINVOKE_BEGIN,         JIT_PInvokeBegin,     CORINFO_HELP_SIG_REG_ONLY)
     JITHELPER(CORINFO_HELP_JIT_PINVOKE_END,           JIT_PInvokeEnd,       CORINFO_HELP_SIG_REG_ONLY)
@@ -343,8 +344,6 @@
     JITHELPER(CORINFO_HELP_DISPATCH_INDIRECT_CALL, NULL, CORINFO_HELP_SIG_REG_ONLY)
 #endif
 
-    JITHELPER(CORINFO_HELP_STATIC_VIRTUAL_AMBIGUOUS_RESOLUTION, JIT_StaticVirtualAmbiguousResolution, CORINFO_HELP_SIG_REG_ONLY)
-
 #undef JITHELPER
 #undef DYNAMICJITHELPER
 #undef JITHELPER
diff --git a/src/coreclr/jit/ICorJitInfo_API_names.h b/src/coreclr/jit/ICorJitInfo_API_names.h
@@ -176,6 +176,5 @@ DEF_CLR_API(recordRelocation)
 DEF_CLR_API(getRelocTypeHint)
 DEF_CLR_API(getExpectedTargetArchitecture)
 DEF_CLR_API(getJitFlags)
-DEF_CLR_API(doesFieldBelongToClass)
 
 #undef DEF_CLR_API
diff --git a/src/coreclr/jit/ICorJitInfo_API_wrapper.hpp b/src/coreclr/jit/ICorJitInfo_API_wrapper.hpp
@@ -1690,16 +1690,6 @@ uint32_t WrapICorJitInfo::getJitFlags(
     return temp;
 }
 
-bool WrapICorJitInfo::doesFieldBelongToClass(
-          CORINFO_FIELD_HANDLE fldHnd,
-          CORINFO_CLASS_HANDLE cls)
-{
-    API_ENTER(doesFieldBelongToClass);
-    bool temp = wrapHnd->doesFieldBelongToClass(fldHnd, cls);
-    API_LEAVE(doesFieldBelongToClass);
-    return temp;
-}
-
 /**********************************************************************************/
 // clang-format on
 /**********************************************************************************/
diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoBase.cs b/src/coreclr/tools/Common/JitInterface/CorInfoBase.cs
@@ -2549,25 +2549,10 @@ static uint _getJitFlags(IntPtr thisHandle, IntPtr* ppException, CORJIT_FLAGS* f
             }
         }
 
-        [UnmanagedCallersOnly]
-        static byte _doesFieldBelongToClass(IntPtr thisHandle, IntPtr* ppException, CORINFO_FIELD_STRUCT_* fldHnd, CORINFO_CLASS_STRUCT_* cls)
-        {
-            var _this = GetThis(thisHandle);
-            try
-            {
-                return _this.doesFieldBelongToClass(fldHnd, cls) ? (byte)1 : (byte)0;
-            }
-            catch (Exception ex)
-            {
-                *ppException = _this.AllocException(ex);
-                return default;
-            }
-        }
-
 
         static IntPtr GetUnmanagedCallbacks()
         {
-            void** callbacks = (void**)Marshal.AllocCoTaskMem(sizeof(IntPtr) * 173);
+            void** callbacks = (void**)Marshal.AllocCoTaskMem(sizeof(IntPtr) * 172);
 
             callbacks[0] = (delegate* unmanaged<IntPtr, IntPtr*, CORINFO_METHOD_STRUCT_*, byte>)&_isIntrinsic;
             callbacks[1] = (delegate* unmanaged<IntPtr, IntPtr*, CORINFO_METHOD_STRUCT_*, uint>)&_getMethodAttribs;
@@ -2741,7 +2726,6 @@ static IntPtr GetUnmanagedCallbacks()
             callbacks[169] = (delegate* unmanaged<IntPtr, IntPtr*, void*, ushort>)&_getRelocTypeHint;
             callbacks[170] = (delegate* unmanaged<IntPtr, IntPtr*, uint>)&_getExpectedTargetArchitecture;
             callbacks[171] = (delegate* unmanaged<IntPtr, IntPtr*, CORJIT_FLAGS*, uint, uint>)&_getJitFlags;
-            callbacks[172] = (delegate* unmanaged<IntPtr, IntPtr*, CORINFO_FIELD_STRUCT_*, CORINFO_CLASS_STRUCT_*, byte>)&_doesFieldBelongToClass;
 
             return (IntPtr)callbacks;
         }
diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs b/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs
@@ -271,6 +271,7 @@ which is the right helper to use to allocate an object of a given type. */
         CORINFO_HELP_THROW_NOT_IMPLEMENTED,             // throw NotImplementedException
         CORINFO_HELP_THROW_PLATFORM_NOT_SUPPORTED,      // throw PlatformNotSupportedException
         CORINFO_HELP_THROW_TYPE_NOT_SUPPORTED,          // throw TypeNotSupportedException
+        CORINFO_HELP_THROW_AMBIGUOUS_RESOLUTION_EXCEPTION, // throw AmbiguousResolutionException for failed static virtual method resolution
 
         CORINFO_HELP_JIT_PINVOKE_BEGIN, // Transition to preemptive mode before a P/Invoke, frame is the first argument
         CORINFO_HELP_JIT_PINVOKE_END,   // Transition to cooperative mode after a P/Invoke, frame is the first argument
diff --git a/src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt b/src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt
@@ -325,4 +325,3 @@ FUNCTIONS
     uint16_t getRelocTypeHint(void* target)
     uint32_t getExpectedTargetArchitecture()
     uint32_t getJitFlags(CORJIT_FLAGS* flags, uint32_t  sizeInBytes)
-    bool doesFieldBelongToClass(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE cls)
diff --git a/src/coreclr/tools/aot/jitinterface/jitinterface.h b/src/coreclr/tools/aot/jitinterface/jitinterface.h
@@ -183,7 +183,6 @@ struct JitInterfaceCallbacks
     uint16_t (* getRelocTypeHint)(void * thisHandle, CorInfoExceptionClass** ppException, void* target);
     uint32_t (* getExpectedTargetArchitecture)(void * thisHandle, CorInfoExceptionClass** ppException);
     uint32_t (* getJitFlags)(void * thisHandle, CorInfoExceptionClass** ppException, CORJIT_FLAGS* flags, uint32_t sizeInBytes);
-    bool (* doesFieldBelongToClass)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE cls);
 
 };
 
@@ -1858,14 +1857,4 @@ class JitInterfaceWrapper : public ICorJitInfo
     if (pException != nullptr) throw pException;
     return temp;
 }
-
-    virtual bool doesFieldBelongToClass(
-          CORINFO_FIELD_HANDLE fldHnd,
-          CORINFO_CLASS_HANDLE cls)
-{
-    CorInfoExceptionClass* pException = nullptr;
-    bool temp = _callbacks->doesFieldBelongToClass(_thisHandle, &pException, fldHnd, cls);
-    if (pException != nullptr) throw pException;
-    return temp;
-}
 };
diff --git a/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitinfo.cpp b/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitinfo.cpp
@@ -2034,13 +2034,3 @@ bool interceptor_ICJI::notifyInstructionSetUsage(CORINFO_InstructionSet instruct
 {
     return original_ICorJitInfo->notifyInstructionSetUsage(instructionSet, supported);
 }
-
-bool interceptor_ICJI::doesFieldBelongToClass(
-    CORINFO_FIELD_HANDLE fldHnd,
-    CORINFO_CLASS_HANDLE cls)
-{
-    mc->cr->AddCall("doesFieldBelongToClass");
-    bool result = original_ICorJitInfo->doesFieldBelongToClass(fldHnd, cls);
-    mc->recDoesFieldBelongToClass(fldHnd, cls, result);
-    return result;
-}
diff --git a/src/coreclr/tools/superpmi/superpmi-shim-counter/icorjitinfo.cpp b/src/coreclr/tools/superpmi/superpmi-shim-counter/icorjitinfo.cpp
@@ -1389,11 +1389,3 @@ uint32_t interceptor_ICJI::getJitFlags(
     return original_ICorJitInfo->getJitFlags(flags, sizeInBytes);
 }
 
-bool interceptor_ICJI::doesFieldBelongToClass(
-          CORINFO_FIELD_HANDLE fldHnd,
-          CORINFO_CLASS_HANDLE cls)
-{
-    mcs->AddCall("doesFieldBelongToClass");
-    return original_ICorJitInfo->doesFieldBelongToClass(fldHnd, cls);
-}
-
diff --git a/src/coreclr/tools/superpmi/superpmi-shim-simple/icorjitinfo.cpp b/src/coreclr/tools/superpmi/superpmi-shim-simple/icorjitinfo.cpp
@@ -1217,10 +1217,3 @@ uint32_t interceptor_ICJI::getJitFlags(
     return original_ICorJitInfo->getJitFlags(flags, sizeInBytes);
 }
 
-bool interceptor_ICJI::doesFieldBelongToClass(
-          CORINFO_FIELD_HANDLE fldHnd,
-          CORINFO_CLASS_HANDLE cls)
-{
-    return original_ICorJitInfo->doesFieldBelongToClass(fldHnd, cls);
-}
-
diff --git a/src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp b/src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp
@@ -1576,13 +1576,6 @@ uint32_t MyICJI::getJitFlags(CORJIT_FLAGS* jitFlags, uint32_t sizeInBytes)
     return ret;
 }
 
-bool MyICJI::doesFieldBelongToClass(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE cls)
-{
-    jitInstance->mc->cr->AddCall("doesFieldBelongToClass");
-    bool result = jitInstance->mc->repDoesFieldBelongToClass(fldHnd, cls);
-    return result;
-}
-
 // Runs the given function with the given parameter under an error trap
 // and returns true if the function completes successfully. We fake this
 // up a bit for SuperPMI and simply catch all exceptions.
diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp
@@ -3375,34 +3375,6 @@ NOINLINE HCIMPL3(CORINFO_MethodPtr, JIT_VirtualFunctionPointer_Framed, Object *
 }
 HCIMPLEND
 
-HCIMPL3(void, JIT_StaticVirtualAmbiguousResolution, 
-    MethodDesc *method,
-    MethodTable *interfaceType,
-    MethodTable *targetType)
-{
-    FCALL_CONTRACT;
-
-    SString strMethodName;
-    SString strInterfaceName;
-    SString strTargetClassName;
-
-    HELPER_METHOD_FRAME_BEGIN_0();    // Set up a frame
-
-    TypeString::AppendMethod(strMethodName, method, method->GetMethodInstantiation());
-    TypeString::AppendType(strInterfaceName, TypeHandle(interfaceType));
-    TypeString::AppendType(strTargetClassName, targetType);
-
-    HELPER_METHOD_FRAME_END();    // Set up a frame
-
-    FCThrowExVoid(
-        kAmbiguousImplementationException,
-        IDS_CLASSLOAD_AMBIGUOUS_OVERRIDE,
-        strMethodName,
-        strInterfaceName,
-        strTargetClassName);
-}
-HCIMPLEND
-
 HCIMPL1(Object*, JIT_GetRuntimeFieldStub, CORINFO_FIELD_HANDLE field)
 {
     FCALL_CONTRACT;
@@ -4181,6 +4153,35 @@ HCIMPL0(void, JIT_ThrowTypeNotSupportedException)
 }
 HCIMPLEND
 
+/*********************************************************************/
+HCIMPL3(void, JIT_ThrowAmbiguousResolutionException,
+    MethodDesc *method,
+    MethodTable *interfaceType,
+    MethodTable *targetType)
+{
+    FCALL_CONTRACT;
+
+    SString strMethodName;
+    SString strInterfaceName;
+    SString strTargetClassName;
+
+    HELPER_METHOD_FRAME_BEGIN_0();    // Set up a frame
+
+    TypeString::AppendMethod(strMethodName, method, method->GetMethodInstantiation());
+    TypeString::AppendType(strInterfaceName, TypeHandle(interfaceType));
+    TypeString::AppendType(strTargetClassName, targetType);
+
+    HELPER_METHOD_FRAME_END();    // Set up a frame
+
+    FCThrowExVoid(
+        kAmbiguousImplementationException,
+        IDS_CLASSLOAD_AMBIGUOUS_OVERRIDE,
+        strMethodName,
+        strInterfaceName,
+        strTargetClassName);
+}
+HCIMPLEND
+
 /*********************************************************************/
 HCIMPL0(void, JIT_Overflow)
 {
diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp
@@ -5433,7 +5433,7 @@ void CEEInfo::getCallInfo(
             // shared generics is covered by the ConstrainedMethodEntrySlot dictionary entry.
             pResult->kind = CORINFO_CALL;
             pResult->accessAllowed = CORINFO_ACCESS_ILLEGAL;
-            pResult->callsiteCalloutHelper.helperNum = CORINFO_HELP_STATIC_VIRTUAL_AMBIGUOUS_RESOLUTION;
+            pResult->callsiteCalloutHelper.helperNum = CORINFO_HELP_THROW_AMBIGUOUS_RESOLUTION_EXCEPTION;
             pResult->callsiteCalloutHelper.numArgs = 3;
             pResult->callsiteCalloutHelper.args[0].methodHandle = (CORINFO_METHOD_HANDLE)pMD;
             pResult->callsiteCalloutHelper.args[0].argType = CORINFO_HELPER_ARG_TYPE_Method;
@@ -11617,50 +11617,6 @@ uint32_t CEEJitInfo::getExpectedTargetArchitecture()
     return IMAGE_FILE_MACHINE_NATIVE;
 }
 
-bool CEEJitInfo::doesFieldBelongToClass(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE cls)
-{
-    CONTRACTL {
-        THROWS;
-        GC_TRIGGERS;
-        MODE_PREEMPTIVE;
-    } CONTRACTL_END;
-
-    bool result = false;
-
-    JIT_TO_EE_TRANSITION();
-
-    FieldDesc* field = (FieldDesc*) fldHnd;
-    TypeHandle th(cls);
-
-    _ASSERTE(!field->IsStatic());
-
-    // doesFieldBelongToClass implements the predicate of...
-    // if field is not associated with the class in any way, return false.
-    // if field is the only FieldDesc that the JIT might see for a given class handle
-    // and logical field pair then return true. This is needed as the field handle here
-    // is used as a key into a hashtable mapping writes to fields to value numbers.
-    //
-    // In the CoreCLR VM implementation, verifying that the canonical MethodTable of
-    // the field matches the type found via GetExactDeclaringType, as all instance fields
-    // are only held on the canonical MethodTable.
-    // This yields a truth table such as
-
-    // BaseType._field, BaseType -> true
-    // BaseType._field, DerivedType -> true
-    // BaseType<__Canon>._field, BaseType<__Canon> -> true
-    // BaseType<__Canon>._field, BaseType<string> -> true
-    // BaseType<__Canon>._field, BaseType<object> -> true
-    // BaseType<sbyte>._field, BaseType<sbyte> -> true
-    // BaseType<sbyte>._field, BaseType<byte> -> false
-
-    MethodTable* pMT = field->GetExactDeclaringType(th.GetMethodTable());
-    result = (pMT != nullptr) && (pMT->GetCanonicalMethodTable() == field->GetApproxEnclosingMethodTable()->GetCanonicalMethodTable());
-
-    EE_TO_JIT_TRANSITION();
-
-    return result;
-}
-
 void CEEInfo::JitProcessShutdownWork()
 {
     LIMITED_METHOD_CONTRACT;
@@ -14283,12 +14239,6 @@ uint32_t CEEInfo::getExpectedTargetArchitecture()
     return IMAGE_FILE_MACHINE_NATIVE;
 }
 
-bool CEEInfo::doesFieldBelongToClass(CORINFO_FIELD_HANDLE fld, CORINFO_CLASS_HANDLE cls)
-{
-    LIMITED_METHOD_CONTRACT;
-    UNREACHABLE_RET();      // only called on derived class.
-}
-
 void CEEInfo::setBoundaries(CORINFO_METHOD_HANDLE ftn, ULONG32 cMap,
                                ICorDebugInfo::OffsetMapping *pMap)
 {
diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h
@@ -670,8 +670,6 @@ class CEEJitInfo : public CEEInfo
 
     uint32_t getExpectedTargetArchitecture() override final;
 
-    bool doesFieldBelongToClass(CORINFO_FIELD_HANDLE fld, CORINFO_CLASS_HANDLE cls) override final;
-
     void ResetForJitRetry()
     {
         CONTRACTL {
