From 9544cf7e7bb79c557ae10be3d37bc0133c95d4e6 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 23 May 2022 15:47:42 -0700 Subject: [PATCH 1/4] Don't pin managed collection objects when the contents are non-blittable. --- ...ributedMarshallingModelGeneratorFactory.cs | 2 +- .../CollectionTests.cs | 12 ++++- .../TestAssets/SharedTypes/NonBlittable.cs | 53 +++++++++++++++++++ 3 files changed, 65 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/AttributedMarshallingModelGeneratorFactory.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/AttributedMarshallingModelGeneratorFactory.cs index 43c89b3cd2c16..070c0189dcc4e 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/AttributedMarshallingModelGeneratorFactory.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/AttributedMarshallingModelGeneratorFactory.cs @@ -346,7 +346,7 @@ private IMarshallingGenerator CreateNativeCollectionMarshaller( IMarshallingGenerator marshallingGenerator = new CustomNativeTypeMarshallingGenerator(marshallingStrategy, enableByValueContentsMarshalling: false); - if (collectionInfo.PinningFeatures.HasFlag(CustomTypeMarshallerPinning.ManagedType)) + if (collectionInfo.PinningFeatures.HasFlag(CustomTypeMarshallerPinning.ManagedType) && isBlittable) { return new PinnableManagedValueMarshaller(marshallingGenerator); } diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/CollectionTests.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/CollectionTests.cs index 947e9b28e4ed4..62a483a246fd0 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/CollectionTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/CollectionTests.cs @@ -39,6 +39,9 @@ public partial class Collections [LibraryImport(NativeExportsNE_Binary, EntryPoint = "sum_string_lengths")] public static partial int SumStringLengths([MarshalUsing(typeof(ListMarshaller)), MarshalUsing(typeof(Utf16StringMarshaller), ElementIndirectionDepth = 1)] List strArray); + [LibraryImport(NativeExportsNE_Binary, EntryPoint = "sum_string_lengths")] + public static partial int SumStringLengths([MarshalUsing(typeof(Utf16StringMarshaller), ElementIndirectionDepth = 1)] WrappedList strArray); + [LibraryImport(NativeExportsNE_Binary, EntryPoint = "reverse_strings_replace")] public static partial void ReverseStrings_Ref([MarshalUsing(typeof(ListMarshaller), CountElementName = "numElements"), MarshalUsing(typeof(Utf16StringMarshaller), ElementIndirectionDepth = 1)] ref List strArray, out int numElements); @@ -57,7 +60,7 @@ public static partial void ReverseStrings_Out( public static partial List GetLongBytes(long l); [LibraryImport(NativeExportsNE_Binary, EntryPoint = "and_all_members")] - [return:MarshalAs(UnmanagedType.U1)] + [return: MarshalAs(UnmanagedType.U1)] public static partial bool AndAllMembers([MarshalUsing(typeof(ListMarshaller))] List pArray, int length); } } @@ -143,6 +146,13 @@ public void ByValueNullCollectionWithNonBlittableElements() Assert.Equal(0, NativeExportsNE.Collections.SumStringLengths(null)); } + [Fact] + public void ByValueCollectionWithNonBlittableElements_WithDefaultMarshalling() + { + var strings = new WrappedList(GetStringList()); + Assert.Equal(strings.Sum(str => str?.Length ?? 0), NativeExportsNE.Collections.SumStringLengths(strings)); + } + [Fact] public void ByRefCollectionWithNonBlittableElements() { diff --git a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/NonBlittable.cs b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/NonBlittable.cs index 7732abe4c102c..92767e53c0644 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/NonBlittable.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/NonBlittable.cs @@ -227,4 +227,57 @@ public void FreeNative() Marshal.FreeCoTaskMem(allocatedMemory); } } + + [NativeMarshalling(typeof(WrappedListMarshaller<>))] + public struct WrappedList + { + public WrappedList(List list) + { + Wrapped = list; + } + + public List Wrapped { get; } + + public ref T GetPinnableReference() => ref CollectionsMarshal.AsSpan(Wrapped).GetPinnableReference(); + } + + [CustomTypeMarshaller(typeof(WrappedList<>), CustomTypeMarshallerKind.LinearCollection, Features = CustomTypeMarshallerFeatures.UnmanagedResources | CustomTypeMarshallerFeatures.TwoStageMarshalling | CustomTypeMarshallerFeatures.CallerAllocatedBuffer, BufferSize = 0x200)] + public unsafe ref struct WrappedListMarshaller + { + private ListMarshaller _marshaller; + + public WrappedListMarshaller(int sizeOfNativeElement) + : this() + { + this._marshaller = new ListMarshaller(sizeOfNativeElement); + } + + public WrappedListMarshaller(List managed, int sizeOfNativeElement) + : this(managed, Span.Empty, sizeOfNativeElement) + { + } + + public WrappedListMarshaller(List managed, Span stackSpace, int sizeOfNativeElement) + { + this._marshaller = new ListMarshaller(managed, stackSpace, sizeOfNativeElement); + } + + public ReadOnlySpan GetManagedValuesSource() => _marshaller.GetManagedValuesSource(); + + public Span GetManagedValuesDestination(int length) => _marshaller.GetManagedValuesDestination(length); + + public Span GetNativeValuesDestination() => _marshaller.GetNativeValuesDestination(); + + public ReadOnlySpan GetNativeValuesSource(int length) => _marshaller.GetNativeValuesSource(length); + + public ref byte GetPinnableReference() => ref _marshaller.GetPinnableReference(); + + public byte* ToNativeValue() => _marshaller.ToNativeValue(); + + public void FromNativeValue(byte* value) => _marshaller.FromNativeValue(value); + + public WrappedList ToManaged() => new(_marshaller.ToManaged()); + + public void FreeNative() => _marshaller.FreeNative(); + } } From 95c4e2aa791988c840be2ea5f924e9c413e42355 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 23 May 2022 16:50:21 -0700 Subject: [PATCH 2/4] Update src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/AttributedMarshallingModelGeneratorFactory.cs Co-authored-by: Aaron Robinson --- .../Marshalling/AttributedMarshallingModelGeneratorFactory.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/AttributedMarshallingModelGeneratorFactory.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/AttributedMarshallingModelGeneratorFactory.cs index 070c0189dcc4e..89dd6c26d0615 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/AttributedMarshallingModelGeneratorFactory.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/AttributedMarshallingModelGeneratorFactory.cs @@ -346,6 +346,7 @@ private IMarshallingGenerator CreateNativeCollectionMarshaller( IMarshallingGenerator marshallingGenerator = new CustomNativeTypeMarshallingGenerator(marshallingStrategy, enableByValueContentsMarshalling: false); + // Elements in the collection must be blittable to use the pinnable marshaller. if (collectionInfo.PinningFeatures.HasFlag(CustomTypeMarshallerPinning.ManagedType) && isBlittable) { return new PinnableManagedValueMarshaller(marshallingGenerator); From 6a52f17c1ce7a4738c8b8b9ac6cf2a9fb54cc278 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 24 May 2022 13:02:43 -0700 Subject: [PATCH 3/4] Feedback and build fix --- .../AttributedMarshallingModelGeneratorFactory.cs | 8 ++++---- .../tests/LibraryImportGenerator.Tests/CollectionTests.cs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/AttributedMarshallingModelGeneratorFactory.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/AttributedMarshallingModelGeneratorFactory.cs index 070c0189dcc4e..233678b73fe9e 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/AttributedMarshallingModelGeneratorFactory.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/AttributedMarshallingModelGeneratorFactory.cs @@ -315,9 +315,9 @@ private IMarshallingGenerator CreateNativeCollectionMarshaller( numElementsExpression = GetNumElementsExpressionFromMarshallingInfo(info, collectionInfo.ElementCountInfo, context); } - bool isBlittable = elementMarshaller is BlittableMarshaller; + bool elementIsBlittable = elementMarshaller is BlittableMarshaller; - if (isBlittable) + if (elementIsBlittable) { marshallingStrategy = new LinearCollectionWithBlittableElementsMarshalling(marshallingStrategy, collectionInfo.ElementType.Syntax, numElementsExpression); } @@ -341,12 +341,12 @@ private IMarshallingGenerator CreateNativeCollectionMarshaller( return new ArrayMarshaller( new CustomNativeTypeMarshallingGenerator(marshallingStrategy, enableByValueContentsMarshalling: true), elementType, - isBlittable); + elementIsBlittable); } IMarshallingGenerator marshallingGenerator = new CustomNativeTypeMarshallingGenerator(marshallingStrategy, enableByValueContentsMarshalling: false); - if (collectionInfo.PinningFeatures.HasFlag(CustomTypeMarshallerPinning.ManagedType) && isBlittable) + if (collectionInfo.PinningFeatures.HasFlag(CustomTypeMarshallerPinning.ManagedType) && elementIsBlittable) { return new PinnableManagedValueMarshaller(marshallingGenerator); } diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/CollectionTests.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/CollectionTests.cs index 62a483a246fd0..f0c35bf2ed24b 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/CollectionTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/CollectionTests.cs @@ -150,7 +150,7 @@ public void ByValueNullCollectionWithNonBlittableElements() public void ByValueCollectionWithNonBlittableElements_WithDefaultMarshalling() { var strings = new WrappedList(GetStringList()); - Assert.Equal(strings.Sum(str => str?.Length ?? 0), NativeExportsNE.Collections.SumStringLengths(strings)); + Assert.Equal(strings.Wrapped.Sum(str => str?.Length ?? 0), NativeExportsNE.Collections.SumStringLengths(strings)); } [Fact] From 8c884a2b7b6c3e98a0831c5d441824c696c161fc Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 25 May 2022 15:17:00 -0700 Subject: [PATCH 4/4] Fix types --- .../tests/TestAssets/SharedTypes/NonBlittable.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/NonBlittable.cs b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/NonBlittable.cs index 92767e53c0644..d580054ad4ab1 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/NonBlittable.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/NonBlittable.cs @@ -252,14 +252,14 @@ public WrappedListMarshaller(int sizeOfNativeElement) this._marshaller = new ListMarshaller(sizeOfNativeElement); } - public WrappedListMarshaller(List managed, int sizeOfNativeElement) + public WrappedListMarshaller(WrappedList managed, int sizeOfNativeElement) : this(managed, Span.Empty, sizeOfNativeElement) { } - public WrappedListMarshaller(List managed, Span stackSpace, int sizeOfNativeElement) + public WrappedListMarshaller(WrappedList managed, Span stackSpace, int sizeOfNativeElement) { - this._marshaller = new ListMarshaller(managed, stackSpace, sizeOfNativeElement); + this._marshaller = new ListMarshaller(managed.Wrapped, stackSpace, sizeOfNativeElement); } public ReadOnlySpan GetManagedValuesSource() => _marshaller.GetManagedValuesSource();