ensure MetadataReader fast path works with every FileStream on every OS

[adamsitnik]

This code predates me, but according to my understanding:

1. We had a Windows-specific optimization:

runtime/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/FileStreamReadLightUp.cs

Lines 12 to 17 in 49e5d17

	private static bool IsReadFileAvailable =>
	#if NETCOREAPP
	OperatingSystem.IsWindows();
	#else
	Path.DirectorySeparatorChar == '\\';
	#endif

2. That was calling ReadFile syscall:

runtime/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/FileStreamReadLightUp.cs

Line 58 in 49e5d17

int result = Interop.Kernel32.ReadFile(handle, buffer, size, out int bytesRead, IntPtr.Zero);

3. That was working only for sync FileStreams:

runtime/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/FileStreamReadLightUp.cs

Lines 62 to 66 in 49e5d17

	// We used to throw here, but this is where we land if the FileStream was
	// opened with useAsync: true, which is currently the default on .NET Core.
	// https://github.com/dotnet/corefx/pull/987 filed to look in to how best to
	// handle this, but in the meantime, we'll fall back to the slower code path
	// just as in the case where the native API is unavailable in the current platform.

Based on the comment from #14276

Perhaps, this is the right opportunity to define a portable FileStream.Read overload that reads to unmanaged memory?

It seems that this code was created before we have added a Stream.Read(Span) overload that allows for reading from a file to an unmanaged memory buffer.

Moreover, I think that partial reads were not handled properly, as in cases when bytesRead != requestedSize the block.Pointer was not incremented properly:

runtime/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/StreamMemoryBlockProvider.cs

Lines 79 to 81 in 79ae74f

	if (!isFileStream || !FileStreamReadLightUp.TryReadFile(stream, block.Pointer, start, size))
	{
	stream.CopyTo(block.Pointer, size);

fixes #14276

[jkotas]

It looks good to me as is.

[stephentoub]

Consider changing the signature of TryReadFile from:

bool TryReadFile(Stream stream, byte* buffer, int size, out int bytesRead)

to instead be:

int ReadFile(Stream stream, byte* buffer, int size)

The sole caller of TryReadFile always proceeds to use the stream and the out bytesRead if TryReadFile returns false, so it seems like it'd be more understandable if instead of:

int bytesRead = 0;
if (!isFileStream || !FileStreamReadLightUp.TryReadFile(stream, block.Pointer, size, out bytesRead))
{
    stream.CopyTo(block.Pointer + bytesRead, size - bytesRead);

the caller was:

int bytesRead = 0;
if (!isFileStream ||
    (bytesRead = FileStreamReadLightUp.ReadFile(stream, block.Pointer, size)) != size)
{
    stream.CopyTo(block.Pointer + bytesRead, size - bytesRead);

That way it's clear to the caller that the stream may have still advanced even if TryReadFile returned false, that bytesRead is still meaningful, etc., and it's clear why bytesRead is being factored into the CopyTo.