[cDAC] Implement GetModuleSimpleName for cDAC

docs/design/datacontracts/Loader.md

@@ -80,7 +80,7 @@ IEnumerable<TargetPointer> GetInstantiatedMethods(ModuleHandle handle);
 bool IsProbeExtensionResultValid(ModuleHandle handle);
 ModuleFlags GetFlags(ModuleHandle handle);
 bool IsReadyToRun(ModuleHandle handle);
-bool TryGetSimpleName(ModuleHandle handle, out string simpleName);
+string GetSimpleName(ModuleHandle handle);
 string GetPath(ModuleHandle handle);
 string GetFileName(ModuleHandle handle);
 TargetPointer GetLoaderAllocator(ModuleHandle handle);
@@ -622,14 +622,11 @@ ModuleFlags GetFlags(ModuleHandle handle)
     return GetFlags(target.Read<uint>(handle.Address + /* Module::Flags offset */));
 }
 
-bool TryGetSimpleName(ModuleHandle handle, out string simpleName)
+string GetSimpleName(ModuleHandle handle)
 {
     TargetPointer simpleNameStart = target.ReadPointer(handle.Address + /* Module::SimpleName offset */);
-    if (simpleNameStart == TargetPointer.Null)
-        return false;
     byte[] simpleNameBytes = // Read<byte> from target starting at simpleNameStart until null terminator
-    simpleName = // convert to string, throw on invalid UTF-8
-    return true;
+    return // convert to string, throw on invalid UTF-8
 }
 
 string GetPath(ModuleHandle handle)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ILoader.cs

@@ -103,7 +103,7 @@ public interface ILoader : IContract
     bool IsProbeExtensionResultValid(ModuleHandle handle) => throw new NotImplementedException();
     ModuleFlags GetFlags(ModuleHandle handle) => throw new NotImplementedException();
     bool IsReadyToRun(ModuleHandle handle) => throw new NotImplementedException();
-    bool TryGetSimpleName(ModuleHandle handle, out string simpleName) => 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();

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs

@@ -447,17 +447,12 @@ bool ILoader.IsReadyToRun(ModuleHandle handle)
         return module.ReadyToRunInfo != TargetPointer.Null;
     }
 
-    bool ILoader.TryGetSimpleName(ModuleHandle handle, out string simpleName)
+    string ILoader.GetSimpleName(ModuleHandle handle)
     {
-        simpleName = string.Empty;
         Data.Module module = _target.ProcessedData.GetOrAdd<Data.Module>(handle.Address);
-        if (module.SimpleName != TargetPointer.Null)
-        {
-            simpleName = _target.ReadUtf8String(module.SimpleName, strict: true);
-            return true;
-        }
-        else
-            return false;
+        return module.SimpleName != TargetPointer.Null
+            ? _target.ReadUtf8String(module.SimpleName, strict: true)
+            : string.Empty;
     }
 
     string ILoader.GetPath(ModuleHandle handle)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs

@@ -460,8 +460,7 @@ int IXCLRDataModule.GetName(uint bufLen, uint* nameLen, char* name)
                 *nameLen = 0;
             Contracts.ILoader loader = _target.Contracts.Loader;
             Contracts.ModuleHandle handle = loader.GetModuleHandleFromModulePtr(_address);
-            if (!loader.TryGetSimpleName(handle, out string result))
-                throw new ArgumentException("Module does not have a simple name");
+            string result = loader.GetSimpleName(handle);
 
             uint nameLenLocal = 0;
             OutputBufferHelpers.CopyStringToBuffer(name, bufLen, &nameLenLocal, result);

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs

@@ -145,7 +145,36 @@ public int GetAppDomainFullName(ulong vmAppDomain, nint pStrName)
     }
 
     public int GetModuleSimpleName(ulong vmModule, nint pStrFilename)
-        => LegacyFallbackHelper.CanFallback() && _legacy is not null ? _legacy.GetModuleSimpleName(vmModule, pStrFilename) : HResults.E_NOTIMPL;
+    {
+        int hr = HResults.S_OK;
+        string? cdacSimpleName = null;
+        try
+        {
+            Contracts.ILoader loader = _target.Contracts.Loader;
+            Contracts.ModuleHandle handle = loader.GetModuleHandleFromModulePtr(new TargetPointer(vmModule));
+            cdacSimpleName = loader.GetSimpleName(handle);
+            hr = StringHolderAssignCopy(pStrFilename, cdacSimpleName);
+        }
+        catch (System.Exception ex)
+        {
+            hr = ex.HResult;
+        }
+#if DEBUG
+        if (_legacy is not null)
+        {
+            using var legacyHolder = new NativeStringHolder();
+            int hrLocal = _legacy.GetModuleSimpleName(vmModule, legacyHolder.Ptr);
+            Debug.ValidateHResult(hr, hrLocal);
+            if (hr == HResults.S_OK)
+            {
+                Debug.Assert(
+                    string.Equals(cdacSimpleName, legacyHolder.Value, System.StringComparison.Ordinal),
+                    $"GetModuleSimpleName string mismatch - cDAC: '{cdacSimpleName}', DAC: '{legacyHolder.Value}'");
+            }
+        }
+#endif
+        return hr;
+    }
 
     public int GetAssemblyPath(ulong vmAssembly, nint pStrFilename, Interop.BOOL* pResult)
     {

src/native/managed/cdac/tests/DumpTests/DacDbi/NativeStringHolder.cs → src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/NativeStringHolder.cs

@@ -4,7 +4,7 @@
 using System;
 using System.Runtime.InteropServices;
 
-namespace Microsoft.Diagnostics.DataContractReader.DumpTests;
+namespace Microsoft.Diagnostics.DataContractReader.Legacy;
 
 /// <summary>
 /// Creates a native-memory object that mimics the C++ IStringHolder vtable layout.
@@ -52,7 +52,7 @@ public NativeStringHolder()
     private int AssignCopyImpl(IntPtr thisPtr, IntPtr psz)
     {
         Value = Marshal.PtrToStringUni(psz);
-        return System.HResults.S_OK;
+        return HResults.S_OK;
     }
 
     public void Dispose()

src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiLoaderDumpTests.cs

@@ -10,11 +10,11 @@ namespace Microsoft.Diagnostics.DataContractReader.DumpTests;
 
 /// <summary>
 /// Dump-based integration tests for DacDbiImpl loader, assembly, and module methods.
-/// Uses the MultiModule debuggee (full dump), which loads assemblies from multiple ALCs.
+/// Uses the StackRefs debuggee (full dump).
 /// </summary>
 public class DacDbiLoaderDumpTests : DumpTestBase
 {
-    protected override string DebuggeeName => "MultiModule";
+    protected override string DebuggeeName => "StackRefs";
     private DacDbiImpl CreateDacDbi() => new DacDbiImpl(Target, legacyObj: null);
 
     private IEnumerable<ModuleHandle> GetAllModules()
@@ -137,6 +137,30 @@ public unsafe void GetModuleData_ReturnsValidFields(TestConfiguration config)
         Assert.True(testedAtLeastOne, "Expected at least one module in the dump");
     }
 
+    [ConditionalTheory]
+    [MemberData(nameof(TestConfigurations))]
+    public void GetModuleSimpleName_ReturnsNonEmpty(TestConfiguration config)
+    {
+        InitializeDumpTest(config);
+        DacDbiImpl dbi = CreateDacDbi();
+        ILoader loader = Target.Contracts.Loader;
+
+        bool testedAtLeastOne = false;
+        foreach (ModuleHandle module in GetAllModules())
+        {
+            TargetPointer moduleAddr = loader.GetModule(module);
+
+            using var holder = new NativeStringHolder();
+            int hr = dbi.GetModuleSimpleName(moduleAddr.Value, holder.Ptr);
+            Assert.Equal(System.HResults.S_OK, hr);
+            Assert.False(string.IsNullOrEmpty(holder.Value), "Module simple name should not be empty");
+            Assert.Equal(loader.GetSimpleName(module), holder.Value);
+
+            testedAtLeastOne = true;
+        }
+        Assert.True(testedAtLeastOne, "Expected at least one module in the dump");
+    }
+
     [ConditionalTheory]
     [MemberData(nameof(TestConfigurations))]
     public unsafe void IsModuleMapped_ReturnsValidResult(TestConfiguration config)

src/native/managed/cdac/tests/LoaderTests.cs

@@ -112,35 +112,40 @@ public void GetFileName(MockTarget.Architecture arch)
 
     [Theory]
     [ClassData(typeof(MockTarget.StdArch))]
-    public void TryGetSimpleName(MockTarget.Architecture arch)
+    public void GetSimpleName(MockTarget.Architecture arch)
     {
         string expected = "TestModule";
         TargetPointer moduleAddr = TargetPointer.Null;
-        TargetPointer moduleAddrEmptyName = TargetPointer.Null;
 
         ILoader contract = CreateLoaderContract(arch, loader =>
         {
             moduleAddr = loader.AddModule(simpleName: expected).Address;
-            moduleAddrEmptyName = loader.AddModule().Address;
         });
 
+        Contracts.ModuleHandle handle = contract.GetModuleHandleFromModulePtr(moduleAddr);
+        string actual = contract.GetSimpleName(handle);
+        Assert.Equal(expected, actual);
+    }
+
+    [Theory]
+    [ClassData(typeof(MockTarget.StdArch))]
+    public void GetSimpleName_NullSimpleName(MockTarget.Architecture arch)
+    {
+        TargetPointer moduleAddr = TargetPointer.Null;
+
+        ILoader contract = CreateLoaderContract(arch, loader =>
         {
-            Contracts.ModuleHandle handle = contract.GetModuleHandleFromModulePtr(moduleAddr);
-            bool result = contract.TryGetSimpleName(handle, out string actual);
-            Assert.True(result);
-            Assert.Equal(expected, actual);
-        }
-        {
-            Contracts.ModuleHandle handle = contract.GetModuleHandleFromModulePtr(moduleAddrEmptyName);
-            bool result = contract.TryGetSimpleName(handle, out string actual);
-            Assert.False(result);
-            Assert.Equal(string.Empty, actual);
-        }
+            moduleAddr = loader.AddModule().Address;
+        });
+
+        Contracts.ModuleHandle handle = contract.GetModuleHandleFromModulePtr(moduleAddr);
+        string actual = contract.GetSimpleName(handle);
+        Assert.Equal(string.Empty, actual);
     }
 
     [Theory]
     [ClassData(typeof(MockTarget.StdArch))]
-    public void TryGetSimpleName_InvalidUtf8(MockTarget.Architecture arch)
+    public void GetSimpleName_InvalidUtf8(MockTarget.Architecture arch)
     {
         // 0xFF is not valid UTF-8
         byte[] invalidUtf8 = [0xFF, 0xFE];
@@ -151,7 +156,7 @@ public void TryGetSimpleName_InvalidUtf8(MockTarget.Architecture arch)
         });
 
         Contracts.ModuleHandle handle = contract.GetModuleHandleFromModulePtr(moduleAddr);
-        Assert.Throws<DecoderFallbackException>(() => contract.TryGetSimpleName(handle, out _));
+        Assert.Throws<DecoderFallbackException>(() => contract.GetSimpleName(handle));
     }
 
     private static readonly Dictionary<string, TargetPointer> MockHeapDictionary = new()