From bd618d4da2a1ed6831ab8923293eab4057b6b11e Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Wed, 28 Dec 2022 17:29:08 -0800 Subject: [PATCH 01/16] [NativeAOT] Fix Objective-C reference tracking Fixes #80032 --- .../RuntimeHelpers.CoreCLR.cs | 11 +++ .../classlibnative/bcltype/objectnative.cpp | 72 +++++++++++++------ .../classlibnative/bcltype/objectnative.h | 1 + .../RuntimeHelpers.NativeAot.cs | 13 ++++ .../src/System/Threading/ObjectHeader.cs | 61 +++++++++++----- src/coreclr/vm/ecalllist.h | 1 + .../CompilerServices/ConditionalWeakTable.cs | 26 ++++++- .../CompilerServices/RuntimeHelpers.Mono.cs | 6 ++ 8 files changed, 149 insertions(+), 42 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs index 86afe347af9c8..0c85e6a43a80a 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs @@ -114,6 +114,17 @@ public static unsafe void PrepareMethod(RuntimeMethodHandle method, RuntimeTypeH [MethodImpl(MethodImplOptions.InternalCall)] public static extern int GetHashCode(object? o); + /// + /// If a hash code has been assigned to the object, it is returned. Otherwise zero is + /// returned. + /// + /// + /// The advantage of this over is that it avoids assigning a hash + /// code to the object if it does not already have one. + /// + [MethodImpl(MethodImplOptions.InternalCall)] + internal static extern int TryGetHashCode(object o); + [MethodImpl(MethodImplOptions.InternalCall)] public static extern new bool Equals(object? o1, object? o2); diff --git a/src/coreclr/classlibnative/bcltype/objectnative.cpp b/src/coreclr/classlibnative/bcltype/objectnative.cpp index 4b93d41631d2d..5c4595b6f1ff7 100644 --- a/src/coreclr/classlibnative/bcltype/objectnative.cpp +++ b/src/coreclr/classlibnative/bcltype/objectnative.cpp @@ -65,6 +65,34 @@ FCIMPL1(Object*, ObjectNative::GetObjectValue, Object* obj) } FCIMPLEND +static INT32 TryGetHashCodeHelper(OBJECTREF objRef) +{ + DWORD bits = objRef->GetHeader()->GetBits(); + + if (bits & BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX) + { + if (bits & BIT_SBLK_IS_HASHCODE) + { + // Common case: the object already has a hash code + return bits & MASK_HASHCODE; + } + else + { + // We have a sync block index. This means if we already have a hash code, + // it is in the sync block, otherwise we generate a new one and store it there + SyncBlock *psb = objRef->PassiveGetSyncBlock(); + if (psb != NULL) + { + DWORD hashCode = psb->GetHashCode(); + if (hashCode != 0) + return hashCode; + } + } + } + + // No hash code currently assigned. + return 0; +} NOINLINE static INT32 GetHashCodeHelper(OBJECTREF objRef) { @@ -99,32 +127,30 @@ FCIMPL1(INT32, ObjectNative::GetHashCode, Object* obj) { OBJECTREF objRef(obj); - { - DWORD bits = objRef->GetHeader()->GetBits(); + INT32 ret = TryGetHashCodeHelper(objRef); + if (ret != 0) + return ret; - if (bits & BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX) - { - if (bits & BIT_SBLK_IS_HASHCODE) - { - // Common case: the object already has a hash code - return bits & MASK_HASHCODE; - } - else - { - // We have a sync block index. This means if we already have a hash code, - // it is in the sync block, otherwise we generate a new one and store it there - SyncBlock *psb = objRef->PassiveGetSyncBlock(); - if (psb != NULL) - { - DWORD hashCode = psb->GetHashCode(); - if (hashCode != 0) - return hashCode; - } - } - } + FC_INNER_RETURN(INT32, GetHashCodeHelper(objRef)); +} +FCIMPLEND + +FCIMPL1(INT32, ObjectNative::TryGetHashCode, Object* obj) { + + CONTRACTL + { + FCALL_CHECK; } + CONTRACTL_END; - FC_INNER_RETURN(INT32, GetHashCodeHelper(objRef)); + VALIDATEOBJECT(obj); + + if (obj == 0) + return 0; + + OBJECTREF objRef(obj); + + return TryGetHashCodeHelper(objRef); } FCIMPLEND diff --git a/src/coreclr/classlibnative/bcltype/objectnative.h b/src/coreclr/classlibnative/bcltype/objectnative.h index c700ff2394987..819469a983458 100644 --- a/src/coreclr/classlibnative/bcltype/objectnative.h +++ b/src/coreclr/classlibnative/bcltype/objectnative.h @@ -30,6 +30,7 @@ class ObjectNative // If the Class object doesn't exist then you must call the GetClass() method. static FCDECL1(Object*, GetObjectValue, Object* vThisRef); 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 FCDECL1(Object*, GetClass, Object* pThis); diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.NativeAot.cs index 5b8a86cee2878..29e92efb817fc 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.NativeAot.cs @@ -104,6 +104,19 @@ public static unsafe int GetHashCode(object o) return ObjectHeader.GetHashCode(o); } + /// + /// If a hash code has been assigned to the object, it is returned. Otherwise zero is + /// returned. + /// + /// + /// The advantage of this over is that it avoids assigning a hash + /// code to the object if it does not already have one. + /// + internal static int TryGetHashCode(object o) + { + return ObjectHeader.TryGetHashCode(o); + } + [Obsolete("OffsetToStringData has been deprecated. Use string.GetPinnableReference() instead.")] public static int OffsetToStringData { diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/ObjectHeader.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/ObjectHeader.cs index 0de12219eb913..c5ce9d86acd3d 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/ObjectHeader.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/ObjectHeader.cs @@ -49,6 +49,28 @@ internal static class ObjectHeader return (int*)ppMethodTable - 1; } + // returns zero if no hash code is currently assigned + private static unsafe int TryGetHashCode(int* pHeader) + { + int bits = *pHeader; + int hashOrIndex = bits & MASK_HASHCODE_INDEX; + if ((bits & BIT_SBLK_IS_HASHCODE) != 0) + { + // Found the hash code in the header + Debug.Assert(hashOrIndex != 0); + return hashOrIndex; + } + + if ((bits & BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX) != 0) + { + // Look up the hash code in the SyncTable + return SyncTable.GetHashCode(hashOrIndex); + } + + // The hash code has not yet been set. + return 0; + } + /// /// Returns the hash code assigned to the object. If no hash code has yet been assigned, /// it assigns one in a thread-safe way. @@ -61,23 +83,10 @@ public static unsafe int GetHashCode(object o) fixed (MethodTable** ppMethodTable = &o.GetMethodTableRef()) { int* pHeader = GetHeaderPtr(ppMethodTable); - int bits = *pHeader; - int hashOrIndex = bits & MASK_HASHCODE_INDEX; - if ((bits & BIT_SBLK_IS_HASHCODE) != 0) - { - // Found the hash code in the header - Debug.Assert(hashOrIndex != 0); - return hashOrIndex; - } - - if ((bits & BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX) != 0) + int hashCode = TryGetHashCode(pHeader); + if (hashCode != 0) { - // Look up the hash code in the SyncTable - int hashCode = SyncTable.GetHashCode(hashOrIndex); - if (hashCode != 0) - { - return hashCode; - } + return hashCode; } // The hash code has not yet been set. Assign some value. @@ -85,6 +94,26 @@ public static unsafe int GetHashCode(object o) } } + /// + /// If a hash code has been assigned to the object, it is returned. Otherwise zero is + /// returned. + /// + /// + /// This method needs to follow the rules in RestrictedCallouts.h, as it may be called + /// while a garbage collection in in progress. + /// + public static unsafe int TryGetHashCode(object o) + { + if (o == null) + return 0; + + fixed (MethodTable** ppMethodTable = &o.GetMethodTableRef()) + { + int* pHeader = GetHeaderPtr(ppMethodTable); + return TryGetHashCode(pHeader); + } + } + /// /// Assigns a hash code to the object in a thread-safe way. /// diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index 9015de9e8a90a..edcb2afccdd9f 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -552,6 +552,7 @@ FCFuncStart(gRuntimeHelpers) FCFuncElement("GetSpanDataFrom", ArrayNative::GetSpanDataFrom) FCFuncElement("PrepareDelegate", ReflectionInvocation::PrepareDelegate) FCFuncElement("GetHashCode", ObjectNative::GetHashCode) + FCFuncElement("TryGetHashCode", ObjectNative::TryGetHashCode) FCFuncElement("Equals", ObjectNative::Equals) FCFuncElement("AllocateUninitializedClone", ObjectNative::AllocateUninitializedClone) FCFuncElement("EnsureSufficientExecutionStack", ReflectionInvocation::EnsureSufficientExecutionStack) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs index 4233c291010e5..36992b860bc93 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs @@ -48,8 +48,10 @@ public ConditionalWeakTable() /// /// Returns "true" if key was found, "false" otherwise. /// - /// The key may get garbaged collected during the TryGetValue operation. If so, TryGetValue + /// The key may get garbage collected during the TryGetValue operation. If so, TryGetValue /// may at its discretion, return "false" and set "value" to the default (as if the key was not present.) + /// This method needs to follow the rules in RestrictedCallouts.h, as it may be called + /// while a garbage collection in in progress. /// public bool TryGetValue(TKey key, [MaybeNullWhen(false)] out TValue value) { @@ -520,6 +522,10 @@ internal void CreateEntryNoResize(TKey key, TValue value) } /// Worker for finding a key/value pair. Must hold _lock. + /// + /// This method needs to follow the rules in RestrictedCallouts.h, as it may be called + /// while a garbage collection in in progress. + /// internal bool TryGetValueWorker(TKey key, [MaybeNullWhen(false)] out TValue value) { Debug.Assert(key != null); // Key already validated as non-null @@ -533,12 +539,26 @@ internal bool TryGetValueWorker(TKey key, [MaybeNullWhen(false)] out TValue valu /// Returns -1 if not found (if key expires during FindEntry, this can be treated as "not found."). /// Must hold _lock, or be prepared to retry the search while holding _lock. /// - /// This method requires to be on the stack to be properly tracked. + /// + /// This method requires to be on the stack to be properly tracked. + /// This method needs to follow the rules in RestrictedCallouts.h, as it may be called + /// while a garbage collection in in progress. + /// internal int FindEntry(TKey key, out object? value) { Debug.Assert(key != null); // Key already validated as non-null. - int hashCode = RuntimeHelpers.GetHashCode(key) & int.MaxValue; + int hashCode = RuntimeHelpers.TryGetHashCode(key); + + if (hashCode == 0) + { + // No hash code has been assigned to the key, so therefore it has not been added + // to any ConditionalWeakTable. + value = null; + return -1; + } + + hashCode &= int.MaxValue; int bucket = hashCode & (_buckets.Length - 1); for (int entriesIndex = Volatile.Read(ref _buckets[bucket]); entriesIndex != -1; entriesIndex = _entries[entriesIndex].Next) { diff --git a/src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.Mono.cs index 9d49d158a457f..fb4af6b8f1846 100644 --- a/src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.Mono.cs @@ -44,6 +44,12 @@ public static int GetHashCode(object? o) return InternalGetHashCode(o); } + internal static int TryGetHashCode(object o) + { + // On Mono the fast path is not yet implemented, so we just defer to GetHashCode. + return InternalGetHashCode(o); + } + public static new bool Equals(object? o1, object? o2) { if (o1 == o2) From ce3880224fcde56592af7e503820e1b292704fbd Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Fri, 30 Dec 2022 10:05:01 -0800 Subject: [PATCH 02/16] Move implementation-specific comment out of public doc comment --- .../System/Runtime/CompilerServices/ConditionalWeakTable.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs index 36992b860bc93..c645b651956af 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs @@ -50,11 +50,12 @@ public ConditionalWeakTable() /// /// The key may get garbage collected during the TryGetValue operation. If so, TryGetValue /// may at its discretion, return "false" and set "value" to the default (as if the key was not present.) - /// This method needs to follow the rules in RestrictedCallouts.h, as it may be called - /// while a garbage collection in in progress. /// public bool TryGetValue(TKey key, [MaybeNullWhen(false)] out TValue value) { + // This method needs to follow the rules in RestrictedCallouts.h, as it may be called + // while a garbage collection in in progress. + if (key is null) { ThrowHelper.ThrowArgumentNullException(ExceptionArgument.key); From 9967b1fe1f546bb6da513933a416ca1d2572106e Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Fri, 30 Dec 2022 10:25:15 -0800 Subject: [PATCH 03/16] Duplicate code for TryGetHashCode implementations. --- .../classlibnative/bcltype/objectnative.cpp | 79 +++++++++++-------- .../src/System/Threading/ObjectHeader.cs | 59 ++++++++------ 2 files changed, 80 insertions(+), 58 deletions(-) diff --git a/src/coreclr/classlibnative/bcltype/objectnative.cpp b/src/coreclr/classlibnative/bcltype/objectnative.cpp index 5c4595b6f1ff7..01192057756d7 100644 --- a/src/coreclr/classlibnative/bcltype/objectnative.cpp +++ b/src/coreclr/classlibnative/bcltype/objectnative.cpp @@ -65,34 +65,6 @@ FCIMPL1(Object*, ObjectNative::GetObjectValue, Object* obj) } FCIMPLEND -static INT32 TryGetHashCodeHelper(OBJECTREF objRef) -{ - DWORD bits = objRef->GetHeader()->GetBits(); - - if (bits & BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX) - { - if (bits & BIT_SBLK_IS_HASHCODE) - { - // Common case: the object already has a hash code - return bits & MASK_HASHCODE; - } - else - { - // We have a sync block index. This means if we already have a hash code, - // it is in the sync block, otherwise we generate a new one and store it there - SyncBlock *psb = objRef->PassiveGetSyncBlock(); - if (psb != NULL) - { - DWORD hashCode = psb->GetHashCode(); - if (hashCode != 0) - return hashCode; - } - } - } - - // No hash code currently assigned. - return 0; -} NOINLINE static INT32 GetHashCodeHelper(OBJECTREF objRef) { @@ -127,9 +99,30 @@ FCIMPL1(INT32, ObjectNative::GetHashCode, Object* obj) { OBJECTREF objRef(obj); - INT32 ret = TryGetHashCodeHelper(objRef); - if (ret != 0) - return ret; + { + DWORD bits = objRef->GetHeader()->GetBits(); + + if (bits & BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX) + { + if (bits & BIT_SBLK_IS_HASHCODE) + { + // Common case: the object already has a hash code + return bits & MASK_HASHCODE; + } + else + { + // We have a sync block index. This means if we already have a hash code, + // it is in the sync block, otherwise we generate a new one and store it there + SyncBlock *psb = objRef->PassiveGetSyncBlock(); + if (psb != NULL) + { + DWORD hashCode = psb->GetHashCode(); + if (hashCode != 0) + return hashCode; + } + } + } + } FC_INNER_RETURN(INT32, GetHashCodeHelper(objRef)); } @@ -150,7 +143,29 @@ FCIMPL1(INT32, ObjectNative::TryGetHashCode, Object* obj) { OBJECTREF objRef(obj); - return TryGetHashCodeHelper(objRef); + { + DWORD bits = objRef->GetHeader()->GetBits(); + + if (bits & BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX) + { + if (bits & BIT_SBLK_IS_HASHCODE) + { + // Common case: the object already has a hash code + return bits & MASK_HASHCODE; + } + else + { + // We have a sync block index. There may be a hash code stored within the sync block. + SyncBlock *psb = objRef->PassiveGetSyncBlock(); + if (psb != NULL) + { + return psb->GetHashCode(); + } + } + } + } + + return 0; } FCIMPLEND diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/ObjectHeader.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/ObjectHeader.cs index c5ce9d86acd3d..9bd44158ddcd8 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/ObjectHeader.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/ObjectHeader.cs @@ -49,28 +49,6 @@ internal static class ObjectHeader return (int*)ppMethodTable - 1; } - // returns zero if no hash code is currently assigned - private static unsafe int TryGetHashCode(int* pHeader) - { - int bits = *pHeader; - int hashOrIndex = bits & MASK_HASHCODE_INDEX; - if ((bits & BIT_SBLK_IS_HASHCODE) != 0) - { - // Found the hash code in the header - Debug.Assert(hashOrIndex != 0); - return hashOrIndex; - } - - if ((bits & BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX) != 0) - { - // Look up the hash code in the SyncTable - return SyncTable.GetHashCode(hashOrIndex); - } - - // The hash code has not yet been set. - return 0; - } - /// /// Returns the hash code assigned to the object. If no hash code has yet been assigned, /// it assigns one in a thread-safe way. @@ -83,10 +61,23 @@ public static unsafe int GetHashCode(object o) fixed (MethodTable** ppMethodTable = &o.GetMethodTableRef()) { int* pHeader = GetHeaderPtr(ppMethodTable); - int hashCode = TryGetHashCode(pHeader); - if (hashCode != 0) + int bits = *pHeader; + int hashOrIndex = bits & MASK_HASHCODE_INDEX; + if ((bits & BIT_SBLK_IS_HASHCODE) != 0) + { + // Found the hash code in the header + Debug.Assert(hashOrIndex != 0); + return hashOrIndex; + } + + if ((bits & BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX) != 0) { - return hashCode; + // Look up the hash code in the SyncTable + int hashCode = SyncTable.GetHashCode(hashOrIndex); + if (hashCode != 0) + { + return hashCode; + } } // The hash code has not yet been set. Assign some value. @@ -110,7 +101,23 @@ public static unsafe int TryGetHashCode(object o) fixed (MethodTable** ppMethodTable = &o.GetMethodTableRef()) { int* pHeader = GetHeaderPtr(ppMethodTable); - return TryGetHashCode(pHeader); + int bits = *pHeader; + int hashOrIndex = bits & MASK_HASHCODE_INDEX; + if ((bits & BIT_SBLK_IS_HASHCODE) != 0) + { + // Found the hash code in the header + Debug.Assert(hashOrIndex != 0); + return hashOrIndex; + } + + if ((bits & BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX) != 0) + { + // Look up the hash code in the SyncTable + return SyncTable.GetHashCode(hashOrIndex); + } + + // The hash code has not yet been set. + return 0; } } From 07bb690debce695e6314aeaa71e5fd7123627289 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Fri, 30 Dec 2022 10:48:28 -0800 Subject: [PATCH 04/16] Replace comments with a passing test. --- .../src/System/Threading/ObjectHeader.cs | 4 ---- .../CompilerServices/ConditionalWeakTable.cs | 13 +------------ .../ObjectiveC/ObjectiveCMarshalAPI/Program.cs | 7 +++++++ 3 files changed, 8 insertions(+), 16 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/ObjectHeader.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/ObjectHeader.cs index 9bd44158ddcd8..327586771fe07 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/ObjectHeader.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/ObjectHeader.cs @@ -89,10 +89,6 @@ public static unsafe int GetHashCode(object o) /// If a hash code has been assigned to the object, it is returned. Otherwise zero is /// returned. /// - /// - /// This method needs to follow the rules in RestrictedCallouts.h, as it may be called - /// while a garbage collection in in progress. - /// public static unsafe int TryGetHashCode(object o) { if (o == null) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs index c645b651956af..0c9806eadf4cf 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs @@ -53,9 +53,6 @@ public ConditionalWeakTable() /// public bool TryGetValue(TKey key, [MaybeNullWhen(false)] out TValue value) { - // This method needs to follow the rules in RestrictedCallouts.h, as it may be called - // while a garbage collection in in progress. - if (key is null) { ThrowHelper.ThrowArgumentNullException(ExceptionArgument.key); @@ -523,10 +520,6 @@ internal void CreateEntryNoResize(TKey key, TValue value) } /// Worker for finding a key/value pair. Must hold _lock. - /// - /// This method needs to follow the rules in RestrictedCallouts.h, as it may be called - /// while a garbage collection in in progress. - /// internal bool TryGetValueWorker(TKey key, [MaybeNullWhen(false)] out TValue value) { Debug.Assert(key != null); // Key already validated as non-null @@ -540,11 +533,7 @@ internal bool TryGetValueWorker(TKey key, [MaybeNullWhen(false)] out TValue valu /// Returns -1 if not found (if key expires during FindEntry, this can be treated as "not found."). /// Must hold _lock, or be prepared to retry the search while holding _lock. /// - /// - /// This method requires to be on the stack to be properly tracked. - /// This method needs to follow the rules in RestrictedCallouts.h, as it may be called - /// while a garbage collection in in progress. - /// + /// This method requires to be on the stack to be properly tracked. internal int FindEntry(TKey key, out object? value) { Debug.Assert(key != null); // Key already validated as non-null. diff --git a/src/tests/Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs b/src/tests/Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs index ebb089dc147f2..b27ff2e584681 100644 --- a/src/tests/Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs +++ b/src/tests/Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs @@ -285,6 +285,13 @@ class Scenario // Do not call this method from Main as it depends on a previous test for set up. static void _Validate_ExceptionPropagation() { + // Not yet implemented for NativeAOT. + if (TestLibrary.Utilities.IsNativeAot) + { + Console.WriteLine($"Skipping {nameof(_Validate_ExceptionPropagation)}, NYI"); + return; + } + Console.WriteLine($"Running {nameof(_Validate_ExceptionPropagation)}"); var delThrowInt = new ThrowExceptionDelegate(DEL_ThrowIntException); From 6a13d237c70de3ea726b2274b09fe828be4b3b0d Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Fri, 30 Dec 2022 11:49:19 -0800 Subject: [PATCH 05/16] Add moke test for restricted callouts. --- .../GcRestrictedCallouts.csproj | 16 ++++++++ .../GcRestrictedCallouts/Program.cs | 38 +++++++++++++++++++ .../RuntimeImportAttribute.cs | 24 ++++++++++++ .../GcRestrictedCallouts/RuntimeImports.cs | 31 +++++++++++++++ 4 files changed, 109 insertions(+) create mode 100644 src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/GcRestrictedCallouts.csproj create mode 100644 src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/Program.cs create mode 100644 src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/RuntimeImportAttribute.cs create mode 100644 src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/RuntimeImports.cs diff --git a/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/GcRestrictedCallouts.csproj b/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/GcRestrictedCallouts.csproj new file mode 100644 index 0000000000000..56619473a8fce --- /dev/null +++ b/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/GcRestrictedCallouts.csproj @@ -0,0 +1,16 @@ + + + + Exe + BuildAndRun + 0 + true + + + + + + + + + diff --git a/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/Program.cs b/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/Program.cs new file mode 100644 index 0000000000000..374d0ca11ffb7 --- /dev/null +++ b/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/Program.cs @@ -0,0 +1,38 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime; +using System.Runtime.InteropServices; +using System.Runtime.CompilerServices; + +static class Program +{ + static readonly ConditionalWeakTable s_weakTable = new(); + static readonly object s_inTableObject = new(); + static readonly object s_notInTableObject = new(); + + static volatile bool s_testPass; + + static unsafe int Main() + { + s_weakTable.Add(s_inTableObject, new object()); + + Console.WriteLine("RhRegisterGcCallout"); + RuntimeImports.RhRegisterGcCallout(RuntimeImports.GcRestrictedCalloutKind.AfterMarkPhase, + (IntPtr)(delegate* unmanaged)&GcCallback); + + Console.WriteLine("GC.Collect"); + GC.Collect(); + + Console.WriteLine("Test passed: " + s_testPass); + return s_testPass ? 100 : 1; + } + + [UnmanagedCallersOnly] + static void GcCallback(uint uiCondemnedGeneration) + { + s_testPass = s_weakTable.TryGetValue(s_inTableObject, out object _) && + !s_weakTable.TryGetValue(s_notInTableObject, out object _); + } +} \ No newline at end of file diff --git a/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/RuntimeImportAttribute.cs b/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/RuntimeImportAttribute.cs new file mode 100644 index 0000000000000..bd6c6caff5f1b --- /dev/null +++ b/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/RuntimeImportAttribute.cs @@ -0,0 +1,24 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Runtime +{ + // Exposed in Internal.CompilerServices only + [AttributeUsage(AttributeTargets.Method | AttributeTargets.Constructor, Inherited = false)] + public sealed class RuntimeImportAttribute : Attribute + { + public string DllName { get; } + public string EntryPoint { get; } + + public RuntimeImportAttribute(string entry) + { + EntryPoint = entry; + } + + public RuntimeImportAttribute(string dllName, string entry) + { + EntryPoint = entry; + DllName = dllName; + } + } +} diff --git a/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/RuntimeImports.cs b/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/RuntimeImports.cs new file mode 100644 index 0000000000000..b55cc28e311a8 --- /dev/null +++ b/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/RuntimeImports.cs @@ -0,0 +1,31 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime; +using System.Runtime.CompilerServices; + +namespace System.Runtime +{ + // Copied from src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs + static class RuntimeImports + { + private const string RuntimeLibrary = "*"; + + internal enum GcRestrictedCalloutKind + { + StartCollection = 0, // Collection is about to begin + EndCollection = 1, // Collection has completed + AfterMarkPhase = 2, // All live objects are marked (not including ready for finalization objects), + // no handles have been cleared + } + + [MethodImplAttribute(MethodImplOptions.InternalCall)] + [RuntimeImport(RuntimeLibrary, "RhRegisterGcCallout")] + internal static extern bool RhRegisterGcCallout(GcRestrictedCalloutKind eKind, IntPtr pCalloutMethod); + + [MethodImplAttribute(MethodImplOptions.InternalCall)] + [RuntimeImport(RuntimeLibrary, "RhUnregisterGcCallout")] + internal static extern void RhUnregisterGcCallout(GcRestrictedCalloutKind eKind, IntPtr pCalloutMethod); + } +} \ No newline at end of file From d9a6dc22b4c3afdb35f450c258ad7c3ffc3011fb Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Fri, 30 Dec 2022 12:35:28 -0800 Subject: [PATCH 06/16] Remove TryGetHashCode from Mono It does not guarantee that hash codes are non-zero. --- .../Runtime/CompilerServices/ConditionalWeakTable.cs | 8 +++++++- .../Runtime/CompilerServices/RuntimeHelpers.Mono.cs | 6 ------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs index 0c9806eadf4cf..58b12bcd07cdf 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs @@ -538,7 +538,10 @@ internal int FindEntry(TKey key, out object? value) { Debug.Assert(key != null); // Key already validated as non-null. - int hashCode = RuntimeHelpers.TryGetHashCode(key); + int hashCode; + +#if CORECLR || NATIVEAOT + hashCode = RuntimeHelpers.TryGetHashCode(key); if (hashCode == 0) { @@ -547,6 +550,9 @@ internal int FindEntry(TKey key, out object? value) value = null; return -1; } +#else + hashCode = RuntimeHelpers.GetHashCode(key); +#endif hashCode &= int.MaxValue; int bucket = hashCode & (_buckets.Length - 1); diff --git a/src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.Mono.cs index fb4af6b8f1846..9d49d158a457f 100644 --- a/src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.Mono.cs @@ -44,12 +44,6 @@ public static int GetHashCode(object? o) return InternalGetHashCode(o); } - internal static int TryGetHashCode(object o) - { - // On Mono the fast path is not yet implemented, so we just defer to GetHashCode. - return InternalGetHashCode(o); - } - public static new bool Equals(object? o1, object? o2) { if (o1 == o2) From cd22146b687bf59e588efea397de37b809e48d63 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Fri, 30 Dec 2022 12:39:10 -0800 Subject: [PATCH 07/16] Update src/tests/Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs Co-authored-by: Jan Kotas --- src/tests/Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tests/Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs b/src/tests/Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs index b27ff2e584681..97de52f1f489a 100644 --- a/src/tests/Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs +++ b/src/tests/Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs @@ -286,6 +286,7 @@ class Scenario static void _Validate_ExceptionPropagation() { // Not yet implemented for NativeAOT. + // https://github.com/dotnet/runtime/issues/77472 if (TestLibrary.Utilities.IsNativeAot) { Console.WriteLine($"Skipping {nameof(_Validate_ExceptionPropagation)}, NYI"); From 2131f4d3e52d1e69dddece2af0c4aef3645dbd85 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Fri, 30 Dec 2022 15:13:30 -0800 Subject: [PATCH 08/16] Update src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs Co-authored-by: Jan Kotas --- .../System/Runtime/CompilerServices/ConditionalWeakTable.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs index 58b12bcd07cdf..41f4ee8d55dfd 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs @@ -540,7 +540,9 @@ internal int FindEntry(TKey key, out object? value) int hashCode; -#if CORECLR || NATIVEAOT +#if MONO + hashCode = RuntimeHelpers.GetHashCode(key); +#else hashCode = RuntimeHelpers.TryGetHashCode(key); if (hashCode == 0) @@ -550,8 +552,6 @@ internal int FindEntry(TKey key, out object? value) value = null; return -1; } -#else - hashCode = RuntimeHelpers.GetHashCode(key); #endif hashCode &= int.MaxValue; From 129eee0e7bccb997b056cd75fc464dde68878537 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Fri, 30 Dec 2022 15:17:17 -0800 Subject: [PATCH 09/16] Revert "Add moke test for restricted callouts." This reverts commit 6a13d237c70de3ea726b2274b09fe828be4b3b0d. --- .../GcRestrictedCallouts.csproj | 16 -------- .../GcRestrictedCallouts/Program.cs | 38 ------------------- .../RuntimeImportAttribute.cs | 24 ------------ .../GcRestrictedCallouts/RuntimeImports.cs | 31 --------------- 4 files changed, 109 deletions(-) delete mode 100644 src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/GcRestrictedCallouts.csproj delete mode 100644 src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/Program.cs delete mode 100644 src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/RuntimeImportAttribute.cs delete mode 100644 src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/RuntimeImports.cs diff --git a/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/GcRestrictedCallouts.csproj b/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/GcRestrictedCallouts.csproj deleted file mode 100644 index 56619473a8fce..0000000000000 --- a/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/GcRestrictedCallouts.csproj +++ /dev/null @@ -1,16 +0,0 @@ - - - - Exe - BuildAndRun - 0 - true - - - - - - - - - diff --git a/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/Program.cs b/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/Program.cs deleted file mode 100644 index 374d0ca11ffb7..0000000000000 --- a/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/Program.cs +++ /dev/null @@ -1,38 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.Runtime; -using System.Runtime.InteropServices; -using System.Runtime.CompilerServices; - -static class Program -{ - static readonly ConditionalWeakTable s_weakTable = new(); - static readonly object s_inTableObject = new(); - static readonly object s_notInTableObject = new(); - - static volatile bool s_testPass; - - static unsafe int Main() - { - s_weakTable.Add(s_inTableObject, new object()); - - Console.WriteLine("RhRegisterGcCallout"); - RuntimeImports.RhRegisterGcCallout(RuntimeImports.GcRestrictedCalloutKind.AfterMarkPhase, - (IntPtr)(delegate* unmanaged)&GcCallback); - - Console.WriteLine("GC.Collect"); - GC.Collect(); - - Console.WriteLine("Test passed: " + s_testPass); - return s_testPass ? 100 : 1; - } - - [UnmanagedCallersOnly] - static void GcCallback(uint uiCondemnedGeneration) - { - s_testPass = s_weakTable.TryGetValue(s_inTableObject, out object _) && - !s_weakTable.TryGetValue(s_notInTableObject, out object _); - } -} \ No newline at end of file diff --git a/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/RuntimeImportAttribute.cs b/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/RuntimeImportAttribute.cs deleted file mode 100644 index bd6c6caff5f1b..0000000000000 --- a/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/RuntimeImportAttribute.cs +++ /dev/null @@ -1,24 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace System.Runtime -{ - // Exposed in Internal.CompilerServices only - [AttributeUsage(AttributeTargets.Method | AttributeTargets.Constructor, Inherited = false)] - public sealed class RuntimeImportAttribute : Attribute - { - public string DllName { get; } - public string EntryPoint { get; } - - public RuntimeImportAttribute(string entry) - { - EntryPoint = entry; - } - - public RuntimeImportAttribute(string dllName, string entry) - { - EntryPoint = entry; - DllName = dllName; - } - } -} diff --git a/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/RuntimeImports.cs b/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/RuntimeImports.cs deleted file mode 100644 index b55cc28e311a8..0000000000000 --- a/src/tests/nativeaot/SmokeTests/GcRestrictedCallouts/RuntimeImports.cs +++ /dev/null @@ -1,31 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.Runtime; -using System.Runtime.CompilerServices; - -namespace System.Runtime -{ - // Copied from src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs - static class RuntimeImports - { - private const string RuntimeLibrary = "*"; - - internal enum GcRestrictedCalloutKind - { - StartCollection = 0, // Collection is about to begin - EndCollection = 1, // Collection has completed - AfterMarkPhase = 2, // All live objects are marked (not including ready for finalization objects), - // no handles have been cleared - } - - [MethodImplAttribute(MethodImplOptions.InternalCall)] - [RuntimeImport(RuntimeLibrary, "RhRegisterGcCallout")] - internal static extern bool RhRegisterGcCallout(GcRestrictedCalloutKind eKind, IntPtr pCalloutMethod); - - [MethodImplAttribute(MethodImplOptions.InternalCall)] - [RuntimeImport(RuntimeLibrary, "RhUnregisterGcCallout")] - internal static extern void RhUnregisterGcCallout(GcRestrictedCalloutKind eKind, IntPtr pCalloutMethod); - } -} \ No newline at end of file From e81000f31e4e89ba1d149ded73147b6934e5538d Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Fri, 30 Dec 2022 15:42:09 -0800 Subject: [PATCH 10/16] Add test coverage for untracked objective objects. --- .../ObjectiveCMarshalAPI/Program.cs | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/tests/Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs b/src/tests/Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs index 97de52f1f489a..3c6b6ba471b1a 100644 --- a/src/tests/Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs +++ b/src/tests/Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs @@ -10,6 +10,7 @@ namespace ObjectiveCMarshalAPI using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.InteropServices.ObjectiveC; + using System.Threading; using Xunit; @@ -131,6 +132,39 @@ class DerivedWithFinalizer : Base [ObjectiveCTrackedTypeAttribute] class AttributedNoFinalizer { } + class HasNoHashCode : Base + { + } + + class HasHashCode : Base + { + public HasHashCode() + { + // this will write a hash code into the object header. + RuntimeHelpers.GetHashCode(this); + } + } + + class HasThinkLockHelp : Base + { + public HasThinkLockHelp() + { + // This will write lock information into the object header. + // An attempt to generate a hash code for this object will cause the lock to be + // upgrade to a thick lock. + Monitor.Enter(this); + } + } + + class HasSyncBlock : Base + { + public HasSyncBlock() + { + RuntimeHelpers.GetHashCode(this); + Monitor.Enter(this); + } + } + static void InitializeObjectiveCMarshal() { delegate* unmanaged beginEndCallback; @@ -156,6 +190,12 @@ static void InitializeObjectiveCMarshal() return h; } + [MethodImpl(MethodImplOptions.NoInlining)] + static void AllocUntrackedObject() where T : Base, new() + { + new T(); + } + static void Validate_AllocAndFreeAnotherHandle(GCHandle handle) where T : Base, new() { var obj = (T)handle.Target; @@ -191,6 +231,14 @@ static unsafe void Validate_ReferenceTracking_Scenario() ObjectiveCMarshal.CreateReferenceTrackingHandle(new AttributedNoFinalizer(), out _); }); + // Ensure objects who have no tagged memory allocated are handled when they enter the + // finalization queue. The NativeAOT implementation looks up objects in a hash table, + // so we exercise the various ways a hash code can be stored. + AllocUntrackedObject(); + AllocUntrackedObject(); + AllocUntrackedObject(); + AllocUntrackedObject(); + // Provide the minimum number of times the reference callback should run. // See IsRefCb() in NativeObjCMarshalTests.cpp for usage logic. const uint callbackCount = 3; From 2eaed505c77878d7015a928fe0973953489b9649 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Fri, 30 Dec 2022 15:43:38 -0800 Subject: [PATCH 11/16] Spelling --- .../Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tests/Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs b/src/tests/Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs index 3c6b6ba471b1a..dbb1c660c0f74 100644 --- a/src/tests/Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs +++ b/src/tests/Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs @@ -145,9 +145,9 @@ public HasHashCode() } } - class HasThinkLockHelp : Base + class HasThinLockHeld : Base { - public HasThinkLockHelp() + public HasThinLockHeld() { // This will write lock information into the object header. // An attempt to generate a hash code for this object will cause the lock to be @@ -236,7 +236,7 @@ static unsafe void Validate_ReferenceTracking_Scenario() // so we exercise the various ways a hash code can be stored. AllocUntrackedObject(); AllocUntrackedObject(); - AllocUntrackedObject(); + AllocUntrackedObject(); AllocUntrackedObject(); // Provide the minimum number of times the reference callback should run. From 1ccbe6182a55b913c521441f18249c7d61a8cb10 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Fri, 30 Dec 2022 20:18:06 -0800 Subject: [PATCH 12/16] Implement RuntimeHelpers.TryGetHashCode for Mono --- .../CompilerServices/ConditionalWeakTable.cs | 8 +-- .../CompilerServices/RuntimeHelpers.Mono.cs | 16 +++++ src/mono/mono/metadata/icall-def.h | 1 + src/mono/mono/metadata/monitor.c | 62 +++++++++++++++++-- src/mono/mono/metadata/object-internals.h | 3 + src/mono/mono/mini/interp/interp.c | 5 ++ src/mono/mono/mini/interp/mintops.def | 1 + src/mono/mono/mini/interp/transform.c | 2 + src/mono/mono/mini/intrinsics.c | 2 +- 9 files changed, 86 insertions(+), 14 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs index 41f4ee8d55dfd..0c9806eadf4cf 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs @@ -538,12 +538,7 @@ internal int FindEntry(TKey key, out object? value) { Debug.Assert(key != null); // Key already validated as non-null. - int hashCode; - -#if MONO - hashCode = RuntimeHelpers.GetHashCode(key); -#else - hashCode = RuntimeHelpers.TryGetHashCode(key); + int hashCode = RuntimeHelpers.TryGetHashCode(key); if (hashCode == 0) { @@ -552,7 +547,6 @@ internal int FindEntry(TKey key, out object? value) value = null; return -1; } -#endif hashCode &= int.MaxValue; int bucket = hashCode & (_buckets.Length - 1); diff --git a/src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.Mono.cs index 9d49d158a457f..862bdeb00eee1 100644 --- a/src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.Mono.cs @@ -44,6 +44,22 @@ public static int GetHashCode(object? o) return InternalGetHashCode(o); } + [MethodImplAttribute(MethodImplOptions.InternalCall)] + private static extern int InternalTryGetHashCode(object? o); + + /// + /// If a hash code has been assigned to the object, it is returned. Otherwise zero is + /// returned. + /// + /// + /// The advantage of this over is that it avoids assigning a hash + /// code to the object if it does not already have one. + /// + public static int TryGetHashCode(object? o) + { + return InternalTryGetHashCode(o); + } + public static new bool Equals(object? o1, object? o2) { if (o1 == o2) diff --git a/src/mono/mono/metadata/icall-def.h b/src/mono/mono/metadata/icall-def.h index b310ee2730cc2..439fc36a4d389 100644 --- a/src/mono/mono/metadata/icall-def.h +++ b/src/mono/mono/metadata/icall-def.h @@ -428,6 +428,7 @@ HANDLES(RUNH_6, "GetSpanDataFrom", ves_icall_System_Runtime_CompilerServices_Run HANDLES(RUNH_2, "GetUninitializedObjectInternal", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_GetUninitializedObjectInternal, MonoObject, 1, (MonoType_ptr)) HANDLES(RUNH_3, "InitializeArray", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_InitializeArray, void, 2, (MonoArray, MonoClassField_ptr)) HANDLES(RUNH_7, "InternalGetHashCode", mono_object_hash_icall, int, 1, (MonoObject)) +HANDLES(RUNH_8, "InternalTryGetHashCode", mono_object_try_get_hash_icall, int, 1, (MonoObject)) HANDLES(RUNH_3a, "PrepareMethod", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_PrepareMethod, void, 3, (MonoMethod_ptr, gpointer, int)) HANDLES(RUNH_4, "RunClassConstructor", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_RunClassConstructor, void, 1, (MonoType_ptr)) HANDLES(RUNH_5, "RunModuleConstructor", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_RunModuleConstructor, void, 1, (MonoImage_ptr)) diff --git a/src/mono/mono/metadata/monitor.c b/src/mono/mono/metadata/monitor.c index fc3487b38776e..7e7f167ca6310 100644 --- a/src/mono/mono/metadata/monitor.c +++ b/src/mono/mono/metadata/monitor.c @@ -515,6 +515,12 @@ mono_monitor_inflate (MonoObject *obj) #define MONO_OBJECT_ALIGNMENT_SHIFT 3 +/* + * Wang's address-based hash function: + * http://www.concentric.net/~Ttwang/tech/addrhash.htm + */ +#define HASH_OBJECT(obj) (GPOINTER_TO_UINT (obj) >> MONO_OBJECT_ALIGNMENT_SHIFT) * 2654435761u + int mono_object_hash_internal (MonoObject* obj) { @@ -542,11 +548,14 @@ mono_object_hash_internal (MonoObject* obj) * another thread computes the hash at the same time, because it'll end up * with the same value. */ - hash = (GPOINTER_TO_UINT (obj) >> MONO_OBJECT_ALIGNMENT_SHIFT) * 2654435761u; + hash = HASH_OBJECT(obj); #if SIZEOF_VOID_P == 4 /* clear the top bits as they can be discarded */ hash &= ~(LOCK_WORD_STATUS_MASK << (32 - LOCK_WORD_STATUS_BITS)); #endif + if (hash == 0) { + hash = 1; + } if (lock_word_is_free (lw)) { LockWord old_lw; lw = lock_word_new_thin_hash (hash); @@ -581,11 +590,12 @@ mono_object_hash_internal (MonoObject* obj) #else -/* - * Wang's address-based hash function: - * http://www.concentric.net/~Ttwang/tech/addrhash.htm - */ - return (GPOINTER_TO_UINT (obj) >> MONO_OBJECT_ALIGNMENT_SHIFT) * 2654435761u; + unsigned int hash = HASH_OBJECT(obj); + if (hash == 0) { + hash = 1; + } + return hash; + #endif } @@ -596,6 +606,46 @@ mono_object_hash_icall (MonoObjectHandle obj, MonoError* error) return mono_object_hash_internal (MONO_HANDLE_RAW (obj)); } +int +mono_object_try_get_hash_internal (MonoObject* obj) +{ +#ifdef HAVE_MOVING_COLLECTOR + + LockWord lw; + if (!obj) + return 0; + lw.sync = obj->synchronisation; + + LOCK_DEBUG (g_message("%s: (%d) Get hash for object %p; LW = %p", __func__, mono_thread_info_get_small_id (), obj, obj->synchronisation)); + + if (lock_word_has_hash (lw)) { + if (lock_word_is_inflated (lw)) { + return lock_word_get_inflated_lock (lw)->hash_code; + } else { + return lock_word_get_hash (lw); + } + } + + return 0; + +#else + + unsigned int hash = HASH_OBJECT(obj); + if (hash == 0) { + hash = 1; + } + return hash; + +#endif + +} + +int +mono_object_try_get_hash_icall (MonoObjectHandle obj, MonoError* error) +{ + return mono_object_try_get_hash_internal (MONO_HANDLE_RAW (obj)); +} + /* * mono_object_hash: * @obj: an object diff --git a/src/mono/mono/metadata/object-internals.h b/src/mono/mono/metadata/object-internals.h index 119d54ffd800c..7bee00f417f81 100644 --- a/src/mono/mono/metadata/object-internals.h +++ b/src/mono/mono/metadata/object-internals.h @@ -1997,6 +1997,9 @@ mono_string_hash_internal (MonoString *s); MONO_COMPONENT_API int mono_object_hash_internal (MonoObject* obj); +MONO_COMPONENT_API int +mono_object_try_get_hash_internal (MonoObject* obj); + ICALL_EXPORT void mono_value_copy_internal (void* dest, const void* src, MonoClass *klass); diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c index 65e623a4a0310..5d844c4ed8fda 100644 --- a/src/mono/mono/mini/interp/interp.c +++ b/src/mono/mono/mini/interp/interp.c @@ -7510,6 +7510,11 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK; ip += 3; MINT_IN_BREAK; } + MINT_IN_CASE(MINT_INTRINS_TRY_GET_HASHCODE) { + LOCAL_VAR (ip [1], gint32) = mono_object_try_get_hash_internal (LOCAL_VAR (ip [2], MonoObject*)); + ip += 3; + MINT_IN_BREAK; + } MINT_IN_CASE(MINT_INTRINS_GET_TYPE) { MonoObject *o = LOCAL_VAR (ip [2], MonoObject*); NULL_CHECK (o); diff --git a/src/mono/mono/mini/interp/mintops.def b/src/mono/mono/mini/interp/mintops.def index b550fb0737024..444ceb417b59d 100644 --- a/src/mono/mono/mini/interp/mintops.def +++ b/src/mono/mono/mini/interp/mintops.def @@ -794,6 +794,7 @@ OPDEF(MINT_TIER_PATCHPOINT, "tier_patchpoint", 2, 0, 0, MintOpShortInt) OPDEF(MINT_INTRINS_ENUM_HASFLAG, "intrins_enum_hasflag", 5, 1, 2, MintOpClassToken) OPDEF(MINT_INTRINS_GET_HASHCODE, "intrins_get_hashcode", 3, 1, 1, MintOpNoArgs) +OPDEF(MINT_INTRINS_TRY_GET_HASHCODE, "intrins_try_get_hashcode", 3, 1, 1, MintOpNoArgs) OPDEF(MINT_INTRINS_GET_TYPE, "intrins_get_type", 3, 1, 1, MintOpNoArgs) OPDEF(MINT_INTRINS_SPAN_CTOR, "intrins_span_ctor", 4, 1, 2, MintOpNoArgs) OPDEF(MINT_INTRINS_UNSAFE_BYTE_OFFSET, "intrins_unsafe_byte_offset", 4, 1, 2, MintOpNoArgs) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 994033b8613b1..b27f3b85ab346 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -2428,6 +2428,8 @@ interp_handle_intrinsics (TransformData *td, MonoMethod *target_method, MonoClas } else if (in_corlib && target_method->klass == mono_defaults.object_class) { if (!strcmp (tm, "InternalGetHashCode")) { *op = MINT_INTRINS_GET_HASHCODE; + } else if (!strcmp (tm, "InternalTryGetHashCode")) { + *op = MINT_INTRINS_TRY_GET_HASHCODE; } else if (!strcmp (tm, "GetType")) { if (constrained_class && m_class_is_valuetype (constrained_class) && !mono_class_is_nullable (constrained_class)) { // If constrained_class is valuetype we already know its type. diff --git a/src/mono/mono/mini/intrinsics.c b/src/mono/mono/mini/intrinsics.c index d8d6020ddf30c..2d56519261e0a 100644 --- a/src/mono/mono/mini/intrinsics.c +++ b/src/mono/mono/mini/intrinsics.c @@ -878,7 +878,7 @@ mini_emit_inst_for_method (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSign ins->klass = mono_defaults.runtimetype_class; *ins_type_initialized = TRUE; return ins; - } else if (!cfg->backend->emulate_mul_div && strcmp (cmethod->name, "InternalGetHashCode") == 0 && fsig->param_count == 1 && !mono_gc_is_moving ()) { + } else if (!cfg->backend->emulate_mul_div && (strcmp (cmethod->name, "InternalGetHashCode") == 0 || strcmp (cmethod->name, "InternalTryGetHashCode") == 0) && fsig->param_count == 1 && !mono_gc_is_moving ()) { int dreg = alloc_ireg (cfg); int t1 = alloc_ireg (cfg); From 84d8c40095ae829ac5d9899317fad2b4ca2ace60 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 1 Jan 2023 09:47:07 -0800 Subject: [PATCH 13/16] Remove unneeded MONO_COMPONENT_API --- src/mono/mono/metadata/object-internals.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/mono/metadata/object-internals.h b/src/mono/mono/metadata/object-internals.h index 7bee00f417f81..dd904a92269f6 100644 --- a/src/mono/mono/metadata/object-internals.h +++ b/src/mono/mono/metadata/object-internals.h @@ -1997,7 +1997,7 @@ mono_string_hash_internal (MonoString *s); MONO_COMPONENT_API int mono_object_hash_internal (MonoObject* obj); -MONO_COMPONENT_API int +int mono_object_try_get_hash_internal (MonoObject* obj); ICALL_EXPORT From 58f36677cd4608dec7c9233b8af88bec286aae31 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 1 Jan 2023 18:14:27 -0800 Subject: [PATCH 14/16] Remove Mono intrinsic for InternalGetHashCode This is dead code because this method no longer lives on System.Object. --- src/mono/mono/mini/intrinsics.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/mono/mono/mini/intrinsics.c b/src/mono/mono/mini/intrinsics.c index 2d56519261e0a..df07e02cc39ae 100644 --- a/src/mono/mono/mini/intrinsics.c +++ b/src/mono/mono/mini/intrinsics.c @@ -877,15 +877,6 @@ mini_emit_inst_for_method (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSign mini_type_to_eval_stack_type (cfg, fsig->ret, ins); ins->klass = mono_defaults.runtimetype_class; *ins_type_initialized = TRUE; - return ins; - } else if (!cfg->backend->emulate_mul_div && (strcmp (cmethod->name, "InternalGetHashCode") == 0 || strcmp (cmethod->name, "InternalTryGetHashCode") == 0) && fsig->param_count == 1 && !mono_gc_is_moving ()) { - int dreg = alloc_ireg (cfg); - int t1 = alloc_ireg (cfg); - - MONO_EMIT_NEW_BIALU_IMM (cfg, OP_SHR_IMM, t1, args [0]->dreg, 3); - EMIT_NEW_BIALU_IMM (cfg, ins, OP_MUL_IMM, dreg, t1, 2654435761u); - ins->type = STACK_I4; - return ins; } else if (strcmp (cmethod->name, ".ctor") == 0 && fsig->param_count == 0) { MONO_INST_NEW (cfg, ins, OP_NOP); From 1d94c9c6175c165f64ef55423507fa963ffe8c6b Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 1 Jan 2023 18:18:20 -0800 Subject: [PATCH 15/16] Move interpreter transforms to correct class. --- src/mono/mono/mini/interp/transform.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index b27f3b85ab346..cee648b0d2c3b 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -2368,6 +2368,10 @@ interp_handle_intrinsics (TransformData *td, MonoMethod *target_method, MonoClas interp_ins_set_dreg (td->last_ins, td->sp [-1].local); td->ip += 5; return TRUE; + } else if (!strcmp (tm, "InternalGetHashCode")) { + *op = MINT_INTRINS_GET_HASHCODE; + } else if (!strcmp (tm, "InternalTryGetHashCode")) { + *op = MINT_INTRINS_TRY_GET_HASHCODE; } else if (!strcmp (tm, "GetRawData")) { interp_add_ins (td, MINT_LDFLDA_UNSAFE); td->last_ins->data [0] = (gint16) MONO_ABI_SIZEOF (MonoObject); @@ -2426,11 +2430,7 @@ interp_handle_intrinsics (TransformData *td, MonoMethod *target_method, MonoClas td->sp [-1].klass == mono_defaults.runtimetype_class && td->sp [-2].klass == mono_defaults.runtimetype_class) { *op = MINT_CNE_P; } else if (in_corlib && target_method->klass == mono_defaults.object_class) { - if (!strcmp (tm, "InternalGetHashCode")) { - *op = MINT_INTRINS_GET_HASHCODE; - } else if (!strcmp (tm, "InternalTryGetHashCode")) { - *op = MINT_INTRINS_TRY_GET_HASHCODE; - } else if (!strcmp (tm, "GetType")) { + if (!strcmp (tm, "GetType")) { if (constrained_class && m_class_is_valuetype (constrained_class) && !mono_class_is_nullable (constrained_class)) { // If constrained_class is valuetype we already know its type. // Resolve GetType to a constant so we can fold type comparisons From 93f22e158f13c2f0a06692b48161780efc004408 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Mon, 2 Jan 2023 11:17:03 -0800 Subject: [PATCH 16/16] Rename and move icall to match convention. --- src/mono/mono/metadata/icall-def.h | 4 ++-- src/mono/mono/metadata/icall.c | 12 ++++++++++++ src/mono/mono/metadata/monitor.c | 12 ------------ 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/mono/mono/metadata/icall-def.h b/src/mono/mono/metadata/icall-def.h index 439fc36a4d389..eaad5ea49b736 100644 --- a/src/mono/mono/metadata/icall-def.h +++ b/src/mono/mono/metadata/icall-def.h @@ -427,8 +427,8 @@ HANDLES(RUNH_1, "GetObjectValue", ves_icall_System_Runtime_CompilerServices_Runt HANDLES(RUNH_6, "GetSpanDataFrom", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_GetSpanDataFrom, gpointer, 3, (MonoClassField_ptr, MonoType_ptr, gpointer)) HANDLES(RUNH_2, "GetUninitializedObjectInternal", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_GetUninitializedObjectInternal, MonoObject, 1, (MonoType_ptr)) HANDLES(RUNH_3, "InitializeArray", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_InitializeArray, void, 2, (MonoArray, MonoClassField_ptr)) -HANDLES(RUNH_7, "InternalGetHashCode", mono_object_hash_icall, int, 1, (MonoObject)) -HANDLES(RUNH_8, "InternalTryGetHashCode", mono_object_try_get_hash_icall, int, 1, (MonoObject)) +HANDLES(RUNH_7, "InternalGetHashCode", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_InternalGetHashCode, int, 1, (MonoObject)) +HANDLES(RUNH_8, "InternalTryGetHashCode", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_InternalTryGetHashCode, int, 1, (MonoObject)) HANDLES(RUNH_3a, "PrepareMethod", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_PrepareMethod, void, 3, (MonoMethod_ptr, gpointer, int)) HANDLES(RUNH_4, "RunClassConstructor", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_RunClassConstructor, void, 1, (MonoType_ptr)) HANDLES(RUNH_5, "RunModuleConstructor", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_RunModuleConstructor, void, 1, (MonoImage_ptr)) diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index 34a4f476e7d61..8c00f3792904d 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -1072,6 +1072,18 @@ ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_InitializeArray (MonoAr #endif } +int +ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_InternalGetHashCode (MonoObjectHandle obj, MonoError* error) +{ + return mono_object_hash_internal (MONO_HANDLE_RAW (obj)); +} + +int +ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_InternalTryGetHashCode (MonoObjectHandle obj, MonoError* error) +{ + return mono_object_try_get_hash_internal (MONO_HANDLE_RAW (obj)); +} + MonoObjectHandle ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_GetObjectValue (MonoObjectHandle obj, MonoError *error) { diff --git a/src/mono/mono/metadata/monitor.c b/src/mono/mono/metadata/monitor.c index 7e7f167ca6310..322ff8459cb3b 100644 --- a/src/mono/mono/metadata/monitor.c +++ b/src/mono/mono/metadata/monitor.c @@ -600,12 +600,6 @@ mono_object_hash_internal (MonoObject* obj) } -int -mono_object_hash_icall (MonoObjectHandle obj, MonoError* error) -{ - return mono_object_hash_internal (MONO_HANDLE_RAW (obj)); -} - int mono_object_try_get_hash_internal (MonoObject* obj) { @@ -640,12 +634,6 @@ mono_object_try_get_hash_internal (MonoObject* obj) } -int -mono_object_try_get_hash_icall (MonoObjectHandle obj, MonoError* error) -{ - return mono_object_try_get_hash_internal (MONO_HANDLE_RAW (obj)); -} - /* * mono_object_hash: * @obj: an object