Mark PEImage internal

ClrMD is getting out of the business of file format reading. :) While it's useful to have a bunch of file reading abilities hanging around, there are simply better libraries for reading PE, Elf, and Mach-O images. The intent here is still read images enough to perform ClrMD's operations, but we will: Drop our dependency on System.Reflection.Runtime (unfortunately it doesn't react well when there's missing chunks of PE files and it's causing problems with Single File executables). Mark all file readers as internal and slim them down to just what ClrMD needs. Provide a new way to open Stream objects for files loaded into memory. These Streams will be formatted as if they were files on disk (not mapped into memory), meaning you can pass these files off to libraries like System.Reflection.Metadata or Marklio.Metadata and get fully featured file format readers without relying on ClrMD's buggy implementation. In effect, you may have to pick up a new dependency, but the end result will be better than where we are today. This is only a small piece of this overhaul.
microsoft · leculver · May 14, 2022 · May 12, 2022 · May 12, 2022 · May 13, 2022
commit b65b75f7070808d93e340b178445ae9d2f0a8ca8
diff --git a/src/Microsoft.Diagnostics.Runtime/Microsoft.Diagnostics.Runtime.csproj b/src/Microsoft.Diagnostics.Runtime/Microsoft.Diagnostics.Runtime.csproj
@@ -31,7 +31,6 @@
     <PackageReference Include="System.Buffers" Version="4.5.1" />
     <PackageReference Include="System.Collections.Immutable" Version="5.0.0" />
     <PackageReference Include="System.Memory" Version="4.5.4" />
-    <PackageReference Include="System.Reflection.Metadata" Version="5.0.0" />
     <PackageReference Include="System.Runtime.CompilerServices.Unsafe" Version="5.0.0" />
   </ItemGroup>
 

diff --git a/src/Microsoft.Diagnostics.Runtime/src/Common/ModuleInfo.cs b/src/Microsoft.Diagnostics.Runtime/src/Common/ModuleInfo.cs
@@ -57,34 +57,32 @@ public DataTarget DataTarget
         /// If the PEImage cannot be constructed, <see langword="null"/> is returned.
         /// </summary>
         /// <returns></returns>
-        public PEImage? GetPEImage()
+        internal PEImage? GetPEImage()
         {
             try
             {
                 PEImage image = new PEImage(new ReadVirtualStream(DataReader, (long)ImageBase, IndexFileSize), leaveOpen: false, isVirtual: _isVirtual);
-                if (image.PEHeader == null)
+                if (!image.IsValid)
                 {
-                    PEImage otherLayout = new PEImage(new ReadVirtualStream(DataReader, (long)ImageBase, IndexFileSize), leaveOpen: false, isVirtual: !_isVirtual);
-                    if (otherLayout.PEHeader != null)
-                    {
-                        image.Dispose();
-                        image = otherLayout;
-                    }
-                    else
-                    {
-                        otherLayout.Dispose();
-                    }
+                    image.Dispose();
+                    image = new PEImage(new ReadVirtualStream(DataReader, (long)ImageBase, IndexFileSize), leaveOpen: false, isVirtual: !_isVirtual);
                 }
 
-                if (!_isManaged.HasValue)
-                    _isManaged = image.IsManaged;
+                if (image.IsValid)
+                {
+                    if (!_isManaged.HasValue)
+                        _isManaged = image.IsManaged;
 
-                return image.IsValid ? image : null;
+                    return image;
+                }
+
+                image.Dispose();
             }
             catch
             {
-                return null;
             }
+
+            return null;
         }
 
         /// <summary>

diff --git a/src/Microsoft.Diagnostics.Runtime/src/DacInterface/DacDataTargetWrapper.cs b/src/Microsoft.Diagnostics.Runtime/src/DacInterface/DacDataTargetWrapper.cs
@@ -6,7 +6,6 @@
 using System.Diagnostics;
 using System.IO;
 using System.Linq;
-using System.Reflection.PortableExecutable;
 using System.Runtime.InteropServices;
 using System.Threading;
 using Microsoft.Diagnostics.Runtime.DbgEng;
@@ -254,32 +253,23 @@ public unsafe HResult GetMetadata(
             if (peimage is null)
                 return HResult.E_FAIL;
 
-            lock (peimage)
+            int rva = (int)mdRva;
+            int size = (int)bufferSize;
+            if (rva == 0)
             {
-                CorHeader? corHeader = peimage.CorHeader;
-                if (corHeader is null)
+                ImageDataDirectory metadata = peimage.MetadataDirectory;
+                if (metadata.VirtualAddress == 0)
                     return HResult.E_FAIL;
 
-                DebugOnly.Assert(peimage.IsValid);
-
-                int rva = (int)mdRva;
-                int size = (int)bufferSize;
-                if (rva == 0)
-                {
-                    DirectoryEntry metadata = corHeader.MetadataDirectory;
-                    if (metadata.RelativeVirtualAddress == 0)
-                        return HResult.E_FAIL;
-
-                    rva = metadata.RelativeVirtualAddress;
-                    size = Math.Min(size, metadata.Size);
-                }
+                rva = metadata.VirtualAddress;
+                size = Math.Min(size, (int)metadata.Size);
+            }
 
-                checked
-                {
-                    int read = peimage.Read(rva, new Span<byte>(buffer.ToPointer(), size));
-                    if (pDataSize != null)
-                        *pDataSize = read;
-                }
+            checked
+            {
+                int read = peimage.Read(rva, new Span<byte>(buffer.ToPointer(), size));
+                if (pDataSize != null)
+                    *pDataSize = read;
             }
 
             return HResult.S_OK;

diff --git a/src/Microsoft.Diagnostics.Runtime/src/Utilities/PEImage/CvInfoPdb70.cs b/src/Microsoft.Diagnostics.Runtime/src/Utilities/PEImage/CvInfoPdb70.cs
@@ -0,0 +1,27 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.Runtime.InteropServices;
+
+namespace Microsoft.Diagnostics.Runtime.Utilities
+{
+    internal unsafe struct CvInfoPdb70
+    {
+        public const int PDB70CvSignature = 0x53445352; // RSDS in ascii
+
+        public int CvSignature;
+        public Guid Signature;
+        public int Age;
+        public fixed byte bytePdbFileName[1]; // Actually variable sized.
+        public string PdbFileName
+        {
+            get
+            {
+                fixed (byte* ptr = bytePdbFileName)
+                    return Marshal.PtrToStringAnsi((IntPtr)ptr)!;
+            }
+        }
+    }
+}
diff --git a/src/Microsoft.Diagnostics.Runtime/src/Utilities/PEImage/ImageCor20Header.cs b/src/Microsoft.Diagnostics.Runtime/src/Utilities/PEImage/ImageCor20Header.cs
@@ -0,0 +1,44 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Runtime.InteropServices;
+
+namespace Microsoft.Diagnostics.Runtime.Utilities
+{
+    [StructLayout(LayoutKind.Sequential)]
+    public struct ImageCor20Header
+    {
+        // Header versioning
+        public uint cb;
+        public ushort MajorRuntimeVersion;
+        public ushort MinorRuntimeVersion;
+
+        // Symbol table and startup information
+        public ImageDataDirectory MetaData;
+        public uint Flags;
+
+        // The main program if it is an EXE (not used if a DLL?)
+        // If COMIMAGE_FLAGS_NATIVE_ENTRYPOINT is not set, EntryPointToken represents a managed entrypoint.
+        // If COMIMAGE_FLAGS_NATIVE_ENTRYPOINT is set, EntryPointRVA represents an RVA to a native entrypoint
+        // (depricated for DLLs, use modules constructors intead).
+        public ImageCor20HeaderEntrypoint EntryPoint;
+
+        // This is the blob of managed resources. Fetched using code:AssemblyNative.GetResource and
+        // code:PEFile.GetResource and accessible from managed code from
+        // System.Assembly.GetManifestResourceStream.  The meta data has a table that maps names to offsets into
+        // this blob, so logically the blob is a set of resources.
+        public ImageDataDirectory Resources;
+        // IL assemblies can be signed with a public-private key to validate who created it.  The signature goes
+        // here if this feature is used.
+        public ImageDataDirectory StrongNameSignature;
+
+        public ImageDataDirectory CodeManagerTable; // Depricated, not used
+                                                      // Used for manged codee that has unmaanaged code inside it (or exports methods as unmanaged entry points)
+        public ImageDataDirectory VTableFixups;
+        public ImageDataDirectory ExportAddressTableJumps;
+
+        // null for ordinary IL images.  NGEN images it points at a code:CORCOMPILE_HEADER structure
+        public ImageDataDirectory ManagedNativeHeader;
+    }
+}
diff --git a/src/Microsoft.Diagnostics.Runtime/src/Utilities/PEImage/ImageCor20HeaderEntrypoint.cs b/src/Microsoft.Diagnostics.Runtime/src/Utilities/PEImage/ImageCor20HeaderEntrypoint.cs
@@ -0,0 +1,17 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Runtime.InteropServices;
+
+namespace Microsoft.Diagnostics.Runtime.Utilities
+{
+    [StructLayout(LayoutKind.Explicit)]
+    public readonly struct ImageCor20HeaderEntrypoint
+    {
+        [FieldOffset(0)]
+        public readonly uint Token;
+        [FieldOffset(0)]
+        public readonly uint RVA;
+    }
+}
diff --git a/src/Microsoft.Diagnostics.Runtime/src/Utilities/PEImage/ImageDataDirectory.cs b/src/Microsoft.Diagnostics.Runtime/src/Utilities/PEImage/ImageDataDirectory.cs
@@ -0,0 +1,15 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Runtime.InteropServices;
+
+namespace Microsoft.Diagnostics.Runtime.Utilities
+{
+    [StructLayout(LayoutKind.Sequential)]
+    public struct ImageDataDirectory
+    {
+        public int VirtualAddress;
+        public int Size;
+    }
+}
diff --git a/src/Microsoft.Diagnostics.Runtime/src/Utilities/PEImage/ImageDebugDirectory.cs b/src/Microsoft.Diagnostics.Runtime/src/Utilities/PEImage/ImageDebugDirectory.cs
@@ -0,0 +1,18 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+namespace Microsoft.Diagnostics.Runtime.Utilities
+{
+    internal struct ImageDebugDirectory
+    {
+        public int Characteristics;
+        public int TimeDateStamp;
+        public short MajorVersion;
+        public short MinorVersion;
+        public ImageDebugType Type;
+        public int SizeOfData;
+        public int AddressOfRawData;
+        public int PointerToRawData;
+    }
+}
diff --git a/src/Microsoft.Diagnostics.Runtime/src/Utilities/PEImage/ImageDebugType.cs b/src/Microsoft.Diagnostics.Runtime/src/Utilities/PEImage/ImageDebugType.cs
@@ -0,0 +1,16 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+namespace Microsoft.Diagnostics.Runtime.Utilities
+{
+    internal enum ImageDebugType
+    {
+        UNKNOWN = 0,
+        COFF = 1,
+        CODEVIEW = 2,
+        FPO = 3,
+        MISC = 4,
+        BBT = 10
+    }
+}
diff --git a/src/Microsoft.Diagnostics.Runtime/src/Utilities/PEImage/ImageExportDirectory.cs b/src/Microsoft.Diagnostics.Runtime/src/Utilities/PEImage/ImageExportDirectory.cs
@@ -4,7 +4,7 @@
 
 namespace Microsoft.Diagnostics.Runtime.Utilities
 {
-    internal struct IMAGE_EXPORT_DIRECTORY
+    internal struct ImageExportDirectory
     {
         public int Characteristics;
         public int TimeDateStamp;

diff --git a/src/Microsoft.Diagnostics.Runtime/src/Utilities/PEImage/ImageFileHeader.cs b/src/Microsoft.Diagnostics.Runtime/src/Utilities/PEImage/ImageFileHeader.cs
@@ -0,0 +1,20 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Runtime.InteropServices;
+
+namespace Microsoft.Diagnostics.Runtime.Utilities
+{
+    [StructLayout(LayoutKind.Sequential, Pack = 1)]
+    internal struct ImageFileHeader
+    {
+        public ushort Machine;
+        public ushort NumberOfSections;
+        public int TimeDateStamp;
+        public uint PointerToSymbolTable;
+        public uint NumberOfSymbols;
+        public ushort SizeOfOptionalHeader;
+        public ushort Characteristics;
+    }
+}
diff --git a/src/Microsoft.Diagnostics.Runtime/src/Utilities/PEImage/ImageOptionalHeader.cs b/src/Microsoft.Diagnostics.Runtime/src/Utilities/PEImage/ImageOptionalHeader.cs
@@ -0,0 +1,63 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Runtime.InteropServices;
+
+namespace Microsoft.Diagnostics.Runtime.Utilities
+{
+    [StructLayout(LayoutKind.Explicit)]
+    internal struct ImageOptionalHeader
+    {
+        [FieldOffset(0)]
+        public ushort Magic;
+        [FieldOffset(2)]
+        public byte MajorLinkerVersion;
+        [FieldOffset(3)]
+        public byte MinorLinkerVersion;
+        [FieldOffset(4)]
+        public uint SizeOfCode;
+        [FieldOffset(8)]
+        public uint SizeOfInitializedData;
+        [FieldOffset(12)]
+        public uint SizeOfUninitializedData;
+        [FieldOffset(16)]
+        public uint AddressOfEntryPoint;
+        [FieldOffset(20)]
+        public uint BaseOfCode;
+        [FieldOffset(24)]
+        public ulong ImageBase64;
+        [FieldOffset(24)]
+        public uint BaseOfData;
+        [FieldOffset(28)]
+        public uint ImageBase;
+        [FieldOffset(32)]
+        public uint SectionAlignment;
+        [FieldOffset(36)]
+        public uint FileAlignment;
+        [FieldOffset(40)]
+        public ushort MajorOperatingSystemVersion;
+        [FieldOffset(42)]
+        public ushort MinorOperatingSystemVersion;
+        [FieldOffset(44)]
+        public ushort MajorImageVersion;
+        [FieldOffset(46)]
+        public ushort MinorImageVersion;
+        [FieldOffset(48)]
+        public ushort MajorSubsystemVersion;
+        [FieldOffset(50)]
+        public ushort MinorSubsystemVersion;
+        [FieldOffset(52)]
+        public uint Win32VersionValue;
+        [FieldOffset(56)]
+        public int SizeOfImage;
+        [FieldOffset(60)]
+        public uint SizeOfHeaders;
+        [FieldOffset(64)]
+        public uint CheckSum;
+        [FieldOffset(68)]
+        public ushort Subsystem;
+        [FieldOffset(70)]
+        public ushort DllCharacteristics;
+    }
+}
diff --git a/src/Microsoft.Diagnostics.Runtime/src/Utilities/PEImage/ImageResourceDataEntry.cs b/src/Microsoft.Diagnostics.Runtime/src/Utilities/PEImage/ImageResourceDataEntry.cs
@@ -12,7 +12,7 @@ namespace Microsoft.Diagnostics.Runtime.Utilities
     /// decoding code point values within the resource data.  Typically for new
     /// applications the code page would be the Unicode code page.
     /// </summary>
-    internal struct IMAGE_RESOURCE_DATA_ENTRY
+    internal struct ImageResourceDataEntry
     {
         public int RvaToData;
         public int Size;

diff --git a/src/Microsoft.Diagnostics.Runtime/src/Utilities/PEImage/ImageResourceDirectoryEntry.cs b/src/Microsoft.Diagnostics.Runtime/src/Utilities/PEImage/ImageResourceDirectoryEntry.cs
@@ -23,7 +23,7 @@ namespace Microsoft.Diagnostics.Runtime.Utilities
     /// field points to a resource data entry.
     /// </summary>
     [StructLayout(LayoutKind.Sequential)]
-    internal readonly struct IMAGE_RESOURCE_DIRECTORY_ENTRY
+    internal readonly struct ImageResourceDirectoryEntry
     {
         private readonly int _nameOffsetAndFlag;
         private readonly int _dataOffsetAndFlag;