Merge requiresunsafe-corelib2 into feature/requires-unsafe

src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj

@@ -22,6 +22,7 @@
     <FeaturePortableThreadPool Condition="'$(TargetsBrowser)' != 'true'">true</FeaturePortableThreadPool>
     <FeaturePortableTimer Condition="'$(TargetsBrowser)' != 'true'">true</FeaturePortableTimer>
     <FeatureSingleThread Condition="'$(TargetsBrowser)' == 'true'">true</FeatureSingleThread>
+    <EnableUnsafeAnalyzer>true</EnableUnsafeAnalyzer>
   </PropertyGroup>
 
   <ItemGroup>

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

@@ -401,7 +401,11 @@ internal IEnumerator<T> GetEnumerator<T>()
         {
             // ! Warning: "this" is an array, not an SZArrayHelper. See comments above
             // ! or you may introduce a security hole!
-            T[] @this = Unsafe.As<T[]>(this);
+            T[] @this;
+            unsafe
+            {
+                @this = Unsafe.As<T[]>(this);
+            }
             int length = @this.Length;
             return length == 0 ? SZGenericArrayEnumerator<T>.Empty : new SZGenericArrayEnumerator<T>(@this, length);
         }
@@ -411,23 +415,35 @@ private void CopyTo<T>(T[] array, int index)
             // ! Warning: "this" is an array, not an SZArrayHelper. See comments above
             // ! or you may introduce a security hole!
 
-            T[] @this = Unsafe.As<T[]>(this);
+            T[] @this;
+            unsafe
+            {
+                @this = Unsafe.As<T[]>(this);
+            }
             Array.Copy(@this, 0, array, index, @this.Length);
         }
 
         internal int get_Count<T>()
         {
             // ! Warning: "this" is an array, not an SZArrayHelper. See comments above
             // ! or you may introduce a security hole!
-            T[] @this = Unsafe.As<T[]>(this);
+            T[] @this;
+            unsafe
+            {
+                @this = Unsafe.As<T[]>(this);
+            }
             return @this.Length;
         }
 
         internal T get_Item<T>(int index)
         {
             // ! Warning: "this" is an array, not an SZArrayHelper. See comments above
             // ! or you may introduce a security hole!
-            T[] @this = Unsafe.As<T[]>(this);
+            T[] @this;
+            unsafe
+            {
+                @this = Unsafe.As<T[]>(this);
+            }
             if ((uint)index >= (uint)@this.Length)
             {
                 ThrowHelper.ThrowArgumentOutOfRange_IndexMustBeLessException();
@@ -440,7 +456,11 @@ internal void set_Item<T>(int index, T value)
         {
             // ! Warning: "this" is an array, not an SZArrayHelper. See comments above
             // ! or you may introduce a security hole!
-            T[] @this = Unsafe.As<T[]>(this);
+            T[] @this;
+            unsafe
+            {
+                @this = Unsafe.As<T[]>(this);
+            }
             if ((uint)index >= (uint)@this.Length)
             {
                 ThrowHelper.ThrowArgumentOutOfRange_IndexMustBeLessException();
@@ -459,7 +479,11 @@ private bool Contains<T>(T value)
         {
             // ! Warning: "this" is an array, not an SZArrayHelper. See comments above
             // ! or you may introduce a security hole!
-            T[] @this = Unsafe.As<T[]>(this);
+            T[] @this;
+            unsafe
+            {
+                @this = Unsafe.As<T[]>(this);
+            }
             return Array.IndexOf(@this, value, 0, @this.Length) >= 0;
         }
 
@@ -480,7 +504,11 @@ private int IndexOf<T>(T value)
         {
             // ! Warning: "this" is an array, not an SZArrayHelper. See comments above
             // ! or you may introduce a security hole!
-            T[] @this = Unsafe.As<T[]>(this);
+            T[] @this;
+            unsafe
+            {
+                @this = Unsafe.As<T[]>(this);
+            }
             return Array.IndexOf(@this, value, 0, @this.Length);
         }
 

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

@@ -435,7 +435,11 @@ private bool BindToMethodInfo(object? target, IRuntimeMethodInfo method, Runtime
         private static MulticastDelegate InternalAlloc(RuntimeType type)
         {
             Debug.Assert(type.IsAssignableTo(typeof(MulticastDelegate)));
-            return Unsafe.As<MulticastDelegate>(RuntimeTypeHandle.InternalAlloc(type));
+            // FIXME: review unsafe to confirm correct annotation
+            unsafe
+            {
+                return Unsafe.As<MulticastDelegate>(RuntimeTypeHandle.InternalAlloc(type));
+            }
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]

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

@@ -55,7 +55,11 @@ public sealed override bool Equals([NotNullWhen(true)] object? obj)
             // the types are the same, obj should also be a
             // MulticastDelegate
             Debug.Assert(obj is MulticastDelegate, "Shouldn't have failed here since we already checked the types are the same!");
-            MulticastDelegate d = Unsafe.As<MulticastDelegate>(obj);
+            MulticastDelegate d;
+            unsafe
+            {
+                d = Unsafe.As<MulticastDelegate>(obj);
+            }
 
             if (_invocationCount != 0)
             {
@@ -165,10 +169,14 @@ private static bool TrySetSlot(object?[] a, int index, object o)
             return false;
         }
 
-        private unsafe MulticastDelegate NewMulticastDelegate(object[] invocationList, int invocationCount, bool thisIsMultiCastAlready)
+        private MulticastDelegate NewMulticastDelegate(object[] invocationList, int invocationCount, bool thisIsMultiCastAlready)
         {
             // First, allocate a new multicast delegate just like this one, i.e. same type as the this object
-            MulticastDelegate result = Unsafe.As<MulticastDelegate>(RuntimeTypeHandle.InternalAllocNoChecks(RuntimeHelpers.GetMethodTable(this)));
+            MulticastDelegate result;
+            unsafe
+            {
+                result = Unsafe.As<MulticastDelegate>(RuntimeTypeHandle.InternalAllocNoChecks(RuntimeHelpers.GetMethodTable(this)));
+            }
 
             // Performance optimization - if this already points to a true multicast delegate,
             // copy _methodPtr and _methodPtrAux fields rather than calling into the EE to get them

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

@@ -2407,7 +2407,8 @@ private static bool FilterApplyMethodBase(
         private readonly object m_keepalive; // This will be filled with a LoaderAllocator reference when this RuntimeType represents a collectible type
 #pragma warning restore CS0169
 #pragma warning restore CA1823
-        private IntPtr m_cache;
+        // Must be a handle to a RuntimeTypeCache type
+        private unsafe IntPtr m_cache;
         internal IntPtr m_handle;
 
         internal static readonly RuntimeType ValueType = (RuntimeType)typeof(ValueType);
@@ -2448,7 +2449,10 @@ private RuntimeTypeCache? CacheIfExists
                 {
                     object? cache = GCHandle.InternalGet(m_cache);
                     Debug.Assert(cache == null || cache is RuntimeTypeCache);
-                    return Unsafe.As<RuntimeTypeCache>(cache);
+                    unsafe
+                    {
+                        return Unsafe.As<RuntimeTypeCache>(cache);
+                    }
                 }
                 return null;
             }
@@ -2465,7 +2469,10 @@ private RuntimeTypeCache Cache
                     if (cache != null)
                     {
                         Debug.Assert(cache is RuntimeTypeCache);
-                        return Unsafe.As<RuntimeTypeCache>(cache);
+                        unsafe
+                        {
+                            return Unsafe.As<RuntimeTypeCache>(cache);
+                        }
                     }
                 }
                 return InitializeCache();

src/libraries/Directory.Build.props

@@ -50,6 +50,7 @@
     <Nullable Condition="'$(Nullable)' == '' and '$(IsTestProject)' == 'true'">annotations</Nullable>
     <!-- AOT compatibility is enabled by default for src/ref projects. -->
     <IsAotCompatible Condition="'$(IsAotCompatible)' == '' and ('$(IsSourceProject)' == 'true' or '$(IsReferenceAssemblyProject)' == 'true')">true</IsAotCompatible>
+    <EnableUnsafeAnalyzer>true</EnableUnsafeAnalyzer>
   </PropertyGroup>
 
   <!-- Set up common paths -->

src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems

@@ -286,6 +286,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)System\Diagnostics\CodeAnalysis\RequiresAssemblyFilesAttribute.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Diagnostics\CodeAnalysis\RequiresDynamicCodeAttribute.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Diagnostics\CodeAnalysis\RequiresUnreferencedCodeAttribute.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\Diagnostics\CodeAnalysis\RequiresUnsafeAttribute.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Diagnostics\CodeAnalysis\SetsRequiredMembersAttribute.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Diagnostics\CodeAnalysis\StringSyntaxAttribute.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Diagnostics\CodeAnalysis\SuppressMessageAttribute.cs" />

src/libraries/System.Private.CoreLib/src/System/Buffers/SharedArrayPool.cs

@@ -60,7 +60,11 @@ public override T[] Rent(int minimumLength)
             SharedArrayPoolThreadLocalArray[]? tlsBuckets = t_tlsBuckets;
             if (tlsBuckets is not null && (uint)bucketIndex < (uint)tlsBuckets.Length)
             {
-                buffer = Unsafe.As<T[]>(tlsBuckets[bucketIndex].Array);
+                unsafe
+                {
+                    // Array should always be an array of T[]
+                    buffer = Unsafe.As<T[]>(tlsBuckets[bucketIndex].Array);
+                }
                 if (buffer is not null)
                 {
                     tlsBuckets[bucketIndex].Array = null;
@@ -79,7 +83,10 @@ public override T[] Rent(int minimumLength)
                 SharedArrayPoolPartitions? b = perCoreBuckets[bucketIndex];
                 if (b is not null)
                 {
-                    buffer = Unsafe.As<T[]>(b.TryPop());
+                    unsafe
+                    {
+                        buffer = Unsafe.As<T[]>(b.TryPop());
+                    }
                     if (buffer is not null)
                     {
                         if (log.IsEnabled())
@@ -302,7 +309,8 @@ private SharedArrayPoolThreadLocalArray[] InitializeTlsBucketsAndTrimming()
     internal struct SharedArrayPoolThreadLocalArray
     {
         /// <summary>The stored array.</summary>
-        public Array? Array;
+        // Must be an array of T[] at runtime
+        public unsafe Array? Array;
         /// <summary>Environment.TickCount timestamp for when this array was observed by Trim.</summary>
         public int MillisecondsTimeStamp;
 
@@ -390,7 +398,8 @@ private sealed class Partition
             private int _millisecondsTimestamp;
 
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            public bool TryPush(Array array)
+            // Array parameter must be a T[]
+            public unsafe bool TryPush(Array array)
             {
                 bool enqueued = false;
                 Monitor.Enter(this);

src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/ConcurrentDictionary.cs

@@ -2258,7 +2258,8 @@
         private sealed class Tables
         {
             /// <summary>The comparer to use for lookups in the tables.</summary>
-            internal readonly IEqualityComparer<TKey>? _comparer;
+            // Must be IAlternateEqualityComparer<TAlternateKey, TKey>
+            internal readonly unsafe IEqualityComparer<TKey>? _comparer;
             /// <summary>A singly-linked list for each bucket.</summary>
             internal readonly VolatileNode[] _buckets;
             /// <summary>Pre-computed multiplier for use on 64-bit performing faster modulo operations.</summary>
@@ -2317,6 +2318,7 @@
 
         /// <summary>Gets the dictionary's alternate comparer. The dictionary must have already been verified as compatible.</summary>
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        [RequiresUnsafe]
         private static IAlternateEqualityComparer<TAlternateKey, TKey> GetAlternateComparer<TAlternateKey>(ConcurrentDictionary<TKey, TValue>.Tables tables)
              where TAlternateKey : notnull, allows ref struct
         {
@@ -2384,7 +2386,7 @@
                 }
 
                 Tables tables = Dictionary._tables;
                 IAlternateEqualityComparer<TAlternateKey, TKey> comparer = GetAlternateComparer<TAlternateKey>(tables);
 
                 int hashcode = comparer.GetHashCode(key);
 
@@ -2407,7 +2409,7 @@
                             tables = Dictionary._tables;
                             if (!ReferenceEquals(comparer, tables._comparer))
                             {
                                 comparer = GetAlternateComparer<TAlternateKey>(tables);
                                 hashcode = comparer.GetHashCode(key);
                             }
                             continue;
@@ -2543,7 +2545,7 @@
                 }
 
                 Tables tables = Dictionary._tables;
                 IAlternateEqualityComparer<TAlternateKey, TKey> comparer = GetAlternateComparer<TAlternateKey>(tables);
 
                 int hashcode = comparer.GetHashCode(key);
                 for (Node? n = GetBucket(tables, hashcode); n is not null; n = n._next)
@@ -2589,7 +2591,7 @@
                 }
 
                 Tables tables = Dictionary._tables;
                 IAlternateEqualityComparer<TAlternateKey, TKey> comparer = GetAlternateComparer<TAlternateKey>(tables);
                 int hashcode = comparer.GetHashCode(key);
 
                 while (true)
@@ -2610,7 +2612,7 @@
                                 tables = Dictionary._tables;
                                 if (!ReferenceEquals(comparer, tables._comparer))
                                 {
                                     comparer = GetAlternateComparer<TAlternateKey>(tables);
                                     hashcode = comparer.GetHashCode(key);
                                 }
                                 continue;

src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Dictionary.cs

@@ -32,7 +32,8 @@
         private int _freeList;
         private int _freeCount;
         private int _version;
-        private IEqualityComparer<TKey>? _comparer;
+        // Must be IAlternateEqualityComparer<TAlternateKey, TKey>
+        private unsafe IEqualityComparer<TKey>? _comparer;
         private KeyCollection? _keys;
         private ValueCollection? _values;
         private const int StartOfFreeList = -3;
@@ -716,7 +717,7 @@
                     ref TValue value = ref FindValue(key, out _);
                     if (Unsafe.IsNullRef(ref value))
                     {
                         ThrowHelper.ThrowKeyNotFoundException(GetAlternateComparer(Dictionary).Create(key));
                     }
 
                     return value;
@@ -734,6 +735,7 @@
 
             /// <summary>Gets the dictionary's alternate comparer. The dictionary must have already been verified as compatible.</summary>
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            [RequiresUnsafe]
             internal static IAlternateEqualityComparer<TAlternateKey, TKey> GetAlternateComparer(Dictionary<TKey, TValue> dictionary)
             {
                 Debug.Assert(IsCompatibleKey(dictionary));
@@ -801,7 +803,7 @@
             internal ref TValue FindValue(TAlternateKey key, [MaybeNullWhen(false)] out TKey actualKey)
             {
                 Dictionary<TKey, TValue> dictionary = Dictionary;
                 IAlternateEqualityComparer<TAlternateKey, TKey> comparer = GetAlternateComparer(dictionary);
 
                 ref Entry entry = ref Unsafe.NullRef<Entry>();
                 if (dictionary._buckets != null)
@@ -872,7 +874,7 @@
             public bool Remove(TAlternateKey key, [MaybeNullWhen(false)] out TKey actualKey, [MaybeNullWhen(false)] out TValue value)
             {
                 Dictionary<TKey, TValue> dictionary = Dictionary;
                 IAlternateEqualityComparer<TAlternateKey, TKey> comparer = GetAlternateComparer(dictionary);
 
                 if (dictionary._buckets != null)
                 {

src/libraries/System.Private.CoreLib/src/System/Collections/Generic/HashSet.cs

@@ -46,7 +46,8 @@
         private int _freeList;
         private int _freeCount;
         private int _version;
-        private IEqualityComparer<T>? _comparer;
+        // Must be IAlternateEqualityComparer<TAlternate, T>
+        private unsafe IEqualityComparer<T>? _comparer;
 
         #region Constructors
 
@@ -441,6 +442,7 @@
 
             /// <summary>Gets the set's alternate comparer. The set must have already been verified as compatible.</summary>
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            [RequiresUnsafe]
             internal static IAlternateEqualityComparer<TAlternate, T> GetAlternateComparer(HashSet<T> set)
             {
                 Debug.Assert(IsCompatibleItem(set));
@@ -453,7 +455,7 @@
             public bool Add(TAlternate item)
             {
                 HashSet<T> set = Set;
                 IAlternateEqualityComparer<TAlternate, T> comparer = GetAlternateComparer(set);
 
                 if (set._buckets == null)
                 {
@@ -541,7 +543,7 @@
             public bool Remove(TAlternate item)
             {
                 HashSet<T> set = Set;
                 IAlternateEqualityComparer<TAlternate, T> comparer = GetAlternateComparer(set);
 
                 if (set._buckets != null)
                 {

src/libraries/System.Private.CoreLib/src/System/ComAwareWeakReference.cs

@@ -112,7 +112,11 @@ private void SetTarget(object? target, ComInfo? comInfo)
                 if (_comInfo != null)
                 {
                     // Check if the target is still null
-                    target = Unsafe.As<T>(GCHandle.InternalGet(_weakHandle));
+                    // FIXME: review unsafe to confirm correct annotation
+                    unsafe
+                    {
+                        target = Unsafe.As<T>(GCHandle.InternalGet(_weakHandle));
+                    }
                     if (target == null)
                     {
                         // Resolve and reset. Perform runtime cast to catch bugs
@@ -146,22 +150,40 @@ private static ComAwareWeakReference EnsureComAwareReference(ref nint taggedHand
                 GC.SuppressFinalize(newRef);
             }
 
-            return Unsafe.As<ComAwareWeakReference>(GCHandle.InternalGet(taggedHandle & ~HandleTagBits)!);
+            // FIXME: review unsafe to confirm correct annotation
+            unsafe
+            {
+                return Unsafe.As<ComAwareWeakReference>(GCHandle.InternalGet(taggedHandle & ~HandleTagBits)!);
+            }
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         internal static ComAwareWeakReference GetFromTaggedReference(nint taggedHandle)
         {
             Debug.Assert((taggedHandle & ComAwareBit) != 0);
-            return Unsafe.As<ComAwareWeakReference>(GCHandle.InternalGet(taggedHandle & ~HandleTagBits)!);
+            // FIXME: review unsafe to confirm correct annotation
+            unsafe
+            {
+                return Unsafe.As<ComAwareWeakReference>(GCHandle.InternalGet(taggedHandle & ~HandleTagBits)!);
+            }
         }
 
         [MethodImpl(MethodImplOptions.NoInlining)]
         internal static void SetTarget(ref nint taggedHandle, object? target, ComInfo? comInfo)
         {
-            ComAwareWeakReference comAwareRef = comInfo != null ?
-                EnsureComAwareReference(ref taggedHandle) :
-                Unsafe.As<ComAwareWeakReference>(GCHandle.InternalGet(taggedHandle & ~HandleTagBits)!);
+            ComAwareWeakReference comAwareRef;
+            if (comInfo != null)
+            {
+                comAwareRef = EnsureComAwareReference(ref taggedHandle);
+            }
+            else
+            {
+                // FIXME: review unsafe to confirm correct annotation
+                unsafe
+                {
+                    comAwareRef = Unsafe.As<ComAwareWeakReference>(GCHandle.InternalGet(taggedHandle & ~HandleTagBits)!);
+                }
+            }
 
             comAwareRef.SetTarget(target, comInfo);
         }