From d3320e0b307c2054a34481e489860f64211647e9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 22 Mar 2026 21:51:23 +0000 Subject: [PATCH 1/8] implement cDAC IXCLRDataModule::GetName via new Loader API GetSimpleName Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/8d8354fa-39ad-4227-b870-1b29a92a587f --- docs/design/datacontracts/Loader.md | 9 +++++ src/coreclr/vm/ceeload.h | 1 + .../Contracts/ILoader.cs | 1 + .../Contracts/Loader_1.cs | 8 +++++ .../Data/Module.cs | 2 ++ .../ClrDataModule.cs | 35 ++++++++++++++++++- src/native/managed/cdac/tests/LoaderTests.cs | 34 ++++++++++++++++++ .../MockDescriptors/MockDescriptors.Loader.cs | 17 ++++++++- .../tests/MockDescriptors/MockDescriptors.cs | 1 + .../cdac/tests/TestPlaceholderTarget.cs | 18 +++++++++- 10 files changed, 123 insertions(+), 3 deletions(-) diff --git a/docs/design/datacontracts/Loader.md b/docs/design/datacontracts/Loader.md index 7707e5d1cc228f..b1f08006c2c27b 100644 --- a/docs/design/datacontracts/Loader.md +++ b/docs/design/datacontracts/Loader.md @@ -69,6 +69,7 @@ IEnumerable GetInstantiatedMethods(ModuleHandle handle); bool IsProbeExtensionResultValid(ModuleHandle handle); ModuleFlags GetFlags(ModuleHandle handle); +string GetSimpleName(ModuleHandle handle); string GetPath(ModuleHandle handle); string GetFileName(ModuleHandle handle); TargetPointer GetLoaderAllocator(ModuleHandle handle); @@ -102,6 +103,7 @@ IReadOnlyDictionary GetLoaderAllocatorHeaps(TargetPointer | `Module` | `LoaderAllocator` | LoaderAllocator of the Module | | `Module` | `Path` | Path of the Module (UTF-16, null-terminated) | | `Module` | `FileName` | File name of the Module (UTF-16, null-terminated) | +| `Module` | `SimpleName` | Simple name of the Module (UTF-8, null-terminated) | | `Module` | `GrowableSymbolStream` | Pointer to the in memory symbol stream | | `Module` | `AvailableTypeParams` | Pointer to an EETypeHashTable | | `Module` | `InstMethodHashTable` | Pointer to an InstMethodHashTable | @@ -575,6 +577,13 @@ ModuleFlags GetFlags(ModuleHandle handle) return GetFlags(target.Read(handle.Address + /* Module::Flags offset */)); } +string GetSimpleName(ModuleHandle handle) +{ + TargetPointer simpleNameStart = target.ReadPointer(handle.Address + /* Module::SimpleName offset */); + byte[] simpleName = // Read bytes from target starting at simpleNameStart until null terminator + return Encoding.UTF8.GetString(simpleName); +} + string GetPath(ModuleHandle handle) { TargetPointer pathStart = target.ReadPointer(handle.Address + /* Module::Path offset */); diff --git a/src/coreclr/vm/ceeload.h b/src/coreclr/vm/ceeload.h index 102f776aa5214c..1bcc3b2bda95e1 100644 --- a/src/coreclr/vm/ceeload.h +++ b/src/coreclr/vm/ceeload.h @@ -1717,6 +1717,7 @@ struct cdac_data static constexpr size_t Flags = offsetof(Module, m_dwTransientFlags); static constexpr size_t LoaderAllocator = offsetof(Module, m_loaderAllocator); static constexpr size_t DynamicMetadata = offsetof(Module, m_pDynamicMetadata); + static constexpr size_t SimpleName = offsetof(Module, m_pSimpleName); static constexpr size_t Path = offsetof(Module, m_path); static constexpr size_t FileName = offsetof(Module, m_fileName); static constexpr size_t ReadyToRunInfo = offsetof(Module, m_pReadyToRunInfo); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ILoader.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ILoader.cs index f814eb82aa4e5f..1f4958b8f38961 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ILoader.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ILoader.cs @@ -75,6 +75,7 @@ public interface ILoader : IContract bool IsProbeExtensionResultValid(ModuleHandle handle) => throw new NotImplementedException(); ModuleFlags GetFlags(ModuleHandle handle) => throw new NotImplementedException(); + string GetSimpleName(ModuleHandle handle) => throw new NotImplementedException(); string GetPath(ModuleHandle handle) => throw new NotImplementedException(); string GetFileName(ModuleHandle handle) => throw new NotImplementedException(); TargetPointer GetLoaderAllocator(ModuleHandle handle) => throw new NotImplementedException(); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs index 55151b6073c3fb..950051c4cfdad7 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs @@ -376,6 +376,14 @@ ModuleFlags ILoader.GetFlags(ModuleHandle handle) return GetFlags(module); } + string ILoader.GetSimpleName(ModuleHandle handle) + { + Data.Module module = _target.ProcessedData.GetOrAdd(handle.Address); + return module.SimpleName != TargetPointer.Null + ? _target.ReadUtf8String(module.SimpleName) + : string.Empty; + } + string ILoader.GetPath(ModuleHandle handle) { Data.Module module = _target.ProcessedData.GetOrAdd(handle.Address); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/Module.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/Module.cs index 7fc833df437e84..2e7b7b589eb412 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/Module.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/Module.cs @@ -20,6 +20,7 @@ public Module(Target target, TargetPointer address) Base = target.ReadPointer(address + (ulong)type.Fields[nameof(Base)].Offset); LoaderAllocator = target.ReadPointer(address + (ulong)type.Fields[nameof(LoaderAllocator)].Offset); DynamicMetadata = target.ReadPointer(address + (ulong)type.Fields[nameof(DynamicMetadata)].Offset); + SimpleName = target.ReadPointer(address + (ulong)type.Fields[nameof(SimpleName)].Offset); Path = target.ReadPointer(address + (ulong)type.Fields[nameof(Path)].Offset); FileName = target.ReadPointer(address + (ulong)type.Fields[nameof(FileName)].Offset); ReadyToRunInfo = target.ReadPointer(address + (ulong)type.Fields[nameof(ReadyToRunInfo)].Offset); @@ -43,6 +44,7 @@ public Module(Target target, TargetPointer address) public TargetPointer Base { get; init; } public TargetPointer LoaderAllocator { get; init; } public TargetPointer DynamicMetadata { get; init; } + public TargetPointer SimpleName { get; init; } public TargetPointer Path { get; init; } public TargetPointer FileName { get; init; } public TargetPointer ReadyToRunInfo { get; init; } diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs index 1e01218e074dbe..876c888723d147 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs @@ -123,7 +123,40 @@ int IXCLRDataModule.EndEnumDataByName(ulong handle) => _legacyModule is not null ? _legacyModule.EndEnumDataByName(handle) : HResults.E_NOTIMPL; int IXCLRDataModule.GetName(uint bufLen, uint* nameLen, char* name) - => _legacyModule is not null ? _legacyModule.GetName(bufLen, nameLen, name) : HResults.E_NOTIMPL; + { + try + { + Contracts.ILoader contract = _target.Contracts.Loader; + Contracts.ModuleHandle handle = contract.GetModuleHandleFromModulePtr(_address); + string result = contract.GetSimpleName(handle); + + if (string.IsNullOrEmpty(result)) + return HResults.E_FAIL; + + OutputBufferHelpers.CopyStringToBuffer(name, bufLen, nameLen, result); + } + catch (System.Exception ex) + { + return ex.HResult; + } + +#if DEBUG + if (_legacyModule is not null) + { + char[] nameLocal = new char[bufLen]; + uint nameLenLocal; + int hrLocal; + fixed (char* ptr = nameLocal) + { + hrLocal = _legacyModule.GetName(bufLen, &nameLenLocal, ptr); + } + Debug.Assert(hrLocal == HResults.S_OK); + Debug.Assert(nameLen == null || *nameLen == nameLenLocal); + Debug.Assert(name == null || new ReadOnlySpan(nameLocal, 0, (int)nameLenLocal - 1).SequenceEqual(new string(name))); + } +#endif + return HResults.S_OK; + } int IXCLRDataModule.GetFileName(uint bufLen, uint* nameLen, char* name) { try diff --git a/src/native/managed/cdac/tests/LoaderTests.cs b/src/native/managed/cdac/tests/LoaderTests.cs index 999fd6a9fb1320..a6e30eb65e977d 100644 --- a/src/native/managed/cdac/tests/LoaderTests.cs +++ b/src/native/managed/cdac/tests/LoaderTests.cs @@ -84,6 +84,40 @@ public void GetFileName(MockTarget.Architecture arch) } } + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void GetSimpleName(MockTarget.Architecture arch) + { + // Set up the target + TargetTestHelpers helpers = new(arch); + MockMemorySpace.Builder builder = new(helpers); + MockLoader loader = new(builder); + + string expected = "TestModule"; + + // Add the modules + TargetPointer moduleAddr = loader.AddModule(simpleName: expected); + TargetPointer moduleAddrEmptyName = loader.AddModule(); + + var target = new TestPlaceholderTarget(arch, builder.GetMemoryContext().ReadFromTarget, loader.Types); + target.SetContracts(Mock.Of( + c => c.Loader == ((IContractFactory)new LoaderFactory()).CreateContract(target, 1))); + + // Validate the expected module data + Contracts.ILoader contract = target.Contracts.Loader; + Assert.NotNull(contract); + { + Contracts.ModuleHandle handle = contract.GetModuleHandleFromModulePtr(moduleAddr); + string actual = contract.GetSimpleName(handle); + Assert.Equal(expected, actual); + } + { + Contracts.ModuleHandle handle = contract.GetModuleHandleFromModulePtr(moduleAddrEmptyName); + string actual = contract.GetSimpleName(handle); + Assert.Equal(string.Empty, actual); + } + } + private static readonly Dictionary MockHeapDictionary = new() { ["LowFrequencyHeap"] = new(0x1000), diff --git a/src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Loader.cs b/src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Loader.cs index 303cc468df3adc..c2bc835417ab85 100644 --- a/src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Loader.cs +++ b/src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Loader.cs @@ -41,7 +41,7 @@ public Loader(MockMemorySpace.Builder builder, (ulong Start, ulong End) allocati ]); } - internal TargetPointer AddModule(string? path = null, string? fileName = null) + internal TargetPointer AddModule(string? path = null, string? fileName = null, string? simpleName = null) { TargetTestHelpers helpers = _builder.TargetTestHelpers; Target.TypeInfo typeInfo = Types[DataType.Module]; @@ -49,6 +49,21 @@ internal TargetPointer AddModule(string? path = null, string? fileName = null) MockMemorySpace.HeapFragment module = _allocator.Allocate(size, "Module"); _builder.AddHeapFragment(module); + if (simpleName != null) + { + // Simple name data (UTF-8, null-terminated) + ulong simpleNameSize = (ulong)Encoding.UTF8.GetByteCount(simpleName) + 1; + MockMemorySpace.HeapFragment simpleNameFragment = _allocator.Allocate(simpleNameSize, $"Module simple name = {simpleName}"); + Encoding.UTF8.GetBytes(simpleName).AsSpan().CopyTo(simpleNameFragment.Data); + simpleNameFragment.Data[^1] = 0; + _builder.AddHeapFragment(simpleNameFragment); + + // Pointer to simple name + helpers.WritePointer( + module.Data.AsSpan().Slice(typeInfo.Fields[nameof(Data.Module.SimpleName)].Offset, helpers.PointerSize), + simpleNameFragment.Address); + } + if (path != null) { // Path data diff --git a/src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.cs b/src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.cs index 104a6e0c22bd59..9ccdb3852caa53 100644 --- a/src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.cs +++ b/src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.cs @@ -151,6 +151,7 @@ internal record TypeFields new(nameof(Data.Module.Flags), DataType.uint32), new(nameof(Data.Module.LoaderAllocator), DataType.pointer), new(nameof(Data.Module.DynamicMetadata), DataType.pointer), + new(nameof(Data.Module.SimpleName), DataType.pointer), new(nameof(Data.Module.Path), DataType.pointer), new(nameof(Data.Module.FileName), DataType.pointer), new(nameof(Data.Module.ReadyToRunInfo), DataType.pointer), diff --git a/src/native/managed/cdac/tests/TestPlaceholderTarget.cs b/src/native/managed/cdac/tests/TestPlaceholderTarget.cs index b8e7017885cbdd..7f3cb59f6ee0a9 100644 --- a/src/native/managed/cdac/tests/TestPlaceholderTarget.cs +++ b/src/native/managed/cdac/tests/TestPlaceholderTarget.cs @@ -159,7 +159,23 @@ public override void ReadBuffer(ulong address, Span buffer) } public override void WriteBuffer(ulong address, Span buffer) => throw new NotImplementedException(); - public override string ReadUtf8String(ulong address) => throw new NotImplementedException(); + public override string ReadUtf8String(ulong address) + { + // Read bytes until we find the null terminator + ulong end = address; + while (Read(end) != 0) + { + end += sizeof(byte); + } + + int length = (int)(end - address); + if (length == 0) + return string.Empty; + + Span span = new byte[length]; + ReadBuffer(address, span); + return Encoding.UTF8.GetString(span); + } public override string ReadUtf16String(ulong address) { // Read characters until we find the null terminator From f245be57ce087e4b37991ce752796441ffb51a10 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 22 Mar 2026 22:40:39 +0000 Subject: [PATCH 2/8] fixup: throw E_FAIL and fix Loader.md GetSimpleName pseudocode Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/39def273-a823-4c95-9f33-063d207f30d7 --- docs/design/datacontracts/Loader.md | 4 ++-- .../ClrDataModule.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/design/datacontracts/Loader.md b/docs/design/datacontracts/Loader.md index b1f08006c2c27b..5f3f00557fc804 100644 --- a/docs/design/datacontracts/Loader.md +++ b/docs/design/datacontracts/Loader.md @@ -580,8 +580,8 @@ ModuleFlags GetFlags(ModuleHandle handle) string GetSimpleName(ModuleHandle handle) { TargetPointer simpleNameStart = target.ReadPointer(handle.Address + /* Module::SimpleName offset */); - byte[] simpleName = // Read bytes from target starting at simpleNameStart until null terminator - return Encoding.UTF8.GetString(simpleName); + byte[] simpleName = // Read from target starting at simpleNameStart until null terminator + return new string(simpleName); } string GetPath(ModuleHandle handle) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs index 876c888723d147..1435efe0956067 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs @@ -131,7 +131,7 @@ int IXCLRDataModule.GetName(uint bufLen, uint* nameLen, char* name) string result = contract.GetSimpleName(handle); if (string.IsNullOrEmpty(result)) - return HResults.E_FAIL; + throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; OutputBufferHelpers.CopyStringToBuffer(name, bufLen, nameLen, result); } From e8b7346ef07812f127a9350c5575cfca83b1f75a Mon Sep 17 00:00:00 2001 From: Rachel Date: Sun, 22 Mar 2026 18:50:38 -0700 Subject: [PATCH 3/8] Update ClrDataModule.cs --- .../ClrDataModule.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs index 1435efe0956067..1300e3c191ef8b 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs @@ -124,11 +124,12 @@ int IXCLRDataModule.EndEnumDataByName(ulong handle) int IXCLRDataModule.GetName(uint bufLen, uint* nameLen, char* name) { + int hr = HResults.S_OK; try { - Contracts.ILoader contract = _target.Contracts.Loader; - Contracts.ModuleHandle handle = contract.GetModuleHandleFromModulePtr(_address); - string result = contract.GetSimpleName(handle); + Contracts.ILoader loader = _target.Contracts.Loader; + Contracts.ModuleHandle handle = loader.GetModuleHandleFromModulePtr(_address); + string result = loader.GetSimpleName(handle); if (string.IsNullOrEmpty(result)) throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; @@ -137,7 +138,7 @@ int IXCLRDataModule.GetName(uint bufLen, uint* nameLen, char* name) } catch (System.Exception ex) { - return ex.HResult; + hr = ex.HResult; } #if DEBUG @@ -150,12 +151,12 @@ int IXCLRDataModule.GetName(uint bufLen, uint* nameLen, char* name) { hrLocal = _legacyModule.GetName(bufLen, &nameLenLocal, ptr); } - Debug.Assert(hrLocal == HResults.S_OK); + Debug.ValidateHResult(hr, hrLocal); Debug.Assert(nameLen == null || *nameLen == nameLenLocal); Debug.Assert(name == null || new ReadOnlySpan(nameLocal, 0, (int)nameLenLocal - 1).SequenceEqual(new string(name))); } #endif - return HResults.S_OK; + return hr; } int IXCLRDataModule.GetFileName(uint bufLen, uint* nameLen, char* name) { From 7f240e42453782f2a5401a8f2c4b27f32e6c71e9 Mon Sep 17 00:00:00 2001 From: rcj1 Date: Mon, 23 Mar 2026 13:53:46 -0700 Subject: [PATCH 4/8] e --- docs/design/datacontracts/Loader.md | 8 +++++--- src/coreclr/vm/datadescriptor/datadescriptor.inc | 1 + .../Contracts/ILoader.cs | 2 +- .../Target.cs | 3 ++- .../Contracts/Loader_1.cs | 13 +++++++++---- .../ClrDataModule.cs | 12 ++++++++---- .../ContractDescriptorTarget.cs | 7 +++++-- src/native/managed/cdac/tests/LoaderTests.cs | 8 +++++--- .../managed/cdac/tests/TestPlaceholderTarget.cs | 2 +- 9 files changed, 37 insertions(+), 19 deletions(-) diff --git a/docs/design/datacontracts/Loader.md b/docs/design/datacontracts/Loader.md index 5f3f00557fc804..f13c776ae30367 100644 --- a/docs/design/datacontracts/Loader.md +++ b/docs/design/datacontracts/Loader.md @@ -69,7 +69,7 @@ IEnumerable GetInstantiatedMethods(ModuleHandle handle); bool IsProbeExtensionResultValid(ModuleHandle handle); ModuleFlags GetFlags(ModuleHandle handle); -string GetSimpleName(ModuleHandle handle); +bool TryGetSimpleName(ModuleHandle handle, out string simpleName); string GetPath(ModuleHandle handle); string GetFileName(ModuleHandle handle); TargetPointer GetLoaderAllocator(ModuleHandle handle); @@ -577,11 +577,13 @@ ModuleFlags GetFlags(ModuleHandle handle) return GetFlags(target.Read(handle.Address + /* Module::Flags offset */)); } -string GetSimpleName(ModuleHandle handle) +bool TryGetSimpleName(ModuleHandle handle, out string simpleName) { TargetPointer simpleNameStart = target.ReadPointer(handle.Address + /* Module::SimpleName offset */); + if (simpleNameStart == TargetPointer.Null) + return false byte[] simpleName = // Read from target starting at simpleNameStart until null terminator - return new string(simpleName); + simpleName = // convert to string, throw on invalid UTF-8 } string GetPath(ModuleHandle handle) diff --git a/src/coreclr/vm/datadescriptor/datadescriptor.inc b/src/coreclr/vm/datadescriptor/datadescriptor.inc index e73483de784ae5..3e7993b5dbb078 100644 --- a/src/coreclr/vm/datadescriptor/datadescriptor.inc +++ b/src/coreclr/vm/datadescriptor/datadescriptor.inc @@ -238,6 +238,7 @@ CDAC_TYPE_FIELD(Module, /*pointer*/, Base, cdac_data::Base) CDAC_TYPE_FIELD(Module, /*uint32*/, Flags, cdac_data::Flags) CDAC_TYPE_FIELD(Module, /*pointer*/, LoaderAllocator, cdac_data::LoaderAllocator) CDAC_TYPE_FIELD(Module, /*pointer*/, DynamicMetadata, cdac_data::DynamicMetadata) +CDAC_TYPE_FIELD(Module, /*pointer*/, SimpleName, cdac_data::SimpleName) CDAC_TYPE_FIELD(Module, /*pointer*/, Path, cdac_data::Path) CDAC_TYPE_FIELD(Module, /*pointer*/, FileName, cdac_data::FileName) CDAC_TYPE_FIELD(Module, /*pointer*/, ReadyToRunInfo, cdac_data::ReadyToRunInfo) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ILoader.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ILoader.cs index 1f4958b8f38961..11593d1b302e1a 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ILoader.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ILoader.cs @@ -75,7 +75,7 @@ public interface ILoader : IContract bool IsProbeExtensionResultValid(ModuleHandle handle) => throw new NotImplementedException(); ModuleFlags GetFlags(ModuleHandle handle) => throw new NotImplementedException(); - string GetSimpleName(ModuleHandle handle) => throw new NotImplementedException(); + bool TryGetSimpleName(ModuleHandle handle, out string simpleName) => throw new NotImplementedException(); string GetPath(ModuleHandle handle) => throw new NotImplementedException(); string GetFileName(ModuleHandle handle) => throw new NotImplementedException(); TargetPointer GetLoaderAllocator(ModuleHandle handle) => throw new NotImplementedException(); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Target.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Target.cs index d0d691f242c3a5..027ce5c4522c2e 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Target.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Target.cs @@ -102,9 +102,10 @@ public abstract class Target /// Read a null-terminated UTF-8 string from the target /// /// Address to start reading from + /// If true, throw if the string is not valid UTF-8. If false, replace invalid sequences with the replacement character. /// String read from the target /// Thrown when the read operation fails - public abstract string ReadUtf8String(ulong address); + public abstract string ReadUtf8String(ulong address, bool strict); /// /// Read a null-terminated UTF-16 string from the target in target endianness diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs index 950051c4cfdad7..ac726eeed7f6e5 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs @@ -376,12 +376,17 @@ ModuleFlags ILoader.GetFlags(ModuleHandle handle) return GetFlags(module); } - string ILoader.GetSimpleName(ModuleHandle handle) + bool ILoader.TryGetSimpleName(ModuleHandle handle, out string simpleName) { + simpleName = string.Empty; Data.Module module = _target.ProcessedData.GetOrAdd(handle.Address); - return module.SimpleName != TargetPointer.Null - ? _target.ReadUtf8String(module.SimpleName) - : string.Empty; + if (module.SimpleName != TargetPointer.Null) + { + simpleName = _target.ReadUtf8String(module.SimpleName); + return true; + } + else + return false; } string ILoader.GetPath(ModuleHandle handle) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs index 1300e3c191ef8b..beb3bb9cc3b136 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs @@ -125,16 +125,20 @@ int IXCLRDataModule.EndEnumDataByName(ulong handle) int IXCLRDataModule.GetName(uint bufLen, uint* nameLen, char* name) { int hr = HResults.S_OK; + int E_INSUFFICIENT_BUFFER = unchecked((int)0x8007007A); try { + if (nameLen != null) + *nameLen = 0; Contracts.ILoader loader = _target.Contracts.Loader; Contracts.ModuleHandle handle = loader.GetModuleHandleFromModulePtr(_address); - string result = loader.GetSimpleName(handle); - - if (string.IsNullOrEmpty(result)) - throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; + if (!loader.TryGetSimpleName(handle, out string result)) + throw new ArgumentException("Module does not have a simple name"); OutputBufferHelpers.CopyStringToBuffer(name, bufLen, nameLen, result); + // throw on insufficient buffer + if (*nameLen > bufLen) + throw Marshal.GetExceptionForHR(E_INSUFFICIENT_BUFFER)!; } catch (System.Exception ex) { diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs index db0ff00bdcd7f5..e148e8ab724f4d 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs @@ -46,6 +46,8 @@ private readonly struct Configuration public delegate int ReadFromTargetDelegate(ulong address, Span bufferToFill); public delegate int WriteToTargetDelegate(ulong address, Span bufferToWrite); public delegate int GetTargetThreadContextDelegate(uint threadId, uint contextFlags, Span bufferToFill); + private readonly UTF8Encoding strictUTF8Encoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true); + private readonly UTF8Encoding looseUTF8Encoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: false); /// /// Create a new target instance from a contract descriptor embedded in the target memory. @@ -595,8 +597,9 @@ public void ReadPointers(ulong address, Span buffer) /// Read a null-terminated UTF-8 string from the target /// /// Address to start reading from + /// Whether to throw on invalid UTF-8 sequences. If false, invalid sequences will be replaced with the replacement character. /// String read from the target - public override string ReadUtf8String(ulong address) + public override string ReadUtf8String(ulong address, bool strict = false) { // Read characters until we find the null terminator ulong end = address; @@ -613,7 +616,7 @@ public override string ReadUtf8String(ulong address) ? stackalloc byte[length] : new byte[length]; ReadBuffer(address, span); - return Encoding.UTF8.GetString(span); + return strict ? strictUTF8Encoding.GetString(span) : looseUTF8Encoding.GetString(span); } /// diff --git a/src/native/managed/cdac/tests/LoaderTests.cs b/src/native/managed/cdac/tests/LoaderTests.cs index a6e30eb65e977d..9890a4e1500010 100644 --- a/src/native/managed/cdac/tests/LoaderTests.cs +++ b/src/native/managed/cdac/tests/LoaderTests.cs @@ -86,7 +86,7 @@ public void GetFileName(MockTarget.Architecture arch) [Theory] [ClassData(typeof(MockTarget.StdArch))] - public void GetSimpleName(MockTarget.Architecture arch) + public void TryGetSimpleName(MockTarget.Architecture arch) { // Set up the target TargetTestHelpers helpers = new(arch); @@ -108,12 +108,14 @@ public void GetSimpleName(MockTarget.Architecture arch) Assert.NotNull(contract); { Contracts.ModuleHandle handle = contract.GetModuleHandleFromModulePtr(moduleAddr); - string actual = contract.GetSimpleName(handle); + bool result = contract.TryGetSimpleName(handle, out string actual); + Assert.True(result); Assert.Equal(expected, actual); } { Contracts.ModuleHandle handle = contract.GetModuleHandleFromModulePtr(moduleAddrEmptyName); - string actual = contract.GetSimpleName(handle); + bool result = contract.TryGetSimpleName(handle, out string actual); + Assert.False(result); Assert.Equal(string.Empty, actual); } } diff --git a/src/native/managed/cdac/tests/TestPlaceholderTarget.cs b/src/native/managed/cdac/tests/TestPlaceholderTarget.cs index 7f3cb59f6ee0a9..2bc8789ea3cca1 100644 --- a/src/native/managed/cdac/tests/TestPlaceholderTarget.cs +++ b/src/native/managed/cdac/tests/TestPlaceholderTarget.cs @@ -159,7 +159,7 @@ public override void ReadBuffer(ulong address, Span buffer) } public override void WriteBuffer(ulong address, Span buffer) => throw new NotImplementedException(); - public override string ReadUtf8String(ulong address) + public override string ReadUtf8String(ulong address, bool strict = false) { // Read bytes until we find the null terminator ulong end = address; From 46a89fdad2e6ddee02cd659cf42f7e14d1031219 Mon Sep 17 00:00:00 2001 From: rcj1 Date: Mon, 23 Mar 2026 14:34:05 -0700 Subject: [PATCH 5/8] code review --- docs/design/datacontracts/Loader.md | 5 +++-- .../Target.cs | 2 +- .../ClrDataModule.cs | 14 ++++++++++---- .../ContractDescriptorTarget.cs | 4 ++-- 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/docs/design/datacontracts/Loader.md b/docs/design/datacontracts/Loader.md index f13c776ae30367..c6943aacd0c33f 100644 --- a/docs/design/datacontracts/Loader.md +++ b/docs/design/datacontracts/Loader.md @@ -581,9 +581,10 @@ bool TryGetSimpleName(ModuleHandle handle, out string simpleName) { TargetPointer simpleNameStart = target.ReadPointer(handle.Address + /* Module::SimpleName offset */); if (simpleNameStart == TargetPointer.Null) - return false - byte[] simpleName = // Read from target starting at simpleNameStart until null terminator + return false; + byte[] simpleNameBytes = // Read from target starting at simpleNameStart until null terminator simpleName = // convert to string, throw on invalid UTF-8 + return true; } string GetPath(ModuleHandle handle) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Target.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Target.cs index 027ce5c4522c2e..39bdaabcb53be9 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Target.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Target.cs @@ -105,7 +105,7 @@ public abstract class Target /// If true, throw if the string is not valid UTF-8. If false, replace invalid sequences with the replacement character. /// String read from the target /// Thrown when the read operation fails - public abstract string ReadUtf8String(ulong address, bool strict); + public abstract string ReadUtf8String(ulong address, bool strict = false); /// /// Read a null-terminated UTF-16 string from the target in target endianness diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs index beb3bb9cc3b136..dd1e0cf69d57bb 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs @@ -135,9 +135,12 @@ int IXCLRDataModule.GetName(uint bufLen, uint* nameLen, char* name) if (!loader.TryGetSimpleName(handle, out string result)) throw new ArgumentException("Module does not have a simple name"); - OutputBufferHelpers.CopyStringToBuffer(name, bufLen, nameLen, result); + uint nameLenLocal = 0; + OutputBufferHelpers.CopyStringToBuffer(name, bufLen, &nameLenLocal, result); + if (nameLen != null) + *nameLen = nameLenLocal; // throw on insufficient buffer - if (*nameLen > bufLen) + if (nameLenLocal > bufLen) throw Marshal.GetExceptionForHR(E_INSUFFICIENT_BUFFER)!; } catch (System.Exception ex) @@ -156,8 +159,11 @@ int IXCLRDataModule.GetName(uint bufLen, uint* nameLen, char* name) hrLocal = _legacyModule.GetName(bufLen, &nameLenLocal, ptr); } Debug.ValidateHResult(hr, hrLocal); - Debug.Assert(nameLen == null || *nameLen == nameLenLocal); - Debug.Assert(name == null || new ReadOnlySpan(nameLocal, 0, (int)nameLenLocal - 1).SequenceEqual(new string(name))); + if (hr == HResults.S_OK) + { + Debug.Assert(nameLen == null || *nameLen == nameLenLocal); + Debug.Assert(name == null || new ReadOnlySpan(nameLocal, 0, (int)nameLenLocal - 1).SequenceEqual(new string(name))); + } } #endif return hr; diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs index e148e8ab724f4d..71786f8d9a7d30 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs @@ -46,8 +46,8 @@ private readonly struct Configuration public delegate int ReadFromTargetDelegate(ulong address, Span bufferToFill); public delegate int WriteToTargetDelegate(ulong address, Span bufferToWrite); public delegate int GetTargetThreadContextDelegate(uint threadId, uint contextFlags, Span bufferToFill); - private readonly UTF8Encoding strictUTF8Encoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true); - private readonly UTF8Encoding looseUTF8Encoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: false); + private static readonly UTF8Encoding strictUTF8Encoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true); + private static readonly UTF8Encoding looseUTF8Encoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: false); /// /// Create a new target instance from a contract descriptor embedded in the target memory. From ba8945c2f0940c414715c1c944aacb7a95b3c22e Mon Sep 17 00:00:00 2001 From: Rachel Date: Mon, 23 Mar 2026 17:54:16 -0700 Subject: [PATCH 6/8] Update src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs Co-authored-by: Juan Hoyos <19413848+hoyosjs@users.noreply.github.com> --- .../Contracts/Loader_1.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs index ac726eeed7f6e5..1a80d73c95cc90 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs @@ -382,7 +382,7 @@ bool ILoader.TryGetSimpleName(ModuleHandle handle, out string simpleName) Data.Module module = _target.ProcessedData.GetOrAdd(handle.Address); if (module.SimpleName != TargetPointer.Null) { - simpleName = _target.ReadUtf8String(module.SimpleName); + simpleName = _target.ReadUtf8String(module.SimpleName, strict: true); return true; } else From 51cbee21c839c85a86003931a4c611e73f943a81 Mon Sep 17 00:00:00 2001 From: Rachel Date: Tue, 24 Mar 2026 07:04:07 -0700 Subject: [PATCH 7/8] Update TestPlaceholderTarget.cs --- src/native/managed/cdac/tests/TestPlaceholderTarget.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/native/managed/cdac/tests/TestPlaceholderTarget.cs b/src/native/managed/cdac/tests/TestPlaceholderTarget.cs index 2bc8789ea3cca1..2ba7c98187bffc 100644 --- a/src/native/managed/cdac/tests/TestPlaceholderTarget.cs +++ b/src/native/managed/cdac/tests/TestPlaceholderTarget.cs @@ -25,6 +25,8 @@ internal class TestPlaceholderTarget : Target internal delegate int ReadFromTargetDelegate(ulong address, Span buffer); private readonly ReadFromTargetDelegate _dataReader; + private static readonly UTF8Encoding strictUTF8Encoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true); + private static readonly UTF8Encoding looseUTF8Encoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: false); public TestPlaceholderTarget(MockTarget.Architecture arch, ReadFromTargetDelegate reader, Dictionary types = null, (string Name, ulong Value)[] globals = null, (string Name, string Value)[] globalStrings = null) { @@ -174,7 +176,7 @@ public override string ReadUtf8String(ulong address, bool strict = false) Span span = new byte[length]; ReadBuffer(address, span); - return Encoding.UTF8.GetString(span); + return strict ? strictUTF8Encoding.GetString(span) : looseUTF8Encoding.GetString(span); } public override string ReadUtf16String(ulong address) { From 18bed2f3ee5f98d6c06f6a9f1a8d829aaff39fc0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Mar 2026 14:58:25 +0000 Subject: [PATCH 8/8] test: add TryGetSimpleName_InvalidUtf8 unit test Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/0e43d4be-e4a7-4497-9940-81a4b3a67490 --- src/native/managed/cdac/tests/LoaderTests.cs | 24 +++++++++++++++++++ .../MockDescriptors/MockDescriptors.Loader.cs | 11 +++++---- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/native/managed/cdac/tests/LoaderTests.cs b/src/native/managed/cdac/tests/LoaderTests.cs index 9890a4e1500010..82b3fe8afc4491 100644 --- a/src/native/managed/cdac/tests/LoaderTests.cs +++ b/src/native/managed/cdac/tests/LoaderTests.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.IO; using System.Runtime.InteropServices; +using System.Text; using Microsoft.Diagnostics.DataContractReader.Contracts; using Microsoft.Diagnostics.DataContractReader.Legacy; using Moq; @@ -120,6 +121,29 @@ public void TryGetSimpleName(MockTarget.Architecture arch) } } + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void TryGetSimpleName_InvalidUtf8(MockTarget.Architecture arch) + { + // Set up the target + TargetTestHelpers helpers = new(arch); + MockMemorySpace.Builder builder = new(helpers); + MockLoader loader = new(builder); + + // 0xFF is not valid UTF-8 + byte[] invalidUtf8 = [0xFF, 0xFE]; + TargetPointer moduleAddr = loader.AddModule(simpleNameBytes: invalidUtf8); + + var target = new TestPlaceholderTarget(arch, builder.GetMemoryContext().ReadFromTarget, loader.Types); + target.SetContracts(Mock.Of( + c => c.Loader == ((IContractFactory)new LoaderFactory()).CreateContract(target, 1))); + + Contracts.ILoader contract = target.Contracts.Loader; + Assert.NotNull(contract); + Contracts.ModuleHandle handle = contract.GetModuleHandleFromModulePtr(moduleAddr); + Assert.Throws(() => contract.TryGetSimpleName(handle, out _)); + } + private static readonly Dictionary MockHeapDictionary = new() { ["LowFrequencyHeap"] = new(0x1000), diff --git a/src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Loader.cs b/src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Loader.cs index c2bc835417ab85..bc7d2de6920f05 100644 --- a/src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Loader.cs +++ b/src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Loader.cs @@ -41,7 +41,7 @@ public Loader(MockMemorySpace.Builder builder, (ulong Start, ulong End) allocati ]); } - internal TargetPointer AddModule(string? path = null, string? fileName = null, string? simpleName = null) + internal TargetPointer AddModule(string? path = null, string? fileName = null, string? simpleName = null, byte[]? simpleNameBytes = null) { TargetTestHelpers helpers = _builder.TargetTestHelpers; Target.TypeInfo typeInfo = Types[DataType.Module]; @@ -49,12 +49,13 @@ internal TargetPointer AddModule(string? path = null, string? fileName = null, s MockMemorySpace.HeapFragment module = _allocator.Allocate(size, "Module"); _builder.AddHeapFragment(module); - if (simpleName != null) + byte[]? rawSimpleName = simpleName is not null ? Encoding.UTF8.GetBytes(simpleName) : simpleNameBytes; + if (rawSimpleName != null) { // Simple name data (UTF-8, null-terminated) - ulong simpleNameSize = (ulong)Encoding.UTF8.GetByteCount(simpleName) + 1; - MockMemorySpace.HeapFragment simpleNameFragment = _allocator.Allocate(simpleNameSize, $"Module simple name = {simpleName}"); - Encoding.UTF8.GetBytes(simpleName).AsSpan().CopyTo(simpleNameFragment.Data); + ulong simpleNameSize = (ulong)rawSimpleName.Length + 1; + MockMemorySpace.HeapFragment simpleNameFragment = _allocator.Allocate(simpleNameSize, "Module simple name"); + rawSimpleName.AsSpan().CopyTo(simpleNameFragment.Data); simpleNameFragment.Data[^1] = 0; _builder.AddHeapFragment(simpleNameFragment);