From d6c56953600f9ac4d5249d00a6aa32656b07bf4e Mon Sep 17 00:00:00 2001
From: Maoni0 <maoni@microsoft.com>
Date: Fri, 7 Oct 2022 00:56:08 -0700
Subject: [PATCH] filtering out addresses conservatively reported that land in
 the GC range but not in bookkeeping range

---
 src/coreclr/gc/gc.cpp   | 30 ++++++++++++++----------------
 src/coreclr/gc/gcpriv.h |  4 ++--
 2 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp
index ee747875ae4b8..48fa93148cd91 100644
--- a/src/coreclr/gc/gc.cpp
+++ b/src/coreclr/gc/gc.cpp
@@ -7925,18 +7925,12 @@ bool gc_heap::is_in_condemned_gc (uint8_t* o)
     return true;
 }
 
-// REGIONS TODO -
-// This method can be called by GCHeap::Promote/Relocate which means
-// it could be in the heap range but not actually in a valid region.
-// This would return true but find_object will return 0. But this
-// seems counter-intuitive so we should consider a better implementation.
+// 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_condemned (uint8_t* o)
+bool gc_heap::is_in_bookkeeping_range (uint8_t* o)
 {
-    if ((o >= g_gc_lowest_address) && (o < g_gc_highest_address))
-        return is_in_condemned_gc (o);
-    else
-        return false;
+    return ((o >= g_gc_lowest_address) && (o < bookkeeping_covered_committed));
 }
 
 inline
@@ -45898,7 +45892,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_condemned (o))
+    if (!gc_heap::is_in_bookkeeping_range (o) || !gc_heap::is_in_condemned_gc (o))
 #else //USE_REGIONS
     if ((o < hp->gc_low) || (o >= hp->gc_high))
 #endif //USE_REGIONS
@@ -45957,7 +45951,12 @@ void GCHeap::Relocate (Object** ppObject, ScanContext* sc,
     //dprintf (3, ("Relocate location %Ix\n", (size_t)ppObject));
     dprintf (3, ("R: %Ix", (size_t)ppObject));
 
-    if (!object || !((object >= g_gc_lowest_address) && (object < g_gc_highest_address)))
+    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);
@@ -46412,17 +46411,16 @@ GCHeap::GetContainingObject (void *pInteriorPtr, bool fCollectedGenOnly)
     gc_heap* hp = gc_heap::heap_of (o);
 
 #ifdef USE_REGIONS
-    if (fCollectedGenOnly)
+    if (gc_heap::is_in_bookkeeping_range (o))
     {
-        if (!gc_heap::is_in_condemned (o))
+        if (fCollectedGenOnly && !gc_heap::is_in_condemned_gc (o))
         {
             return NULL;
         }
     }
     else
     {
-        if (!((o >= g_gc_lowest_address) && (o < g_gc_highest_address)))
-            return NULL;
+        return NULL;
     }
 #else //USE_REGIONS
 
diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h
index 74bba945a867d..bc7b148bda8e8 100644
--- a/src/coreclr/gc/gcpriv.h
+++ b/src/coreclr/gc/gcpriv.h
@@ -52,7 +52,7 @@ inline void FATAL_GC_ERROR()
 // This means any empty regions can be freely used for any generation. For
 // Server GC we will balance regions between heaps.
 // For now disable regions for StandAlone GC, NativeAOT and MacOS builds
-#if defined (HOST_64BIT) && !defined (BUILD_AS_STANDALONE) && !defined(__APPLE__) && !defined(FEATURE_NATIVEAOT)
+#if defined (HOST_64BIT) && !defined (BUILD_AS_STANDALONE) && !defined(__APPLE__)
 #define USE_REGIONS
 #endif //HOST_64BIT && BUILD_AS_STANDALONE
 
@@ -3142,7 +3142,7 @@ class gc_heap
     bool is_in_condemned_gc (uint8_t* o);
     // requires checking if o is in the heap range first.
     PER_HEAP_ISOLATED
-    bool is_in_condemned (uint8_t* o);
+    bool is_in_bookkeeping_range (uint8_t* o);
     PER_HEAP_ISOLATED
     bool should_check_brick_for_reloc (uint8_t* o);
 #endif //USE_REGIONS