From 2a02896735063b39d493f680c65c129102e2ad51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Matou=C5=A1ek?= Date: Tue, 27 Jul 2021 15:16:59 -0700 Subject: [PATCH] Use native allocator instead of pinning when decompressing embedded PDB (#56336) --- .../System.Reflection.Metadata.sln | 31 +++++--- .../Internal/Utilities/StreamExtensions.cs | 21 +++++- .../Metadata/MetadataReaderProvider.cs | 2 +- .../PEReader.EmbeddedPortablePdb.cs | 75 ++++++++++++------- .../DebugDirectoryBuilderTests.cs | 36 ++++----- 5 files changed, 102 insertions(+), 63 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/System.Reflection.Metadata.sln b/src/libraries/System.Reflection.Metadata/System.Reflection.Metadata.sln index 35971fd7edffb4..4a91b9b849a672 100644 --- a/src/libraries/System.Reflection.Metadata/System.Reflection.Metadata.sln +++ b/src/libraries/System.Reflection.Metadata/System.Reflection.Metadata.sln @@ -25,19 +25,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 +73,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/Internal/Utilities/StreamExtensions.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/StreamExtensions.cs index 4035e3aa0a4693..89175b7f84afd6 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/Metadata/MetadataReaderProvider.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/MetadataReaderProvider.cs index 7179ae12a52066..836ca232a53c9c 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 ba58af0d0cffcd..43d6e31584e928 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.TryReadAll(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 89ca4a59938929..ed0270f6994fff 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]