[release/10.0] Fix profiler spurious/corrupt transition callbacks for reverse P/Invoke marshaling stubs.

src/coreclr/vm/dllimport.cpp

@@ -691,8 +691,7 @@ class ILStubState : public StubState
         // 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)
             && !SF_IsStructMarshalStub(m_dwStubFlags))
         {
             dwMethodDescLocalNum = m_slIL.EmitProfilerBeginTransitionCallback(pcsDispatch, m_dwStubFlags);

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()

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;
+    }
+};

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;
+        }
+    }
+}

src/tests/profiler/ijw/ijw.csproj

@@ -0,0 +1,42 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <TargetFrameworkIdentifier>.NETCoreApp</TargetFrameworkIdentifier>
+    <OutputType>exe</OutputType>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <CrossGenTest>false</CrossGenTest>
+    <!-- IJW / C++/CLI tests are Windows-only and aren't enabled on ARM64 in .NET 10 -->
+    <CLRTestTargetUnsupported Condition="'$(TargetsWindows)' != 'true' or '$(TargetArchitecture)' == 'arm64'">true</CLRTestTargetUnsupported>
+    <!-- IJW assemblies contain native code that cannot be round-tripped through ilasm -->
+    <IlasmRoundTripIncompatible>true</IlasmRoundTripIncompatible>
+    <!-- The test launches a secondary process; loading IJW assemblies into an
+    unloadable context is not allowed and process launch prevents unloading. -->
+    <UnloadabilityIncompatible>true</UnloadabilityIncompatible>
+    <!-- Temporarily disabled due to https://github.com/dotnet/runtime/issues/106243 -->
+    <GCStressIncompatible>true</GCStressIncompatible>
+  </PropertyGroup>
+  <PropertyGroup>
+    <CopyDebugCRTDllsToOutputDirectory>true</CopyDebugCRTDllsToOutputDirectory>
+  </PropertyGroup>
+  <Target Name="CopyIjwDebugCRTDependencies"
+    BeforeTargets="CopyAllNativeProjectReferenceBinaries"
+    Condition="'$(TargetsWindows)' == 'true' And ('$(Configuration)' == 'Debug' Or '$(Configuration)' == 'Checked') And '$(CopyDebugCRTDllsToOutputDirectory)' == 'true'" >
+    <Warning Text="Building C++/CLI tests requires a Visual Studio Dev Command Prompt" Condition="'$(VCToolsRedistDir)' == '' or '$(ExtensionSdkDir)' == ''" />
+    <ItemGroup Condition="'$(VCToolsRedistDir)' != '' and '$(ExtensionSdkDir)' != ''">
+      <IjwNativeRuntimeDependencies Include="$(VCToolsRedistDir)onecore/debug_nonredist/$(TargetArchitecture)/Microsoft.VC*.DebugCRT/vcruntime*d.dll" />
+      <IjwNativeRuntimeDependencies Include="$(VCToolsRedistDir)onecore/debug_nonredist/$(TargetArchitecture)/Microsoft.VC*.DebugCRT/msvcp*d.dll" />
+      <IjwNativeRuntimeDependencies Include="$(ExtensionSdkDir)/Microsoft.UniversalCRT.Debug/$(UCRTVersion)/Redist/Debug/$(TargetArchitecture)/ucrtbased.dll" />
+    </ItemGroup>
+    <Copy SourceFiles="@(IjwNativeRuntimeDependencies)" DestinationFolder="$(OutputPath)" />
+  </Target>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+    <ProjectReference Include="$(TestLibraryProjectPath)" />
+    <ProjectReference Include="../common/profiler_common.csproj" />
+    <CMakeProjectReference Include="$(MSBuildThisFileDirectory)/../native/CMakeLists.txt" />
+    <!-- The IJW (C++/CLI) profilee and the ijwhost mock are Windows-only; only
+         reference their native CMake projects on Windows so non-Windows configure
+         doesn't process the /clr build. -->
+    <CMakeProjectReference Include="CMakeLists.txt" Condition="'$(TargetsWindows)' == 'true'" />
+    <CMakeProjectReference Include="../../Interop/IJW/ijwhostmock/CMakeLists.txt" Condition="'$(TargetsWindows)' == 'true'" />
+  </ItemGroup>
+</Project>

src/tests/profiler/native/CMakeLists.txt

@@ -19,6 +19,7 @@ set(SOURCES
     gcprofiler/gcprofiler.cpp
     getappdomainstaticaddress/getappdomainstaticaddress.cpp
     handlesprofiler/handlesprofiler.cpp
+    ijw/ijwprofiler.cpp
     inlining/inlining.cpp
     metadatagetdispenser/metadatagetdispenser.cpp
     moduleload/moduleload.cpp

src/tests/profiler/native/classfactory.cpp

@@ -14,6 +14,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"
@@ -140,6 +141,10 @@ HRESULT STDMETHODCALLTYPE ClassFactory::CreateInstance(IUnknown *pUnkOuter, REFI
     {
         profiler = new HandlesProfiler();
     }
+    else if (clsid == IjwProfiler::GetClsid())
+    {
+        profiler = new IjwProfiler();
+    }
     else if (clsid == ModuleLoad::GetClsid())
     {
         profiler = new ModuleLoad();

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("<null>"));
+    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("<unresolved>");
+            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;
+}