[cDAC] Add FeatureFlags contract, reduce direct global reads in legacy impl, expand NativeAOT descriptor parity#129637
Conversation
…y impl, expand NativeAOT descriptor parity - Add IFeatureFlags contract and FeatureFlags_1 implementation that treats absent globals as disabled (feature flag globals are now only emitted when enabled, removing the #else zero-value branches from CoreCLR datadescriptor) - Migrate direct ReadGlobal<byte>(FeatureX) calls in ExecutionManager, GC_1, and SOSDacImpl to use IFeatureFlags.IsEnabled() - Make GC heap-analyze globals optional (InternalRootArray, InternalRootArrayIndex, HeapAnalyzeSuccess now nullable) to handle builds where they are absent - Add RuntimeInstance/TypeManager cdac_data<T> specializations and type descriptors in NativeAOT datadescriptor for module enumeration - Declare additional NativeAOT contract parity entries (Loader, ExecutionManager, SyncBlock, CodeVersions, ReJIT, PrecodeStubs, PlatformMetadata, FeatureFlags) - Add FeatureFlags.md data contract documentation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a new cDAC FeatureFlags contract to centralize “feature enabled” checks, updates GC heap-analyze globals to be optional, and expands CoreCLR/NativeAOT data-descriptor parity by adding additional NativeAOT type/global descriptors and contract declarations.
Changes:
- Add
IFeatureFlags/RuntimeFeatureabstractions plus aFeatureFlags_1implementation, and register the contract for CoreCLR. - Make GC heap-analyze globals optional end-to-end (contracts + abstractions + SOS legacy bridge) to tolerate missing descriptor entries.
- Update CoreCLR and NativeAOT
datadescriptor.incfiles to align contract/global/type coverage, and add contract documentation.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs | Uses FeatureFlags for COM-related gating; makes heap-analyze fields nullable in COM output; switches CLR notification arg count to TryReadGlobal. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/GC/GCHeapSVR.cs | Makes heap-analyze fields nullable in the generated GCHeap data type. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.cs | Registers the new IFeatureFlags contract for CoreCLR (“c1”). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GC/IGCHeap.cs | Updates IGCHeap heap-analyze properties to nullable. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GC/GCHeapWKS.cs | Reads heap-analyze globals via TryReadGlobalPointer and exposes them as nullable. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GC/GC_1.cs | Replaces direct feature-global reads with IFeatureFlags checks for handle-type behavior. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/FeatureFlags_1.cs | Adds the CoreCLR FeatureFlags contract implementation backed by optional global reads. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManagerCore.EEJitManager.cs | Gates debug-info flag byte logic on OnStackReplacement via FeatureFlags. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManagerCore.cs | Gates portable-entrypoint handling via FeatureFlags. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IGC.cs | Updates GCHeapData to carry nullable heap-analyze properties. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IFeatureFlags.cs | Introduces the RuntimeFeature enum and IFeatureFlags contract interface (new public surface). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs | Adds ContractRegistry.FeatureFlags accessor. |
| src/coreclr/vm/datadescriptor/datadescriptor.inc | Changes feature-flag globals to be emitted only when enabled; declares FeatureFlags contract. |
| src/coreclr/nativeaot/Runtime/TypeManager.h | Exposes TypeManager offsets via cdac_data<TypeManager> specialization. |
| src/coreclr/nativeaot/Runtime/RuntimeInstance.h | Exposes RuntimeInstance offsets via cdac_data<RuntimeInstance> specialization. |
| src/coreclr/nativeaot/Runtime/datadescriptor/datadescriptor.inc | Adds NativeAOT type/global descriptors (RuntimeInstance/TypeManager/StressLog.ModuleOffset) and declares additional contracts including FeatureFlags. |
| src/coreclr/nativeaot/Runtime/datadescriptor/datadescriptor.h | Declares g_pTheRuntimeInstance pointer for NativeAOT descriptor use. |
| docs/design/datacontracts/FeatureFlags.md | Adds the FeatureFlags contract documentation and semantics (“missing or 0 => disabled”). |
Comments suppressed due to low confidence (2)
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManagerCore.cs:291
_target.Contracts.FeatureFlagswill throwNotImplementedExceptionon targets whose descriptor predates theFeatureFlagscontract declaration, which is a likely scenario for cDAC (tools not version-matched to the runtime). Since this check is just gating behavior, it should gracefully fall back to reading the legacy global directly (treat missing/0 as disabled) when the contract isn't available.
if (_target.Contracts.FeatureFlags.IsEnabled(RuntimeFeature.PortableEntrypoints))
{
Data.PortableEntryPoint portableEntryPoint = _target.ProcessedData.GetOrAdd<Data.PortableEntryPoint>(entrypoint.AsTargetPointer);
return portableEntryPoint.MethodDesc;
}
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManagerCore.EEJitManager.cs:113
- Same compatibility issue as other call sites:
Target.Contracts.FeatureFlagswill throw when the target doesn't declare theFeatureFlagscontract (older runtimes/dumps). This should fall back to the legacyFeatureOnStackReplacementglobal (treat missing/0 as disabled) so the rest of the contract can still function.
if (!GetRealCodeHeader(rangeSection, codeStart, out Data.RealCodeHeader? realCodeHeader))
return TargetPointer.Null;
bool featureOnStackReplacement = Target.Contracts.FeatureFlags.IsEnabled(RuntimeFeature.OnStackReplacement);
Data.EEJitManager eeJitManager = Target.ProcessedData.GetOrAdd<Data.EEJitManager>(rangeSection.Data.JitManager);
if (featureOnStackReplacement || eeJitManager.StoreRichDebugInfo)
hasFlagByte = true;
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
Summary
This PR reduces direct
ReadGlobalcalls for feature flags in the cDAC legacy implementation and adds parity between CoreCLR and NativeAOT data descriptors.Changes
FeatureFlags contract (new)
IFeatureFlagscontract withIsEnabled(RuntimeFeature)APIFeatureFlags_1implementation: a feature is disabled when its global is absent from the descriptor or present with a zero value#elsezero-value branches) — absent = disabled is the semanticFeatureFlags c1in both CoreCLR and NativeAOT datadescriptorsLegacy impl cleanup
ReadGlobal<byte>(Constants.Globals.FeatureX)calls inExecutionManagerCore,GC_1, andSOSDacImplto useIFeatureFlags.IsEnabled()InternalRootArray,InternalRootArrayIndex,HeapAnalyzeSuccessare now nullable) to gracefully handle builds where they are absentMaxClrNotificationArgsreads now useTryReadGlobalwith a zero fallback for runtimes that don't support CLR notificationsNativeAOT descriptor parity
cdac_data<RuntimeInstance>andcdac_data<TypeManager>specializations exposing module-list offsetsRuntimeInstanceandTypeManagertype descriptors andg_pTheRuntimeInstanceglobal pointer to NativeAOT datadescriptorStressLog.ModuleOffsetfield to NativeAOT datadescriptorLoader,ExecutionManager,SyncBlock,CodeVersions,ReJIT,PrecodeStubs,PlatformMetadataFeatureFlags.mddata contract documentationNote
This pull request was created with the assistance of GitHub Copilot.