diff --git a/src/coreclr/src/debug/daccess/dacdbiimpl.cpp b/src/coreclr/src/debug/daccess/dacdbiimpl.cpp index 8f46469816faa..2bf8efaee3c19 100644 --- a/src/coreclr/src/debug/daccess/dacdbiimpl.cpp +++ b/src/coreclr/src/debug/daccess/dacdbiimpl.cpp @@ -3633,7 +3633,7 @@ void DacDbiInterfaceImpl::GetStackFramesFromException(VMPTR_Object vmObject, Dac currentFrame.vmDomainFile.SetHostPtr(pDomainFile); currentFrame.ip = currentElement.ip; currentFrame.methodDef = currentElement.pFunc->GetMemberDef(); - currentFrame.isLastForeignExceptionFrame = currentElement.fIsLastFrameFromForeignStackTrace; + currentFrame.isLastForeignExceptionFrame = (currentElement.flags & STEF_LAST_FRAME_FROM_FOREIGN_STACK_TRACE) != 0; } } } diff --git a/src/coreclr/src/debug/daccess/task.cpp b/src/coreclr/src/debug/daccess/task.cpp index 63f7e27a64be0..ac87333e6f1ef 100644 --- a/src/coreclr/src/debug/daccess/task.cpp +++ b/src/coreclr/src/debug/daccess/task.cpp @@ -4091,9 +4091,11 @@ ClrDataMethodInstance::GetILOffsetsByAddress( for (ULONG32 i = 0; i < numMap; i++) { if (codeOffset >= map[i].nativeStartOffset && - (((LONG)map[i].ilOffset == ICorDebugInfo::EPILOG && - !map[i].nativeEndOffset) || - codeOffset < map[i].nativeEndOffset)) + // Found the entry if it is the epilog or the last entry and nativeEndOffset == 0. For methods that don't + // have a epilog (i.e. IL_Throw is the last instruction) the last map entry has a valid IL offset (not EPILOG) + // and nativeEndOffset == 0. + ((((LONG)map[i].ilOffset == ICorDebugInfo::EPILOG || i == (numMap - 1)) && map[i].nativeEndOffset == 0) || + codeOffset < map[i].nativeEndOffset)) { hits++; diff --git a/src/coreclr/src/vm/clrex.h b/src/coreclr/src/vm/clrex.h index c715c54cbb6ae..34ca1c0823dd8 100644 --- a/src/coreclr/src/vm/clrex.h +++ b/src/coreclr/src/vm/clrex.h @@ -22,14 +22,23 @@ class AssemblySpec; class PEFile; class PEAssembly; +enum StackTraceElementFlags +{ + // Set if this element represents the last frame of the foreign exception stack trace + STEF_LAST_FRAME_FROM_FOREIGN_STACK_TRACE = 0x0001, + + // Set if the "ip" field has already been adjusted (decremented) + STEF_IP_ADJUSTED = 0x0002, +}; + +// This struct is used by SOS in the diagnostic repo. +// See: https://github.com/dotnet/diagnostics/blob/9ff35f13af2f03a68a166cfd53f1a4bb32425f2f/src/SOS/Strike/strike.cpp#L2245 struct StackTraceElement { UINT_PTR ip; UINT_PTR sp; PTR_MethodDesc pFunc; - // TRUE if this element represents the last frame of the foreign - // exception stack trace. - BOOL fIsLastFrameFromForeignStackTrace; + INT flags; // This is StackTraceElementFlags but it needs to be "int" sized for compatibility with SOS. bool operator==(StackTraceElement const & rhs) const { @@ -44,6 +53,8 @@ struct StackTraceElement } }; +// This struct is used by SOS in the diagnostic repo. +// See: https://github.com/dotnet/diagnostics/blob/9ff35f13af2f03a68a166cfd53f1a4bb32425f2f/src/SOS/Strike/strike.cpp#L2669 class StackTraceInfo { private: diff --git a/src/coreclr/src/vm/debugdebugger.cpp b/src/coreclr/src/vm/debugdebugger.cpp index 9cb271b840a2d..862082ff75f7a 100644 --- a/src/coreclr/src/vm/debugdebugger.cpp +++ b/src/coreclr/src/vm/debugdebugger.cpp @@ -510,7 +510,7 @@ FCIMPL4(void, DebugStackTrace::GetStackFramesInternal, // Set the BOOL indicating if the frame represents the last frame from a foreign exception stack trace. U1 *pIsLastFrameFromForeignExceptionStackTraceU1 = (U1 *)((BOOLARRAYREF)pStackFrameHelper->rgiLastFrameFromForeignExceptionStackTrace) ->GetDirectPointerToNonObjectElements(); - pIsLastFrameFromForeignExceptionStackTraceU1 [iNumValidFrames] = (U1) data.pElements[i].fIsLastFrameFromForeignStackTrace; + pIsLastFrameFromForeignExceptionStackTraceU1 [iNumValidFrames] = (U1)(data.pElements[i].flags & STEF_LAST_FRAME_FROM_FOREIGN_STACK_TRACE); } MethodDesc *pMethod = data.pElements[i].pFunc; @@ -939,7 +939,7 @@ void DebugStackTrace::GetStackFramesHelper(Frame *pStartFrame, // Do a 2nd pass outside of any locks. // This will compute IL offsets. - for(INT32 i = 0; i < pData->cElements; i++) + for (INT32 i = 0; i < pData->cElements; i++) { pData->pElements[i].InitPass2(); } @@ -1026,14 +1026,17 @@ StackWalkAction DebugStackTrace::GetStackFramesCallback(CrawlFrame* pCf, VOID* d dwNativeOffset = 0; } + // Pass on to InitPass2 that the IP has already been adjusted (decremented by 1) + INT flags = pCf->IsIPadjusted() ? STEF_IP_ADJUSTED : 0; + pData->pElements[pData->cElements].InitPass1( dwNativeOffset, pFunc, - ip); + ip, + flags); // We'll init the IL offsets outside the TSL lock. - ++pData->cElements; // Since we may be asynchronously walking another thread's stack, @@ -1109,7 +1112,7 @@ void DebugStackTrace::GetStackFramesFromException(OBJECTREF * e, // If we come across any frame representing foreign exception stack trace, // then set the flag indicating so. This will be used to allocate the // corresponding array in StackFrameHelper. - if (cur.fIsLastFrameFromForeignStackTrace) + if ((cur.flags & STEF_LAST_FRAME_FROM_FOREIGN_STACK_TRACE) != 0) { pData->fDoWeHaveAnyFramesFromForeignStackTrace = TRUE; } @@ -1134,9 +1137,11 @@ void DebugStackTrace::GetStackFramesFromException(OBJECTREF * e, dwNativeOffset = 0; } - pData->pElements[i].InitPass1(dwNativeOffset, pMD, (PCODE) cur.ip - , cur.fIsLastFrameFromForeignStackTrace - ); + pData->pElements[i].InitPass1( + dwNativeOffset, + pMD, + (PCODE)cur.ip, + cur.flags); #ifndef DACCESS_COMPILE pData->pElements[i].InitPass2(); #endif @@ -1156,8 +1161,8 @@ void DebugStackTrace::GetStackFramesFromException(OBJECTREF * e, void DebugStackTrace::DebugStackTraceElement::InitPass1( DWORD dwNativeOffset, MethodDesc *pFunc, - PCODE ip - , BOOL fIsLastFrameFromForeignStackTrace /*= FALSE*/ + PCODE ip, + INT flags ) { LIMITED_METHOD_CONTRACT; @@ -1169,7 +1174,7 @@ void DebugStackTrace::DebugStackTraceElement::InitPass1( this->pFunc = pFunc; this->dwOffset = dwNativeOffset; this->ip = ip; - this->fIsLastFrameFromForeignStackTrace = fIsLastFrameFromForeignStackTrace; + this->flags = flags; } #ifndef DACCESS_COMPILE @@ -1188,14 +1193,35 @@ void DebugStackTrace::DebugStackTraceElement::InitPass2() _ASSERTE(!ThreadStore::HoldingThreadStore()); - bool bRes = false; + bool bRes = false; #ifdef DEBUGGING_SUPPORTED // Calculate the IL offset using the debugging services if ((this->ip != NULL) && g_pDebugInterface) { + // To get the source line number of the actual code that threw an exception, the dwOffset needs to be + // adjusted in certain cases when calculating the IL offset. + // + // The dwOffset of the stack frame points to either: + // + // 1) The instruction that caused a hardware exception (div by zero, null ref, etc). + // 2) The instruction after the call to an internal runtime function (FCALL like IL_Throw, IL_Rethrow, + // JIT_OverFlow, etc.) that caused a software exception. + // 3) The instruction after the call to a managed function (non-leaf node). + // + // #2 and #3 are the cases that need to adjust dwOffset because they point after the call instruction + // and may point to the next (incorrect) IL instruction/source line. If STEF_IP_ADJUSTED is set, + // IP/dwOffset has already be decremented so don't decrement it again. + // + // When the dwOffset needs to be adjusted it is a lot simpler to decrement instead of trying to figure out + // the beginning of the instruction. It is enough for GetILOffsetFromNative to return the IL offset of the + // instruction throwing the exception. + bool fAdjustOffset = (this->flags & STEF_IP_ADJUSTED) == 0 && this->dwOffset >= STACKWALK_CONTROLPC_ADJUST_OFFSET; bRes = g_pDebugInterface->GetILOffsetFromNative( - pFunc, (LPCBYTE) this->ip, this->dwOffset, &this->dwILOffset); + pFunc, + (LPCBYTE)this->ip, + fAdjustOffset ? this->dwOffset - STACKWALK_CONTROLPC_ADJUST_OFFSET : this->dwOffset, + &this->dwILOffset); } #endif // !DEBUGGING_SUPPORTED diff --git a/src/coreclr/src/vm/debugdebugger.h b/src/coreclr/src/vm/debugdebugger.h index e8334ff85545b..92a5f601d08b2 100644 --- a/src/coreclr/src/vm/debugdebugger.h +++ b/src/coreclr/src/vm/debugdebugger.h @@ -96,22 +96,20 @@ class DebugStackTrace private: #endif // DACCESS_COMPILE struct DebugStackTraceElement { - DWORD dwOffset; // native offset + DWORD dwOffset; // native offset DWORD dwILOffset; MethodDesc *pFunc; PCODE ip; - // TRUE if this element represents the last frame of the foreign - // exception stack trace. - BOOL fIsLastFrameFromForeignStackTrace; + INT flags; // StackStackElementFlags // Initialization done under TSL. // This is used when first collecting the stack frame data. void InitPass1( DWORD dwNativeOffset, MethodDesc *pFunc, - PCODE ip - , BOOL fIsLastFrameFromForeignStackTrace = FALSE - ); + PCODE ip, + INT flags // StackStackElementFlags + ); // Initialization done outside the TSL. // This will init the dwILOffset field (and potentially anything else @@ -131,7 +129,7 @@ class DebugStackTrace DebugStackTraceElement* pElements; THREADBASEREF TargetThread; AppDomain *pDomain; - BOOL fDoWeHaveAnyFramesFromForeignStackTrace; + BOOL fDoWeHaveAnyFramesFromForeignStackTrace; GetStackFramesData() : skip(0), diff --git a/src/coreclr/src/vm/excep.cpp b/src/coreclr/src/vm/excep.cpp index ab1bde1c8c180..16fe93e235b38 100644 --- a/src/coreclr/src/vm/excep.cpp +++ b/src/coreclr/src/vm/excep.cpp @@ -2367,7 +2367,7 @@ void StackTraceInfo::SaveStackTrace(BOOL bAllowAllocMem, OBJECTHANDLE hThrowable // "numCurrentFrames" can be zero if the user created an EDI using // an unthrown exception. StackTraceElement & refLastElementFromForeignStackTrace = gc.stackTrace[numCurrentFrames - 1]; - refLastElementFromForeignStackTrace.fIsLastFrameFromForeignStackTrace = TRUE; + refLastElementFromForeignStackTrace.flags |= STEF_LAST_FRAME_FROM_FOREIGN_STACK_TRACE; } } @@ -3555,7 +3555,7 @@ BOOL StackTraceInfo::AppendElement(BOOL bAllowAllocMem, UINT_PTR currentIP, UINT // When we are building stack trace as we encounter managed frames during exception dispatch, // then none of those frames represent a stack trace from a foreign exception (as they represent // the current exception). Hence, set the corresponding flag to FALSE. - pStackTraceElem->fIsLastFrameFromForeignStackTrace = FALSE; + pStackTraceElem->flags = 0; // This is a workaround to fix the generation of stack traces from exception objects so that // they point to the line that actually generated the exception instead of the line @@ -3563,6 +3563,7 @@ BOOL StackTraceInfo::AppendElement(BOOL bAllowAllocMem, UINT_PTR currentIP, UINT if (!(pCf->HasFaulted() || pCf->IsIPadjusted()) && pStackTraceElem->ip != 0) { pStackTraceElem->ip -= 1; + pStackTraceElem->flags |= STEF_IP_ADJUSTED; } ++m_dFrameCount; diff --git a/src/coreclr/src/vm/exceptionhandling.cpp b/src/coreclr/src/vm/exceptionhandling.cpp index 37e85b28d00c5..f7fccb89b882f 100644 --- a/src/coreclr/src/vm/exceptionhandling.cpp +++ b/src/coreclr/src/vm/exceptionhandling.cpp @@ -1333,7 +1333,12 @@ void ExceptionTracker::InitializeCrawlFrameForExplicitFrame(CrawlFrame* pcfThisF INDEBUG(memset(pcfThisFrame, 0xCC, sizeof(*pcfThisFrame))); + // Clear various flags pcfThisFrame->isFrameless = false; + pcfThisFrame->isInterrupted = false; + pcfThisFrame->hasFaulted = false; + pcfThisFrame->isIPadjusted = false; + pcfThisFrame->pFrame = pFrame; pcfThisFrame->pFunc = pFrame->GetFunction(); @@ -1417,6 +1422,12 @@ void ExceptionTracker::InitializeCrawlFrame(CrawlFrame* pcfThisFrame, Thread* pT INDEBUG(memset(pcfThisFrame, 0xCC, sizeof(*pcfThisFrame))); pcfThisFrame->pRD = pRD; + // Clear various flags + pcfThisFrame->pFunc = NULL; + pcfThisFrame->isInterrupted = false; + pcfThisFrame->hasFaulted = false; + pcfThisFrame->isIPadjusted = false; + #ifdef FEATURE_INTERPRETER pcfThisFrame->pFrame = NULL; #endif // FEATURE_INTERPRETER @@ -1571,7 +1582,6 @@ void ExceptionTracker::InitializeCrawlFrame(CrawlFrame* pcfThisFrame, Thread* pT } pcfThisFrame->pThread = pThread; - pcfThisFrame->hasFaulted = false; Frame* pTopFrame = pThread->GetFrame(); pcfThisFrame->isIPadjusted = (FRAME_TOP != pTopFrame) && (pTopFrame->GetVTablePtr() != FaultingExceptionFrame::GetMethodFrameVPtr()); diff --git a/src/coreclr/src/vm/stackwalk.cpp b/src/coreclr/src/vm/stackwalk.cpp index 1e475b02ae874..a81667157b77e 100644 --- a/src/coreclr/src/vm/stackwalk.cpp +++ b/src/coreclr/src/vm/stackwalk.cpp @@ -1499,7 +1499,7 @@ void StackFrameIterator::ResetCrawlFrame() m_crawl.isFirst = true; m_crawl.isInterrupted = false; m_crawl.hasFaulted = false; - m_crawl.isIPadjusted = false; // can be removed + m_crawl.isIPadjusted = false; m_crawl.isNativeMarker = false; m_crawl.isProfilerDoStackSnapshot = !!(this->m_flags & PROFILER_DO_STACK_SNAPSHOT); diff --git a/src/coreclr/src/vm/stackwalk.h b/src/coreclr/src/vm/stackwalk.h index 77f62647e6477..e9a28f509d8d8 100644 --- a/src/coreclr/src/vm/stackwalk.h +++ b/src/coreclr/src/vm/stackwalk.h @@ -102,6 +102,7 @@ class CrawlFrame inline Frame* GetFrame() // will return NULL for "frameless methods" { LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE((int)isFrameless != 0xcc); if (isFrameless) return NULL; @@ -173,6 +174,7 @@ class CrawlFrame inline bool IsFrameless() { LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE((int)isFrameless != 0xcc); return isFrameless; } @@ -183,6 +185,7 @@ class CrawlFrame inline bool IsActiveFrame() { LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE((int)isFirst != 0xcc); return isFirst; } @@ -193,6 +196,7 @@ class CrawlFrame inline bool IsActiveFunc() { LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE((int)isFirst != 0xcc); return (pFunc && isFirst); } @@ -204,6 +208,7 @@ class CrawlFrame bool IsInterrupted() { LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE((int)isInterrupted != 0xcc); return (pFunc && isInterrupted /* && isFrameless?? */); } @@ -214,6 +219,7 @@ class CrawlFrame bool HasFaulted() { LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE((int)hasFaulted != 0xcc); return (pFunc && hasFaulted /* && isFrameless?? */); } @@ -225,6 +231,8 @@ class CrawlFrame bool IsNativeMarker() { LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE((int)isNativeMarker != 0xcc); + return isNativeMarker; } @@ -239,6 +247,8 @@ class CrawlFrame bool IsNoFrameTransition() { LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE((int)isNoFrameTransition != 0xcc); + return isNoFrameTransition; } @@ -249,6 +259,8 @@ class CrawlFrame TADDR GetNoFrameTransitionMarker() { LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE((int)isNoFrameTransition != 0xcc); + return (isNoFrameTransition ? taNoFrameTransitionMarker : NULL); } @@ -259,6 +271,7 @@ class CrawlFrame bool IsIPadjusted() { LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE((int)isIPadjusted != 0xcc); return (pFunc && isIPadjusted /* && isFrameless?? */); } @@ -336,6 +349,7 @@ class CrawlFrame GCInfoToken GetGCInfoToken() { LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE((int)isFrameless != 0xcc); _ASSERTE(isFrameless); return codeInfo.GetGCInfoToken(); } @@ -343,6 +357,7 @@ class CrawlFrame PTR_VOID GetGCInfo() { LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE((int)isFrameless != 0xcc); _ASSERTE(isFrameless); return codeInfo.GetGCInfo(); } @@ -350,6 +365,7 @@ class CrawlFrame const METHODTOKEN& GetMethodToken() { LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE((int)isFrameless != 0xcc); _ASSERTE(isFrameless); return codeInfo.GetMethodToken(); } @@ -357,6 +373,7 @@ class CrawlFrame unsigned GetRelOffset() { LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE((int)isFrameless != 0xcc); _ASSERTE(isFrameless); return codeInfo.GetRelOffset(); } @@ -364,6 +381,7 @@ class CrawlFrame IJitManager* GetJitManager() { LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE((int)isFrameless != 0xcc); _ASSERTE(isFrameless); return codeInfo.GetJitManager(); } @@ -371,6 +389,7 @@ class CrawlFrame ICodeManager* GetCodeManager() { LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE((int)isFrameless != 0xcc); _ASSERTE(isFrameless); return codeInfo.GetCodeManager(); }