Fix issues with rejit profiler test

src/coreclr/vm/rejit.cpp

@@ -548,7 +548,9 @@ HRESULT ReJitManager::UpdateActiveILVersions(
 
         if ((flags & COR_PRF_REJIT_BLOCK_INLINING) == COR_PRF_REJIT_BLOCK_INLINING)
         {
-            hr = UpdateNativeInlinerActiveILVersions(&mgrToCodeActivationBatch, pModule, rgMethodDefs[i], fIsRevert, flags);
+            _ASSERTE(!fIsRevert);
+
+            hr = UpdateNativeInlinerActiveILVersions(&mgrToCodeActivationBatch, pModule, rgMethodDefs[i], flags);
             if (FAILED(hr))
             {
                 return hr;
@@ -711,7 +713,6 @@ HRESULT ReJitManager::UpdateNativeInlinerActiveILVersions(
     SHash<CodeActivationBatchTraits>   *pMgrToCodeActivationBatch,
     Module                             *pInlineeModule,
     mdMethodDef                         inlineeMethodDef,
-    BOOL                                fIsRevert,
     COR_PRF_REJIT_FLAGS                 flags)
 {
     CONTRACTL
@@ -758,7 +759,7 @@ HRESULT ReJitManager::UpdateNativeInlinerActiveILVersions(
                     }
                 }
 
-                hr = UpdateActiveILVersion(pMgrToCodeActivationBatch, inliner.m_module, inliner.m_methodDef, fIsRevert, flags);
+                hr = UpdateActiveILVersion(pMgrToCodeActivationBatch, inliner.m_module, inliner.m_methodDef, FALSE /* fIsRevert */, flags);
                 if (FAILED(hr))
                 {
                     ReportReJITError(inliner.m_module, inliner.m_methodDef, NULL, hr);

src/coreclr/vm/rejit.h

@@ -166,7 +166,6 @@ class ReJitManager
         SHash<CodeActivationBatchTraits> *pMgrToCodeActivationBatch,
         Module             *pInlineeModule,
         mdMethodDef         inlineeMethodDef,
-        BOOL                fIsRevert,
         COR_PRF_REJIT_FLAGS flags);
 
     static HRESULT UpdateJitInlinerActiveILVersions(

src/tests/issues.targets

@@ -570,9 +570,6 @@
         <ExcludeList Include="$(XunitTestBinBase)/baseservices/multidimmarray/rank1array/**">
             <Issue>https://github.com/dotnet/runtime/issues/108255</Issue>
         </ExcludeList>
-        <ExcludeList Include="$(XunitTestBinBase)/profiler/rejit/rejit/**">
-            <Issue>https://github.com/dotnet/runtime/issues/109310</Issue>
-        </ExcludeList>
         <ExcludeList Include="$(XunitTestBinBase)/JIT/Methodical/Arrays/misc/arrres_il_r/**">
             <Issue>https://github.com/dotnet/runtime/issues/109311</Issue>
         </ExcludeList>

src/tests/profiler/native/rejitprofiler/rejitprofiler.cpp

@@ -95,13 +95,19 @@ HRESULT ReJITProfiler::Shutdown()
         _profInfo10 = nullptr;
     }
 
-    int expectedRejitCount = -1;
+    int expectedRejitCount;
     auto it = _inlinings.find(_targetFuncId);
     if (it != _inlinings.end())
     {
         // The number of inliners are expected to ReJIT, plus the method itself
         expectedRejitCount = (int)((*it).second->size() + 1);
     }
+    else
+    {
+        // No inlinings happened, which can occur in composite R2R mode on some targets.
+        // This is fine as long as we rejitted the target method itself.
+        expectedRejitCount = 1;
+    }
 
     INFO(L" rejit count=" << _rejits << L" expected rejit count=" << expectedRejitCount);
 
@@ -320,7 +326,8 @@ HRESULT STDMETHODCALLTYPE ReJITProfiler::GetReJITParameters(ModuleID moduleId, m
 {
     SHUTDOWNGUARD();
 
-    INFO(L"Starting to build IL for method " << GetFunctionIDName(GetFunctionIDFromToken(moduleId, methodId, false)));
+    String functionName = GetFunctionIDName(GetFunctionIDFromToken(moduleId, methodId, false));
+    INFO(L"Starting to build IL for method " << functionName);
     COMPtrHolder<IUnknown> pUnk;
     HRESULT hr = _profInfo10->GetModuleMetaData(moduleId, ofWrite, IID_IMetaDataEmit2, &pUnk);
     if (FAILED(hr))
@@ -340,10 +347,18 @@ HRESULT STDMETHODCALLTYPE ReJITProfiler::GetReJITParameters(ModuleID moduleId, m
     }
 
 
-    const WCHAR *wszNewUserDefinedString = WCHAR("Hello from profiler rejit!");
+    String newUserDefinedString = String(WCHAR("Hello from profiler rejit method '"));
+    newUserDefinedString += functionName;
+    newUserDefinedString += WCHAR("'! ");
     mdString tokmdsUserDefined = mdTokenNil;
-    hr = pTargetEmit->DefineUserString(wszNewUserDefinedString,
-                                       (ULONG)wcslen(wszNewUserDefinedString),
+
+    // There's no portable way to convert a String to LPCWSTR so just make a manual copy on the stack.
+    char16_t buf[4096] = { 0 };
+    for (size_t i = 0, c = newUserDefinedString.Length(); i < c; i++)
+        buf[i] = (char16_t)newUserDefinedString[i];
+
+    hr = pTargetEmit->DefineUserString((LPCWSTR)(void *)buf,
+                                       (ULONG)newUserDefinedString.Length(),
                                        &tokmdsUserDefined);
     if (FAILED(hr))
     {
@@ -427,28 +442,39 @@ HRESULT STDMETHODCALLTYPE ReJITProfiler::ReJITError(ModuleID moduleId, mdMethodD
 
 void ReJITProfiler::AddInlining(FunctionID inliner, FunctionID inlinee)
 {
-    shared_ptr<unordered_set<FunctionID>> inliners;
-    auto result = _inlinings.find(inlinee);
-    if (result == _inlinings.end())
+    String calleeName = GetFunctionIDName(inlinee);
+    String moduleName = GetModuleIDName(GetModuleIDForFunction(inlinee));
+
+    // Depending on various things it's possible the JIT will inline code during our test run that isn't part of the
+    //  rejit test module. For example if part of the BCL didn't get crossgen'd or the crossgen'd code isn't used.
+    // We don't care about those inlinings, so we won't track them. This makes the test more reliable.
+    if (EndsWith(moduleName, String(WCHAR("rejit.dll"))))
     {
-        auto p = make_pair(inlinee, make_shared<unordered_set<FunctionID>>());
-        inliners = p.second;
-        _inlinings.insert(p);
+        shared_ptr<unordered_set<FunctionID>> inliners;
+        auto result = _inlinings.find(inlinee);
+        if (result == _inlinings.end())
+        {
+            auto p = make_pair(inlinee, make_shared<unordered_set<FunctionID>>());
+            inliners = p.second;
+            _inlinings.insert(p);
+        }
+        else
+        {
+            inliners = (*result).second;
+        }
+
+        auto it = inliners->find(inliner);
+        if (it == inliners->end())
+        {
+            inliners->insert(inliner);
+        }
+
+        INFO(L"Inlining in test module! Inliner=" << GetFunctionIDName(inliner) << L" Inlinee=" << calleeName << L" module=" << moduleName);
     }
     else
     {
-        inliners = (*result).second;
+        INFO(L"Inlining in non-test module! Inliner=" << GetFunctionIDName(inliner) << L" Inlinee=" << calleeName << L" module=" << moduleName);
     }
-
-    auto it = inliners->find(inliner);
-    if (it == inliners->end())
-    {
-        inliners->insert(inliner);
-    }
-
-    String calleeName = GetFunctionIDName(inlinee);
-    String moduleName = GetModuleIDName(GetModuleIDForFunction(inlinee));
-    INFO(L"Inlining occurred! Inliner=" << GetFunctionIDName(inliner) << L" Inlinee=" << calleeName << L" Inlinee module name=" << moduleName);
 }
 
 FunctionID ReJITProfiler::GetFunctionIDFromToken(ModuleID module, mdMethodDef token, bool invalidArgNotFailure)

src/tests/profiler/rejit/rejit.cs

@@ -10,6 +10,15 @@ class RejitWithInlining //: ProfilerTest
     {
         static readonly Guid ReJitProfilerGuid = new Guid("66F7A9DF-8858-4A32-9CFF-3AD0787E0186");
 
+        static System.Text.StringBuilder OutputBuilder = new ();
+
+        [MethodImplAttribute(MethodImplOptions.NoInlining)]
+        public static void TestWriteLine(string s)
+        {
+            OutputBuilder.AppendLine(s);
+            Console.WriteLine(s);
+        }
+
         [MethodImplAttribute(MethodImplOptions.NoInlining)]
         private static int MaxInlining()
         {
@@ -20,18 +29,41 @@ private static int MaxInlining()
 
             TriggerReJIT();
 
+            OutputBuilder.Clear();
+
             // TriggerInliningChain triggers a ReJIT, now this time we should call
             // in to the ReJITted code
             TriggerDirectInlining();
             CallMethodWithoutInlining();
             TriggerInliningChain();
 
+            string matchString = "Hello from profiler rejit method 'InlineeTarget'!";
+            int numRejittedTargets = OutputBuilder.ToString().Split(matchString).Length;
+            if (numRejittedTargets != 4)
+            {
+                Console.WriteLine("ReJIT did not update all instances of InlineeTarget!");
+                return 1234;
+            }
+
             TriggerRevert();
 
+            OutputBuilder.Clear();
+
             TriggerDirectInlining();
             CallMethodWithoutInlining();
             TriggerInliningChain();
 
+            if (OutputBuilder.ToString().Contains(matchString))
+            {
+                Console.WriteLine("ReJIT revert of InlineeTarget was not complete!");
+                return 1235;
+            }
+
+            // Currently there will still be some 'Hello from profiler rejit method' messages left
+            //  in the output in certain configurations. This is because the test profiler does not revert all
+            //  the methods that it modified - reverts are not symmetric with rejits.
+            // See https://github.com/dotnet/runtime/issues/117823
+
             return 100;
         }
 
@@ -50,42 +82,48 @@ private static void TriggerRevert()
         [MethodImplAttribute(MethodImplOptions.NoInlining)]
         private static void TriggerInliningChain()
         {
-            Console.WriteLine("TriggerInlining");
+            TestWriteLine("TriggerInliningChain");
             // Test Inlining through another method
             InlineeChain1();
         }
 
         [MethodImplAttribute(MethodImplOptions.NoInlining)]
         private static void TriggerDirectInlining()
         {
-            Console.WriteLine("TriggerDirectInlining");
+            TestWriteLine("TriggerDirectInlining");
             InlineeTarget();
         }
 
         [MethodImplAttribute(MethodImplOptions.NoInlining)]
         private static void CallMethodWithoutInlining()
         {
-            Console.WriteLine("CallMethodWithoutInlining");
-            Action callMethod = InlineeTarget;
-            callMethod();
+            TestWriteLine("CallMethodWithoutInlining");
+            Action<string> callMethod = InlineeTarget;
+            callMethod("CallMethodWithoutInlining");
         }
 
         [MethodImplAttribute(MethodImplOptions.AggressiveInlining)]
         private static void InlineeChain1()
         {
-            Console.WriteLine("Inline.InlineeChain1");
+            TestWriteLine(" Inline.InlineeChain1");
             InlineeTarget();
         }
 
         [MethodImplAttribute(MethodImplOptions.AggressiveInlining)]
-        public static void InlineeTarget()
+        public static void InlineeTarget([CallerMemberName] string callerMemberName = null)
         {
-            Console.WriteLine("Inline.InlineeTarget");
+            var sb = new System.Text.StringBuilder();
+            sb.Append("  Inline.InlineeTarget");
+            sb.Append(' ');
+            sb.Append('(');
+            sb.Append(callerMemberName);
+            sb.Append(')');
+            TestWriteLine(sb.ToString());
         }
 
         public static int RunTest(string[] args)
         {
-            Console.WriteLine("maxinlining");
+            TestWriteLine("maxinlining");
             return MaxInlining();
         }
 
@@ -108,22 +146,22 @@ public class SeparateClassNeverLoaded
         [MethodImplAttribute(MethodImplOptions.NoInlining)]
         private static void TriggerInliningChain()
         {
-            Console.WriteLine("TriggerInlining");
+            RejitWithInlining.TestWriteLine("TriggerInlining");
             // Test Inlining through another method
             InlineeChain1();
         }
 
         [MethodImplAttribute(MethodImplOptions.NoInlining)]
         private static void TriggerDirectInlining()
         {
-            Console.WriteLine("TriggerDirectInlining");
+            RejitWithInlining.TestWriteLine("TriggerDirectInlining");
             RejitWithInlining.InlineeTarget();
         }
 
         [MethodImplAttribute(MethodImplOptions.AggressiveInlining)]
         private static void InlineeChain1()
         {
-            Console.WriteLine("Inline.InlineeChain1");
+            RejitWithInlining.TestWriteLine("Inline.InlineeChain1");
             RejitWithInlining.InlineeTarget();
         }
     }