[cDAC] Consolidate metadata contract APIs

docs/design/datacontracts/EcmaMetadata.md

@@ -5,10 +5,14 @@ This contract provides methods to get a view of the ECMA-335 metadata for a give
 ## APIs of contract
 
 ```csharp
-TargetSpan GetReadOnlyMetadataAddress(ModuleHandle handle);
-TargetSpan GetReadWriteSavedMetadataAddress(ModuleHandle handle);
-System.Reflection.Metadata.MetadataReader? GetMetadata(ModuleHandle handle);
-byte[] GetReadWriteMetadata(ModuleHandle handle);
+enum MetadataAddressKind
+{
+    ReadOnly,
+    ReadWriteSavedCopy,
+}
+
+TargetSpan GetMetadataAddress(ModuleHandle handle, MetadataAddressKind kind);
+System.Reflection.Metadata.MetadataReader? GetMetadata(ModuleHandle handle, bool requireReadWriteMetadata = false);
 ```
 
 Types from other contracts:
@@ -71,8 +75,16 @@ using System.IO;
 using System.Reflection.Metadata;
 using System.Runtime.InteropServices;
 
-TargetSpan GetReadOnlyMetadataAddress(ModuleHandle handle)
+TargetSpan GetMetadataAddress(ModuleHandle handle, MetadataAddressKind kind)
 {
+    if (kind == MetadataAddressKind.ReadWriteSavedCopy)
+    {
+        TargetPointer dynamicMetadata = Target.ReadPointer(handle.Address + /* Module::DynamicMetadata offset */);
+        ulong size = Target.Read<uint>(dynamicMetadata + /* DynamicMetadata::Size offset */);
+        TargetPointer result = dynamicMetadata + /* DynamicMetadata::Data offset */;
+        return new(result, size);
+    }
+
     TargetPointer baseAddress = Target.ReadPointer(handle.Address + /* Module::Base offset */);
     if (baseAddress == TargetPointer.Null)
     {
@@ -95,33 +107,35 @@ TargetSpan GetReadOnlyMetadataAddress(ModuleHandle handle)
     return new(baseAddress + rva, size);
 }
 
-MetadataReader? GetMetadata(ModuleHandle handle)
+MetadataReader? GetMetadata(ModuleHandle handle, bool requireReadWriteMetadata = false)
 {
     AvailableMetadataType type = GetAvailableMetadataType(handle);
+    if (requireReadWriteMetadata && type != AvailableMetadataType.ReadWrite)
+    {
+        throw new ArgumentException();
+    }
 
     switch (type)
     {
         case AvailableMetadataType.None:
             return null;
         case AvailableMetadataType.ReadOnly:
         {
-            TargetSpan address = GetReadOnlyMetadataAddress(handle);
+            TargetSpan address = GetMetadataAddress(handle, MetadataAddressKind.ReadOnly);
             byte[] data = new byte[address.Size];
             _target.ReadBuffer(address.Address, data);
             return MetadataReaderProvider.FromMetadataImage(ImmutableCollectionsMarshal.AsImmutableArray(data)).GetMetadataReader();
         }
         case AvailableMetadataType.ReadWriteSavedCopy:
         {
-            TargetSpan address = GetReadWriteSavedMetadataAddress(handle);
+            TargetSpan address = GetMetadataAddress(handle, MetadataAddressKind.ReadWriteSavedCopy);
             byte[] data = new byte[address.Size];
             _target.ReadBuffer(address.Address, data);
             return MetadataReaderProvider.FromMetadataImage(ImmutableCollectionsMarshal.AsImmutableArray(data)).GetMetadataReader();
         }
         case AvailableMetadataType.ReadWrite:
         {
-            // Reconstruct a contiguous ECMA-335 image from the module's writable
-            // (MDInternalRW) metadata and return a reader over it.
-            byte[] data = GetReadWriteMetadata(handle);
+            byte[] data = GetReadWriteMetadataBlob(handle);
             return MetadataReaderProvider.FromMetadataImage(ImmutableCollectionsMarshal.AsImmutableArray(data)).GetMetadataReader();
         }
     }
@@ -130,7 +144,7 @@ MetadataReader? GetMetadata(ModuleHandle handle)
 // Reconstructs the module's writable (MDInternalRW) metadata as a single contiguous
 // ECMA-335 metadata image. The result is cached per module and reused until the
 // module's metadata generation counter (Module::MetadataGeneration) changes.
-byte[] GetReadWriteMetadata(ModuleHandle handle)
+byte[] GetReadWriteMetadataBlob(ModuleHandle handle)
 {
     // If a blob was previously built for this handle and the module's metadata
     // generation counter is unchanged, return the cached blob.
@@ -223,11 +237,4 @@ AvailableMetadataType GetAvailableMetadataType(ModuleHandle handle)
     return flags;
 }
 
-TargetSpan GetReadWriteSavedMetadataAddress(ModuleHandle handle)
-{
-    TargetPointer dynamicMetadata = Target.ReadPointer(handle.Address + /* Module::DynamicMetadata offset */);
-    ulong size = Target.Read<uint>(handle.Address + /* DynamicMetadata::Size offset */);
-    TargetPointer result = handle.Address + /* DynamicMetadata::Data offset */;
-    return new(result, size);
-}
 ```

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

@@ -6,14 +6,17 @@
 
 namespace Microsoft.Diagnostics.DataContractReader.Contracts;
 
+public enum MetadataAddressKind
+{
+    ReadOnly,
+    ReadWriteSavedCopy,
+}
+
 public interface IEcmaMetadata : IContract
 {
     static string IContract.Name { get; } = nameof(EcmaMetadata);
-    TargetSpan GetReadOnlyMetadataAddress(ModuleHandle handle) => throw new NotImplementedException();
-    TargetSpan GetReadWriteSavedMetadataAddress(ModuleHandle handle) => throw new NotImplementedException();
-
-    MetadataReader? GetMetadata(ModuleHandle module) => throw new NotImplementedException();
-    byte[] GetReadWriteMetadata(ModuleHandle handle) => throw new NotImplementedException();
+    TargetSpan GetMetadataAddress(ModuleHandle handle, MetadataAddressKind kind) => throw new NotImplementedException();
+    MetadataReader? GetMetadata(ModuleHandle handle, bool requireReadWriteMetadata = false) => throw new NotImplementedException();
 }
 
 public readonly struct EcmaMetadata : IEcmaMetadata

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

@@ -31,8 +31,17 @@ public void Flush(FlushScope scope)
         }
     }
 
-    public TargetSpan GetReadOnlyMetadataAddress(ModuleHandle handle)
+    public TargetSpan GetMetadataAddress(ModuleHandle handle, MetadataAddressKind kind)
     {
+        if (kind == MetadataAddressKind.ReadWriteSavedCopy)
+        {
+            Data.Module module = target.ProcessedData.GetOrAdd<Data.Module>(handle.Address);
+            Data.DynamicMetadata dynamicMetadata = target.ProcessedData.GetOrAdd<Data.DynamicMetadata>(module.DynamicMetadata);
+            return new TargetSpan(dynamicMetadata.Data, dynamicMetadata.Size);
+        }
+        if (kind != MetadataAddressKind.ReadOnly)
+            throw new ArgumentOutOfRangeException(nameof(kind));
+
         if (_readOnlyMetadataAddress.TryGetValue(handle, out TargetSpan cached))
             return cached;
 
@@ -94,8 +103,13 @@ private TargetSpan GetWebcilReadOnlyMetadataAddress(ModuleHandle handle, TargetP
         return new TargetSpan(metadataAddress, metadataDirectory.Size);
     }
 
-    public MetadataReader? GetMetadata(ModuleHandle handle)
+    public MetadataReader? GetMetadata(ModuleHandle handle, bool requireReadWriteMetadata = false)
     {
+        if (requireReadWriteMetadata && GetAvailableMetadataType(handle) != AvailableMetadataType.ReadWrite)
+        {
+            throw new ArgumentException("Module does not have read/write metadata.", nameof(handle));
+        }
+
         uint generation = GetMetadataGeneration(handle);
 
         if (_metadata.TryGetValue(handle, out (uint Generation, MetadataReaderProvider? Provider) cached))
@@ -121,30 +135,28 @@ private TargetSpan GetWebcilReadOnlyMetadataAddress(ModuleHandle handle, TargetP
             case AvailableMetadataType.None:
                 return null;
             case AvailableMetadataType.ReadOnly:
-            {
-                TargetSpan address = GetReadOnlyMetadataAddress(handle);
-                byte[] data = new byte[address.Size];
-                target.ReadBuffer(address.Address, data);
-                return MetadataReaderProvider.FromMetadataImage(ImmutableCollectionsMarshal.AsImmutableArray(data));
-            }
+                return GetMetadataProviderFromAddress(handle, MetadataAddressKind.ReadOnly);
             case AvailableMetadataType.ReadWriteSavedCopy:
-            {
-                TargetSpan address = GetReadWriteSavedMetadataAddress(handle);
-                byte[] data = new byte[address.Size];
-                target.ReadBuffer(address.Address, data);
-                return MetadataReaderProvider.FromMetadataImage(ImmutableCollectionsMarshal.AsImmutableArray(data));
-            }
+                return GetMetadataProviderFromAddress(handle, MetadataAddressKind.ReadWriteSavedCopy);
             case AvailableMetadataType.ReadWrite:
             {
-                byte[] data = GetReadWriteMetadata(handle);
+                byte[] data = GetReadWriteMetadataBlob(handle);
                 return MetadataReaderProvider.FromMetadataImage(ImmutableCollectionsMarshal.AsImmutableArray(data));
             }
             default:
                 throw new NotImplementedException();
         }
     }
 
-    public byte[] GetReadWriteMetadata(ModuleHandle handle)
+    private MetadataReaderProvider GetMetadataProviderFromAddress(ModuleHandle handle, MetadataAddressKind kind)
+    {
+        TargetSpan address = GetMetadataAddress(handle, kind);
+        byte[] data = new byte[address.Size];
+        target.ReadBuffer(address.Address, data);
+        return MetadataReaderProvider.FromMetadataImage(ImmutableCollectionsMarshal.AsImmutableArray(data));
+    }
+
+    private byte[] GetReadWriteMetadataBlob(ModuleHandle handle)
     {
         if (GetAvailableMetadataType(handle) != AvailableMetadataType.ReadWrite)
         {
@@ -413,14 +425,6 @@ private uint GetMetadataGeneration(ModuleHandle handle)
         return module.MetadataGeneration;
     }
 
-    public TargetSpan GetReadWriteSavedMetadataAddress(ModuleHandle handle)
-    {
-        Data.Module module = target.ProcessedData.GetOrAdd<Data.Module>(handle.Address);
-        Data.DynamicMetadata dynamicMetadata = target.ProcessedData.GetOrAdd<Data.DynamicMetadata>(module.DynamicMetadata);
-
-        return new TargetSpan(dynamicMetadata.Data, dynamicMetadata.Size);
-    }
-
     private TargetEcmaMetadata GetTargetEcmaMetadata(ModuleHandle handle)
     {
         TargetPointer peAssemblyPtr = target.Contracts.Loader.GetPEAssembly(handle);

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

@@ -360,10 +360,10 @@ public int GetMetadata(ulong vmModule, DacDbiTargetBuffer* pTargetBuffer)
             Contracts.ModuleFlags flags = loader.GetFlags(handle);
             Contracts.IEcmaMetadata ecmaMetadata = _target.Contracts.EcmaMetadata;
 
-            // Dynamic modules keep an eagerly-serialized metadata buffer, while non-dynamic modules read metadata from the loaded PE image.
-            TargetSpan targetSpan = flags.HasFlag(Contracts.ModuleFlags.ReflectionEmit)
-                ? ecmaMetadata.GetReadWriteSavedMetadataAddress(handle)
-                : ecmaMetadata.GetReadOnlyMetadataAddress(handle);
+            MetadataAddressKind kind = flags.HasFlag(Contracts.ModuleFlags.ReflectionEmit)
+                ? MetadataAddressKind.ReadWriteSavedCopy
+                : MetadataAddressKind.ReadOnly;
+            TargetSpan targetSpan = ecmaMetadata.GetMetadataAddress(handle, kind);
 
             pTargetBuffer->pAddress = targetSpan.Address.Value;
             pTargetBuffer->cbSize = checked((uint)targetSpan.Size);
@@ -407,8 +407,8 @@ public int GetReadWriteMetadataSize(ulong vmModule, uint* pSize)
             Contracts.ILoader loader = _target.Contracts.Loader;
             Contracts.ModuleHandle handle = loader.GetModuleHandleFromModulePtr(new TargetPointer(vmModule));
 
-            byte[] blob = _target.Contracts.EcmaMetadata.GetReadWriteMetadata(handle);
-            *pSize = (uint)blob.Length;
+            MetadataReader reader = _target.Contracts.EcmaMetadata.GetMetadata(handle, requireReadWriteMetadata: true)!;
+            *pSize = checked((uint)reader.MetadataLength);
         }
         catch (System.Exception ex)
         {
@@ -443,12 +443,12 @@ public int FillReadWriteMetadata(ulong vmModule, byte* pBuffer, uint cbBuffer)
             Contracts.ILoader loader = _target.Contracts.Loader;
             Contracts.ModuleHandle handle = loader.GetModuleHandleFromModulePtr(new TargetPointer(vmModule));
 
-            byte[] blob = _target.Contracts.EcmaMetadata.GetReadWriteMetadata(handle);
-            blobLength = blob.Length;
-            if (cbBuffer < (uint)blob.Length)
+            MetadataReader reader = _target.Contracts.EcmaMetadata.GetMetadata(handle, requireReadWriteMetadata: true)!;
+            blobLength = reader.MetadataLength;
+            if (cbBuffer < (uint)blobLength)
                 throw Marshal.GetExceptionForHR(CorDbgHResults.ERROR_INSUFFICIENT_BUFFER)!;
 
-            blob.AsSpan().CopyTo(new Span<byte>(pBuffer, blobLength));
+            new ReadOnlySpan<byte>(reader.MetadataPointer, blobLength).CopyTo(new Span<byte>(pBuffer, blobLength));
         }
         catch (System.Exception ex)
         {

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

@@ -3136,7 +3136,7 @@ int ISOSDacInterface.GetModuleData(ClrDataAddress moduleAddr, DacpModuleData* da
 
             try
             {
-                TargetSpan readOnlyMetadata = _target.Contracts.EcmaMetadata.GetReadOnlyMetadataAddress(handle);
+                TargetSpan readOnlyMetadata = _target.Contracts.EcmaMetadata.GetMetadataAddress(handle, MetadataAddressKind.ReadOnly);
                 data->metadataStart = readOnlyMetadata.Address.Value;
                 data->metadataSize = readOnlyMetadata.Size;
             }

src/native/managed/cdac/tests/DumpTests/EcmaMetadataDumpTests.cs

@@ -28,7 +28,7 @@ public void EcmaMetadata_RootModuleHasMetadataAddress(TestConfiguration config)
         TargetPointer rootAssembly = loader.GetRootAssembly();
         ModuleHandle moduleHandle = loader.GetModuleHandleFromAssemblyPtr(rootAssembly);
 
-        TargetSpan metadataSpan = ecmaMetadata.GetReadOnlyMetadataAddress(moduleHandle);
+        TargetSpan metadataSpan = ecmaMetadata.GetMetadataAddress(moduleHandle, MetadataAddressKind.ReadOnly);
         Assert.NotEqual(TargetPointer.Null, metadataSpan.Address);
         Assert.True(metadataSpan.Size > 0, "Expected metadata size > 0");
     }