Add Array.GetMaxLength<T> (#43301)

* Add Array.GetMaxLength<T> implementation. * Implement Array.MaxLength and fix all remaining references * Apply suggestions from code review Co-authored-by: Jan Kotas <[email protected]> * Apply suggestions from code review Co-authored-by: Jan Kotas <[email protected]> * Update src/libraries/System.Collections/tests/Generic/Stack/Stack.Generic.Tests.cs Co-authored-by: Jan Kotas <[email protected]> Co-authored-by: Stephen Toub <[email protected]> Co-authored-by: Jan Kotas <[email protected]>
dotnet · Apr 17, 2021 · df6da13 · df6da13
1 parent 408e01f
commit df6da13
Show file tree

Hide file tree

Showing 27 changed files with 93 additions and 104 deletions.
diff --git a/src/coreclr/vm/gchelpers.cpp b/src/coreclr/vm/gchelpers.cpp
@@ -342,13 +342,11 @@ void PublishObjectAndNotify(TObj* &orObject, GC_ALLOC_FLAGS flags)
 #endif // FEATURE_EVENT_TRACE
 }
 
-inline SIZE_T MaxArrayLength(SIZE_T componentSize)
+inline SIZE_T MaxArrayLength()
 {
-    // Impose limits on maximum array length in each dimension to allow efficient
-    // implementation of advanced range check elimination in future. We have to allow
-    // higher limit for array of bytes (or one byte structs) for backward compatibility.
-    // Keep in sync with Array.MaxArrayLength in BCL.
-    return (componentSize == 1) ? 0X7FFFFFC7 : 0X7FEFFFFF;
+    // Impose limits on maximum array length to prevent corner case integer overflow bugs
+    // Keep in sync with Array.MaxLength in BCL.
+    return 0X7FFFFFC7;
 }
 
 OBJECTREF AllocateSzArray(TypeHandle arrayType, INT32 cElements, GC_ALLOC_FLAGS flags)
@@ -388,11 +386,11 @@ OBJECTREF AllocateSzArray(MethodTable* pArrayMT, INT32 cElements, GC_ALLOC_FLAGS
     if (cElements < 0)
         COMPlusThrow(kOverflowException);
 
-    SIZE_T componentSize = pArrayMT->GetComponentSize();
-    if ((SIZE_T)cElements > MaxArrayLength(componentSize))
+    if ((SIZE_T)cElements > MaxArrayLength())
         ThrowOutOfMemoryDimensionsExceeded();
 
     // Allocate the space from the GC heap
+    SIZE_T componentSize = pArrayMT->GetComponentSize();
 #ifdef TARGET_64BIT
     // POSITIVE_INT32 * UINT16 + SMALL_CONST
     // this cannot overflow on 64bit
@@ -568,7 +566,6 @@ OBJECTREF AllocateArrayEx(MethodTable *pArrayMT, INT32 *pArgs, DWORD dwNumArgs,
 
     // Calculate the total number of elements in the array
     UINT32 cElements;
-    SIZE_T componentSize = pArrayMT->GetComponentSize();
     bool maxArrayDimensionLengthOverflow = false;
     bool providedLowerBounds = false;
 
@@ -599,7 +596,7 @@ OBJECTREF AllocateArrayEx(MethodTable *pArrayMT, INT32 *pArgs, DWORD dwNumArgs,
             int length = pArgs[i];
             if (length < 0)
                 COMPlusThrow(kOverflowException);
-            if ((SIZE_T)length > MaxArrayLength(componentSize))
+            if ((SIZE_T)length > MaxArrayLength())
                 maxArrayDimensionLengthOverflow = true;
             if ((length > 0) && (lowerBound + (length - 1) < lowerBound))
                 COMPlusThrow(kArgumentOutOfRangeException, W("ArgumentOutOfRange_ArrayLBAndLength"));
@@ -615,7 +612,7 @@ OBJECTREF AllocateArrayEx(MethodTable *pArrayMT, INT32 *pArgs, DWORD dwNumArgs,
         int length = pArgs[0];
         if (length < 0)
             COMPlusThrow(kOverflowException);
-        if ((SIZE_T)length > MaxArrayLength(componentSize))
+        if ((SIZE_T)length > MaxArrayLength())
             maxArrayDimensionLengthOverflow = true;
         cElements = length;
     }
@@ -625,6 +622,7 @@ OBJECTREF AllocateArrayEx(MethodTable *pArrayMT, INT32 *pArgs, DWORD dwNumArgs,
         ThrowOutOfMemoryDimensionsExceeded();
 
     // Allocate the space from the GC heap
+    SIZE_T componentSize = pArrayMT->GetComponentSize();
 #ifdef TARGET_64BIT
     // POSITIVE_INT32 * UINT16 + SMALL_CONST
     // this cannot overflow on 64bit

diff --git a/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs b/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs
@@ -15,8 +15,8 @@ namespace System.Buffers
 #endif
     sealed class ArrayBufferWriter<T> : IBufferWriter<T>
     {
-        // Copy of Array.MaxArrayLength. For byte arrays the limit is slightly larger
-        private const int MaxArrayLength = 0X7FEFFFFF;
+        // Copy of Array.MaxLength.
+        private const int ArrayMaxLength = 0x7FFFFFC7;
 
         private const int DefaultInitialBufferSize = 256;
 
@@ -184,16 +184,16 @@ private void CheckAndResizeBuffer(int sizeHint)
 
                 if ((uint)newSize > int.MaxValue)
                 {
-                    // Attempt to grow to MaxArrayLength.
+                    // Attempt to grow to ArrayMaxLength.
                     uint needed = (uint)(currentLength - FreeCapacity + sizeHint);
                     Debug.Assert(needed > currentLength);
 
-                    if (needed > MaxArrayLength)
+                    if (needed > ArrayMaxLength)
                     {
                         ThrowOutOfMemoryException(needed);
                     }
 
-                    newSize = MaxArrayLength;
+                    newSize = ArrayMaxLength;
                 }
 
                 Array.Resize(ref _buffer, newSize);

diff --git a/src/libraries/Common/src/System/Collections/Generic/ArrayBuilder.cs b/src/libraries/Common/src/System/Collections/Generic/ArrayBuilder.cs
@@ -12,7 +12,6 @@ namespace System.Collections.Generic
     internal struct ArrayBuilder<T>
     {
         private const int DefaultCapacity = 4;
-        private const int MaxCoreClrArrayLength = 0x7fefffff; // For byte arrays the limit is slightly larger
 
         private T[]? _array; // Starts out null, initialized on first Add.
         private int _count; // Number of items into _array we're using.
@@ -144,9 +143,9 @@ private void EnsureCapacity(int minimum)
             int capacity = Capacity;
             int nextCapacity = capacity == 0 ? DefaultCapacity : 2 * capacity;
 
-            if ((uint)nextCapacity > (uint)MaxCoreClrArrayLength)
+            if ((uint)nextCapacity > (uint)Array.MaxLength)
             {
-                nextCapacity = Math.Max(capacity + 1, MaxCoreClrArrayLength);
+                nextCapacity = Math.Max(capacity + 1, Array.MaxLength);
             }
 
             nextCapacity = Math.Max(nextCapacity, minimum);

diff --git a/src/libraries/Common/src/System/Collections/Generic/EnumerableHelpers.cs b/src/libraries/Common/src/System/Collections/Generic/EnumerableHelpers.cs
@@ -49,27 +49,16 @@ internal static T[] ToArray<T>(IEnumerable<T> source, out int length)
                         {
                             if (count == arr.Length)
                             {
-                                // MaxArrayLength is defined in Array.MaxArrayLength and in gchelpers in CoreCLR.
-                                // It represents the maximum number of elements that can be in an array where
-                                // the size of the element is greater than one byte; a separate, slightly larger constant,
-                                // is used when the size of the element is one.
-                                const int MaxArrayLength = 0x7FEFFFFF;
-
                                 // This is the same growth logic as in List<T>:
                                 // If the array is currently empty, we make it a default size.  Otherwise, we attempt to
                                 // double the size of the array.  Doubling will overflow once the size of the array reaches
                                 // 2^30, since doubling to 2^31 is 1 larger than Int32.MaxValue.  In that case, we instead
-                                // constrain the length to be MaxArrayLength (this overflow check works because of the
-                                // cast to uint).  Because a slightly larger constant is used when T is one byte in size, we
-                                // could then end up in a situation where arr.Length is MaxArrayLength or slightly larger, such
-                                // that we constrain newLength to be MaxArrayLength but the needed number of elements is actually
-                                // larger than that.  For that case, we then ensure that the newLength is large enough to hold
-                                // the desired capacity.  This does mean that in the very rare case where we've grown to such a
-                                // large size, each new element added after MaxArrayLength will end up doing a resize.
+                                // constrain the length to be Array.MaxLength (this overflow check works because of the
+                                // cast to uint).
                                 int newLength = count << 1;
-                                if ((uint)newLength > MaxArrayLength)
+                                if ((uint)newLength > Array.MaxLength)
                                 {
-                                    newLength = MaxArrayLength <= count ? count + 1 : MaxArrayLength;
+                                    newLength = Array.MaxLength <= count ? count + 1 : Array.MaxLength;
                                 }
 
                                 Array.Resize(ref arr, newLength);

diff --git a/...s/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs b/...s/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs
@@ -1908,7 +1908,6 @@ private bool AreAllBucketsEmpty()
         /// </summary>
         private void GrowTable(Tables tables)
         {
-            const int MaxArrayLength = 0X7FEFFFFF;
             int locksAcquired = 0;
             try
             {
@@ -1964,7 +1963,7 @@ private void GrowTable(Tables tables)
 
                         Debug.Assert(newLength % 2 != 0);
 
-                        if (newLength > MaxArrayLength)
+                        if (newLength > Array.MaxLength)
                         {
                             maximizeTableSize = true;
                         }
@@ -1977,7 +1976,7 @@ private void GrowTable(Tables tables)
 
                 if (maximizeTableSize)
                 {
-                    newLength = MaxArrayLength;
+                    newLength = Array.MaxLength;
 
                     // We want to make sure that GrowTable will not be called again, since table is at the maximum size.
                     // To achieve that, we set the budget to int.MaxValue.

diff --git a/src/libraries/System.Collections.NonGeneric/src/System/Collections/SortedList.cs b/src/libraries/System.Collections.NonGeneric/src/System/Collections/SortedList.cs
@@ -70,9 +70,6 @@ public class SortedList : IDictionary, ICloneable
         private KeyList? keyList; // Do not rename (binary serialization)
         private ValueList? valueList; // Do not rename (binary serialization)
 
-        // Copy of Array.MaxArrayLength
-        internal const int MaxArrayLength = 0X7FEFFFFF;
-
         // Constructs a new sorted list. The sorted list is initially empty and has
         // a capacity of zero. Upon adding the first element to the sorted list the
         // capacity is increased to 16, and then increased in multiples of two as
@@ -378,7 +375,7 @@ private void EnsureCapacity(int min)
             int newCapacity = keys.Length == 0 ? 16 : keys.Length * 2;
             // Allow the list to grow to maximum possible capacity (~2G elements) before encountering overflow.
             // Note that this check works even when _items.Length overflowed thanks to the (uint) cast
-            if ((uint)newCapacity > MaxArrayLength) newCapacity = MaxArrayLength;
+            if ((uint)newCapacity > Array.MaxLength) newCapacity = Array.MaxLength;
             if (newCapacity < min) newCapacity = min;
             Capacity = newCapacity;
         }

diff --git a/src/libraries/System.Collections.NonGeneric/tests/SortedListTests.cs b/src/libraries/System.Collections.NonGeneric/tests/SortedListTests.cs
@@ -255,7 +255,7 @@ public void DebuggerAttribute_NullSortedList_ThrowsArgumentNullException()
         }
 
         [Fact]
-        public void EnsureCapacity_NewCapacityLessThanMin_CapsToMaxArrayLength()
+        public void EnsureCapacity_NewCapacityLessThanMin_CapsToArrayMaxLength()
         {
             // A situation like this occurs for very large lengths of SortedList.
             // To avoid allocating several GBs of memory and making this test run for a very

diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs
@@ -529,22 +529,20 @@ private void Grow(int minCapacity)
         {
             Debug.Assert(_nodes.Length < minCapacity);
 
-            // Array.MaxArrayLength is internal to S.P.CoreLib, replicate here.
-            const int MaxArrayLength = 0X7FEFFFFF;
             const int GrowFactor = 2;
             const int MinimumGrow = 4;
 
             int newcapacity = GrowFactor * _nodes.Length;
 
             // Allow the queue to grow to maximum possible capacity (~2G elements) before encountering overflow.
             // Note that this check works even when _nodes.Length overflowed thanks to the (uint) cast
-            if ((uint)newcapacity > MaxArrayLength) newcapacity = MaxArrayLength;
+            if ((uint)newcapacity > Array.MaxLength) newcapacity = Array.MaxLength;
 
             // Ensure minimum growth is respected.
             newcapacity = Math.Max(newcapacity, _nodes.Length + MinimumGrow);
 
             // If the computed capacity is still less than specified, set to the original argument.
-            // Capacities exceeding MaxArrayLength will be surfaced as OutOfMemoryException by Array.Resize.
+            // Capacities exceeding Array.MaxLength will be surfaced as OutOfMemoryException by Array.Resize.
             if (newcapacity < minCapacity) newcapacity = minCapacity;
 
             Array.Resize(ref _nodes, newcapacity);

diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/Queue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/Queue.cs
@@ -400,22 +400,20 @@ private void Grow(int capacity)
         {
             Debug.Assert(_array.Length < capacity);
 
-            // Array.MaxArrayLength is internal to S.P.CoreLib, replicate here.
-            const int MaxArrayLength = 0X7FEFFFFF;
             const int GrowFactor = 2;
             const int MinimumGrow = 4;
 
             int newcapacity = GrowFactor * _array.Length;
 
             // Allow the list to grow to maximum possible capacity (~2G elements) before encountering overflow.
             // Note that this check works even when _items.Length overflowed thanks to the (uint) cast
-            if ((uint)newcapacity > MaxArrayLength) newcapacity = MaxArrayLength;
+            if ((uint)newcapacity > Array.MaxLength) newcapacity = Array.MaxLength;
 
             // Ensure minimum growth is respected.
             newcapacity = Math.Max(newcapacity, _array.Length + MinimumGrow);
 
             // If the computed capacity is still less than specified, set to the original argument.
-            // Capacities exceeding MaxArrayLength will be surfaced as OutOfMemoryException by Array.Resize.
+            // Capacities exceeding Array.MaxLength will be surfaced as OutOfMemoryException by Array.Resize.
             if (newcapacity < capacity) newcapacity = capacity;
 
             SetCapacity(newcapacity);

diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/SortedList.cs b/src/libraries/System.Collections/src/System/Collections/Generic/SortedList.cs
@@ -522,8 +522,6 @@ void ICollection.CopyTo(Array array, int index)
             }
         }
 
-        private const int MaxArrayLength = 0X7FEFFFFF;
-
         // Ensures that the capacity of this sorted list is at least the given
         // minimum value. The capacity is increased to twice the current capacity
         // or to min, whichever is larger.
@@ -532,7 +530,7 @@ private void EnsureCapacity(int min)
             int newCapacity = keys.Length == 0 ? DefaultCapacity : keys.Length * 2;
             // Allow the list to grow to maximum possible capacity (~2G elements) before encountering overflow.
             // Note that this check works even when _items.Length overflowed thanks to the (uint) cast
-            if ((uint)newCapacity > MaxArrayLength) newCapacity = MaxArrayLength;
+            if ((uint)newCapacity > Array.MaxLength) newCapacity = Array.MaxLength;
             if (newCapacity < min) newCapacity = min;
             Capacity = newCapacity;
         }

diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/Stack.cs b/src/libraries/System.Collections/src/System/Collections/Generic/Stack.cs
@@ -315,17 +315,14 @@ private void Grow(int capacity)
         {
             Debug.Assert(_array.Length < capacity);
 
-            // Array.MaxArrayLength is internal to S.P.CoreLib, replicate here.
-            const int MaxArrayLength = 0X7FEFFFFF;
-
             int newcapacity = _array.Length == 0 ? DefaultCapacity : 2 * _array.Length;
 
             // Allow the list to grow to maximum possible capacity (~2G elements) before encountering overflow.
             // Note that this check works even when _items.Length overflowed thanks to the (uint) cast.
-            if ((uint)newcapacity > MaxArrayLength) newcapacity = MaxArrayLength;
+            if ((uint)newcapacity > Array.MaxLength) newcapacity = Array.MaxLength;
 
             // If computed capacity is still less than specified, set to the original argument.
-            // Capacities exceeding MaxArrayLength will be surfaced as OutOfMemoryException by Array.Resize.
+            // Capacities exceeding Array.MaxLength will be surfaced as OutOfMemoryException by Array.Resize.
             if (newcapacity < capacity) newcapacity = capacity;
 
             Array.Resize(ref _array, newcapacity);

diff --git a/src/libraries/System.Collections/tests/Generic/List/List.Generic.Tests.EnsureCapacity.cs b/src/libraries/System.Collections/tests/Generic/List/List.Generic.Tests.EnsureCapacity.cs
@@ -41,12 +41,15 @@ public void EnsureCapacity_NegativeCapacityRequested_Throws()
             AssertExtensions.Throws<ArgumentOutOfRangeException>("capacity", () => list.EnsureCapacity(-1));
         }
 
-        const int MaxArraySize = 0X7FEFFFFF;
+        public static IEnumerable<object[]> EnsureCapacity_LargeCapacity_Throws_MemberData()
+        {
+            yield return new object[] { 5, Array.MaxLength + 1 };
+            yield return new object[] { 1, int.MaxValue };
+        }
 
         [Theory]
-        [InlineData(5, MaxArraySize + 1)]
-        [InlineData(1, int.MaxValue)]
-        [SkipOnMono("mono forces no restrictions on array size.")]
+        [MemberData(nameof(EnsureCapacity_LargeCapacity_Throws_MemberData))]
+        [ActiveIssue("https://github.com/dotnet/runtime/issues/51411", TestRuntimes.Mono)]
         public void EnsureCapacity_LargeCapacity_Throws(int count, int requestCapacity)
         {
             List<T> list = GenericListFactory(count);

diff --git a/src/libraries/System.Collections/tests/Generic/Queue/Queue.Generic.Tests.cs b/src/libraries/System.Collections/tests/Generic/Queue/Queue.Generic.Tests.cs
@@ -348,12 +348,15 @@ public void Queue_Generic_EnsureCapacity_NegativeCapacityRequested_Throws()
             AssertExtensions.Throws<ArgumentOutOfRangeException>("capacity", () => queue.EnsureCapacity(-1));
         }
 
-        const int MaxArraySize = 0X7FEFFFFF;
+        public static IEnumerable<object[]> Queue_Generic_EnsureCapacity_LargeCapacityRequested_Throws_MemberData()
+        {
+            yield return new object[] { Array.MaxLength + 1 };
+            yield return new object[] { int.MaxValue };
+        }
 
         [Theory]
-        [InlineData(MaxArraySize + 1)]
-        [InlineData(int.MaxValue)]
-        [SkipOnMono("mono forces no restrictions on array size.")]
+        [MemberData(nameof(Queue_Generic_EnsureCapacity_LargeCapacityRequested_Throws_MemberData))]
+        [ActiveIssue("https://github.com/dotnet/runtime/issues/51411", TestRuntimes.Mono)]
         public void Queue_Generic_EnsureCapacity_LargeCapacityRequested_Throws(int requestedCapacity)
         {
             var queue = GenericQueueFactory();

diff --git a/src/libraries/System.Collections/tests/Generic/Stack/Stack.Generic.Tests.cs b/src/libraries/System.Collections/tests/Generic/Stack/Stack.Generic.Tests.cs
@@ -314,12 +314,15 @@ public void Stack_Generic_EnsureCapacity_NegativeCapacityRequested_Throws()
             AssertExtensions.Throws<ArgumentOutOfRangeException>("capacity", () => stack.EnsureCapacity(-1));
         }
 
-        const int MaxArraySize = 0X7FEFFFFF;
+        public static IEnumerable<object[]> Stack_Generic_EnsureCapacity_LargeCapacityRequested_Throws_MemberData()
+        {
+            yield return new object[] { Array.MaxLength + 1 };
+            yield return new object[] { int.MaxValue };
+        }
 
         [Theory]
-        [InlineData(MaxArraySize + 1)]
-        [InlineData(int.MaxValue)]
-        [SkipOnMono("mono forces no restrictions on array size.")]
+        [MemberData(nameof(Stack_Generic_EnsureCapacity_LargeCapacityRequested_Throws_MemberData))]
+        [ActiveIssue("https://github.com/dotnet/runtime/issues/51411", TestRuntimes.Mono)]
         public void Stack_Generic_EnsureCapacity_LargeCapacityRequested_Throws(int requestedCapacity)
         {
             var stack = GenericStackFactory();

diff --git a/src/libraries/System.Linq.Expressions/tests/Array/NewArrayBoundsTests.cs b/src/libraries/System.Linq.Expressions/tests/Array/NewArrayBoundsTests.cs
@@ -10,8 +10,6 @@ namespace System.Linq.Expressions.Tests
 {
     public static class ArrayBoundsTests
     {
-        private const int MaxArraySize = 0X7FEFFFFF;
-
         private class BogusCollection<T> : IList<T>
         {
             public T this[int index]
@@ -168,7 +166,7 @@ private static void VerifyArrayGenerator(Func<Array> func, Type arrayType, long?
             {
                 Assert.Throws<OverflowException>(() => func());
             }
-            else if (size > MaxArraySize)
+            else if (size > Array.MaxLength)
             {
                 Assert.Throws<OutOfMemoryException>(() => func());
             }

diff --git a/src/libraries/System.Memory/tests/ArrayBufferWriter/ArrayBufferWriterTests.Byte.cs b/src/libraries/System.Memory/tests/ArrayBufferWriter/ArrayBufferWriterTests.Byte.cs
@@ -98,7 +98,6 @@ public async Task WriteAndCopyToStreamAsync()
         [OuterLoop]
         public void GetMemory_ExceedMaximumBufferSize()
         {
-            const int MaxArrayLength = 0X7FEFFFFF;
             int initialCapacity = int.MaxValue / 2 + 1;
 
             try
@@ -111,8 +110,8 @@ public void GetMemory_ExceedMaximumBufferSize()
                 memory = output.GetMemory(1);
 
                 // The buffer should grow more than the 1 byte requested otherwise performance will not be usable
-                // between 1GB and 2GB. The current implementation maxes out the buffer size to MaxArrayLength.
-                Assert.Equal(MaxArrayLength - initialCapacity, memory.Length);
+                // between 1GB and 2GB. The current implementation maxes out the buffer size to Array.MaxLength.
+                Assert.Equal(Array.MaxLength - initialCapacity, memory.Length);
                 Assert.Throws<OutOfMemoryException>(() => output.GetMemory(int.MaxValue));
             }
             catch (OutOfMemoryException)