From 62063121620771b4c9cc3e66d883050e4f82378b Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Wed, 11 May 2022 15:39:54 -0700 Subject: [PATCH 01/11] Add BStrStringMarshaller to source generator --- .../System.Private.CoreLib.Shared.projitems | 1 + .../Marshalling/BStrStringMarshaller.cs | 120 ++++++++++++++++++ .../MarshallingAttributeInfo.cs | 39 +++--- .../TypeNames.cs | 1 + .../ref/System.Runtime.InteropServices.cs | 14 ++ .../StringTests.cs | 77 +++++++++++ .../Compiles.cs | 2 + .../tests/TestAssets/NativeExports/Strings.cs | 62 +++++++++ 8 files changed, 296 insertions(+), 20 deletions(-) create mode 100644 src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index 48c1fbecb3fb4f..3d9b0374b78418 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -867,6 +867,7 @@ + diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs new file mode 100644 index 00000000000000..93577e321b7ab2 --- /dev/null +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs @@ -0,0 +1,120 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using System.Runtime.CompilerServices; +using System.Text; + +namespace System.Runtime.InteropServices.Marshalling +{ + /// + /// Marshaller for BSTR strings + /// + [CLSCompliant(false)] + [CustomTypeMarshaller(typeof(string), BufferSize = 0x200, + Features = CustomTypeMarshallerFeatures.UnmanagedResources | CustomTypeMarshallerFeatures.TwoStageMarshalling | CustomTypeMarshallerFeatures.CallerAllocatedBuffer)] + public unsafe ref struct BStrStringMarshaller + { + private byte* _ptrToFirstChar; + private bool _allocated; + + /// + /// Initializes a new instance of the . + /// + /// The string to marshal. + public BStrStringMarshaller(string? str) + : this(str, default) + { } + + /// + /// Initializes a new instance of the . + /// + /// The string to marshal. + /// Buffer that may be used for marshalling. + /// + /// The must not be movable - that is, it should not be + /// on the managed heap or it should be pinned. + /// + /// + public BStrStringMarshaller(string? str, Span buffer) + { + _allocated = false; + + if (str is null) + { + _ptrToFirstChar = null; + return; + } + + int lengthInBytes = checked(sizeof(char) * str.Length); + + // A caller provided buffer must be at least (lengthInBytes + 6) bytes + // in order to be constructed manually. The 6 extra bytes are 4 for byte length and 2 for wide null. + if ((lengthInBytes + 6) > buffer.Length) + { + // Use precise byte count when the provided stack-allocated buffer is not sufficient + _ptrToFirstChar = (byte*)Marshal.AllocBSTRByteLen((uint)lengthInBytes); + _allocated = true; + } + else + { + // set length and update buffer target + byte* pBuffer = (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(buffer)); + *((uint*)pBuffer) = (uint)lengthInBytes; + _ptrToFirstChar = pBuffer + sizeof(uint); + } + + // Confirm the size is properly set for the allocated BSTR. + Debug.Assert(lengthInBytes == Marshal.SysStringByteLen((IntPtr)_ptrToFirstChar)); + + // Copy characters from the managed string + Buffer.Memmove(ref *(char*)_ptrToFirstChar, ref str.GetRawStringData(), (nuint)str.Length + 1); + } + + /// + /// Returns the native value representing the string. + /// + /// + /// + /// + public byte* ToNativeValue() => _ptrToFirstChar; + + /// + /// Sets the native value representing the string. + /// + /// The native value. + /// + /// + /// + public void FromNativeValue(byte* value) + { + _ptrToFirstChar = value; + _allocated = true; + } + + /// + /// Returns the managed string. + /// + /// + /// + /// + public string? ToManaged() + { + if (_ptrToFirstChar is null) + return null; + return Marshal.PtrToStringBSTR((IntPtr)_ptrToFirstChar); + } + + /// + /// Frees native resources. + /// + /// + /// + /// + public void FreeNative() + { + if (_allocated) + Marshal.FreeBSTR((IntPtr)_ptrToFirstChar); + } + } +} diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/MarshallingAttributeInfo.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/MarshallingAttributeInfo.cs index 795a49642c0797..c888ca035a9420 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/MarshallingAttributeInfo.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/MarshallingAttributeInfo.cs @@ -12,7 +12,6 @@ namespace Microsoft.Interop { - /// /// Type used to pass on default marshalling details. /// @@ -73,7 +72,6 @@ public enum CharEncoding Undefined, Utf8, Utf16, - Ansi, Custom } @@ -762,7 +760,13 @@ private bool TryCreateTypeBasedMarshallingInfo( } else { - marshallingInfo = CreateStringMarshallingInfo(type, _defaultInfo.CharEncoding); + marshallingInfo = _defaultInfo.CharEncoding switch + { + CharEncoding.Utf16 => CreateStringMarshallingInfo(type, TypeNames.Utf16StringMarshaller), + CharEncoding.Utf8 => CreateStringMarshallingInfo(type, TypeNames.Utf8StringMarshaller), + _ => throw new InvalidOperationException() + }; + return true; } @@ -843,30 +847,25 @@ private MarshallingInfo CreateStringMarshallingInfo( ITypeSymbol type, UnmanagedType unmanagedType) { - CharEncoding charEncoding = unmanagedType switch + string? marshallerName = unmanagedType switch { - UnmanagedType.LPStr => CharEncoding.Ansi, - UnmanagedType.LPTStr or UnmanagedType.LPWStr => CharEncoding.Utf16, - MarshalAsInfo.UnmanagedType_LPUTF8Str => CharEncoding.Utf8, - _ => CharEncoding.Undefined + UnmanagedType.BStr => TypeNames.BStrStringMarshaller, + UnmanagedType.LPStr => TypeNames.AnsiStringMarshaller, + UnmanagedType.LPTStr or UnmanagedType.LPWStr => TypeNames.Utf16StringMarshaller, + MarshalAsInfo.UnmanagedType_LPUTF8Str => TypeNames.Utf8StringMarshaller, + _ => null }; - if (charEncoding == CharEncoding.Undefined) + + if (marshallerName is null) return new MarshalAsInfo(unmanagedType, _defaultInfo.CharEncoding); - return CreateStringMarshallingInfo(type, charEncoding); + return CreateStringMarshallingInfo(type, marshallerName); } private MarshallingInfo CreateStringMarshallingInfo( ITypeSymbol type, - CharEncoding charEncoding) + string marshallerName) { - string? marshallerName = charEncoding switch - { - CharEncoding.Ansi => TypeNames.AnsiStringMarshaller, - CharEncoding.Utf16 => TypeNames.Utf16StringMarshaller, - CharEncoding.Utf8 => TypeNames.Utf8StringMarshaller, - _ => throw new InvalidOperationException() - }; INamedTypeSymbol? stringMarshaller = _compilation.GetTypeByMetadataName(marshallerName); if (stringMarshaller is null) return new MissingSupportMarshallingInfo(); @@ -877,9 +876,9 @@ private MarshallingInfo CreateStringMarshallingInfo( return CreateNativeMarshallingInfoForValue( type, stringMarshaller, - default, + null, customTypeMarshallerData.Value, - allowPinningManagedType: charEncoding == CharEncoding.Utf16, + allowPinningManagedType: marshallerName is TypeNames.Utf16StringMarshaller, useDefaultMarshalling: false); } diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/TypeNames.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/TypeNames.cs index 876dc649001f97..778c1cd2beb39a 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/TypeNames.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/TypeNames.cs @@ -18,6 +18,7 @@ public static class TypeNames public const string CustomTypeMarshallerAttributeGenericPlaceholder = "System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerAttribute.GenericPlaceholder"; public const string AnsiStringMarshaller = "System.Runtime.InteropServices.Marshalling.AnsiStringMarshaller"; + public const string BStrStringMarshaller = "System.Runtime.InteropServices.Marshalling.BStrStringMarshaller"; public const string Utf16StringMarshaller = "System.Runtime.InteropServices.Marshalling.Utf16StringMarshaller"; public const string Utf8StringMarshaller = "System.Runtime.InteropServices.Marshalling.Utf8StringMarshaller"; 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 6c196e7f100621..9b8cf7383c5f34 100644 --- a/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs +++ b/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs @@ -2078,6 +2078,20 @@ public void FromNativeValue(byte* value) { } public string? ToManaged() { throw null; } public void FreeNative() { } } + [System.CLSCompliant(false)] + [System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerAttribute(typeof(string), BufferSize = 0x200, + Features = System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerFeatures.UnmanagedResources + | System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerFeatures.CallerAllocatedBuffer + | System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerFeatures.TwoStageMarshalling )] + public unsafe ref struct BStrStringMarshaller + { + public BStrStringMarshaller(string? str) { } + public BStrStringMarshaller(string? str, System.Span buffer) { } + public byte* ToNativeValue() { throw null; } + public void FromNativeValue(byte* value) { } + public string? ToManaged() { throw null; } + public void FreeNative() { } + } [System.CLSCompliantAttribute(false)] [System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerAttribute(typeof(System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerAttribute.GenericPlaceholder[]), System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerKind.LinearCollection, BufferSize = 0x200, diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/StringTests.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/StringTests.cs index d9c1f82c8e8d86..00757b1449a5a9 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/StringTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/StringTests.cs @@ -23,6 +23,7 @@ private class EntryPoints private const string UShortSuffix = "_ushort"; private const string ByteSuffix = "_byte"; + private const string BStrSuffix = "_bstr"; public class Byte { @@ -41,6 +42,15 @@ public class UShort public const string ReverseInplace = EntryPoints.ReverseInplace + UShortSuffix; public const string ReverseReplace = EntryPoints.ReverseReplace + UShortSuffix; } + + public class BStr + { + public const string ReturnLength = EntryPoints.ReturnLength + BStrSuffix; + public const string ReverseReturn = EntryPoints.ReverseReturn + BStrSuffix; + public const string ReverseOut = EntryPoints.ReverseOut + BStrSuffix; + public const string ReverseInplace = EntryPoints.ReverseInplace + BStrSuffix; + public const string ReverseReplace = EntryPoints.ReverseReplace + BStrSuffix; + } } public partial class Utf16 @@ -185,6 +195,31 @@ public partial class LPStr public static partial void Reverse_Replace_Ref([MarshalAs(UnmanagedType.LPStr)] ref string s); } + public partial class BStr + { + [LibraryImport(NativeExportsNE_Binary, EntryPoint = EntryPoints.BStr.ReturnLength)] + public static partial int ReturnLength([MarshalAs(UnmanagedType.BStr)] string s); + + [LibraryImport(NativeExportsNE_Binary, EntryPoint = EntryPoints.BStr.ReturnLength, StringMarshalling = StringMarshalling.Utf16)] + public static partial int ReturnLength_IgnoreStringMarshalling([MarshalAs(UnmanagedType.BStr)] string s); + + [LibraryImport(NativeExportsNE_Binary, EntryPoint = EntryPoints.BStr.ReverseReturn)] + [return: MarshalAs(UnmanagedType.LPStr)] + public static partial string Reverse_Return([MarshalAs(UnmanagedType.BStr)] string s); + + [LibraryImport(NativeExportsNE_Binary, EntryPoint = EntryPoints.BStr.ReverseOut)] + public static partial void Reverse_Out([MarshalAs(UnmanagedType.BStr)] string s, [MarshalAs(UnmanagedType.BStr)] out string ret); + + [LibraryImport(NativeExportsNE_Binary, EntryPoint = EntryPoints.BStr.ReverseInplace)] + public static partial void Reverse_Ref([MarshalAs(UnmanagedType.BStr)] ref string s); + + [LibraryImport(NativeExportsNE_Binary, EntryPoint = EntryPoints.BStr.ReverseInplace)] + public static partial void Reverse_In([MarshalAs(UnmanagedType.BStr)] in string s); + + [LibraryImport(NativeExportsNE_Binary, EntryPoint = EntryPoints.BStr.ReverseInplace)] + public static partial void Reverse_Replace_Ref([MarshalAs(UnmanagedType.BStr)] ref string s); + } + public partial class StringMarshallingCustomType { public partial class Utf16 @@ -418,6 +453,48 @@ public void AnsiStringByRef(string value) Assert.Equal(expected, refValue); } + [Theory] + [MemberData(nameof(UnicodeStrings))] + public void BStrStringMarshalledAsExpected(string value) + { + int expectedLen = value != null ? value.Length : -1; + + Assert.Equal(expectedLen, NativeExportsNE.BStr.ReturnLength(value)); + Assert.Equal(expectedLen, NativeExportsNE.BStr.ReturnLength_IgnoreStringMarshalling(value)); + } + + [Theory] + [MemberData(nameof(UnicodeStrings))] + public void BStrStringReturn(string value) + { + string expected = ReverseChars(value); + + Assert.Equal(expected, NativeExportsNE.BStr.Reverse_Return(value)); + + string ret; + NativeExportsNE.BStr.Reverse_Out(value, out ret); + Assert.Equal(expected, ret); + } + + [Theory] + [MemberData(nameof(UnicodeStrings))] + public void BStrStringByRef(string value) + { + string refValue = value; + string expected = ReverseChars(value); + + NativeExportsNE.BStr.Reverse_In(in refValue); + Assert.Equal(value, refValue); // Should not be updated when using 'in' + + refValue = value; + NativeExportsNE.BStr.Reverse_Ref(ref refValue); + Assert.Equal(expected, refValue); + + refValue = value; + NativeExportsNE.BStr.Reverse_Replace_Ref(ref refValue); + Assert.Equal(expected, refValue); + } + [Theory] [MemberData(nameof(UnicodeStrings))] public void StringMarshallingCustomType_MarshalledAsExpected(string value) diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs index e62247a20e1b9c..a9da6093b1e7ef 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs @@ -108,9 +108,11 @@ public static IEnumerable CodeSnippetsToCompile() yield return new[] { CodeSnippets.MarshalAsParametersAndModifiers(UnmanagedType.LPTStr) }; yield return new[] { CodeSnippets.MarshalAsParametersAndModifiers(UnmanagedType.LPUTF8Str) }; yield return new[] { CodeSnippets.MarshalAsParametersAndModifiers(UnmanagedType.LPStr) }; + yield return new[] { CodeSnippets.MarshalAsParametersAndModifiers(UnmanagedType.BStr) }; yield return new[] { CodeSnippets.MarshalAsArrayParameterWithNestedMarshalInfo(UnmanagedType.LPWStr) }; yield return new[] { CodeSnippets.MarshalAsArrayParameterWithNestedMarshalInfo(UnmanagedType.LPUTF8Str) }; yield return new[] { CodeSnippets.MarshalAsArrayParameterWithNestedMarshalInfo(UnmanagedType.LPStr) }; + yield return new[] { CodeSnippets.MarshalAsArrayParameterWithNestedMarshalInfo(UnmanagedType.BStr) }; // [In, Out] attributes // By value non-blittable array diff --git a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/NativeExports/Strings.cs b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/NativeExports/Strings.cs index acb72da759ce53..1610a49069eede 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/NativeExports/Strings.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/NativeExports/Strings.cs @@ -26,6 +26,15 @@ public static int ReturnLengthByte(byte* input) return GetLength(input); } + [UnmanagedCallersOnly(EntryPoint = "return_length_bstr")] + public static int ReturnLengthBStr(byte* input) + { + if (input == null) + return -1; + + return GetLengthBStr(input); + } + [UnmanagedCallersOnly(EntryPoint = "reverse_return_ushort")] public static ushort* ReverseReturnUShort(ushort* input) { @@ -38,6 +47,12 @@ public static int ReturnLengthByte(byte* input) return Reverse(input); } + [UnmanagedCallersOnly(EntryPoint = "reverse_return_bstr")] + public static byte* ReverseReturnBStr(byte* input) + { + return ReverseBStr(input); + } + [UnmanagedCallersOnly(EntryPoint = "reverse_out_ushort")] public static void ReverseReturnAsOutUShort(ushort* input, ushort** ret) { @@ -50,6 +65,12 @@ public static void ReverseReturnAsOutByte(byte* input, byte** ret) *ret = Reverse(input); } + [UnmanagedCallersOnly(EntryPoint = "reverse_out_bstr")] + public static void ReverseReturnAsOutBStr(byte* input, byte** ret) + { + *ret = ReverseBStr(input); + } + [UnmanagedCallersOnly(EntryPoint = "reverse_inplace_ref_ushort")] public static void ReverseInPlaceUShort(ushort** refInput) { @@ -69,6 +90,17 @@ public static void ReverseInPlaceByte(byte** refInput) span.Reverse(); } + [UnmanagedCallersOnly(EntryPoint = "reverse_inplace_ref_bstr")] + public static void ReverseInPlaceBStr(byte** refInput) + { + int len = GetLengthBStr(*refInput); + + // Testing of BSTRs is done under the assumption the + // test character input size is 16 bit. + var span = new Span(*refInput, len); + span.Reverse(); + } + [UnmanagedCallersOnly(EntryPoint = "reverse_replace_ref_ushort")] public static void ReverseReplaceRefUShort(ushort** s) { @@ -91,6 +123,17 @@ public static void ReverseReplaceRefByte(byte** s) *s = ret; } + [UnmanagedCallersOnly(EntryPoint = "reverse_replace_ref_bstr")] + public static void ReverseReplaceRefBStr(byte** s) + { + if (*s == null) + return; + + byte* ret = ReverseBStr(*s); + Marshal.FreeBSTR((IntPtr)(*s)); + *s = ret; + } + internal static ushort* Reverse(ushort *s) { if (s == null) @@ -121,6 +164,17 @@ public static void ReverseReplaceRefByte(byte** s) return ret; } + internal static byte* ReverseBStr(byte* s) + { + if (s == null) + return null; + + var arr = Marshal.PtrToStringBSTR((IntPtr)s).ToCharArray(); + Array.Reverse(arr); + var revStr = new string(arr); + return (byte*)Marshal.StringToBSTR(revStr); + } + private static int GetLength(ushort* input) { if (input == null) @@ -150,5 +204,13 @@ private static int GetLength(byte* input) return len; } + + private static int GetLengthBStr(byte* input) + { + if (input == null) + return 0; + + return Marshal.PtrToStringBSTR((IntPtr)input).Length; + } } } From f4cad5ccb60d98f9a80a0f12c74b5442a760f19e Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Wed, 11 May 2022 17:07:48 -0700 Subject: [PATCH 02/11] Update src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/StringTests.cs Wrong marshalling type. --- .../tests/LibraryImportGenerator.Tests/StringTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/StringTests.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/StringTests.cs index 00757b1449a5a9..c164521b080827 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/StringTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/StringTests.cs @@ -204,7 +204,7 @@ public partial class BStr public static partial int ReturnLength_IgnoreStringMarshalling([MarshalAs(UnmanagedType.BStr)] string s); [LibraryImport(NativeExportsNE_Binary, EntryPoint = EntryPoints.BStr.ReverseReturn)] - [return: MarshalAs(UnmanagedType.LPStr)] + [return: MarshalAs(UnmanagedType.BStr)] public static partial string Reverse_Return([MarshalAs(UnmanagedType.BStr)] string s); [LibraryImport(NativeExportsNE_Binary, EntryPoint = EntryPoints.BStr.ReverseOut)] From 5c6e9554059eb1a4cfc6a3a8b5df1b7e36bee32c Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Wed, 11 May 2022 21:27:44 -0700 Subject: [PATCH 03/11] Change to checked addition --- .../InteropServices/Marshalling/BStrStringMarshaller.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs index 93577e321b7ab2..c86a6669ba3a23 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs @@ -50,7 +50,8 @@ public BStrStringMarshaller(string? str, Span buffer) // A caller provided buffer must be at least (lengthInBytes + 6) bytes // in order to be constructed manually. The 6 extra bytes are 4 for byte length and 2 for wide null. - if ((lengthInBytes + 6) > buffer.Length) + int manualBstrNeeds = checked(lengthInBytes + 6); + if (manualBstrNeeds > buffer.Length) { // Use precise byte count when the provided stack-allocated buffer is not sufficient _ptrToFirstChar = (byte*)Marshal.AllocBSTRByteLen((uint)lengthInBytes); @@ -58,7 +59,7 @@ public BStrStringMarshaller(string? str, Span buffer) } else { - // set length and update buffer target + // Set length and update buffer target byte* pBuffer = (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(buffer)); *((uint*)pBuffer) = (uint)lengthInBytes; _ptrToFirstChar = pBuffer + sizeof(uint); From be406c81fd44982cbab7fb78de70d60137f2323c Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Wed, 11 May 2022 21:29:04 -0700 Subject: [PATCH 04/11] Apply suggestions from code review Co-authored-by: Jan Kotas --- .../InteropServices/Marshalling/BStrStringMarshaller.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs index c86a6669ba3a23..8bc078cbe765bc 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs @@ -69,7 +69,8 @@ public BStrStringMarshaller(string? str, Span buffer) Debug.Assert(lengthInBytes == Marshal.SysStringByteLen((IntPtr)_ptrToFirstChar)); // Copy characters from the managed string - Buffer.Memmove(ref *(char*)_ptrToFirstChar, ref str.GetRawStringData(), (nuint)str.Length + 1); + str.CopyTo(new Span(_ptrToFirstChar, str.Length)); + _ptrToFirstChar[str.Length] = '\0'; // null-terminate } /// From d52db688143b0533e3eaa046ac25b6f07f4f104d Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Wed, 11 May 2022 21:29:30 -0700 Subject: [PATCH 05/11] Update src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/StringTests.cs --- .../tests/LibraryImportGenerator.Tests/StringTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/StringTests.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/StringTests.cs index c164521b080827..e23fe087784ddf 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/StringTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/StringTests.cs @@ -216,7 +216,7 @@ public partial class BStr [LibraryImport(NativeExportsNE_Binary, EntryPoint = EntryPoints.BStr.ReverseInplace)] public static partial void Reverse_In([MarshalAs(UnmanagedType.BStr)] in string s); - [LibraryImport(NativeExportsNE_Binary, EntryPoint = EntryPoints.BStr.ReverseInplace)] + [LibraryImport(NativeExportsNE_Binary, EntryPoint = EntryPoints.BStr.ReverseReplace)] public static partial void Reverse_Replace_Ref([MarshalAs(UnmanagedType.BStr)] ref string s); } From 74a27447332f85639d5ac80433d1759d25817cac Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Wed, 11 May 2022 21:55:22 -0700 Subject: [PATCH 06/11] Update src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs --- .../Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs index 8bc078cbe765bc..f9b61a4b4cb323 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs @@ -70,7 +70,7 @@ public BStrStringMarshaller(string? str, Span buffer) // Copy characters from the managed string str.CopyTo(new Span(_ptrToFirstChar, str.Length)); - _ptrToFirstChar[str.Length] = '\0'; // null-terminate + ((char*)_ptrToFirstChar)[str.Length] = '\0'; // null-terminate } /// From 7044848fdfa8ce6622d5db9adf5282891a65d393 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 12 May 2022 08:33:39 -0700 Subject: [PATCH 07/11] Change native type to ushort. --- .../Marshalling/BStrStringMarshaller.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs index f9b61a4b4cb323..a62f7528473c15 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs @@ -15,7 +15,7 @@ namespace System.Runtime.InteropServices.Marshalling Features = CustomTypeMarshallerFeatures.UnmanagedResources | CustomTypeMarshallerFeatures.TwoStageMarshalling | CustomTypeMarshallerFeatures.CallerAllocatedBuffer)] public unsafe ref struct BStrStringMarshaller { - private byte* _ptrToFirstChar; + private ushort* _ptrToFirstChar; private bool _allocated; /// @@ -36,7 +36,7 @@ public BStrStringMarshaller(string? str) /// on the managed heap or it should be pinned. /// /// - public BStrStringMarshaller(string? str, Span buffer) + public BStrStringMarshaller(string? str, Span buffer) { _allocated = false; @@ -54,7 +54,7 @@ public BStrStringMarshaller(string? str, Span buffer) if (manualBstrNeeds > buffer.Length) { // Use precise byte count when the provided stack-allocated buffer is not sufficient - _ptrToFirstChar = (byte*)Marshal.AllocBSTRByteLen((uint)lengthInBytes); + _ptrToFirstChar = (ushort*)Marshal.AllocBSTRByteLen((uint)lengthInBytes); _allocated = true; } else @@ -62,7 +62,7 @@ public BStrStringMarshaller(string? str, Span buffer) // Set length and update buffer target byte* pBuffer = (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(buffer)); *((uint*)pBuffer) = (uint)lengthInBytes; - _ptrToFirstChar = pBuffer + sizeof(uint); + _ptrToFirstChar = (ushort*)(pBuffer + sizeof(uint)); } // Confirm the size is properly set for the allocated BSTR. @@ -70,7 +70,7 @@ public BStrStringMarshaller(string? str, Span buffer) // Copy characters from the managed string str.CopyTo(new Span(_ptrToFirstChar, str.Length)); - ((char*)_ptrToFirstChar)[str.Length] = '\0'; // null-terminate + _ptrToFirstChar[str.Length] = '\0'; // null-terminate } /// @@ -79,7 +79,7 @@ public BStrStringMarshaller(string? str, Span buffer) /// /// /// - public byte* ToNativeValue() => _ptrToFirstChar; + public ushort* ToNativeValue() => _ptrToFirstChar; /// /// Sets the native value representing the string. @@ -88,7 +88,7 @@ public BStrStringMarshaller(string? str, Span buffer) /// /// /// - public void FromNativeValue(byte* value) + public void FromNativeValue(ushort* value) { _ptrToFirstChar = value; _allocated = true; From 4c22b5266ff18e1f40b248c072e88a8385e76022 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 12 May 2022 09:00:09 -0700 Subject: [PATCH 08/11] Update System.Runtime.InteropServices.cs Update ref assembly. --- .../ref/System.Runtime.InteropServices.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 9b8cf7383c5f34..8681fb03854048 100644 --- a/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs +++ b/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs @@ -2086,9 +2086,9 @@ public void FreeNative() { } public unsafe ref struct BStrStringMarshaller { public BStrStringMarshaller(string? str) { } - public BStrStringMarshaller(string? str, System.Span buffer) { } - public byte* ToNativeValue() { throw null; } - public void FromNativeValue(byte* value) { } + public BStrStringMarshaller(string? str, System.Span buffer) { } + public ushort* ToNativeValue() { throw null; } + public void FromNativeValue(ushort* value) { } public string? ToManaged() { throw null; } public void FreeNative() { } } From e0ca27d900d80994cefa1ed0ba2b6e8eda9e643d Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 12 May 2022 09:51:57 -0700 Subject: [PATCH 09/11] Update src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs Co-authored-by: Jan Kotas --- .../Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs index a62f7528473c15..786623eee3d0a1 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs @@ -11,7 +11,7 @@ namespace System.Runtime.InteropServices.Marshalling /// Marshaller for BSTR strings /// [CLSCompliant(false)] - [CustomTypeMarshaller(typeof(string), BufferSize = 0x200, + [CustomTypeMarshaller(typeof(string), BufferSize = 0x100, Features = CustomTypeMarshallerFeatures.UnmanagedResources | CustomTypeMarshallerFeatures.TwoStageMarshalling | CustomTypeMarshallerFeatures.CallerAllocatedBuffer)] public unsafe ref struct BStrStringMarshaller { From 57055aed2e60ea915c39595dfc25a2199864e09e Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 12 May 2022 12:18:13 -0700 Subject: [PATCH 10/11] Convert to use void* for BStr and Utf16 marshallers' native types. --- .../Marshalling/BStrStringMarshaller.cs | 18 ++++++++++-------- .../Marshalling/Utf16StringMarshaller.cs | 13 +++++++------ .../ref/System.Runtime.InteropServices.cs | 10 +++++----- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs index 786623eee3d0a1..b348ab27711a2f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs @@ -15,7 +15,7 @@ namespace System.Runtime.InteropServices.Marshalling Features = CustomTypeMarshallerFeatures.UnmanagedResources | CustomTypeMarshallerFeatures.TwoStageMarshalling | CustomTypeMarshallerFeatures.CallerAllocatedBuffer)] public unsafe ref struct BStrStringMarshaller { - private ushort* _ptrToFirstChar; + private void* _ptrToFirstChar; private bool _allocated; /// @@ -46,6 +46,7 @@ public BStrStringMarshaller(string? str, Span buffer) return; } + ushort* ptrToFirstChar; int lengthInBytes = checked(sizeof(char) * str.Length); // A caller provided buffer must be at least (lengthInBytes + 6) bytes @@ -54,7 +55,7 @@ public BStrStringMarshaller(string? str, Span buffer) if (manualBstrNeeds > buffer.Length) { // Use precise byte count when the provided stack-allocated buffer is not sufficient - _ptrToFirstChar = (ushort*)Marshal.AllocBSTRByteLen((uint)lengthInBytes); + ptrToFirstChar = (ushort*)Marshal.AllocBSTRByteLen((uint)lengthInBytes); _allocated = true; } else @@ -62,15 +63,16 @@ public BStrStringMarshaller(string? str, Span buffer) // Set length and update buffer target byte* pBuffer = (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(buffer)); *((uint*)pBuffer) = (uint)lengthInBytes; - _ptrToFirstChar = (ushort*)(pBuffer + sizeof(uint)); + ptrToFirstChar = (ushort*)(pBuffer + sizeof(uint)); } // Confirm the size is properly set for the allocated BSTR. - Debug.Assert(lengthInBytes == Marshal.SysStringByteLen((IntPtr)_ptrToFirstChar)); + Debug.Assert(lengthInBytes == Marshal.SysStringByteLen((IntPtr)ptrToFirstChar)); // Copy characters from the managed string - str.CopyTo(new Span(_ptrToFirstChar, str.Length)); - _ptrToFirstChar[str.Length] = '\0'; // null-terminate + str.CopyTo(new Span(ptrToFirstChar, str.Length)); + ptrToFirstChar[str.Length] = '\0'; // null-terminate + _ptrToFirstChar = ptrToFirstChar; } /// @@ -79,7 +81,7 @@ public BStrStringMarshaller(string? str, Span buffer) /// /// /// - public ushort* ToNativeValue() => _ptrToFirstChar; + public void* ToNativeValue() => _ptrToFirstChar; /// /// Sets the native value representing the string. @@ -88,7 +90,7 @@ public BStrStringMarshaller(string? str, Span buffer) /// /// /// - public void FromNativeValue(ushort* value) + public void FromNativeValue(void* value) { _ptrToFirstChar = value; _allocated = true; diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/Utf16StringMarshaller.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/Utf16StringMarshaller.cs index 7d77bedc19ff51..64fb70b3127025 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/Utf16StringMarshaller.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/Utf16StringMarshaller.cs @@ -13,7 +13,7 @@ namespace System.Runtime.InteropServices.Marshalling Features = CustomTypeMarshallerFeatures.UnmanagedResources | CustomTypeMarshallerFeatures.TwoStageMarshalling)] public unsafe ref struct Utf16StringMarshaller { - private ushort* _nativeValue; + private void* _nativeValue; /// /// Initializes a new instance of the . @@ -32,10 +32,11 @@ public Utf16StringMarshaller(string? str) } // + 1 for null terminator - _nativeValue = (ushort*)Marshal.AllocCoTaskMem((str.Length + 1) * sizeof(ushort)); + ushort* nativeValue = (ushort*)Marshal.AllocCoTaskMem((str.Length + 1) * sizeof(char)); - str.CopyTo(new Span(_nativeValue, str.Length)); - _nativeValue[str.Length] = '\0'; // null-terminate + str.CopyTo(new Span(nativeValue, str.Length)); + nativeValue[str.Length] = '\0'; // null-terminate + _nativeValue = nativeValue; } /// @@ -44,7 +45,7 @@ public Utf16StringMarshaller(string? str) /// /// /// - public ushort* ToNativeValue() => _nativeValue; + public void* ToNativeValue() => _nativeValue; /// /// Sets the native value representing the string. @@ -53,7 +54,7 @@ public Utf16StringMarshaller(string? str) /// /// /// - public void FromNativeValue(ushort* value) => _nativeValue = value; + public void FromNativeValue(void* value) => _nativeValue = value; /// /// Returns the managed string. 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 8681fb03854048..263fc63b0a6b04 100644 --- a/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs +++ b/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs @@ -2079,7 +2079,7 @@ public void FromNativeValue(byte* value) { } public void FreeNative() { } } [System.CLSCompliant(false)] - [System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerAttribute(typeof(string), BufferSize = 0x200, + [System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerAttribute(typeof(string), BufferSize = 0x100, Features = System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerFeatures.UnmanagedResources | System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerFeatures.CallerAllocatedBuffer | System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerFeatures.TwoStageMarshalling )] @@ -2087,8 +2087,8 @@ public unsafe ref struct BStrStringMarshaller { public BStrStringMarshaller(string? str) { } public BStrStringMarshaller(string? str, System.Span buffer) { } - public ushort* ToNativeValue() { throw null; } - public void FromNativeValue(ushort* value) { } + public void* ToNativeValue() { throw null; } + public void FromNativeValue(void* value) { } public string? ToManaged() { throw null; } public void FreeNative() { } } @@ -2207,8 +2207,8 @@ public void FreeNative() { } public unsafe ref struct Utf16StringMarshaller { public Utf16StringMarshaller(string? str) { } - public ushort* ToNativeValue() { throw null; } - public void FromNativeValue(ushort* value) { } + public void* ToNativeValue() { throw null; } + public void FromNativeValue(void* value) { } public string? ToManaged() { throw null; } public void FreeNative() { } } From 2379671fd2748aa622f885c28ed35460a794cd69 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 19 May 2022 21:32:56 -0700 Subject: [PATCH 11/11] Review feedback --- .../Marshalling/BStrStringMarshaller.cs | 1 + .../ref/System.Runtime.InteropServices.cs | 28 +++++++++---------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs index b348ab27711a2f..ce5a83a69e1abb 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/BStrStringMarshaller.cs @@ -106,6 +106,7 @@ public void FromNativeValue(void* value) { if (_ptrToFirstChar is null) return null; + return Marshal.PtrToStringBSTR((IntPtr)_ptrToFirstChar); } 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 263fc63b0a6b04..6e39fb98a84d22 100644 --- a/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs +++ b/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs @@ -2078,20 +2078,6 @@ public void FromNativeValue(byte* value) { } public string? ToManaged() { throw null; } public void FreeNative() { } } - [System.CLSCompliant(false)] - [System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerAttribute(typeof(string), BufferSize = 0x100, - Features = System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerFeatures.UnmanagedResources - | System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerFeatures.CallerAllocatedBuffer - | System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerFeatures.TwoStageMarshalling )] - public unsafe ref struct BStrStringMarshaller - { - public BStrStringMarshaller(string? str) { } - public BStrStringMarshaller(string? str, System.Span buffer) { } - public void* ToNativeValue() { throw null; } - public void FromNativeValue(void* value) { } - public string? ToManaged() { throw null; } - public void FreeNative() { } - } [System.CLSCompliantAttribute(false)] [System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerAttribute(typeof(System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerAttribute.GenericPlaceholder[]), System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerKind.LinearCollection, BufferSize = 0x200, @@ -2113,6 +2099,20 @@ public void FromNativeValue(byte* value) { } public T[]? ToManaged() { throw null; } public void FreeNative() { } } + [System.CLSCompliant(false)] + [System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerAttribute(typeof(string), BufferSize = 0x100, + Features = System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerFeatures.UnmanagedResources + | System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerFeatures.CallerAllocatedBuffer + | System.Runtime.InteropServices.Marshalling.CustomTypeMarshallerFeatures.TwoStageMarshalling )] + public unsafe ref struct BStrStringMarshaller + { + public BStrStringMarshaller(string? str) { } + public BStrStringMarshaller(string? str, System.Span buffer) { } + public void* ToNativeValue() { throw null; } + public void FromNativeValue(void* value) { } + public string? ToManaged() { throw null; } + public void FreeNative() { } + } [System.AttributeUsageAttribute(System.AttributeTargets.Struct)] public sealed partial class CustomTypeMarshallerAttribute : System.Attribute {