From 2acef87b802f573e28e1a74f6e4c7c67ca422222 Mon Sep 17 00:00:00 2001
From: Andrew Au <andrewau@microsoft.com>
Date: Fri, 21 Oct 2022 09:28:14 -0700
Subject: [PATCH] Filter out addresses that are not in bookkeeping range during
 background promote (#77067)

---
 src/coreclr/gc/gc.cpp   | 76 ++++++++++++++++++++++++++---------------
 src/coreclr/gc/gcpriv.h |  6 ++--
 2 files changed, 51 insertions(+), 31 deletions(-)

diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp
index 8a17c23ff39a9..6564fb239d279 100644
--- a/src/coreclr/gc/gc.cpp
+++ b/src/coreclr/gc/gc.cpp
@@ -7907,6 +7907,32 @@ BOOL gc_heap::ephemeral_pointer_p (uint8_t* o)
 #endif //USE_REGIONS
 }
 
+// This needs to check the range that's covered by bookkeeping because find_object will
+// need to look at the brick table.
+inline
+bool gc_heap::is_in_find_object_range (uint8_t* o)
+{
+    if (o == nullptr)
+    {
+        return false;
+    }
+#if defined(USE_REGIONS) && defined(FEATURE_CONSERVATIVE_GC)
+    return ((o >= g_gc_lowest_address) && (o < bookkeeping_covered_committed));
+#else //USE_REGIONS && FEATURE_CONSERVATIVE_GC
+    if ((o >= g_gc_lowest_address) && (o < g_gc_highest_address))
+    {
+#ifdef USE_REGIONS
+        assert ((o >= g_gc_lowest_address) && (o < bookkeeping_covered_committed));
+#endif //USE_REGIONS
+        return true;
+    }
+    else
+    {
+        return false;
+    }
+#endif //USE_REGIONS && FEATURE_CONSERVATIVE_GC
+}
+
 #ifdef USE_REGIONS
 // This assumes o is guaranteed to be in a region.
 inline
@@ -7927,14 +7953,6 @@ bool gc_heap::is_in_condemned_gc (uint8_t* o)
     return true;
 }
 
-// This needs to check the range that's covered by bookkeeping because find_object will
-// need to look at the brick table.
-inline
-bool gc_heap::is_in_bookkeeping_range (uint8_t* o)
-{
-    return ((o >= g_gc_lowest_address) && (o < bookkeeping_covered_committed));
-}
-
 inline
 bool gc_heap::should_check_brick_for_reloc (uint8_t* o)
 {
@@ -25214,8 +25232,10 @@ void gc_heap::background_promote (Object** ppObject, ScanContext* sc, uint32_t f
 
     uint8_t* o = (uint8_t*)*ppObject;
 
-    if (o == 0)
+    if (!is_in_find_object_range (o))
+    {
         return;
+    }
 
 #ifdef DEBUG_DestroyedHandleValue
     // we can race with destroy handle during concurrent scan
@@ -35978,8 +35998,10 @@ void gc_heap::background_promote_callback (Object** ppObject, ScanContext* sc,
 
     uint8_t* o = (uint8_t*)*ppObject;
 
-    if (o == 0)
+    if (!is_in_find_object_range (o))
+    {
         return;
+    }
 
     HEAP_FROM_THREAD;
 
@@ -45907,8 +45929,10 @@ void GCHeap::Promote(Object** ppObject, ScanContext* sc, uint32_t flags)
 
     uint8_t* o = (uint8_t*)*ppObject;
 
-    if (o == 0)
+    if (!gc_heap::is_in_find_object_range (o))
+    {
         return;
+    }
 
 #ifdef DEBUG_DestroyedHandleValue
     // we can race with destroy handle during concurrent scan
@@ -45921,7 +45945,7 @@ void GCHeap::Promote(Object** ppObject, ScanContext* sc, uint32_t flags)
     gc_heap* hp = gc_heap::heap_of (o);
 
 #ifdef USE_REGIONS
-    if (!gc_heap::is_in_bookkeeping_range (o) || !gc_heap::is_in_condemned_gc (o))
+    if (!gc_heap::is_in_condemned_gc (o))
 #else //USE_REGIONS
     if ((o < hp->gc_low) || (o >= hp->gc_high))
 #endif //USE_REGIONS
@@ -45975,19 +45999,16 @@ void GCHeap::Relocate (Object** ppObject, ScanContext* sc,
 
     uint8_t* object = (uint8_t*)(Object*)(*ppObject);
 
+    if (!gc_heap::is_in_find_object_range (object))
+    {
+        return;
+    }
+
     THREAD_NUMBER_FROM_CONTEXT;
 
     //dprintf (3, ("Relocate location %Ix\n", (size_t)ppObject));
     dprintf (3, ("R: %Ix", (size_t)ppObject));
 
-    if (!object 
-#ifdef USE_REGIONS
-        || !gc_heap::is_in_bookkeeping_range (object))
-#else //USE_REGIONS
-        || !((object >= g_gc_lowest_address) && (object < g_gc_highest_address)))
-#endif //USE_REGIONS
-        return;
-
     gc_heap* hp = gc_heap::heap_of (object);
 
 #ifdef _DEBUG
@@ -46437,20 +46458,19 @@ GCHeap::GetContainingObject (void *pInteriorPtr, bool fCollectedGenOnly)
 {
     uint8_t *o = (uint8_t*)pInteriorPtr;
 
+    if (!gc_heap::is_in_find_object_range (o))
+    {
+        return NULL;
+    }
+
     gc_heap* hp = gc_heap::heap_of (o);
 
 #ifdef USE_REGIONS
-    if (gc_heap::is_in_bookkeeping_range (o))
-    {
-        if (fCollectedGenOnly && !gc_heap::is_in_condemned_gc (o))
-        {
-            return NULL;
-        }
-    }
-    else
+    if (fCollectedGenOnly && !gc_heap::is_in_condemned_gc (o))
     {
         return NULL;
     }
+
 #else //USE_REGIONS
 
     uint8_t* lowest = (fCollectedGenOnly ? hp->gc_low : hp->lowest_address);
diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h
index a87d85869c492..c992e1a93ccad 100644
--- a/src/coreclr/gc/gcpriv.h
+++ b/src/coreclr/gc/gcpriv.h
@@ -3134,15 +3134,15 @@ class gc_heap
     void copy_mark_bits_for_addresses (uint8_t* dest, uint8_t* src, size_t len);
 #endif //BACKGROUND_GC
 
+    PER_HEAP_ISOLATED
+    bool is_in_find_object_range (uint8_t* o);
+
 #ifdef USE_REGIONS
     PER_HEAP_ISOLATED
     bool is_in_gc_range (uint8_t* o);
     // o is guaranteed to be in the heap range.
     PER_HEAP_ISOLATED
     bool is_in_condemned_gc (uint8_t* o);
-    // requires checking if o is in the heap range first.
-    PER_HEAP_ISOLATED
-    bool is_in_bookkeeping_range (uint8_t* o);
     PER_HEAP_ISOLATED
     bool should_check_brick_for_reloc (uint8_t* o);
 #endif //USE_REGIONS