From fd4d0a9ae99d88f0dca7b5c95005149c641c2211 Mon Sep 17 00:00:00 2001
From: Max Charlamb <44248479+max-charlamb@users.noreply.github.com>
Date: Fri, 4 Oct 2024 13:45:42 -0700
Subject: [PATCH] [cdac] Tighten cdac_data friend declarations (#108525)

tighten cdac_data friend declarations

Previously classes exposed internals to all cdac_data specializations. By changing the
friend declarations to the exact specialization which needs will access the internals,
we can better verify where the internals are accessed and ensure cdac_data<T> accesses
the correct class.
---
 src/coreclr/inc/slist.h      |  1 +
 src/coreclr/vm/cdacdata.h    | 21 ++++++++++++++-------
 src/coreclr/vm/ceeload.h     |  4 ++--
 src/coreclr/vm/class.h       |  4 ++--
 src/coreclr/vm/exstate.h     |  2 +-
 src/coreclr/vm/method.hpp    | 10 +++++-----
 src/coreclr/vm/methodtable.h |  2 +-
 src/coreclr/vm/object.h      |  8 ++++----
 src/coreclr/vm/syncblk.h     |  6 +++---
 src/coreclr/vm/threads.h     |  4 ++--
 src/coreclr/vm/typedesc.h    |  8 ++++----
 11 files changed, 39 insertions(+), 31 deletions(-)

diff --git a/src/coreclr/inc/slist.h b/src/coreclr/inc/slist.h
index ebd892b1882a9..487c5dca38255 100644
--- a/src/coreclr/inc/slist.h
+++ b/src/coreclr/inc/slist.h
@@ -120,6 +120,7 @@ class SList
     PTR_SLink m_pHead;
     PTR_SLink m_pTail;
 
+    // as a generic data structure, friend to all specializations of cdac_data
     template<typename U> friend struct ::cdac_data;
 
     // get the list node within the object
diff --git a/src/coreclr/vm/cdacdata.h b/src/coreclr/vm/cdacdata.h
index e8bd2c6f2cb69..a16796d09d270 100644
--- a/src/coreclr/vm/cdacdata.h
+++ b/src/coreclr/vm/cdacdata.h
@@ -7,16 +7,23 @@
 // See datadescriptor.h
 //
 // This struct enables exposing information that is private to a class to the cDAC. For example,
-// if class C has private information that must be provided, declare cdac_data<T> as a friend:
+// if class C has private information that must be provided, declare cdac_data<D> as a friend of C
+// where D is the specialization of cdac_data that will expose the information. Then provide a 
+// specialization cdac_data<D> with constexpr members exposing the information.
 //
-//     template<typename T> friend struct ::cdac_data;
+// Note: in the common case, type D will be type C.
 //
-// and provide a specialization cdac_data<C> with constexpr members exposing the information.
-// For example, if the offset of field F is required:
+// For example, if the offset of field F in class C is required:
+//
+//      class C {
+//      private:
+//          int F;
+//          friend struct ::cdac_data<C>;
+//      };
+//      template<> struct cdac_data<C> {
+//          static constexpr size_t F_Offset = offsetof(C, F);
+//      };
 //
-//     template<> struct cdac_data<C> {
-//         static constexpr size_t F_Offset = offsetof(C, F);
-//     };
 template<typename T>
 struct cdac_data
 {
diff --git a/src/coreclr/vm/ceeload.h b/src/coreclr/vm/ceeload.h
index b2c1068c0ae5b..2d0d266494a17 100644
--- a/src/coreclr/vm/ceeload.h
+++ b/src/coreclr/vm/ceeload.h
@@ -90,7 +90,7 @@ struct DynamicMetadata
 {
     uint32_t Size;
     BYTE Data[0];
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<DynamicMetadata>;
 };
 
 template<>
@@ -1634,7 +1634,7 @@ class Module : public ModuleBase
     uint32_t GetNativeMetadataAssemblyCount();
 #endif // !defined(DACCESS_COMPILE)
 
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<Module>;
 };
 
 template<>
diff --git a/src/coreclr/vm/class.h b/src/coreclr/vm/class.h
index 71044dd056b37..2c586351ac961 100644
--- a/src/coreclr/vm/class.h
+++ b/src/coreclr/vm/class.h
@@ -1798,7 +1798,7 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW!
     }
 #endif // !DACCESS_COMPILE
 
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<EEClass>;
 };
 
 template<> struct cdac_data<EEClass>
@@ -1974,7 +1974,7 @@ class ArrayClass : public EEClass
                                       BOOL fForStubAsIL
     );
 
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<ArrayClass>;
 };
 
 template<> struct cdac_data<ArrayClass>
diff --git a/src/coreclr/vm/exstate.h b/src/coreclr/vm/exstate.h
index 3d57a935b65d9..e66a829b7fc5c 100644
--- a/src/coreclr/vm/exstate.h
+++ b/src/coreclr/vm/exstate.h
@@ -51,7 +51,7 @@ class ThreadExceptionState
     // ExceptionTracker or the ExInfo as appropriate for the platform
     friend class ProfToEEInterfaceImpl;
 
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<Thread>;
 
 #ifdef FEATURE_EH_FUNCLETS
     friend class ExceptionTracker;
diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp
index 5a9dab1fc25dd..956765b76ff05 100644
--- a/src/coreclr/vm/method.hpp
+++ b/src/coreclr/vm/method.hpp
@@ -1907,7 +1907,7 @@ class MethodDesc
     static void Init();
 #endif
 
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<MethodDesc>;
 };
 
 template<> struct cdac_data<MethodDesc>
@@ -2349,7 +2349,7 @@ class MethodDescChunk
 
     // Followed by array of method descs...
 
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<MethodDescChunk>;
 };
 
 template<>
@@ -2436,7 +2436,7 @@ class StoredSigMethodDesc : public MethodDesc
 #ifdef DACCESS_COMPILE
     void EnumMemoryRegions(CLRDataEnumMemoryFlags flags);
 #endif
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<StoredSigMethodDesc>;
 };
 
 template<>
@@ -2694,7 +2694,7 @@ class DynamicMethodDesc : public StoredSigMethodDesc
     // following implementations defined in DynamicMethod.cpp
     //
     void Destroy();
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<DynamicMethodDesc>;
 };
 
 template<>
@@ -3453,7 +3453,7 @@ class InstantiatedMethodDesc final : public MethodDesc
                                                              MethodDesc* pSharedMDescForStub,
                                                              Instantiation methodInst,
                                                              BOOL getSharedNotStub);
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<InstantiatedMethodDesc>;
 };
 
 template<> struct cdac_data<InstantiatedMethodDesc>
diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h
index e1dce807d381f..9338b6b29a738 100644
--- a/src/coreclr/vm/methodtable.h
+++ b/src/coreclr/vm/methodtable.h
@@ -3980,7 +3980,7 @@ public :
 
     static void GetStaticsOffsets(StaticsOffsetType staticsOffsetType, bool fGenericsStatics, uint32_t *dwGCOffset, uint32_t *dwNonGCOffset);
 
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<MethodTable>;
 };  // class MethodTable
 
 template<> struct cdac_data<MethodTable>
diff --git a/src/coreclr/vm/object.h b/src/coreclr/vm/object.h
index e637a64da2fd1..fca89bc39b24a 100644
--- a/src/coreclr/vm/object.h
+++ b/src/coreclr/vm/object.h
@@ -462,7 +462,7 @@ class Object
  private:
     VOID ValidateInner(BOOL bDeep, BOOL bVerifyNextHeader, BOOL bVerifySyncBlock);
 
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<Object>;
 };
 
 template<>
@@ -638,7 +638,7 @@ class ArrayBase : public Object
     inline static unsigned GetBoundsOffset(MethodTable* pMT);
     inline static unsigned GetLowerBoundsOffset(MethodTable* pMT);
 
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<ArrayBase>;
 };
 
 #ifndef DACCESS_COMPILE
@@ -950,7 +950,7 @@ class StringObject : public Object
     static STRINGREF* EmptyStringRefPtr;
     static bool EmptyStringIsFrozen;
 
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<StringObject>;
 };
 
 template<>
@@ -2437,7 +2437,7 @@ class ExceptionObject : public Object
     INT32       _xcode;
     INT32       _HResult;
 
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<ExceptionObject>;
 };
 
 template<>
diff --git a/src/coreclr/vm/syncblk.h b/src/coreclr/vm/syncblk.h
index e3eb90b663fd3..7cdf92cdda878 100644
--- a/src/coreclr/vm/syncblk.h
+++ b/src/coreclr/vm/syncblk.h
@@ -970,7 +970,7 @@ class InteropSyncBlockInfo
     BYTE m_taggedAlloc[2 * sizeof(void*)];
 #endif // FEATURE_OBJCMARSHAL
 
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<InteropSyncBlockInfo>;
 };
 
 template<>
@@ -1286,7 +1286,7 @@ class SyncBlock
     }
 #endif // defined(ENABLE_CONTRACTS_IMPL)
 
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<SyncBlock>;
 };
 
 template<>
@@ -1674,7 +1674,7 @@ class ObjHeader
 
     BOOL Validate (BOOL bVerifySyncBlkIndex = TRUE);
 
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<ObjHeader>;
 };
 
 template<>
diff --git a/src/coreclr/vm/threads.h b/src/coreclr/vm/threads.h
index aca4905e419c0..8fe3db20ab2d4 100644
--- a/src/coreclr/vm/threads.h
+++ b/src/coreclr/vm/threads.h
@@ -3965,7 +3965,7 @@ class Thread
 private:
     bool m_hasPendingActivation;
 
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<Thread>;
 };
 
 template<>
@@ -4247,7 +4247,7 @@ class ThreadStore
     bool ShouldTriggerGCForDeadThreads();
     void TriggerGCForDeadThreadsIfNecessary();
 
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<ThreadStore>;
 };
 
 template<>
diff --git a/src/coreclr/vm/typedesc.h b/src/coreclr/vm/typedesc.h
index 593e71b63b8c9..62905fea99f25 100644
--- a/src/coreclr/vm/typedesc.h
+++ b/src/coreclr/vm/typedesc.h
@@ -204,7 +204,7 @@ class TypeDesc
 
     // internal RuntimeType object handle
     RUNTIMETYPEHANDLE m_hExposedClassObject;
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<TypeDesc>;
 };
 
 template<>
@@ -269,7 +269,7 @@ class ParamTypeDesc : public TypeDesc {
 
     // The type that is being modified
     TypeHandle        m_Arg;
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<ParamTypeDesc>;
 };
 
 template<>
@@ -395,7 +395,7 @@ class TypeVarTypeDesc : public TypeDesc
     // index within declaring type or method, numbered from zero
     unsigned int m_index;
 
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<TypeVarTypeDesc>;
 };
 
 template<>
@@ -495,7 +495,7 @@ class FnPtrTypeDesc : public TypeDesc
     // Return type first, then argument types
     TypeHandle m_RetAndArgTypes[1];
 
-    template<typename T> friend struct ::cdac_data;
+    friend struct ::cdac_data<FnPtrTypeDesc>;
 }; // class FnPtrTypeDesc
 
 template<>