From ce060415550334e598ee2efbc4beed0f07ede3f9 Mon Sep 17 00:00:00 2001 From: Koundinya Veluri Date: Sun, 11 Feb 2018 16:57:09 -0800 Subject: [PATCH] Fix stack trace population to get proper source/line info for tier 1 methods (#16302) Fixes https://github.com/dotnet/coreclr/issues/16224 --- src/debug/ee/debugger.cpp | 38 ++++++++++---- src/debug/ee/debugger.h | 2 +- src/debug/ee/functioninfo.cpp | 6 +-- src/vm/codeman.cpp | 13 +++++ src/vm/codeman.h | 3 ++ src/vm/eedbginterface.h | 2 + src/vm/eedbginterfaceimpl.cpp | 8 +++ src/vm/eedbginterfaceimpl.h | 2 + .../exceptions/stacktrace/Tier1StackTrace.cs | 58 ++++++++++++++++++++++ .../exceptions/stacktrace/Tier1StackTrace.csproj | 33 ++++++++++++ 10 files changed, 150 insertions(+), 15 deletions(-) create mode 100644 tests/src/baseservices/exceptions/stacktrace/Tier1StackTrace.cs create mode 100644 tests/src/baseservices/exceptions/stacktrace/Tier1StackTrace.csproj diff --git a/src/debug/ee/debugger.cpp b/src/debug/ee/debugger.cpp index 94792da..c5b8a63 100644 --- a/src/debug/ee/debugger.cpp +++ b/src/debug/ee/debugger.cpp @@ -14139,6 +14139,9 @@ bool Debugger::GetILOffsetFromNative (MethodDesc *pFunc, const BYTE *pbAddr, } CONTRACTL_END; + _ASSERTE(pFunc != NULL); + _ASSERTE(pbAddr != NULL); + if (!HasLazyData()) { DebuggerLockHolder dbgLockHolder(this); @@ -14152,23 +14155,36 @@ bool Debugger::GetILOffsetFromNative (MethodDesc *pFunc, const BYTE *pbAddr, pFunc = pFunc->GetWrappedMethodDesc(); } - DebuggerJitInfo *jitInfo = - GetJitInfo(pFunc, (const BYTE *)pbAddr); + if (pFunc->IsDynamicMethod()) + { + return false; + } - if (jitInfo != NULL) + DebuggerMethodInfo *methodInfo = GetOrCreateMethodInfo(pFunc->GetModule(), pFunc->GetMemberDef()); + if (methodInfo == NULL) { - CorDebugMappingResult map; - DWORD whichIDontCare; + return false; + } - *ilOffset = jitInfo->MapNativeOffsetToIL( - nativeOffset, - &map, - &whichIDontCare); + PCODE methodStartAddress = g_pEEInterface->GetNativeCodeStartAddress((PCODE)pbAddr); + if (methodStartAddress == NULL) + { + return false; + } - return true; + DebuggerJitInfo *jitInfo = methodInfo->FindOrCreateInitAndAddJitInfo(pFunc, methodStartAddress); + if (jitInfo == NULL) + { + return false; } - return false; + CorDebugMappingResult map; + DWORD whichIDontCare; + *ilOffset = jitInfo->MapNativeOffsetToIL( + nativeOffset, + &map, + &whichIDontCare); + return true; } /****************************************************************************** diff --git a/src/debug/ee/debugger.h b/src/debug/ee/debugger.h index 25e4682..6ba4e69 100644 --- a/src/debug/ee/debugger.h +++ b/src/debug/ee/debugger.h @@ -1010,7 +1010,7 @@ public: DebuggerJitInfo * FindJitInfo(MethodDesc * pMD, TADDR addrNativeStartAddr); // Creating the Jit-infos. - DebuggerJitInfo *FindOrCreateInitAndAddJitInfo(MethodDesc* fd, TADDR startAddr); + DebuggerJitInfo *FindOrCreateInitAndAddJitInfo(MethodDesc* fd, PCODE startAddr); DebuggerJitInfo *CreateInitAndAddJitInfo(MethodDesc* fd, TADDR startAddr, BOOL* jitInfoWasCreated); diff --git a/src/debug/ee/functioninfo.cpp b/src/debug/ee/functioninfo.cpp index fc443f9..d452111 100644 --- a/src/debug/ee/functioninfo.cpp +++ b/src/debug/ee/functioninfo.cpp @@ -1548,7 +1548,7 @@ DebuggerJitInfo * DebuggerMethodInfo::FindJitInfo(MethodDesc * pMD, * */ -DebuggerJitInfo *DebuggerMethodInfo::FindOrCreateInitAndAddJitInfo(MethodDesc* fd, TADDR startAddr) +DebuggerJitInfo *DebuggerMethodInfo::FindOrCreateInitAndAddJitInfo(MethodDesc* fd, PCODE startAddr) { CONTRACTL { @@ -1569,7 +1569,7 @@ DebuggerJitInfo *DebuggerMethodInfo::FindOrCreateInitAndAddJitInfo(MethodDesc* f if (startAddr == NULL) { // This will grab the start address for the current code version. - startAddr = (TADDR)g_pEEInterface->GetFunctionAddress(fd); + startAddr = g_pEEInterface->GetFunctionAddress(fd); if (startAddr == NULL) { return NULL; @@ -1577,7 +1577,7 @@ DebuggerJitInfo *DebuggerMethodInfo::FindOrCreateInitAndAddJitInfo(MethodDesc* f } else { - _ASSERTE(g_pEEInterface->GetNativeCodeMethodDesc((PCODE)startAddr) == fd); + _ASSERTE(g_pEEInterface->GetNativeCodeMethodDesc(startAddr) == fd); } // Check the lsit to see if we've already populated an entry for this JitInfo. diff --git a/src/vm/codeman.cpp b/src/vm/codeman.cpp index b3ff114..ec5313f 100644 --- a/src/vm/codeman.cpp +++ b/src/vm/codeman.cpp @@ -4156,6 +4156,19 @@ ExecutionManager::FindCodeRangeWithLock(PCODE currentPC) return GetRangeSection(currentPC); } + +//************************************************************************** +PCODE ExecutionManager::GetCodeStartAddress(PCODE currentPC) +{ + WRAPPER_NO_CONTRACT; + _ASSERTE(currentPC != NULL); + + EECodeInfo codeInfo(currentPC); + if (!codeInfo.IsValid()) + return NULL; + return (PCODE)codeInfo.GetStartAddress(); +} + //************************************************************************** MethodDesc * ExecutionManager::GetCodeMethodDesc(PCODE currentPC) { diff --git a/src/vm/codeman.h b/src/vm/codeman.h index 983e2ca..0663d59 100644 --- a/src/vm/codeman.h +++ b/src/vm/codeman.h @@ -1245,6 +1245,9 @@ public: // Special version with profiler hook static BOOL IsManagedCode(PCODE currentPC, HostCallPreference hostCallPreference, BOOL *pfFailedReaderLock); + // Returns method's start address for a given PC + static PCODE GetCodeStartAddress(PCODE currentPC); + // Returns methodDesc for given PC static MethodDesc * GetCodeMethodDesc(PCODE currentPC); diff --git a/src/vm/eedbginterface.h b/src/vm/eedbginterface.h index 241ef33..3c5bdc8 100644 --- a/src/vm/eedbginterface.h +++ b/src/vm/eedbginterface.h @@ -135,6 +135,8 @@ public: #endif // #ifndef DACCESS_COMPILE + virtual PCODE GetNativeCodeStartAddress(PCODE address) = 0; + virtual MethodDesc *GetNativeCodeMethodDesc(const PCODE address) = 0; #ifndef DACCESS_COMPILE diff --git a/src/vm/eedbginterfaceimpl.cpp b/src/vm/eedbginterfaceimpl.cpp index 976e6b6..69fe3bd 100644 --- a/src/vm/eedbginterfaceimpl.cpp +++ b/src/vm/eedbginterfaceimpl.cpp @@ -469,6 +469,14 @@ BOOL EEDbgInterfaceImpl::IsManagedNativeCode(const BYTE *address) return ExecutionManager::IsManagedCode((PCODE)address); } +PCODE EEDbgInterfaceImpl::GetNativeCodeStartAddress(PCODE address) +{ + WRAPPER_NO_CONTRACT; + _ASSERTE(address != NULL); + + return ExecutionManager::GetCodeStartAddress(address); +} + MethodDesc *EEDbgInterfaceImpl::GetNativeCodeMethodDesc(const PCODE address) { CONTRACT(MethodDesc *) diff --git a/src/vm/eedbginterfaceimpl.h b/src/vm/eedbginterfaceimpl.h index 7451246..7172e9a 100644 --- a/src/vm/eedbginterfaceimpl.h +++ b/src/vm/eedbginterfaceimpl.h @@ -122,6 +122,8 @@ public: BOOL IsManagedNativeCode(const BYTE *address); + PCODE GetNativeCodeStartAddress(PCODE address) DAC_UNEXPECTED(); + MethodDesc *GetNativeCodeMethodDesc(const PCODE address) DAC_UNEXPECTED(); #ifndef USE_GC_INFO_DECODER diff --git a/tests/src/baseservices/exceptions/stacktrace/Tier1StackTrace.cs b/tests/src/baseservices/exceptions/stacktrace/Tier1StackTrace.cs new file mode 100644 index 0000000..e5eba33 --- /dev/null +++ b/tests/src/baseservices/exceptions/stacktrace/Tier1StackTrace.cs @@ -0,0 +1,58 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Diagnostics; +using System.Runtime.CompilerServices; +using System.Threading; + +internal static class Program +{ + private static int Main() + { + const int Pass = 100, Fail = 1; + + string tier0StackTrace = Capture(true); + PromoteToTier1(() => Capture(false)); + string tier1StackTrace = Capture(true); + return tier0StackTrace == tier1StackTrace ? Pass : Fail; + } + + private static void PromoteToTier1(Action action) + { + // Call the method once to register a call for call counting + action(); + + // Allow time for call counting to begin + Thread.Sleep(500); + + // Call the method enough times to trigger tier 1 promotion + for (int i = 0; i < 100; i++) + { + action(); + } + + // Allow time for the method to be jitted at tier 1 + Thread.Sleep(500); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static string Capture(bool doWork) + { + if (!doWork) + { + return null; + } + + string stackTrace = new StackTrace(true).ToString().Trim(); + + // Remove the last line of the stack trace, which would correspond with Main() + int lastNewLineIndex = stackTrace.LastIndexOf('\n'); + if (lastNewLineIndex == -1) + { + return null; + } + return stackTrace.Substring(0, lastNewLineIndex).Trim(); + } +} diff --git a/tests/src/baseservices/exceptions/stacktrace/Tier1StackTrace.csproj b/tests/src/baseservices/exceptions/stacktrace/Tier1StackTrace.csproj new file mode 100644 index 0000000..c426338 --- /dev/null +++ b/tests/src/baseservices/exceptions/stacktrace/Tier1StackTrace.csproj @@ -0,0 +1,33 @@ + + + + + Debug + AnyCPU + {8758BFAC-7D36-4244-8A36-4C464C0AFA6D} + Exe + latest + true + true + 1 + + + + + + + + + + + + + + + -- 2.7.4