From 0164d68225520d2292de144e69d8187ef9768be5 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 3 Apr 2019 14:08:06 -0700 Subject: [PATCH] Change Auto charset to mean UTF-8 off-Windows (#23664) Match Mono's behavior by changing the Auto character set to mean UTF-8 on non-Windows platforms (new behavior) and UCS-2/UTF-16 on Windows (current behavior). Fixes #23464 Fixes dotnet/corefx#32442 Impact of breaking change: It is highly unlikely that anyone is actively using current behavior since it is inconsistent with Mono and doesn't match any native system APIs on non-Windows platforms (they're all UTF-8 based). We will need to update our documentation to reflect this updated behavior. --- .../System/Runtime/InteropServices/Marshal.Unix.cs | 22 +++++++++++- .../Runtime/InteropServices/Marshal.Windows.cs | 22 +++++++++++- .../System/Runtime/InteropServices/Marshal.cs | 42 +++++++++++----------- src/vm/dllimport.cpp | 20 ++++++++--- src/vm/methodtablebuilder.cpp | 10 +++++- tests/CoreFX/CoreFX.issues.json | 8 +++++ .../PInvoke/ExactSpelling/ExactSpellingTest.cs | 15 ++++++++ .../PInvoke/MarshalStructAsLayoutSeq.cs | 20 ++++++++++- .../PInvoke/MarshalStructAsParamDLL.cpp | 10 ++++++ .../PInvoke/MarshalStructAsParamDLL.h | 9 +++++ .../Interop/StructMarshalling/PInvoke/Struct.cs | 5 +++ 11 files changed, 154 insertions(+), 29 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/Marshal.Unix.cs b/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/Marshal.Unix.cs index 1f049bd..08892b1 100644 --- a/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/Marshal.Unix.cs +++ b/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/Marshal.Unix.cs @@ -9,6 +9,26 @@ namespace System.Runtime.InteropServices { public static partial class Marshal { + public static string PtrToStringAuto(IntPtr ptr, int len) + { + return PtrToStringUTF8(ptr, len); + } + + public static string PtrToStringAuto(IntPtr ptr) + { + return PtrToStringUTF8(ptr); + } + + public static IntPtr StringToHGlobalAuto(string s) + { + return StringToHGlobalUTF8(s); + } + + public static IntPtr StringToCoTaskMemAuto(string s) + { + return StringToCoTaskMemUTF8(s); + } + private static int GetSystemMaxDBCSCharSize() => 3; private static bool IsWin32Atom(IntPtr ptr) => false; @@ -29,4 +49,4 @@ namespace System.Runtime.InteropServices return convertedBytes; } } -} \ No newline at end of file +} diff --git a/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/Marshal.Windows.cs b/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/Marshal.Windows.cs index 5a44e49..c9e1baf 100644 --- a/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/Marshal.Windows.cs +++ b/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/Marshal.Windows.cs @@ -8,6 +8,26 @@ namespace System.Runtime.InteropServices { public static partial class Marshal { + public static string PtrToStringAuto(IntPtr ptr, int len) + { + return PtrToStringUni(ptr, len); + } + + public static string PtrToStringAuto(IntPtr ptr) + { + return PtrToStringUni(ptr); + } + + public static IntPtr StringToHGlobalAuto(string s) + { + return StringToHGlobalUni(s); + } + + public static IntPtr StringToCoTaskMemAuto(string s) + { + return StringToCoTaskMemUni(s); + } + private static unsafe int GetSystemMaxDBCSCharSize() { Interop.Kernel32.CPINFO cpInfo = default; @@ -63,4 +83,4 @@ namespace System.Runtime.InteropServices return nb; } } -} \ No newline at end of file +} diff --git a/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/Marshal.cs b/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/Marshal.cs index a1c2f17..99ab101 100644 --- a/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/Marshal.cs +++ b/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/Marshal.cs @@ -87,18 +87,6 @@ namespace System.Runtime.InteropServices return new string((char*)ptr, 0, len); } - public static string PtrToStringAuto(IntPtr ptr, int len) - { - // Ansi platforms are no longer supported - return PtrToStringUni(ptr, len); - } - - public static string PtrToStringAuto(IntPtr ptr) - { - // Ansi platforms are no longer supported - return PtrToStringUni(ptr); - } - public static unsafe string PtrToStringUTF8(IntPtr ptr) { if (ptr == IntPtr.Zero) @@ -726,10 +714,28 @@ namespace System.Runtime.InteropServices return hglobal; } - public static IntPtr StringToHGlobalAuto(string s) + private static unsafe IntPtr StringToHGlobalUTF8(string s) { - // Ansi platforms are no longer supported - return StringToHGlobalUni(s); + if (s == null) + { + return IntPtr.Zero; + } + + int nb = Encoding.UTF8.GetMaxByteCount(s.Length); + + IntPtr pMem = AllocHGlobal(nb + 1); + + int nbWritten; + byte* pbMem = (byte*)pMem; + + fixed (char* firstChar = s) + { + nbWritten = Encoding.UTF8.GetBytes(firstChar, s.Length, pbMem, nb); + } + + pbMem[nbWritten] = 0; + + return pMem; } public static unsafe IntPtr StringToCoTaskMemUni(string s) @@ -780,12 +786,6 @@ namespace System.Runtime.InteropServices return pMem; } - public static IntPtr StringToCoTaskMemAuto(string s) - { - // Ansi platforms are no longer supported - return StringToCoTaskMemUni(s); - } - public static unsafe IntPtr StringToCoTaskMemAnsi(string s) { if (s == null) diff --git a/src/vm/dllimport.cpp b/src/vm/dllimport.cpp index 5fa9ac4..252ec56 100644 --- a/src/vm/dllimport.cpp +++ b/src/vm/dllimport.cpp @@ -2965,8 +2965,14 @@ PInvokeStaticSigInfo::PInvokeStaticSigInfo(MethodDesc* pMD, ThrowOnError throwOn case nltAnsi: nlt = nltAnsi; break; case nltUnicode: - case nltAuto: // Since Win9x isn't supported anymore, nltAuto always represents unicode strings. nlt = nltUnicode; break; + case nltAuto: +#ifdef PLATFORM_WINDOWS + nlt = nltUnicode; +#else + nlt = nltAnsi; // We don't have a utf8 charset in metadata yet, but ANSI == UTF-8 off-Windows +#endif + break; default: hr = E_FAIL; goto ErrExit; } @@ -3088,7 +3094,6 @@ void PInvokeStaticSigInfo::DllImportInit(MethodDesc* pMD, LPCUTF8 *ppLibName, LP if (mappingFlags & pmNoMangle) SetLinkFlags ((CorNativeLinkFlags)(GetLinkFlags() | nlfNoMangle)); - // XXX Tue 07/19/2005 // Keep in sync with the handling of CorNativeLinkType in // PInvokeStaticSigInfo::PInvokeStaticSigInfo. @@ -3098,11 +3103,18 @@ void PInvokeStaticSigInfo::DllImportInit(MethodDesc* pMD, LPCUTF8 *ppLibName, LP { SetCharSet (nltAnsi); } - else if (charSetMask == pmCharSetUnicode || charSetMask == pmCharSetAuto) + else if (charSetMask == pmCharSetUnicode) { - // Since Win9x isn't supported anymore, pmCharSetAuto always represents unicode strings. SetCharSet (nltUnicode); } + else if (charSetMask == pmCharSetAuto) + { +#ifdef PLATFORM_WINDOWS + SetCharSet(nltUnicode); +#else + SetCharSet(nltAnsi); // We don't have a utf8 charset in metadata yet, but ANSI == UTF-8 off-Windows +#endif + } else { SetError(IDS_EE_NDIRECT_BADNATL); diff --git a/src/vm/methodtablebuilder.cpp b/src/vm/methodtablebuilder.cpp index d4ce5b0..b946026 100644 --- a/src/vm/methodtablebuilder.cpp +++ b/src/vm/methodtablebuilder.cpp @@ -11913,10 +11913,18 @@ BOOL HasLayoutMetadata(Assembly* pAssembly, IMDInternalImport* pInternalImport, { *pNLTType = nltAnsi; } - else if (IsTdUnicodeClass(clFlags) || IsTdAutoClass(clFlags)) + else if (IsTdUnicodeClass(clFlags)) { *pNLTType = nltUnicode; } + else if (IsTdAutoClass(clFlags)) + { +#ifdef PLATFORM_WINDOWS + *pNLTType = nltUnicode; +#else + *pNLTType = nltAnsi; // We don't have a utf8 charset in metadata yet, but ANSI == UTF-8 off-Windows +#endif + } else { pAssembly->ThrowTypeLoadException(pInternalImport, cl, IDS_CLASSLOAD_BADFORMAT); diff --git a/tests/CoreFX/CoreFX.issues.json b/tests/CoreFX/CoreFX.issues.json index 28eb929..faa6e4b 100644 --- a/tests/CoreFX/CoreFX.issues.json +++ b/tests/CoreFX/CoreFX.issues.json @@ -1057,6 +1057,14 @@ { "name": "System.Runtime.InteropServices.Tests.ArrayWithOffsetTests.Ctor_Array_Offset", "reason": "outdated" + }, + { + "name": "System.Runtime.InteropServices.Tests.StringToCoTaskMemAutoTests.StringToCoTaskMemAuto_NonNullString_Roundtrips", + "reason": "outdated" + }, + { + "name": "System.Runtime.InteropServices.Tests.StringToHGlobalAutoTests.StringToHGlobalAuto_NonNullString_Roundtrips", + "reason": "https://github.com/dotnet/coreclr/pull/23664" } ] } diff --git a/tests/src/Interop/PInvoke/ExactSpelling/ExactSpellingTest.cs b/tests/src/Interop/PInvoke/ExactSpelling/ExactSpellingTest.cs index 7616e6d..f2e1a01 100644 --- a/tests/src/Interop/PInvoke/ExactSpelling/ExactSpellingTest.cs +++ b/tests/src/Interop/PInvoke/ExactSpelling/ExactSpellingTest.cs @@ -37,6 +37,12 @@ class ExactSpellingTest public static extern int MarshalPointer_Int_InOut2([In, Out] ref int intValue); } + class Auto + { + [DllImport("ExactSpellingNative", CharSet = CharSet.Auto, ExactSpelling = false)] + public static extern int Marshal_Int_InOut2([In, Out] int intValue); + } + public static int Main(string[] args) { int failures = 0; @@ -87,6 +93,15 @@ class ExactSpellingTest int int8 = intManaged; int intRet8 = Ansi.MarshalPointer_Int_InOut2(ref int8); failures += Verify(intReturnAnsi, intNative, intRet8, int8); + + Console.WriteLine("Method Auto.Marshal_Int_InOut: ExactSpelling = false. Verify CharSet.Auto behavior per-platform."); + int int9 = intManaged; + int intRet9 = Auto.Marshal_Int_InOut2(int9); +#if PLATFORM_WINDOWS + failures += Verify(intReturnUnicode, intManaged, intRet9, int9); +#else + failures += Verify(intReturnAnsi, intManaged, intRet9, int9); +#endif return 100 + failures; } diff --git a/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsLayoutSeq.cs b/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsLayoutSeq.cs index 22bb03c..6eb77a4 100644 --- a/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsLayoutSeq.cs +++ b/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsLayoutSeq.cs @@ -23,6 +23,7 @@ public class Managed S9Id, IncludeOuterIntegerStructSequentialId, S11Id, + AutoStringId, IntWithInnerSequentialId, SequentialWrapperId, SequentialDoubleWrapperId, @@ -317,6 +318,9 @@ public class Managed static extern bool MarshalStructAsParam_AsSeqByValSequentialAggregateSequentialWrapper(AggregateSequentialWrapper wrapper); [DllImport("MarshalStructAsParam")] + static extern int GetStringLength(AutoString str); + + [DllImport("MarshalStructAsParam")] static extern HFA GetHFA(float f1, float f2, float f3, float f4); [DllImport("MarshalStructAsParam")] @@ -539,7 +543,20 @@ public class Managed failures++; } break; - + case StructID.AutoStringId: + Console.WriteLine("\tCalling GetStringLength..."); + { + AutoString autoStr = new AutoString + { + str = "Auto CharSet test string" + }; + int actual = GetStringLength(autoStr); + if (actual != autoStr.str.Length) + { + Console.WriteLine($"\tFAILED! Managed to Native failed in GetStringLength. Expected {autoStr.str.Length}, got {actual}"); + } + } + break; case StructID.IntWithInnerSequentialId: IntWithInnerSequential intWithInnerSeq = new IntWithInnerSequential { @@ -2217,6 +2234,7 @@ public class Managed MarshalStructAsParam_AsSeqByVal(StructID.S9Id); MarshalStructAsParam_AsSeqByVal(StructID.IncludeOuterIntegerStructSequentialId); MarshalStructAsParam_AsSeqByVal(StructID.S11Id); + MarshalStructAsParam_AsSeqByVal(StructID.AutoStringId); MarshalStructAsParam_AsSeqByVal(StructID.IntWithInnerSequentialId); MarshalStructAsParam_AsSeqByVal(StructID.SequentialWrapperId); MarshalStructAsParam_AsSeqByVal(StructID.SequentialDoubleWrapperId); diff --git a/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsParamDLL.cpp b/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsParamDLL.cpp index be3f046..9de3f78 100644 --- a/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsParamDLL.cpp +++ b/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsParamDLL.cpp @@ -1178,6 +1178,16 @@ extern "C" DLL_EXPORT BOOL STDMETHODCALLTYPE MarshalStructAsParam_AsExpByRefOutL return TRUE; } +extern "C" DLL_EXPORT int GetStringLength(AutoString str) +{ +#ifdef _WIN32 + return (int)wcslen(str.str); +#else + return (int)strlen(str.str); +#endif +} + + extern "C" DLL_EXPORT HFA STDMETHODCALLTYPE GetHFA(float f1, float f2, float f3, float f4) { return {f1, f2, f3, f4}; diff --git a/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsParamDLL.h b/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsParamDLL.h index 494a8ea..dcedc46 100644 --- a/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsParamDLL.h +++ b/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsParamDLL.h @@ -819,6 +819,15 @@ bool IsCorrectLongStructPack16Explicit(LongStructPack16Explicit* p) return true; } +struct AutoString +{ +#ifdef _WIN32 + LPCWSTR str; +#else + LPCSTR str; +#endif +}; + struct HFA { float f1; diff --git a/tests/src/Interop/StructMarshalling/PInvoke/Struct.cs b/tests/src/Interop/StructMarshalling/PInvoke/Struct.cs index af9d762..7ade4c3 100644 --- a/tests/src/Interop/StructMarshalling/PInvoke/Struct.cs +++ b/tests/src/Interop/StructMarshalling/PInvoke/Struct.cs @@ -298,6 +298,11 @@ public struct LongStructPack16Explicit public long l2; } +[StructLayout(LayoutKind.Sequential, CharSet = CharSet.Auto)] +public struct AutoString +{ + public string str; +} [StructLayout(LayoutKind.Sequential)] public struct HFA -- 2.7.4