From 3d9dcd088af26d0628cb8b98a0e4acc30867f732 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Fri, 12 Jun 2020 19:37:00 -0700 Subject: [PATCH 1/8] Update UnmanagedCallersOnlyAttribute API surface --- src/coreclr/src/vm/customattribute.h | 2 +- src/coreclr/src/vm/dllimportcallback.cpp | 94 +++++++++++++++---- .../UnmanagedCallersOnlyTest.cs | 4 +- .../System/Globalization/CalendarData.Nls.cs | 4 +- .../System/Globalization/CultureData.Nls.cs | 6 +- .../InteropServices/CallingConvention.cs | 2 +- .../UnmanagedCallersOnlyAttribute.cs | 6 +- .../ref/System.Runtime.InteropServices.cs | 2 +- 8 files changed, 93 insertions(+), 27 deletions(-) diff --git a/src/coreclr/src/vm/customattribute.h b/src/coreclr/src/vm/customattribute.h index 4aa3f227a121f..dffedfbacab23 100644 --- a/src/coreclr/src/vm/customattribute.h +++ b/src/coreclr/src/vm/customattribute.h @@ -105,7 +105,6 @@ class Attribute CaNamedArgArrayREF* ppCustomAttributeNamedArguments, AssemblyBaseObject* pAssemblyUNSAFE); -private: static HRESULT ParseAttributeArgumentValues( void* pCa, INT32 cCa, @@ -116,6 +115,7 @@ class Attribute COUNT_T cNamedArgs, DomainAssembly* pDomainAssembly); +private: static HRESULT ParseCaValue( CustomAttributeParser &ca, CaValue* pCaArg, diff --git a/src/coreclr/src/vm/dllimportcallback.cpp b/src/coreclr/src/vm/dllimportcallback.cpp index be989d1f6097d..a1d0f076fad70 100644 --- a/src/coreclr/src/vm/dllimportcallback.cpp +++ b/src/coreclr/src/vm/dllimportcallback.cpp @@ -23,6 +23,7 @@ #include "appdomain.inl" #include "callingconvention.h" #include "customattribute.h" +#include "typeparse.h" #ifndef CROSSGEN_COMPILE @@ -626,32 +627,93 @@ VOID UMThunkMarshInfo::SetUpForUnmanagedCallersOnly() BYTE* pData = NULL; LONG cData = 0; - CorPinvokeMap callConv = (CorPinvokeMap)0; + bool nativeCallableInternalData = false; HRESULT hr = pMD->GetCustomAttribute(WellKnownAttribute::UnmanagedCallersOnly, (const VOID **)(&pData), (ULONG *)&cData); if (hr == S_FALSE) + { hr = pMD->GetCustomAttribute(WellKnownAttribute::NativeCallableInternal, (const VOID **)(&pData), (ULONG *)&cData); + nativeCallableInternalData = SUCCEEDED(hr); + } IfFailThrow(hr); _ASSERTE(cData > 0); CustomAttributeParser ca(pData, cData); + // UnmanagedCallersOnly has two optional named arguments CallingConvention and EntryPoint. CaNamedArg namedArgs[2]; - CaTypeCtor caType(SERIALIZATION_TYPE_STRING); - // First, the void constructor. - IfFailThrow(ParseKnownCaArgs(ca, NULL, 0)); - - // Now the optional named properties - namedArgs[0].InitI4FieldEnum("CallingConvention", "System.Runtime.InteropServices.CallingConvention", (ULONG)callConv); - namedArgs[1].Init("EntryPoint", SERIALIZATION_TYPE_STRING, caType); - IfFailThrow(ParseKnownCaNamedArgs(ca, namedArgs, lengthof(namedArgs))); - - callConv = (CorPinvokeMap)(namedArgs[0].val.u4 << 8); - // Let UMThunkMarshalInfo choose the default if calling convension not definied. - if (namedArgs[0].val.type.tag != SERIALIZATION_TYPE_UNDEFINED) - m_callConv = (UINT16)callConv; + + // For the UnmanagedCallersOnly scenario. + CaType caCallConvs; + + // Define the optional named properties + if (nativeCallableInternalData) + { + namedArgs[0].InitI4FieldEnum("CallingConvention", "System.Runtime.InteropServices.CallingConvention", (ULONG)(CorPinvokeMap)0); + } + else + { + caCallConvs.Init(SERIALIZATION_TYPE_SZARRAY, SERIALIZATION_TYPE_TYPE, SERIALIZATION_TYPE_UNDEFINED, NULL, 0); + namedArgs[0].Init("CallConvs", SERIALIZATION_TYPE_SZARRAY, caCallConvs); + } + + CaTypeCtor caEntryPoint(SERIALIZATION_TYPE_STRING); + namedArgs[1].Init("EntryPoint", SERIALIZATION_TYPE_STRING, caEntryPoint); + + InlineFactory, 4> caValueArrayFactory; + DomainAssembly* domainAssembly = pMD->GetLoaderModule()->GetDomainAssembly(); + IfFailThrow(Attribute::ParseAttributeArgumentValues( + pData, + cData, + &caValueArrayFactory, + NULL, + 0, + namedArgs, + lengthof(namedArgs), + domainAssembly)); + + // If the value isn't defined, then return without setting anything. + if (namedArgs[0].val.type.tag == SERIALIZATION_TYPE_UNDEFINED) + return; + + CorPinvokeMap callConvLocal = (CorPinvokeMap)0; + if (nativeCallableInternalData) + { + callConvLocal = (CorPinvokeMap)(namedArgs[0].val.u4 << 8); + } + else + { + // Set WinAPI as the default + callConvLocal = CorPinvokeMap::pmCallConvWinapi; + + CaValue* arrayOfTypes = &namedArgs[0].val; + for (ULONG i = 0; i < arrayOfTypes->arr.length; i++) + { + CaValue& typeNameValue = arrayOfTypes->arr[i]; + + StackSString typeFQName(SString::Utf8, typeNameValue.str.pStr, typeNameValue.str.cbStr); + if (typeFQName.BeginsWith(W("System.Runtime.CompilerServices.CallConvCdecl"))) + { + callConvLocal = CorPinvokeMap::pmCallConvCdecl; + } + else if (typeFQName.BeginsWith(W("System.Runtime.CompilerServices.CallConvStdcall"))) + { + callConvLocal = CorPinvokeMap::pmCallConvStdcall; + } + else if (typeFQName.BeginsWith(W("System.Runtime.CompilerServices.CallConvFastcall"))) + { + callConvLocal = CorPinvokeMap::pmCallConvFastcall; + } + else if (typeFQName.BeginsWith(W("System.Runtime.CompilerServices.CallConvThiscall"))) + { + callConvLocal = CorPinvokeMap::pmCallConvThiscall; + } + } + } + + m_callConv = (UINT16)callConvLocal; } // Compiles an unmanaged to managed thunk for the given signature. @@ -776,7 +838,7 @@ Stub *UMThunkMarshInfo::CompileNExportThunk(LoaderHeap *pLoaderHeap, PInvokeStat stubInfo.m_cbSrcStack = static_cast(m_cbActualArgSize); stubInfo.m_cbDstStack = nStackBytes; - if (pSigInfo->GetCallConv() == pmCallConvCdecl) + if (m_callConv == pmCallConvCdecl) { // caller pop m_cbRetPop = 0; @@ -786,7 +848,7 @@ Stub *UMThunkMarshInfo::CompileNExportThunk(LoaderHeap *pLoaderHeap, PInvokeStat // callee pop m_cbRetPop = static_cast(m_cbActualArgSize); - if (pSigInfo->GetCallConv() == pmCallConvThiscall) + if (m_callConv == pmCallConvThiscall) { stubInfo.m_wFlags |= umtmlThisCall; if (argit.HasRetBuffArg()) diff --git a/src/coreclr/tests/src/Interop/UnmanagedCallersOnly/UnmanagedCallersOnlyTest.cs b/src/coreclr/tests/src/Interop/UnmanagedCallersOnly/UnmanagedCallersOnlyTest.cs index 6c0acc1eab94e..0991d6143a416 100644 --- a/src/coreclr/tests/src/Interop/UnmanagedCallersOnly/UnmanagedCallersOnlyTest.cs +++ b/src/coreclr/tests/src/Interop/UnmanagedCallersOnly/UnmanagedCallersOnlyTest.cs @@ -539,7 +539,7 @@ .locals init (native int V_0) testNativeMethod(); } - [UnmanagedCallersOnly(CallingConvention = CallingConvention.StdCall)] + [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvStdcall) })] public static int CallbackViaUnmanagedCalli(int val) { return DoubleImpl(val); @@ -587,7 +587,7 @@ .locals init (native int V_0) Assert.AreEqual(expected, testNativeMethod()); } - [UnmanagedCallersOnly(CallingConvention = CallingConvention.StdCall)] + [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvStdcall) })] public static int CallbackViaUnmanagedCalliThrows(int val) { throw new Exception() { HResult = CallbackThrowsErrorCode }; diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/CalendarData.Nls.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/CalendarData.Nls.cs index 0d80d68bceea5..ad637cb7f4915 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Globalization/CalendarData.Nls.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/CalendarData.Nls.cs @@ -272,7 +272,7 @@ private struct EnumData } // EnumCalendarInfoExEx callback itself. - // [UnmanagedCallersOnly(CallingConvention = CallingConvention.StdCall)] + // [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvStdcall) })] private static unsafe Interop.BOOL EnumCalendarInfoCallback(char* lpCalendarInfoString, uint calendar, IntPtr pReserved, void* lParam) { ref EnumData context = ref Unsafe.As(ref *(byte*)lParam); @@ -425,7 +425,7 @@ public struct NlsEnumCalendarsData public List calendars; // list of calendars found so far } - // [UnmanagedCallersOnly(CallingConvention = CallingConvention.StdCall)] + // [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvStdcall) })] private static unsafe Interop.BOOL EnumCalendarsCallback(char* lpCalendarInfoString, uint calendar, IntPtr reserved, void* lParam) { ref NlsEnumCalendarsData context = ref Unsafe.As(ref *(byte*)lParam); diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Nls.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Nls.cs index e8d4d05c974b9..30708c46d3f0f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Nls.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Nls.cs @@ -359,7 +359,7 @@ private struct EnumLocaleData } // EnumSystemLocaleEx callback. - // [UnmanagedCallersOnly(CallingConvention = CallingConvention.StdCall)] + // [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvStdcall) })] private static unsafe Interop.BOOL EnumSystemLocalesProc(char* lpLocaleString, uint flags, void* contextHandle) { ref EnumLocaleData context = ref Unsafe.As(ref *(byte*)contextHandle); @@ -382,7 +382,7 @@ private static unsafe Interop.BOOL EnumSystemLocalesProc(char* lpLocaleString, u } // EnumSystemLocaleEx callback. - // [UnmanagedCallersOnly(CallingConvention = CallingConvention.StdCall)] + // [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvStdcall) })] private static unsafe Interop.BOOL EnumAllSystemLocalesProc(char* lpLocaleString, uint flags, void* contextHandle) { ref EnumData context = ref Unsafe.As(ref *(byte*)contextHandle); @@ -404,7 +404,7 @@ private struct EnumData } // EnumTimeFormatsEx callback itself. - // [UnmanagedCallersOnly(CallingConvention = CallingConvention.StdCall)] + // [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvStdcall) })] private static unsafe Interop.BOOL EnumTimeCallback(char* lpTimeFormatString, void* lParam) { ref EnumData context = ref Unsafe.As(ref *(byte*)lParam); diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/CallingConvention.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/CallingConvention.cs index 1b515eda81910..6a406020a8940 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/CallingConvention.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/CallingConvention.cs @@ -4,7 +4,7 @@ namespace System.Runtime.InteropServices { - // Used for the CallingConvention named argument to the DllImport and UnmanagedCallersOnly attribute + // Used for the CallingConvention named argument to the DllImport attribute public enum CallingConvention { Winapi = 1, diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/UnmanagedCallersOnlyAttribute.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/UnmanagedCallersOnlyAttribute.cs index 4acf93de99475..5b2479c6ab023 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/UnmanagedCallersOnlyAttribute.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/UnmanagedCallersOnlyAttribute.cs @@ -25,7 +25,11 @@ public UnmanagedCallersOnlyAttribute() /// /// Optional. If omitted, the runtime will use the default platform calling convention. /// - public CallingConvention CallingConvention; + /// + /// Supplied types must be from the official "System.Runtime.CompilerServices" namespace and + /// be of the form "CallConvXXX". + /// + public Type[]? CallConvs; /// /// Optional. If omitted, no named export is emitted during compilation. 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 96e3af5a431ca..43e5b5d2f0804 100644 --- a/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs +++ b/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs @@ -1010,7 +1010,7 @@ public static void RegisterForMarshalling(ComWrappers instance) { } public sealed class UnmanagedCallersOnlyAttribute : System.Attribute { public UnmanagedCallersOnlyAttribute() { } - public System.Runtime.InteropServices.CallingConvention CallingConvention; + public System.Type[]? CallConvs; public string? EntryPoint; } } From 081e626c75e689a6b04e0da76005551083a08793 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Fri, 12 Jun 2020 20:58:35 -0700 Subject: [PATCH 2/8] Update comments and block unknown calling convention types for UnmanagedCallersOnlyAttribute. --- src/coreclr/src/vm/dllimportcallback.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/coreclr/src/vm/dllimportcallback.cpp b/src/coreclr/src/vm/dllimportcallback.cpp index a1d0f076fad70..cc5c7910dff0c 100644 --- a/src/coreclr/src/vm/dllimportcallback.cpp +++ b/src/coreclr/src/vm/dllimportcallback.cpp @@ -622,7 +622,7 @@ VOID UMThunkMarshInfo::SetUpForUnmanagedCallersOnly() MethodDesc* pMD = GetMethod(); _ASSERTE(pMD != NULL && pMD->HasUnmanagedCallersOnlyAttribute()); - // Validate UnmanagedCallersOnlyAttribute usage + // Validate usage COMDelegate::ThrowIfInvalidUnmanagedCallersOnlyUsage(pMD); BYTE* pData = NULL; @@ -642,13 +642,14 @@ VOID UMThunkMarshInfo::SetUpForUnmanagedCallersOnly() CustomAttributeParser ca(pData, cData); - // UnmanagedCallersOnly has two optional named arguments CallingConvention and EntryPoint. + // UnmanagedCallersOnly and NativeCallableInternal each + // have optional named arguments. CaNamedArg namedArgs[2]; // For the UnmanagedCallersOnly scenario. CaType caCallConvs; - // Define the optional named properties + // Define attribute specific optional named properties if (nativeCallableInternalData) { namedArgs[0].InitI4FieldEnum("CallingConvention", "System.Runtime.InteropServices.CallingConvention", (ULONG)(CorPinvokeMap)0); @@ -659,6 +660,7 @@ VOID UMThunkMarshInfo::SetUpForUnmanagedCallersOnly() namedArgs[0].Init("CallConvs", SERIALIZATION_TYPE_SZARRAY, caCallConvs); } + // Define common optional named properties CaTypeCtor caEntryPoint(SERIALIZATION_TYPE_STRING); namedArgs[1].Init("EntryPoint", SERIALIZATION_TYPE_STRING, caEntryPoint); @@ -710,6 +712,10 @@ VOID UMThunkMarshInfo::SetUpForUnmanagedCallersOnly() { callConvLocal = CorPinvokeMap::pmCallConvThiscall; } + else + { + COMPlusThrow(kArgumentException, W("Argument_UnknownUnmanagedCallConv")); + } } } From e32484f0c86e5092ec18b3a04b365da79371e6a3 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Fri, 12 Jun 2020 21:33:08 -0700 Subject: [PATCH 3/8] Change exception type and create new message string. --- src/coreclr/src/vm/dllimportcallback.cpp | 2 +- .../System.Private.CoreLib/src/Resources/Strings.resx | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/coreclr/src/vm/dllimportcallback.cpp b/src/coreclr/src/vm/dllimportcallback.cpp index cc5c7910dff0c..1c5d522442c80 100644 --- a/src/coreclr/src/vm/dllimportcallback.cpp +++ b/src/coreclr/src/vm/dllimportcallback.cpp @@ -714,7 +714,7 @@ VOID UMThunkMarshInfo::SetUpForUnmanagedCallersOnly() } else { - COMPlusThrow(kArgumentException, W("Argument_UnknownUnmanagedCallConv")); + COMPlusThrow(kNotSupportedException, W("NotSupported_UnknownUnmanagedCallConv")); } } } diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx index c5c6a8603e8df..8253b070b5352 100644 --- a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx +++ b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx @@ -1537,6 +1537,9 @@ Unknown unmanaged calling convention for function signature. + + Unknown unmanaged calling convention for function signature. + The UnmanagedMemoryAccessor capacity and offset would wrap around the high end of the address space. From 748efe6f4191507a3613bb96cca6262bd003b21a Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Fri, 12 Jun 2020 21:51:09 -0700 Subject: [PATCH 4/8] Use existing exception message. --- src/coreclr/src/vm/dllimportcallback.cpp | 2 +- .../System.Private.CoreLib/src/Resources/Strings.resx | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/coreclr/src/vm/dllimportcallback.cpp b/src/coreclr/src/vm/dllimportcallback.cpp index 1c5d522442c80..f2878001340a6 100644 --- a/src/coreclr/src/vm/dllimportcallback.cpp +++ b/src/coreclr/src/vm/dllimportcallback.cpp @@ -714,7 +714,7 @@ VOID UMThunkMarshInfo::SetUpForUnmanagedCallersOnly() } else { - COMPlusThrow(kNotSupportedException, W("NotSupported_UnknownUnmanagedCallConv")); + COMPlusThrow(kTypeLoadException, IDS_INVALID_PINVOKE_CALLCONV); } } } diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx index 8253b070b5352..c5c6a8603e8df 100644 --- a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx +++ b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx @@ -1537,9 +1537,6 @@ Unknown unmanaged calling convention for function signature. - - Unknown unmanaged calling convention for function signature. - The UnmanagedMemoryAccessor capacity and offset would wrap around the high end of the address space. From 17079bcd0666d5d14fa75dbb7b6dfd2d7c285edd Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Sun, 14 Jun 2020 22:21:47 -0700 Subject: [PATCH 5/8] UnmanagedCallersOnlyAttribute should not be inheritable. --- .../Runtime/InteropServices/UnmanagedCallersOnlyAttribute.cs | 2 +- .../ref/System.Runtime.InteropServices.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/UnmanagedCallersOnlyAttribute.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/UnmanagedCallersOnlyAttribute.cs index 5b2479c6ab023..8193657a7b6aa 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/UnmanagedCallersOnlyAttribute.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/UnmanagedCallersOnlyAttribute.cs @@ -15,7 +15,7 @@ namespace System.Runtime.InteropServices /// * Must not be called from managed code. /// * Must only have blittable arguments. /// - [AttributeUsage(AttributeTargets.Method)] + [AttributeUsage(AttributeTargets.Method, Inherited = false)] public sealed class UnmanagedCallersOnlyAttribute : Attribute { public UnmanagedCallersOnlyAttribute() 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 43e5b5d2f0804..f47880cfda5de 100644 --- a/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs +++ b/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs @@ -1006,7 +1006,7 @@ public static void RegisterForTrackerSupport(ComWrappers instance) { } public static void RegisterForMarshalling(ComWrappers instance) { } protected static void GetIUnknownImpl(out System.IntPtr fpQueryInterface, out System.IntPtr fpAddRef, out System.IntPtr fpRelease) { throw null; } } - [System.AttributeUsageAttribute(System.AttributeTargets.Method)] + [System.AttributeUsageAttribute(System.AttributeTargets.Method, Inherited = false)] public sealed class UnmanagedCallersOnlyAttribute : System.Attribute { public UnmanagedCallersOnlyAttribute() { } From eca5ee0498c5f385e589d3358291d765741a4854 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Fri, 19 Jun 2020 20:54:22 -0700 Subject: [PATCH 6/8] Ignore calling conventions that don't apply. --- src/coreclr/src/vm/dllimportcallback.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/coreclr/src/vm/dllimportcallback.cpp b/src/coreclr/src/vm/dllimportcallback.cpp index f2878001340a6..a78582b1f2c15 100644 --- a/src/coreclr/src/vm/dllimportcallback.cpp +++ b/src/coreclr/src/vm/dllimportcallback.cpp @@ -712,10 +712,6 @@ VOID UMThunkMarshInfo::SetUpForUnmanagedCallersOnly() { callConvLocal = CorPinvokeMap::pmCallConvThiscall; } - else - { - COMPlusThrow(kTypeLoadException, IDS_INVALID_PINVOKE_CALLCONV); - } } } From 8668ae3c3bf66b52596af522967a79c6b6eb624f Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Sat, 20 Jun 2020 11:28:45 -0700 Subject: [PATCH 7/8] Remove allocation and length compute cost for string constant compare. --- src/coreclr/src/vm/dllimportcallback.cpp | 27 +++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/src/coreclr/src/vm/dllimportcallback.cpp b/src/coreclr/src/vm/dllimportcallback.cpp index a78582b1f2c15..5998d7a9fe439 100644 --- a/src/coreclr/src/vm/dllimportcallback.cpp +++ b/src/coreclr/src/vm/dllimportcallback.cpp @@ -615,6 +615,20 @@ VOID UMEntryThunk::CompileUMThunkWorker(UMThunkStubInfo *pInfo, pcpusl->X86EmitNearJump(pEnableRejoin); } +namespace +{ + // Templated function to compute if a char string begins with a constant string. + template + bool BeginsWith(ULONG s1Len, const char* s1, const char (&s2)[S2LEN]) + { + WRAPPER_NO_CONTRACT; + + ULONG s2Len = (ULONG)S2LEN - 1; // Remove null + ULONG minLen = s1Len < s2Len ? s1Len : s2Len; + return (0 == strncmp(s1, s2, minLen)); + } +} + VOID UMThunkMarshInfo::SetUpForUnmanagedCallersOnly() { STANDARD_VM_CONTRACT; @@ -695,20 +709,23 @@ VOID UMThunkMarshInfo::SetUpForUnmanagedCallersOnly() { CaValue& typeNameValue = arrayOfTypes->arr[i]; - StackSString typeFQName(SString::Utf8, typeNameValue.str.pStr, typeNameValue.str.cbStr); - if (typeFQName.BeginsWith(W("System.Runtime.CompilerServices.CallConvCdecl"))) + // According to ECMA-335, type name strings are UTF-8. Since we are + // looking for type names that are equivalent in ASCII and UTF-8, + // using a const char constant is acceptable. Type name strings are + // in Fully Qualified form, so we include the ',' delimiter. + if (BeginsWith(typeNameValue.str.cbStr, typeNameValue.str.pStr, "System.Runtime.CompilerServices.CallConvCdecl,")) { callConvLocal = CorPinvokeMap::pmCallConvCdecl; } - else if (typeFQName.BeginsWith(W("System.Runtime.CompilerServices.CallConvStdcall"))) + else if (BeginsWith(typeNameValue.str.cbStr, typeNameValue.str.pStr, "System.Runtime.CompilerServices.CallConvStdcall,")) { callConvLocal = CorPinvokeMap::pmCallConvStdcall; } - else if (typeFQName.BeginsWith(W("System.Runtime.CompilerServices.CallConvFastcall"))) + else if (BeginsWith(typeNameValue.str.cbStr, typeNameValue.str.pStr, "System.Runtime.CompilerServices.CallConvFastcall,")) { callConvLocal = CorPinvokeMap::pmCallConvFastcall; } - else if (typeFQName.BeginsWith(W("System.Runtime.CompilerServices.CallConvThiscall"))) + else if (BeginsWith(typeNameValue.str.cbStr, typeNameValue.str.pStr, "System.Runtime.CompilerServices.CallConvThiscall,")) { callConvLocal = CorPinvokeMap::pmCallConvThiscall; } From b384c5350a08f3c23cf1820842755b1aaf09ef39 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Sat, 20 Jun 2020 12:45:54 -0700 Subject: [PATCH 8/8] Fix logical error in generic BeginWith() function. --- src/coreclr/src/vm/dllimportcallback.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/coreclr/src/vm/dllimportcallback.cpp b/src/coreclr/src/vm/dllimportcallback.cpp index 5998d7a9fe439..633d07fa803f2 100644 --- a/src/coreclr/src/vm/dllimportcallback.cpp +++ b/src/coreclr/src/vm/dllimportcallback.cpp @@ -624,8 +624,10 @@ namespace WRAPPER_NO_CONTRACT; ULONG s2Len = (ULONG)S2LEN - 1; // Remove null - ULONG minLen = s1Len < s2Len ? s1Len : s2Len; - return (0 == strncmp(s1, s2, minLen)); + if (s1Len < s2Len) + return false; + + return (0 == strncmp(s1, s2, s2Len)); } }