[cdac] Fix calculation of MethodDesc optional slot addresses

src/coreclr/debug/runtimeinfo/datadescriptor.h

@@ -342,6 +342,18 @@ CDAC_TYPE_FIELD(MethodDescChunk, /*uint8*/, Count, cdac_data<MethodDescChunk>::C
 CDAC_TYPE_FIELD(MethodDescChunk, /*uint16*/, FlagsAndTokenRange, cdac_data<MethodDescChunk>::FlagsAndTokenRange)
 CDAC_TYPE_END(MethodDescChunk)
 
+CDAC_TYPE_BEGIN(NonVtableSlot)
+CDAC_TYPE_SIZE(sizeof(MethodDesc::NonVtableSlot))
+CDAC_TYPE_END(NonVtableSlot)
+
+CDAC_TYPE_BEGIN(MethodImpl)
+CDAC_TYPE_SIZE(sizeof(MethodImpl))
+CDAC_TYPE_END(MethodImpl)
+
+CDAC_TYPE_BEGIN(NativeCodeSlot)
+CDAC_TYPE_SIZE(sizeof(MethodDesc::NativeCodeSlot))
+CDAC_TYPE_END(NativeCodeSlot)
+
 CDAC_TYPE_BEGIN(InstantiatedMethodDesc)
 CDAC_TYPE_SIZE(sizeof(InstantiatedMethodDesc))
 CDAC_TYPE_FIELD(InstantiatedMethodDesc, /*pointer*/, PerInstInfo, cdac_data<InstantiatedMethodDesc>::PerInstInfo)

src/coreclr/vm/method.hpp

@@ -1853,10 +1853,8 @@ class MethodDesc
     //
     // Optional MethodDesc slots appear after the end of base MethodDesc in this order:
     //
-
-    // class MethodImpl;                            // Present if HasMethodImplSlot() is true
-
     typedef PCODE NonVtableSlot;   // Present if HasNonVtableSlot() is true
+    // class MethodImpl;           // Present if HasMethodImplSlot() is true
     typedef PCODE NativeCodeSlot;  // Present if HasNativeCodeSlot() is true
 
 // Stub Dispatch code

src/native/managed/cdacreader/Microsoft.Diagnostics.DataContractReader.Abstractions/DataType.cs

@@ -73,4 +73,7 @@ public enum DataType
     HashMap,
     Bucket,
     UnwindInfo,
+    NonVtableSlot,
+    MethodImpl,
+    NativeCodeSlot,
 }

src/native/managed/cdacreader/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs

@@ -145,45 +145,14 @@ private static uint ComputeToken(Target target, Data.MethodDesc desc, Data.Metho
         public TargetPointer CodeData => _desc.CodeData;
         public bool IsIL => Classification == MethodClassification.IL || Classification == MethodClassification.Instantiated;
 
-        public bool HasNativeCodeSlot => HasFlags(MethodDescFlags_1.MethodDescFlags.HasNativeCodeSlot);
-        internal bool HasNonVtableSlot => HasFlags(MethodDescFlags_1.MethodDescFlags.HasNonVtableSlot);
-        internal bool HasMethodImpl => HasFlags(MethodDescFlags_1.MethodDescFlags.HasMethodImpl);
+        internal bool HasNonVtableSlot => MethodDescOptionalSlots.HasNonVtableSlot(_desc.Flags);
+        internal bool HasNativeCodeSlot => MethodDescOptionalSlots.HasNativeCodeSlot(_desc.Flags);
 
         internal bool HasStableEntryPoint => HasFlags(MethodDescFlags_1.MethodDescFlags3.HasStableEntryPoint);
         internal bool HasPrecode => HasFlags(MethodDescFlags_1.MethodDescFlags3.HasPrecode);
 
-        #region Additional Pointers
-        private int AdditionalPointersHelper(MethodDescFlags_1.MethodDescFlags extraFlags)
-            => int.PopCount(_desc.Flags & (ushort)extraFlags);
-
-        // non-vtable slot, native code slot and MethodImpl slots are stored after the MethodDesc itself, packed tightly
-        // in the order: [non-vtable; methhod impl; native code].
-        internal int NonVtableSlotIndex => HasNonVtableSlot ? 0 : throw new InvalidOperationException("no non-vtable slot");
-        internal int MethodImplIndex
-        {
-            get
-            {
-                if (!HasMethodImpl)
-                {
-                    throw new InvalidOperationException("no method impl slot");
-                }
-                return AdditionalPointersHelper(MethodDescFlags_1.MethodDescFlags.HasNonVtableSlot);
-            }
-        }
-        internal int NativeCodeSlotIndex
-        {
-            get
-            {
-                if (!HasNativeCodeSlot)
-                {
-                    throw new InvalidOperationException("no native code slot");
-                }
-                return AdditionalPointersHelper(MethodDescFlags_1.MethodDescFlags.HasNonVtableSlot | MethodDescFlags_1.MethodDescFlags.HasMethodImpl);
-            }
-        }
-
-        internal int AdditionalPointersCount => AdditionalPointersHelper(MethodDescFlags_1.MethodDescFlags.MethodDescAdditionalPointersMask);
-        #endregion Additional Pointers
+        internal TargetPointer GetAddressOfNonVtableSlot() => MethodDescOptionalSlots.GetAddressOfNonVtableSlot(Address, Classification, _desc.Flags, _target);
+        internal TargetPointer GetAddressOfNativeCodeSlot() => MethodDescOptionalSlots.GetAddressOfNativeCodeSlot(Address, Classification, _desc.Flags, _target);
 
         internal bool IsLoaderModuleAttachedToChunk => HasFlags(MethodDescChunkFlags.LoaderModuleAttachedToChunk);
 
@@ -996,48 +965,10 @@ bool IRuntimeTypeSystem.HasNativeCodeSlot(MethodDescHandle methodDesc)
         return md.HasNativeCodeSlot;
     }
 
-    internal static DataType GetMethodClassificationDataType(MethodClassification classification)
-    => classification switch
-    {
-        MethodClassification.IL => DataType.MethodDesc,
-        MethodClassification.FCall => throw new NotImplementedException(), //TODO[cdac]:
-        MethodClassification.PInvoke => throw new NotImplementedException(), //TODO[cdac]:
-        MethodClassification.EEImpl => throw new NotImplementedException(), //TODO[cdac]:
-        MethodClassification.Array => throw new NotImplementedException(), //TODO[cdac]:
-        MethodClassification.Instantiated => DataType.InstantiatedMethodDesc,
-        MethodClassification.ComInterop => throw new NotImplementedException(), //TODO[cdac]:
-        MethodClassification.Dynamic => DataType.DynamicMethodDesc,
-        _ => throw new InvalidOperationException($"Unexpected method classification 0x{classification:x2} for MethodDesc")
-    };
-
-    private uint MethodDescAdditionalPointersOffset(MethodDesc md)
-    {
-        // See MethodDesc::GetBaseSize and s_ClassificationSizeTable
-        // sizeof(MethodDesc),                 mcIL
-        // sizeof(FCallMethodDesc),            mcFCall
-        // sizeof(NDirectMethodDesc),          mcPInvoke
-        // sizeof(EEImplMethodDesc),           mcEEImpl
-        // sizeof(ArrayMethodDesc),            mcArray
-        // sizeof(InstantiatedMethodDesc),     mcInstantiated
-        // sizeof(CLRToCOMCallMethodDesc),     mcComInterOp
-        // sizeof(DynamicMethodDesc)           mcDynamic
-        MethodClassification cls = md.Classification;
-        DataType type = GetMethodClassificationDataType(cls);
-        return _target.GetTypeInfo(type).Size ?? throw new InvalidOperationException($"size of MethodDesc not known");
-    }
-
     TargetPointer IRuntimeTypeSystem.GetAddressOfNativeCodeSlot(MethodDescHandle methodDesc)
     {
         MethodDesc md = _methodDescs[methodDesc.Address];
-        uint offset = MethodDescAdditionalPointersOffset(md);
-        offset += (uint)(_target.PointerSize * md.NativeCodeSlotIndex);
-        return methodDesc.Address + offset;
-    }
-    private TargetPointer GetAddressOfNonVtableSlot(TargetPointer methodDescPointer, MethodDesc md)
-    {
-        uint offset = MethodDescAdditionalPointersOffset(md);
-        offset += (uint)(_target.PointerSize * md.NonVtableSlotIndex);
-        return methodDescPointer.Value + offset;
+        return md.GetAddressOfNativeCodeSlot();
     }
 
     TargetCodePointer IRuntimeTypeSystem.GetNativeCode(MethodDescHandle methodDescHandle)
@@ -1049,30 +980,30 @@ TargetCodePointer IRuntimeTypeSystem.GetNativeCode(MethodDescHandle methodDescHa
             // When profiler is enabled, profiler may ask to rejit a code even though we
             // we have ngen code for this MethodDesc.  (See MethodDesc::DoPrestub).
             // This means that *ppCode is not stable. It can turn from non-zero to zero.
-            TargetPointer ppCode = ((IRuntimeTypeSystem)this).GetAddressOfNativeCodeSlot(methodDescHandle);
+            TargetPointer ppCode = md.GetAddressOfNativeCodeSlot();
             TargetCodePointer pCode = _target.ReadCodePointer(ppCode);
             return CodePointerUtils.CodePointerFromAddress(pCode.AsTargetPointer, _target);
         }
 
         if (!md.HasStableEntryPoint || md.HasPrecode)
             return TargetCodePointer.Null;
 
-        return GetStableEntryPoint(methodDescHandle.Address, md);
+        return GetStableEntryPoint(md);
     }
 
-    private TargetCodePointer GetStableEntryPoint(TargetPointer methodDescAddress, MethodDesc md)
+    private TargetCodePointer GetStableEntryPoint(MethodDesc md)
     {
-        // TODO(cdac): _ASSERTE(HasStableEntryPoint());
         // TODO(cdac): _ASSERTE(!IsVersionableWithVtableSlotBackpatch());
+        Debug.Assert(md.HasStableEntryPoint);
 
-        return GetMethodEntryPointIfExists(methodDescAddress, md);
+        return GetMethodEntryPointIfExists(md);
     }
 
-    private TargetCodePointer GetMethodEntryPointIfExists(TargetPointer methodDescAddress, MethodDesc md)
+    private TargetCodePointer GetMethodEntryPointIfExists(MethodDesc md)
     {
         if (md.HasNonVtableSlot)
         {
-            TargetPointer pSlot = GetAddressOfNonVtableSlot(methodDescAddress, md);
+            TargetPointer pSlot = md.GetAddressOfNonVtableSlot();
             return _target.ReadCodePointer(pSlot);
         }
 

src/native/managed/cdacreader/Microsoft.Diagnostics.DataContractReader.Contracts/RuntimeTypeSystemHelpers/MethodDescFlags_1.cs

@@ -10,14 +10,12 @@ internal static class MethodDescFlags_1
     internal enum MethodDescFlags : ushort
     {
         ClassificationMask = 0x7,
-        #region Additional pointers
-        // The below flags each imply that there's an extra pointer-sized piece of data after the MethodDesc in the MethodDescChunk
+        #region Optional slots
+        // The below flags each imply that there's an extra pointer-size-aligned piece of data after the MethodDesc in the MethodDescChunk
         HasNonVtableSlot = 0x0008,
         HasMethodImpl = 0x0010,
         HasNativeCodeSlot = 0x0020,
-        // Mask for the above flags
-        MethodDescAdditionalPointersMask = 0x0038,
-        #endregion Additional pointers
+        #endregion Optional slots
     }
 
     [Flags]
@@ -35,5 +33,4 @@ internal enum MethodDescEntryPointFlags : byte
     {
         TemporaryEntryPointAssigned = 0x04,
     }
-
 }

src/native/managed/cdacreader/Microsoft.Diagnostics.DataContractReader.Contracts/RuntimeTypeSystemHelpers/MethodDescOptionalSlots.cs

@@ -0,0 +1,92 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+using System;
+
+namespace Microsoft.Diagnostics.DataContractReader.RuntimeTypeSystemHelpers;
+
+// Non-vtable slot, native code slot, and MethodImpl slots are stored after the MethodDesc itself, packed tightly
+// in the order: [non-vtable; method impl; native code].
+internal static class MethodDescOptionalSlots
+{
+    internal static bool HasNonVtableSlot(ushort flags)
+        => (flags & (ushort)MethodDescFlags_1.MethodDescFlags.HasNonVtableSlot) != 0;
+
+    internal static bool HasMethodImpl(ushort flags)
+        => (flags & (ushort)MethodDescFlags_1.MethodDescFlags.HasMethodImpl) != 0;
+
+    internal static bool HasNativeCodeSlot(ushort flags)
+        => (flags & (ushort)MethodDescFlags_1.MethodDescFlags.HasNativeCodeSlot) != 0;
+
+    internal static TargetPointer GetAddressOfNonVtableSlot(TargetPointer methodDesc, MethodClassification classification, ushort flags, Target target)
+    {
+        uint offset = StartOffset(classification, target);
+        offset += NonVtableSlotOffset(flags);
+        return methodDesc + offset;
+    }
+
+    internal static TargetPointer GetAddressOfNativeCodeSlot(TargetPointer methodDesc, MethodClassification classification, ushort flags, Target target)
+    {
+        uint offset = StartOffset(classification, target);
+        offset += NativeCodeSlotOffset(flags, target);
+        return methodDesc + offset;
+    }
+
+    // Offset from the MethodDesc address to the start of its optional slots
+    private static uint StartOffset(MethodClassification classification, Target target)
+    {
+        // See MethodDesc::GetBaseSize and s_ClassificationSizeTable
+        // sizeof(MethodDesc),                 mcIL
+        // sizeof(FCallMethodDesc),            mcFCall
+        // sizeof(NDirectMethodDesc),          mcPInvoke
+        // sizeof(EEImplMethodDesc),           mcEEImpl
+        // sizeof(ArrayMethodDesc),            mcArray
+        // sizeof(InstantiatedMethodDesc),     mcInstantiated
+        // sizeof(CLRToCOMCallMethodDesc),     mcComInterOp
+        // sizeof(DynamicMethodDesc)           mcDynamic
+        DataType type = classification switch
+        {
+            MethodClassification.IL => DataType.MethodDesc,
+            MethodClassification.FCall => throw new NotImplementedException(), //TODO[cdac]:
+            MethodClassification.PInvoke => throw new NotImplementedException(), //TODO[cdac]:
+            MethodClassification.EEImpl => throw new NotImplementedException(), //TODO[cdac]:
+            MethodClassification.Array => throw new NotImplementedException(), //TODO[cdac]:
+            MethodClassification.Instantiated => DataType.InstantiatedMethodDesc,
+            MethodClassification.ComInterop => throw new NotImplementedException(), //TODO[cdac]:
+            MethodClassification.Dynamic => DataType.DynamicMethodDesc,
+            _ => throw new InvalidOperationException($"Unexpected method classification 0x{classification:x2} for MethodDesc")
+        };
+        return target.GetTypeInfo(type).Size ?? throw new InvalidOperationException($"size of MethodDesc not known");
+    }
+
+    // Offsets are from the start of optional slots data (so right after the MethodDesc), obtained via StartOffset
+    private static uint NonVtableSlotOffset(ushort flags)
+    {
+        if (!HasNonVtableSlot(flags))
+            throw new InvalidOperationException("no non-vtable slot");
+
+        return 0u;
+    }
+
+    private static uint MethodImplOffset(ushort flags, Target target)
+    {
+        if (!HasMethodImpl(flags))
+            throw new InvalidOperationException("no method impl slot");
+
+        return HasNonVtableSlot(flags) ? (uint)target.PointerSize : 0;
+    }
+
+    private static uint NativeCodeSlotOffset(ushort flags, Target target)
+    {
+        if (!HasNativeCodeSlot(flags))
+            throw new InvalidOperationException("no native code slot");
+
+        uint offset = 0;
+        if (HasNonVtableSlot(flags))
+            offset += target.GetTypeInfo(DataType.NonVtableSlot).Size!.Value;
+
+        if (HasMethodImpl(flags))
+            offset += target.GetTypeInfo(DataType.MethodImpl).Size!.Value;
+
+        return offset;
+    }
+}