[Local GC] Forbid inclusion of gcscan.h from VM and DAC directories (dotnet/coreclr...

* Forbid inclusion of gcscan.h from VM and DAC directories

* Address code review feedback - hoist IsGCHeapInitialized check to an assert, since the heap should definitely be initialized if we are validating objects that ostensibly came from it

Commit migrated from https://github.com/dotnet/coreclr/commit/d5906ff3e0d0f864c1432f1f39d586ca3d79fbb6

diff --git a/src/coreclr/src/debug/daccess/enummem.cpp b/src/coreclr/src/debug/daccess/enummem.cpp
index 4a217d0..0dc7a42 100644 (file)
--- a/src/coreclr/src/debug/daccess/enummem.cpp
+++ b/src/coreclr/src/debug/daccess/enummem.cpp
@@ -21,7 +21,6 @@
 #include "ipcmanagerinterface.h"
 #include "binder.h"
 #include "win32threadpool.h"
-#include "gcscan.h"
 
 #ifdef FEATURE_APPX
 #include "appxutil.h"
@@ -407,7 +406,7 @@ HRESULT ClrDataAccess::DumpManagedObject(CLRDataEnumMemoryFlags flags, OBJECTREF
         return status;
     }
     
-    if (!GCScan::GetGcRuntimeStructuresValid ())
+    if (*g_gcDacGlobals->gc_structures_invalid_cnt != 0)
     {
         // GC is in progress, don't dump this object
         return S_OK;
@@ -465,7 +464,7 @@ HRESULT ClrDataAccess::DumpManagedExcepObject(CLRDataEnumMemoryFlags flags, OBJE
         return S_OK;
     }
 
-    if (!GCScan::GetGcRuntimeStructuresValid ())
+    if (*g_gcDacGlobals->gc_structures_invalid_cnt != 0)
     {
         // GC is in progress, don't dump this object
         return S_OK;

diff --git a/src/coreclr/src/debug/daccess/request.cpp b/src/coreclr/src/debug/daccess/request.cpp
index 88efe22..c30501f 100644 (file)
--- a/src/coreclr/src/debug/daccess/request.cpp
+++ b/src/coreclr/src/debug/daccess/request.cpp
@@ -3827,6 +3827,7 @@ ClrDataAccess::EnumWksGlobalMemoryRegions(CLRDataEnumMemoryFlags flags)
 
     Dereference(g_gcDacGlobals->ephemeral_heap_segment).EnumMem();
     g_gcDacGlobals->alloc_allocated.EnumMem();
+    g_gcDacGlobals->gc_structures_invalid_cnt.EnumMem();
     Dereference(g_gcDacGlobals->finalize_queue).EnumMem();
 
     // Enumerate the entire generation table, which has variable size

diff --git a/src/coreclr/src/debug/daccess/request_svr.cpp b/src/coreclr/src/debug/daccess/request_svr.cpp
index 4a90ec1..930a35b 100644 (file)
--- a/src/coreclr/src/debug/daccess/request_svr.cpp
+++ b/src/coreclr/src/debug/daccess/request_svr.cpp
@@ -210,6 +210,7 @@ ClrDataAccess::EnumSvrGlobalMemoryRegions(CLRDataEnumMemoryFlags flags)
     DacEnumMemoryRegion(g_gcDacGlobals->g_heaps.GetAddr(),
                     sizeof(TADDR) * *g_gcDacGlobals->n_heaps);
 
+    g_gcDacGlobals->gc_structures_invalid_cnt.EnumMem();
     g_gcDacGlobals->g_heaps.EnumMem();
     
     for (int i=0; i < *g_gcDacGlobals->n_heaps; i++)

diff --git a/src/coreclr/src/debug/ee/dactable.cpp b/src/coreclr/src/debug/ee/dactable.cpp
index a54fa75..af7de17 100644 (file)
--- a/src/coreclr/src/debug/ee/dactable.cpp
+++ b/src/coreclr/src/debug/ee/dactable.cpp
@@ -21,14 +21,6 @@
 #include "../../vm/gcenv.h"
 #include "../../vm/ecall.h"
 #include "../../vm/rcwwalker.h"
-#include "../../gc/gc.h"
-#include "../../gc/gcscan.h"
-
-#undef SERVER_GC
-namespace WKS {
-#include "../../gc/gcimpl.h"
-#include "../../gc/gcpriv.h"
-}
 
 #ifdef DEBUGGING_SUPPORTED
 

diff --git a/src/coreclr/src/gc/gcee.cpp b/src/coreclr/src/gc/gcee.cpp
index c93cc91..6513fde 100644 (file)
--- a/src/coreclr/src/gc/gcee.cpp
+++ b/src/coreclr/src/gc/gcee.cpp
@@ -681,6 +681,11 @@ void GCHeap::UnregisterFrozenSegment(segment_handle seg)
 #endif // FEATURE_BASICFREEZE
 }
 
+bool GCHeap::RuntimeStructuresValid()
+{
+    return GCScan::GetGcRuntimeStructuresValid();
+}
+
 
 #endif // !DACCESS_COMPILE
 

diff --git a/src/coreclr/src/gc/gcimpl.h b/src/coreclr/src/gc/gcimpl.h
index 67d84b9..7b4b2eb 100644 (file)
--- a/src/coreclr/src/gc/gcimpl.h
+++ b/src/coreclr/src/gc/gcimpl.h
@@ -91,6 +91,8 @@ public:
 
     void     SetGCInProgress(BOOL fInProgress);
 
+    bool RuntimeStructuresValid();
+
     CLREvent * GetWaitForGCEvent();
 
     HRESULT Initialize ();

diff --git a/src/coreclr/src/gc/gcinterface.h b/src/coreclr/src/gc/gcinterface.h
index 42cdcc0..c463ba7 100644 (file)
--- a/src/coreclr/src/gc/gcinterface.h
+++ b/src/coreclr/src/gc/gcinterface.h
@@ -581,6 +581,9 @@ public:
     // Sets whether or not a GC is in progress.
     virtual void SetGCInProgress(BOOL fInProgress) = 0;
 
+    // Gets whether or not the GC runtime structures are in a valid state for heap traversal.
+    virtual bool RuntimeStructuresValid() = 0;
+
     /*
     ============================================================================
     Add/RemoveMemoryPressure support routines. These are on the interface

diff --git a/src/coreclr/src/gc/gcscan.cpp b/src/coreclr/src/gc/gcscan.cpp
index b4e6352..edcb533 100644 (file)
--- a/src/coreclr/src/gc/gcscan.cpp
+++ b/src/coreclr/src/gc/gcscan.cpp
@@ -19,11 +19,7 @@
 #include "gc.h"
 #include "objecthandle.h"
 
-#ifdef DACCESS_COMPILE
-SVAL_IMPL_INIT(int32_t, GCScan, m_GcStructuresInvalidCnt, 1);
-#else //DACCESS_COMPILE
 VOLATILE(int32_t) GCScan::m_GcStructuresInvalidCnt = 1;
-#endif //DACCESS_COMPILE
 
 bool GCScan::GetGcRuntimeStructuresValid ()
 {
@@ -33,18 +29,7 @@ bool GCScan::GetGcRuntimeStructuresValid ()
     return (int32_t)m_GcStructuresInvalidCnt == 0;
 }
 
-#ifdef DACCESS_COMPILE
-
-#ifndef FEATURE_REDHAWK
-void
-GCScan::EnumMemoryRegions(CLRDataEnumMemoryFlags flags)
-{
-    UNREFERENCED_PARAMETER(flags);
-    m_GcStructuresInvalidCnt.EnumMem();
-}
-#endif
-
-#else
+#ifndef DACCESS_COMPILE
 
 //
 // Dependent handle promotion scan support

diff --git a/src/coreclr/src/gc/gcscan.h b/src/coreclr/src/gc/gcscan.h
index c17f0ce..306d5f2 100644 (file)
--- a/src/coreclr/src/gc/gcscan.h
+++ b/src/coreclr/src/gc/gcscan.h
@@ -89,11 +89,7 @@ class GCScan
 
     static void VerifyHandleTable(int condemned, int max_gen, ScanContext* sc);
     
-#ifdef DACCESS_COMPILE    
-    SVAL_DECL(int32_t, m_GcStructuresInvalidCnt);
-#else
     static VOLATILE(int32_t) m_GcStructuresInvalidCnt;
-#endif //DACCESS_COMPILE
 };
 
 // These two functions are utilized to scan the heap if requested by ETW

diff --git a/src/coreclr/src/inc/dacvars.h b/src/coreclr/src/inc/dacvars.h
index 6180210..ab3bca2 100644 (file)
--- a/src/coreclr/src/inc/dacvars.h
+++ b/src/coreclr/src/inc/dacvars.h
@@ -138,8 +138,6 @@ DEFINE_DACVAR(ULONG, PTR_SharedDomain, SharedDomain__m_pSharedDomain, SharedDoma
 
 DEFINE_DACVAR(ULONG, DWORD, CExecutionEngine__TlsIndex, CExecutionEngine::TlsIndex)
 
-DEFINE_DACVAR(ULONG, LONG, GCScan__m_GcStructuresInvalidCnt, GCScan::m_GcStructuresInvalidCnt)
-
 #if defined(FEATURE_WINDOWSPHONE)
 DEFINE_DACVAR(ULONG, int, CCLRErrorReportingManager__g_ECustomDumpFlavor, CCLRErrorReportingManager::g_ECustomDumpFlavor)
 #endif

diff --git a/src/coreclr/src/vm/object.cpp b/src/coreclr/src/vm/object.cpp
index dc53757..c37aacc 100644 (file)
--- a/src/coreclr/src/vm/object.cpp
+++ b/src/coreclr/src/vm/object.cpp
@@ -19,7 +19,6 @@
 #include "eeconfig.h"
 #include "gcheaputilities.h"
 #include "field.h"
-#include "gcscan.h"
 #include "argdestination.h"
 
 
@@ -1773,12 +1772,13 @@ VOID Object::ValidateInner(BOOL bDeep, BOOL bVerifyNextHeader, BOOL bVerifySyncB
 
         lastTest = 7;
 
+        _ASSERTE(GCHeapUtilities::IsGCHeapInitialized());
         // try to validate next object's header
         if (bDeep 
             && bVerifyNextHeader 
-            && GCScan::GetGcRuntimeStructuresValid ()
+            && GCHeapUtilities::GetGCHeap()->RuntimeStructuresValid()
             //NextObj could be very slow if concurrent GC is going on
-            && !(GCHeapUtilities::IsGCHeapInitialized() && GCHeapUtilities::GetGCHeap ()->IsConcurrentGCInProgress ()))
+            && !GCHeapUtilities::GetGCHeap ()->IsConcurrentGCInProgress ())
         {
             Object * nextObj = GCHeapUtilities::GetGCHeap ()->NextObj (this);
             if ((nextObj != NULL) &&

diff --git a/src/coreclr/src/vm/stubhelpers.cpp b/src/coreclr/src/vm/stubhelpers.cpp
index 899e88e..bf83c72 100644 (file)
--- a/src/coreclr/src/vm/stubhelpers.cpp
+++ b/src/coreclr/src/vm/stubhelpers.cpp
@@ -21,7 +21,6 @@
 #include "comdatetime.h"
 #include "gcheaputilities.h"
 #include "interoputil.h"
-#include "gcscan.h"
 
 #ifdef FEATURE_COMINTEROP
 #include <oletls.h>
@@ -58,7 +57,7 @@ void StubHelpers::ValidateObjectInternal(Object *pObjUNSAFE, BOOL fValidateNextO
 }
        CONTRACTL_END;
 
-       _ASSERTE(GCScan::GetGcRuntimeStructuresValid());
+       _ASSERTE(GCHeapUtilities::GetGCHeap()->RuntimeStructuresValid());
 
        // validate the object - there's no need to validate next object's
        // header since we validate the next object explicitly below

diff --git a/src/coreclr/src/vm/syncblk.cpp b/src/coreclr/src/vm/syncblk.cpp
index b8a818c..78e455a 100644 (file)
--- a/src/coreclr/src/vm/syncblk.cpp
+++ b/src/coreclr/src/vm/syncblk.cpp
@@ -30,7 +30,6 @@
 #include "corhost.h"
 #include "comdelegate.h"
 #include "finalizerthread.h"
-#include "gcscan.h"
 
 #ifdef FEATURE_COMINTEROP
 #include "runtimecallablewrapper.h"
@@ -2518,7 +2517,7 @@ BOOL ObjHeader::Validate (BOOL bVerifySyncBlkIndex)
         //rest of the DWORD is SyncBlk Index
         if (!(bits & BIT_SBLK_IS_HASHCODE))
         {
-            if (bVerifySyncBlkIndex  && GCScan::GetGcRuntimeStructuresValid ())
+            if (bVerifySyncBlkIndex  && GCHeapUtilities::GetGCHeap()->RuntimeStructuresValid ())
             {
                 DWORD sbIndex = bits & MASK_SYNCBLOCKINDEX;
                 ASSERT_AND_CHECK(SyncTableEntry::GetSyncTableEntry()[sbIndex].m_Object == obj);