diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 1598b2b45e498e..4c36d8e065f941 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -653,8 +653,7 @@ class ILStubState // Notify the profiler of call out of the runtime if (CORProfilerTrackTransitions() && !SF_SkipTransitionNotify(m_dwStubFlags) - && !SF_IsReverseCOMStub(m_dwStubFlags) - && !SF_IsReverseDelegateStub(m_dwStubFlags)) + && SF_IsForwardStub(m_dwStubFlags)) { dwMethodDescLocalNum = m_slIL.EmitProfilerBeginTransitionCallback(pcsDispatch, m_dwStubFlags); _ASSERTE(dwMethodDescLocalNum != (DWORD)-1); diff --git a/src/tests/profiler/ijw/CMakeLists.txt b/src/tests/profiler/ijw/CMakeLists.txt new file mode 100644 index 00000000000000..1922fb04e46a04 --- /dev/null +++ b/src/tests/profiler/ijw/CMakeLists.txt @@ -0,0 +1,19 @@ +# The IJW (C++/CLI) profilee is Windows-only. This directory is picked up +# unconditionally by the recursive add_subdirectory globber in +# src/tests/CMakeLists.txt, so it must guard itself against non-Windows targets: +# IJW.cmake only defines add_ijw_msbuild_project_properties on Windows hosts. +if (CLR_CMAKE_TARGET_WIN32) + project (IjwProfileeDll) + include(${CLR_ENG_NATIVE_DIR}/ijw/IJW.cmake) + + include_directories( ${INC_PLATFORM_DIR} ) + set(SOURCES IjwProfileeDll.cpp) + + # add the shared library + add_library (IjwProfileeDll SHARED ${SOURCES}) + target_link_libraries(IjwProfileeDll PRIVATE ${LINK_LIBRARIES_ADDITIONAL}) + add_ijw_msbuild_project_properties(IjwProfileeDll ijwhost) + + # add the install targets + install (TARGETS IjwProfileeDll DESTINATION bin) +endif() diff --git a/src/tests/profiler/ijw/IjwProfileeDll.cpp b/src/tests/profiler/ijw/IjwProfileeDll.cpp new file mode 100644 index 00000000000000..37faf92c0b527a --- /dev/null +++ b/src/tests/profiler/ijw/IjwProfileeDll.cpp @@ -0,0 +1,28 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// C++/CLI (IJW) mixed-mode assembly used by the ijw profiler test. +// +// Repro shape for https://github.com/dotnet/runtime/issues/120151: a managed +// method calls a native helper 'call' which invokes the managed +// 'ManagedByPointerTarget' through a raw function pointer. Invoking a managed +// method by native function pointer goes through a reverse (unmanaged->managed) +// marshaling stub, and the profiler code transition callback for that stub used +// to report a bogus, non-null FunctionID. +// +// Compiled with default /clr per-function codegen (no #pragma managed/unmanaged); +// the reporter's sample used a file-static function, but a named function +// reproduces identically and lets the profiler match it by name. +__declspec(noinline) void ManagedByPointerTarget() {} +__declspec(noinline) static void call(void (*f)()) { f(); } + +public ref class TestClass +{ +public: + // Managed -> native 'call' -> managed 'ManagedByPointerTarget' by pointer. + int CallManagedFunctionByPointer() + { + call(ManagedByPointerTarget); + return 100; + } +}; diff --git a/src/tests/profiler/ijw/ijw.cs b/src/tests/profiler/ijw/ijw.cs new file mode 100644 index 00000000000000..e357aa29e1eb58 --- /dev/null +++ b/src/tests/profiler/ijw/ijw.cs @@ -0,0 +1,63 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Reflection; + +namespace Profiler.Tests +{ + class Ijw + { + static readonly Guid IjwProfilerGuid = new Guid("D6973314-9E66-4EAD-8129-9B1D3AD7CB85"); + + // Managed -> native -> managed-by-pointer scenario. Exercises a reverse + // (unmanaged->managed) marshaling stub, which must not emit a spurious + // profiler code transition callback (it used to report a bogus FunctionID). + // Regression test for https://github.com/dotnet/runtime/issues/120151. + private static int CallManagedFunctionByPointer() + { + Assembly ijwDll = Assembly.Load("IjwProfileeDll"); + Type testType = ijwDll.GetType("TestClass") + ?? throw new InvalidOperationException("Could not find type 'TestClass' in IjwProfileeDll."); + object testInstance = Activator.CreateInstance(testType); + MethodInfo method = testType.GetMethod("CallManagedFunctionByPointer") + ?? throw new InvalidOperationException("Could not find method 'CallManagedFunctionByPointer' on TestClass."); + return (int)method.Invoke(testInstance, null); + } + + public static int Main(string[] args) + { + if (args.Length > 1 && args[0].Equals("RunTest", StringComparison.OrdinalIgnoreCase)) + { + switch (args[1]) + { + case nameof(CallManagedFunctionByPointer): + return CallManagedFunctionByPointer(); + } + } + + if (!RunProfilerTest(nameof(CallManagedFunctionByPointer))) + { + return 101; + } + + return 100; + } + + private static bool RunProfilerTest(string testName) + { + try + { + return ProfilerTestRunner.Run(profileePath: Assembly.GetExecutingAssembly().Location, + testName: "Ijw", + profilerClsid: IjwProfilerGuid, + profileeArguments: testName) == 100; + } + catch (Exception ex) + { + Console.WriteLine(ex); + } + return false; + } + } +} diff --git a/src/tests/profiler/ijw/ijw.csproj b/src/tests/profiler/ijw/ijw.csproj new file mode 100644 index 00000000000000..6175432077fcb4 --- /dev/null +++ b/src/tests/profiler/ijw/ijw.csproj @@ -0,0 +1,42 @@ + + + .NETCoreApp + exe + true + false + + true + + true + + true + + true + + + true + + + + + + + + + + + + + + + + + + + + diff --git a/src/tests/profiler/native/CMakeLists.txt b/src/tests/profiler/native/CMakeLists.txt index 60c43b3309f48a..dbc2c0d3bc21f9 100644 --- a/src/tests/profiler/native/CMakeLists.txt +++ b/src/tests/profiler/native/CMakeLists.txt @@ -19,6 +19,7 @@ set(SOURCES gcskipobjectsallocatedbyclasscallbackprofiler/gcskipobjectsallocatedbyclasscallbackprofiler.cpp getappdomainstaticaddress/getappdomainstaticaddress.cpp handlesprofiler/handlesprofiler.cpp + ijw/ijwprofiler.cpp inlining/inlining.cpp metadatagetdispenser/metadatagetdispenser.cpp moduleload/moduleload.cpp diff --git a/src/tests/profiler/native/classfactory.cpp b/src/tests/profiler/native/classfactory.cpp index 0ff8a07467c27c..dace6277d6f030 100644 --- a/src/tests/profiler/native/classfactory.cpp +++ b/src/tests/profiler/native/classfactory.cpp @@ -15,6 +15,7 @@ #include "gcheapenumerationprofiler/gcheapenumerationprofiler.h" #include "gcprofiler/gcprofiler.h" #include "handlesprofiler/handlesprofiler.h" +#include "ijw/ijwprofiler.h" #include "metadatagetdispenser/metadatagetdispenser.h" #include "nullprofiler/nullprofiler.h" #include "rejitprofiler/rejitprofiler.h" @@ -174,6 +175,10 @@ HRESULT STDMETHODCALLTYPE ClassFactory::CreateInstance(IUnknown *pUnkOuter, REFI { profiler = new GCSkipObjectsAllocatedByClassCallbackProfiler(); } + else if (clsid == IjwProfiler::GetClsid()) + { + profiler = new IjwProfiler(); + } else { printf("No profiler found in ClassFactory::CreateInstance. Did you add your profiler to the list?\n"); diff --git a/src/tests/profiler/native/ijw/ijwprofiler.cpp b/src/tests/profiler/native/ijw/ijwprofiler.cpp new file mode 100644 index 00000000000000..6d82abd7377a32 --- /dev/null +++ b/src/tests/profiler/native/ijw/ijwprofiler.cpp @@ -0,0 +1,152 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#include "ijwprofiler.h" + +GUID IjwProfiler::GetClsid() +{ + // {D6973314-9E66-4EAD-8129-9B1D3AD7CB85} + GUID clsid = { 0xd6973314, 0x9e66, 0x4ead, { 0x81, 0x29, 0x9b, 0x1d, 0x3a, 0xd7, 0xcb, 0x85 } }; + return clsid; +} + +HRESULT IjwProfiler::Initialize(IUnknown* pICorProfilerInfoUnk) +{ + Profiler::Initialize(pICorProfilerInfoUnk); + + HRESULT hr = S_OK; + if (FAILED(hr = pCorProfilerInfo->SetEventMask2(COR_PRF_MONITOR_CODE_TRANSITIONS | COR_PRF_DISABLE_INLINING, 0))) + { + _failures++; + printf("FAIL: ICorProfilerInfo::SetEventMask2() failed hr=0x%x\n", hr); + return hr; + } + + return S_OK; +} + +HRESULT IjwProfiler::Shutdown() +{ + Profiler::Shutdown(); + + // The managed target reached through the native function pointer must have + // been reported CALL on the way in and RETURN on the way out. This also + // ensures the by-pointer scenario actually ran. + bool targetOk = _targetUnmanagedToManaged == COR_PRF_TRANSITION_CALL + && _targetManagedToUnmanaged == COR_PRF_TRANSITION_RETURN; + + // No nested code transitions may surround the target: reverse P/Invoke stubs + // no longer emit a stub-level transition, so the spurious callback that used + // to report a bogus FunctionID must not appear at all. + if (_failures == 0 && _transitions > 0 && targetOk && _nestedTransitions == 0) + { + printf("PROFILER TEST PASSES\n"); + } + else + { + printf("Test failed _failures=%d _transitions=%d targetU2M=%d targetM2U=%d nested=%d\n", + _failures.load(), _transitions.load(), + (int)_targetUnmanagedToManaged, (int)_targetManagedToUnmanaged, + _nestedTransitions.load()); + } + fflush(stdout); + return S_OK; +} + +void IjwProfiler::HandleTransition(bool unmanagedToManaged, FunctionID functionID, COR_PRF_TRANSITION_REASON reason) +{ + _transitions++; + + const char* which = unmanagedToManaged ? "UnmanagedToManagedTransition" : "ManagedToUnmanagedTransition"; + + // A non-null FunctionID must be a real MethodDesc that GetFunctionInfo can + // resolve. Before the fix for https://github.com/dotnet/runtime/issues/120151 + // a reverse marshaling stub reported a bogus pointer here, and this call + // would fail or crash. + bool isTarget = false; + String name(WCHAR("")); + if (functionID != 0) + { + ClassID classId = 0; + ModuleID moduleId = 0; + mdToken token = 0; + HRESULT hr = pCorProfilerInfo->GetFunctionInfo(functionID, &classId, &moduleId, &token); + if (FAILED(hr)) + { + _failures++; + name = WCHAR(""); + printf("FAIL: %s reported FunctionID=0x%p (reason=%d) that GetFunctionInfo could not resolve hr=0x%x\n", + which, (void*)functionID, (int)reason, hr); + fflush(stdout); + } + else + { + name = GetFunctionIDName(functionID); + isTarget = name == WCHAR("ManagedByPointerTarget"); + } + } + + // Trace every transition so the expected shape can be inspected in the log: + // the target reported CALL in / RETURN out, and no nested transitions. + printf(" %s FunctionID=0x%p reason=%d insideTarget=%d name=%ls\n", + which, (void*)functionID, (int)reason, (int)_insideTarget.load(), name.ToCStr()); + fflush(stdout); + + if (isTarget) + { + // Record the reason for each direction and open/close the window in which + // the nested reverse-stub transitions are checked. The target should be + // seen exactly once per direction. + if (unmanagedToManaged) + { + if (_targetUnmanagedToManaged != NO_TRANSITION) + { + _failures++; + } + _targetUnmanagedToManaged = reason; + if (reason == COR_PRF_TRANSITION_CALL) + { + _insideTarget = true; + } + } + else + { + if (_targetManagedToUnmanaged != NO_TRANSITION) + { + _failures++; + } + _targetManagedToUnmanaged = reason; + if (reason == COR_PRF_TRANSITION_RETURN) + { + _insideTarget = false; + } + } + return; + } + + // Any non-target transition seen while executing the managed target is + // unexpected: reverse P/Invoke stubs no longer emit a stub-level transition, + // so none of these nested callbacks (the vehicle for the 120151 bug) should + // occur while inside the target. + if (_insideTarget.load()) + { + _nestedTransitions++; + printf("FAIL: unexpected nested %s inside target reported FunctionID=0x%p (reason=%d)\n", + which, (void*)functionID, (int)reason); + fflush(stdout); + } +} + +HRESULT IjwProfiler::UnmanagedToManagedTransition(FunctionID functionID, COR_PRF_TRANSITION_REASON reason) +{ + SHUTDOWNGUARD(); + HandleTransition(/* unmanagedToManaged */ true, functionID, reason); + return S_OK; +} + +HRESULT IjwProfiler::ManagedToUnmanagedTransition(FunctionID functionID, COR_PRF_TRANSITION_REASON reason) +{ + SHUTDOWNGUARD(); + HandleTransition(/* unmanagedToManaged */ false, functionID, reason); + return S_OK; +} diff --git a/src/tests/profiler/native/ijw/ijwprofiler.h b/src/tests/profiler/native/ijw/ijwprofiler.h new file mode 100644 index 00000000000000..1549f1a5c2ccd4 --- /dev/null +++ b/src/tests/profiler/native/ijw/ijwprofiler.h @@ -0,0 +1,61 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#pragma once + +#include "../profiler.h" + +#define NO_TRANSITION ((COR_PRF_TRANSITION_REASON)-1) + +// Profiler used by the C++/CLI (IJW) profiler tests. It monitors code +// transitions and validates that every non-null FunctionID reported for a +// transition can be resolved via ICorProfilerInfo::GetFunctionInfo. This is a +// regression guard for https://github.com/dotnet/runtime/issues/120151, where a +// reverse (unmanaged->managed) marshaling stub reported a bogus, non-null +// FunctionID that crashed GetFunctionInfo. +// +// In addition to the "must resolve" check it validates the transition shape of +// the managed method invoked through a native function pointer: the target must +// be reported CALL on the way in and RETURN on the way out, and no nested code +// transitions may surround it. Reverse P/Invoke stubs no longer emit a +// stub-level transition, so the spurious nested callback that used to report a +// bogus FunctionID must not appear at all. +class IjwProfiler : public Profiler +{ +public: + IjwProfiler() + : Profiler() + , _failures(0) + , _transitions(0) + , _targetUnmanagedToManaged(NO_TRANSITION) + , _targetManagedToUnmanaged(NO_TRANSITION) + , _insideTarget(false) + , _nestedTransitions(0) + {} + virtual ~IjwProfiler() = default; + + static GUID GetClsid(); + virtual HRESULT STDMETHODCALLTYPE Initialize(IUnknown* pICorProfilerInfoUnk); + virtual HRESULT STDMETHODCALLTYPE Shutdown(); + virtual HRESULT STDMETHODCALLTYPE UnmanagedToManagedTransition(FunctionID functionID, COR_PRF_TRANSITION_REASON reason); + virtual HRESULT STDMETHODCALLTYPE ManagedToUnmanagedTransition(FunctionID functionID, COR_PRF_TRANSITION_REASON reason); + +private: + std::atomic _failures; + std::atomic _transitions; + + // Transition reasons recorded for the managed method reached through the + // native function pointer (see IjwProfileeDll.cpp). + COR_PRF_TRANSITION_REASON _targetUnmanagedToManaged; + COR_PRF_TRANSITION_REASON _targetManagedToUnmanaged; + + // Set while executing the managed target so nested transitions can be + // checked. The profilee is single-threaded around this call. + std::atomic _insideTarget; + + // Number of nested code transitions observed while inside the target. Reverse + // P/Invoke stubs no longer emit a stub-level transition, so this must be 0. + std::atomic _nestedTransitions; + + void HandleTransition(bool unmanagedToManaged, FunctionID functionID, COR_PRF_TRANSITION_REASON reason); +};