diff --git a/src/coreclr/src/vm/clrex.h b/src/coreclr/src/vm/clrex.h index c715c54cbb6ae..fdb95a97db91c 100644 --- a/src/coreclr/src/vm/clrex.h +++ b/src/coreclr/src/vm/clrex.h @@ -30,6 +30,9 @@ struct StackTraceElement // TRUE if this element represents the last frame of the foreign // exception stack trace. BOOL fIsLastFrameFromForeignStackTrace; + // TRUE if the ip needs to be adjusted back to the calling or + // throwing instruction. + BOOL fAdjustOffset; bool operator==(StackTraceElement const & rhs) const { diff --git a/src/coreclr/src/vm/debugdebugger.cpp b/src/coreclr/src/vm/debugdebugger.cpp index 9cb271b840a2d..a4833f95d5339 100644 --- a/src/coreclr/src/vm/debugdebugger.cpp +++ b/src/coreclr/src/vm/debugdebugger.cpp @@ -939,8 +939,9 @@ 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++) { + // pStartFrame is NULL when collecting frames for current thread. pData->pElements[i].InitPass2(); } @@ -1026,14 +1027,20 @@ StackWalkAction DebugStackTrace::GetStackFramesCallback(CrawlFrame* pCf, VOID* d dwNativeOffset = 0; } + // We can assume that dwNativeOffset points to an the instruction after [call IL_Throw] when these conditions are met: + // 1. !pCf->HasFaulted() - It wasn't a "hardware" exception (Access violation, dev by 0, etc.) + // 2. !pCf->IsIPadjusted() - It hasn't been previously adjusted to point to a call (like [call IL_Throw], etc.) + BOOL fAdjustOffset = !pCf->HasFaulted() && !pCf->IsIPadjusted(); + pData->pElements[pData->cElements].InitPass1( dwNativeOffset, pFunc, - ip); + ip, + FALSE, + fAdjustOffset); // We'll init the IL offsets outside the TSL lock. - ++pData->cElements; // Since we may be asynchronously walking another thread's stack, @@ -1134,9 +1141,12 @@ 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.fIsLastFrameFromForeignStackTrace, + cur.fAdjustOffset); #ifndef DACCESS_COMPILE pData->pElements[i].InitPass2(); #endif @@ -1156,8 +1166,9 @@ void DebugStackTrace::GetStackFramesFromException(OBJECTREF * e, void DebugStackTrace::DebugStackTraceElement::InitPass1( DWORD dwNativeOffset, MethodDesc *pFunc, - PCODE ip - , BOOL fIsLastFrameFromForeignStackTrace /*= FALSE*/ + PCODE ip, + BOOL fIsLastFrameFromForeignStackTrace, /*= FALSE*/ + BOOL fAdjustOffset /*= FALSE*/ ) { LIMITED_METHOD_CONTRACT; @@ -1170,6 +1181,7 @@ void DebugStackTrace::DebugStackTraceElement::InitPass1( this->dwOffset = dwNativeOffset; this->ip = ip; this->fIsLastFrameFromForeignStackTrace = fIsLastFrameFromForeignStackTrace; + this->fAdjustOffset = fAdjustOffset; } #ifndef DACCESS_COMPILE @@ -1188,14 +1200,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 instructionand + // may point to the next (incorrect) IL instruction/source line. fAdjustOffset is false if the dwOffset/ip + // has already been adjusted or is case #1. fAdjustOffset is true if the dwOffset/IP hasn't been already + // adjusted for cases #2 or #3. + // + // 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. bRes = g_pDebugInterface->GetILOffsetFromNative( - pFunc, (LPCBYTE) this->ip, this->dwOffset, &this->dwILOffset); + pFunc, + (LPCBYTE)this->ip, + this->fAdjustOffset && this->dwOffset >= STACKWALK_CONTROLPC_ADJUST_OFFSET ? 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..d807896887342 100644 --- a/src/coreclr/src/vm/debugdebugger.h +++ b/src/coreclr/src/vm/debugdebugger.h @@ -102,16 +102,20 @@ class DebugStackTrace PCODE ip; // TRUE if this element represents the last frame of the foreign // exception stack trace. - BOOL fIsLastFrameFromForeignStackTrace; + BOOL fIsLastFrameFromForeignStackTrace; + // TRUE if the dwOffset (native offset) needs to be adjusted back to the + // calling or throwing instruction. + BOOL fAdjustOffset; // 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, + BOOL fIsLastFrameFromForeignStackTrace = FALSE, + BOOL fAdjustOffset = FALSE + ); // Initialization done outside the TSL. // This will init the dwILOffset field (and potentially anything else @@ -131,7 +135,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..a93ea5722d832 100644 --- a/src/coreclr/src/vm/excep.cpp +++ b/src/coreclr/src/vm/excep.cpp @@ -3557,10 +3557,15 @@ BOOL StackTraceInfo::AppendElement(BOOL bAllowAllocMem, UINT_PTR currentIP, UINT // the current exception). Hence, set the corresponding flag to FALSE. pStackTraceElem->fIsLastFrameFromForeignStackTrace = FALSE; + // We can assume that IP points to an the instruction after [call IL_Throw] when these conditions are met: + // 1. !pCf->HasFaulted() - It wasn't a "hardware" exception (Access violation, dev by 0, etc.) + // 2. !pCf->IsIPadjusted() - It hasn't been previously adjusted to point to the call (i.e. like [call IL_Throw], etc.) + pStackTraceElem->fAdjustOffset = !pCf->HasFaulted() && !pCf->IsIPadjusted(); + // 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 // following. - if (!(pCf->HasFaulted() || pCf->IsIPadjusted()) && pStackTraceElem->ip != 0) + if (!pStackTraceElem->fAdjustOffset && pStackTraceElem->ip != 0) { pStackTraceElem->ip -= 1; } diff --git a/src/coreclr/src/vm/exceptionhandling.cpp b/src/coreclr/src/vm/exceptionhandling.cpp index 37e85b28d00c5..a614b299b0cbb 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 because the above INDEBUG sets them to true 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 because the above INDEBUG sets them to true + 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.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(); }