Convert HELPER_METHOD_FRAME in objectnative.cpp to QCall (#97641)

* Equals

* AllocateUninitializedClone

* Compare object content in native code

* Do not compare padding in Equals

* Add test coverage for Equals

Author: huoyaoyuan

src/coreclr/System.Private.CoreLib/src/System/Object.CoreCLR.cs

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics;
 using System.Runtime.CompilerServices;
 
 namespace System
@@ -19,7 +20,9 @@ public partial class Object
         [Intrinsic]
         protected internal unsafe object MemberwiseClone()
         {
-            object clone = RuntimeHelpers.AllocateUninitializedClone(this);
+            object clone = this;
+            RuntimeHelpers.AllocateUninitializedClone(ObjectHandleOnStack.Create(ref clone));
+            Debug.Assert(clone != this);
 
             // copy contents of "this" to the clone
 

src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs

@@ -139,8 +139,32 @@ public static unsafe void PrepareMethod(RuntimeMethodHandle method, RuntimeTypeH
         [MethodImpl(MethodImplOptions.InternalCall)]
         internal static extern int TryGetHashCode(object o);
 
+        public static new unsafe bool Equals(object? o1, object? o2)
+        {
+            // Compare by ref for normal classes, by value for value types.
+
+            if (ReferenceEquals(o1, o2))
+                return true;
+
+            if (o1 is null || o2 is null)
+                return false;
+
+            MethodTable* pMT = GetMethodTable(o1);
+
+            // If it's not a value class, don't compare by value
+            if (!pMT->IsValueType)
+                return false;
+
+            // Make sure they are the same type.
+            if (pMT != GetMethodTable(o2))
+                return false;
+
+            // Compare the contents
+            return ContentEquals(o1, o2);
+        }
+
         [MethodImpl(MethodImplOptions.InternalCall)]
-        public static extern new bool Equals(object? o1, object? o2);
+        private static extern unsafe bool ContentEquals(object o1, object o2);
 
         [Obsolete("OffsetToStringData has been deprecated. Use string.GetPinnableReference() instead.")]
         public static int OffsetToStringData
@@ -194,8 +218,8 @@ public static object GetUninitializedObject(
             return rt.GetUninitializedObject();
         }
 
-        [MethodImpl(MethodImplOptions.InternalCall)]
-        internal static extern object AllocateUninitializedClone(object obj);
+        [LibraryImport(QCall, EntryPoint = "ObjectNative_AllocateUninitializedClone")]
+        internal static partial void AllocateUninitializedClone(ObjectHandleOnStack objHandle);
 
         /// <returns>true if given type is reference type or value type that contains references</returns>
         [Intrinsic]

src/coreclr/classlibnative/bcltype/objectnative.cpp

@@ -123,48 +123,22 @@ FCIMPL1(INT32, ObjectNative::TryGetHashCode, Object* obj) {
 }
 FCIMPLEND
 
-//
-// Compare by ref for normal classes, by value for value types.
-//
-// <TODO>@todo: it would be nice to customize this method based on the
-// defining class rather than doing a runtime check whether it is
-// a value type.</TODO>
-//
-
-FCIMPL2(FC_BOOL_RET, ObjectNative::Equals, Object *pThisRef, Object *pCompareRef)
+FCIMPL2(FC_BOOL_RET, ObjectNative::ContentEquals, Object *pThisRef, Object *pCompareRef)
 {
-    CONTRACTL
-    {
-        FCALL_CHECK;
-        INJECT_FAULT(FCThrow(kOutOfMemoryException););
-    }
-    CONTRACTL_END;
-
-    if (pThisRef == pCompareRef)
-        FC_RETURN_BOOL(TRUE);
+    FCALL_CONTRACT;
 
-    // Since we are in FCALL, we must handle NULL specially.
-    if (pThisRef == NULL || pCompareRef == NULL)
-        FC_RETURN_BOOL(FALSE);
+    // Should be ensured by caller
+    _ASSERTE(pThisRef != NULL);
+    _ASSERTE(pCompareRef != NULL);
+    _ASSERTE(pThisRef->GetMethodTable() == pCompareRef->GetMethodTable());
 
     MethodTable *pThisMT = pThisRef->GetMethodTable();
 
-    // If it's not a value class, don't compare by value
-    if (!pThisMT->IsValueType())
-        FC_RETURN_BOOL(FALSE);
-
-    // Make sure they are the same type.
-    if (pThisMT != pCompareRef->GetMethodTable())
-        FC_RETURN_BOOL(FALSE);
-
-    // Compare the contents (size - vtable - sync block index).
-    DWORD dwBaseSize = pThisMT->GetBaseSize();
-    if(pThisMT == g_pStringClass)
-        dwBaseSize -= sizeof(WCHAR);
+    // Compare the contents
     BOOL ret = memcmp(
-        (void *) (pThisRef+1),
-        (void *) (pCompareRef+1),
-        dwBaseSize - sizeof(Object) - sizeof(int)) == 0;
+        pThisRef->GetData(),
+        pCompareRef->GetData(),
+        pThisMT->GetNumInstanceFieldBytes()) == 0;
 
     FC_GC_POLL_RET();
 
@@ -215,36 +189,34 @@ FCIMPL1(Object*, ObjectNative::GetClass, Object* pThis)
 }
 FCIMPLEND
 
-FCIMPL1(Object*, ObjectNative::AllocateUninitializedClone, Object* pObjUNSAFE)
+extern "C" void QCALLTYPE ObjectNative_AllocateUninitializedClone(QCall::ObjectHandleOnStack objHandle)
 {
-    FCALL_CONTRACT;
-
-    // Delegate error handling to managed side (it will throw NullReferenceException)
-    if (pObjUNSAFE == NULL)
-        return NULL;
+    QCALL_CONTRACT;
 
-    OBJECTREF refClone  = ObjectToOBJECTREF(pObjUNSAFE);
+    BEGIN_QCALL;
 
-    HELPER_METHOD_FRAME_BEGIN_RET_1(refClone);
+    GCX_COOP();
 
+    OBJECTREF refClone = objHandle.Get();
+    _ASSERTE(refClone != NULL); // Should be handled at managed side
     MethodTable* pMT = refClone->GetMethodTable();
-
+    
     // assert that String has overloaded the Clone() method
     _ASSERTE(pMT != g_pStringClass);
-
-    if (pMT->IsArray()) {
-        refClone = DupArrayForCloning((BASEARRAYREF)refClone);
-    } else {
+    
+    if (pMT->IsArray())
+    {
+        objHandle.Set(DupArrayForCloning((BASEARRAYREF)refClone));
+    }
+    else
+    {
         // We don't need to call the <cinit> because we know
         //  that it has been called....(It was called before this was created)
-        refClone = AllocateObject(pMT);
+        objHandle.Set(AllocateObject(pMT));
     }
 
-    HELPER_METHOD_FRAME_END();
-
-    return OBJECTREFToObject(refClone);
+    END_QCALL;
 }
-FCIMPLEND
 
 extern "C" BOOL QCALLTYPE Monitor_Wait(QCall::ObjectHandleOnStack pThis, INT32 Timeout)
 {

src/coreclr/classlibnative/bcltype/objectnative.h

@@ -27,12 +27,12 @@ class ObjectNative
 
     static FCDECL1(INT32, GetHashCode, Object* vThisRef);
     static FCDECL1(INT32, TryGetHashCode, Object* vThisRef);
-    static FCDECL2(FC_BOOL_RET, Equals, Object *pThisRef, Object *pCompareRef);
-    static FCDECL1(Object*, AllocateUninitializedClone, Object* pObjUNSAFE);
+    static FCDECL2(FC_BOOL_RET, ContentEquals, Object *pThisRef, Object *pCompareRef);
     static FCDECL1(Object*, GetClass, Object* pThis);
     static FCDECL1(FC_BOOL_RET, IsLockHeld, Object* pThisUNSAFE);
 };
 
+extern "C" void QCALLTYPE ObjectNative_AllocateUninitializedClone(QCall::ObjectHandleOnStack objHandle);
 extern "C" BOOL QCALLTYPE Monitor_Wait(QCall::ObjectHandleOnStack pThis, INT32 Timeout);
 extern "C" void QCALLTYPE Monitor_Pulse(QCall::ObjectHandleOnStack pThis);
 extern "C" void QCALLTYPE Monitor_PulseAll(QCall::ObjectHandleOnStack pThis);

src/coreclr/vm/ecalllist.h

@@ -471,8 +471,7 @@ FCFuncStart(gRuntimeHelpers)
     FCFuncElement("PrepareDelegate", ReflectionInvocation::PrepareDelegate)
     FCFuncElement("GetHashCode", ObjectNative::GetHashCode)
     FCFuncElement("TryGetHashCode", ObjectNative::TryGetHashCode)
-    FCFuncElement("Equals", ObjectNative::Equals)
-    FCFuncElement("AllocateUninitializedClone", ObjectNative::AllocateUninitializedClone)
+    FCFuncElement("ContentEquals", ObjectNative::ContentEquals)
     FCFuncElement("EnsureSufficientExecutionStack", ReflectionInvocation::EnsureSufficientExecutionStack)
     FCFuncElement("TryEnsureSufficientExecutionStack", ReflectionInvocation::TryEnsureSufficientExecutionStack)
     FCFuncElement("AllocTailCallArgBuffer", TailCallHelp::AllocTailCallArgBuffer)

src/coreclr/vm/qcallentrypoints.cpp

@@ -324,6 +324,7 @@ static const Entry s_QCall[] =
     DllImportEntry(GetFileLoadExceptionMessage)
     DllImportEntry(FileLoadException_GetMessageForHR)
     DllImportEntry(Interlocked_MemoryBarrierProcessWide)
+    DllImportEntry(ObjectNative_AllocateUninitializedClone)
     DllImportEntry(Monitor_Wait)
     DllImportEntry(Monitor_Pulse)
     DllImportEntry(Monitor_PulseAll)

src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Runtime/CompilerServices/RuntimeHelpersTests.cs

@@ -68,6 +68,26 @@ public static unsafe void GetObjectValue()
             Assert.Equal(i, (int)iOV);
         }
 
+        [Fact]
+        public static void EqualsTest()
+        {
+            // Boolean RuntimeHelpers.Equals(Object, Object)
+
+            Assert.True(RuntimeHelpers.Equals(Guid.Empty, Guid.Empty));
+            Assert.False(RuntimeHelpers.Equals(Guid.Empty, Guid.NewGuid()));
+
+            // Reference equal
+            object o = new object();
+            Assert.True(RuntimeHelpers.Equals(o, o));
+
+            // Type mismatch
+            Assert.False(RuntimeHelpers.Equals(Guid.Empty, string.Empty));
+
+            // Non value types
+            Assert.False(RuntimeHelpers.Equals(new object(), new object()));
+            Assert.False(RuntimeHelpers.Equals(new int[] { 1, 2, 3 }, new int[] { 1, 2, 3 }));
+        }
+
         [Fact]
         public static void InitializeArray()
         {