From 7d8abf5b54a6fd4b922ab6af676d893759c80788 Mon Sep 17 00:00:00 2001 From: Luqun Lou Date: Thu, 11 Oct 2018 12:21:40 -0700 Subject: [PATCH] Enable BSTR Field Marshaller for x-plat (dotnet/coreclr#20264) Commit migrated from https://github.com/dotnet/coreclr/commit/ebe631e4e0e3f1490c3052ad0a3983d9ba3f1a50 --- src/coreclr/src/vm/callingconvention.h | 3 +- src/coreclr/src/vm/fieldmarshaler.cpp | 14 ++-- src/coreclr/src/vm/fieldmarshaler.h | 6 +- src/coreclr/src/vm/methodtable.cpp | 2 +- src/coreclr/src/vm/nsenums.h | 2 +- .../src/Interop/StringMarshalling/BSTR/BSTRTest.cs | 93 +++++++++++++++++++++- .../StringMarshalling/BSTR/BSTRTestNative.cpp | 85 ++++++++++++++++++++ .../Interop/StringMarshalling/BSTR/PinvokeDef.cs | 25 +++++- 8 files changed, 212 insertions(+), 18 deletions(-) diff --git a/src/coreclr/src/vm/callingconvention.h b/src/coreclr/src/vm/callingconvention.h index c219d70..59553fb 100644 --- a/src/coreclr/src/vm/callingconvention.h +++ b/src/coreclr/src/vm/callingconvention.h @@ -1016,7 +1016,7 @@ int ArgIteratorTemplate::GetNextOffset() case ELEMENT_TYPE_VALUETYPE: { #ifdef UNIX_AMD64_ABI - MethodTable *pMT = m_argTypeHandle.AsMethodTable(); + MethodTable *pMT = m_argTypeHandle.GetMethodTable(); if (pMT->IsRegPassedStruct()) { EEClass* eeClass = pMT->GetClass(); @@ -1061,7 +1061,6 @@ int ArgIteratorTemplate::GetNextOffset() // Set the register counts to indicate that this argument will not be passed in registers cFPRegs = 0; cGenRegs = 0; - #else // UNIX_AMD64_ABI argSize = sizeof(TADDR); #endif // UNIX_AMD64_ABI diff --git a/src/coreclr/src/vm/fieldmarshaler.cpp b/src/coreclr/src/vm/fieldmarshaler.cpp index 2f93130..ef7dc9d 100644 --- a/src/coreclr/src/vm/fieldmarshaler.cpp +++ b/src/coreclr/src/vm/fieldmarshaler.cpp @@ -795,11 +795,12 @@ do \ // We no longer support Win9x so LPTSTR always maps to a Unicode string. INITFIELDMARSHALER(NFT_STRINGUNI, FieldMarshaler_StringUni, ()); break; -#ifdef FEATURE_COMINTEROP + case NATIVE_TYPE_BSTR: INITFIELDMARSHALER(NFT_BSTR, FieldMarshaler_BSTR, ()); break; +#ifdef FEATURE_COMINTEROP case NATIVE_TYPE_HSTRING: INITFIELDMARSHALER(NFT_HSTRING, FieldMarshaler_HSTRING, ()); break; @@ -2297,9 +2298,6 @@ VOID FmtValueTypeUpdateCLR(LPVOID pProtectedManagedData, MethodTable *pMT, BYTE } } - -#ifdef FEATURE_COMINTEROP - //======================================================================= // BSTR <--> System.String // See FieldMarshaler for details. @@ -2398,14 +2396,13 @@ VOID FieldMarshaler_BSTR::DestroyNativeImpl(LPVOID pNativeValue) const if (pBSTR) { - _ASSERTE (GetModuleHandleA("oleaut32.dll") != NULL); - // BSTR has been created, which means oleaut32 should have been loaded. - // Delay load will not fail. + // BSTR has been created, Delay load will not fail. CONTRACT_VIOLATION(ThrowsViolation); SysFreeString(pBSTR); } } +#ifdef FEATURE_COMINTEROP //=========================================================================================== // Windows.Foundation.IReference'1<-- System.Nullable'1 // @@ -4683,10 +4680,10 @@ VOID NStructFieldTypeToString(FieldMarshaler* pFM, SString& strNStructFieldType) // All other NStruct Field Types which do not require special handling. switch (cls) { -#ifdef FEATURE_COMINTEROP case NFT_BSTR: strRetVal = W("BSTR"); break; +#ifdef FEATURE_COMINTEROP case NFT_HSTRING: strRetVal = W("HSTRING"); break; @@ -4837,6 +4834,7 @@ VOID NStructFieldTypeToString(FieldMarshaler* pFM, SString& strNStructFieldType) case NFT_DECIMAL: rettype ((FieldMarshaler_Decimal*)this)->name##Impl args; break; \ case NFT_SAFEHANDLE: rettype ((FieldMarshaler_SafeHandle*)this)->name##Impl args; break; \ case NFT_CRITICALHANDLE: rettype ((FieldMarshaler_CriticalHandle*)this)->name##Impl args; break; \ + case NFT_BSTR: rettype ((FieldMarshaler_BSTR*)this)->name##Impl args; break; \ case NFT_ILLEGAL: rettype ((FieldMarshaler_Illegal*)this)->name##Impl args; break; \ default: UNREACHABLE_MSG("unexpected type of FieldMarshaler"); break; \ } \ diff --git a/src/coreclr/src/vm/fieldmarshaler.h b/src/coreclr/src/vm/fieldmarshaler.h index 934f058..e3390c7 100644 --- a/src/coreclr/src/vm/fieldmarshaler.h +++ b/src/coreclr/src/vm/fieldmarshaler.h @@ -54,9 +54,9 @@ class FieldMarshaler_WinBool; class FieldMarshaler_CBool; class FieldMarshaler_Decimal; class FieldMarshaler_Date; +class FieldMarshaler_BSTR; #ifdef FEATURE_COMINTEROP class FieldMarshaler_SafeArray; -class FieldMarshaler_BSTR; class FieldMarshaler_HSTRING; class FieldMarshaler_Interface; class FieldMarshaler_Variant; @@ -466,9 +466,6 @@ protected: }; - -#ifdef FEATURE_COMINTEROP - //======================================================================= // BSTR <--> System.String //======================================================================= @@ -484,6 +481,7 @@ public: COPY_TO_IMPL_BASE_STRUCT_ONLY() }; +#ifdef FEATURE_COMINTEROP //======================================================================= // HSTRING <--> System.String //======================================================================= diff --git a/src/coreclr/src/vm/methodtable.cpp b/src/coreclr/src/vm/methodtable.cpp index c3441d7..c97dd10 100644 --- a/src/coreclr/src/vm/methodtable.cpp +++ b/src/coreclr/src/vm/methodtable.cpp @@ -2901,7 +2901,6 @@ bool MethodTable::ClassifyEightBytesWithNativeLayout(SystemVStructRegisterPassin switch (cls) { #ifdef FEATURE_COMINTEROP - case NFT_BSTR: case NFT_HSTRING: case NFT_VARIANT: case NFT_VARIANTBOOL: @@ -2919,6 +2918,7 @@ bool MethodTable::ClassifyEightBytesWithNativeLayout(SystemVStructRegisterPassin case NFT_DELEGATE: case NFT_SAFEHANDLE: case NFT_CRITICALHANDLE: + case NFT_BSTR: fieldClassificationType = SystemVClassificationTypeInteger; break; diff --git a/src/coreclr/src/vm/nsenums.h b/src/coreclr/src/vm/nsenums.h index f5facc7..fbf4d1f 100644 --- a/src/coreclr/src/vm/nsenums.h +++ b/src/coreclr/src/vm/nsenums.h @@ -54,10 +54,10 @@ DEFINE_NFT(NFT_INTERFACE, sizeof(IUnknown*), false) DEFINE_NFT(NFT_SAFEHANDLE, sizeof(LPVOID), false) DEFINE_NFT(NFT_CRITICALHANDLE, sizeof(LPVOID), false) +DEFINE_NFT(NFT_BSTR, sizeof(BSTR), false) #ifdef FEATURE_COMINTEROP DEFINE_NFT(NFT_SAFEARRAY, 0, false) -DEFINE_NFT(NFT_BSTR, sizeof(BSTR), false) DEFINE_NFT(NFT_HSTRING, sizeof(HSTRING), true) DEFINE_NFT(NFT_VARIANT, sizeof(VARIANT), false) DEFINE_NFT(NFT_VARIANTBOOL, sizeof(VARIANT_BOOL), false) diff --git a/src/coreclr/tests/src/Interop/StringMarshalling/BSTR/BSTRTest.cs b/src/coreclr/tests/src/Interop/StringMarshalling/BSTR/BSTRTest.cs index fbd0b72..f12c4bf 100644 --- a/src/coreclr/tests/src/Interop/StringMarshalling/BSTR/BSTRTest.cs +++ b/src/coreclr/tests/src/Interop/StringMarshalling/BSTR/BSTRTest.cs @@ -7,13 +7,14 @@ using System; using System.Reflection; using System.Text; using NativeDefs; +using System.Diagnostics; class Test { #region "Report Failure" static int fails = 0; //record the fail numbers - // Overload methods for reportfailure + // Overload methods for reportfailure static int ReportFailure(string s) { Console.WriteLine(" === Fail:" + s); @@ -94,6 +95,91 @@ class Test } #endregion + + #region "Struct" + static void TestStructIn() + { + string strManaged = "Managed\0String\0"; + Person person = new Person(); + person.age = 12; + person.name = strManaged; + if (!PInvokeDef.Marshal_Struct_In(person)) + { + ReportFailure("Method PInvokeDef.Marshal_Struct_In[Managed Side],The native return false"); + } + } + + static void TestStructPointerInOut() + { + string strManaged = "Managed\0String\0"; + Person person = new Person(); + person.age = 12; + person.name = strManaged; + + if (!PInvokeDef.MarshalPointer_Struct_InOut(ref person)) + { + ReportFailure("Method PInvokeDef.MarshalPointer_Struct_InOut[Managed Side],The native return false"); + } + + if(person.age != 21 || person.name != " Native") + { + ReportFailure("Method PInvokeDef.MarshalPointer_Struct_InOut[Managed Side],The Return struct is wrong"); + } + } + + static bool Call_Struct_Delegate_In(Person person) + { + if (person.age != 21 || person.name != " Native") + { + ReportFailure("Method Call_Struct_Delegate_In[Managed Side], The native passing in struct is incorrect"); + return false; + } + + return true; + } + + static void TestRPInvokeStructIn() + { + DelMarshal_Struct_In d = new DelMarshal_Struct_In(Call_Struct_Delegate_In); + if (!PInvokeDef.RPInvoke_DelMarshal_Struct_In(d)) + { + ReportFailure("Method PInvokeDef.RPInvoke_DelMarshal_Struct_In[Managed Side],The Return value is wrong"); + } + } + + static bool Call_StructPointer_Delegate_InOut(ref Person person) + { + if (person.age != 21 || person.name != " Native") + { + ReportFailure("Method Call_StructPointer_Delegate_InOut[Managed Side], The native passing in struct is incorrect"); + return false; + } + + string strManaged = "Managed\0String\0"; + Person managedPerson = new Person(); + managedPerson.age = 12; + managedPerson.name = strManaged; + + person = managedPerson; + return true; + } + + static void TestRPInvokeStructPointerInOut() + { + DelMarshalPointer_Struct_InOut d = new DelMarshalPointer_Struct_InOut(Call_StructPointer_Delegate_InOut); + if(!PInvokeDef.RPInvoke_DelMarshalStructPointer_InOut(d)) + { + ReportFailure("Method PInvokeDef.RPInvoke_DelMarshalStructPointer_InOut[Managed Side], The Return value is wrong"); + } + } + static void TestStruct() + { + TestStructIn(); + TestStructPointerInOut(); + TestRPInvokeStructIn(); + TestRPInvokeStructPointerInOut(); + } + #endregion public static int Main(string[] args) { @@ -195,6 +281,11 @@ class Test } #endregion + + #region "Struct" + TestStruct(); + #endregion + return ExitTest(); } } diff --git a/src/coreclr/tests/src/Interop/StringMarshalling/BSTR/BSTRTestNative.cpp b/src/coreclr/tests/src/Interop/StringMarshalling/BSTR/BSTRTestNative.cpp index 9a4668b..26f78c7 100644 --- a/src/coreclr/tests/src/Interop/StringMarshalling/BSTR/BSTRTestNative.cpp +++ b/src/coreclr/tests/src/Interop/StringMarshalling/BSTR/BSTRTestNative.cpp @@ -162,3 +162,88 @@ extern "C" DLL_EXPORT BOOL __stdcall ReverseP_MarshalStrB_InOut(Test_Del_Marsha } return TRUE; } + +typedef struct Person Person; +struct Person{ + int age; + int _padding; + BSTR name; +}; + +extern "C" DLL_EXPORT BOOL Marshal_Struct_In(Person person) +{ + if (person.age != 12) + { + printf("Error in Marshal_Struct_In, The value for age field is incorrect\n"); + return FALSE; + } + + size_t len = TP_SysStringByteLen(person.name); + if (len != lenstrManaged || memcmp(person.name, strManaged, lenstrManaged) != 0) + { + printf("Error in Marshal_Struct_In, The value for name field is incorrect\n"); + return FALSE; + } + + return TRUE; +} + +extern "C" DLL_EXPORT BOOL MarshalPointer_Struct_InOut(Person* person) +{ + if (person->age != 12) + { + printf("Error in MarshalPointer_Struct_InOut, The value for age field is incorrect\n"); + return FALSE; + } + + size_t len = TP_SysStringByteLen(person->name); + if (len != lenstrManaged || memcmp(person->name, strManaged, lenstrManaged) != 0) + { + printf("Error in MarshalPointer_Struct_InOut, The value for name field is incorrect\n"); + return FALSE; + } + + person->age = 21; + person->name = TP_SysAllocString(strNative); + return TRUE; +} + +typedef BOOL (* Test_DelMarshal_Struct_In)(Person person); +extern "C" DLL_EXPORT BOOL RPInvoke_DelMarshal_Struct_In(Test_DelMarshal_Struct_In d) +{ + Person * pPerson = (Person *)TP_CoTaskMemAlloc(sizeof(Person)); + pPerson->age = 21; + pPerson->name = TP_SysAllocString(strNative); + + if (!d(*pPerson)) + { + printf("Error in RPInvoke_DelMarshal_Struct_In, Managed delegate return false\n"); + return FALSE; + } + + return TRUE; +} + +typedef BOOL (* Test_DelMarshalPointer_Struct_InOut)(Person * person); +extern "C" DLL_EXPORT BOOL RPInvoke_DelMarshalStructPointer_InOut(Test_DelMarshalPointer_Struct_InOut d) +{ + Person * pPerson = (Person *)TP_CoTaskMemAlloc(sizeof(Person)); + pPerson->age = 21; + pPerson->name = TP_SysAllocString(strNative); + + if (!d(pPerson)) + { + printf("Error in RPInvoke_DelMarshalStructPointer_InOut,The delegate return false\n"); + return FALSE; + } + + size_t len = TP_SysStringByteLen(pPerson->name); + if (len != lenstrManaged || memcmp(pPerson->name, strManaged, lenstrManaged) != 0) + { + printf("Error in RPInvoke_DelMarshalStructPointer_InOut,The value for name field for pPerson is incorrect\n"); + return FALSE; + } + + return TRUE; +} + diff --git a/src/coreclr/tests/src/Interop/StringMarshalling/BSTR/PinvokeDef.cs b/src/coreclr/tests/src/Interop/StringMarshalling/BSTR/PinvokeDef.cs index 241eeff..e1d3056 100644 --- a/src/coreclr/tests/src/Interop/StringMarshalling/BSTR/PinvokeDef.cs +++ b/src/coreclr/tests/src/Interop/StringMarshalling/BSTR/PinvokeDef.cs @@ -9,7 +9,14 @@ using System.Text; namespace NativeDefs { - + public struct Person + { + public int age; + public int _padding; + [MarshalAs(UnmanagedType.BStr)] + public string name; + } + [return: MarshalAs(UnmanagedType.BStr)] public delegate string Del_MarshalPointer_Out([MarshalAs(UnmanagedType.BStr)] out string s); @@ -24,6 +31,10 @@ namespace NativeDefs [return: MarshalAs(UnmanagedType.BStr)] public delegate string DelMarshal_InOut([MarshalAs(UnmanagedType.BStr)][In, Out]string s); + public delegate bool DelMarshal_Struct_In(Person person); + + public delegate bool DelMarshalPointer_Struct_InOut(ref Person person); + public static class PInvokeDef { public const string NativeBinaryName = "BSTRTestNative"; @@ -49,5 +60,17 @@ namespace NativeDefs [DllImport(NativeBinaryName, CallingConvention = CallingConvention.StdCall)] public static extern bool RPinvoke_DelMarshalPointer_Out(DelMarshalPointer_Out d); + + [DllImport(NativeBinaryName)] + public static extern bool Marshal_Struct_In(Person person); + + [DllImport(NativeBinaryName)] + public static extern bool MarshalPointer_Struct_InOut(ref Person person); + + [DllImport(NativeBinaryName)] + public static extern bool RPInvoke_DelMarshal_Struct_In(DelMarshal_Struct_In d); + + [DllImport(NativeBinaryName)] + public static extern bool RPInvoke_DelMarshalStructPointer_InOut(DelMarshalPointer_Struct_InOut d); } } -- 2.7.4