From ffc4b8ac49303d1591e3b8c01b9aca34e23b5003 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 8 Jul 2024 15:35:32 -0400 Subject: [PATCH 01/24] start GetMethodDescDataImpl --- src/coreclr/debug/daccess/dacimpl.h | 1 + src/coreclr/debug/daccess/request.cpp | 65 ++++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/src/coreclr/debug/daccess/dacimpl.h b/src/coreclr/debug/daccess/dacimpl.h index f3d12c3427263..4041a015e9b85 100644 --- a/src/coreclr/debug/daccess/dacimpl.h +++ b/src/coreclr/debug/daccess/dacimpl.h @@ -1241,6 +1241,7 @@ class ClrDataAccess HRESULT GetMethodTableForEEClassImpl (CLRDATA_ADDRESS eeClassReallyMT, CLRDATA_ADDRESS *value); HRESULT GetMethodTableNameImpl(CLRDATA_ADDRESS mt, unsigned int count, _Inout_updates_z_(count) WCHAR *mtName, unsigned int *pNeeded); HRESULT GetObjectExceptionDataImpl(CLRDATA_ADDRESS objAddr, struct DacpExceptionObjectData *data); + HRESULT GetMethodDescDataImpl(CLRDATA_ADDRESS methodDesc, CLRDATA_ADDRESS ip, struct DacpMethodDescData *data, ULONG cRevertedRejitVersions, DacpReJitData * rgRevertedRejitData, ULONG * pcNeededRevertedRejitData); BOOL IsExceptionFromManagedCode(EXCEPTION_RECORD * pExceptionRecord); #ifndef TARGET_UNIX diff --git a/src/coreclr/debug/daccess/request.cpp b/src/coreclr/debug/daccess/request.cpp index 5c759faeac4f9..7062fdfab8db4 100644 --- a/src/coreclr/debug/daccess/request.cpp +++ b/src/coreclr/debug/daccess/request.cpp @@ -1041,6 +1041,70 @@ HRESULT ClrDataAccess::GetMethodDescData( } SOSDacEnter(); + if (m_cdacSos != NULL) + { + // Try the cDAC first - it will return E_NOTIMPL if it doesn't support this method yet. Fall back to the DAC. + hr = m_cdacSos->GetMethodDescData(methodDesc, ip, methodDescData, cRevertedRejitVersions, rgRevertedRejitData, pcNeededRevertedRejitData); + if (FAILED(hr)) + { + hr = GetMethodDescDataImpl(methodDesc, ip, methodDescData, cRevertedRejitVersions, rgRevertedRejitData, pcNeededRevertedRejitData); + } +#ifdef _DEBUG + else + { + // Assert that the data is the same as what we get from the DAC. + DacpMethodDescData mdDataLocal; + DacpReJitData *rgRevertedRejitDataLocal = NULL; + if (rgRevertedRejitData != NULL) + { + rgRevertedRejitDataLocal = new DacpReJitData[cRevertedRejitVersions]; + } + ULONG cNeededRevertedRejitDataLocal = 0; + ULONG *pcNeededRevertedRejitDataLocal = NULL; + if (pcNeededRevertedRejitData != NULL) + { + pcNeededRevertedRejitDataLocal = &cNeededRevertedRejitDataLocal; + } + HRESULT hrLocal = GetMethodDescDataImpl(methodDesc, ip,&mdDataLocal, cRevertedRejitVersions, rgRevertedRejitDataLocal, pcNeededRevertedRejitDataLocal); + _ASSERTE(hr == hrLocal); + _ASSERTE(methodDescData->bHasNativeCode == mdDataLocal.bHasNativeCode); + _ASSERTE(methodDescData->bIsDynamic == mdDataLocal.bIsDynamic); + _ASSERTE(methodDescData->wSlotNumber == mdDataLocal.wSlotNumber); + _ASSERTE(methodDescData->NativeCodeAddr == mdDataLocal.NativeCodeAddr); + _ASSERTE(methodDescData->AddressOfNativeCodeSlot == mdDataLocal.AddressOfNativeCodeSlot); + //TODO[cdac]: assert the rest of mdDataLocal contains the same info as methodDescData + if (rgRevertedRejitData != NULL) + { + _ASSERTE (cNeededRevertedRejitDataLocal == *pcNeededRevertedRejitData); + for (ULONG i = 0; i < cNeededRevertedRejitDataLocal; i++) + { + _ASSERTE(rgRevertedRejitData[i].rejitID == rgRevertedRejitDataLocal[i].rejitID); + _ASSERTE(rgRevertedRejitData[i].NativeCodeAddr == rgRevertedRejitDataLocal[i].NativeCodeAddr); + _ASSERTE(rgRevertedRejitData[i].flags == rgRevertedRejitDataLocal[i].flags); + } + } + } +#endif + } + else + { + hr = GetMethodDescDataImpl(methodDesc, ip, methodDescData, cRevertedRejitVersions, rgRevertedRejitData, pcNeededRevertedRejitData); + } + + SOSDacLeave(); + return hr; +} + +HRESULT ClrDataAccess::GetMethodDescDataImpl( + CLRDATA_ADDRESS methodDesc, + CLRDATA_ADDRESS ip, + struct DacpMethodDescData *methodDescData, + ULONG cRevertedRejitVersions, + DacpReJitData * rgRevertedRejitData, + ULONG * pcNeededRevertedRejitData) +{ + + HRESULT hr = S_OK; PTR_MethodDesc pMD = PTR_MethodDesc(TO_TADDR(methodDesc)); @@ -1236,7 +1300,6 @@ HRESULT ClrDataAccess::GetMethodDescData( } } - SOSDacLeave(); return hr; } From 9e0272abfbe1ef48c61682abc1fd8aa74cdc75b3 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 8 Jul 2024 16:40:52 -0400 Subject: [PATCH 02/24] WIP: managed GetMethodDescData skeleton --- .../cdacreader/src/Legacy/ISOSDacInterface.cs | 51 ++++++++++++++++++- .../cdacreader/src/Legacy/SOSDacImpl.cs | 13 ++++- 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/src/native/managed/cdacreader/src/Legacy/ISOSDacInterface.cs b/src/native/managed/cdacreader/src/Legacy/ISOSDacInterface.cs index ef4952f64f61c..248b3cd5e6eb1 100644 --- a/src/native/managed/cdacreader/src/Legacy/ISOSDacInterface.cs +++ b/src/native/managed/cdacreader/src/Legacy/ISOSDacInterface.cs @@ -93,6 +93,55 @@ internal struct DacpMethodTableData } #pragma warning restore CS0649 // Field is never assigned to, and will always have its default value +internal struct DacpReJitData +{ + // FIXME[cdac]: the C++ definition enum doesn't have an explicit underlying type or constant values for the flags + public enum Flags : uint + { + kUnknown = 0, + kRequested = 1, + kActive = 2, + kReverted = 3, + }; + + public ulong /*CLRDATA_ADDRESS*/ rejitID; + public Flags flags; /* = Flags::kUnknown*/ + public ulong /*CLRDATA_ADDRESS*/ NativeCodeAddr; +}; + +internal struct DacpMethodDescData +{ + public int bHasNativeCode; + public int bIsDynamic; + public ushort wSlotNumber; + public ulong /*CLRDATA_ADDRESS*/ NativeCodeAddr; + // Useful for breaking when a method is jitted. + public ulong /*CLRDATA_ADDRESS*/ AddressOfNativeCodeSlot; + + public ulong /*CLRDATA_ADDRESS*/ MethodDescPtr; + public ulong /*CLRDATA_ADDRESS*/ MethodTablePtr; + public ulong /*CLRDATA_ADDRESS*/ ModulePtr; + + public uint /*mdToken*/ MDToken; + public ulong /*CLRDATA_ADDRESS*/ GCInfo; + public ulong /*CLRDATA_ADDRESS*/ GCStressCodeCopy; + + // This is only valid if bIsDynamic is true + public ulong /*CLRDATA_ADDRESS*/ managedDynamicMethodObject; + + public ulong /*CLRDATA_ADDRESS*/ requestedIP; + + // Gives info for the single currently active version of a method + public DacpReJitData rejitDataCurrent; + + // Gives info corresponding to requestedIP (for !ip2md) + public DacpReJitData rejitDataRequested; + + // Total number of rejit versions that have been jitted + public uint /*ULONG*/ cJittedRejitVersions; + +} + [GeneratedComInterface] [Guid("436f00f2-b42a-4b9f-870c-e73db66ae930")] internal unsafe partial interface ISOSDacInterface @@ -146,7 +195,7 @@ internal unsafe partial interface ISOSDacInterface // MethodDescs [PreserveSig] - int GetMethodDescData(ulong methodDesc, ulong ip, /*struct DacpMethodDescData*/ void* data, uint cRevertedRejitVersions, /*struct DacpReJitData*/ void* rgRevertedRejitData, uint* pcNeededRevertedRejitData); + int GetMethodDescData(ulong methodDesc, ulong ip, DacpMethodDescData* data, uint cRevertedRejitVersions, DacpReJitData* rgRevertedRejitData, uint* pcNeededRevertedRejitData); [PreserveSig] int GetMethodDescPtrFromIP(ulong ip, ulong* ppMD); [PreserveSig] diff --git a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs index e4f2968f29f79..9634ef1db21ab 100644 --- a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs @@ -79,7 +79,18 @@ public int GetBreakingChangeVersion() public unsafe int GetJitHelperFunctionName(ulong ip, uint count, byte* name, uint* pNeeded) => HResults.E_NOTIMPL; public unsafe int GetJitManagerList(uint count, void* managers, uint* pNeeded) => HResults.E_NOTIMPL; public unsafe int GetJumpThunkTarget(void* ctx, ulong* targetIP, ulong* targetMD) => HResults.E_NOTIMPL; - public unsafe int GetMethodDescData(ulong methodDesc, ulong ip, void* data, uint cRevertedRejitVersions, void* rgRevertedRejitData, uint* pcNeededRevertedRejitData) => HResults.E_NOTIMPL; + public unsafe int GetMethodDescData(ulong methodDesc, ulong ip, DacpMethodDescData* data, uint cRevertedRejitVersions, DacpReJitData* rgRevertedRejitData, uint* pcNeededRevertedRejitData) + { + try + { + return HResults.E_NOTIMPL; + } + catch (Exception ex) + { + return ex.HResult; + } + } + public unsafe int GetMethodDescFromToken(ulong moduleAddr, uint token, ulong* methodDesc) => HResults.E_NOTIMPL; public unsafe int GetMethodDescName(ulong methodDesc, uint count, char* name, uint* pNeeded) => HResults.E_NOTIMPL; public unsafe int GetMethodDescPtrFromFrame(ulong frameAddr, ulong* ppMD) => HResults.E_NOTIMPL; From 29e404c828626f4a2f356e1c99ef96c588799324 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 9 Jul 2024 17:00:31 -0400 Subject: [PATCH 03/24] wip: MethodDesc --- .../debug/runtimeinfo/datadescriptor.h | 10 ++++ src/coreclr/vm/method.hpp | 7 +++ .../src/Contracts/RuntimeTypeSystem.cs | 14 +++++ .../RuntimeTypeSystem_1.NonValidated.cs | 52 +++++++++++++++++++ .../src/Contracts/RuntimeTypeSystem_1.cs | 40 ++++++++++++++ .../managed/cdacreader/src/Data/MethodDesc.cs | 19 +++++++ src/native/managed/cdacreader/src/DataType.cs | 1 + .../cdacreader/src/Legacy/SOSDacImpl.cs | 17 ++++++ 8 files changed, 160 insertions(+) create mode 100644 src/native/managed/cdacreader/src/Data/MethodDesc.cs diff --git a/src/coreclr/debug/runtimeinfo/datadescriptor.h b/src/coreclr/debug/runtimeinfo/datadescriptor.h index da83bebcfea39..09db780e0b313 100644 --- a/src/coreclr/debug/runtimeinfo/datadescriptor.h +++ b/src/coreclr/debug/runtimeinfo/datadescriptor.h @@ -252,6 +252,15 @@ CDAC_TYPE_FIELD(DynamicMetadata, /*uint32*/, Size, cdac_offsets CDAC_TYPE_FIELD(DynamicMetadata, /*inline byte array*/, Data, cdac_offsets::Data) CDAC_TYPE_END(DynamicMetadata) +CDAC_TYPE_BEGIN(MethodDesc) +CDAC_TYPE_INDETERMINATE(EEClass) +CDAC_TYPE_FIELD(MethodDesc, /*uint8*/, ChunkIndex, cdac_offsets::ChunkIndex) +CDAC_TYPE_END(MethodDesc) + +CDAC_TYPE_BEGIN(MethodDescChunk) +CDAC_TYPE_SIZE(sizeof(MethodDescChunk)) +CDAC_TYPE_END(MethodDescChunk) + CDAC_TYPES_END() CDAC_GLOBALS_BEGIN() @@ -265,6 +274,7 @@ CDAC_GLOBAL(FeatureEHFunclets, uint8, 1) CDAC_GLOBAL(FeatureEHFunclets, uint8, 0) #endif CDAC_GLOBAL(SOSBreakingChangeVersion, uint8, SOS_BREAKING_CHANGE_VERSION) +CDAC_GLOBAL(MethodDescAlignment, uint64, MethodDesc::ALIGNMENT) CDAC_GLOBAL_POINTER(FreeObjectMethodTable, &::g_pFreeObjectMethodTable) CDAC_GLOBAL_POINTER(MiniMetaDataBuffAddress, &::g_MiniMetaDataBuffAddress) CDAC_GLOBAL_POINTER(MiniMetaDataBuffMaxSize, &::g_MiniMetaDataBuffMaxSize) diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index 3d1f41eaa0144..57a8ebc4cbf76 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -1908,6 +1908,13 @@ class MethodDesc public: static void Init(); #endif + + template friend struct ::cdac_offsets; +}; + +template<> struct cdac_offsets +{ + static const size_t ChunkIndex = offsetof(MethodDesc, m_chunkIndex); }; #ifndef DACCESS_COMPILE diff --git a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem.cs b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem.cs index b7267b5f8c23c..73514e7e8df11 100644 --- a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem.cs +++ b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem.cs @@ -55,6 +55,16 @@ internal enum CorElementType Sentinel = 0x41, } +internal readonly struct MethodDescHandle +{ + internal MethodDescHandle(TargetPointer address) + { + Address = address; + } + + internal TargetPointer Address { get; } +} + internal interface IRuntimeTypeSystem : IContract { static string IContract.Name => nameof(RuntimeTypeSystem); @@ -113,6 +123,10 @@ static IContract IContract.Create(Target target, int version) public virtual bool IsFunctionPointer(TypeHandle typeHandle, out ReadOnlySpan retAndArgTypes, out byte callConv) => throw new NotImplementedException(); // Returns null if the TypeHandle is not a class/struct/generic variable #endregion TypeHandle inspection APIs + + #region MethodDesc inspection APIs + public virtual MethodDescHandle GetMethodDescHandle(TargetPointer targetPointer) => throw new NotImplementedException(); + #endregion MethodDesc inspection APIs } internal struct RuntimeTypeSystem : IRuntimeTypeSystem diff --git a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs index ef392130e6231..3f9a0ea6fef58 100644 --- a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs +++ b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs @@ -85,6 +85,26 @@ internal EEClass(Target target, TargetPointer eeClassPointer) internal TargetPointer MethodTable => _target.ReadPointer(Address + (ulong)_type.Fields[nameof(MethodTable)].Offset); } + + internal struct MethodDesc + { + public readonly Target _target; + public readonly Target.TypeInfo _type; + internal TargetPointer Address { get; init; } + + internal MethodDesc(Target target, TargetPointer methodDescPointer) + { + _target = target; + _type = target.GetTypeInfo(DataType.pointer /*DataType.MethodDesc*/); // TODO + Address = methodDescPointer; + } + +#pragma warning disable CA1822 // Mark members as static + internal TargetPointer MethodTable => TargetPointer.Null; // TODO + internal ushort Slot => (ushort)0xffffu; // TODO + internal bool HasNonVtableSlot => false; // TODO +#pragma warning restore CA1822 // Mark members as static + } internal static MethodTable GetMethodTableData(Target target, TargetPointer methodTablePointer) { return new MethodTable(target, methodTablePointer); @@ -95,6 +115,10 @@ internal static EEClass GetEEClassData(Target target, TargetPointer eeClassPoint return new EEClass(target, eeClassPointer); } + internal static MethodDesc GetMethodDescData(Target target, TargetPointer methodDescPointer) + { + return new MethodDesc(target, methodDescPointer); + } } /// @@ -194,5 +218,33 @@ private TargetPointer GetClassThrowing(NonValidated.MethodTable methodTable) } } + private bool ValidateMethodDescPointer(NonValidated.MethodDesc umd) + { + try + { + TargetPointer methodTablePointer = umd.MethodTable; + if (methodTablePointer == TargetPointer.Null + || methodTablePointer == new TargetPointer(0xffffffff_fffffffful) + || methodTablePointer == new TargetPointer(0x00000000_fffffffful)) + { + return false; + } + MethodTableHandle methodTableHandle = GetMethodTableHandle(methodTablePointer); + + if (umd.Slot >= GetNumMethods(methodTableHandle) && !umd.HasNonVtableSlot) // FIXME: request.cpp looks at m_usNumVtableSlots + { + return false; + } + } + catch (System.Exception) + { + // TODO(cdac): maybe don't swallow all exceptions? We could consider a richer contract that + // helps to track down what sort of memory corruption caused the validation to fail. + // TODO(cdac): we could also consider a more fine-grained exception type so we don't mask + // programmer mistakes in cdacreader. + return false; + } + return true; + } } diff --git a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs index 9c3c3bc3f4974..9496b8ca50dfe 100644 --- a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs +++ b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs @@ -19,6 +19,7 @@ internal partial struct RuntimeTypeSystem_1 : IRuntimeTypeSystem // TODO(cdac): we mutate this dictionary - copies of the RuntimeTypeSystem_1 struct share this instance. // If we need to invalidate our view of memory, we should clear this dictionary. private readonly Dictionary _methodTables = new(); + private readonly Dictionary _methodDescs = new(); internal struct MethodTable @@ -67,6 +68,11 @@ internal enum TypeHandleBits ValidMask = 2, } + internal struct MethodDesc + { + + } + internal RuntimeTypeSystem_1(Target target, TargetPointer freeObjectMethodTablePointer) { _target = target; @@ -417,4 +423,38 @@ private FunctionPointerRetAndArgs(Target target, TargetPointer typePointer) } } } + + public MethodDescHandle GetMethodDescHandle(TargetPointer methodDescPointer) + { + // if we already validated this address, return a handle + if (_methodDescs.ContainsKey(methodDescPointer)) + { + return new MethodDescHandle(methodDescPointer); + } +#if false // TODO + // Check if we cached the underlying data already + if (_target.ProcessedData.TryGet(methodTablePointer, out Data.MethodTable? methodTableData)) + { + // we already cached the data, we must have validated the address, create the representation struct for our use + MethodTable trustedMethodTable = new MethodTable(methodTableData); + _ = _methodTables.TryAdd(methodTablePointer, trustedMethodTable); + return new MethodTableHandle(methodTablePointer); + } +#endif + + // Otherwse, get ready to validate + NonValidated.MethodDesc nonvalidatedMethodDesc = NonValidated.GetMethodDescData(_target, methodDescPointer); + + if (!ValidateMethodDescPointer(nonvalidatedMethodDesc)) + { + throw new InvalidOperationException("Invalid method desc pointer"); + } + // ok, we validated it, cache the data and add the MethodTable_1 struct to the dictionary +#if false // TODO + Data.MethodDesc trustedMethodDescData = _target.ProcessedData.GetOrAdd(methodDescPointer); +#endif + MethodDesc trustedMethodDescF = default; // new MethodDesc(trustedMethodTableData); // TODO + _ = _methodDescs.TryAdd(methodDescPointer, trustedMethodDescF); + return new MethodDescHandle(methodDescPointer); + } } diff --git a/src/native/managed/cdacreader/src/Data/MethodDesc.cs b/src/native/managed/cdacreader/src/Data/MethodDesc.cs new file mode 100644 index 0000000000000..eff33f246a998 --- /dev/null +++ b/src/native/managed/cdacreader/src/Data/MethodDesc.cs @@ -0,0 +1,19 @@ +// 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.Data; + +internal sealed class MethodDesc : IData +{ + static MethodDesc IData.Create(Target target, TargetPointer address) => new MethodDesc(target, address); +#pragma warning disable IDE0060 // Remove unused parameter + public MethodDesc(Target target, TargetPointer address) + { + + } +#pragma warning restore IDE0060 + + public byte ChunkIndex { get; init; } +} diff --git a/src/native/managed/cdacreader/src/DataType.cs b/src/native/managed/cdacreader/src/DataType.cs index ef037be8ed1c8..7b6c3a3257b3b 100644 --- a/src/native/managed/cdacreader/src/DataType.cs +++ b/src/native/managed/cdacreader/src/DataType.cs @@ -37,4 +37,5 @@ public enum DataType TypeVarTypeDesc, FnPtrTypeDesc, DynamicMetadata, + MethodDesc, } diff --git a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs index 9634ef1db21ab..02a99ae0c9b03 100644 --- a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs @@ -81,8 +81,25 @@ public int GetBreakingChangeVersion() public unsafe int GetJumpThunkTarget(void* ctx, ulong* targetIP, ulong* targetMD) => HResults.E_NOTIMPL; public unsafe int GetMethodDescData(ulong methodDesc, ulong ip, DacpMethodDescData* data, uint cRevertedRejitVersions, DacpReJitData* rgRevertedRejitData, uint* pcNeededRevertedRejitData) { + if (methodDesc == 0) + { + return HResults.E_INVALIDARG; + } + if (cRevertedRejitVersions != 0 && rgRevertedRejitData == null) + { + return HResults.E_INVALIDARG; + } + if (rgRevertedRejitData != null && pcNeededRevertedRejitData == null) + { + // If you're asking for reverted rejit data, you'd better ask for the number of + // elements we return + return HResults.E_INVALIDARG; + } try { + Contracts.IRuntimeTypeSystem rtsContract = _target.Contracts.RuntimeTypeSystem; + Contracts.MethodDescHandle methodDescHandle = rtsContract.GetMethodDescHandle(methodDesc); + return HResults.E_NOTIMPL; } catch (Exception ex) From 9719c7590b017ee2e789d03e6f147f6f0e456dc0 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 10 Jul 2024 10:37:42 -0400 Subject: [PATCH 04/24] add MethodDescChunk --- .../cdacreader/src/Data/MethodDescChunk.cs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 src/native/managed/cdacreader/src/Data/MethodDescChunk.cs diff --git a/src/native/managed/cdacreader/src/Data/MethodDescChunk.cs b/src/native/managed/cdacreader/src/Data/MethodDescChunk.cs new file mode 100644 index 0000000000000..617693efcb199 --- /dev/null +++ b/src/native/managed/cdacreader/src/Data/MethodDescChunk.cs @@ -0,0 +1,18 @@ +// 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.Data; + +internal sealed class MethodDescChunk : IData +{ + static MethodDescChunk IData.Create(Target target, TargetPointer address) => new MethodDescChunk(target, address); +#pragma warning disable IDE0060 // Remove unused parameter + public MethodDescChunk(Target target, TargetPointer address) + { + + } +#pragma warning restore IDE0060 + +} From f076f192549284a7d7fcdca410c9cf0979feb4c2 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 11 Jul 2024 15:52:16 -0400 Subject: [PATCH 05/24] WIP: validating a MethodDesc --- .../design/datacontracts/RuntimeTypeSystem.md | 16 +++- .../debug/runtimeinfo/datadescriptor.h | 9 +- src/coreclr/vm/class.h | 1 + src/coreclr/vm/method.hpp | 13 +++ src/coreclr/vm/methodtable.h | 2 +- .../managed/cdacreader/src/Constants.cs | 2 + .../src/Contracts/RuntimeTypeSystem.cs | 3 +- .../RuntimeTypeSystem_1.NonValidated.cs | 87 +++++++++++++++---- .../src/Contracts/RuntimeTypeSystem_1.cs | 28 ++++-- .../managed/cdacreader/src/Data/EEClass.cs | 3 + .../managed/cdacreader/src/Data/MethodDesc.cs | 8 +- .../cdacreader/src/Data/MethodDescChunk.cs | 11 ++- src/native/managed/cdacreader/src/DataType.cs | 1 + 13 files changed, 155 insertions(+), 29 deletions(-) diff --git a/docs/design/datacontracts/RuntimeTypeSystem.md b/docs/design/datacontracts/RuntimeTypeSystem.md index 41d01f6e7db47..dda33dc1f9d5e 100644 --- a/docs/design/datacontracts/RuntimeTypeSystem.md +++ b/docs/design/datacontracts/RuntimeTypeSystem.md @@ -4,10 +4,11 @@ This contract is for exploring the properties of the runtime types of values on ## APIs of contract +### TypeHandle + A `TypeHandle` is the runtime representation of the type information about a value which is represented as a TypeHandle. Given a `TargetPointer` address, the `RuntimeTypeSystem` contract provides a `TypeHandle` for querying the details of the `TypeHandle`. - ``` csharp struct TypeHandle { @@ -75,8 +76,14 @@ A `TypeHandle` is the runtime representation of the type information about a val #endregion TypeHandle inspection APIs ``` +### MethodDesc + +A `MethodDesc` is the runtime representationn of a managed method (either from IL, from reflection emit, or generated by the runtime). + ## Version 1 +### TypeHandle + The `MethodTable` inspection APIs are implemented in terms of the following flags on the runtime `MethodTable` structure: ``` csharp @@ -523,3 +530,10 @@ The contract additionally depends on these data descriptors return true; } ``` + +### MethodDesc + +The version 1 `MethodDesc` APIs depend on the `MethodDescAlignment` global and the `MethodDesc` and `MethodDescChunk` data descriptors. + +In the runtime a `MethodDesc` implicitly belongs to a single `MethodDescChunk` and some common data is shared between method descriptors that belong to the same chunk. A single method table +will typically have multiple chunks. There are subkinds of MethodDescs at runtime of varying sizes (but the sizes must be mutliples of `MethodDescAlignment`) and each chunk contains method descriptors of the same size. diff --git a/src/coreclr/debug/runtimeinfo/datadescriptor.h b/src/coreclr/debug/runtimeinfo/datadescriptor.h index 09db780e0b313..09c269cf0e138 100644 --- a/src/coreclr/debug/runtimeinfo/datadescriptor.h +++ b/src/coreclr/debug/runtimeinfo/datadescriptor.h @@ -212,6 +212,7 @@ CDAC_TYPE_FIELD(EEClass, /*pointer*/, MethodTable, cdac_offsets::Method CDAC_TYPE_FIELD(EEClass, /*uint16*/, NumMethods, cdac_offsets::NumMethods) CDAC_TYPE_FIELD(EEClass, /*uint32*/, CorTypeAttr, cdac_offsets::CorTypeAttr) CDAC_TYPE_FIELD(EEClass, /*uint8*/, InternalCorElementType, cdac_offsets::InternalCorElementType) +CDAC_TYPE_FIELD(EEClass, /*uint16*/, NumNonVirtualSlots, cdac_offsets::NumNonVirtualSlots) CDAC_TYPE_END(EEClass) CDAC_TYPE_BEGIN(ArrayClass) @@ -253,12 +254,18 @@ CDAC_TYPE_FIELD(DynamicMetadata, /*inline byte array*/, Data, cdac_offsets::ChunkIndex) +CDAC_TYPE_FIELD(MethodDesc, /*uint8*/, Slot, cdac_offsets::Slot) +CDAC_TYPE_FIELD(MethodDesc, /*uint16*/, Flags, cdac_offsets::Flags) CDAC_TYPE_END(MethodDesc) CDAC_TYPE_BEGIN(MethodDescChunk) CDAC_TYPE_SIZE(sizeof(MethodDescChunk)) +CDAC_TYPE_FIELD(MethodDescChunk, /*pointer*/, MethodTable, cdac_offsets::MethodTable) +CDAC_TYPE_FIELD(MethodDescChunk, /*pointer*/, Next, cdac_offsets::Next) +CDAC_TYPE_FIELD(MethodDescChunk, /*uint8*/, Size, cdac_offsets::Size) +CDAC_TYPE_FIELD(MethodDescChunk, /*uint8*/, Count, cdac_offsets::Count) CDAC_TYPE_END(MethodDescChunk) CDAC_TYPES_END() diff --git a/src/coreclr/vm/class.h b/src/coreclr/vm/class.h index e281b8e365e7e..260abc08fa6fa 100644 --- a/src/coreclr/vm/class.h +++ b/src/coreclr/vm/class.h @@ -1807,6 +1807,7 @@ template<> struct cdac_offsets static constexpr size_t MethodTable = offsetof(EEClass, m_pMethodTable); static constexpr size_t NumMethods = offsetof(EEClass, m_NumMethods); static constexpr size_t CorTypeAttr = offsetof(EEClass, m_dwAttrClass); + static constexpr size_t NumNonVirtualSlots = offsetof(EEClass, m_NumNonVirtualSlots); }; // -------------------------------------------------------------------------------------------- diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index 57a8ebc4cbf76..2fa5125d1f373 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -1915,6 +1915,8 @@ class MethodDesc template<> struct cdac_offsets { static const size_t ChunkIndex = offsetof(MethodDesc, m_chunkIndex); + static const size_t Slot = offsetof(MethodDesc, m_wSlotNumber); + static const size_t Flags = offsetof(MethodDesc, m_wFlags); }; #ifndef DACCESS_COMPILE @@ -2335,6 +2337,17 @@ class MethodDescChunk UINT16 m_flagsAndTokenRange; // Followed by array of method descs... + + template friend struct ::cdac_offsets; +}; + +template<> +struct cdac_offsets +{ + static constexpr size_t MethodTable = offsetof(MethodDescChunk, m_methodTable); + static constexpr size_t Next = offsetof(MethodDescChunk, m_next); + static constexpr size_t Size = offsetof(MethodDescChunk, m_size); + static constexpr size_t Count = offsetof(MethodDescChunk, m_count); }; inline int MethodDesc::GetMethodDescChunkIndex() const diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h index c372ac692f112..b3177ef4c989c 100644 --- a/src/coreclr/vm/methodtable.h +++ b/src/coreclr/vm/methodtable.h @@ -621,7 +621,7 @@ struct DynamicStaticsInfo // If it has, then we don't need to do anything return false; } - + if (isClassInitedByUpdatingStaticPointer) { oldValFromInterlockedOp = InterlockedCompareExchangeT(pAddr, newVal, oldVal); diff --git a/src/native/managed/cdacreader/src/Constants.cs b/src/native/managed/cdacreader/src/Constants.cs index 6ca10e46a949b..24ab384f75362 100644 --- a/src/native/managed/cdacreader/src/Constants.cs +++ b/src/native/managed/cdacreader/src/Constants.cs @@ -20,5 +20,7 @@ internal static class Globals internal const string MiniMetaDataBuffAddress = nameof(MiniMetaDataBuffAddress); internal const string MiniMetaDataBuffMaxSize = nameof(MiniMetaDataBuffMaxSize); + + internal const string MethodDescAlignment = nameof(MethodDescAlignment); } } diff --git a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem.cs b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem.cs index 73514e7e8df11..d5b7c48f8dc19 100644 --- a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem.cs +++ b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem.cs @@ -72,9 +72,10 @@ static IContract IContract.Create(Target target, int version) { TargetPointer targetPointer = target.ReadGlobalPointer(Constants.Globals.FreeObjectMethodTable); TargetPointer freeObjectMethodTable = target.ReadPointer(targetPointer); + ulong methodDescAlignment = target.ReadGlobal(Constants.Globals.MethodDescAlignment); return version switch { - 1 => new RuntimeTypeSystem_1(target, freeObjectMethodTable), + 1 => new RuntimeTypeSystem_1(target, freeObjectMethodTable, methodDescAlignment), _ => default(RuntimeTypeSystem), }; } diff --git a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs index 3f9a0ea6fef58..997a0b5a71ef4 100644 --- a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs +++ b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs @@ -89,22 +89,25 @@ internal EEClass(Target target, TargetPointer eeClassPointer) internal struct MethodDesc { public readonly Target _target; - public readonly Target.TypeInfo _type; - internal TargetPointer Address { get; init; } - - internal MethodDesc(Target target, TargetPointer methodDescPointer) + public TargetPointer Address { get; init; } + public readonly Data.MethodDesc _desc; + public readonly Data.MethodDescChunk _chunk; + internal MethodDesc(Target target, TargetPointer methodDescPointer, Data.MethodDesc desc, Data.MethodDescChunk chunk) { _target = target; - _type = target.GetTypeInfo(DataType.pointer /*DataType.MethodDesc*/); // TODO + _desc = desc; + _chunk = chunk; Address = methodDescPointer; } -#pragma warning disable CA1822 // Mark members as static - internal TargetPointer MethodTable => TargetPointer.Null; // TODO - internal ushort Slot => (ushort)0xffffu; // TODO - internal bool HasNonVtableSlot => false; // TODO -#pragma warning restore CA1822 // Mark members as static + private bool HasFlag(MethodDescFlags flag) => (_desc.Flags & (ushort)flag) != 0; + + internal byte ChunkIndex => _desc.ChunkIndex; + internal TargetPointer MethodTable => _chunk.MethodTable; + internal ushort Slot => _desc.Slot; + internal bool HasNonVtableSlot => HasFlag(MethodDescFlags.HasNonVtableSlot); } + internal static MethodTable GetMethodTableData(Target target, TargetPointer methodTablePointer) { return new MethodTable(target, methodTablePointer); @@ -115,10 +118,6 @@ internal static EEClass GetEEClassData(Target target, TargetPointer eeClassPoint return new EEClass(target, eeClassPointer); } - internal static MethodDesc GetMethodDescData(Target target, TargetPointer methodDescPointer) - { - return new MethodDesc(target, methodDescPointer); - } } /// @@ -218,10 +217,36 @@ private TargetPointer GetClassThrowing(NonValidated.MethodTable methodTable) } } - private bool ValidateMethodDescPointer(NonValidated.MethodDesc umd) + private TargetPointer GetMethodDescChunkPointerMayThrow(TargetPointer methodDescPointer, Data.MethodDesc umd) + { + ulong? methodDescSize = _target.GetTypeInfo(DataType.MethodDesc).Size; + if (!methodDescSize.HasValue) + { + throw new InvalidOperationException("Target has no definite MethodDesc size"); + } + ulong chunkAddress = (ulong)methodDescPointer - methodDescSize.Value - umd.ChunkIndex * MethodDescAlignment; + return new TargetPointer(chunkAddress); + } + + private Data.MethodDescChunk GetMethodDescChunkMayThrow(TargetPointer methodDescPointer, Data.MethodDesc md) + { + return new Data.MethodDescChunk(_target, GetMethodDescChunkPointerMayThrow(methodDescPointer, md)); + } + + private NonValidated.MethodDesc GetMethodDescMayThrow(TargetPointer methodDescPointer) + { + // may throw if the method desc at methodDescPointer is corrupted + // we bypass the target data cache here because we don't want to cache non-validated data + Data.MethodDesc desc = new Data.MethodDesc(_target, methodDescPointer); + Data.MethodDescChunk chunk = GetMethodDescChunkMayThrow(methodDescPointer, desc); + return new NonValidated.MethodDesc(_target, methodDescPointer, desc, chunk); + } + + private bool ValidateMethodDescPointer(TargetPointer methodDescPointer) { try { + NonValidated.MethodDesc umd = GetMethodDescMayThrow(methodDescPointer); TargetPointer methodTablePointer = umd.MethodTable; if (methodTablePointer == TargetPointer.Null || methodTablePointer == new TargetPointer(0xffffffff_fffffffful) @@ -231,10 +256,40 @@ private bool ValidateMethodDescPointer(NonValidated.MethodDesc umd) } MethodTableHandle methodTableHandle = GetMethodTableHandle(methodTablePointer); - if (umd.Slot >= GetNumMethods(methodTableHandle) && !umd.HasNonVtableSlot) // FIXME: request.cpp looks at m_usNumVtableSlots + if (umd.Slot >= GetNumVtableSlots(methodTableHandle) && !umd.HasNonVtableSlot) { return false; } + // TODO: request.cpp +#if false + MethodDesc *pMDCheck = MethodDesc::GetMethodDescFromStubAddr(pMD->GetTemporaryEntryPoint(), TRUE); + + if (PTR_HOST_TO_TADDR(pMD) != PTR_HOST_TO_TADDR(pMDCheck)) + { + retval = FALSE; + } + } + + if (retval && pMD->HasNativeCode() && !pMD->IsFCall()) + { + PCODE jitCodeAddr = pMD->GetNativeCode(); + + MethodDesc *pMDCheck = ExecutionManager::GetCodeMethodDesc(jitCodeAddr); + if (pMDCheck) + { + // Check that the given MethodDesc matches the MethodDesc from + // the CodeHeader + if (PTR_HOST_TO_TADDR(pMD) != PTR_HOST_TO_TADDR(pMDCheck)) + { + retval = FALSE; + } + } + else + { + retval = FALSE; + } + } +#endif } catch (System.Exception) diff --git a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs index 9496b8ca50dfe..0367e972fbec1 100644 --- a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs +++ b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs @@ -15,6 +15,7 @@ internal partial struct RuntimeTypeSystem_1 : IRuntimeTypeSystem { private readonly Target _target; private readonly TargetPointer _freeObjectMethodTablePointer; + private readonly ulong _methodDescAlignment; // TODO(cdac): we mutate this dictionary - copies of the RuntimeTypeSystem_1 struct share this instance. // If we need to invalidate our view of memory, we should clear this dictionary. @@ -46,6 +47,9 @@ internal MethodTable(Data.MethodTable data) ParentMethodTable = data.ParentMethodTable; PerInstInfo = data.PerInstInfo; } + + // this MethodTable is a canonical MethodTable if it's EEClassOrCanonMT is an EEClass + internal bool IsCanonMT => GetEEClassOrCanonMTBits(EEClassOrCanonMT) == EEClassOrCanonMTBits.EEClass; } // Low order bit of EEClassOrCanonMT. @@ -68,19 +72,28 @@ internal enum TypeHandleBits ValidMask = 2, } + [Flags] + internal enum MethodDescFlags : ushort + { + HasNonVtableSlot = 0x0008, + } + internal struct MethodDesc { } - internal RuntimeTypeSystem_1(Target target, TargetPointer freeObjectMethodTablePointer) + internal RuntimeTypeSystem_1(Target target, TargetPointer freeObjectMethodTablePointer, ulong methodDescAlignment) { _target = target; _freeObjectMethodTablePointer = freeObjectMethodTablePointer; + _methodDescAlignment = methodDescAlignment; } internal TargetPointer FreeObjectMethodTablePointer => _freeObjectMethodTablePointer; + internal ulong MethodDescAlignment => _methodDescAlignment; + public TypeHandle GetTypeHandle(TargetPointer typeHandlePointer) { TypeHandleBits addressLowBits = (TypeHandleBits)((ulong)typeHandlePointer & ((ulong)_target.PointerSize - 1)); @@ -422,6 +435,14 @@ private FunctionPointerRetAndArgs(Target target, TargetPointer typePointer) TypeHandles[i] = rts.GetTypeHandle(target.ReadPointer(retAndArgs + (ulong)target.PointerSize * (ulong)i)); } } + + internal ushort GetNumVtableSlots(TypeHandleHandle typeHandle) + { + if (!typeHandle.IsMethodTable()) + return 0; + MethodTable methodTable = _methodTables[typeHandleHandle.Address]; + ushort numNonVirtualSlots = methodTable.IsCanonMT ? GetClassData(typeHandle).NumNonVirtualSlots : (ushort)0; + return checked((ushort)(methodTable.NumVirtuals + numNonVirtualSlots)); } public MethodDescHandle GetMethodDescHandle(TargetPointer methodDescPointer) @@ -442,10 +463,7 @@ public MethodDescHandle GetMethodDescHandle(TargetPointer methodDescPointer) } #endif - // Otherwse, get ready to validate - NonValidated.MethodDesc nonvalidatedMethodDesc = NonValidated.GetMethodDescData(_target, methodDescPointer); - - if (!ValidateMethodDescPointer(nonvalidatedMethodDesc)) + if (!ValidateMethodDescPointer(methodDescPointer)) { throw new InvalidOperationException("Invalid method desc pointer"); } diff --git a/src/native/managed/cdacreader/src/Data/EEClass.cs b/src/native/managed/cdacreader/src/Data/EEClass.cs index ea863ce178e51..fd0ad35adbf9d 100644 --- a/src/native/managed/cdacreader/src/Data/EEClass.cs +++ b/src/native/managed/cdacreader/src/Data/EEClass.cs @@ -14,6 +14,7 @@ public EEClass(Target target, TargetPointer address) NumMethods = target.Read(address + (ulong)type.Fields[nameof(NumMethods)].Offset); CorTypeAttr = target.Read(address + (ulong)type.Fields[nameof(CorTypeAttr)].Offset); InternalCorElementType = target.Read(address + (ulong)type.Fields[nameof(InternalCorElementType)].Offset); + NumNonVirtualSlots = target.Read(address + (ulong)type.Fields[nameof(NumNonVirtualSlots)].Offset); } public TargetPointer MethodTable { get; init; } @@ -28,6 +29,8 @@ public EEClass(Target target, TargetPointer address) // Enums are the element type of their underlying type // ValueTypes which can exactly be represented as an element type are represented as such public byte InternalCorElementType { get; init; } + + public ushort NumNonVirtualSlots { get; init; } } public sealed class ArrayClass : IData diff --git a/src/native/managed/cdacreader/src/Data/MethodDesc.cs b/src/native/managed/cdacreader/src/Data/MethodDesc.cs index eff33f246a998..edaa5ccaffd7f 100644 --- a/src/native/managed/cdacreader/src/Data/MethodDesc.cs +++ b/src/native/managed/cdacreader/src/Data/MethodDesc.cs @@ -8,12 +8,16 @@ namespace Microsoft.Diagnostics.DataContractReader.Data; internal sealed class MethodDesc : IData { static MethodDesc IData.Create(Target target, TargetPointer address) => new MethodDesc(target, address); -#pragma warning disable IDE0060 // Remove unused parameter public MethodDesc(Target target, TargetPointer address) { + Target.TypeInfo type = target.GetTypeInfo(DataType.MethodDesc); + ChunkIndex = target.Read(address + (ulong)type.Fields[nameof(ChunkIndex)].Offset); + Slot = target.Read(address + (ulong)type.Fields[nameof(Slot)].Offset); + Flags = target.Read(address + (ulong)type.Fields[nameof(Flags)].Offset); } -#pragma warning restore IDE0060 public byte ChunkIndex { get; init; } + public byte Slot { get; init; } + public ushort Flags { get; init; } } diff --git a/src/native/managed/cdacreader/src/Data/MethodDescChunk.cs b/src/native/managed/cdacreader/src/Data/MethodDescChunk.cs index 617693efcb199..3d397a124df6d 100644 --- a/src/native/managed/cdacreader/src/Data/MethodDescChunk.cs +++ b/src/native/managed/cdacreader/src/Data/MethodDescChunk.cs @@ -8,11 +8,18 @@ namespace Microsoft.Diagnostics.DataContractReader.Data; internal sealed class MethodDescChunk : IData { static MethodDescChunk IData.Create(Target target, TargetPointer address) => new MethodDescChunk(target, address); -#pragma warning disable IDE0060 // Remove unused parameter public MethodDescChunk(Target target, TargetPointer address) { + Target.TypeInfo type = target.GetTypeInfo(DataType.MethodDescChunk); + MethodTable = target.ReadPointer(address + (ulong)type.Fields[nameof(MethodTable)].Offset); + Next = target.ReadPointer(address + (ulong)type.Fields[nameof(Next)].Offset); + Size = target.Read(address + (ulong)type.Fields[nameof(Size)].Offset); + Count = target.Read(address + (ulong)type.Fields[nameof(Count)].Offset); } -#pragma warning restore IDE0060 + public TargetPointer MethodTable { get; init; } + public TargetPointer Next { get; init; } + public byte Size { get; init; } + public byte Count { get; init; } } diff --git a/src/native/managed/cdacreader/src/DataType.cs b/src/native/managed/cdacreader/src/DataType.cs index 7b6c3a3257b3b..4fa5f198068a0 100644 --- a/src/native/managed/cdacreader/src/DataType.cs +++ b/src/native/managed/cdacreader/src/DataType.cs @@ -38,4 +38,5 @@ public enum DataType FnPtrTypeDesc, DynamicMetadata, MethodDesc, + MethodDescChunk, } From c93562624fe86b32f2cf67048a85336695ab959e Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 12 Jul 2024 12:25:47 -0400 Subject: [PATCH 06/24] checkpoint: MethodDesc validation --- .../src/Contracts/RuntimeTypeSystem.cs | 1 + .../RuntimeTypeSystem_1.NonValidated.cs | 30 +++++++----- .../src/Contracts/RuntimeTypeSystem_1.cs | 48 +++++++++++++------ .../cdacreader/src/Legacy/SOSDacImpl.cs | 2 + 4 files changed, 56 insertions(+), 25 deletions(-) diff --git a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem.cs b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem.cs index d5b7c48f8dc19..8cb8a2c8675c5 100644 --- a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem.cs +++ b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem.cs @@ -127,6 +127,7 @@ static IContract IContract.Create(Target target, int version) #region MethodDesc inspection APIs public virtual MethodDescHandle GetMethodDescHandle(TargetPointer targetPointer) => throw new NotImplementedException(); + public virtual TargetPointer GetMethodTable(MethodDescHandle methodDesc) => throw new NotImplementedException(); #endregion MethodDesc inspection APIs } diff --git a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs index 997a0b5a71ef4..b37b5d002121c 100644 --- a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs +++ b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Diagnostics.CodeAnalysis; namespace Microsoft.Diagnostics.DataContractReader.Contracts; @@ -88,10 +89,10 @@ internal EEClass(Target target, TargetPointer eeClassPointer) internal struct MethodDesc { - public readonly Target _target; - public TargetPointer Address { get; init; } - public readonly Data.MethodDesc _desc; - public readonly Data.MethodDescChunk _chunk; + private readonly Target _target; + private TargetPointer Address { get; init; } + private readonly Data.MethodDesc _desc; + private readonly Data.MethodDescChunk _chunk; internal MethodDesc(Target target, TargetPointer methodDescPointer, Data.MethodDesc desc, Data.MethodDescChunk chunk) { _target = target; @@ -228,25 +229,27 @@ private TargetPointer GetMethodDescChunkPointerMayThrow(TargetPointer methodDesc return new TargetPointer(chunkAddress); } - private Data.MethodDescChunk GetMethodDescChunkMayThrow(TargetPointer methodDescPointer, Data.MethodDesc md) + private Data.MethodDescChunk GetMethodDescChunkMayThrow(TargetPointer methodDescPointer, Data.MethodDesc md, out TargetPointer methodDescChunkPointer) { - return new Data.MethodDescChunk(_target, GetMethodDescChunkPointerMayThrow(methodDescPointer, md)); + methodDescChunkPointer = GetMethodDescChunkPointerMayThrow(methodDescPointer, md); + return new Data.MethodDescChunk(_target, methodDescChunkPointer); } - private NonValidated.MethodDesc GetMethodDescMayThrow(TargetPointer methodDescPointer) + private NonValidated.MethodDesc GetMethodDescMayThrow(TargetPointer methodDescPointer, out TargetPointer methodDescChunkPointer) { // may throw if the method desc at methodDescPointer is corrupted // we bypass the target data cache here because we don't want to cache non-validated data Data.MethodDesc desc = new Data.MethodDesc(_target, methodDescPointer); - Data.MethodDescChunk chunk = GetMethodDescChunkMayThrow(methodDescPointer, desc); + Data.MethodDescChunk chunk = GetMethodDescChunkMayThrow(methodDescPointer, desc, out methodDescChunkPointer); return new NonValidated.MethodDesc(_target, methodDescPointer, desc, chunk); } - private bool ValidateMethodDescPointer(TargetPointer methodDescPointer) + private bool ValidateMethodDescPointer(TargetPointer methodDescPointer, [NotNullWhen(true)] out TargetPointer methodDescChunkPointer) { + methodDescChunkPointer = TargetPointer.Null; try { - NonValidated.MethodDesc umd = GetMethodDescMayThrow(methodDescPointer); + NonValidated.MethodDesc umd = GetMethodDescMayThrow(methodDescPointer, out methodDescChunkPointer); TargetPointer methodTablePointer = umd.MethodTable; if (methodTablePointer == TargetPointer.Null || methodTablePointer == new TargetPointer(0xffffffff_fffffffful) @@ -261,6 +264,8 @@ private bool ValidateMethodDescPointer(TargetPointer methodDescPointer) return false; } // TODO: request.cpp + // TODO[cdac]: this needs a Precode lookup + // see MethodDescChunk::GetTemporaryEntryPoint #if false MethodDesc *pMDCheck = MethodDesc::GetMethodDescFromStubAddr(pMD->GetTemporaryEntryPoint(), TRUE); @@ -268,8 +273,11 @@ private bool ValidateMethodDescPointer(TargetPointer methodDescPointer) { retval = FALSE; } - } +#endif + // TODO: request.cpp + // TODO[cdac]: needs MethodDesc::GetNativeCode and MethodDesc::GetMethodEntryPoint() +#if false if (retval && pMD->HasNativeCode() && !pMD->IsFCall()) { PCODE jitCodeAddr = pMD->GetNativeCode(); diff --git a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs index 0367e972fbec1..afbbe91fdfec6 100644 --- a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs +++ b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs @@ -80,7 +80,18 @@ internal enum MethodDescFlags : ushort internal struct MethodDesc { + private readonly Data.MethodDesc _desc; + private readonly Data.MethodDescChunk _chunk; + internal TargetPointer Address { get; init; } + internal MethodDesc(TargetPointer methodDescPointer, Data.MethodDesc desc, Data.MethodDescChunk chunk) + { + _desc = desc; + _chunk = chunk; + Address = methodDescPointer; + } + public TargetPointer MethodTable => _chunk.MethodTable; + public ushort Slot => _desc.Slot; } internal RuntimeTypeSystem_1(Target target, TargetPointer freeObjectMethodTablePointer, ulong methodDescAlignment) @@ -452,27 +463,36 @@ public MethodDescHandle GetMethodDescHandle(TargetPointer methodDescPointer) { return new MethodDescHandle(methodDescPointer); } -#if false // TODO // Check if we cached the underlying data already - if (_target.ProcessedData.TryGet(methodTablePointer, out Data.MethodTable? methodTableData)) + if (_target.ProcessedData.TryGet(methodDescPointer, out Data.MethodDesc? methodDescData)) { // we already cached the data, we must have validated the address, create the representation struct for our use - MethodTable trustedMethodTable = new MethodTable(methodTableData); - _ = _methodTables.TryAdd(methodTablePointer, trustedMethodTable); - return new MethodTableHandle(methodTablePointer); + TargetPointer mdescChunkPtr = GetMethodDescChunkPointerMayThrow(methodDescPointer, methodDescData); + // FIXME[cdac]: this isn't threadsafe + if (!_target.ProcessedData.TryGet(mdescChunkPtr, out Data.MethodDescChunk? methodDescChunkData)) + { + throw new InvalidOperationException("cached MethodDesc data but not its containing MethodDescChunk"); + } + MethodDesc validatedMethodDesc = new MethodDesc(methodDescPointer, methodDescData, methodDescChunkData); + _ = _methodDescs.TryAdd(methodDescPointer, validatedMethodDesc); + return new MethodDescHandle(methodDescPointer); } -#endif - if (!ValidateMethodDescPointer(methodDescPointer)) + if (!ValidateMethodDescPointer(methodDescPointer, out TargetPointer methodDescChunkPointer)) { throw new InvalidOperationException("Invalid method desc pointer"); } - // ok, we validated it, cache the data and add the MethodTable_1 struct to the dictionary -#if false // TODO - Data.MethodDesc trustedMethodDescData = _target.ProcessedData.GetOrAdd(methodDescPointer); -#endif - MethodDesc trustedMethodDescF = default; // new MethodDesc(trustedMethodTableData); // TODO - _ = _methodDescs.TryAdd(methodDescPointer, trustedMethodDescF); - return new MethodDescHandle(methodDescPointer); + else + { + // ok, we validated it, cache the data and add the MethodTable_1 struct to the dictionary + Data.MethodDescChunk validatedMethodDescChunkData = _target.ProcessedData.GetOrAdd(methodDescChunkPointer); + Data.MethodDesc validatedMethodDescData = _target.ProcessedData.GetOrAdd(methodDescPointer); + + MethodDesc trustedMethodDescF = new MethodDesc(methodDescPointer, validatedMethodDescData, validatedMethodDescChunkData); + _ = _methodDescs.TryAdd(methodDescPointer, trustedMethodDescF); + return new MethodDescHandle(methodDescPointer); + } } + + public TargetPointer GetMethodTable(MethodDescHandle methodDescHandle) => _methodDescs[methodDescHandle.Address].MethodTable; } diff --git a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs index 02a99ae0c9b03..111ac0e8bed82 100644 --- a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs @@ -100,6 +100,8 @@ public unsafe int GetMethodDescData(ulong methodDesc, ulong ip, DacpMethodDescDa Contracts.IRuntimeTypeSystem rtsContract = _target.Contracts.RuntimeTypeSystem; Contracts.MethodDescHandle methodDescHandle = rtsContract.GetMethodDescHandle(methodDesc); + data->MethodTablePtr = rtsContract.GetMethodTable(methodDescHandle); + return HResults.E_NOTIMPL; } catch (Exception ex) From fa42ef089fb09b198ffd86b2a6579bc47bbf1460 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 12 Jul 2024 13:16:19 -0400 Subject: [PATCH 07/24] update contract --- .../design/datacontracts/RuntimeTypeSystem.md | 25 +++++++++++++++++++ .../src/Contracts/RuntimeTypeSystem_1.cs | 2 +- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/docs/design/datacontracts/RuntimeTypeSystem.md b/docs/design/datacontracts/RuntimeTypeSystem.md index dda33dc1f9d5e..f86bfe99bb1e9 100644 --- a/docs/design/datacontracts/RuntimeTypeSystem.md +++ b/docs/design/datacontracts/RuntimeTypeSystem.md @@ -29,6 +29,8 @@ internal enum CorElementType A `TypeHandle` is the runtime representation of the type information about a value. This can be constructed from the address of a `TypeHandle` or a `MethodTable`. ``` csharp +partial interface IRuntimeTypeSystem : IContract +{ #region TypeHandle inspection APIs public virtual TypeHandle GetTypeHandle(TargetPointer targetPointer); @@ -74,12 +76,33 @@ A `TypeHandle` is the runtime representation of the type information about a val public virtual bool IsFunctionPointer(TypeHandle typeHandle, out ReadOnlySpan retAndArgTypes, out byte callConv); #endregion TypeHandle inspection APIs +} ``` ### MethodDesc A `MethodDesc` is the runtime representationn of a managed method (either from IL, from reflection emit, or generated by the runtime). +```csharp +struct MethodDescHandle +{ + // no public properties or constructors + + internal TargetPointer Address { get; } +} +``` + +```csharp +partial interface IRuntimeTypeSystem : IContract +{ + #region MethodDesc inspection APIs + public virtual MethodDescHandle GetMethodDescHandle (TargetPointer methodDescPointer); + + public virtual TargetPointer GetMethodTable (MethodDescHandle methodDesc); + #endregion MethodDesc inspection APIs +} +``` + ## Version 1 ### TypeHandle @@ -537,3 +560,5 @@ The version 1 `MethodDesc` APIs depend on the `MethodDescAlignment` global and t In the runtime a `MethodDesc` implicitly belongs to a single `MethodDescChunk` and some common data is shared between method descriptors that belong to the same chunk. A single method table will typically have multiple chunks. There are subkinds of MethodDescs at runtime of varying sizes (but the sizes must be mutliples of `MethodDescAlignment`) and each chunk contains method descriptors of the same size. + +**TODO(cdac)** diff --git a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs index afbbe91fdfec6..3d86923290eee 100644 --- a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs +++ b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs @@ -48,7 +48,7 @@ internal MethodTable(Data.MethodTable data) PerInstInfo = data.PerInstInfo; } - // this MethodTable is a canonical MethodTable if it's EEClassOrCanonMT is an EEClass + // this MethodTable is a canonical MethodTable if its EEClassOrCanonMT is an EEClass internal bool IsCanonMT => GetEEClassOrCanonMTBits(EEClassOrCanonMT) == EEClassOrCanonMTBits.EEClass; } From 27f4a968f60433c8427479b3bb66377489dbbc18 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 12 Jul 2024 15:57:42 -0400 Subject: [PATCH 08/24] fix RuntimeTypeSystem unit tests mock the additional data and globals --- .../managed/cdacreader/tests/MethodTableTests.cs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/native/managed/cdacreader/tests/MethodTableTests.cs b/src/native/managed/cdacreader/tests/MethodTableTests.cs index 1e2c2cf53d9fa..66ff051022885 100644 --- a/src/native/managed/cdacreader/tests/MethodTableTests.cs +++ b/src/native/managed/cdacreader/tests/MethodTableTests.cs @@ -35,6 +35,7 @@ public unsafe class MethodTableTests { nameof (Data.EEClass.CorTypeAttr), new () { Offset = 16, Type = DataType.uint32}}, { nameof (Data.EEClass.NumMethods), new () { Offset = 20, Type = DataType.uint16}}, { nameof (Data.EEClass.InternalCorElementType), new () { Offset = 22, Type = DataType.uint8}}, + { nameof (Data.EEClass.NumNonVirtualSlots), new () { Offset = 24, Type = DataType.uint16}}, } }; @@ -48,6 +49,7 @@ private static readonly (DataType Type, Target.TypeInfo Info)[] RTSTypes = private static readonly (string Name, ulong Value, string? Type)[] RTSGlobals = [ (nameof(Constants.Globals.FreeObjectMethodTable), TestFreeObjectMethodTableGlobalAddress, null), + (nameof(Constants.Globals.MethodDescAlignment), 8, nameof(DataType.uint64)), ]; private static MockMemorySpace.Builder AddFreeObjectMethodTable(TargetTestHelpers targetTestHelpers, MockMemorySpace.Builder builder) @@ -60,13 +62,14 @@ private static MockMemorySpace.Builder AddFreeObjectMethodTable(TargetTestHelper ]); } - private static MockMemorySpace.Builder AddEEClass(TargetTestHelpers targetTestHelpers, MockMemorySpace.Builder builder, TargetPointer eeClassPtr, string name, TargetPointer canonMTPtr, uint attr, ushort numMethods) + private static MockMemorySpace.Builder AddEEClass(TargetTestHelpers targetTestHelpers, MockMemorySpace.Builder builder, TargetPointer eeClassPtr, string name, TargetPointer canonMTPtr, uint attr, ushort numMethods, ushort numNonVirtualSlots) { MockMemorySpace.HeapFragment eeClassFragment = new() { Name = $"EEClass '{name}'", Address = eeClassPtr, Data = new byte[targetTestHelpers.SizeOfTypeInfo(EEClassTypeInfo)] }; Span dest = eeClassFragment.Data; targetTestHelpers.WritePointer(dest.Slice(EEClassTypeInfo.Fields[nameof(Data.EEClass.MethodTable)].Offset), canonMTPtr); targetTestHelpers.Write(dest.Slice(EEClassTypeInfo.Fields[nameof(Data.EEClass.CorTypeAttr)].Offset), attr); targetTestHelpers.Write(dest.Slice(EEClassTypeInfo.Fields[nameof(Data.EEClass.NumMethods)].Offset), numMethods); + targetTestHelpers.Write(dest.Slice(EEClassTypeInfo.Fields[nameof(Data.EEClass.NumNonVirtualSlots)].Offset), numNonVirtualSlots); return builder.AddHeapFragment(eeClassFragment); } @@ -163,7 +166,7 @@ private static MockMemorySpace.Builder AddSystemObject(TargetTestHelpers targetT System.Reflection.TypeAttributes typeAttributes = System.Reflection.TypeAttributes.Public | System.Reflection.TypeAttributes.Class; const int numMethods = 8; // System.Object has 8 methods const int numVirtuals = 3; // System.Object has 3 virtual methods - builder = AddEEClass(targetTestHelpers, builder, systemObjectEEClassPtr, "System.Object", systemObjectMethodTablePtr, attr: (uint)typeAttributes, numMethods: numMethods); + builder = AddEEClass(targetTestHelpers, builder, systemObjectEEClassPtr, "System.Object", systemObjectMethodTablePtr, attr: (uint)typeAttributes, numMethods: numMethods, numNonVirtualSlots: 0); builder = AddMethodTable(targetTestHelpers, builder, systemObjectMethodTablePtr, "System.Object", systemObjectEEClassPtr, mtflags: default, mtflags2: default, baseSize: targetTestHelpers.ObjectBaseSize, module: TargetPointer.Null, parentMethodTable: TargetPointer.Null, numInterfaces: 0, numVirtuals: numVirtuals); @@ -218,7 +221,7 @@ public void ValidateSystemStringMethodTable(MockTarget.Architecture arch) const int numInterfaces = 8; // Arbitrary const int numVirtuals = 3; // at least as many as System.Object uint mtflags = (uint)RuntimeTypeSystem_1.WFLAGS_HIGH.HasComponentSize | /*componentSize: */2; - builder = AddEEClass(targetTestHelpers, builder, systemStringEEClassPtr, "System.String", systemStringMethodTablePtr, attr: (uint)typeAttributes, numMethods: numMethods); + builder = AddEEClass(targetTestHelpers, builder, systemStringEEClassPtr, "System.String", systemStringMethodTablePtr, attr: (uint)typeAttributes, numMethods: numMethods, numNonVirtualSlots: 0); builder = AddMethodTable(targetTestHelpers, builder, systemStringMethodTablePtr, "System.String", systemStringEEClassPtr, mtflags: mtflags, mtflags2: default, baseSize: targetTestHelpers.StringBaseSize, module: TargetPointer.Null, parentMethodTable: systemObjectMethodTablePtr, numInterfaces: numInterfaces, numVirtuals: numVirtuals); @@ -293,7 +296,7 @@ public void ValidateGenericInstMethodTable(MockTarget.Architecture arch) const int numInterfaces = 0; const int numVirtuals = 3; const uint gtd_mtflags = 0x00000030; // TODO: GenericsMask_TypicalInst - builder = AddEEClass(targetTestHelpers, builder, genericDefinitionEEClassPtr, "EEClass GenericDefinition", genericDefinitionMethodTablePtr, attr: (uint)typeAttributes, numMethods: numMethods); + builder = AddEEClass(targetTestHelpers, builder, genericDefinitionEEClassPtr, "EEClass GenericDefinition", genericDefinitionMethodTablePtr, attr: (uint)typeAttributes, numMethods: numMethods, numNonVirtualSlots: 0); builder = AddMethodTable(targetTestHelpers, builder, genericDefinitionMethodTablePtr, "MethodTable GenericDefinition", genericDefinitionEEClassPtr, mtflags: gtd_mtflags, mtflags2: default, baseSize: targetTestHelpers.ObjectBaseSize, module: TargetPointer.Null, parentMethodTable: systemObjectMethodTablePtr, numInterfaces: numInterfaces, numVirtuals: numVirtuals); @@ -348,7 +351,7 @@ public void ValidateArrayInstMethodTable(MockTarget.Architecture arch) const ushort systemArrayNumMethods = 37; // Arbitrary. Not trying to exactly match the real System.Array const uint systemArrayCorTypeAttr = (uint)(System.Reflection.TypeAttributes.Public | System.Reflection.TypeAttributes.Class); - builder = AddEEClass(targetTestHelpers, builder, systemArrayEEClassPtr, "EEClass System.Array", systemArrayMethodTablePtr, attr: systemArrayCorTypeAttr, numMethods: systemArrayNumMethods); + builder = AddEEClass(targetTestHelpers, builder, systemArrayEEClassPtr, "EEClass System.Array", systemArrayMethodTablePtr, attr: systemArrayCorTypeAttr, numMethods: systemArrayNumMethods, numNonVirtualSlots: 0); builder = AddMethodTable(targetTestHelpers, builder, systemArrayMethodTablePtr, "MethodTable System.Array", systemArrayEEClassPtr, mtflags: default, mtflags2: default, baseSize: targetTestHelpers.ObjectBaseSize, module: TargetPointer.Null, parentMethodTable: systemObjectMethodTablePtr, numInterfaces: systemArrayNumInterfaces, numVirtuals: 3); @@ -356,7 +359,7 @@ public void ValidateArrayInstMethodTable(MockTarget.Architecture arch) const uint arrayInst_mtflags = (uint)(RuntimeTypeSystem_1.WFLAGS_HIGH.HasComponentSize | RuntimeTypeSystem_1.WFLAGS_HIGH.Category_Array) | arrayInstanceComponentSize; const uint arrayInstCorTypeAttr = (uint)(System.Reflection.TypeAttributes.Public | System.Reflection.TypeAttributes.Class | System.Reflection.TypeAttributes.Sealed); - builder = AddEEClass(targetTestHelpers, builder, arrayInstanceEEClassPtr, "EEClass ArrayInstance", arrayInstanceMethodTablePtr, attr: arrayInstCorTypeAttr, numMethods: systemArrayNumMethods); + builder = AddEEClass(targetTestHelpers, builder, arrayInstanceEEClassPtr, "EEClass ArrayInstance", arrayInstanceMethodTablePtr, attr: arrayInstCorTypeAttr, numMethods: systemArrayNumMethods, numNonVirtualSlots: 0); builder = AddMethodTable(targetTestHelpers, builder, arrayInstanceMethodTablePtr, "MethodTable ArrayInstance", arrayInstanceEEClassPtr, mtflags: arrayInst_mtflags, mtflags2: default, baseSize: targetTestHelpers.ObjectBaseSize, module: TargetPointer.Null, parentMethodTable: systemArrayMethodTablePtr, numInterfaces: systemArrayNumInterfaces, numVirtuals: 3); From 9dbd432bd6af08850ff67796283e5f3860622ea9 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 15 Jul 2024 13:56:42 -0400 Subject: [PATCH 09/24] update contract --- src/coreclr/debug/runtimeinfo/datadescriptor.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/debug/runtimeinfo/datadescriptor.h b/src/coreclr/debug/runtimeinfo/datadescriptor.h index 09c269cf0e138..a8fd2dcbc69de 100644 --- a/src/coreclr/debug/runtimeinfo/datadescriptor.h +++ b/src/coreclr/debug/runtimeinfo/datadescriptor.h @@ -191,7 +191,7 @@ CDAC_TYPE_FIELD(Module, /*pointer*/, TypeDefToMethodTableMap, cdac_offsets::TypeRefToMethodTableMap) CDAC_TYPE_END(Module) -// Metadata +// RuntimeTypeSystem CDAC_TYPE_BEGIN(MethodTable) CDAC_TYPE_INDETERMINATE(MethodTable) From 793681fd5ee6ea9828253c38ad7371697870c356 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 15 Jul 2024 15:57:45 -0400 Subject: [PATCH 10/24] fix GetMethodDescChunkPointerMayThrow --- .../src/Contracts/RuntimeTypeSystem_1.NonValidated.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs index b37b5d002121c..dcc6eec8e068d 100644 --- a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs +++ b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs @@ -220,12 +220,12 @@ private TargetPointer GetClassThrowing(NonValidated.MethodTable methodTable) private TargetPointer GetMethodDescChunkPointerMayThrow(TargetPointer methodDescPointer, Data.MethodDesc umd) { - ulong? methodDescSize = _target.GetTypeInfo(DataType.MethodDesc).Size; - if (!methodDescSize.HasValue) + ulong? methodDescChunkSize = _target.GetTypeInfo(DataType.MethodDescChunk).Size; + if (!methodDescChunkSize.HasValue) { - throw new InvalidOperationException("Target has no definite MethodDesc size"); + throw new InvalidOperationException("Target has no definite MethodDescChunk size"); } - ulong chunkAddress = (ulong)methodDescPointer - methodDescSize.Value - umd.ChunkIndex * MethodDescAlignment; + ulong chunkAddress = (ulong)methodDescPointer - methodDescChunkSize.Value - umd.ChunkIndex * MethodDescAlignment; return new TargetPointer(chunkAddress); } From a46f91ee8dc766ed4e6e6fa399c9ffffcdb9b2ca Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 16 Jul 2024 16:38:44 -0400 Subject: [PATCH 11/24] fixup rebase --- .../src/Contracts/RuntimeTypeSystem_1.NonValidated.cs | 4 ++-- .../managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs | 5 +++-- src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs index dcc6eec8e068d..15eca7b396cd1 100644 --- a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs +++ b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs @@ -257,9 +257,9 @@ private bool ValidateMethodDescPointer(TargetPointer methodDescPointer, [NotNull { return false; } - MethodTableHandle methodTableHandle = GetMethodTableHandle(methodTablePointer); + TypeHandle typeHandle = GetTypeHandle(methodTablePointer); - if (umd.Slot >= GetNumVtableSlots(methodTableHandle) && !umd.HasNonVtableSlot) + if (umd.Slot >= GetNumVtableSlots(typeHandle) && !umd.HasNonVtableSlot) { return false; } diff --git a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs index 3d86923290eee..0ffad344b134d 100644 --- a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs +++ b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs @@ -446,12 +446,13 @@ private FunctionPointerRetAndArgs(Target target, TargetPointer typePointer) TypeHandles[i] = rts.GetTypeHandle(target.ReadPointer(retAndArgs + (ulong)target.PointerSize * (ulong)i)); } } + } - internal ushort GetNumVtableSlots(TypeHandleHandle typeHandle) + internal ushort GetNumVtableSlots(TypeHandle typeHandle) { if (!typeHandle.IsMethodTable()) return 0; - MethodTable methodTable = _methodTables[typeHandleHandle.Address]; + MethodTable methodTable = _methodTables[typeHandle.Address]; ushort numNonVirtualSlots = methodTable.IsCanonMT ? GetClassData(typeHandle).NumNonVirtualSlots : (ushort)0; return checked((ushort)(methodTable.NumVirtuals + numNonVirtualSlots)); } diff --git a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs index 111ac0e8bed82..a28f2d062e21d 100644 --- a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs @@ -104,7 +104,7 @@ public unsafe int GetMethodDescData(ulong methodDesc, ulong ip, DacpMethodDescDa return HResults.E_NOTIMPL; } - catch (Exception ex) + catch (global::System.Exception ex) { return ex.HResult; } From 3f82a8910df57758a2efd7665056fb9a413f7f01 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 18 Jul 2024 16:43:39 -0400 Subject: [PATCH 12/24] add data descriptor description to the contract --- docs/design/datacontracts/RuntimeTypeSystem.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/design/datacontracts/RuntimeTypeSystem.md b/docs/design/datacontracts/RuntimeTypeSystem.md index f86bfe99bb1e9..fcdc3ee607670 100644 --- a/docs/design/datacontracts/RuntimeTypeSystem.md +++ b/docs/design/datacontracts/RuntimeTypeSystem.md @@ -281,6 +281,7 @@ The contract additionally depends on these data descriptors | `EEClass` | `InternalCorElementType` | An InternalCorElementType uses the enum values of a CorElementType to indicate some of the information about the type of the type which uses the EEClass In particular, all reference types are CorElementType.Class, Enums are the element type of their underlying type and ValueTypes which can exactly be represented as an element type are represented as such, all other values types are represented as CorElementType.ValueType. | | `EEClass` | `MethodTable` | Pointer to the canonical MethodTable of this type | | `EEClass` | `NumMethods` | Count of methods attached to the EEClass | +| `EEClass` | `NumNonVirtualSlots` | Count of non-virtual slots for the EEClass | | `EEClass` | `CorTypeAttr` | Various flags | | `ArrayClass` | `Rank` | Rank of the associated array MethodTable | | `TypeDesc` | `TypeAndFlags` | The lower 8 bits are the CorElementType of the `TypeDesc`, the upper 24 bits are reserved for flags | @@ -561,4 +562,15 @@ The version 1 `MethodDesc` APIs depend on the `MethodDescAlignment` global and t In the runtime a `MethodDesc` implicitly belongs to a single `MethodDescChunk` and some common data is shared between method descriptors that belong to the same chunk. A single method table will typically have multiple chunks. There are subkinds of MethodDescs at runtime of varying sizes (but the sizes must be mutliples of `MethodDescAlignment`) and each chunk contains method descriptors of the same size. +We depend on the following data descriptors: +| Data Descriptor Name | Field | Meaning | +| --- | --- | --- | +| `MethodDesc` | `ChunkIndex` | Offset of this `MethodDesc` relative to the end of its containing `MethodDescChunk` - in multiples of `MethodDescAlignment` +| `MethodDesc` | `Slot` | The method's slot +| `MethodDesc` | `Flags` | The method's flags +| `MethodDescChunk` | `MethodTable` | The method table set of methods belongs to +| `MethodDescChunk` | `Next` | The next chunk of methods +| `MethodDescChunk` | `Size` | The size - 1 of this `MethodDescChunk` following this `MethodDescChunk` header. In multiples of `MethodDescAlignment` +| `MethodDescChunk` | `Count` | The number of `MethodDesc` entries in this chunk, minus 1. + **TODO(cdac)** From 8111d53a885e0379da2fbf6e79613364b1fbdac8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksey=20Kliger=20=28=CE=BBgeek=29?= Date: Fri, 19 Jul 2024 11:34:00 -0400 Subject: [PATCH 13/24] Apply suggestions from code review Co-authored-by: Elinor Fung --- docs/design/datacontracts/RuntimeTypeSystem.md | 4 +--- src/coreclr/debug/runtimeinfo/datadescriptor.h | 2 +- .../src/Contracts/RuntimeTypeSystem_1.cs | 16 +++++++--------- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/docs/design/datacontracts/RuntimeTypeSystem.md b/docs/design/datacontracts/RuntimeTypeSystem.md index fcdc3ee607670..b4994cc6b7a4d 100644 --- a/docs/design/datacontracts/RuntimeTypeSystem.md +++ b/docs/design/datacontracts/RuntimeTypeSystem.md @@ -95,11 +95,9 @@ struct MethodDescHandle ```csharp partial interface IRuntimeTypeSystem : IContract { - #region MethodDesc inspection APIs public virtual MethodDescHandle GetMethodDescHandle (TargetPointer methodDescPointer); public virtual TargetPointer GetMethodTable (MethodDescHandle methodDesc); - #endregion MethodDesc inspection APIs } ``` @@ -570,7 +568,7 @@ We depend on the following data descriptors: | `MethodDesc` | `Flags` | The method's flags | `MethodDescChunk` | `MethodTable` | The method table set of methods belongs to | `MethodDescChunk` | `Next` | The next chunk of methods -| `MethodDescChunk` | `Size` | The size - 1 of this `MethodDescChunk` following this `MethodDescChunk` header. In multiples of `MethodDescAlignment` +| `MethodDescChunk` | `Size` | The size of this `MethodDescChunk` following this `MethodDescChunk` header, minus 1. In multiples of `MethodDescAlignment` | `MethodDescChunk` | `Count` | The number of `MethodDesc` entries in this chunk, minus 1. **TODO(cdac)** diff --git a/src/coreclr/debug/runtimeinfo/datadescriptor.h b/src/coreclr/debug/runtimeinfo/datadescriptor.h index a8fd2dcbc69de..f077de088d017 100644 --- a/src/coreclr/debug/runtimeinfo/datadescriptor.h +++ b/src/coreclr/debug/runtimeinfo/datadescriptor.h @@ -256,7 +256,7 @@ CDAC_TYPE_END(DynamicMetadata) CDAC_TYPE_BEGIN(MethodDesc) CDAC_TYPE_INDETERMINATE(MethodDesc) CDAC_TYPE_FIELD(MethodDesc, /*uint8*/, ChunkIndex, cdac_offsets::ChunkIndex) -CDAC_TYPE_FIELD(MethodDesc, /*uint8*/, Slot, cdac_offsets::Slot) +CDAC_TYPE_FIELD(MethodDesc, /*uint16*/, Slot, cdac_offsets::Slot) CDAC_TYPE_FIELD(MethodDesc, /*uint16*/, Flags, cdac_offsets::Flags) CDAC_TYPE_END(MethodDesc) diff --git a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs index 0ffad344b134d..7b916dd0e7f76 100644 --- a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs +++ b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs @@ -483,16 +483,14 @@ public MethodDescHandle GetMethodDescHandle(TargetPointer methodDescPointer) { throw new InvalidOperationException("Invalid method desc pointer"); } - else - { - // ok, we validated it, cache the data and add the MethodTable_1 struct to the dictionary - Data.MethodDescChunk validatedMethodDescChunkData = _target.ProcessedData.GetOrAdd(methodDescChunkPointer); - Data.MethodDesc validatedMethodDescData = _target.ProcessedData.GetOrAdd(methodDescPointer); - MethodDesc trustedMethodDescF = new MethodDesc(methodDescPointer, validatedMethodDescData, validatedMethodDescChunkData); - _ = _methodDescs.TryAdd(methodDescPointer, trustedMethodDescF); - return new MethodDescHandle(methodDescPointer); - } + // ok, we validated it, cache the data and add the MethodDesc struct to the dictionary + Data.MethodDescChunk validatedMethodDescChunkData = _target.ProcessedData.GetOrAdd(methodDescChunkPointer); + Data.MethodDesc validatedMethodDescData = _target.ProcessedData.GetOrAdd(methodDescPointer); + + MethodDesc trustedMethodDescF = new MethodDesc(methodDescPointer, validatedMethodDescData, validatedMethodDescChunkData); + _ = _methodDescs.TryAdd(methodDescPointer, trustedMethodDescF); + return new MethodDescHandle(methodDescPointer); } public TargetPointer GetMethodTable(MethodDescHandle methodDescHandle) => _methodDescs[methodDescHandle.Address].MethodTable; From 07c810e9d285f477e8fa4670c187dc43e9972402 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 19 Jul 2024 11:47:13 -0400 Subject: [PATCH 14/24] MayThrow -> Throwing --- .../Contracts/RuntimeTypeSystem_1.NonValidated.cs | 12 ++++++------ .../cdacreader/src/Contracts/RuntimeTypeSystem_1.cs | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs index 15eca7b396cd1..339577501d34d 100644 --- a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs +++ b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs @@ -218,7 +218,7 @@ private TargetPointer GetClassThrowing(NonValidated.MethodTable methodTable) } } - private TargetPointer GetMethodDescChunkPointerMayThrow(TargetPointer methodDescPointer, Data.MethodDesc umd) + private TargetPointer GetMethodDescChunkPointerThrowing(TargetPointer methodDescPointer, Data.MethodDesc umd) { ulong? methodDescChunkSize = _target.GetTypeInfo(DataType.MethodDescChunk).Size; if (!methodDescChunkSize.HasValue) @@ -229,18 +229,18 @@ private TargetPointer GetMethodDescChunkPointerMayThrow(TargetPointer methodDesc return new TargetPointer(chunkAddress); } - private Data.MethodDescChunk GetMethodDescChunkMayThrow(TargetPointer methodDescPointer, Data.MethodDesc md, out TargetPointer methodDescChunkPointer) + private Data.MethodDescChunk GetMethodDescChunkThrowing(TargetPointer methodDescPointer, Data.MethodDesc md, out TargetPointer methodDescChunkPointer) { - methodDescChunkPointer = GetMethodDescChunkPointerMayThrow(methodDescPointer, md); + methodDescChunkPointer = GetMethodDescChunkPointerThrowing(methodDescPointer, md); return new Data.MethodDescChunk(_target, methodDescChunkPointer); } - private NonValidated.MethodDesc GetMethodDescMayThrow(TargetPointer methodDescPointer, out TargetPointer methodDescChunkPointer) + private NonValidated.MethodDesc GetMethodDescThrowing(TargetPointer methodDescPointer, out TargetPointer methodDescChunkPointer) { // may throw if the method desc at methodDescPointer is corrupted // we bypass the target data cache here because we don't want to cache non-validated data Data.MethodDesc desc = new Data.MethodDesc(_target, methodDescPointer); - Data.MethodDescChunk chunk = GetMethodDescChunkMayThrow(methodDescPointer, desc, out methodDescChunkPointer); + Data.MethodDescChunk chunk = GetMethodDescChunkThrowing(methodDescPointer, desc, out methodDescChunkPointer); return new NonValidated.MethodDesc(_target, methodDescPointer, desc, chunk); } @@ -249,7 +249,7 @@ private bool ValidateMethodDescPointer(TargetPointer methodDescPointer, [NotNull methodDescChunkPointer = TargetPointer.Null; try { - NonValidated.MethodDesc umd = GetMethodDescMayThrow(methodDescPointer, out methodDescChunkPointer); + NonValidated.MethodDesc umd = GetMethodDescThrowing(methodDescPointer, out methodDescChunkPointer); TargetPointer methodTablePointer = umd.MethodTable; if (methodTablePointer == TargetPointer.Null || methodTablePointer == new TargetPointer(0xffffffff_fffffffful) diff --git a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs index 7b916dd0e7f76..f81243ef35dff 100644 --- a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs +++ b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs @@ -468,7 +468,7 @@ public MethodDescHandle GetMethodDescHandle(TargetPointer methodDescPointer) if (_target.ProcessedData.TryGet(methodDescPointer, out Data.MethodDesc? methodDescData)) { // we already cached the data, we must have validated the address, create the representation struct for our use - TargetPointer mdescChunkPtr = GetMethodDescChunkPointerMayThrow(methodDescPointer, methodDescData); + TargetPointer mdescChunkPtr = GetMethodDescChunkPointerThrowing(methodDescPointer, methodDescData); // FIXME[cdac]: this isn't threadsafe if (!_target.ProcessedData.TryGet(mdescChunkPtr, out Data.MethodDescChunk? methodDescChunkData)) { From 6f1d9a8f8046da824e62177c5a51fb7905d3479d Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 19 Jul 2024 11:47:21 -0400 Subject: [PATCH 15/24] Slot is ushort not byte --- src/native/managed/cdacreader/src/Data/MethodDesc.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/native/managed/cdacreader/src/Data/MethodDesc.cs b/src/native/managed/cdacreader/src/Data/MethodDesc.cs index edaa5ccaffd7f..7c4d6b9c8ebb3 100644 --- a/src/native/managed/cdacreader/src/Data/MethodDesc.cs +++ b/src/native/managed/cdacreader/src/Data/MethodDesc.cs @@ -13,11 +13,11 @@ public MethodDesc(Target target, TargetPointer address) Target.TypeInfo type = target.GetTypeInfo(DataType.MethodDesc); ChunkIndex = target.Read(address + (ulong)type.Fields[nameof(ChunkIndex)].Offset); - Slot = target.Read(address + (ulong)type.Fields[nameof(Slot)].Offset); + Slot = target.Read(address + (ulong)type.Fields[nameof(Slot)].Offset); Flags = target.Read(address + (ulong)type.Fields[nameof(Flags)].Offset); } public byte ChunkIndex { get; init; } - public byte Slot { get; init; } + public ushort Slot { get; init; } public ushort Flags { get; init; } } From fa426844c9388c1ed21a00c64964de8e2191d82b Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 19 Jul 2024 12:13:03 -0400 Subject: [PATCH 16/24] remove unused property --- .../src/Contracts/RuntimeTypeSystem_1.NonValidated.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs index 339577501d34d..33e57a23cf523 100644 --- a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs +++ b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs @@ -90,15 +90,13 @@ internal EEClass(Target target, TargetPointer eeClassPointer) internal struct MethodDesc { private readonly Target _target; - private TargetPointer Address { get; init; } private readonly Data.MethodDesc _desc; private readonly Data.MethodDescChunk _chunk; - internal MethodDesc(Target target, TargetPointer methodDescPointer, Data.MethodDesc desc, Data.MethodDescChunk chunk) + internal MethodDesc(Target target, Data.MethodDesc desc, Data.MethodDescChunk chunk) { _target = target; _desc = desc; _chunk = chunk; - Address = methodDescPointer; } private bool HasFlag(MethodDescFlags flag) => (_desc.Flags & (ushort)flag) != 0; @@ -225,6 +223,9 @@ private TargetPointer GetMethodDescChunkPointerThrowing(TargetPointer methodDesc { throw new InvalidOperationException("Target has no definite MethodDescChunk size"); } + // The runtime allocates a contiguous block of memory for a MethodDescChunk followedd by MethodDescAlignment * Size bytes of space + // that is filled with MethodDesc (or its subclasses) instances. Each MethodDesc has a ChunkIndex that indicates its + // offset from the end of the MethodDescChunk. ulong chunkAddress = (ulong)methodDescPointer - methodDescChunkSize.Value - umd.ChunkIndex * MethodDescAlignment; return new TargetPointer(chunkAddress); } @@ -241,7 +242,7 @@ private NonValidated.MethodDesc GetMethodDescThrowing(TargetPointer methodDescPo // we bypass the target data cache here because we don't want to cache non-validated data Data.MethodDesc desc = new Data.MethodDesc(_target, methodDescPointer); Data.MethodDescChunk chunk = GetMethodDescChunkThrowing(methodDescPointer, desc, out methodDescChunkPointer); - return new NonValidated.MethodDesc(_target, methodDescPointer, desc, chunk); + return new NonValidated.MethodDesc(_target, desc, chunk); } private bool ValidateMethodDescPointer(TargetPointer methodDescPointer, [NotNullWhen(true)] out TargetPointer methodDescChunkPointer) From 9613999722a9ace211fd94f2c16499a7f08bdbee Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 19 Jul 2024 12:14:44 -0400 Subject: [PATCH 17/24] add TargetPointer 32-/64-bit max constants --- .../src/Contracts/RuntimeTypeSystem_1.NonValidated.cs | 4 ++-- src/native/managed/cdacreader/src/Target.cs | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs index 33e57a23cf523..1753b6ea66b5b 100644 --- a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs +++ b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs @@ -253,8 +253,8 @@ private bool ValidateMethodDescPointer(TargetPointer methodDescPointer, [NotNull NonValidated.MethodDesc umd = GetMethodDescThrowing(methodDescPointer, out methodDescChunkPointer); TargetPointer methodTablePointer = umd.MethodTable; if (methodTablePointer == TargetPointer.Null - || methodTablePointer == new TargetPointer(0xffffffff_fffffffful) - || methodTablePointer == new TargetPointer(0x00000000_fffffffful)) + || methodTablePointer == TargetPointer.Max64Bit + || methodTablePointer == TargetPointer.Max32Bit) { return false; } diff --git a/src/native/managed/cdacreader/src/Target.cs b/src/native/managed/cdacreader/src/Target.cs index 244b917a508d8..b4ee6145800f0 100644 --- a/src/native/managed/cdacreader/src/Target.cs +++ b/src/native/managed/cdacreader/src/Target.cs @@ -14,6 +14,8 @@ namespace Microsoft.Diagnostics.DataContractReader; public readonly struct TargetPointer : IEquatable { public static TargetPointer Null = new(0); + public static TargetPointer Max32Bit = new(uint.MaxValue); + public static TargetPointer Max64Bit = new(ulong.MaxValue); public readonly ulong Value; public TargetPointer(ulong value) => Value = value; From 214ffe97ce9bcbe3f5d14d400619e6a62a91e5b5 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 19 Jul 2024 13:25:11 -0400 Subject: [PATCH 18/24] use NewArrayHolder --- src/coreclr/debug/daccess/request.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/debug/daccess/request.cpp b/src/coreclr/debug/daccess/request.cpp index 7062fdfab8db4..58c0f183b4f46 100644 --- a/src/coreclr/debug/daccess/request.cpp +++ b/src/coreclr/debug/daccess/request.cpp @@ -1054,8 +1054,8 @@ HRESULT ClrDataAccess::GetMethodDescData( { // Assert that the data is the same as what we get from the DAC. DacpMethodDescData mdDataLocal; - DacpReJitData *rgRevertedRejitDataLocal = NULL; - if (rgRevertedRejitData != NULL) + NewArrayHolder rgRevertedRejitDataLocal{}; + if (rgRevertedRejitData != nullptr) { rgRevertedRejitDataLocal = new DacpReJitData[cRevertedRejitVersions]; } From c6e057ad774023593f40f68b1699d0d1953f6d5f Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 19 Jul 2024 13:28:49 -0400 Subject: [PATCH 19/24] spelling --- docs/design/datacontracts/RuntimeTypeSystem.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/design/datacontracts/RuntimeTypeSystem.md b/docs/design/datacontracts/RuntimeTypeSystem.md index b4994cc6b7a4d..6a671b4dd3dd6 100644 --- a/docs/design/datacontracts/RuntimeTypeSystem.md +++ b/docs/design/datacontracts/RuntimeTypeSystem.md @@ -81,7 +81,7 @@ partial interface IRuntimeTypeSystem : IContract ### MethodDesc -A `MethodDesc` is the runtime representationn of a managed method (either from IL, from reflection emit, or generated by the runtime). +A `MethodDesc` is the runtime representation of a managed method (either from IL, from reflection emit, or generated by the runtime). ```csharp struct MethodDescHandle From b000f7ad506497f727327633fccec1a2a4db897e Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 19 Jul 2024 15:08:29 -0400 Subject: [PATCH 20/24] add globals to RTS contract --- docs/design/datacontracts/RuntimeTypeSystem.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/docs/design/datacontracts/RuntimeTypeSystem.md b/docs/design/datacontracts/RuntimeTypeSystem.md index 6a671b4dd3dd6..39a71ac3003ed 100644 --- a/docs/design/datacontracts/RuntimeTypeSystem.md +++ b/docs/design/datacontracts/RuntimeTypeSystem.md @@ -261,7 +261,11 @@ static class RuntimeTypeSystem_1_Helpers } ``` -The contract depends on the global pointer value `FreeObjectMethodTablePointer`. +The contract depends on the following globals + +| Global name | Meaning | +| --- | --- | +| `FreeObjectMethodTablePointer` | A pointer to the address of a `MethodTable` used by the GC to indicate reclaimed memory The contract additionally depends on these data descriptors @@ -557,6 +561,11 @@ The contract additionally depends on these data descriptors The version 1 `MethodDesc` APIs depend on the `MethodDescAlignment` global and the `MethodDesc` and `MethodDescChunk` data descriptors. +| Global name | Meaning | +| --- | --- | +| `MethodDescAlignment` | `MethodDescChunk` trailing data is allocated in multiples of this constant. The size (in bytes) of each `MethodDesc` (or subclass) instance is a multiple of this constant. + + In the runtime a `MethodDesc` implicitly belongs to a single `MethodDescChunk` and some common data is shared between method descriptors that belong to the same chunk. A single method table will typically have multiple chunks. There are subkinds of MethodDescs at runtime of varying sizes (but the sizes must be mutliples of `MethodDescAlignment`) and each chunk contains method descriptors of the same size. From d4f6299139b514f3cc42aae2da7e817b1668b44c Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 19 Jul 2024 15:28:34 -0400 Subject: [PATCH 21/24] remove unused usings --- .../managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs | 1 - src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs | 3 --- 2 files changed, 4 deletions(-) diff --git a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs index f81243ef35dff..813e067d68ee6 100644 --- a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs +++ b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs @@ -6,7 +6,6 @@ using System.Reflection.Metadata.Ecma335; using Microsoft.Diagnostics.DataContractReader.Data; using Microsoft.Diagnostics.DataContractReader.Contracts.RuntimeTypeSystem_1_NS; -using System.Security.Cryptography.X509Certificates; using System.Diagnostics; namespace Microsoft.Diagnostics.DataContractReader.Contracts; diff --git a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs index 3ff8f5f2594b0..779076f3efa62 100644 --- a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs @@ -3,9 +3,6 @@ using Microsoft.Diagnostics.DataContractReader.Contracts; using System; -using System.Collections.Generic; -using System.Diagnostics; -using System.Diagnostics.Contracts; using System.Runtime.InteropServices; using System.Runtime.InteropServices.Marshalling; From 10f09433f8eb01723985dd6eb8c6d264bf093805 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksey=20Kliger=20=28=CE=BBgeek=29?= Date: Fri, 19 Jul 2024 16:37:07 -0400 Subject: [PATCH 22/24] constexpr cdac_offsets, not const --- src/coreclr/vm/method.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index 2fa5125d1f373..320565b39d01c 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -1914,9 +1914,9 @@ class MethodDesc template<> struct cdac_offsets { - static const size_t ChunkIndex = offsetof(MethodDesc, m_chunkIndex); - static const size_t Slot = offsetof(MethodDesc, m_wSlotNumber); - static const size_t Flags = offsetof(MethodDesc, m_wFlags); + static constexpr size_t ChunkIndex = offsetof(MethodDesc, m_chunkIndex); + static constexpr size_t Slot = offsetof(MethodDesc, m_wSlotNumber); + static constexpr size_t Flags = offsetof(MethodDesc, m_wFlags); }; #ifndef DACCESS_COMPILE From 515a3e33387945428e82b4d2cc06979b5f2fdf91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksey=20Kliger=20=28=CE=BBgeek=29?= Date: Sat, 20 Jul 2024 09:36:34 -0400 Subject: [PATCH 23/24] Apply suggestions from code review Co-authored-by: Elinor Fung --- docs/design/datacontracts/RuntimeTypeSystem.md | 4 ++-- .../src/Contracts/RuntimeTypeSystem_1.NonValidated.cs | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/design/datacontracts/RuntimeTypeSystem.md b/docs/design/datacontracts/RuntimeTypeSystem.md index 39a71ac3003ed..5693ab205ce59 100644 --- a/docs/design/datacontracts/RuntimeTypeSystem.md +++ b/docs/design/datacontracts/RuntimeTypeSystem.md @@ -95,9 +95,9 @@ struct MethodDescHandle ```csharp partial interface IRuntimeTypeSystem : IContract { - public virtual MethodDescHandle GetMethodDescHandle (TargetPointer methodDescPointer); + public virtual MethodDescHandle GetMethodDescHandle(TargetPointer methodDescPointer); - public virtual TargetPointer GetMethodTable (MethodDescHandle methodDesc); + public virtual TargetPointer GetMethodTable(MethodDescHandle methodDesc); } ``` diff --git a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs index 1753b6ea66b5b..b319f728eaf91 100644 --- a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs +++ b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.NonValidated.cs @@ -86,7 +86,6 @@ internal EEClass(Target target, TargetPointer eeClassPointer) internal TargetPointer MethodTable => _target.ReadPointer(Address + (ulong)_type.Fields[nameof(MethodTable)].Offset); } - internal struct MethodDesc { private readonly Target _target; @@ -223,7 +222,7 @@ private TargetPointer GetMethodDescChunkPointerThrowing(TargetPointer methodDesc { throw new InvalidOperationException("Target has no definite MethodDescChunk size"); } - // The runtime allocates a contiguous block of memory for a MethodDescChunk followedd by MethodDescAlignment * Size bytes of space + // The runtime allocates a contiguous block of memory for a MethodDescChunk followed by MethodDescAlignment * Size bytes of space // that is filled with MethodDesc (or its subclasses) instances. Each MethodDesc has a ChunkIndex that indicates its // offset from the end of the MethodDescChunk. ulong chunkAddress = (ulong)methodDescPointer - methodDescChunkSize.Value - umd.ChunkIndex * MethodDescAlignment; From 1f182f1bd71f1bb59f9a7dd92043ab95d8b97888 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Sat, 20 Jul 2024 09:41:25 -0400 Subject: [PATCH 24/24] make GetNumVtableSlots private --- .../managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs index 813e067d68ee6..8a690f6a79cf3 100644 --- a/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs +++ b/src/native/managed/cdacreader/src/Contracts/RuntimeTypeSystem_1.cs @@ -447,7 +447,7 @@ private FunctionPointerRetAndArgs(Target target, TargetPointer typePointer) } } - internal ushort GetNumVtableSlots(TypeHandle typeHandle) + private ushort GetNumVtableSlots(TypeHandle typeHandle) { if (!typeHandle.IsMethodTable()) return 0;