[clr-interp] Tracing fixes.

src/coreclr/inc/eventtracebase.h

@@ -31,6 +31,7 @@ struct EventStructTypeData;
 void InitializeEventTracing();
 
 class PrepareCodeConfig;
+class CodeHeapIterator;
 
 // !!!!!!! NOTE !!!!!!!!
 // The flags must match those in the ETW manifest exactly
@@ -909,6 +910,16 @@ namespace ETW
             BOOL fSendILToNativeMapEvent,
             BOOL fSendRichDebugInfoEvent,
             BOOL fGetCodeIds);
+
+        static VOID SendEventsForJitMethodsHelper2(
+            CodeHeapIterator codeHeapIterator,
+            DWORD dwEventOptions,
+            BOOL fLoadOrDCStart,
+            BOOL fUnloadOrDCEnd,
+            BOOL fSendMethodEvent,
+            BOOL fSendILToNativeMapEvent,
+            BOOL fSendRichDebugInfoEvent,
+            BOOL fGetCodeIds);
         static VOID SendEventsForNgenMethods(Module *pModule, DWORD dwEventOptions);
         static VOID SendMethodJitStartEvent(MethodDesc *pMethodDesc, COR_ILMETHOD_DECODER* methodDecoder, SString *namespaceOrClassName=NULL, SString *methodName=NULL, SString *methodSignature=NULL);
         static VOID SendMethodILToNativeMapEvent(MethodDesc * pMethodDesc, DWORD dwEventOptions, PCODE pNativeCodeStartAddress, DWORD nativeCodeId, ReJITID ilCodeId);

src/coreclr/inc/utilcode.h

@@ -716,9 +716,9 @@ class CUnorderedArrayWithAllocator
     CUnorderedArrayWithAllocator(CUnorderedArrayWithAllocator const&) = delete;
     CUnorderedArrayWithAllocator& operator=(CUnorderedArrayWithAllocator const&) = delete;
     CUnorderedArrayWithAllocator(CUnorderedArrayWithAllocator&& other)
-        : m_iCount{ 0 }
-        , m_iSize{ 0 }
-        , m_pTable{ NULL}
+        : m_iCount{ other.m_iCount }
+        , m_iSize{ other.m_iSize }
+        , m_pTable{ other.m_pTable }
     {
         LIMITED_METHOD_CONTRACT;
         other.m_iCount = 0;

src/coreclr/vm/codeman.cpp

@@ -461,12 +461,13 @@ CodeHeapIterator::CodeHeapIterator(EECodeGenManager* manager, HeapList* heapList
     , m_HeapsIndexNext{ 0 }
     , m_pLoaderAllocatorFilter{ pLoaderAllocatorFilter }
     , m_pCurrent{ NULL }
+    , m_codeType(manager->GetCodeType())
 {
     CONTRACTL
     {
         THROWS;
         GC_NOTRIGGER;
-        PRECONDITION(m_manager != NULL);
+        PRECONDITION(manager != NULL);
     }
     CONTRACTL_END;
 
@@ -485,11 +486,37 @@ CodeHeapIterator::CodeHeapIterator(EECodeGenManager* manager, HeapList* heapList
 
     // Move to the first method section.
     (void)NextMethodSectionIterator();
+}
+
+CodeHeapIterator::EECodeGenManagerReleaseIteratorHolder::EECodeGenManagerReleaseIteratorHolder(EECodeGenManager* manager) : m_manager(manager)
+{
+    CONTRACTL
+    {
+        NOTHROW;
+        GC_NOTRIGGER;
+    }
+    CONTRACTL_END;
+
+    manager->AddRefIterator();
+}
+
+CodeHeapIterator::EECodeGenManagerReleaseIteratorHolder::~EECodeGenManagerReleaseIteratorHolder()
+{
+    CONTRACTL
+    {
+        NOTHROW;
+        GC_NOTRIGGER;
+    }
+    CONTRACTL_END;
 
-    m_manager->AddRefIterator();
+    if (m_manager)
+    {
+        m_manager->ReleaseIterator();
+        m_manager = NULL;
+    }
 }
 
-CodeHeapIterator::~CodeHeapIterator()
+CodeHeapIterator::EECodeGenManagerReleaseIteratorHolder& CodeHeapIterator::EECodeGenManagerReleaseIteratorHolder::operator=(EECodeGenManagerReleaseIteratorHolder&& other)
 {
     CONTRACTL
     {
@@ -498,7 +525,16 @@ CodeHeapIterator::~CodeHeapIterator()
     }
     CONTRACTL_END;
 
-    m_manager->ReleaseIterator();
+    if (this != &other)
+    {
+        if (m_manager)
+        {
+            m_manager->ReleaseIterator();
+        }
+        m_manager = other.m_manager;
+        other.m_manager = NULL;
+    }
+    return *this;
 }
 
 bool CodeHeapIterator::Next()
@@ -520,8 +556,19 @@ bool CodeHeapIterator::Next()
         else
         {
             BYTE* code = m_Iterator.GetMethodCode();
-            CodeHeader* pHdr = (CodeHeader*)(code - sizeof(CodeHeader));
-            m_pCurrent = !pHdr->IsStubCodeBlock() ? pHdr->GetMethodDesc() : NULL;
+#ifdef FEATURE_INTERPRETER
+            if (m_codeType == (miManaged | miIL | miOPTIL))
+            {
+                // Interpreter case
+                InterpreterCodeHeader* pHdr = (InterpreterCodeHeader*)(code - sizeof(InterpreterCodeHeader));
+                m_pCurrent = pHdr->GetMethodDesc();
+            }
+            else
+#endif
+            {
+                CodeHeader* pHdr = (CodeHeader*)(code - sizeof(CodeHeader));
+                m_pCurrent = !pHdr->IsStubCodeBlock() ? pHdr->GetMethodDesc() : NULL;
+            }
 
             // LoaderAllocator filter
             if (m_pLoaderAllocatorFilter && m_pCurrent)

src/coreclr/vm/codeman.h

@@ -1813,16 +1813,33 @@ class CodeHeapIterator final
         size_t MaxCodeHeapSize;
     };
 
-    EECodeGenManager* m_manager;
+    class EECodeGenManagerReleaseIteratorHolder
+    {
+        EECodeGenManager* m_manager;
+    public:
+        EECodeGenManagerReleaseIteratorHolder(EECodeGenManager* manager);
+        EECodeGenManagerReleaseIteratorHolder(EECodeGenManagerReleaseIteratorHolder const&) = delete;
+        EECodeGenManagerReleaseIteratorHolder& operator=(EECodeGenManagerReleaseIteratorHolder const&) = delete;
+        EECodeGenManagerReleaseIteratorHolder(EECodeGenManagerReleaseIteratorHolder&& other)
+        {
+            LIMITED_METHOD_CONTRACT;
+            m_manager = other.m_manager;
+            other.m_manager = NULL;
+        }
+        EECodeGenManagerReleaseIteratorHolder& operator=(EECodeGenManagerReleaseIteratorHolder&& other);
+        ~EECodeGenManagerReleaseIteratorHolder();
+    };
+
+    EECodeGenManagerReleaseIteratorHolder m_manager;
     MethodSectionIterator m_Iterator;
     CUnorderedArray<HeapListState, 64> m_Heaps;
     int32_t m_HeapsIndexNext;
     LoaderAllocator* m_pLoaderAllocatorFilter;
     MethodDesc* m_pCurrent;
+    DWORD m_codeType;
 
 public:
     CodeHeapIterator(EECodeGenManager* manager, HeapList* heapList, LoaderAllocator* pLoaderAllocatorFilter);
-    ~CodeHeapIterator();
 
     CodeHeapIterator(CodeHeapIterator const&) = delete;
     CodeHeapIterator& operator=(CodeHeapIterator const&) = delete;

src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.cpp

@@ -49,11 +49,24 @@ stack_walk_callback (
 
 	// Get the IP.
 	UINT_PTR control_pc = (UINT_PTR)frame->GetRegisterSet ()->ControlPC;
+
+	if (!frame->IsFrameless() && frame->GetFrame()->GetFrameIdentifier() == FrameIdentifier::PrestubMethodFrame) {
+		// At the PrestubMethodFrame, the ControlPC is not valid. Since the eventpipe stackwalk is actually only based on the ip, skip this frame.
+		return SWA_CONTINUE;
+	}
 	if (control_pc == 0) {
+#ifdef DEBUG
 		if (ep_stack_contents_get_length (stack_contents) == 0) {
 			// This happens for pinvoke stubs on the top of the stack.
-			return SWA_CONTINUE;
 		}
+		else {
+			EP_ASSERT (!"Unexpected null ControlPC in stack walk callback");
+		}
+#endif
+		// With FUNCTIONSONLY flag, we may hit frames without a meaningful control_pc, but with a valid MethodDesc.
+		// There is no point in reporting those frames as ep_stack_contents_append doesn't actually record the function
+		// in a Frame in release builds, it only records the control_pc.
+		return SWA_CONTINUE;
 	}
 
 	EP_ASSERT (control_pc != 0);

src/coreclr/vm/eventtrace.cpp

@@ -4975,22 +4975,57 @@ VOID ETW::MethodLog::SendEventsForJitMethodsHelper(LoaderAllocator *pLoaderAlloc
                                                    BOOL fSendILToNativeMapEvent,
                                                    BOOL fSendRichDebugInfoEvent,
                                                    BOOL fGetCodeIds)
+{
+    _ASSERTE(pLoaderAllocatorFilter == nullptr || pLoaderAllocatorFilter->IsCollectible());
+    _ASSERTE(pLoaderAllocatorFilter == nullptr || !fGetCodeIds);
+
+#ifdef FEATURE_JIT
+    SendEventsForJitMethodsHelper2(
+        ExecutionManager::GetEEJitManager()->GetCodeHeapIterator(pLoaderAllocatorFilter),
+        dwEventOptions,
+        fLoadOrDCStart,
+        fUnloadOrDCEnd,
+        fSendMethodEvent,
+        fSendILToNativeMapEvent,
+        fSendRichDebugInfoEvent,
+        fGetCodeIds);
+#endif // FEATURE_JIT
+
+#ifdef FEATURE_INTERPRETER
+    SendEventsForJitMethodsHelper2(
+        ExecutionManager::GetInterpreterJitManager()->GetCodeHeapIterator(pLoaderAllocatorFilter),
+        dwEventOptions,
+        fLoadOrDCStart,
+        fUnloadOrDCEnd,
+        fSendMethodEvent,
+        fSendILToNativeMapEvent,
+        fSendRichDebugInfoEvent,
+        fGetCodeIds);
+#endif // FEATURE_INTERPRETER
+}
+// Called be ETW::MethodLog::SendEventsForJitMethods
+// Sends the ETW events once our caller determines whether or not rejit locks can be acquired
+VOID ETW::MethodLog::SendEventsForJitMethodsHelper2(
+                                                   CodeHeapIterator heapIterator,
+                                                   DWORD dwEventOptions,
+                                                   BOOL fLoadOrDCStart,
+                                                   BOOL fUnloadOrDCEnd,
+                                                   BOOL fSendMethodEvent,
+                                                   BOOL fSendILToNativeMapEvent,
+                                                   BOOL fSendRichDebugInfoEvent,
+                                                   BOOL fGetCodeIds)
 {
     CONTRACTL{
         THROWS;
         GC_NOTRIGGER;
     } CONTRACTL_END;
 
-    _ASSERTE(pLoaderAllocatorFilter == nullptr || pLoaderAllocatorFilter->IsCollectible());
-    _ASSERTE(pLoaderAllocatorFilter == nullptr || !fGetCodeIds);
-
     // Set of methods for which we already have sent a MethodDetails event.
     // Only used when sending rich debug info that would otherwise send a lot
     // of duplicate events.
     MethodDescSet sentMethodDetailsSet;
     MethodDescSet* pSentMethodDetailsSet = fSendRichDebugInfoEvent ? &sentMethodDetailsSet : NULL;
 
-    CodeHeapIterator heapIterator = ExecutionManager::GetEEJitManager()->GetCodeHeapIterator(pLoaderAllocatorFilter);
     while (heapIterator.Next())
     {
         MethodDesc * pMD = heapIterator.GetMethod();
@@ -4999,14 +5034,6 @@ VOID ETW::MethodLog::SendEventsForJitMethodsHelper(LoaderAllocator *pLoaderAlloc
 
         PCODE codeStart = PINSTRToPCODE(heapIterator.GetMethodCode());
 
-#ifdef FEATURE_INTERPRETER
-        // Interpreter-TODO - If the code for this was generated by the interpreter, the native
-        // code start which will match up with the NativeCodeVersion/GetNativeCode result is the
-        // IntepreterPrecode, which is not what is found in the NativeCode portion of the MethodDesc
-        // or in the NativeCodeVersion. So we should build out the ability to get the right result
-        // as otherwise we will continue through the loop and skip all the interpreter data.
-#endif // FEATURE_INTERPRETER
-
         // Get info relevant to the native code version. In some cases, such as collectible loader
         // allocators, we don't support code versioning so we need to short circuit the call.
         // This also allows our caller to avoid having to pre-enter the relevant locks.
@@ -5022,7 +5049,7 @@ VOID ETW::MethodLog::SendEventsForJitMethodsHelper(LoaderAllocator *pLoaderAlloc
             if (nativeCodeVersion.IsNull())
             {
                 // The code version manager hasn't been updated with the jitted code
-                if (codeStart != pMD->GetNativeCode())
+                if (codeStart != MethodAndStartAddressToEECodeInfoPointer(pMD, (PCODE)NULL))
                 {
                     continue;
                 }
@@ -5035,7 +5062,7 @@ VOID ETW::MethodLog::SendEventsForJitMethodsHelper(LoaderAllocator *pLoaderAlloc
         }
         else
 #endif // FEATURE_CODE_VERSIONING
-        if (codeStart != pMD->GetNativeCode())
+        if (codeStart != MethodAndStartAddressToEECodeInfoPointer(pMD, (PCODE)NULL))
         {
             continue;
         }

src/tests/tracing/eventpipe/rundownvalidation/rundownvalidation.cs

@@ -65,9 +65,20 @@ public static int TestEntryPoint()
             rundownParser.MethodILToNativeMapDCStop += (eventData) => hasMethodILToNativeMap = true;
             rundownParser.LoaderAppDomainDCStop += (eventData) => hasAppDomainDCStop = true;
             return () =>
-                hasRuntimeStart && hasMethodDCStopInit && hasMethodDCStopComplete &&
+            {
+                Logger.logger.Log("hasRuntimeStart: " + hasRuntimeStart);
+                Logger.logger.Log("hasMethodDCStopInit: " + hasMethodDCStopInit);
+                Logger.logger.Log("hasMethodDCStopComplete: " + hasMethodDCStopComplete);
+                Logger.logger.Log("hasLoaderModuleDCStop: " + hasLoaderModuleDCStop);
+                Logger.logger.Log("hasLoaderDomainModuleDCStop: " + hasLoaderDomainModuleDCStop);
+                Logger.logger.Log("hasAssemblyModuleDCStop: " + hasAssemblyModuleDCStop);
+                Logger.logger.Log("hasMethodDCStopVerbose: " + hasMethodDCStopVerbose);
+                Logger.logger.Log("hasMethodILToNativeMap: " + hasMethodILToNativeMap);
+                Logger.logger.Log("hasAppDomainDCStop: " + hasAppDomainDCStop);
+                return hasRuntimeStart && hasMethodDCStopInit && hasMethodDCStopComplete &&
                 hasLoaderModuleDCStop && hasLoaderDomainModuleDCStop && hasAssemblyModuleDCStop &&
                 hasMethodDCStopVerbose && hasMethodILToNativeMap && hasAppDomainDCStop ? 100 : -1;
+            };
         };
     }
 }