From af0b19702f81cbb6a32ca7882cc9a35c089c3f02 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 20 Apr 2026 22:40:33 +0000 Subject: [PATCH 1/4] Implement GetModuleForAssembly on DacDbiImpl using ILoader contract Replace the legacy-delegation stub with a contract-based implementation that uses ILoader.GetModuleHandleFromAssemblyPtr and ILoader.GetModule to resolve the assembly pointer to a module pointer. Includes #if DEBUG legacy validation block comparing the cDAC result against the legacy implementation. Adds dump-based integration test in DacDbiLoaderDumpTests.cs. Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/861680de-9c52-43c5-9a55-342e15da1710 Co-authored-by: barosiak <76071368+barosiak@users.noreply.github.com> --- .../Dbi/DacDbiImpl.cs | 27 ++++++++++++++++++- .../DumpTests/DacDbi/DacDbiLoaderDumpTests.cs | 27 +++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs index 6448703ec5be38..b2cd1f97d73b37 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs @@ -223,7 +223,32 @@ public int GetAssemblyInfo(ulong vmAssembly, DacDbiAssemblyInfo* pData) => LegacyFallbackHelper.CanFallback() && _legacy is not null ? _legacy.GetAssemblyInfo(vmAssembly, pData) : HResults.E_NOTIMPL; public int GetModuleForAssembly(ulong vmAssembly, ulong* pModule) - => LegacyFallbackHelper.CanFallback() && _legacy is not null ? _legacy.GetModuleForAssembly(vmAssembly, pModule) : HResults.E_NOTIMPL; + { + *pModule = 0; + int hr = HResults.S_OK; + try + { + Contracts.ILoader loader = _target.Contracts.Loader; + Contracts.ModuleHandle handle = loader.GetModuleHandleFromAssemblyPtr(new TargetPointer(vmAssembly)); + TargetPointer modulePtr = loader.GetModule(handle); + *pModule = modulePtr.Value; + } + catch (System.Exception ex) + { + hr = ex.HResult; + } +#if DEBUG + if (_legacy is not null) + { + ulong moduleLocal; + int hrLocal = _legacy.GetModuleForAssembly(vmAssembly, &moduleLocal); + Debug.ValidateHResult(hr, hrLocal); + if (hr == HResults.S_OK) + Debug.Assert(*pModule == moduleLocal, $"cDAC: {*pModule}, DAC: {moduleLocal}"); + } +#endif + return hr; + } public int GetAddressType(ulong address, int* pRetVal) => LegacyFallbackHelper.CanFallback() && _legacy is not null ? _legacy.GetAddressType(address, pRetVal) : HResults.E_NOTIMPL; diff --git a/src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiLoaderDumpTests.cs b/src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiLoaderDumpTests.cs index 3b31580d7550d5..069fd7be3c86de 100644 --- a/src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiLoaderDumpTests.cs +++ b/src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiLoaderDumpTests.cs @@ -34,6 +34,33 @@ public void GetAppDomainFullName_ReturnsNonEmpty(TestConfiguration config) Assert.False(string.IsNullOrEmpty(holder.Value), "AppDomain name should not be empty"); } + [ConditionalTheory] + [MemberData(nameof(TestConfigurations))] + public unsafe void GetModuleForAssembly_ReturnsNonNullModule(TestConfiguration config) + { + InitializeDumpTest(config); + DacDbiImpl dbi = CreateDacDbi(); + ILoader loader = Target.Contracts.Loader; + + TargetPointer appDomainPtr = Target.ReadGlobalPointer(Constants.Globals.AppDomain); + ulong appDomain = Target.ReadPointer(appDomainPtr); + var modules = loader.GetModuleHandles(new TargetPointer(appDomain), + AssemblyIterationFlags.IncludeLoaded | AssemblyIterationFlags.IncludeExecution); + + foreach (ModuleHandle module in modules) + { + TargetPointer assemblyPtr = loader.GetAssembly(module); + TargetPointer expectedModulePtr = loader.GetModule(module); + + ulong resultModule; + int hr = dbi.GetModuleForAssembly(assemblyPtr.Value, &resultModule); + Assert.Equal(System.HResults.S_OK, hr); + Assert.NotEqual(0UL, resultModule); + Assert.Equal(expectedModulePtr.Value, resultModule); + break; + } + } + [ConditionalTheory] [MemberData(nameof(TestConfigurations))] public unsafe void GetTypeHandle_ReturnsMethodTableForTypeDef(TestConfiguration config) From d8646541119e4b5bd313d563608444043e185d01 Mon Sep 17 00:00:00 2001 From: Barbara Rosiak Date: Fri, 24 Apr 2026 15:08:01 -0700 Subject: [PATCH 2/4] Add a test --- .../DumpTests/DacDbi/DacDbiLoaderDumpTests.cs | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiLoaderDumpTests.cs b/src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiLoaderDumpTests.cs index cb7b81822c6067..12736fbd4294cb 100644 --- a/src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiLoaderDumpTests.cs +++ b/src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiLoaderDumpTests.cs @@ -46,18 +46,14 @@ public void GetAppDomainFullName_ReturnsNonEmpty(TestConfiguration config) [ConditionalTheory] [MemberData(nameof(TestConfigurations))] - public unsafe void GetModuleForAssembly_ReturnsNonNullModule(TestConfiguration config) + public unsafe void GetModuleForAssembly(TestConfiguration config) { InitializeDumpTest(config); DacDbiImpl dbi = CreateDacDbi(); ILoader loader = Target.Contracts.Loader; - TargetPointer appDomainPtr = Target.ReadGlobalPointer(Constants.Globals.AppDomain); - ulong appDomain = Target.ReadPointer(appDomainPtr); - var modules = loader.GetModuleHandles(new TargetPointer(appDomain), - AssemblyIterationFlags.IncludeLoaded | AssemblyIterationFlags.IncludeExecution); - - foreach (ModuleHandle module in modules) + bool testedAtLeastOne = false; + foreach (ModuleHandle module in GetAllModules()) { TargetPointer assemblyPtr = loader.GetAssembly(module); TargetPointer expectedModulePtr = loader.GetModule(module); @@ -67,8 +63,21 @@ public unsafe void GetModuleForAssembly_ReturnsNonNullModule(TestConfiguration c Assert.Equal(System.HResults.S_OK, hr); Assert.NotEqual(0UL, resultModule); Assert.Equal(expectedModulePtr.Value, resultModule); - break; + testedAtLeastOne = true; } + Assert.True(testedAtLeastOne, "Expected at least one module in the dump"); + } + + [ConditionalTheory] + [MemberData(nameof(TestConfigurations))] + public unsafe void GetModuleForAssembly_InvalidAssembly(TestConfiguration config) + { + InitializeDumpTest(config); + DacDbiImpl dbi = CreateDacDbi(); + + ulong resultModule; + int hr = dbi.GetModuleForAssembly(0xDEAD, &resultModule); + Assert.NotEqual(System.HResults.S_OK, hr); } [ConditionalTheory] From dd5cd1b453dfe1bd8d62301a92d7d5b71e55f8c6 Mon Sep 17 00:00:00 2001 From: Barbara Rosiak <76071368+barosiak@users.noreply.github.com> Date: Fri, 24 Apr 2026 15:24:39 -0700 Subject: [PATCH 3/4] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../Dbi/DacDbiImpl.cs | 2 +- .../cdac/tests/DumpTests/DacDbi/DacDbiLoaderDumpTests.cs | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs index f06b4a22302b36..54ae8e8d815648 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs @@ -304,7 +304,7 @@ public int GetModuleForAssembly(ulong vmAssembly, ulong* pModule) int hrLocal = _legacy.GetModuleForAssembly(vmAssembly, &moduleLocal); Debug.ValidateHResult(hr, hrLocal); if (hr == HResults.S_OK) - Debug.Assert(*pModule == moduleLocal, $"cDAC: {*pModule}, DAC: {moduleLocal}"); + Debug.Assert(*pModule == moduleLocal, $"cDAC: {*pModule:x}, DAC: {moduleLocal:x}"); } #endif return hr; diff --git a/src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiLoaderDumpTests.cs b/src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiLoaderDumpTests.cs index 12736fbd4294cb..f75f1aa9319fa8 100644 --- a/src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiLoaderDumpTests.cs +++ b/src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiLoaderDumpTests.cs @@ -75,9 +75,10 @@ public unsafe void GetModuleForAssembly_InvalidAssembly(TestConfiguration config InitializeDumpTest(config); DacDbiImpl dbi = CreateDacDbi(); - ulong resultModule; - int hr = dbi.GetModuleForAssembly(0xDEAD, &resultModule); + ulong resultModule = ulong.MaxValue; + int hr = dbi.GetModuleForAssembly(0, &resultModule); Assert.NotEqual(System.HResults.S_OK, hr); + Assert.Equal(0UL, resultModule); } [ConditionalTheory] From 42bd0dc3ea9725ed87661e33c92ce5241efe40c4 Mon Sep 17 00:00:00 2001 From: Barbara Rosiak <76071368+barosiak@users.noreply.github.com> Date: Fri, 24 Apr 2026 16:04:37 -0700 Subject: [PATCH 4/4] Rename a test Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../cdac/tests/DumpTests/DacDbi/DacDbiLoaderDumpTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiLoaderDumpTests.cs b/src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiLoaderDumpTests.cs index f75f1aa9319fa8..d7a776ff19fc5f 100644 --- a/src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiLoaderDumpTests.cs +++ b/src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiLoaderDumpTests.cs @@ -46,7 +46,7 @@ public void GetAppDomainFullName_ReturnsNonEmpty(TestConfiguration config) [ConditionalTheory] [MemberData(nameof(TestConfigurations))] - public unsafe void GetModuleForAssembly(TestConfiguration config) + public unsafe void GetModuleForAssembly_ReturnsExpectedModule(TestConfiguration config) { InitializeDumpTest(config); DacDbiImpl dbi = CreateDacDbi();