GC-report all associated LoaderAllocators (#110856)

* Report all LoaderAllocators associated with an interior pointer

* Remove IsInRange's unused out parameter

* Remove assertion for at least one valid LoaderAllocator object ref

---------

Co-authored-by: Jan Kotas <[email protected]>

Author: tomeksowi

src/coreclr/inc/utilcode.h

@@ -3104,11 +3104,11 @@ class RangeList
         return this->RemoveRangesWorker(id);
     }
 
-    BOOL IsInRange(TADDR address, TADDR *pID = NULL)
+    BOOL IsInRange(TADDR address)
     {
         SUPPORTS_DAC;
 
-        return this->IsInRangeWorker(address, pID);
+        return this->IsInRangeWorker(address);
     }
 
 #ifndef DACCESS_COMPILE
@@ -3125,7 +3125,32 @@ class RangeList
     virtual void RemoveRangesWorker(void *id) { }
 #endif // !DACCESS_COMPILE
 
-    virtual BOOL IsInRangeWorker(TADDR address, TADDR *pID = NULL);
+    virtual BOOL IsInRangeWorker(TADDR address);
+
+    template<class F>
+    void ForEachInRangeWorker(TADDR address, F func) const
+    {
+        CONTRACTL
+        {
+            INSTANCE_CHECK;
+            NOTHROW;
+            FORBID_FAULT;
+            GC_NOTRIGGER;
+        }
+        CONTRACTL_END
+
+        SUPPORTS_DAC;
+
+        for (const RangeListBlock* b = &m_starterBlock; b != nullptr; b = b->next)
+        {
+            for (const Range r : b->ranges)
+            {
+                if (r.id != (TADDR)nullptr && address >= r.start && address < r.end)
+                    func(r.id);
+            }
+        }
+    }
+
 
 #ifdef DACCESS_COMPILE
     void EnumMemoryRegions(enum CLRDataEnumMemoryFlags flags);

src/coreclr/utilcode/loaderheap.cpp

@@ -209,7 +209,7 @@ void RangeList::RemoveRangesWorker(void *id)
 
 #endif // #ifndef DACCESS_COMPILE
 
-BOOL RangeList::IsInRangeWorker(TADDR address, TADDR *pID /* = NULL */)
+BOOL RangeList::IsInRangeWorker(TADDR address)
 {
     CONTRACTL
     {
@@ -222,46 +222,15 @@ BOOL RangeList::IsInRangeWorker(TADDR address, TADDR *pID /* = NULL */)
 
     SUPPORTS_DAC;
 
-    RangeListBlock* b = &m_starterBlock;
-    Range* r = b->ranges;
-    Range* rEnd = r + RANGE_COUNT;
-
-    //
-    // Look for a matching element
-    //
-
-    while (TRUE)
+    for (const RangeListBlock* b = &m_starterBlock; b != nullptr; b = b->next)
     {
-        while (r < rEnd)
+        for (const Range r : b->ranges)
         {
-            if (r->id != (TADDR)NULL &&
-                address >= r->start
-                && address < r->end)
-            {
-                if (pID != NULL)
-                {
-                    *pID = r->id;
-                }
+            if (r.id != (TADDR)nullptr && address >= r.start && address < r.end)
                 return TRUE;
-            }
-            r++;
         }
-
-        //
-        // If there are no more blocks, we're done.
-        //
-
-        if (b->next == NULL)
-            return FALSE;
-
-        //
-        // Next block.
-        //
-
-        b = b->next;
-        r = b->ranges;
-        rEnd = r + RANGE_COUNT;
     }
+    return FALSE;
 }
 
 #ifdef DACCESS_COMPILE

src/coreclr/vm/assemblyspec.cpp

@@ -872,7 +872,7 @@ BOOL AssemblySpecBindingCache::StoreAssembly(AssemblySpec *pSpec, Assembly *pAss
 
         abHolder.SuppressRelease();
 
-        STRESS_LOG2(LF_CLASSLOADER,LL_INFO10,"StorePEAssembly (StoreAssembly): Add cached entry (%p) with PEAssembly %p",entry,pAssembly->GetPEAssembly());
+        STRESS_LOG2(LF_CLASSLOADER,LL_INFO10,"StorePEAssembly (StoreAssembly): Add cached entry (%p) with PEAssembly %p\n",entry,pAssembly->GetPEAssembly());
         RETURN TRUE;
     }
     else
@@ -1022,7 +1022,7 @@ BOOL AssemblySpecBindingCache::StoreException(AssemblySpec *pSpec, Exception* pE
         m_map.InsertValue(key, entry);
         abHolder.SuppressRelease();
 
-        STRESS_LOG2(LF_CLASSLOADER,LL_INFO10,"StorePEAssembly (StoreException): Add cached entry (%p) with exception %p",entry,pEx);
+        STRESS_LOG2(LF_CLASSLOADER,LL_INFO10,"StorePEAssembly (StoreException): Add cached entry (%p) with exception %p\n",entry,pEx);
         RETURN TRUE;
     }
     else

src/coreclr/vm/gcenv.ee.common.cpp

@@ -213,16 +213,14 @@ void GcReportLoaderAllocator(promote_func* fn, ScanContext* sc, LoaderAllocator
     if (pLoaderAllocator != NULL && pLoaderAllocator->IsCollectible())
     {
         Object *refCollectionObject = OBJECTREFToObject(pLoaderAllocator->GetExposedObject());
+        if (refCollectionObject != NULL)
+        {
+            INDEBUG(Object *oldObj = refCollectionObject;)
+            fn(&refCollectionObject, sc, CHECK_APP_DOMAIN);
 
-#ifdef _DEBUG
-        Object *oldObj = refCollectionObject;
-#endif
-
-        _ASSERTE(refCollectionObject != NULL);
-        fn(&refCollectionObject, sc, CHECK_APP_DOMAIN);
-
-        // We are reporting the location of a local variable, assert it doesn't change.
-        _ASSERTE(oldObj == refCollectionObject);
+            // We are reporting the location of a local variable, assert it doesn't change.
+            _ASSERTE(oldObj == refCollectionObject);
+        }
     }
 }
 

src/coreclr/vm/loaderallocator.cpp

@@ -2141,17 +2141,17 @@ void LoaderAllocator::AssociateMemoryWithLoaderAllocator(BYTE *start, const BYTE
 }
 
 /* static */
-PTR_LoaderAllocator LoaderAllocator::GetAssociatedLoaderAllocator_Unsafe(TADDR ptr)
+void LoaderAllocator::GcReportAssociatedLoaderAllocators_Unsafe(TADDR ptr, promote_func* fn, ScanContext* sc)
 {
     LIMITED_METHOD_CONTRACT;
 
     GlobalLoaderAllocator* pGlobalAllocator = (GlobalLoaderAllocator*)SystemDomain::GetGlobalLoaderAllocator();
-    LoaderAllocator* pLoaderAllocator;
-    if (pGlobalAllocator->m_memoryAssociations.IsInRangeWorker_Unlocked(ptr, reinterpret_cast<TADDR *>(&pLoaderAllocator)))
-    {
-        return pLoaderAllocator;
-    }
-    return NULL;
+    pGlobalAllocator->m_memoryAssociations.ForEachInRangeWorker_Unlocked(ptr,
+        [fn, sc](TADDR laAddr)
+        {
+            GcReportLoaderAllocator(fn, sc, (LoaderAllocator*)laAddr);
+        }
+    );
 }
 
 

src/coreclr/vm/loaderallocator.hpp

@@ -159,7 +159,7 @@ class CodeRangeMapRangeList : public RangeList
 #endif // DACCESS_COMPILE
     }
 
-    virtual BOOL IsInRangeWorker(TADDR address, TADDR *pID = NULL)
+    virtual BOOL IsInRangeWorker(TADDR address)
     {
         WRAPPER_NO_CONTRACT;
         RangeSection *pRS = ExecutionManager::FindCodeRange(address, ExecutionManager::ScanReaderLock);
@@ -570,7 +570,7 @@ class LoaderAllocator
 
     // This function may only be called while the runtime is suspended
     // As it does not lock around access to a RangeList
-    static PTR_LoaderAllocator GetAssociatedLoaderAllocator_Unsafe(TADDR ptr);
+    static void GcReportAssociatedLoaderAllocators_Unsafe(TADDR ptr, promote_func* fn, ScanContext* sc);
 
     static void AssociateMemoryWithLoaderAllocator(BYTE *start, const BYTE *end, LoaderAllocator* pLoaderAllocator);
     static void RemoveMemoryToLoaderAllocatorAssociation(LoaderAllocator* pLoaderAllocator);

src/coreclr/vm/lockedrangelist.h

@@ -24,11 +24,19 @@ class LockedRangeList : public RangeList
         LIMITED_METHOD_CONTRACT;
     }
 
-    BOOL IsInRangeWorker_Unlocked(TADDR address, TADDR *pID = NULL)
+    BOOL IsInRangeWorker_Unlocked(TADDR address)
     {
         WRAPPER_NO_CONTRACT;
         SUPPORTS_DAC;
-        return RangeList::IsInRangeWorker(address, pID);
+        return RangeList::IsInRangeWorker(address);
+    }
+
+    template<class F>
+    void ForEachInRangeWorker_Unlocked(TADDR address, F func) const
+    {
+        WRAPPER_NO_CONTRACT;
+        SUPPORTS_DAC;
+        return RangeList::ForEachInRangeWorker(address, func);
     }
 
   protected:
@@ -47,12 +55,12 @@ class LockedRangeList : public RangeList
         RangeList::RemoveRangesWorker(id);
     }
 
-    virtual BOOL IsInRangeWorker(TADDR address, TADDR *pID = NULL)
+    virtual BOOL IsInRangeWorker(TADDR address)
     {
         WRAPPER_NO_CONTRACT;
         SUPPORTS_DAC;
         SimpleReadLockHolder lh(&m_RangeListRWLock);
-        return RangeList::IsInRangeWorker(address, pID);
+        return RangeList::IsInRangeWorker(address);
     }
 
     SimpleRWLock m_RangeListRWLock;

src/coreclr/vm/siginfo.cpp

@@ -5145,12 +5145,9 @@ void PromoteCarefully(promote_func   fn,
 
     if (sc->promotion)
     {
-        LoaderAllocator*pLoaderAllocator = LoaderAllocator::GetAssociatedLoaderAllocator_Unsafe(PTR_TO_TADDR(*ppObj));
-        if (pLoaderAllocator != NULL)
-        {
-            GcReportLoaderAllocator(fn, sc, pLoaderAllocator);
-        }
+        LoaderAllocator::GcReportAssociatedLoaderAllocators_Unsafe(PTR_TO_TADDR(*ppObj), fn, sc);
     }
+
 #endif // !defined(DACCESS_COMPILE)
 
     (*fn) (ppObj, sc, flags);