diff options
author | Michal Strehovský <MichalStrehovsky@users.noreply.github.com> | 2017-04-06 21:54:29 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-04-06 21:54:29 +0300 |
commit | 0c2c9ffd68aa575bbf1e90a839d6b23cdefb8f67 (patch) | |
tree | 7e04b2cc3387c1d4d08c2d89d24d732be525181a /src | |
parent | 312865e4a77cef8626cf23a4f9835470d1bc0526 (diff) | |
parent | 5b69cf3cab2f902a63793596d689fd0008f9b1fd (diff) |
Merge pull request #3232 from dotnet-bot/from-tfs
Merge changes from TFS
Diffstat (limited to 'src')
-rw-r--r-- | src/Native/Runtime/DebuggerHook.cpp | 18 | ||||
-rw-r--r-- | src/Native/Runtime/DebuggerHook.h | 10 | ||||
-rw-r--r-- | src/Native/Runtime/StackFrameIterator.cpp | 68 | ||||
-rw-r--r-- | src/Native/Runtime/StackFrameIterator.h | 5 | ||||
-rw-r--r-- | src/Native/Runtime/amd64/AsmOffsetsCpu.h | 6 | ||||
-rw-r--r-- | src/Native/Runtime/arm/AsmOffsetsCpu.h | 6 | ||||
-rw-r--r-- | src/Native/Runtime/gcrhenv.cpp | 3 | ||||
-rw-r--r-- | src/Native/Runtime/gcrhscan.cpp | 10 | ||||
-rw-r--r-- | src/Native/Runtime/i386/AsmOffsetsCpu.h | 6 | ||||
-rw-r--r-- | src/Native/Runtime/thread.cpp | 17 |
10 files changed, 126 insertions, 23 deletions
diff --git a/src/Native/Runtime/DebuggerHook.cpp b/src/Native/Runtime/DebuggerHook.cpp index a6f65831d..688eb69f6 100644 --- a/src/Native/Runtime/DebuggerHook.cpp +++ b/src/Native/Runtime/DebuggerHook.cpp @@ -11,6 +11,8 @@ GVAL_IMPL_INIT(UInt32, g_numGcProtectionRequests, 0); #ifndef DACCESS_COMPILE +/* static */ DebuggerProtectedBufferList* DebuggerHook::s_debuggerProtectedBuffers = nullptr; + // TODO: Tab to space, overall, just to make sure I will actually do it :) // TODO: This structure needs to match with DBI struct FuncEvalParameterCommand @@ -54,6 +56,8 @@ struct GcProtectionRequest { // If the request requires extra memory, allocate for it requests[i].address = (uint64_t)new (nothrow) uint8_t[requests[i].size]; + + // The debugger will handle the case when address is nullptr (we have to break our promise) } } @@ -66,7 +70,19 @@ struct GcProtectionRequest { if (requests[i].type == 1) { - // What shall I do? + // TODO: Release them when there should be gone + DebuggerProtectedBufferList* tail = DebuggerHook::s_debuggerProtectedBuffers; + s_debuggerProtectedBuffers = new (std::nothrow) DebuggerProtectedBufferList(); + if (s_debuggerProtectedBuffers == nullptr) + { + // TODO: We cannot handle the debugger request to protect a buffer (we have to break our promise) + } + else + { + s_debuggerProtectedBuffers->address = requests[i].address; + s_debuggerProtectedBuffers->size = requests[i].size; + s_debuggerProtectedBuffers->next = tail; + } } } diff --git a/src/Native/Runtime/DebuggerHook.h b/src/Native/Runtime/DebuggerHook.h index 26feedd06..fb8d0090c 100644 --- a/src/Native/Runtime/DebuggerHook.h +++ b/src/Native/Runtime/DebuggerHook.h @@ -17,10 +17,18 @@ #ifndef DACCESS_COMPILE +struct DebuggerProtectedBufferList +{ + UInt64 address; + UInt16 size; + struct DebuggerProtectedBufferList* next; +}; + class DebuggerHook { public: - static void OnBeforeGcCollection(); + static void OnBeforeGcCollection(); + static DebuggerProtectedBufferList* s_debuggerProtectedBuffers; }; #endif //!DACCESS_COMPILE diff --git a/src/Native/Runtime/StackFrameIterator.cpp b/src/Native/Runtime/StackFrameIterator.cpp index 979ef48d9..8c7d4f1d2 100644 --- a/src/Native/Runtime/StackFrameIterator.cpp +++ b/src/Native/Runtime/StackFrameIterator.cpp @@ -87,12 +87,13 @@ GVAL_IMPL_INIT(PTR_VOID, g_RhpRethrow2Addr, &RhpRethrow2); #define FAILFAST_OR_DAC_FAIL_UNCONDITIONALLY(msg) { ASSERT_UNCONDITIONALLY(msg); RhFailFast(); } #endif - PTR_PInvokeTransitionFrame GetPInvokeTransitionFrame(PTR_VOID pTransitionFrame) { return static_cast<PTR_PInvokeTransitionFrame>(pTransitionFrame); } +// TODO: Remove the assumption that there is only 1 func eval in progress +GVAL_IMPL_INIT(UInt64, g_debuggermagic, 0); StackFrameIterator::StackFrameIterator(Thread * pThreadToWalk, PTR_VOID pInitialTransitionFrame) { @@ -124,6 +125,7 @@ void StackFrameIterator::EnterInitialInvalidState(Thread * pThreadToWalk) m_HijackedReturnValueKind = GCRK_Unknown; m_pConservativeStackRangeLowerBound = NULL; m_pConservativeStackRangeUpperBound = NULL; + m_ShouldSkipRegularGcReporting = false; m_pendingFuncletFramePointer = NULL; m_pNextExInfo = pThreadToWalk->GetCurExInfo(); m_ControlPC = 0; @@ -1098,6 +1100,7 @@ UnwindOutOfCurrentManagedFrame: PTR_VOID pPreviousTransitionFrame; FAILFAST_OR_DAC_FAIL(GetCodeManager()->UnwindStackFrame(&m_methodInfo, &m_RegDisplay, &pPreviousTransitionFrame)); + bool doingFuncletUnwind = GetCodeManager()->IsFunclet(&m_methodInfo); if (pPreviousTransitionFrame != NULL) @@ -1357,8 +1360,23 @@ void StackFrameIterator::PrepareToYieldFrame() ASSERT(m_pInstance->FindCodeManagerByAddress(m_ControlPC)); + bool atDebuggerHijackSite = (this->m_ControlPC == (PTR_VOID)(TADDR)g_debuggermagic); + + if (atDebuggerHijackSite) + { + FAILFAST_OR_DAC_FAIL_MSG(m_pConservativeStackRangeLowerBound != NULL, + "Debugger hijack unwind is missing the required conservative range from a preceding transition thunk."); + } + if (m_dwFlags & ApplyReturnAddressAdjustment) + { + FAILFAST_OR_DAC_FAIL_MSG(!atDebuggerHijackSite, + "EH stack walk is attempting to propagate an exception across a debugger hijack site."); + m_ControlPC = AdjustReturnAddressBackward(m_ControlPC); + } + + m_ShouldSkipRegularGcReporting = false; // Each time a managed frame is yielded, configure the iterator to explicitly indicate // whether or not unwinding to the current frame has revealed a stack range that must be @@ -1383,8 +1401,45 @@ void StackFrameIterator::PrepareToYieldFrame() // that the upper bound computation never mutates m_RegDisplay. CalculateCurrentMethodState(); ASSERT(IsValid()); - UIntNative rawUpperBound = GetCodeManager()->GetConservativeUpperBoundForOutgoingArgs(&m_methodInfo, &m_RegDisplay); - m_pConservativeStackRangeUpperBound = (PTR_UIntNative)rawUpperBound; + + if (!atDebuggerHijackSite) + { + UIntNative rawUpperBound = GetCodeManager()->GetConservativeUpperBoundForOutgoingArgs(&m_methodInfo, &m_RegDisplay); + m_pConservativeStackRangeUpperBound = (PTR_UIntNative)rawUpperBound; + } + else + { + // Debugger hijack points differ from all other unwind cases in that they are not + // guaranteed to be GC safe points, which implies that regular GC reporting will not + // protect the GC references that the function was using (in registers and/or in local + // stack slots) at the time of the hijack. + // + // GC references held in registers at the time of the hijack are reported by the + // debugger and therefore do not need to be handled here. (The debugger does this by + // conservatively reporting the entire CONTEXT record which lists the full register + // set that was observed when the thread was stopped at the hijack point.) + // + // This code is therefore only responsible for reporting the GC references that were + // stored on the stack at the time of the hijack. Conceptually, this is done by + // conservatively reporting the entire stack frame. Since debugger hijack unwind + // always occurs via a UniversalTransitionThunk, the conservative lower bound + // published by the thunk can be used as a workable lower bound for the entire stack + // frame. + // + // Computing a workable upper bound is more difficult, especially because the stack + // frame of a funclet can contain FP-relative locals which reside arbitrarily far up + // the stack compared to the current SP. The top of the thread's stack is currently + // used as an extremely conservative upper bound as away to cover all cases without + // introducing more stack walker complexity. + + PTR_VOID pStackLow; + PTR_VOID pStackHigh; +#ifndef DACCESS_COMPILE + m_pThread->GetStackBounds(&pStackLow, &pStackHigh); +#endif + m_pConservativeStackRangeUpperBound = (PTR_UIntNative)pStackHigh; + m_ShouldSkipRegularGcReporting = true; + } ASSERT(m_pConservativeStackRangeLowerBound != NULL); ASSERT(m_pConservativeStackRangeUpperBound != NULL); @@ -1541,7 +1596,7 @@ PTR_VOID StackFrameIterator::AdjustReturnAddressBackward(PTR_VOID controlPC) // static StackFrameIterator::ReturnAddressCategory StackFrameIterator::CategorizeUnadjustedReturnAddress(PTR_VOID returnAddress) -{ +{ #if defined(USE_PORTABLE_HELPERS) // @TODO: CORERT: no portable thunks are defined return InManagedCode; @@ -1584,6 +1639,11 @@ StackFrameIterator::ReturnAddressCategory StackFrameIterator::CategorizeUnadjust #endif // defined(USE_PORTABLE_HELPERS) } +bool StackFrameIterator::ShouldSkipRegularGcReporting() +{ + return m_ShouldSkipRegularGcReporting; +} + #ifndef DACCESS_COMPILE COOP_PINVOKE_HELPER(Boolean, RhpSfiInit, (StackFrameIterator* pThis, PAL_LIMITED_CONTEXT* pStackwalkCtx, Boolean instructionFault)) diff --git a/src/Native/Runtime/StackFrameIterator.h b/src/Native/Runtime/StackFrameIterator.h index 214a7aed6..343b52e84 100644 --- a/src/Native/Runtime/StackFrameIterator.h +++ b/src/Native/Runtime/StackFrameIterator.h @@ -56,6 +56,10 @@ public: bool HasStackRangeToReportConservatively(); void GetStackRangeToReportConservatively(PTR_RtuObjectRef * ppLowerBound, PTR_RtuObjectRef * ppUpperBound); + // Debugger Hijacked frame looks very much like a usual managed frame except when the + // frame must be reported conservatively, and when that happens, regular GC reporting should be skipped + bool ShouldSkipRegularGcReporting(); + private: // The invoke of a funclet is a bit special and requires an assembly thunk, but we don't want to break the // stackwalk due to this. So this routine will unwind through the assembly thunks used to invoke funclets. @@ -187,5 +191,6 @@ protected: PreservedRegPtrs m_funcletPtrs; // @TODO: Placing the 'scratch space' in the StackFrameIterator is not // preferred because not all StackFrameIterators require this storage // space. However, the implementation simpler by doing it this way. + bool m_ShouldSkipRegularGcReporting; }; diff --git a/src/Native/Runtime/amd64/AsmOffsetsCpu.h b/src/Native/Runtime/amd64/AsmOffsetsCpu.h index 5dd0c02ff..1bc09bad0 100644 --- a/src/Native/Runtime/amd64/AsmOffsetsCpu.h +++ b/src/Native/Runtime/amd64/AsmOffsetsCpu.h @@ -9,7 +9,7 @@ // NOTE: the offsets MUST be in hex notation WITHOUT the 0x prefix #ifndef UNIX_AMD64_ABI -PLAT_ASM_SIZEOF(250, ExInfo) +PLAT_ASM_SIZEOF(260, ExInfo) PLAT_ASM_OFFSET(0, ExInfo, m_pPrevExInfo) PLAT_ASM_OFFSET(8, ExInfo, m_pExContext) PLAT_ASM_OFFSET(10, ExInfo, m_exception) @@ -17,7 +17,7 @@ PLAT_ASM_OFFSET(18, ExInfo, m_kind) PLAT_ASM_OFFSET(19, ExInfo, m_passNumber) PLAT_ASM_OFFSET(1c, ExInfo, m_idxCurClause) PLAT_ASM_OFFSET(20, ExInfo, m_frameIter) -PLAT_ASM_OFFSET(240, ExInfo, m_notifyDebuggerSP) +PLAT_ASM_OFFSET(250, ExInfo, m_notifyDebuggerSP) PLAT_ASM_OFFSET(0, PInvokeTransitionFrame, m_RIP) PLAT_ASM_OFFSET(8, PInvokeTransitionFrame, m_FramePointer) @@ -25,7 +25,7 @@ PLAT_ASM_OFFSET(10, PInvokeTransitionFrame, m_pThread) PLAT_ASM_OFFSET(18, PInvokeTransitionFrame, m_dwFlags) PLAT_ASM_OFFSET(20, PInvokeTransitionFrame, m_PreservedRegs) -PLAT_ASM_SIZEOF(220, StackFrameIterator) +PLAT_ASM_SIZEOF(230, StackFrameIterator) PLAT_ASM_OFFSET(10, StackFrameIterator, m_FramePointer) PLAT_ASM_OFFSET(18, StackFrameIterator, m_ControlPC) PLAT_ASM_OFFSET(20, StackFrameIterator, m_RegDisplay) diff --git a/src/Native/Runtime/arm/AsmOffsetsCpu.h b/src/Native/Runtime/arm/AsmOffsetsCpu.h index 1c1d1978c..baf8da385 100644 --- a/src/Native/Runtime/arm/AsmOffsetsCpu.h +++ b/src/Native/Runtime/arm/AsmOffsetsCpu.h @@ -8,7 +8,7 @@ // // NOTE: the offsets MUST be in hex notation WITHOUT the 0x prefix -PLAT_ASM_SIZEOF(130, ExInfo) +PLAT_ASM_SIZEOF(138, ExInfo) PLAT_ASM_OFFSET(0, ExInfo, m_pPrevExInfo) PLAT_ASM_OFFSET(4, ExInfo, m_pExContext) PLAT_ASM_OFFSET(8, ExInfo, m_exception) @@ -16,7 +16,7 @@ PLAT_ASM_OFFSET(0c, ExInfo, m_kind) PLAT_ASM_OFFSET(0d, ExInfo, m_passNumber) PLAT_ASM_OFFSET(10, ExInfo, m_idxCurClause) PLAT_ASM_OFFSET(18, ExInfo, m_frameIter) -PLAT_ASM_OFFSET(128, ExInfo, m_notifyDebuggerSP) +PLAT_ASM_OFFSET(130, ExInfo, m_notifyDebuggerSP) PLAT_ASM_OFFSET(4, PInvokeTransitionFrame, m_RIP) PLAT_ASM_OFFSET(8, PInvokeTransitionFrame, m_FramePointer) @@ -24,7 +24,7 @@ PLAT_ASM_OFFSET(0c, PInvokeTransitionFrame, m_pThread) PLAT_ASM_OFFSET(10, PInvokeTransitionFrame, m_dwFlags) PLAT_ASM_OFFSET(14, PInvokeTransitionFrame, m_PreservedRegs) -PLAT_ASM_SIZEOF(110, StackFrameIterator) +PLAT_ASM_SIZEOF(118, StackFrameIterator) PLAT_ASM_OFFSET(08, StackFrameIterator, m_FramePointer) PLAT_ASM_OFFSET(0c, StackFrameIterator, m_ControlPC) PLAT_ASM_OFFSET(10, StackFrameIterator, m_RegDisplay) diff --git a/src/Native/Runtime/gcrhenv.cpp b/src/Native/Runtime/gcrhenv.cpp index 782572acc..53f83eb7a 100644 --- a/src/Native/Runtime/gcrhenv.cpp +++ b/src/Native/Runtime/gcrhenv.cpp @@ -72,6 +72,7 @@ GPTR_IMPL(EEType, g_pFreeObjectEEType); +#include "DebuggerHook.h" #ifndef DACCESS_COMPILE @@ -920,6 +921,8 @@ void GCToEEInterface::RestartEE(bool /*bFinishedGC*/) void GCToEEInterface::GcStartWork(int condemned, int /*max_gen*/) { + DebuggerHook::OnBeforeGcCollection(); + // Invoke any registered callouts for the start of the collection. RestrictedCallouts::InvokeGcCallouts(GCRC_StartCollection, condemned); } diff --git a/src/Native/Runtime/gcrhscan.cpp b/src/Native/Runtime/gcrhscan.cpp index 1216a99bc..31ac4d7a5 100644 --- a/src/Native/Runtime/gcrhscan.cpp +++ b/src/Native/Runtime/gcrhscan.cpp @@ -36,6 +36,8 @@ #ifndef DACCESS_COMPILE +void GcEnumObjectsConservatively(PTR_PTR_Object ppLowerBound, PTR_PTR_Object ppUpperBound, EnumGcRefCallbackFunc * fnGcEnumRef, EnumGcRefScanContext * pSc); + void EnumAllStaticGCRefs(EnumGcRefCallbackFunc * fn, EnumGcRefScanContext * sc) { GetRuntimeInstance()->EnumAllStaticGCRefs(reinterpret_cast<void*>(fn), sc); @@ -47,7 +49,13 @@ void EnumAllStaticGCRefs(EnumGcRefCallbackFunc * fn, EnumGcRefScanContext * sc) void GCToEEInterface::GcScanRoots(EnumGcRefCallbackFunc * fn, int condemned, int max_gen, EnumGcRefScanContext * sc) { - DebuggerHook::OnBeforeGcCollection(); + DebuggerProtectedBufferList* cursor = DebuggerHook::s_debuggerProtectedBuffers; + while (cursor != nullptr) + { + GcEnumObjectsConservatively((PTR_PTR_Object)cursor->address, (PTR_PTR_Object)(cursor->address + cursor->size), fn, sc); + cursor = cursor->next; + } + // STRESS_LOG1(LF_GCROOTS, LL_INFO10, "GCScan: Phase = %s\n", sc->promotion ? "promote" : "relocate"); FOREACH_THREAD(pThread) diff --git a/src/Native/Runtime/i386/AsmOffsetsCpu.h b/src/Native/Runtime/i386/AsmOffsetsCpu.h index a85192760..d7f70f4f2 100644 --- a/src/Native/Runtime/i386/AsmOffsetsCpu.h +++ b/src/Native/Runtime/i386/AsmOffsetsCpu.h @@ -8,7 +8,7 @@ // // NOTE: the offsets MUST be in hex notation WITHOUT the 0x prefix -PLAT_ASM_SIZEOF(b8, ExInfo) +PLAT_ASM_SIZEOF(bc, ExInfo) PLAT_ASM_OFFSET(0, ExInfo, m_pPrevExInfo) PLAT_ASM_OFFSET(4, ExInfo, m_pExContext) PLAT_ASM_OFFSET(8, ExInfo, m_exception) @@ -16,7 +16,7 @@ PLAT_ASM_OFFSET(0c, ExInfo, m_kind) PLAT_ASM_OFFSET(0d, ExInfo, m_passNumber) PLAT_ASM_OFFSET(10, ExInfo, m_idxCurClause) PLAT_ASM_OFFSET(14, ExInfo, m_frameIter) -PLAT_ASM_OFFSET(b4, ExInfo, m_notifyDebuggerSP) +PLAT_ASM_OFFSET(b8, ExInfo, m_notifyDebuggerSP) PLAT_ASM_OFFSET(0, PInvokeTransitionFrame, m_RIP) PLAT_ASM_OFFSET(4, PInvokeTransitionFrame, m_FramePointer) @@ -24,7 +24,7 @@ PLAT_ASM_OFFSET(8, PInvokeTransitionFrame, m_pThread) PLAT_ASM_OFFSET(0c, PInvokeTransitionFrame, m_dwFlags) PLAT_ASM_OFFSET(10, PInvokeTransitionFrame, m_PreservedRegs) -PLAT_ASM_SIZEOF(a0, StackFrameIterator) +PLAT_ASM_SIZEOF(a4, StackFrameIterator) PLAT_ASM_OFFSET(08, StackFrameIterator, m_FramePointer) PLAT_ASM_OFFSET(0c, StackFrameIterator, m_ControlPC) PLAT_ASM_OFFSET(10, StackFrameIterator, m_RegDisplay) diff --git a/src/Native/Runtime/thread.cpp b/src/Native/Runtime/thread.cpp index 62a458cbd..db1668383 100644 --- a/src/Native/Runtime/thread.cpp +++ b/src/Native/Runtime/thread.cpp @@ -481,13 +481,16 @@ void Thread::GcScanRootsWorker(void * pfnEnumCallback, void * pvCallbackData, St frameIterator.CalculateCurrentMethodState(); STRESS_LOG1(LF_GCROOTS, LL_INFO1000, "Scanning method %pK\n", (void*)frameIterator.GetRegisterSet()->IP); - - RedhawkGCInterface::EnumGcRefs(frameIterator.GetCodeManager(), - frameIterator.GetMethodInfo(), - frameIterator.GetEffectiveSafePointAddress(), - frameIterator.GetRegisterSet(), - pfnEnumCallback, - pvCallbackData); + + if (!frameIterator.ShouldSkipRegularGcReporting()) + { + RedhawkGCInterface::EnumGcRefs(frameIterator.GetCodeManager(), + frameIterator.GetMethodInfo(), + frameIterator.GetEffectiveSafePointAddress(), + frameIterator.GetRegisterSet(), + pfnEnumCallback, + pvCallbackData); + } // Each enumerated frame (including the first one) may have an associated stack range we need to // report conservatively (every pointer aligned value that looks like it might be a GC reference is |