[clr-interp] Tracing fixes.

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,28 @@ 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. Instead use the address of the Prestub.
+		control_pc = 0;
+	}
 	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 if (!frame->IsFrameless() && frame->GetFrame()->GetFrameIdentifier() == FrameIdentifier::PrestubMethodFrame) {
+			// This happens for PrestubMethodFrame when we have interpreter frames on the stack
+			// In non-interpreter scenarios the control_pc is not set to anything meaningful, but reporting it doesn't seem to cause problems.
+		}
+		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

@@ -4990,102 +4990,115 @@ VOID ETW::MethodLog::SendEventsForJitMethodsHelper(LoaderAllocator *pLoaderAlloc
     MethodDescSet sentMethodDetailsSet;
     MethodDescSet* pSentMethodDetailsSet = fSendRichDebugInfoEvent ? &sentMethodDetailsSet : NULL;
 
-    CodeHeapIterator heapIterator = ExecutionManager::GetEEJitManager()->GetCodeHeapIterator(pLoaderAllocatorFilter);
-    while (heapIterator.Next())
+#ifdef FEATURE_INTERPRETER
+    for (int i = 0; i < 2; i++)
+#endif // FEATURE_INTERPRETER
     {
-        MethodDesc * pMD = heapIterator.GetMethod();
-        if (pMD == NULL)
-            continue;
+        CodeHeapIterator heapIterator = 
+#ifdef FEATURE_INTERPRETER
+        (i == 0) ?
+#endif
+            ExecutionManager::GetEEJitManager()->GetCodeHeapIterator(pLoaderAllocatorFilter)
+#ifdef FEATURE_INTERPRETER
+        :   ExecutionManager::GetInterpreterJitManager()->GetCodeHeapIterator(pLoaderAllocatorFilter)
+#endif
+            ;
+        while (heapIterator.Next())
+        {
+            MethodDesc * pMD = heapIterator.GetMethod();
+            if (pMD == NULL)
+                continue;
 
-        PCODE codeStart = PINSTRToPCODE(heapIterator.GetMethodCode());
+            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.
+            // 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.
-        // see code:#TableLockHolder
-        DWORD nativeCodeVersionId = 0;
-        ReJITID ilCodeId = 0;
-        NativeCodeVersion nativeCodeVersion;
+            // 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.
+            // see code:#TableLockHolder
+            DWORD nativeCodeVersionId = 0;
+            ReJITID ilCodeId = 0;
+            NativeCodeVersion nativeCodeVersion;
 #ifdef FEATURE_CODE_VERSIONING
-        if (fGetCodeIds && pMD->IsVersionable())
-        {
-            _ASSERTE(CodeVersionManager::IsLockOwnedByCurrentThread());
-            nativeCodeVersion = pMD->GetCodeVersionManager()->GetNativeCodeVersion(pMD, codeStart);
-            if (nativeCodeVersion.IsNull())
+            if (fGetCodeIds && pMD->IsVersionable())
             {
-                // The code version manager hasn't been updated with the jitted code
-                if (codeStart != pMD->GetNativeCode())
+                _ASSERTE(CodeVersionManager::IsLockOwnedByCurrentThread());
+                nativeCodeVersion = pMD->GetCodeVersionManager()->GetNativeCodeVersion(pMD, codeStart);
+                if (nativeCodeVersion.IsNull())
                 {
-                    continue;
+                    // The code version manager hasn't been updated with the jitted code
+                    if (codeStart != MethodAndStartAddressToEECodeInfoPointer(pMD, (PCODE)NULL))
+                    {
+                        continue;
+                    }
+                }
+                else
+                {
+                    nativeCodeVersionId = nativeCodeVersion.GetVersionId();
+                    ilCodeId = nativeCodeVersion.GetILCodeVersionId();
                 }
             }
             else
+#endif // FEATURE_CODE_VERSIONING
+            if (codeStart != MethodAndStartAddressToEECodeInfoPointer(pMD, (PCODE)NULL))
             {
-                nativeCodeVersionId = nativeCodeVersion.GetVersionId();
-                ilCodeId = nativeCodeVersion.GetILCodeVersionId();
+                continue;
             }
-        }
-        else
-#endif // FEATURE_CODE_VERSIONING
-        if (codeStart != pMD->GetNativeCode())
-        {
-            continue;
-        }
 
-        PrepareCodeConfig config(!nativeCodeVersion.IsNull() ? nativeCodeVersion : NativeCodeVersion(pMD), FALSE, FALSE);
+            PrepareCodeConfig config(!nativeCodeVersion.IsNull() ? nativeCodeVersion : NativeCodeVersion(pMD), FALSE, FALSE);
 
-        // When we're called to announce loads, then the methodload event itself must
-        // precede any supplemental events, so that the method load or method jitting
-        // event is the first event the profiler sees for that MethodID (and not, say,
-        // the MethodILToNativeMap event.)
-        if (fLoadOrDCStart)
-        {
-            if (fSendMethodEvent)
+            // When we're called to announce loads, then the methodload event itself must
+            // precede any supplemental events, so that the method load or method jitting
+            // event is the first event the profiler sees for that MethodID (and not, say,
+            // the MethodILToNativeMap event.)
+            if (fLoadOrDCStart)
             {
-                ETW::MethodLog::SendMethodEvent(
-                    pMD,
-                    dwEventOptions,
-                    TRUE,           // bIsJit
-                    NULL,           // namespaceOrClassName
-                    NULL,           // methodName
-                    NULL,           // methodSignature
-                    codeStart,
-                    &config,
-                    pSentMethodDetailsSet);
+                if (fSendMethodEvent)
+                {
+                    ETW::MethodLog::SendMethodEvent(
+                        pMD,
+                        dwEventOptions,
+                        TRUE,           // bIsJit
+                        NULL,           // namespaceOrClassName
+                        NULL,           // methodName
+                        NULL,           // methodSignature
+                        codeStart,
+                        &config,
+                        pSentMethodDetailsSet);
+                }
             }
-        }
 
-        // Send any supplemental events requested for this MethodID
-        if (fSendILToNativeMapEvent)
-            ETW::MethodLog::SendMethodILToNativeMapEvent(pMD, dwEventOptions, codeStart, nativeCodeVersionId, ilCodeId);
+            // Send any supplemental events requested for this MethodID
+            if (fSendILToNativeMapEvent)
+                ETW::MethodLog::SendMethodILToNativeMapEvent(pMD, dwEventOptions, codeStart, nativeCodeVersionId, ilCodeId);
 
-        if (fSendRichDebugInfoEvent)
-            ETW::MethodLog::SendMethodRichDebugInfo(pMD, codeStart, nativeCodeVersion.GetVersionId(), ilCodeId, pSentMethodDetailsSet);
+            if (fSendRichDebugInfoEvent)
+                ETW::MethodLog::SendMethodRichDebugInfo(pMD, codeStart, nativeCodeVersion.GetVersionId(), ilCodeId, pSentMethodDetailsSet);
 
-        // When we're called to announce unloads, then the methodunload event itself must
-        // come after any supplemental events, so that the method unload event is the
-        // last event the profiler sees for this MethodID
-        if (fUnloadOrDCEnd)
-        {
-            if (fSendMethodEvent)
+            // When we're called to announce unloads, then the methodunload event itself must
+            // come after any supplemental events, so that the method unload event is the
+            // last event the profiler sees for this MethodID
+            if (fUnloadOrDCEnd)
             {
-                ETW::MethodLog::SendMethodEvent(
-                    pMD,
-                    dwEventOptions,
-                    TRUE,           // bIsJit
-                    NULL,           // namespaceOrClassName
-                    NULL,           // methodName
-                    NULL,           // methodSignature
-                    codeStart,
-                    &config);
+                if (fSendMethodEvent)
+                {
+                    ETW::MethodLog::SendMethodEvent(
+                        pMD,
+                        dwEventOptions,
+                        TRUE,           // bIsJit
+                        NULL,           // namespaceOrClassName
+                        NULL,           // methodName
+                        NULL,           // methodSignature
+                        codeStart,
+                        &config);
+                }
             }
         }
     }

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