From 5641eb79afd891e020dec59fb1a9c0a6333ecbf0 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Wed, 10 Nov 2021 09:38:25 -0800 Subject: [PATCH 1/2] Initialize by-value out array to default --- .../ICustomNativeTypeMarshallingStrategy.cs | 12 ++++++ .../DllImportGenerator.Tests/ArrayTests.cs | 37 ++++++++++++++++--- .../tests/TestAssets/NativeExports/Arrays.cs | 15 ++++++++ 3 files changed, 58 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/ICustomNativeTypeMarshallingStrategy.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/ICustomNativeTypeMarshallingStrategy.cs index 338c7ee379b0d..71a0026465a13 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/ICustomNativeTypeMarshallingStrategy.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/ICustomNativeTypeMarshallingStrategy.cs @@ -977,6 +977,18 @@ public IEnumerable GenerateMarshalStatements(TypePositionInfo i if (!info.IsByRef && info.ByValueContentsMarshalKind == ByValueContentsMarshalKind.Out) { // If the parameter is marshalled by-value [Out], then we don't marshal the contents of the collection. + // We do clear the span, so that if the invoke target doesn't fill it, we aren't left with undefined content. + // .NativeValueStorage.Clear(); + string nativeIdentifier = context.GetIdentifiers(info).native; + yield return ExpressionStatement( + InvocationExpression( + MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + IdentifierName(nativeIdentifier), + IdentifierName(ManualTypeMarshallingHelper.NativeValueStoragePropertyName)), + IdentifierName("Clear")))); yield break; } diff --git a/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.Tests/ArrayTests.cs b/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.Tests/ArrayTests.cs index b52876d425f25..f4c2dae90a409 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.Tests/ArrayTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.Tests/ArrayTests.cs @@ -38,6 +38,9 @@ public partial class Arrays [GeneratedDllImport(NativeExportsNE_Binary, EntryPoint = "sum_char_array", CharSet = CharSet.Unicode)] public static partial int SumChars(char[] chars, int numElements); + [GeneratedDllImport(NativeExportsNE_Binary, EntryPoint = "fill_char_array", CharSet = CharSet.Unicode)] + public static partial void FillChars([Out] char[] chars, int length, ushort start); + [GeneratedDllImport(NativeExportsNE_Binary, EntryPoint = "reverse_char_array", CharSet = CharSet.Unicode)] public static partial void ReverseChars([MarshalAs(UnmanagedType.LPArray, SizeParamIndex = 1)] ref char[] chars, int numElements); @@ -253,12 +256,34 @@ public void DynamicSizedArrayWithConstantComponent() [Fact] public void ArrayByValueOutParameter() { - var testArray = new IntStructWrapper[10]; - int start = 5; - - NativeExportsNE.Arrays.FillRangeArray(testArray, testArray.Length, start); - - Assert.Equal(Enumerable.Range(start, 10), testArray.Select(wrapper => wrapper.Value)); + { + var testArray = new IntStructWrapper[10]; + int start = 5; + + NativeExportsNE.Arrays.FillRangeArray(testArray, testArray.Length, start); + Assert.Equal(Enumerable.Range(start, testArray.Length), testArray.Select(wrapper => wrapper.Value)); + + // Any items not populated by the invoke target should be initialized to default + testArray = new IntStructWrapper[10]; + int lengthToFill = testArray.Length / 2; + NativeExportsNE.Arrays.FillRangeArray(testArray, lengthToFill, start); + Assert.Equal(Enumerable.Range(start, lengthToFill), testArray[..lengthToFill].Select(wrapper => wrapper.Value)); + Assert.All(testArray[lengthToFill..], wrapper => Assert.Equal(0, wrapper.Value)); + } + { + var testArray = new char[10]; + ushort start = 65; + + NativeExportsNE.Arrays.FillChars(testArray, testArray.Length, start); + Assert.Equal(Enumerable.Range(start, testArray.Length), testArray.Select(c => (int)c)); + + // Any items not populated by the invoke target should be initialized to default + testArray = new char[10]; + int lengthToFill = testArray.Length / 2; + NativeExportsNE.Arrays.FillChars(testArray, lengthToFill, start); + Assert.Equal(Enumerable.Range(start, lengthToFill), testArray[..lengthToFill].Select(c => (int)c)); + Assert.All(testArray[lengthToFill..], c => Assert.Equal(0, c)); + } } [Fact] diff --git a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/NativeExports/Arrays.cs b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/NativeExports/Arrays.cs index a22016275d69c..bb17e028c150c 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/NativeExports/Arrays.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/NativeExports/Arrays.cs @@ -106,6 +106,21 @@ public static int SumChars(ushort* values, int numValues) return sum; } + [UnmanagedCallersOnly(EntryPoint = "fill_char_array")] + public static void FillChars(ushort* values, int length, ushort start) + { + if (values == null) + { + return; + } + + ushort val = start; + for (int i = 0; i < length; i++) + { + values[i] = val++; + } + } + [UnmanagedCallersOnly(EntryPoint = "reverse_char_array")] public static void ReverseChars(ushort** values, int numValues) { From d216d6b5ac0feff6784a7f07b094f2d68680581b Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Wed, 10 Nov 2021 12:27:35 -0800 Subject: [PATCH 2/2] Update compat doc --- docs/design/libraries/DllImportGenerator/Compatibility.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/design/libraries/DllImportGenerator/Compatibility.md b/docs/design/libraries/DllImportGenerator/Compatibility.md index 5f5997a36edaa..3a5dba488421e 100644 --- a/docs/design/libraries/DllImportGenerator/Compatibility.md +++ b/docs/design/libraries/DllImportGenerator/Compatibility.md @@ -71,6 +71,8 @@ Only single-dimensional arrays are supported for source generated marshalling. In the source-generated marshalling, arrays will be allocated on the stack (instead of through [`AllocCoTaskMem`](https://docs.microsoft.com/dotnet/api/system.runtime.interopservices.marshal.alloccotaskmem)) if they are passed by value or by read-only reference if they contain at most 256 bytes of data. The built-in system does not support this optimization for arrays. +In the built-in system, marshalling a `char` array by value with `CharSet.Unicode` would default to also marshalling data out. In the source-generated marshalling, the `char` array must be marked with the `[Out]` attribute for data to be marshalled out by value. + ### `in` keyword For some types - blittable or Unicode `char` - passed by read-only reference via the [`in` keyword](https://docs.microsoft.com/dotnet/csharp/language-reference/keywords/in-parameter-modifier), the built-in system simply pins the parameter. The generated marshalling code does the same, such that there is no behavioural difference. A consequence of this behaviour is that any modifications made by the invoked function will be reflected in the caller. It is left to the user to avoid the situation in which `in` is used for a parameter that will actually be modified by the invoked function.