diff --git a/docs/design/datacontracts/RuntimeTypeSystem.md b/docs/design/datacontracts/RuntimeTypeSystem.md index d153b01d5776a9..33c6ccf8ccb438 100644 --- a/docs/design/datacontracts/RuntimeTypeSystem.md +++ b/docs/design/datacontracts/RuntimeTypeSystem.md @@ -2003,7 +2003,13 @@ Determining if a method supports multiple code versions: if (md.IsEligibleForTieredCompilation) return true; // MethodDesc::IsEligibleForReJIT - if (_target.Contracts.ReJIT.IsEnabled()) + // Targets without profiling support do not advertise ReJIT. + // An invalid advertised contract is still an error. + if (!_target.Contracts.TryGetContract(out IReJIT reJit)) + { + return false; + } + if (reJit.IsEnabled()) { if (!md.IsIL) return false; diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs index d6d89c5c15d473..a95dcae562005f 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs @@ -178,9 +178,32 @@ public TContract GetContract() where TContract : IContract return contract; } + /// + /// Attempts to get the requested contract. + /// + /// The contract type to retrieve. + /// + /// When this method returns, contains the contract if found; otherwise, its default value. + /// + /// + /// if the contract was retrieved; if it is not + /// advertised and no default implementation is registered. + /// + /// + /// The target advertises a contract version that this cDAC cannot provide. + /// + /// + /// Contract-creation errors propagate. Use the two-output overload to inspect availability failures. + /// public bool TryGetContract([NotNullWhen(true)] out TContract contract) where TContract : IContract { - return TryGetContract(out contract, out _); + if (!TryGetContract(out contract, out System.Exception? failureException)) + { + if (failureException is ContractMissingException) + return false; + throw failureException; + } + return true; } /// diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs index 3ed46f8c11e726..79e041c02b8f57 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs @@ -1998,7 +1998,11 @@ bool IRuntimeTypeSystem.IsVersionable(MethodDescHandle methodDesc) if (md.IsEligibleForTieredCompilation) return true; // MethodDesc::IsEligibleForReJIT - if (_target.Contracts.ReJIT.IsEnabled()) + if (!_target.Contracts.TryGetContract(out IReJIT reJit)) + { + return false; + } + if (reJit.IsEnabled()) { if (!md.IsIL) return false; diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.cs index 1e36b4ea04ed33..def70e98c4ac91 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.cs @@ -80,19 +80,16 @@ public static void Register(ContractRegistry registry) } /// - /// Eagerly validates that every contract required by the cDAC data-access interfaces can be - /// provided for the target. Contract availability is checked without instantiating the - /// contracts; is read to determine the target operating system so - /// that OS-specific contracts are validated only when the target platform actually uses them. - /// In-box (main-descriptor) contracts are required unconditionally. Contracts published by a - /// sub-descriptor are version-checked always, but their absence is tolerated while their - /// sub-descriptor is still pending. + /// Validates the contracts required by the cDAC data-access interfaces without instantiating them, + /// except for , which determines the target operating system. + /// ReJIT may be absent, but its advertised version must be supported. Sub-descriptor contracts + /// may be absent while their provider is pending. /// /// The target being validated (source of the contract registry and /// sub-descriptor resolution state). /// - /// Thrown for the first required contract that cannot be provided. The concrete exception type - /// and its identify the failure: + /// A required contract is missing or a checked contract version is unsupported. The concrete + /// exception type and its identify the failure: /// / /// if the target does not advertise a required contract, /// / @@ -105,11 +102,8 @@ public static void ValidateForDataAccess(Target target, Lock? apiLock = null) using Lock.Scope scope = apiLock is null ? default : apiLock.EnterScope(); ContractRegistry registry = target.Contracts; - // In-box (main-descriptor) contract accesses across the ISOSDac* and IXCLRData* surface that - // SOSDacImpl exposes. These live in the main descriptor, present as soon as the runtime module - // is loaded, so they are required eagerly and unconditionally - a genuinely-missing one is a - // serviceability failure even at early attach. IObjectiveCMarshal is intentionally omitted: - // SOS reaches it through TryGetContract so its absence degrades gracefully rather than faulting. + // Main-descriptor contracts are already published at early attach. + // IObjectiveCMarshal is omitted because its callers handle absence through TryGetContract. Validate(registry); Validate(registry); Validate(registry); @@ -125,7 +119,7 @@ public static void ValidateForDataAccess(Target target, Lock? apiLock = null) Validate(registry); Validate(registry); Validate(registry); - Validate(registry); + Validate(registry, allowMissing: true); // Not advertised without PROFILING_SUPPORTED. Validate(registry); Validate(registry); Validate(registry); @@ -170,9 +164,10 @@ public static void ValidateForDataAccess(Target target, Lock? apiLock = null) // sub-descriptor is resolved. Defer and let the tool APIs see a degradation to E_NOTIMPL. ValidateSubDescriptorContract(target); - static void Validate(ContractRegistry registry) where TContract : IContract + static void Validate(ContractRegistry registry, bool allowMissing = false) where TContract : IContract { - if (registry.TryValidate(out System.Exception? failure)) + if (registry.TryValidate(out System.Exception? failure) || + (allowMissing && failure is ContractMissingException)) { return; } diff --git a/src/native/managed/cdac/tests/UnitTests/ContractDescriptor/ContractRegistrationTests.cs b/src/native/managed/cdac/tests/UnitTests/ContractDescriptor/ContractRegistrationTests.cs index 69d207051d87c5..d1e188b5539da2 100644 --- a/src/native/managed/cdac/tests/UnitTests/ContractDescriptor/ContractRegistrationTests.cs +++ b/src/native/managed/cdac/tests/UnitTests/ContractDescriptor/ContractRegistrationTests.cs @@ -98,19 +98,40 @@ public void AdvertisedVersion_UsesVersionedRegistration_NotDefault(MockTarget.Ar Assert.Equal("v1", contract.Tag); } + public static IEnumerable UnsupportedVersionData() + { + foreach (object[] data in new MockTarget.StdArch()) + { + yield return [data[0], false]; + yield return [data[0], true]; + } + } + [Theory] - [ClassData(typeof(MockTarget.StdArch))] - public void AdvertisedVersion_NoMatchingRegistration_DoesNotFallBackToDefault(MockTarget.Architecture arch) + [MemberData(nameof(UnsupportedVersionData))] + public void AdvertisedVersion_NoMatchingRegistration_DoesNotFallBackToDefault(MockTarget.Architecture arch, bool obsolete) { - // Target advertises FakeContract (version "c1"), but only a default ("") - // registration exists. This is a version-skew failure and must NOT - // silently use the default registration. ContractDescriptorTarget target = CreateTarget( arch, advertisedContracts: ["FakeContract"], - registerFake: static r => r.Register(string.Empty, static t => new FakeContract("default"))); + registerFake: r => + { + r.Register(string.Empty, static t => new FakeContract("default")); + if (obsolete) + r.RegisterUnsupported("c1"); + }); - Assert.False(target.Contracts.TryGetContract(out IFakeContract? contract)); + Assert.False(target.Contracts.TryGetContract(out IFakeContract? contract, out System.Exception? failure)); Assert.Null(contract); + if (obsolete) + { + Assert.IsType(failure); + Assert.Throws(() => target.Contracts.TryGetContract(out _)); + } + else + { + Assert.IsType(failure); + Assert.Throws(() => target.Contracts.TryGetContract(out _)); + } } } diff --git a/src/native/managed/cdac/tests/UnitTests/ContractDescriptor/TargetTests.cs b/src/native/managed/cdac/tests/UnitTests/ContractDescriptor/TargetTests.cs index 66ded94c2f11f7..0c3479edf5cf1b 100644 --- a/src/native/managed/cdac/tests/UnitTests/ContractDescriptor/TargetTests.cs +++ b/src/native/managed/cdac/tests/UnitTests/ContractDescriptor/TargetTests.cs @@ -384,8 +384,8 @@ public void TryGetContract_UnrecognizedVersion_ReturnsContractUnrecognizedExcept Assert.Equal("unsupported-version", ex.ContractVersion); } - // The contracts required by the data-access interfaces, advertised at the versions - // CoreCLRContracts registers. Mirrors CoreCLRContracts.ValidateForDataAccess. + // Contracts used by the data-access interfaces, including optional ReJIT. + // Versions match CoreCLRContracts.Register. private static readonly IReadOnlyDictionary s_requiredDataAccessContracts = new Dictionary { @@ -448,19 +448,19 @@ public void TryValidate_RegisteredVersion_ReturnsTrue(MockTarget.Architecture ar [ClassData(typeof(MockTarget.StdArch))] public void TryValidate_DoesNotInstantiateContract(MockTarget.Architecture arch) { - // GCInfo's creator reads RuntimeInfo from target memory. Advertise GCInfo but omit RuntimeInfo: - // TryValidate must succeed (a creator is registered) without invoking it, whereas a real - // GetContract would chain into RuntimeInfo and fail. TargetTestHelpers targetTestHelpers = new(arch); ContractDescriptorBuilder builder = new(targetTestHelpers); ContractDescriptorBuilder.DescriptorBuilder descriptorBuilder = new(builder); + // GCInfo's factory requires RuntimeInfo; presence-only validation must not invoke it. descriptorBuilder.SetContracts(new Dictionary { ["GCInfo"] = "c1" }); Assert.True(builder.TryCreateTarget(descriptorBuilder, out ContractDescriptorTarget? target)); Assert.True(target.Contracts.TryValidate(out System.Exception? failure)); Assert.Null(failure); - Assert.Throws(() => target.Contracts.GCInfo); + ContractMissingException exception = Assert.Throws( + () => target.Contracts.TryGetContract(out _)); + Assert.Equal("RuntimeInfo", exception.ContractName); } [Theory] @@ -492,6 +492,48 @@ public void ValidateForDataAccess_AllRequiredPresent_DoesNotThrow(MockTarget.Arc Contracts.CoreCLRContracts.ValidateForDataAccess(target); } + public static IEnumerable ReJITValidationData() + { + foreach (object[] data in new MockTarget.StdArch()) + { + yield return [data[0], null, null]; + yield return [data[0], "c999", typeof(ContractUnrecognizedException)]; + yield return [data[0], "obsolete-version", typeof(ContractObsoleteException)]; + } + } + + [Theory] + [MemberData(nameof(ReJITValidationData))] + public void ValidateForDataAccess_ReJITIsOptional( + MockTarget.Architecture arch, string? reJitVersion, Type? expectedExceptionType) + { + TargetTestHelpers targetTestHelpers = new(arch); + ContractDescriptorBuilder builder = new(targetTestHelpers); + ContractDescriptorBuilder.DescriptorBuilder descriptorBuilder = new(builder); + Dictionary contracts = new(s_requiredDataAccessContracts); + contracts.Remove("ReJIT"); + if (reJitVersion is not null) + contracts["ReJIT"] = reJitVersion; + + descriptorBuilder.SetContracts(contracts); + Assert.True(builder.TryCreateTarget( + descriptorBuilder, + out ContractDescriptorTarget? target, + registry => registry.RegisterUnsupported("obsolete-version"))); + + if (expectedExceptionType is not null) + { + System.Exception exception = Assert.Throws(expectedExceptionType, () => Contracts.CoreCLRContracts.ValidateForDataAccess(target)); + ContractUnsupportedException failure = Assert.IsAssignableFrom(exception); + Assert.Equal("ReJIT", failure.ContractName); + Assert.Equal(reJitVersion, failure.ContractVersion); + } + else + { + Contracts.CoreCLRContracts.ValidateForDataAccess(target); + } + } + [Theory] [ClassData(typeof(MockTarget.StdArch))] public void ValidateForDataAccess_Net11Target_DoesNotRequireExternalMemoryHandles(MockTarget.Architecture arch) @@ -547,7 +589,7 @@ public void ValidateForDataAccess_MissingRequiredContract_ThrowsNotAdvertised(Mo ContractDescriptorBuilder.DescriptorBuilder descriptorBuilder = new(builder); descriptorBuilder.SetContracts( s_requiredDataAccessContracts - .Where(static pair => pair.Key != "RuntimeInfo") + .Where(static pair => pair.Key is not ("ReJIT" or "RuntimeInfo")) .ToDictionary(static pair => pair.Key, static pair => pair.Value)); Assert.True(builder.TryCreateTarget(descriptorBuilder, out ContractDescriptorTarget? target)); diff --git a/src/native/managed/cdac/tests/UnitTests/MethodDescTests.cs b/src/native/managed/cdac/tests/UnitTests/MethodDescTests.cs index 43a31e7bfa9e9f..0949b0aa0dd90c 100644 --- a/src/native/managed/cdac/tests/UnitTests/MethodDescTests.cs +++ b/src/native/managed/cdac/tests/UnitTests/MethodDescTests.cs @@ -78,7 +78,8 @@ private static IRuntimeTypeSystem CreateRuntimeTypeSystemContract( MockTarget.Architecture arch, Action configure, Mock? mockExecutionManager = null, - Mock? mockPrecodeStubs = null) + Mock? mockPrecodeStubs = null, + Action? configureTarget = null) { var targetBuilder = new TestPlaceholderTarget.Builder(arch); MockDescriptors.RuntimeTypeSystem rtsBuilder = new(targetBuilder.MemoryBuilder); @@ -86,6 +87,7 @@ private static IRuntimeTypeSystem CreateRuntimeTypeSystemContract( MockDescriptors.MockMethodDescriptorsBuilder methodDescBuilder = new(rtsBuilder, loaderBuilder); configure(methodDescBuilder); + configureTarget?.Invoke(targetBuilder); mockExecutionManager ??= new Mock(); mockPrecodeStubs ??= new Mock(); @@ -101,6 +103,87 @@ private static IRuntimeTypeSystem CreateRuntimeTypeSystemContract( return target.Contracts.RuntimeTypeSystem; } + private static (IRuntimeTypeSystem Contract, MethodDescHandle Method) CreateMethodForVersioning( + MockTarget.Architecture arch, + bool tiered, + Action configureTarget) + { + TargetPointer address = TargetPointer.Null; + IRuntimeTypeSystem contract = CreateRuntimeTypeSystemContract(arch, builder => + { + byte size = (byte)(builder.MethodDescLayout.Size / builder.MethodDescAlignment); + MockMethodDescChunk chunk = builder.AddMethodDescChunk("versioning", size); + chunk.MethodTable = builder.RTSBuilder.SystemObjectMethodTable.Address; + chunk.Size = size; + chunk.Count = 1; + MockMethodDesc method = chunk.GetMethodDescAtChunkIndex(0, builder.MethodDescLayout); + method.Flags3AndTokenRemainder = tiered + ? (ushort)MethodDescFlags_1.MethodDescFlags3.IsEligibleForTieredCompilation + : (ushort)0; + address = new TargetPointer(method.Address); + }, configureTarget: configureTarget); + return (contract, contract.GetMethodDescHandle(address)); + } + + public static IEnumerable IsVersionableData() + { + foreach (object[] data in new MockTarget.StdArch()) + { + MockTarget.Architecture arch = (MockTarget.Architecture)data[0]; + yield return [arch, false, null, false, false]; + yield return [arch, true, null, false, true]; + yield return [arch, false, false, false, false]; + yield return [arch, false, true, false, false]; + yield return [arch, false, true, true, true]; + yield return [arch, true, true, false, true]; + } + } + + [Theory] + [MemberData(nameof(IsVersionableData))] + public void IsVersionable_RespectsReJITAvailability( + MockTarget.Architecture arch, bool tiered, bool? reJitEnabled, bool supportsVersions, bool expected) + { + Mock reJit = new(MockBehavior.Strict); + Mock codeVersions = new(MockBehavior.Strict); + codeVersions.Setup(c => c.CodeVersionManagerSupportsMethod(It.IsAny())).Returns(supportsVersions); + if (reJitEnabled is bool enabled) + reJit.Setup(r => r.IsEnabled()).Returns(enabled); + + (IRuntimeTypeSystem contract, MethodDescHandle method) = CreateMethodForVersioning(arch, tiered, builder => + { + builder.AddMockContract(codeVersions); + if (reJitEnabled is not null) + builder.AddMockContract(reJit); + }); + + Assert.Equal(expected, contract.IsVersionable(method)); + reJit.Verify(r => r.IsEnabled(), Times.Exactly(!tiered && reJitEnabled is not null ? 1 : 0)); + codeVersions.Verify(c => c.CodeVersionManagerSupportsMethod(method.Address), + Times.Exactly(!tiered && reJitEnabled == true ? 1 : 0)); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void IsVersionable_AdvertisedReJITFailuresPropagate(MockTarget.Architecture arch) + { + (IRuntimeTypeSystem contract, MethodDescHandle method) = CreateMethodForVersioning( + arch, tiered: false, builder => builder.AddContract("c999")); + Assert.Throws(() => contract.IsVersionable(method)); + + // A partial dump can advertise ReJIT without capturing the profiler's data. + const ulong ProfilerControlBlockAddress = 0x3000_0000; + (contract, method) = CreateMethodForVersioning( + arch, tiered: false, builder => builder + .AddTypes(new Dictionary + { + [DataType.ProfControlBlock] = TargetTestHelpers.CreateTypeInfo(MockProfControlBlock.CreateLayout(arch)), + }) + .AddGlobals((nameof(Constants.Globals.ProfilerControlBlock), ProfilerControlBlockAddress)) + .AddContract("c1")); + Assert.Throws(() => contract.IsVersionable(method)); + } + [Theory] [ClassData(typeof(MockTarget.StdArch))] public void GetMethodDescHandle_ILMethod_GetBasicData(MockTarget.Architecture arch)