From e58100bae089e9eadd13c1404ba53e6d583faab1 Mon Sep 17 00:00:00 2001 From: petris Date: Wed, 15 Feb 2023 02:55:56 +0100 Subject: [PATCH 1/8] Add CollectionsMarshal.SetCount(list, count) Adds the ability to resize lists, exposed in CollectionsMarshal due to potentially risky behaviour caused by the lack of element initialization. Supersedes #77794. Fixes #55217. --- .../InteropServices/CollectionsMarshal.cs | 41 ++++++++++++++++ .../ref/System.Runtime.InteropServices.cs | 1 + .../CollectionsMarshalTests.cs | 47 +++++++++++++++++++ 3 files changed, 89 insertions(+) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/CollectionsMarshal.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/CollectionsMarshal.cs index 6a60224305a6b..67cf440100909 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/CollectionsMarshal.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/CollectionsMarshal.cs @@ -39,5 +39,46 @@ public static ref TValue GetValueRefOrNullRef(DictionaryItems should not be added to or removed from the while the ref is in use. public static ref TValue? GetValueRefOrAddDefault(Dictionary dictionary, TKey key, out bool exists) where TKey : notnull => ref Dictionary.CollectionsMarshalHelper.GetValueRefOrAddDefault(dictionary, key, out exists); + + /// + /// Sets the count of the to the specified value. + /// + /// + /// The list to set the count of. + /// The value to set the list's count to. + /// + /// is . + /// + /// + /// is negative/>. + /// + /// + /// When increasing the count, uninitialized is being exposed. + /// + public static void SetCount(List list, int count) + { + if (count < 0) + { + ThrowHelper.ThrowArgumentOutOfRangeException_NeedNonNegNum(nameof(count)); + } + + if (count == list._size) + { + return; + } + + list._version++; + + if (count > list.Capacity) + { + list.Grow(count); + } + else if (count < list._size && RuntimeHelpers.IsReferenceOrContainsReferences()) + { + Array.Clear(list._items, count, list._size - count); + } + + list._size = count; + } } } diff --git a/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs b/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs index 6e091f27cc3e7..9beac1d41e914 100644 --- a/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs +++ b/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs @@ -483,6 +483,7 @@ public static partial class CollectionsMarshal public static System.Span AsSpan(System.Collections.Generic.List? list) { throw null; } public static ref TValue GetValueRefOrNullRef(System.Collections.Generic.Dictionary dictionary, TKey key) where TKey : notnull { throw null; } public static ref TValue? GetValueRefOrAddDefault(System.Collections.Generic.Dictionary dictionary, TKey key, out bool exists) where TKey : notnull { throw null; } + public static void SetCount(System.Collections.Generic.List list, int count) { throw null; } } [System.AttributeUsageAttribute(System.AttributeTargets.Class, Inherited=false)] public sealed partial class ComDefaultInterfaceAttribute : System.Attribute diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/CollectionsMarshalTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/CollectionsMarshalTests.cs index 876c0681bc664..bfb9c993d5951 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/CollectionsMarshalTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/CollectionsMarshalTests.cs @@ -505,5 +505,52 @@ private class IntAsObject public int Value; public int Property { get; set; } } + + [Fact] + public void ListSetCount() + { + List list = null!; + Assert.Throws(() => CollectionsMarshal.SetCount(list, 3)); + + Assert.Throws(() => CollectionsMarshal.SetCount(list, -1)); + + list = new(); + Assert.Throws(() => CollectionsMarshal.SetCount(list, -1)); + + CollectionsMarshal.SetCount(list, 5); + Assert.Equal(5, list.Count); + + list = new() { 1, 2, 3, 4, 5 }; + // make sure that size decrease preserves content + CollectionsMarshal.SetCount(list, 3); + Assert.Equal(3, list.Count); + Assert.Throws(() => list[3]); + SequenceEquals(CollectionsMarshal.AsSpan(list), new int[] { 1, 2, 3 }); + + // make sure that size increase preserves content + CollectionsMarshal.SetCount(list, 5); + SequenceEquals(CollectionsMarshal.AsSpan(list)[..3], new int[] { 1, 2, 3 }); + + // make sure that reallocations preserve content + int newCount = list.Capacity * 2; + CollectionsMarshal.SetCount(list, newCount); + Assert.Equal(newCount, list.Count); + SequenceEquals(CollectionsMarshal.AsSpan(list)[..3], new int[] { 1, 2, 3 }); + + List listReference = new() { "a", "b", "c", "d", "e" }; + CollectionsMarshal.SetCount(listReference, 3); + // verify that reference types aren't cleared + SequenceEquals(CollectionsMarshal.AsSpan(listReference), new string[] { "a", "b", "c" }); + + static void SequenceEquals(ReadOnlySpan actual, ReadOnlySpan expected) + { + Assert.Equal(actual.Length, expected.Length); + + for (int i = 0; i < actual.Length; i++) + { + Assert.Equal(actual[i], expected[i]); + } + } + } } } From 7a590e1b8c262c5e881f1b5ebff2dbc41f0ed42a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= <35800402+MichalPetryka@users.noreply.github.com> Date: Wed, 15 Feb 2023 03:10:08 +0100 Subject: [PATCH 2/8] Update XML doc --- .../src/System/Runtime/InteropServices/CollectionsMarshal.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/CollectionsMarshal.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/CollectionsMarshal.cs index 67cf440100909..a14bf6e8bc971 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/CollectionsMarshal.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/CollectionsMarshal.cs @@ -43,17 +43,16 @@ public static ref TValue GetValueRefOrNullRef(Dictionary /// Sets the count of the to the specified value. /// - /// /// The list to set the count of. /// The value to set the list's count to. /// /// is . /// /// - /// is negative/>. + /// is negative. /// /// - /// When increasing the count, uninitialized is being exposed. + /// When increasing the count, uninitialized data is being exposed. /// public static void SetCount(List list, int count) { From 605656294f06800af8bafd041a04209eb668d795 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= <35800402+MichalPetryka@users.noreply.github.com> Date: Wed, 15 Feb 2023 03:38:32 +0100 Subject: [PATCH 3/8] Add missing using --- .../src/System/Runtime/InteropServices/CollectionsMarshal.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/CollectionsMarshal.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/CollectionsMarshal.cs index a14bf6e8bc971..86d4475fd180a 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/CollectionsMarshal.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/CollectionsMarshal.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Runtime.CompilerServices; namespace System.Runtime.InteropServices { From 9ed8241098f7886211162056ea6b35e672ab0309 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= <35800402+MichalPetryka@users.noreply.github.com> Date: Wed, 15 Feb 2023 04:30:32 +0100 Subject: [PATCH 4/8] Fix test --- .../Runtime/InteropServices/CollectionsMarshalTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/CollectionsMarshalTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/CollectionsMarshalTests.cs index bfb9c993d5951..e3d44ced585f1 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/CollectionsMarshalTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/CollectionsMarshalTests.cs @@ -525,22 +525,22 @@ public void ListSetCount() CollectionsMarshal.SetCount(list, 3); Assert.Equal(3, list.Count); Assert.Throws(() => list[3]); - SequenceEquals(CollectionsMarshal.AsSpan(list), new int[] { 1, 2, 3 }); + SequenceEquals(CollectionsMarshal.AsSpan(list), new int[] { 1, 2, 3 }); // make sure that size increase preserves content CollectionsMarshal.SetCount(list, 5); - SequenceEquals(CollectionsMarshal.AsSpan(list)[..3], new int[] { 1, 2, 3 }); + SequenceEquals(CollectionsMarshal.AsSpan(list)[..3], new int[] { 1, 2, 3 }); // make sure that reallocations preserve content int newCount = list.Capacity * 2; CollectionsMarshal.SetCount(list, newCount); Assert.Equal(newCount, list.Count); - SequenceEquals(CollectionsMarshal.AsSpan(list)[..3], new int[] { 1, 2, 3 }); + SequenceEquals(CollectionsMarshal.AsSpan(list)[..3], new int[] { 1, 2, 3 }); List listReference = new() { "a", "b", "c", "d", "e" }; CollectionsMarshal.SetCount(listReference, 3); // verify that reference types aren't cleared - SequenceEquals(CollectionsMarshal.AsSpan(listReference), new string[] { "a", "b", "c" }); + SequenceEquals(CollectionsMarshal.AsSpan(listReference), new string[] { "a", "b", "c" }); static void SequenceEquals(ReadOnlySpan actual, ReadOnlySpan expected) { From 28080dff1ac5f173c708cb355219879370040aac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= <35800402+MichalPetryka@users.noreply.github.com> Date: Thu, 20 Apr 2023 16:37:20 +0200 Subject: [PATCH 5/8] Update CollectionsMarshalTests.cs --- .../System/Runtime/InteropServices/CollectionsMarshalTests.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/CollectionsMarshalTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/CollectionsMarshalTests.cs index e3d44ced585f1..3963a0825748d 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/CollectionsMarshalTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/CollectionsMarshalTests.cs @@ -541,6 +541,9 @@ public void ListSetCount() CollectionsMarshal.SetCount(listReference, 3); // verify that reference types aren't cleared SequenceEquals(CollectionsMarshal.AsSpan(listReference), new string[] { "a", "b", "c" }); + CollectionsMarshal.SetCount(listReference, 5); + // verify that removed reference types are cleared + SequenceEquals(CollectionsMarshal.AsSpan(listReference), new string[] { "a", "b", "c", null, null }); static void SequenceEquals(ReadOnlySpan actual, ReadOnlySpan expected) { From 6755dc24d5c8b5fe6efee991dd5ab7a1227a9c7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= <35800402+MichalPetryka@users.noreply.github.com> Date: Thu, 20 Apr 2023 16:38:04 +0200 Subject: [PATCH 6/8] Update CollectionsMarshal.cs --- .../src/System/Runtime/InteropServices/CollectionsMarshal.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/CollectionsMarshal.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/CollectionsMarshal.cs index 86d4475fd180a..a54dc8405f1ab 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/CollectionsMarshal.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/CollectionsMarshal.cs @@ -62,11 +62,6 @@ public static void SetCount(List list, int count) ThrowHelper.ThrowArgumentOutOfRangeException_NeedNonNegNum(nameof(count)); } - if (count == list._size) - { - return; - } - list._version++; if (count > list.Capacity) From 4f3d83926f94f2608d054a1aeaf42ce4e7b31546 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= <35800402+MichalPetryka@users.noreply.github.com> Date: Thu, 20 Apr 2023 18:21:13 +0200 Subject: [PATCH 7/8] Update CollectionsMarshalTests.cs --- .../InteropServices/CollectionsMarshalTests.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/CollectionsMarshalTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/CollectionsMarshalTests.cs index 3963a0825748d..bfb5f84b1e104 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/CollectionsMarshalTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/CollectionsMarshalTests.cs @@ -521,29 +521,36 @@ public void ListSetCount() Assert.Equal(5, list.Count); list = new() { 1, 2, 3, 4, 5 }; + ref int intRef = ref MemoryMarshal.GetReference(CollectionsMarshal.AsSpan(list)); // make sure that size decrease preserves content CollectionsMarshal.SetCount(list, 3); Assert.Equal(3, list.Count); Assert.Throws(() => list[3]); SequenceEquals(CollectionsMarshal.AsSpan(list), new int[] { 1, 2, 3 }); + Assert.True(Unsafe.AreSame(ref intRef, ref MemoryMarshal.GetReference(CollectionsMarshal.AsSpan(list)))); - // make sure that size increase preserves content + // make sure that size increase preserves content and doesn't clear CollectionsMarshal.SetCount(list, 5); - SequenceEquals(CollectionsMarshal.AsSpan(list)[..3], new int[] { 1, 2, 3 }); + SequenceEquals(CollectionsMarshal.AsSpan(list), new int[] { 1, 2, 3, 4, 5 }); + Assert.True(Unsafe.AreSame(ref intRef, ref MemoryMarshal.GetReference(CollectionsMarshal.AsSpan(list)))); // make sure that reallocations preserve content int newCount = list.Capacity * 2; CollectionsMarshal.SetCount(list, newCount); Assert.Equal(newCount, list.Count); SequenceEquals(CollectionsMarshal.AsSpan(list)[..3], new int[] { 1, 2, 3 }); + Assert.True(!Unsafe.AreSame(ref intRef, ref MemoryMarshal.GetReference(CollectionsMarshal.AsSpan(list)))); List listReference = new() { "a", "b", "c", "d", "e" }; + ref int stringRef = ref MemoryMarshal.GetReference(CollectionsMarshal.AsSpan(listReference)); CollectionsMarshal.SetCount(listReference, 3); // verify that reference types aren't cleared SequenceEquals(CollectionsMarshal.AsSpan(listReference), new string[] { "a", "b", "c" }); + Assert.True(Unsafe.AreSame(ref stringRef, ref MemoryMarshal.GetReference(CollectionsMarshal.AsSpan(listReference)))); CollectionsMarshal.SetCount(listReference, 5); // verify that removed reference types are cleared SequenceEquals(CollectionsMarshal.AsSpan(listReference), new string[] { "a", "b", "c", null, null }); + Assert.True(Unsafe.AreSame(ref stringRef, ref MemoryMarshal.GetReference(CollectionsMarshal.AsSpan(listReference)))); static void SequenceEquals(ReadOnlySpan actual, ReadOnlySpan expected) { From 1b5961d0f2103bae9db47fd35a8fa26375e4c63d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= <35800402+MichalPetryka@users.noreply.github.com> Date: Thu, 20 Apr 2023 18:47:27 +0200 Subject: [PATCH 8/8] Update CollectionsMarshalTests.cs --- .../System/Runtime/InteropServices/CollectionsMarshalTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/CollectionsMarshalTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/CollectionsMarshalTests.cs index bfb5f84b1e104..8a3ec2207da48 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/CollectionsMarshalTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/CollectionsMarshalTests.cs @@ -542,7 +542,7 @@ public void ListSetCount() Assert.True(!Unsafe.AreSame(ref intRef, ref MemoryMarshal.GetReference(CollectionsMarshal.AsSpan(list)))); List listReference = new() { "a", "b", "c", "d", "e" }; - ref int stringRef = ref MemoryMarshal.GetReference(CollectionsMarshal.AsSpan(listReference)); + ref string stringRef = ref MemoryMarshal.GetReference(CollectionsMarshal.AsSpan(listReference)); CollectionsMarshal.SetCount(listReference, 3); // verify that reference types aren't cleared SequenceEquals(CollectionsMarshal.AsSpan(listReference), new string[] { "a", "b", "c" });