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,9 @@ 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
+        [RequiresUnsafe]
+        private unsafe IntPtr m_cache;
         internal IntPtr m_handle;
 
         internal static readonly RuntimeType ValueType = (RuntimeType)typeof(ValueType);
@@ -2448,7 +2450,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 +2470,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 @@
             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 @@
                 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,9 @@
     internal struct SharedArrayPoolThreadLocalArray
     {
         /// <summary>The stored array.</summary>
-        public Array? Array;
+        // Must be an array of T[] at runtime
+        [RequiresUnsafe]
+        public unsafe Array? Array;
         /// <summary>Environment.TickCount timestamp for when this array was observed by Trim.</summary>
         public int MillisecondsTimeStamp;
 
@@ -390,6 +399,8 @@
             private int _millisecondsTimestamp;
 
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            // Array parameter must be a T[]
+            [RequiresUnsafe]
             public bool TryPush(Array array)
             {
                 bool enqueued = false;

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

@@ -2258,7 +2258,9 @@ internal Node(TKey key, TValue value, int hashcode, Node? next)
         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>
+            [RequiresUnsafe]
+            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 +2319,7 @@ private static bool IsCompatibleKey<TAlternateKey>(ConcurrentDictionary<TKey, TV
 
         /// <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
         {

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

@@ -32,7 +32,9 @@ public class Dictionary<TKey, TValue> : IDictionary<TKey, TValue>, IDictionary,
         private int _freeList;
         private int _freeCount;
         private int _version;
-        private IEqualityComparer<TKey>? _comparer;
+        // Must be IAlternateEqualityComparer<TAlternateKey, TKey>
+        [RequiresUnsafe]
+        private unsafe IEqualityComparer<TKey>? _comparer;
         private KeyCollection? _keys;
         private ValueCollection? _values;
         private const int StartOfFreeList = -3;
@@ -734,6 +736,7 @@ internal static bool IsCompatibleKey(Dictionary<TKey, TValue> dictionary)
 
             /// <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));

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

@@ -46,7 +46,9 @@ public class HashSet<T> : ICollection<T>, ISet<T>, IReadOnlyCollection<T>, IRead
         private int _freeList;
         private int _freeCount;
         private int _version;
-        private IEqualityComparer<T>? _comparer;
+        // Must be IAlternateEqualityComparer<TAlternate, T>
+        [RequiresUnsafe]
+        private unsafe IEqualityComparer<T>? _comparer;
 
         #region Constructors
 
@@ -441,6 +443,7 @@ internal static bool IsCompatibleItem(HashSet<T> set)
 
             /// <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));

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);
         }