From 449d3da3932b86cae6918d87acda14654dd3c463 Mon Sep 17 00:00:00 2001 From: tmat Date: Mon, 26 Jul 2021 15:05:54 -0700 Subject: [PATCH 1/3] Use native allocator instead of pinning when decompressing embedded PDB --- .../System.Reflection.Metadata.sln | 37 +++++---- .../src/System.Reflection.Metadata.csproj | 8 +- .../Metadata/MetadataReaderProvider.cs | 2 +- .../PEReader.EmbeddedPortablePdb.cs | 75 ++++++++++++------- .../DebugDirectoryBuilderTests.cs | 36 ++++----- 5 files changed, 89 insertions(+), 69 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/System.Reflection.Metadata.sln b/src/libraries/System.Reflection.Metadata/System.Reflection.Metadata.sln index 35971fd7edffb..532f50aedb051 100644 --- a/src/libraries/System.Reflection.Metadata/System.Reflection.Metadata.sln +++ b/src/libraries/System.Reflection.Metadata/System.Reflection.Metadata.sln @@ -1,4 +1,8 @@ -Microsoft Visual Studio Solution File, Format Version 12.00 + +Microsoft Visual Studio Solution File, Format Version 12.00 +# Visual Studio Version 17 +VisualStudioVersion = 17.0.31520.503 +MinimumVisualStudioVersion = 10.0.40219.1 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "TestUtilities", "..\Common\tests\TestUtilities\TestUtilities.csproj", "{2231787B-18C9-493C-A102-1E0E6A3D2CD3}" EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Win32.Registry", "..\Microsoft.Win32.Registry\ref\Microsoft.Win32.Registry.csproj", "{6FB0D012-27EC-49EB-B41C-A01DDE2937ED}" @@ -25,19 +29,9 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "ref", "ref", "{B332ED71-193 EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{C5FDAE2B-91DB-488B-A833-10D7E800447D}" EndProject +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.Collections.Immutable", "..\System.Collections.Immutable\ref\System.Collections.Immutable.csproj", "{282C76D4-54C5-44BF-9F15-1A4302234DBB}" +EndProject Global - GlobalSection(NestedProjects) = preSolution - {2231787B-18C9-493C-A102-1E0E6A3D2CD3} = {31FAA062-EF3C-4F4D-AF64-A6130F92A737} - {7EE935DD-2F8B-4C72-BACF-5DB95DE080BE} = {31FAA062-EF3C-4F4D-AF64-A6130F92A737} - {6FB0D012-27EC-49EB-B41C-A01DDE2937ED} = {B332ED71-1937-4B24-8519-AE665C23DD2F} - {587255BE-DC22-4B85-9E3F-02325E7B4FF7} = {B332ED71-1937-4B24-8519-AE665C23DD2F} - {00147002-F430-4E24-95F8-5E79A4D24AE2} = {B332ED71-1937-4B24-8519-AE665C23DD2F} - {632AE256-CA0A-4EBA-8D64-F9988CDC459D} = {B332ED71-1937-4B24-8519-AE665C23DD2F} - {82DC5DDA-CCFD-4F67-9D15-9BAD27FB4251} = {B332ED71-1937-4B24-8519-AE665C23DD2F} - {A69B0EE0-BE0C-4D53-A16F-5465028D975D} = {C5FDAE2B-91DB-488B-A833-10D7E800447D} - {B905521A-FE25-4D35-9929-B2622F590263} = {C5FDAE2B-91DB-488B-A833-10D7E800447D} - {5E36AB65-74DD-47F5-8F2E-9050561BBFEE} = {C5FDAE2B-91DB-488B-A833-10D7E800447D} - EndGlobalSection GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU Release|Any CPU = Release|Any CPU @@ -83,10 +77,27 @@ Global {82DC5DDA-CCFD-4F67-9D15-9BAD27FB4251}.Debug|Any CPU.Build.0 = Debug|Any CPU {82DC5DDA-CCFD-4F67-9D15-9BAD27FB4251}.Release|Any CPU.ActiveCfg = Release|Any CPU {82DC5DDA-CCFD-4F67-9D15-9BAD27FB4251}.Release|Any CPU.Build.0 = Release|Any CPU + {282C76D4-54C5-44BF-9F15-1A4302234DBB}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {282C76D4-54C5-44BF-9F15-1A4302234DBB}.Debug|Any CPU.Build.0 = Debug|Any CPU + {282C76D4-54C5-44BF-9F15-1A4302234DBB}.Release|Any CPU.ActiveCfg = Release|Any CPU + {282C76D4-54C5-44BF-9F15-1A4302234DBB}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE EndGlobalSection + GlobalSection(NestedProjects) = preSolution + {2231787B-18C9-493C-A102-1E0E6A3D2CD3} = {31FAA062-EF3C-4F4D-AF64-A6130F92A737} + {6FB0D012-27EC-49EB-B41C-A01DDE2937ED} = {B332ED71-1937-4B24-8519-AE665C23DD2F} + {A69B0EE0-BE0C-4D53-A16F-5465028D975D} = {C5FDAE2B-91DB-488B-A833-10D7E800447D} + {587255BE-DC22-4B85-9E3F-02325E7B4FF7} = {B332ED71-1937-4B24-8519-AE665C23DD2F} + {B905521A-FE25-4D35-9929-B2622F590263} = {C5FDAE2B-91DB-488B-A833-10D7E800447D} + {7EE935DD-2F8B-4C72-BACF-5DB95DE080BE} = {31FAA062-EF3C-4F4D-AF64-A6130F92A737} + {00147002-F430-4E24-95F8-5E79A4D24AE2} = {B332ED71-1937-4B24-8519-AE665C23DD2F} + {5E36AB65-74DD-47F5-8F2E-9050561BBFEE} = {C5FDAE2B-91DB-488B-A833-10D7E800447D} + {632AE256-CA0A-4EBA-8D64-F9988CDC459D} = {B332ED71-1937-4B24-8519-AE665C23DD2F} + {82DC5DDA-CCFD-4F67-9D15-9BAD27FB4251} = {B332ED71-1937-4B24-8519-AE665C23DD2F} + {282C76D4-54C5-44BF-9F15-1A4302234DBB} = {B332ED71-1937-4B24-8519-AE665C23DD2F} + EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {64BB97AB-FD23-40BA-B638-FE4756AE6452} EndGlobalSection diff --git a/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj b/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj index 654ed3f906279..57c10df36c4b3 100644 --- a/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj +++ b/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj @@ -12,12 +12,8 @@ $(DefineConstants);FEATURE_CER - - + + diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/MetadataReaderProvider.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/MetadataReaderProvider.cs index 7179ae12a5206..836ca232a53c9 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/MetadataReaderProvider.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/MetadataReaderProvider.cs @@ -30,7 +30,7 @@ public sealed class MetadataReaderProvider : IDisposable private MetadataReader? _lazyMetadataReader; private readonly object _metadataReaderGuard = new object(); - private MetadataReaderProvider(AbstractMemoryBlock metadataBlock) + internal MetadataReaderProvider(AbstractMemoryBlock metadataBlock) { Debug.Assert(metadataBlock != null); _lazyMetadataBlock = metadataBlock; diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEReader.EmbeddedPortablePdb.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEReader.EmbeddedPortablePdb.cs index ba58af0d0cffc..69ea9a41c4870 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEReader.EmbeddedPortablePdb.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEReader.EmbeddedPortablePdb.cs @@ -26,12 +26,13 @@ public sealed partial class PEReader : IDisposable /// Reads the data pointed to by the specified Debug Directory entry and interprets them as Embedded Portable PDB blob. /// /// - /// Provider of a metadata reader reading Portable PDB image. + /// Provider of a metadata reader reading the embedded Portable PDB image. + /// Dispose to release resources allocated for the embedded PDB. /// /// is not a entry. /// Bad format of the data. /// PE image not available. - public MetadataReaderProvider ReadEmbeddedPortablePdbDebugDirectoryData(DebugDirectoryEntry entry) + public unsafe MetadataReaderProvider ReadEmbeddedPortablePdbDebugDirectoryData(DebugDirectoryEntry entry) { if (entry.Type != DebugDirectoryEntryType.EmbeddedPortablePdb) { @@ -40,11 +41,8 @@ public MetadataReaderProvider ReadEmbeddedPortablePdbDebugDirectoryData(DebugDir ValidateEmbeddedPortablePdbVersion(entry); - using (var block = GetDebugDirectoryEntryDataBlock(entry)) - { - var pdbImage = DecodeEmbeddedPortablePdbDebugDirectoryData(block); - return MetadataReaderProvider.FromPortablePdbImage(pdbImage); - } + using var block = GetDebugDirectoryEntryDataBlock(entry); + return new MetadataReaderProvider(DecodeEmbeddedPortablePdbDebugDirectoryData(block)); } // internal for testing @@ -69,9 +67,9 @@ internal static void ValidateEmbeddedPortablePdbVersion(DebugDirectoryEntry entr } // internal for testing - internal static unsafe ImmutableArray DecodeEmbeddedPortablePdbDebugDirectoryData(AbstractMemoryBlock block) + internal static unsafe NativeHeapMemoryBlock DecodeEmbeddedPortablePdbDebugDirectoryData(AbstractMemoryBlock block) { - byte[]? decompressed; + NativeHeapMemoryBlock? decompressed; var headerReader = block.GetReader(); if (headerReader.ReadUInt32() != PortablePdbVersions.DebugDirectoryEmbeddedSignature) @@ -83,44 +81,63 @@ internal static unsafe ImmutableArray DecodeEmbeddedPortablePdbDebugDirect try { - decompressed = new byte[decompressedSize]; + decompressed = new NativeHeapMemoryBlock(decompressedSize); } catch { throw new BadImageFormatException(SR.DataTooBig); } - var compressed = new ReadOnlyUnmanagedMemoryStream(headerReader.CurrentPointer, headerReader.RemainingBytes); - var deflate = new DeflateStream(compressed, CompressionMode.Decompress, leaveOpen: true); - - if (decompressedSize > 0) + bool success = false; + try { - int actualLength; + var compressed = new ReadOnlyUnmanagedMemoryStream(headerReader.CurrentPointer, headerReader.RemainingBytes); + var deflate = new DeflateStream(compressed, CompressionMode.Decompress, leaveOpen: true); - try - { - actualLength = deflate.TryReadAll(decompressed, 0, decompressed.Length); - } - catch (InvalidDataException e) + if (decompressedSize > 0) { - throw new BadImageFormatException(e.Message, e.InnerException); + int actualLength; + + try + { +#if NETCOREAPP3_0_OR_GREATER + actualLength = deflate.Read(new Span(decompressed.Pointer, decompressed.Size)); +#else + using var decompressedStream = new UnmanagedMemoryStream(decompressed.Pointer, decompressed.Size, decompressed.Size, FileAccess.Write); + deflate.CopyTo(decompressedStream); + actualLength = (int)decompressedStream.Position; +#endif + } + catch (Exception e) + { + throw new BadImageFormatException(e.Message, e.InnerException); + } + + if (actualLength != decompressed.Size) + { + throw new BadImageFormatException(SR.SizeMismatch); + } } - if (actualLength != decompressed.Length) + // Check that there is no more compressed data left, + // in case the decompressed size specified in the header is smaller + // than the actual decompressed size of the data. + if (deflate.ReadByte() != -1) { throw new BadImageFormatException(SR.SizeMismatch); } - } - // Check that there is no more compressed data left, - // in case the decompressed size specified in the header is smaller - // than the actual decompressed size of the data. - if (deflate.ReadByte() != -1) + success = true; + } + finally { - throw new BadImageFormatException(SR.SizeMismatch); + if (!success) + { + decompressed.Dispose(); + } } - return ImmutableByteArrayInterop.DangerousCreateFromUnderlyingArray(ref decompressed); + return decompressed; } partial void TryOpenEmbeddedPortablePdb(DebugDirectoryEntry embeddedPdbEntry, ref bool openedEmbeddedPdb, ref MetadataReaderProvider? provider, ref Exception? errorToReport) diff --git a/src/libraries/System.Reflection.Metadata/tests/PortableExecutable/DebugDirectoryBuilderTests.cs b/src/libraries/System.Reflection.Metadata/tests/PortableExecutable/DebugDirectoryBuilderTests.cs index 89ca4a5993892..ed0270f6994ff 100644 --- a/src/libraries/System.Reflection.Metadata/tests/PortableExecutable/DebugDirectoryBuilderTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/PortableExecutable/DebugDirectoryBuilderTests.cs @@ -292,26 +292,22 @@ public void EmbeddedPortablePdb() 0xEB, 0x28, 0x4F, 0x0B, 0x75, 0x31, 0x56, 0x12, 0x04, 0x00 // compressed data }, bytes); - using (var pinned = new PinnedBlob(bytes)) - { - var actual = PEReader.ReadDebugDirectoryEntries(pinned.CreateReader(0, DebugDirectoryEntry.Size)); - Assert.Equal(1, actual.Length); - Assert.Equal(0u, actual[0].Stamp); - Assert.Equal(0x0100, actual[0].MajorVersion); - Assert.Equal(0x0100, actual[0].MinorVersion); - Assert.Equal(DebugDirectoryEntryType.EmbeddedPortablePdb, actual[0].Type); - Assert.False(actual[0].IsPortableCodeView); - Assert.Equal(0x00000012, actual[0].DataSize); - Assert.Equal(0x0000001c, actual[0].DataRelativeVirtualAddress); - Assert.Equal(0x0000001c, actual[0].DataPointer); - - var provider = new ByteArrayMemoryProvider(bytes); - using (var block = provider.GetMemoryBlock(actual[0].DataPointer, actual[0].DataSize)) - { - var decoded = PEReader.DecodeEmbeddedPortablePdbDebugDirectoryData(block); - AssertEx.Equal(new byte[] { 0x88, 0x77, 0x66, 0x55, 0x44, 0x33, 0x22, 0x11 }, decoded); - } - } + using var pinned = new PinnedBlob(bytes); + var actual = PEReader.ReadDebugDirectoryEntries(pinned.CreateReader(0, DebugDirectoryEntry.Size)); + Assert.Equal(1, actual.Length); + Assert.Equal(0u, actual[0].Stamp); + Assert.Equal(0x0100, actual[0].MajorVersion); + Assert.Equal(0x0100, actual[0].MinorVersion); + Assert.Equal(DebugDirectoryEntryType.EmbeddedPortablePdb, actual[0].Type); + Assert.False(actual[0].IsPortableCodeView); + Assert.Equal(0x00000012, actual[0].DataSize); + Assert.Equal(0x0000001c, actual[0].DataRelativeVirtualAddress); + Assert.Equal(0x0000001c, actual[0].DataPointer); + + var provider = new ByteArrayMemoryProvider(bytes); + using var block = provider.GetMemoryBlock(actual[0].DataPointer, actual[0].DataSize); + using var decoded = PEReader.DecodeEmbeddedPortablePdbDebugDirectoryData(block); + AssertEx.Equal(new byte[] { 0x88, 0x77, 0x66, 0x55, 0x44, 0x33, 0x22, 0x11 }, decoded.GetContentUnchecked(0, decoded.Size)); } [Fact] From 0d028eeec18a51369da1355e7b3adf8015ff1e49 Mon Sep 17 00:00:00 2001 From: tmat Date: Mon, 26 Jul 2021 15:09:08 -0700 Subject: [PATCH 2/3] Cleanup --- .../System.Reflection.Metadata.sln | 6 +----- .../src/System.Reflection.Metadata.csproj | 8 ++++++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/System.Reflection.Metadata.sln b/src/libraries/System.Reflection.Metadata/System.Reflection.Metadata.sln index 532f50aedb051..4a91b9b849a67 100644 --- a/src/libraries/System.Reflection.Metadata/System.Reflection.Metadata.sln +++ b/src/libraries/System.Reflection.Metadata/System.Reflection.Metadata.sln @@ -1,8 +1,4 @@ - -Microsoft Visual Studio Solution File, Format Version 12.00 -# Visual Studio Version 17 -VisualStudioVersion = 17.0.31520.503 -MinimumVisualStudioVersion = 10.0.40219.1 +Microsoft Visual Studio Solution File, Format Version 12.00 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "TestUtilities", "..\Common\tests\TestUtilities\TestUtilities.csproj", "{2231787B-18C9-493C-A102-1E0E6A3D2CD3}" EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Win32.Registry", "..\Microsoft.Win32.Registry\ref\Microsoft.Win32.Registry.csproj", "{6FB0D012-27EC-49EB-B41C-A01DDE2937ED}" diff --git a/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj b/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj index 57c10df36c4b3..654ed3f906279 100644 --- a/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj +++ b/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj @@ -12,8 +12,12 @@ $(DefineConstants);FEATURE_CER - - + + From 0c73ae51539e3dc34417772b196fc767a1354722 Mon Sep 17 00:00:00 2001 From: tmat Date: Tue, 27 Jul 2021 09:08:43 -0700 Subject: [PATCH 3/3] TryReadAll --- .../Internal/Utilities/StreamExtensions.cs | 21 ++++++++++++++++++- .../PEReader.EmbeddedPortablePdb.cs | 2 +- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/StreamExtensions.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/StreamExtensions.cs index 4035e3aa0a469..89175b7f84afd 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/StreamExtensions.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/StreamExtensions.cs @@ -60,7 +60,7 @@ internal static int TryReadAll(this Stream stream, byte[] buffer, int offset, in Debug.Assert(count > 0); int totalBytesRead; - int bytesRead = 0; + int bytesRead; for (totalBytesRead = 0; totalBytesRead < count; totalBytesRead += bytesRead) { // Note: Don't attempt to save state in-between calls to .Read as it would @@ -76,6 +76,25 @@ internal static int TryReadAll(this Stream stream, byte[] buffer, int offset, in return totalBytesRead; } +#if NETCOREAPP3_0_OR_GREATER + internal static int TryReadAll(this Stream stream, Span buffer) + { + int totalBytesRead = 0; + while (totalBytesRead < buffer.Length) + { + int bytesRead = stream.Read(buffer.Slice(totalBytesRead)); + if (bytesRead == 0) + { + break; + } + + totalBytesRead += bytesRead; + } + + return totalBytesRead; + } +#endif + /// /// Resolve image size as either the given user-specified size or distance from current position to end-of-stream. /// Also performs the relevant argument validation and publicly visible caller has same argument names. diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEReader.EmbeddedPortablePdb.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEReader.EmbeddedPortablePdb.cs index 69ea9a41c4870..43d6e31584e92 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEReader.EmbeddedPortablePdb.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEReader.EmbeddedPortablePdb.cs @@ -101,7 +101,7 @@ internal static unsafe NativeHeapMemoryBlock DecodeEmbeddedPortablePdbDebugDirec try { #if NETCOREAPP3_0_OR_GREATER - actualLength = deflate.Read(new Span(decompressed.Pointer, decompressed.Size)); + actualLength = deflate.TryReadAll(new Span(decompressed.Pointer, decompressed.Size)); #else using var decompressedStream = new UnmanagedMemoryStream(decompressed.Pointer, decompressed.Size, decompressed.Size, FileAccess.Write); deflate.CopyTo(decompressedStream);