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 66d5a1e0ae7fe6..5d7553f707ba53 100644 --- a/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj +++ b/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj @@ -104,7 +104,6 @@ The System.Reflection.Metadata library is built-in as part of the shared framewo - @@ -113,7 +112,6 @@ The System.Reflection.Metadata library is built-in as part of the shared framewo - diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/AbstractMemoryBlock.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/AbstractMemoryBlock.cs index cee648185fbfdc..89df99decbbb81 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/AbstractMemoryBlock.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/AbstractMemoryBlock.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Immutable; +using System.IO; using System.Reflection.Metadata; namespace System.Reflection.Internal @@ -23,6 +24,11 @@ internal abstract class AbstractMemoryBlock : IDisposable public unsafe BlobReader GetReader() => new BlobReader(Pointer, Size); + /// + /// Creates a new stream wrapping the block's memory. + /// + public unsafe Stream GetStream() => new UnmanagedMemoryStream(Pointer, Size); + /// /// Returns the content of the entire memory block. /// diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/ByteArrayMemoryProvider.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/ByteArrayMemoryProvider.cs index 040baf783fa6eb..d7a265ac665163 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/ByteArrayMemoryProvider.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/ByteArrayMemoryProvider.cs @@ -34,12 +34,6 @@ protected override AbstractMemoryBlock GetMemoryBlockImpl(int start, int size) return new ByteArrayMemoryBlock(this, start, size); } - public override Stream GetStream(out StreamConstraints constraints) - { - constraints = new StreamConstraints(null, 0, Size); - return new ImmutableMemoryStream(_array); - } - internal unsafe byte* Pointer { get diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/ExternalMemoryBlockProvider.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/ExternalMemoryBlockProvider.cs index e47703562c7c5b..dcedd5b49a4e93 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/ExternalMemoryBlockProvider.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/ExternalMemoryBlockProvider.cs @@ -33,12 +33,6 @@ protected override AbstractMemoryBlock GetMemoryBlockImpl(int start, int size) return new ExternalMemoryBlock(this, _memory + start, size); } - public override Stream GetStream(out StreamConstraints constraints) - { - constraints = new StreamConstraints(null, 0, _size); - return new UnmanagedMemoryStream(_memory, _size); - } - protected override void Dispose(bool disposing) { Debug.Assert(disposing); diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/MemoryBlockProvider.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/MemoryBlockProvider.cs index 492b7d752a4f16..1c13b2d8aec966 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/MemoryBlockProvider.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/MemoryBlockProvider.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics.CodeAnalysis; using System.IO; using System.Reflection.PortableExecutable; @@ -39,12 +40,21 @@ public AbstractMemoryBlock GetMemoryBlock(int start, int size) protected abstract AbstractMemoryBlock GetMemoryBlockImpl(int start, int size); /// - /// Gets a seekable and readable that can be used to read all data. - /// The operations on the stream has to be done under a lock of if non-null. - /// The image starts at and has size . - /// It is the caller's responsibility not to read outside those bounds. + /// Gets the backing the , if there is one. /// - public abstract Stream GetStream(out StreamConstraints constraints); + /// + /// It is the caller's responsibility to use only + /// while locking on , and not read outside the + /// bounds defined by and . + /// + public virtual bool TryGetUnderlyingStream([NotNullWhen(true)] out Stream? stream, out long imageStart, out int imageSize, [NotNullWhen(true)] out object? streamGuard) + { + stream = null; + imageStart = 0; + imageSize = 0; + streamGuard = null; + return false; + } /// /// The size of the data. diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/StreamConstraints.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/StreamConstraints.cs deleted file mode 100644 index 3c96540b257087..00000000000000 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/StreamConstraints.cs +++ /dev/null @@ -1,19 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace System.Reflection.Internal -{ - internal readonly struct StreamConstraints - { - public readonly object? GuardOpt; - public readonly long ImageStart; - public readonly int ImageSize; - - public StreamConstraints(object? guardOpt, long startPosition, int imageSize) - { - GuardOpt = guardOpt; - ImageStart = startPosition; - ImageSize = imageSize; - } - } -} diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/StreamMemoryBlockProvider.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/StreamMemoryBlockProvider.cs index 735ae1b182deeb..c072109377c6cb 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/StreamMemoryBlockProvider.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/StreamMemoryBlockProvider.cs @@ -28,7 +28,7 @@ internal sealed class StreamMemoryBlockProvider : MemoryBlockProvider private readonly object _streamGuard; private readonly bool _leaveOpen; - private bool _useMemoryMap; + private readonly bool _useMemoryMap; private readonly long _imageStart; private readonly int _imageSize; @@ -73,13 +73,7 @@ internal static unsafe NativeHeapMemoryBlock ReadMemoryBlockNoLock(Stream stream try { stream.Seek(start, SeekOrigin.Begin); - - int bytesRead = 0; - - if ((bytesRead = stream.Read(block.Pointer, size)) != size) - { - stream.CopyTo(block.Pointer + bytesRead, size - bytesRead); - } + stream.ReadExactly(block.Pointer, size); fault = false; } @@ -94,6 +88,15 @@ internal static unsafe NativeHeapMemoryBlock ReadMemoryBlockNoLock(Stream stream return block; } + public override bool TryGetUnderlyingStream([NotNullWhen(true)] out Stream? stream, out long imageStart, out int imageSize, [NotNullWhen(true)] out object? streamGuard) + { + stream = _stream; + imageStart = _imageStart; + imageSize = _imageSize; + streamGuard = _streamGuard; + return true; + } + /// Error while reading from the stream. protected override AbstractMemoryBlock GetMemoryBlockImpl(int start, int size) { @@ -101,12 +104,7 @@ protected override AbstractMemoryBlock GetMemoryBlockImpl(int start, int size) if (_useMemoryMap && size > MemoryMapThreshold) { - if (TryCreateMemoryMappedFileBlock(absoluteStart, size, out MemoryMappedFileBlock? block)) - { - return block; - } - - _useMemoryMap = false; + return CreateMemoryMappedFileBlock(absoluteStart, size); } lock (_streamGuard) @@ -115,26 +113,18 @@ protected override AbstractMemoryBlock GetMemoryBlockImpl(int start, int size) } } - public override Stream GetStream(out StreamConstraints constraints) - { - constraints = new StreamConstraints(_streamGuard, _imageStart, _imageSize); - return _stream; - } - /// IO error while mapping memory or not enough memory to create the mapping. - private unsafe bool TryCreateMemoryMappedFileBlock(long start, int size, [NotNullWhen(true)] out MemoryMappedFileBlock? block) + private unsafe MemoryMappedFileBlock CreateMemoryMappedFileBlock(long start, int size) { if (_lazyMemoryMap == null) { - // leave the underlying stream open. It will be closed by the Dispose method. - MemoryMappedFile newMemoryMap; - // CreateMemoryMap might modify the stream (calls FileStream.Flush) lock (_streamGuard) { try { - newMemoryMap = + // leave the underlying stream open. It will be closed by the Dispose method. + _lazyMemoryMap ??= MemoryMappedFile.CreateFromFile( fileStream: (FileStream)_stream, mapName: null, @@ -148,17 +138,6 @@ private unsafe bool TryCreateMemoryMappedFileBlock(long start, int size, [NotNul throw new IOException(e.Message, e); } } - - if (newMemoryMap == null) - { - block = null; - return false; - } - - if (Interlocked.CompareExchange(ref _lazyMemoryMap, newMemoryMap, null) != null) - { - newMemoryMap.Dispose(); - } } MemoryMappedViewAccessor accessor; @@ -168,14 +147,7 @@ private unsafe bool TryCreateMemoryMappedFileBlock(long start, int size, [NotNul accessor = _lazyMemoryMap.CreateViewAccessor(start, size, MemoryMappedFileAccess.Read); } - if (accessor == null) - { - block = null; - return false; - } - - block = new MemoryMappedFileBlock(accessor, accessor.SafeMemoryMappedViewHandle, accessor.PointerOffset, size); - return true; + return new MemoryMappedFileBlock(accessor, accessor.SafeMemoryMappedViewHandle, accessor.PointerOffset, size); } } } diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/ImmutableMemoryStream.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/ImmutableMemoryStream.cs deleted file mode 100644 index fe94886ac859d9..00000000000000 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/ImmutableMemoryStream.cs +++ /dev/null @@ -1,124 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Collections.Immutable; -using System.Diagnostics; -using System.IO; - -namespace System.Reflection.Internal -{ - internal sealed class ImmutableMemoryStream : Stream - { - private readonly ImmutableArray _array; - private int _position; - - internal ImmutableMemoryStream(ImmutableArray array) - { - Debug.Assert(!array.IsDefault); - _array = array; - } - - public ImmutableArray GetBuffer() - { - return _array; - } - - public override bool CanRead - { - get { return true; } - } - - public override bool CanSeek - { - get { return true; } - } - - public override bool CanWrite - { - get { return false; } - } - - public override long Length - { - get { return _array.Length; } - } - - public override long Position - { - get - { - return _position; - } - set - { - if (value < 0 || value >= _array.Length) - { - throw new ArgumentOutOfRangeException(nameof(value)); - } - - _position = (int)value; - } - } - - public override void Flush() - { - } - - public override int Read(byte[] buffer, int offset, int count) - { - int result = Math.Min(count, _array.Length - _position); - _array.CopyTo(_position, buffer, offset, result); - _position += result; - return result; - } - -#if NET - // Duplicate the Read(byte[]) logic here instead of refactoring both to use Spans - // so we don't affect perf on .NET Framework. - public override int Read(Span buffer) - { - int result = Math.Min(buffer.Length, _array.Length - _position); - _array.AsSpan(_position, result).CopyTo(buffer); - _position += result; - return result; - } -#endif - - public override long Seek(long offset, SeekOrigin origin) - { - long target; - try - { - target = origin switch - { - SeekOrigin.Begin => offset, - SeekOrigin.Current => checked(offset + _position), - SeekOrigin.End => checked(offset + _array.Length), - _ => throw new ArgumentOutOfRangeException(nameof(origin)), - }; - } - catch (OverflowException) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - - if (target < 0 || target >= _array.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - - _position = (int)target; - return target; - } - - public override void SetLength(long value) - { - throw new NotSupportedException(); - } - - public override void Write(byte[] buffer, int offset, int count) - { - throw new NotSupportedException(); - } - } -} 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 484f138762fe41..b8495aa9fcc079 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 @@ -9,36 +9,6 @@ namespace System.Reflection.Internal { internal static partial class StreamExtensions { - // From System.IO.Stream.CopyTo: - // We pick a value that is the largest multiple of 4096 that is still smaller than the large object heap threshold (85K). - // The CopyTo/CopyToAsync buffer is short-lived and is likely to be collected at Gen0, and it offers a significant - // improvement in Copy performance. - internal const int StreamCopyBufferSize = 81920; - - /// - /// Copies specified amount of data from given stream to a target memory pointer. - /// - /// unexpected stream end. - internal static unsafe void CopyTo(this Stream source, byte* destination, int size) - { - byte[] buffer = new byte[Math.Min(StreamCopyBufferSize, size)]; - while (size > 0) - { - int readSize = Math.Min(size, buffer.Length); - int bytesRead = source.Read(buffer, 0, readSize); - - if (bytesRead <= 0 || bytesRead > readSize) - { - throw new IOException(SR.UnexpectedStreamEnd); - } - - Marshal.Copy(buffer, 0, (IntPtr)destination, bytesRead); - - destination += bytesRead; - size -= bytesRead; - } - } - /// /// Attempts to read all of the requested bytes from the stream into the buffer /// diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/StreamExtensions.netcoreapp.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/StreamExtensions.netcoreapp.cs index ca0d189eeef4c5..3fbe974fe5e442 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/StreamExtensions.netcoreapp.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/StreamExtensions.netcoreapp.cs @@ -7,7 +7,7 @@ namespace System.Reflection.Internal { internal static partial class StreamExtensions { - internal static unsafe int Read(this Stream stream, byte* buffer, int size) - => stream.Read(new Span(buffer, size)); + internal static unsafe void ReadExactly(this Stream stream, byte* buffer, int size) + => stream.ReadExactly(new Span(buffer, size)); } } diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/StreamExtensions.netstandard2.0.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/StreamExtensions.netstandard2.0.cs index ed07d3c1bf7fd9..aa7cd802c52047 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/StreamExtensions.netstandard2.0.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/StreamExtensions.netstandard2.0.cs @@ -10,6 +10,36 @@ internal static partial class StreamExtensions { private static bool IsWindows => Path.DirectorySeparatorChar == '\\'; + // From System.IO.Stream.CopyTo: + // We pick a value that is the largest multiple of 4096 that is still smaller than the large object heap threshold (85K). + // The CopyTo/CopyToAsync buffer is short-lived and is likely to be collected at Gen0, and it offers a significant + // improvement in Copy performance. + internal const int StreamCopyBufferSize = 81920; + + /// + /// Copies specified amount of data from given stream to a target memory pointer. + /// + /// unexpected stream end. + internal static unsafe void CopyTo(this Stream source, byte* destination, int size) + { + byte[] buffer = new byte[Math.Min(StreamCopyBufferSize, size)]; + while (size > 0) + { + int readSize = Math.Min(size, buffer.Length); + int bytesRead = source.Read(buffer, 0, readSize); + + if (bytesRead <= 0 || bytesRead > readSize) + { + throw new IOException(SR.UnexpectedStreamEnd); + } + + Marshal.Copy(buffer, 0, (IntPtr)destination, bytesRead); + + destination += bytesRead; + size -= bytesRead; + } + } + private static SafeHandle? GetSafeFileHandle(FileStream stream) { SafeHandle handle; @@ -34,14 +64,20 @@ internal static partial class StreamExtensions return handle; } - internal static unsafe int Read(this Stream stream, byte* buffer, int size) + /// + /// Reads from a into an unmanaged buffer. + /// + /// + /// If the platform is not Windows, this function will always return zero and perform no reads. + /// + private static unsafe int TryReadWin32File(this FileStream stream, byte* buffer, int size) { - if (!IsWindows || stream is not FileStream fs) + if (!IsWindows) { return 0; } - SafeHandle? handle = GetSafeFileHandle(fs); + SafeHandle? handle = GetSafeFileHandle(stream); if (handle == null) { return 0; @@ -50,5 +86,14 @@ internal static unsafe int Read(this Stream stream, byte* buffer, int size) int result = Interop.Kernel32.ReadFile(handle, buffer, size, out int bytesRead, IntPtr.Zero); return result == 0 ? 0 : bytesRead; } + + internal static unsafe void ReadExactly(this Stream stream, byte* buffer, int size) + { + int bytesRead = stream is FileStream fs ? TryReadWin32File(fs, buffer, size) : 0; + if (bytesRead != size) + { + stream.CopyTo(buffer + bytesRead, size - bytesRead); + } + } } } diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEBinaryReader.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEBinaryReader.cs index 9cf7b5779540d3..642b9085e83db7 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEBinaryReader.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEBinaryReader.cs @@ -32,18 +32,17 @@ public PEBinaryReader(Stream stream, int size) _reader = new BinaryReader(stream, Encoding.UTF8, leaveOpen: true); } - public int CurrentOffset + public int Offset { get { return (int)(_reader.BaseStream.Position - _startOffset); } + set + { + CheckBounds(_startOffset, value); + _reader.BaseStream.Seek(_startOffset + value, SeekOrigin.Begin); + } } - public void Seek(int offset) - { - CheckBounds(_startOffset, offset); - _reader.BaseStream.Seek(offset, SeekOrigin.Begin); - } - - public byte[] ReadBytes(int count) + private byte[] ReadBytes(int count) { CheckBounds(_reader.BaseStream.Position, count); return _reader.ReadBytes(count); diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaders.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaders.cs index 9ea27a83cc9983..cc8ba755199efd 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaders.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaders.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; using System.IO; using System.Reflection.Internal; using System.Reflection.Metadata; @@ -87,27 +88,25 @@ public PEHeaders(Stream peStream, int size, bool isLoadedImage) int actualSize = StreamExtensions.GetAndValidateSize(peStream, size, nameof(peStream)); var reader = new PEBinaryReader(peStream, actualSize); - bool isCoffOnly; - SkipDosHeader(ref reader, out isCoffOnly); + SkipDosHeader(ref reader, out bool isCoffOnly); - _coffHeaderStartOffset = reader.CurrentOffset; + _coffHeaderStartOffset = reader.Offset; _coffHeader = new CoffHeader(ref reader); if (!isCoffOnly) { - _peHeaderStartOffset = reader.CurrentOffset; + _peHeaderStartOffset = reader.Offset; _peHeader = new PEHeader(ref reader); } - _sectionHeaders = this.ReadSectionHeaders(ref reader); + _sectionHeaders = ReadSectionHeaders(ref reader); if (!isCoffOnly) { - int offset; - if (TryCalculateCorHeaderOffset(out offset)) + if (TryCalculateCorHeaderOffset(out int offset)) { _corHeaderStartOffset = offset; - reader.Seek(offset); + reader.Offset = offset; _corHeader = new CorHeader(ref reader); } } @@ -260,7 +259,7 @@ private static void SkipDosHeader(ref PEBinaryReader reader, out bool isCOFFOnly if (dosSig != 0 || reader.ReadUInt16() != 0xffff) { isCOFFOnly = true; - reader.Seek(0); + reader.Offset = 0; } else { @@ -276,10 +275,10 @@ private static void SkipDosHeader(ref PEBinaryReader reader, out bool isCOFFOnly if (!isCOFFOnly) { // Skip the DOS Header - reader.Seek(PESignatureOffsetLocation); + reader.Offset = PESignatureOffsetLocation; int ntHeaderOffset = reader.ReadInt32(); - reader.Seek(ntHeaderOffset); + reader.Offset = ntHeaderOffset; // Look for PESignature "PE\0\0" uint ntSignature = reader.ReadUInt32(); diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEReader.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEReader.cs index 37533d8c3c7c3d..0b7b4a8ea2a640 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEReader.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEReader.cs @@ -3,6 +3,7 @@ using System.Collections.Immutable; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.IO; using System.Reflection.Internal; using System.Reflection.Metadata; @@ -194,7 +195,7 @@ public unsafe PEReader(Stream peStream, PEStreamOptions options, int size) // if the caller asked for metadata initialize the PE headers (calculates metadata offset): if ((options & PEStreamOptions.PrefetchMetadata) != 0) { - InitializePEHeaders(); + _lazyPEHeaders = new PEHeaders(imageBlock.GetStream(), imageBlock.Size, IsLoadedImage); } } else @@ -204,7 +205,7 @@ public unsafe PEReader(Stream peStream, PEStreamOptions options, int size) if (_lazyPEHeaders.MetadataStartOffset != -1) { - _lazyMetadataBlock = StreamMemoryBlockProvider.ReadMemoryBlockNoLock(peStream, _lazyPEHeaders.MetadataStartOffset, _lazyPEHeaders.MetadataSize); + _lazyMetadataBlock = StreamMemoryBlockProvider.ReadMemoryBlockNoLock(peStream, start + _lazyPEHeaders.MetadataStartOffset, _lazyPEHeaders.MetadataSize); } } // We read all we need, the stream is going to be closed. @@ -298,7 +299,6 @@ public PEHeaders PEHeaders if (_lazyPEHeaders == null) { InitializePEHeaders(); - Debug.Assert(_lazyPEHeaders != null); } return _lazyPEHeaders; @@ -306,35 +306,33 @@ public PEHeaders PEHeaders } /// Error reading from the stream. + [MemberNotNull(nameof(_lazyPEHeaders))] private void InitializePEHeaders() { - StreamConstraints constraints; - Stream stream = GetPEImage().GetStream(out constraints); + MemoryBlockProvider peImage = GetPEImage(); PEHeaders headers; - if (constraints.GuardOpt != null) + // If the PE image is backed by a stream, use that to read the headers. + if (peImage.TryGetUnderlyingStream(out Stream? stream, out long imageStart, out int imageSize, out object? streamGuard)) { - lock (constraints.GuardOpt) + lock (streamGuard) { - headers = ReadPEHeadersNoLock(stream, constraints.ImageStart, constraints.ImageSize, IsLoadedImage); + Debug.Assert(imageStart >= 0 && imageStart <= stream.Length); + stream.Seek(imageStart, SeekOrigin.Begin); + headers = new PEHeaders(stream, imageSize, IsLoadedImage); } } + // Otherwise, get the memory block and wrap it in a stream. else { - headers = ReadPEHeadersNoLock(stream, constraints.ImageStart, constraints.ImageSize, IsLoadedImage); + // No need to acquire any lock here; GetStream() creates a new stream. + AbstractMemoryBlock memoryBlock = peImage.GetMemoryBlock(); + headers = new PEHeaders(memoryBlock.GetStream(), memoryBlock.Size, IsLoadedImage); } Interlocked.CompareExchange(ref _lazyPEHeaders, headers, null); } - /// Error reading from the stream. - private static PEHeaders ReadPEHeadersNoLock(Stream stream, long imageStartPosition, int imageSize, bool isLoadedImage) - { - Debug.Assert(imageStartPosition >= 0 && imageStartPosition <= stream.Length); - stream.Seek(imageStartPosition, SeekOrigin.Begin); - return new PEHeaders(stream, imageSize, isLoadedImage); - } - /// /// Returns a view of the entire image as a pointer and length. /// diff --git a/src/libraries/System.Reflection.Metadata/tests/PortableExecutable/PEReaderTests.cs b/src/libraries/System.Reflection.Metadata/tests/PortableExecutable/PEReaderTests.cs index 5aecb9ddc657de..d7010769ffeaa3 100644 --- a/src/libraries/System.Reflection.Metadata/tests/PortableExecutable/PEReaderTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/PortableExecutable/PEReaderTests.cs @@ -87,7 +87,6 @@ public void FromEmptyStream() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/17088")] public void SubStream() { var stream = new MemoryStream(); @@ -105,7 +104,7 @@ public void SubStream() stream.Position = 1; var peReader2 = new PEReader(stream, PEStreamOptions.LeaveOpen | PEStreamOptions.PrefetchMetadata, Misc.Members.Length); - Assert.Equal(Misc.Members.Length, peReader2.GetEntireImage().Length); + // We cannot call GetEntireImage() here; we have fetched only the metadata. peReader2.GetMetadataReader(); stream.Position = 1; diff --git a/src/libraries/System.Reflection.Metadata/tests/System.Reflection.Metadata.Tests.csproj b/src/libraries/System.Reflection.Metadata/tests/System.Reflection.Metadata.Tests.csproj index 5eb2bf7446e71b..e952494537be86 100644 --- a/src/libraries/System.Reflection.Metadata/tests/System.Reflection.Metadata.Tests.csproj +++ b/src/libraries/System.Reflection.Metadata/tests/System.Reflection.Metadata.Tests.csproj @@ -82,7 +82,7 @@ - +