From 84d57b6c7c424eac69eb2c4b5c264d2931803858 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Fri, 12 Nov 2021 15:46:27 -0800 Subject: [PATCH] Treat pointer fields in structs as blittable regardless of what they point at --- .../TypeSymbolExtensions.cs | 2 +- .../BlittableStructTests.cs | 79 ++++++++++++++++++- .../ManualTypeMarshallingAnalyzerTests.cs | 20 +---- .../NativeExports/BlittableStructs.cs | 20 +++++ .../tests/TestAssets/SharedTypes/Blittable.cs | 8 ++ 5 files changed, 110 insertions(+), 19 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/TypeSymbolExtensions.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/TypeSymbolExtensions.cs index 9768e2d03ce692..4f5aaf0cc7d7fc 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/TypeSymbolExtensions.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/TypeSymbolExtensions.cs @@ -31,7 +31,7 @@ private static bool HasOnlyBlittableFields(this ITypeSymbol type, ImmutableHashS bool fieldBlittable = field switch { { Type: { IsReferenceType: true } } => false, - { Type: IPointerTypeSymbol ptr } => IsConsideredBlittable(ptr.PointedAtType), + { Type: IPointerTypeSymbol ptr } => true, { Type: IFunctionPointerTypeSymbol } => true, not { Type: { SpecialType: SpecialType.None } } => IsSpecialTypeBlittable(field.Type.SpecialType), // Assume that type parameters that can be blittable are blittable. diff --git a/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.Tests/BlittableStructTests.cs b/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.Tests/BlittableStructTests.cs index 5aa24bf1af91ab..8fcc27093bb798 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.Tests/BlittableStructTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.Tests/BlittableStructTests.cs @@ -29,12 +29,23 @@ public static partial void DoubleIntFieldsRefReturn( public static partial void DoubleIntFieldsOutReturn( IntFields input, out IntFields result); + + [GeneratedDllImport(NativeExportsNE_Binary, EntryPoint = "blittablestructs_increment_invert_ptrfields_byref")] + public static partial void IncrementInvertPointerFieldsByRef(ref PointerFields result); + + [GeneratedDllImport(NativeExportsNE_Binary, EntryPoint = "blittablestructs_increment_invert_ptrfields_byref")] + public static partial void IncrementInvertPointerFieldsByRefIn(in PointerFields result); + + [GeneratedDllImport(NativeExportsNE_Binary, EntryPoint = "blittablestructs_increment_invert_ptrfields_refreturn")] + public static partial void IncrementInvertPointerFieldsRefReturn( + PointerFields input, + ref PointerFields result); } public class BlittableStructTests { [Fact] - public void ValidateBlittableStruct() + public void ValidateIntFields() { const int A = 24, B = 37, C = 59; var initial = new IntFields() @@ -82,5 +93,71 @@ public void ValidateBlittableStruct() Assert.Equal(expected, input); // Updated even when passed with in keyword (matches built-in system) } } + + [Fact] + public unsafe void ValidatePointerFields() + { + int iInitial = 31; + bool bInitial = false; + char cInitial = 'A'; + + int iExpected = iInitial + 1; + bool bExpected = !bInitial; + char cExpected = (char)(cInitial + 1); + + int i = iInitial; + bool b = bInitial; + char c = cInitial; + var initial = new PointerFields() + { + i = &i, + b = &b, + c = &c, + }; + + PointerFields input = initial; + { + int iResult; + bool bResult; + char cResult; + var result = new PointerFields() + { + i = &iResult, + b = &bResult, + c = &cResult + }; + NativeExportsNE.IncrementInvertPointerFieldsRefReturn(input, ref result); + Assert.Equal(initial, input); + ValidateFieldValues(result); + } + + { + ResetFieldValues(input); + NativeExportsNE.IncrementInvertPointerFieldsByRef(ref input); + Assert.Equal(initial, input); + ValidateFieldValues(input); + } + + { + ResetFieldValues(input); + NativeExportsNE.IncrementInvertPointerFieldsByRefIn(in input); + Assert.Equal(initial, input); + ValidateFieldValues(input); + } + + void ResetFieldValues(PointerFields input) + { + *(input.i) = iInitial; + *(input.b) = bInitial; + *(input.c) = cInitial; + } + + void ValidateFieldValues(PointerFields result) + { + Assert.Equal(iExpected, *result.i); + Assert.Equal(bExpected, *result.b); + Assert.Equal(cExpected, *result.c); + } + } } } diff --git a/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/ManualTypeMarshallingAnalyzerTests.cs b/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/ManualTypeMarshallingAnalyzerTests.cs index ba00865d9f3ae2..2938fa35d314c2 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/ManualTypeMarshallingAnalyzerTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/ManualTypeMarshallingAnalyzerTests.cs @@ -1280,7 +1280,7 @@ public Native(S s) } [ConditionalFact] - public async Task ValueTypeContainingPointerBlittableType_DoesNotReportDiagnostic() + public async Task ValueTypeContainingPointers_DoesNotReportDiagnostic() { var source = @" using System.Runtime.InteropServices; @@ -1288,26 +1288,12 @@ public async Task ValueTypeContainingPointerBlittableType_DoesNotReportDiagnosti [BlittableType] unsafe struct S { - private int* ptr; + private int* intPtr; + private bool* boolPtr; }"; await VerifyCS.VerifyAnalyzerAsync(source); } - [ConditionalFact] - public async Task ValueTypeContainingPointerToNonBlittableType_ReportsDiagnostic() - { - var source = @" -using System.Runtime.InteropServices; - -[{|#0:BlittableType|}] -unsafe struct S -{ - private bool* ptr; -}"; - await VerifyCS.VerifyAnalyzerAsync(source, - VerifyCS.Diagnostic(BlittableTypeMustBeBlittableRule).WithLocation(0).WithArguments("S")); - } - [ConditionalFact] public async Task BlittableValueTypeContainingPointerToSelf_DoesNotReportDiagnostic() { diff --git a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/NativeExports/BlittableStructs.cs b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/NativeExports/BlittableStructs.cs index f9dbaf9b7e015c..71b3ae5d2efbb0 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/NativeExports/BlittableStructs.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/NativeExports/BlittableStructs.cs @@ -40,5 +40,25 @@ public static void DoubleIntFieldsRefReturn( result->b = input.b * 2; result->c = input.c * 2; } + + [UnmanagedCallersOnly(EntryPoint = "blittablestructs_increment_invert_ptrfields_byref")] + [DNNE.C99DeclCode("struct ptr_fields { int* i; int* b; uint16_t* c; };")] + public static void IncrementInvertPointerFieldsByRef( + [DNNE.C99Type("struct ptr_fields*")] PointerFields* result) + { + *(result->i) += 1; + *(result->b) = !*(result->b); + *(result->c) += (char)1; + } + + [UnmanagedCallersOnly(EntryPoint = "blittablestructs_increment_invert_ptrfields_refreturn")] + public static void IncrementInvertPointerFieldsRefReturn( + [DNNE.C99Type("struct ptr_fields")] PointerFields input, + [DNNE.C99Type("struct ptr_fields*")] PointerFields* result) + { + *(result->i) = *(input.i) + 1; + *(result->b) = !(*input.b); + *(result->c) = (char)(*(input.c) + 1); + } } } diff --git a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/Blittable.cs b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/Blittable.cs index dec9f74ab49239..fc25816928f6f1 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/Blittable.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/Blittable.cs @@ -13,4 +13,12 @@ public struct IntFields public int b; public int c; } + + [BlittableType] + public unsafe struct PointerFields + { + public int* i; + public bool* b; + public char* c; + } }