diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 7f2d3cf4c81653..f130b93a49df6e 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -657,7 +657,7 @@ class ILStubState : public StubState DWORD dwMethodDescLocalNum = (DWORD)-1; // Notify the profiler of call out of the runtime - if (!SF_IsReverseCOMStub(m_dwStubFlags) && !SF_IsStructMarshalStub(m_dwStubFlags) && CORProfilerTrackTransitions()) + if (!SF_IsReverseCOMStub(m_dwStubFlags) && !SF_IsReverseDelegateStub(m_dwStubFlags) && !SF_IsStructMarshalStub(m_dwStubFlags) && CORProfilerTrackTransitions()) { dwMethodDescLocalNum = m_slIL.EmitProfilerBeginTransitionCallback(pcsDispatch, m_dwStubFlags); _ASSERTE(dwMethodDescLocalNum != (DWORD)-1); @@ -2251,18 +2251,8 @@ DWORD NDirectStubLinker::EmitProfilerBeginTransitionCallback(ILCodeStream* pcsEm // in StubHelpers::ProfilerEnterCallback(). if (SF_IsDelegateStub(dwStubFlags)) { - if (SF_IsForwardStub(dwStubFlags)) - { - pcsEmit->EmitLoadThis(); - } - else - { - EmitLoadStubContext(pcsEmit, dwStubFlags); // load UMEntryThunk* - pcsEmit->EmitLDC(offsetof(UMEntryThunk, m_pObjectHandle)); - pcsEmit->EmitADD(); - pcsEmit->EmitLDIND_I(); // get OBJECTHANDLE - pcsEmit->EmitLDIND_REF(); // get Delegate object - } + _ASSERTE(SF_IsForwardStub(dwStubFlags)); + pcsEmit->EmitLoadThis(); } else { @@ -2282,15 +2272,8 @@ void NDirectStubLinker::EmitProfilerEndTransitionCallback(ILCodeStream* pcsEmit, STANDARD_VM_CONTRACT; pcsEmit->EmitLDLOC(dwMethodDescLocalNum); - if (SF_IsReverseStub(dwStubFlags)) - { - // we use a null pThread to indicate reverse interop - pcsEmit->EmitLoadNullPtr(); - } - else - { - pcsEmit->EmitLDLOC(GetThreadLocalNum()); - } + _ASSERTE(SF_IsForwardStub(dwStubFlags)); + pcsEmit->EmitLDLOC(GetThreadLocalNum()); pcsEmit->EmitCALL(METHOD__STUBHELPERS__PROFILER_END_TRANSITION_CALLBACK, 2, 0); } #endif // PROFILING_SUPPPORTED diff --git a/src/coreclr/vm/stubhelpers.cpp b/src/coreclr/vm/stubhelpers.cpp index 4fb4e9ff8c656d..0ba31d449f538d 100644 --- a/src/coreclr/vm/stubhelpers.cpp +++ b/src/coreclr/vm/stubhelpers.cpp @@ -569,17 +569,6 @@ FCIMPL3(SIZE_T, StubHelpers::ProfilerBeginTransitionCallback, SIZE_T pSecretPara DELEGATEREF dref = (DELEGATEREF)ObjectToOBJECTREF(unsafe_pThis); HELPER_METHOD_FRAME_BEGIN_RET_1(dref); - bool fReverseInterop = false; - - if (NULL == pThread) - { - // This is our signal for the reverse interop cases. - fReverseInterop = true; - pThread = GET_THREAD(); - // the secret param in this casee is the UMEntryThunk - pRealMD = ((UMEntryThunk*)pSecretParam)->GetMethod(); - } - else if (pSecretParam == 0) { // Secret param is null. This is the calli pinvoke case or the unmanaged delegate case. @@ -611,14 +600,7 @@ FCIMPL3(SIZE_T, StubHelpers::ProfilerBeginTransitionCallback, SIZE_T pSecretPara { GCX_PREEMP_THREAD_EXISTS(pThread); - if (fReverseInterop) - { - ProfilerUnmanagedToManagedTransitionMD(pRealMD, COR_PRF_TRANSITION_CALL); - } - else - { - ProfilerManagedToUnmanagedTransitionMD(pRealMD, COR_PRF_TRANSITION_CALL); - } + ProfilerManagedToUnmanagedTransitionMD(pRealMD, COR_PRF_TRANSITION_CALL); } HELPER_METHOD_FRAME_END(); @@ -646,25 +628,9 @@ FCIMPL2(void, StubHelpers::ProfilerEndTransitionCallback, MethodDesc* pRealMD, T // and the transition requires us to set up a HMF. HELPER_METHOD_FRAME_BEGIN_0(); { - bool fReverseInterop = false; - - if (NULL == pThread) - { - // if pThread is null, we are doing reverse interop - pThread = GET_THREAD(); - fReverseInterop = true; - } - GCX_PREEMP_THREAD_EXISTS(pThread); - if (fReverseInterop) - { - ProfilerManagedToUnmanagedTransitionMD(pRealMD, COR_PRF_TRANSITION_RETURN); - } - else - { - ProfilerUnmanagedToManagedTransitionMD(pRealMD, COR_PRF_TRANSITION_RETURN); - } + ProfilerUnmanagedToManagedTransitionMD(pRealMD, COR_PRF_TRANSITION_RETURN); } HELPER_METHOD_FRAME_END(); diff --git a/src/tests/profiler/native/transitions/transitions.cpp b/src/tests/profiler/native/transitions/transitions.cpp index b8c5c3c39c4d27..13984e0837cb2e 100644 --- a/src/tests/profiler/native/transitions/transitions.cpp +++ b/src/tests/profiler/native/transitions/transitions.cpp @@ -23,6 +23,20 @@ HRESULT Transitions::Initialize(IUnknown* pICorProfilerInfoUnk) return hr; } + constexpr ULONG bufferSize = 1024; + ULONG envVarLen = 0; + WCHAR envVar[bufferSize]; + if (FAILED(hr = pCorProfilerInfo->GetEnvironmentVariable(WCHAR("PInvoke_Transition_Expected_Name"), bufferSize, &envVarLen, envVar))) + { + return E_FAIL; + } + expectedPinvokeName = envVar; + if (FAILED(hr = pCorProfilerInfo->GetEnvironmentVariable(WCHAR("ReversePInvoke_Transition_Expected_Name"), bufferSize, &envVarLen, envVar))) + { + return E_FAIL; + } + expectedReversePInvokeName = envVar; + return S_OK; } @@ -55,6 +69,7 @@ extern "C" EXPORT void STDMETHODCALLTYPE DoPInvoke(int(*callback)(int), int i) printf("PInvoke received i=%d\n", callback(i)); } + HRESULT Transitions::UnmanagedToManagedTransition(FunctionID functionID, COR_PRF_TRANSITION_REASON reason) { SHUTDOWNGUARD(); @@ -62,6 +77,11 @@ HRESULT Transitions::UnmanagedToManagedTransition(FunctionID functionID, COR_PRF TransitionInstance* inst; if (FunctionIsTargetFunction(functionID, &inst)) { + if (inst->UnmanagedToManaged != NO_TRANSITION) + { + // Report a failure for duplicate transitions. + _failures++; + } inst->UnmanagedToManaged = reason; } @@ -75,6 +95,11 @@ HRESULT Transitions::ManagedToUnmanagedTransition(FunctionID functionID, COR_PRF TransitionInstance* inst; if (FunctionIsTargetFunction(functionID, &inst)) { + if (inst->ManagedToUnmanaged != NO_TRANSITION) + { + // Report a failure for duplicate transitions. + _failures++; + } inst->ManagedToUnmanaged = reason; } @@ -85,11 +110,11 @@ bool Transitions::FunctionIsTargetFunction(FunctionID functionID, TransitionInst { String name = GetFunctionIDName(functionID); - if (name == WCHAR("DoPInvoke")) + if (name == expectedPinvokeName) { *inst = &_pinvoke; } - else if (name == WCHAR("DoReversePInvoke")) + else if (name == expectedReversePInvokeName) { *inst = &_reversePinvoke; } diff --git a/src/tests/profiler/native/transitions/transitions.h b/src/tests/profiler/native/transitions/transitions.h index 998739751a36e9..d5495f63abd63b 100644 --- a/src/tests/profiler/native/transitions/transitions.h +++ b/src/tests/profiler/native/transitions/transitions.h @@ -5,6 +5,8 @@ #include "../profiler.h" +#define NO_TRANSITION ((COR_PRF_TRANSITION_REASON)-1) + class Transitions : public Profiler { public: @@ -23,8 +25,8 @@ class Transitions : public Profiler struct TransitionInstance { TransitionInstance() - : UnmanagedToManaged{ (COR_PRF_TRANSITION_REASON)-1 } - , ManagedToUnmanaged{ (COR_PRF_TRANSITION_REASON)-1 } + : UnmanagedToManaged{ NO_TRANSITION } + , ManagedToUnmanaged{ NO_TRANSITION } { } COR_PRF_TRANSITION_REASON UnmanagedToManaged; @@ -33,6 +35,8 @@ class Transitions : public Profiler TransitionInstance _pinvoke; TransitionInstance _reversePinvoke; + String expectedPinvokeName; + String expectedReversePInvokeName; bool FunctionIsTargetFunction(FunctionID functionID, TransitionInstance** inst); }; diff --git a/src/tests/profiler/transitions/transitions.cs b/src/tests/profiler/transitions/transitions.cs index 30d0a73f5d2b59..b513d9c7717def 100644 --- a/src/tests/profiler/transitions/transitions.cs +++ b/src/tests/profiler/transitions/transitions.cs @@ -2,14 +2,53 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Generic; using System.Runtime.InteropServices; namespace Profiler.Tests { unsafe class Transitions { + static readonly string PInvokeExpectedNameEnvVar = "PInvoke_Transition_Expected_Name"; + static readonly string ReversePInvokeExpectedNameEnvVar = "ReversePInvoke_Transition_Expected_Name"; static readonly Guid TransitionsGuid = new Guid("027AD7BB-578E-4921-B29F-B540363D83EC"); + [UnmanagedFunctionPointer(CallingConvention.Winapi)] + delegate int InteropDelegate(int i); + + [UnmanagedFunctionPointer(CallingConvention.Winapi)] + delegate int InteropDelegateNonBlittable(bool b); + + private static int DoDelegateReversePInvoke(int i) + { + return i; + } + + private static int DoDelegateReversePInvokeNonBlittable(bool b) + { + return b ? 1 : 0; + } + + public static int BlittablePInvokeToBlittableInteropDelegate() + { + InteropDelegate del = DoDelegateReversePInvoke; + + DoPInvoke((delegate* unmanaged)Marshal.GetFunctionPointerForDelegate(del), 13); + GC.KeepAlive(del); + + return 100; + } + + public static int NonBlittablePInvokeToNonBlittableInteropDelegate() + { + InteropDelegateNonBlittable del = DoDelegateReversePInvokeNonBlittable; + + DoPInvokeNonBlitable((delegate* unmanaged)Marshal.GetFunctionPointerForDelegate(del), true); + GC.KeepAlive(del); + + return 100; + } + [UnmanagedCallersOnly] private static int DoReversePInvoke(int i) { @@ -19,23 +58,82 @@ private static int DoReversePInvoke(int i) [DllImport("Profiler")] public static extern void DoPInvoke(delegate* unmanaged callback, int i); - public static int RunTest(String[] args) + [DllImport("Profiler", EntryPoint = nameof(DoPInvoke))] + public static extern void DoPInvokeNonBlitable(delegate* unmanaged callback, bool i); + + public static int BlittablePInvokeToUnmanagedCallersOnly() { DoPInvoke(&DoReversePInvoke, 13); return 100; } + public static int NonBlittablePInvokeToUnmanagedCallersOnly() + { + DoPInvokeNonBlitable(&DoReversePInvoke, true); + + return 100; + } + public static int Main(string[] args) { - if (args.Length > 0 && args[0].Equals("RunTest", StringComparison.OrdinalIgnoreCase)) + if (args.Length > 1 && args[0].Equals("RunTest", StringComparison.OrdinalIgnoreCase)) + { + switch (args[1]) + { + case nameof(BlittablePInvokeToUnmanagedCallersOnly): + return BlittablePInvokeToUnmanagedCallersOnly(); + case nameof(BlittablePInvokeToBlittableInteropDelegate): + return BlittablePInvokeToBlittableInteropDelegate(); + case nameof(NonBlittablePInvokeToUnmanagedCallersOnly): + return NonBlittablePInvokeToUnmanagedCallersOnly(); + case nameof(NonBlittablePInvokeToNonBlittableInteropDelegate): + return NonBlittablePInvokeToNonBlittableInteropDelegate(); + } + } + + if (!RunProfilerTest(nameof(BlittablePInvokeToUnmanagedCallersOnly), nameof(DoPInvoke), nameof(DoReversePInvoke))) + { + return 101; + } + + if (!RunProfilerTest(nameof(BlittablePInvokeToBlittableInteropDelegate), nameof(DoPInvoke), "Invoke")) + { + return 102; + } + + if (!RunProfilerTest(nameof(NonBlittablePInvokeToUnmanagedCallersOnly), nameof(DoPInvokeNonBlitable), nameof(DoReversePInvoke))) { - return RunTest(args); + return 101; } - return ProfilerTestRunner.Run(profileePath: System.Reflection.Assembly.GetExecutingAssembly().Location, + if (!RunProfilerTest(nameof(NonBlittablePInvokeToNonBlittableInteropDelegate), nameof(DoPInvokeNonBlitable), "Invoke")) + { + return 102; + } + + return 100; + } + + private static bool RunProfilerTest(string testName, string pInvokeExpectedName, string reversePInvokeExpectedName) + { + try + { + return ProfilerTestRunner.Run(profileePath: System.Reflection.Assembly.GetExecutingAssembly().Location, testName: "Transitions", - profilerClsid: TransitionsGuid); + profilerClsid: TransitionsGuid, + profileeArguments: testName, + envVars: new Dictionary + { + { PInvokeExpectedNameEnvVar, pInvokeExpectedName }, + { ReversePInvokeExpectedNameEnvVar, reversePInvokeExpectedName }, + }) == 100; + } + catch (Exception ex) + { + Console.WriteLine(ex); + } + return false; } } }