From 9616b57ce68c9e84247f247e74dbd887b0cfdfe8 Mon Sep 17 00:00:00 2001 From: Maoni Stephens Date: Wed, 22 Jun 2022 16:10:17 -0700 Subject: [PATCH] NextObj fix (#71113) this is to fix #70231. for regions we could run into this situation - object is the last object before heap_segment_allocated (hs) T0 calls NextObj, gets next obj which starts at heap_segment_allocated (hs) T1 changes ephemeral_heap_segment to hs T0 does these comparisons (nextobj >= heap_segment_allocated(hs) && hs != hp->ephemeral_heap_segment) || (nextobj >= hp->alloc_allocated)) both still false because alloc_allocated hasn't been changed just yet (and the old alloc_allocated is larger than nextobj) T0 validates next obj, concludes its m_alignpad is not 0, asserts T1 forms an allocation context starting at heap_segment_allocated, clears memory so by the time the dump is taken, m_alignpad is already cleared (actually we clear it in a_fit_segment_end) I'm fixing this by saving the ephemeral_heap_segment and alloc_allocated and bail if nextobj is not on the saved eph seg or if those 2 saved values are no long in sync. --- src/coreclr/gc/gc.cpp | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index ec96a2e6fb65d..1021d669b2dc3 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -44526,9 +44526,25 @@ Object * GCHeap::NextObj (Object * object) return NULL; } - if ((nextobj < heap_segment_mem(hs)) || - (nextobj >= heap_segment_allocated(hs) && hs != hp->ephemeral_heap_segment) || - (nextobj >= hp->alloc_allocated)) + if (nextobj < heap_segment_mem (hs)) + { + return NULL; + } + + uint8_t* saved_alloc_allocated = hp->alloc_allocated; + heap_segment* saved_ephemeral_heap_segment = hp->ephemeral_heap_segment; + + // We still want to verify nextobj that lands between heap_segment_allocated and alloc_allocated + // on the ephemeral segment. In regions these 2 could be changed by another thread so we need + // to make sure they are still in sync by the time we check. If they are not in sync, we just + // bail which means we don't validate the next object during that small window and that's fine. + // + // We also miss validating nextobj if it's in the segment that just turned into the new ephemeral + // segment since we saved which is also a very small window and again that's fine. + if ((nextobj >= heap_segment_allocated (hs)) && + ((hs != saved_ephemeral_heap_segment) || + !in_range_for_segment(saved_alloc_allocated, saved_ephemeral_heap_segment) || + (nextobj >= saved_alloc_allocated))) { return NULL; }