From b6f9dbf426d688af91021357c49d9b17c5bdf525 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Wed, 17 May 2017 05:49:01 -0700 Subject: Switch multicast delegate stub on Windows x64 to use stubs-as-il (#11624) Fixes #11611. The old hand generated assembly path did not work well for structs passed by reference. --- clr.coreclr.props | 4 +- clr.defines.targets | 1 + clrdefinitions.cmake | 4 +- src/mscorlib/src/System/StubHelpers.cs | 2 +- src/vm/comdelegate.cpp | 6 +-- src/vm/dllimport.h | 9 ++++- src/vm/ecalllist.h | 4 +- src/vm/ilstubcache.cpp | 18 +++++---- src/vm/ilstubresolver.cpp | 4 +- src/vm/ilstubresolver.h | 4 +- src/vm/method.hpp | 12 +++--- src/vm/mscorlib.h | 2 +- src/vm/stubhelpers.cpp | 4 +- src/vm/stubhelpers.h | 2 +- src/vm/stubmgr.cpp | 10 ++--- .../coreclr/GitHub_11611/Test11611.csproj | 39 +++++++++++++++++++ .../Regressions/coreclr/GitHub_11611/test11611.cs | 44 ++++++++++++++++++++++ 17 files changed, 135 insertions(+), 34 deletions(-) create mode 100644 tests/src/Regressions/coreclr/GitHub_11611/Test11611.csproj create mode 100644 tests/src/Regressions/coreclr/GitHub_11611/test11611.cs diff --git a/clr.coreclr.props b/clr.coreclr.props index 059fb3a597..e8c2ff8a63 100644 --- a/clr.coreclr.props +++ b/clr.coreclr.props @@ -66,6 +66,7 @@ true true + true true @@ -81,7 +82,8 @@ - true + true + true true true true diff --git a/clr.defines.targets b/clr.defines.targets index df6a409f97..c0643aa013 100644 --- a/clr.defines.targets +++ b/clr.defines.targets @@ -3,6 +3,7 @@ $(DefineConstants);BUILDTYPE_RET $(DefineConstants);FEATURE_APPX $(DefineConstants);FEATURE_ARRAYSTUB_AS_IL + $(DefineConstants);FEATURE_MULTICASTSTUB_AS_IL $(DefineConstants);FEATURE_STUBS_AS_IL $(DefineConstants);FEATURE_CLASSIC_COMINTEROP $(DefineConstants);FEATURE_COMINTEROP diff --git a/clrdefinitions.cmake b/clrdefinitions.cmake index 6db2b24483..7fb6537731 100644 --- a/clrdefinitions.cmake +++ b/clrdefinitions.cmake @@ -81,11 +81,13 @@ endif(WIN32) add_definitions(-DFEATURE_APPDOMAIN_RESOURCE_MONITORING) if(WIN32) add_definitions(-DFEATURE_APPX) - if(CLR_CMAKE_TARGET_ARCH_AMD64 OR CLR_CMAKE_TARGET_ARCH_ARM OR CLR_CMAKE_TARGET_ARCH_ARM64) + if(NOT CLR_CMAKE_TARGET_ARCH_I386) add_definitions(-DFEATURE_ARRAYSTUB_AS_IL) + add_definitions(-DFEATURE_MULTICASTSTUB_AS_IL) endif() else(WIN32) add_definitions(-DFEATURE_ARRAYSTUB_AS_IL) + add_definitions(-DFEATURE_MULTICASTSTUB_AS_IL) endif(WIN32) add_definitions(-DFEATURE_COLLECTIBLE_TYPES) diff --git a/src/mscorlib/src/System/StubHelpers.cs b/src/mscorlib/src/System/StubHelpers.cs index 716fa06a41..e8ddd393f6 100644 --- a/src/mscorlib/src/System/StubHelpers.cs +++ b/src/mscorlib/src/System/StubHelpers.cs @@ -1798,7 +1798,7 @@ namespace System.StubHelpers internal static extern void ArrayTypeCheck(object o, Object[] arr); #endif -#if FEATURE_STUBS_AS_IL +#if FEATURE_MULTICASTSTUB_AS_IL [MethodImplAttribute(MethodImplOptions.InternalCall)] internal static extern void MulticastDebuggerTraceHelper(object o, Int32 count); #endif diff --git a/src/vm/comdelegate.cpp b/src/vm/comdelegate.cpp index 2682c2d32a..aef9adb290 100644 --- a/src/vm/comdelegate.cpp +++ b/src/vm/comdelegate.cpp @@ -2631,7 +2631,7 @@ FCIMPL1(MethodDesc*, COMDelegate::GetInvokeMethod, Object* refThisIn) } FCIMPLEND -#ifdef FEATURE_STUBS_AS_IL +#ifdef FEATURE_MULTICASTSTUB_AS_IL FCIMPL1(PCODE, COMDelegate::GetMulticastInvoke, Object* refThisIn) { FCALL_CONTRACT; @@ -2754,7 +2754,7 @@ FCIMPL1(PCODE, COMDelegate::GetMulticastInvoke, Object* refThisIn) } FCIMPLEND -#else // FEATURE_STUBS_AS_IL +#else // FEATURE_MULTICASTSTUB_AS_IL FCIMPL1(PCODE, COMDelegate::GetMulticastInvoke, Object* refThisIn) { @@ -2813,7 +2813,7 @@ FCIMPL1(PCODE, COMDelegate::GetMulticastInvoke, Object* refThisIn) return pStub->GetEntryPoint(); } FCIMPLEND -#endif // FEATURE_STUBS_AS_IL +#endif // FEATURE_MULTICASTSTUB_AS_IL #ifdef FEATURE_STUBS_AS_IL PCODE COMDelegate::GetSecureInvoke(MethodDesc* pMD) diff --git a/src/vm/dllimport.h b/src/vm/dllimport.h index 1dfb4423cc..c918f58651 100644 --- a/src/vm/dllimport.h +++ b/src/vm/dllimport.h @@ -198,8 +198,10 @@ enum ILStubTypes ILSTUB_ARRAYOP_SET = 0x80000002, ILSTUB_ARRAYOP_ADDRESS = 0x80000004, #endif -#ifdef FEATURE_STUBS_AS_IL +#ifdef FEATURE_MULTICASTSTUB_AS_IL ILSTUB_MULTICASTDELEGATE_INVOKE = 0x80000010, +#endif +#ifdef FEATURE_STUBS_AS_IL ILSTUB_UNBOXINGILSTUB = 0x80000020, ILSTUB_INSTANTIATINGSTUB = 0x80000040, ILSTUB_SECUREDELEGATE_INVOKE = 0x80000080, @@ -232,9 +234,12 @@ inline bool SF_IsArrayOpStub (DWORD dwStubFlags) { LIMITED_METHOD_CONT (dwStubFlags == ILSTUB_ARRAYOP_ADDRESS)); } #endif +#ifdef FEATURE_MULTICASTSTUB_AS_IL +inline bool SF_IsMulticastDelegateStub (DWORD dwStubFlags) { LIMITED_METHOD_CONTRACT; return (dwStubFlags == ILSTUB_MULTICASTDELEGATE_INVOKE); } +#endif + #ifdef FEATURE_STUBS_AS_IL inline bool SF_IsSecureDelegateStub (DWORD dwStubFlags) { LIMITED_METHOD_CONTRACT; return (dwStubFlags == ILSTUB_SECUREDELEGATE_INVOKE); } -inline bool SF_IsMulticastDelegateStub (DWORD dwStubFlags) { LIMITED_METHOD_CONTRACT; return (dwStubFlags == ILSTUB_MULTICASTDELEGATE_INVOKE); } inline bool SF_IsUnboxingILStub (DWORD dwStubFlags) { LIMITED_METHOD_CONTRACT; return (dwStubFlags == ILSTUB_UNBOXINGILSTUB); } inline bool SF_IsInstantiatingStub (DWORD dwStubFlags) { LIMITED_METHOD_CONTRACT; return (dwStubFlags == ILSTUB_INSTANTIATINGSTUB); } #endif diff --git a/src/vm/ecalllist.h b/src/vm/ecalllist.h index 39056a9198..a98396af4a 100644 --- a/src/vm/ecalllist.h +++ b/src/vm/ecalllist.h @@ -1231,9 +1231,9 @@ FCFuncStart(gStubHelperFuncs) #ifdef FEATURE_ARRAYSTUB_AS_IL FCFuncElement("ArrayTypeCheck", StubHelpers::ArrayTypeCheck) #endif //FEATURE_ARRAYSTUB_AS_IL -#ifdef FEATURE_STUBS_AS_IL +#ifdef FEATURE_MULTICASTSTUB_AS_IL FCFuncElement("MulticastDebuggerTraceHelper", StubHelpers::MulticastDebuggerTraceHelper) -#endif //FEATURE_STUBS_AS_IL +#endif //FEATURE_MULTICASTSTUB_AS_IL FCFuncEnd() FCFuncStart(gGCHandleFuncs) diff --git a/src/vm/ilstubcache.cpp b/src/vm/ilstubcache.cpp index ff6bdc0335..95e1845e5b 100644 --- a/src/vm/ilstubcache.cpp +++ b/src/vm/ilstubcache.cpp @@ -216,6 +216,14 @@ MethodDesc* ILStubCache::CreateNewMethodDesc(LoaderHeap* pCreationHeap, MethodTa } else #endif +#ifdef FEATURE_MULTICASTSTUB_AS_IL + if (SF_IsMulticastDelegateStub(dwStubFlags)) + { + pMD->m_dwExtendedFlags |= DynamicMethodDesc::nomdMulticastStub; + pMD->GetILStubResolver()->SetStubType(ILStubResolver::MulticastDelegateStub); + } + else +#endif #ifdef FEATURE_STUBS_AS_IL if (SF_IsSecureDelegateStub(dwStubFlags)) { @@ -223,22 +231,16 @@ MethodDesc* ILStubCache::CreateNewMethodDesc(LoaderHeap* pCreationHeap, MethodTa pMD->GetILStubResolver()->SetStubType(ILStubResolver::SecureDelegateStub); } else - if (SF_IsMulticastDelegateStub(dwStubFlags)) - { - pMD->m_dwExtendedFlags |= DynamicMethodDesc::nomdMulticastStub; - pMD->GetILStubResolver()->SetStubType(ILStubResolver::MulticastDelegateStub); - } - else if (SF_IsUnboxingILStub(dwStubFlags)) { pMD->m_dwExtendedFlags |= DynamicMethodDesc::nomdUnboxingILStub; pMD->GetILStubResolver()->SetStubType(ILStubResolver::UnboxingILStub); - } + } else if (SF_IsInstantiatingStub(dwStubFlags)) { pMD->GetILStubResolver()->SetStubType(ILStubResolver::InstantiatingStub); - } + } else #endif #ifdef FEATURE_COMINTEROP diff --git a/src/vm/ilstubresolver.cpp b/src/vm/ilstubresolver.cpp index 5ba6c8a3b0..913c767990 100644 --- a/src/vm/ilstubresolver.cpp +++ b/src/vm/ilstubresolver.cpp @@ -87,8 +87,10 @@ LPCUTF8 ILStubResolver::GetStubMethodName() #ifdef FEATURE_ARRAYSTUB_AS_IL case ArrayOpStub: return "IL_STUB_Array"; #endif -#ifdef FEATURE_STUBS_AS_IL +#ifdef FEATURE_MULTICASTSTUB_AS_IL case MulticastDelegateStub: return "IL_STUB_MulticastDelegate_Invoke"; +#endif +#ifdef FEATURE_STUBS_AS_IL case UnboxingILStub: return "IL_STUB_UnboxingStub"; case InstantiatingStub: return "IL_STUB_InstantiatingStub"; case SecureDelegateStub: return "IL_STUB_SecureDelegate_Invoke"; diff --git a/src/vm/ilstubresolver.h b/src/vm/ilstubresolver.h index 47181c8a94..8eeb510dbb 100644 --- a/src/vm/ilstubresolver.h +++ b/src/vm/ilstubresolver.h @@ -82,9 +82,11 @@ protected: #ifdef FEATURE_ARRAYSTUB_AS_IL ArrayOpStub, #endif +#ifdef FEATURE_MULTICASTSTUB_AS_IL + MulticastDelegateStub, +#endif #ifdef FEATURE_STUBS_AS_IL SecureDelegateStub, - MulticastDelegateStub, UnboxingILStub, InstantiatingStub, #endif diff --git a/src/vm/method.hpp b/src/vm/method.hpp index 3354e5799a..f65bea5773 100644 --- a/src/vm/method.hpp +++ b/src/vm/method.hpp @@ -2441,17 +2441,19 @@ public: bool IsDelegateCOMStub() { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); return (0 != (m_dwExtendedFlags & nomdDelegateCOMStub)); } bool IsSignatureNeedsRestore() { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); return (0 != (m_dwExtendedFlags & nomdSignatureNeedsRestore)); } bool IsStubNeedsCOMStarted() { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); return (0 != (m_dwExtendedFlags & nomdStubNeedsCOMStarted)); } +#ifdef FEATURE_MULTICASTSTUB_AS_IL + bool IsMulticastStub() { + LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE(IsILStub()); + return !!(m_dwExtendedFlags & nomdMulticastStub); + } +#endif #ifdef FEATURE_STUBS_AS_IL bool IsSecureDelegateStub() { LIMITED_METHOD_DAC_CONTRACT; _ASSERTE(IsILStub()); return !!(m_dwExtendedFlags & nomdSecureDelegateStub); } - bool IsMulticastStub() { - LIMITED_METHOD_DAC_CONTRACT; - _ASSERTE(IsILStub()); - return !!(m_dwExtendedFlags & nomdMulticastStub); - } bool IsUnboxingILStub() { LIMITED_METHOD_DAC_CONTRACT; _ASSERTE(IsILStub()); diff --git a/src/vm/mscorlib.h b/src/vm/mscorlib.h index 338ba1efda..909b8a6870 100644 --- a/src/vm/mscorlib.h +++ b/src/vm/mscorlib.h @@ -1099,7 +1099,7 @@ DEFINE_METHOD(STUBHELPERS, PROFILER_END_TRANSITION_CALLBACK, Profiler DEFINE_METHOD(STUBHELPERS, ARRAY_TYPE_CHECK, ArrayTypeCheck, SM_Obj_ArrObject_RetVoid) #endif -#ifdef FEATURE_STUBS_AS_IL +#ifdef FEATURE_MULTICASTSTUB_AS_IL DEFINE_METHOD(STUBHELPERS, MULTICAST_DEBUGGER_TRACE_HELPER, MulticastDebuggerTraceHelper, SM_Obj_Int_RetVoid) #endif diff --git a/src/vm/stubhelpers.cpp b/src/vm/stubhelpers.cpp index db593c66e9..a1f7abbb38 100644 --- a/src/vm/stubhelpers.cpp +++ b/src/vm/stubhelpers.cpp @@ -1970,11 +1970,11 @@ FCIMPL2(void, StubHelpers::ArrayTypeCheck, Object* element, PtrArray* arr) FCIMPLEND #endif // FEATURE_ARRAYSTUB_AS_IL -#ifdef FEATURE_STUBS_AS_IL +#ifdef FEATURE_MULTICASTSTUB_AS_IL FCIMPL2(void, StubHelpers::MulticastDebuggerTraceHelper, Object* element, INT32 count) { FCALL_CONTRACT; FCUnique(0xa5); } FCIMPLEND -#endif // FEATURE_STUBS_AS_IL +#endif // FEATURE_MULTICASTSTUB_AS_IL diff --git a/src/vm/stubhelpers.h b/src/vm/stubhelpers.h index 31693be38a..70fb529cff 100644 --- a/src/vm/stubhelpers.h +++ b/src/vm/stubhelpers.h @@ -153,7 +153,7 @@ public: static FCDECL2(ReflectClassBaseObject *, WinRTTypeNameConverter__GetTypeFromWinRTTypeName, StringObject *pWinRTTypeNameUNSAFE, CLR_BOOL *pbIsPrimitive); #endif // FEATURE_COMINTEROP -#ifdef FEATURE_STUBS_AS_IL +#ifdef FEATURE_MULTICASTSTUB_AS_IL static FCDECL2(void, MulticastDebuggerTraceHelper, Object*, INT32); #endif }; diff --git a/src/vm/stubmgr.cpp b/src/vm/stubmgr.cpp index e30fb3792e..bc6f47aab5 100644 --- a/src/vm/stubmgr.cpp +++ b/src/vm/stubmgr.cpp @@ -1706,14 +1706,14 @@ BOOL ILStubManager::DoTraceStub(PCODE stubStartAddress, PCODE traceDestination = NULL; -#ifdef FEATURE_STUBS_AS_IL +#ifdef FEATURE_MULTICASTSTUB_AS_IL MethodDesc* pStubMD = ExecutionManager::GetCodeMethodDesc(stubStartAddress); if (pStubMD != NULL && pStubMD->AsDynamicMethodDesc()->IsMulticastStub()) { traceDestination = GetEEFuncEntryPoint(StubHelpers::MulticastDebuggerTraceHelper); } else -#endif // FEATURE_STUBS_AS_IL +#endif // FEATURE_MULTICASTSTUB_AS_IL { // This call is going out to unmanaged code, either through pinvoke or COM interop. traceDestination = stubStartAddress; @@ -1813,7 +1813,7 @@ BOOL ILStubManager::TraceManager(Thread *thread, PCODE stubIP = GetIP(pContext); *pRetAddr = (BYTE *)StubManagerHelpers::GetReturnAddress(pContext); -#ifdef FEATURE_STUBS_AS_IL +#ifdef FEATURE_MULTICASTSTUB_AS_IL if (stubIP == GetEEFuncEntryPoint(StubHelpers::MulticastDebuggerTraceHelper)) { stubIP = (PCODE)*pRetAddr; @@ -1830,7 +1830,7 @@ BOOL ILStubManager::TraceManager(Thread *thread, // See code:ILStubCache.CreateNewMethodDesc for the code that sets flags on stub MDs PCODE target; -#ifdef FEATURE_STUBS_AS_IL +#ifdef FEATURE_MULTICASTSTUB_AS_IL if(pStubMD->IsMulticastStub()) { _ASSERTE(GetIP(pContext) == GetEEFuncEntryPoint(StubHelpers::MulticastDebuggerTraceHelper)); @@ -1859,7 +1859,7 @@ BOOL ILStubManager::TraceManager(Thread *thread, } else -#endif // FEATURE_STUBS_AS_IL +#endif // FEATURE_MULTICASTSTUB_AS_IL if (pStubMD->IsReverseStub()) { if (pStubMD->IsStatic()) diff --git a/tests/src/Regressions/coreclr/GitHub_11611/Test11611.csproj b/tests/src/Regressions/coreclr/GitHub_11611/Test11611.csproj new file mode 100644 index 0000000000..3c7b39f172 --- /dev/null +++ b/tests/src/Regressions/coreclr/GitHub_11611/Test11611.csproj @@ -0,0 +1,39 @@ + + + + + Debug + AnyCPU + 2.0 + {E55A6F8B-B9E3-45CE-88F4-22AE70F606CB} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + true + BuildAndRun + 1 + + + + + + + + + False + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/tests/src/Regressions/coreclr/GitHub_11611/test11611.cs b/tests/src/Regressions/coreclr/GitHub_11611/test11611.cs new file mode 100644 index 0000000000..cceaafef73 --- /dev/null +++ b/tests/src/Regressions/coreclr/GitHub_11611/test11611.cs @@ -0,0 +1,44 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. +using System; + +public struct Data +{ + public long Int; + public object Obj; +} + +public class Test11611 +{ + static bool fail = false; + + public static void handle(Data data, long value) + { + Console.WriteLine("handle( ( i:{0}, o:{1} ), {2} )", data.Int, data.Obj, value); + + if (data.Int != 0) + { + fail = true; + } + + if (data.Obj != null) + { + fail = true; + } + + data.Int = value; + data.Obj = value; + } + + public static int Main() + { + Action handler = handle; + handler += handle; + + var data = new Data(); + handler(data, 123); + + return fail ? -1 : 100; + } +} -- cgit v1.2.3