diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 9cb439e650411f..c86d1ea94b7bd1 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -21174,10 +21174,9 @@ uint8_t* gc_heap::find_object (uint8_t* interior) heap_segment* seg = find_segment (interior, FALSE); if (seg) { -#ifdef FEATURE_CONSERVATIVE_GC if (interior >= heap_segment_allocated(seg)) return 0; -#endif + // If interior falls within the first free object at the beginning of a generation, // we don't have brick entry for it, and we may incorrectly treat it as on large object heap. int align_const = get_alignment_constant (heap_segment_read_only_p (seg) @@ -21208,12 +21207,8 @@ uint8_t* gc_heap::find_object (uint8_t* interior) heap_segment* seg = find_segment (interior, TRUE); if (seg) { -#ifdef FEATURE_CONSERVATIVE_GC if (interior >= heap_segment_allocated (seg)) return 0; -#else - assert (interior < heap_segment_allocated (seg)); -#endif uint8_t* o = find_first_object (interior, heap_segment_mem (seg)); return o; } diff --git a/src/coreclr/gc/gcinterface.h b/src/coreclr/gc/gcinterface.h index bfe02b7db9da3b..343f91412da6ac 100644 --- a/src/coreclr/gc/gcinterface.h +++ b/src/coreclr/gc/gcinterface.h @@ -952,11 +952,32 @@ inline GC_ALLOC_FLAGS& operator&=(GC_ALLOC_FLAGS& a, GC_ALLOC_FLAGS b) #define UNCHECKED_OBJECTREF_TO_OBJECTREF(obj) (obj) #endif +#if !defined(FEATURE_HIJACK) || defined(TARGET_UNIX) +#define FEATURE_SUPPORTS_STACK_LIMIT +#else +#if defined(FEATURE_CONSERVATIVE_GC) +// On Windows platforms when we hijack, we use SuspendThread from outside the thread, and if we suspend +// within an interruptible region, we can stop with the top Frame that is an InlinedCallFrame +// that is not active. In that situation, it is possible for the actual top of the stack to be a managed +// function which has a lower SP pointer than is reported from the InlinedCallFrame, or in the case of +// Windows x86 platforms, the InlinedCallFrame may report a GetCallSiteSP of 0, or of an value associated +// with some other callsite within the managed function that set up the InlinedCallFrame. This issue could +// be fixed by capturing the SP after suspension, and storing it on the thread object for use when setting +// stack_limit, but as conservative GC is not needed on any current Windows platforms, and the Unix scenario +// is fully functional, I've added this #error instead. +#error Conservative GC is not supported on this platform as the current computation of stack_limit is not strictly correct +#endif // FEATURE_CONSERVATIVE_GC +#endif // !defined(FEATURE_HIJACK) || defined(TARGET_UNIX) + struct ScanContext { Thread* thread_under_crawl; int thread_number; +#ifdef FEATURE_SUPPORTS_STACK_LIMIT uintptr_t stack_limit; // Lowest point on the thread stack that the scanning logic is permitted to read +#else + uintptr_t stack_limitUNUSED; // Keep the shape of ScanContext the same whether or not STACK_LIMIT is supported for use +#endif // FEATURE_SUPPORTS_STACK_LIMIT bool promotion; //TRUE: Promotion, FALSE: Relocation. bool concurrent; //TRUE: concurrent scanning void* _unused1; @@ -973,7 +994,11 @@ struct ScanContext thread_under_crawl = 0; thread_number = -1; +#ifdef FEATURE_SUPPORTS_STACK_LIMIT stack_limit = 0; +#else + stack_limitUNUSED = 0; +#endif // FEATURE_SUPPORTS_STACK_LIMIT promotion = false; concurrent = false; pMD = NULL; diff --git a/src/coreclr/inc/switches.h b/src/coreclr/inc/switches.h index d0ccfad62797c0..ed3c212f278d5b 100644 --- a/src/coreclr/inc/switches.h +++ b/src/coreclr/inc/switches.h @@ -148,7 +148,9 @@ // are treated as potential pinned interior pointers. When enabled, the runtime flag COMPLUS_GCCONSERVATIVE // determines dynamically whether GC is conservative. Note that appdomain unload, LCG and unloadable assemblies // do not work reliably with conservative GC. +#if defined(TARGET_UNIX) || !defined(FEATURE_HIJACK) // Windows currently doesn't support conservative GC when suspending interruptible code #define FEATURE_CONSERVATIVE_GC 1 +#endif #if (defined(TARGET_ARM) && (!defined(ARM_SOFTFP) || defined(CONFIGURABLE_ARM_ABI))) || defined(TARGET_ARM64) #define FEATURE_HFA diff --git a/src/coreclr/vm/frames.cpp b/src/coreclr/vm/frames.cpp index 8ab02c76423642..3692bcf981a6e4 100644 --- a/src/coreclr/vm/frames.cpp +++ b/src/coreclr/vm/frames.cpp @@ -1528,8 +1528,10 @@ BOOL TransitionFrame::Protects(OBJECTREF * ppORef) { WRAPPER_NO_CONTRACT; IsObjRefProtectedScanContext sc (ppORef); +#ifdef FEATURE_SUPPORTS_STACK_LIMIT // Set the stack limit for the scan to the SP of the managed frame above the transition frame sc.stack_limit = GetSP(); +#endif // FEATURE_SUPPORTS_STACK_LIMIT GcScanRoots (IsObjRefProtected, &sc); return sc.oref_protected; } diff --git a/src/coreclr/vm/gcenv.ee.cpp b/src/coreclr/vm/gcenv.ee.cpp index 5be5bc2d9ecf96..2ccf82b89bf995 100644 --- a/src/coreclr/vm/gcenv.ee.cpp +++ b/src/coreclr/vm/gcenv.ee.cpp @@ -114,6 +114,7 @@ static void ScanStackRoots(Thread * pThread, promote_func* fn, ScanContext* sc) IsGCSpecialThread() || (GetThread() == ThreadSuspend::GetSuspensionThread() && ThreadStore::HoldingThreadStore())); +#ifdef FEATURE_SUPPORTS_STACK_LIMIT Frame* pTopFrame = pThread->GetFrame(); Object ** topStack = (Object **)pTopFrame; if ((pTopFrame != ((Frame*)-1)) @@ -124,6 +125,7 @@ static void ScanStackRoots(Thread * pThread, promote_func* fn, ScanContext* sc) } sc->stack_limit = (uintptr_t)topStack; +#endif // FEATURE_SUPPORTS_STACK_LIMIT #ifdef FEATURE_CONSERVATIVE_GC if (g_pConfig->GetGCConservative()) diff --git a/src/coreclr/vm/siginfo.cpp b/src/coreclr/vm/siginfo.cpp index 2d0889c53240c0..f9ff540ed8912b 100644 --- a/src/coreclr/vm/siginfo.cpp +++ b/src/coreclr/vm/siginfo.cpp @@ -4906,7 +4906,9 @@ void PromoteCarefully(promote_func fn, // // Sanity check the stack scan limit // +#ifdef FEATURE_SUPPORTS_STACK_LIMIT assert(sc->stack_limit != 0); +#endif // FEATURE_SUPPORTS_STACK_LIMIT // Note that the base is at a higher address than the limit, since the stack // grows downwards. @@ -4914,8 +4916,11 @@ void PromoteCarefully(promote_func fn, // The reason is that on Unix, the stack size can be unlimited. In such case, the system can // shrink the current reserved stack space. That causes the real limit of the stack to move up and // the range can be reused for other purposes. But the sc->stack_limit is stable during the scan. - // Even on Windows, we care just about the stack above the stack_limit. - if ((sc->thread_under_crawl->IsAddressInStack(*ppObj)) && (PTR_TO_TADDR(*ppObj) >= sc->stack_limit)) + if ((sc->thread_under_crawl->IsAddressInStack(*ppObj)) +#ifdef FEATURE_SUPPORTS_STACK_LIMIT + && (PTR_TO_TADDR(*ppObj) >= sc->stack_limit) +#endif // FEATURE_SUPPORTS_STACK_LIMIT + ) { return; }