[Local GC] Forbid inclusion of gcscan.h from VM and DAC directories (#10332)

* 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

13 files changed, 20 insertions, 40 deletions

diff --git a/src/debug/daccess/enummem.cpp b/src/debug/daccess/enummem.cpp
index 4a217d022f..0dc7a42b1e 100644
--- a/src/debug/daccess/enummem.cpp
+++ b/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/debug/daccess/request.cpp b/src/debug/daccess/request.cpp
index 88efe22c16..c30501f374 100644
--- a/src/debug/daccess/request.cpp
+++ b/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/debug/daccess/request_svr.cpp b/src/debug/daccess/request_svr.cpp
index 4a90ec1b8e..930a35bf1c 100644
--- a/src/debug/daccess/request_svr.cpp
+++ b/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/debug/ee/dactable.cpp b/src/debug/ee/dactable.cpp
index a54fa7515c..af7de1710d 100644
--- a/src/debug/ee/dactable.cpp
+++ b/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/gc/gcee.cpp b/src/gc/gcee.cpp
index c93cc91b57..6513fde51f 100644
--- a/src/gc/gcee.cpp
+++ b/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/gc/gcimpl.h b/src/gc/gcimpl.h
index 67d84b966d..7b4b2eb592 100644
--- a/src/gc/gcimpl.h
+++ b/src/gc/gcimpl.h
@@ -91,6 +91,8 @@ public:
 
     void     SetGCInProgress(BOOL fInProgress);
 
+    bool RuntimeStructuresValid();
+
     CLREvent * GetWaitForGCEvent();
 
     HRESULT Initialize ();
diff --git a/src/gc/gcinterface.h b/src/gc/gcinterface.h
index 42cdcc0211..c463ba7aac 100644
--- a/src/gc/gcinterface.h
+++ b/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/gc/gcscan.cpp b/src/gc/gcscan.cpp
index b4e6352dd6..edcb533cd4 100644
--- a/src/gc/gcscan.cpp
+++ b/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/gc/gcscan.h b/src/gc/gcscan.h
index c17f0ce006..306d5f23a3 100644
--- a/src/gc/gcscan.h
+++ b/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/inc/dacvars.h b/src/inc/dacvars.h
index 6180210fd6..ab3bca20e3 100644
--- a/src/inc/dacvars.h
+++ b/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/vm/object.cpp b/src/vm/object.cpp
index dc53757bac..c37aacc4d4 100644
--- a/src/vm/object.cpp
+++ b/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/vm/stubhelpers.cpp b/src/vm/stubhelpers.cpp
index 899e88e629..bf83c72ea2 100644
--- a/src/vm/stubhelpers.cpp
+++ b/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/vm/syncblk.cpp b/src/vm/syncblk.cpp
index b8a818c107..78e455a580 100644
--- a/src/vm/syncblk.cpp
+++ b/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);