From 7040d071adaf33ebd13a8f99cb1e4cc51557e9a0 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Wed, 27 Jan 2021 15:24:20 -0500 Subject: [PATCH] Add MemoryMarshal.CreateReadOnlySpanFromNullTerminated --- .../src/System/StubHelpers.cs | 19 +-- .../Globalization/FormatProvider.Number.cs | 12 +- .../src/System/IO/FileSystemWatcher.OSX.cs | 15 +-- .../System.Memory/ref/System.Memory.cs | 4 + .../CreateReadOnlySpanFromNullTerminated.cs | 120 ++++++++++++++++++ .../tests/System.Memory.Tests.csproj | 1 + .../System/Environment.Variables.Windows.cs | 8 +- .../System/Globalization/CalendarData.Icu.cs | 2 +- .../Runtime/InteropServices/MemoryMarshal.cs | 30 ++++- 9 files changed, 164 insertions(+), 47 deletions(-) create mode 100644 src/libraries/System.Memory/tests/MemoryMarshal/CreateReadOnlySpanFromNullTerminated.cs diff --git a/src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs index a12b3f211d7444..433c06957c02ef 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs @@ -183,24 +183,13 @@ internal static unsafe void ConvertFixedToNative(int flags, string strManaged, I internal static unsafe string ConvertFixedToManaged(IntPtr cstr, int length) { - int allocSize = length + 2; - if (allocSize < length) + int end = SpanHelpers.IndexOf(ref *(byte*)cstr, 0, length); + if (end != -1) { - throw new OutOfMemoryException(); + length = end; } - Span originalBuffer = new Span((byte*)cstr, length); - Span tempBuf = stackalloc sbyte[allocSize]; - - originalBuffer.CopyTo(tempBuf); - tempBuf[length - 1] = 0; - tempBuf[length] = 0; - tempBuf[length + 1] = 0; - - fixed (sbyte* buffer = tempBuf) - { - return new string(buffer, 0, string.strlen((byte*)buffer)); - } + return new string((sbyte*)cstr, 0, length); } } // class CSTRMarshaler diff --git a/src/libraries/Common/src/System/Globalization/FormatProvider.Number.cs b/src/libraries/Common/src/System/Globalization/FormatProvider.Number.cs index 4dbf801c263c6c..4311c3debc774c 100644 --- a/src/libraries/Common/src/System/Globalization/FormatProvider.Number.cs +++ b/src/libraries/Common/src/System/Globalization/FormatProvider.Number.cs @@ -897,23 +897,13 @@ private static void FormatCurrency(ref ValueStringBuilder sb, ref NumberBuffer n } } - private static unsafe int wcslen(char* s) - { - int result = 0; - while (*s++ != '\0') - { - result++; - } - return result; - } - private static unsafe void FormatFixed(ref ValueStringBuilder sb, ref NumberBuffer number, int nMinDigits, int nMaxDigits, NumberFormatInfo info, int[]? groupDigits, string sDecimal, string? sGroup) { Debug.Assert(sGroup != null || groupDigits == null); int digPos = number.scale; char* dig = number.digits; - int digLength = wcslen(dig); + int digLength = MemoryMarshal.CreateReadOnlySpanFromNullTerminated(dig).Length; if (digPos > 0) { diff --git a/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.OSX.cs b/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.OSX.cs index 4c159761063b7c..d07e9288792c2a 100644 --- a/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.OSX.cs +++ b/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.OSX.cs @@ -475,22 +475,15 @@ private unsafe void ProcessEvents(int numEvents, static ParsedEvent ParseEvent(byte* nativeEventPath) { - int byteCount = 0; Debug.Assert(nativeEventPath != null); - byte* temp = nativeEventPath; - // Finds the position of null character. - while (*temp != 0) - { - temp++; - byteCount++; - } + ReadOnlySpan eventPath = MemoryMarshal.CreateReadOnlySpanFromNullTerminated(nativeEventPath); + Debug.Assert(!eventPath.IsEmpty, "Empty events are not supported"); - Debug.Assert(byteCount > 0, "Empty events are not supported"); - char[] tempBuffer = ArrayPool.Shared.Rent(Encoding.UTF8.GetMaxCharCount(byteCount)); + char[] tempBuffer = ArrayPool.Shared.Rent(Encoding.UTF8.GetMaxCharCount(eventPath.Length)); // Converting an array of bytes to UTF-8 char array - int charCount = Encoding.UTF8.GetChars(new ReadOnlySpan(nativeEventPath, byteCount), tempBuffer); + int charCount = Encoding.UTF8.GetChars(eventPath, tempBuffer); return new ParsedEvent(tempBuffer.AsSpan(0, charCount), tempBuffer); } diff --git a/src/libraries/System.Memory/ref/System.Memory.cs b/src/libraries/System.Memory/ref/System.Memory.cs index 66e083e5cc1682..4605336934f944 100644 --- a/src/libraries/System.Memory/ref/System.Memory.cs +++ b/src/libraries/System.Memory/ref/System.Memory.cs @@ -497,6 +497,10 @@ public static partial class MemoryMarshal public static System.Span Cast(System.Span span) where TFrom : struct where TTo : struct { throw null; } public static System.Memory CreateFromPinnedArray(T[]? array, int start, int length) { throw null; } public static System.ReadOnlySpan CreateReadOnlySpan(ref T reference, int length) { throw null; } + [System.CLSCompliant(false)] + public static unsafe ReadOnlySpan CreateReadOnlySpanFromNullTerminated(byte* value) { throw null; } + [System.CLSCompliant(false)] + public static unsafe ReadOnlySpan CreateReadOnlySpanFromNullTerminated(char* value) { throw null; } public static System.Span CreateSpan(ref T reference, int length) { throw null; } public static ref T GetArrayDataReference(T[] array) { throw null; } public static ref T GetReference(System.ReadOnlySpan span) { throw null; } diff --git a/src/libraries/System.Memory/tests/MemoryMarshal/CreateReadOnlySpanFromNullTerminated.cs b/src/libraries/System.Memory/tests/MemoryMarshal/CreateReadOnlySpanFromNullTerminated.cs new file mode 100644 index 00000000000000..0b848524edd9da --- /dev/null +++ b/src/libraries/System.Memory/tests/MemoryMarshal/CreateReadOnlySpanFromNullTerminated.cs @@ -0,0 +1,120 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Xunit; +using System.Runtime.InteropServices; +using System.Buffers; +using Microsoft.DotNet.XUnitExtensions; + +namespace System.SpanTests +{ + public static partial class MemoryMarshalTests + { + [Fact] + public static unsafe void CreateReadOnlySpanFromNullTerminated_Char_Null() + { + Assert.True(MemoryMarshal.CreateReadOnlySpanFromNullTerminated((char*)null).IsEmpty); + Assert.True(MemoryMarshal.CreateReadOnlySpanFromNullTerminated((byte*)null).IsEmpty); + } + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(256)] + public static unsafe void CreateReadOnlySpanFromNullTerminated_Char(int expectedLength) + { + using BoundedMemory data = BoundedMemory.Allocate(expectedLength + 1); + data.Span.Fill('s'); + data.Span[^1] = '\0'; + data.MakeReadonly(); + + fixed (char* expectedPtr = data.Span) + { + ReadOnlySpan actual = MemoryMarshal.CreateReadOnlySpanFromNullTerminated(expectedPtr); + Assert.Equal(expectedLength, actual.Length); + fixed (char* actualPtr = &MemoryMarshal.GetReference(actual)) + { + Assert.Equal((IntPtr)expectedPtr, (IntPtr)actualPtr); + } + } + } + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(256)] + public static unsafe void CreateReadOnlySpanFromNullTerminated_Byte(int expectedLength) + { + using BoundedMemory data = BoundedMemory.Allocate(expectedLength + 1); + data.Span.Fill(0xFF); + data.Span[^1] = 0; + data.MakeReadonly(); + + fixed (byte* expectedPtr = data.Span) + { + ReadOnlySpan actual = MemoryMarshal.CreateReadOnlySpanFromNullTerminated(expectedPtr); + Assert.Equal(expectedLength, actual.Length); + fixed (byte* actualPtr = &MemoryMarshal.GetReference(actual)) + { + Assert.Equal((IntPtr)expectedPtr, (IntPtr)actualPtr); + } + } + } + + [OuterLoop] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.Is64BitProcess))] + public static unsafe void CreateReadOnlySpanFromNullTerminated_Char_ExceedsMaximum() + { + char* mem; + try + { + mem = (char*)Marshal.AllocHGlobal((IntPtr)(sizeof(char) * (2L + int.MaxValue))); + } + catch (OutOfMemoryException) + { + throw new SkipTestException("Unable to allocate 4GB of memory"); + } + + try + { + new Span(mem, int.MaxValue).Fill('s'); + *(mem + int.MaxValue) = 's'; + *(mem + int.MaxValue + 1) = '\0'; + + Assert.Throws(() => MemoryMarshal.CreateReadOnlySpanFromNullTerminated(mem)); + } + finally + { + Marshal.FreeHGlobal((IntPtr)mem); + } + } + + [OuterLoop] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.Is64BitProcess))] + public static unsafe void CreateReadOnlySpanFromNullTerminated_Byte_ExceedsMaximum() + { + byte* mem; + try + { + mem = (byte*)Marshal.AllocHGlobal((IntPtr)(2L + int.MaxValue)); + } + catch (OutOfMemoryException) + { + throw new SkipTestException("Unable to allocate 2GB of memory"); + } + + try + { + new Span(mem, int.MaxValue).Fill(0xFF); + *(mem + int.MaxValue) = 0xFF; + *(mem + int.MaxValue + 1) = 0; + + Assert.Throws(() => MemoryMarshal.CreateReadOnlySpanFromNullTerminated(mem)); + } + finally + { + Marshal.FreeHGlobal((IntPtr)mem); + } + } + } +} diff --git a/src/libraries/System.Memory/tests/System.Memory.Tests.csproj b/src/libraries/System.Memory/tests/System.Memory.Tests.csproj index d0508aaa6c78aa..3ef893f749cd9c 100644 --- a/src/libraries/System.Memory/tests/System.Memory.Tests.csproj +++ b/src/libraries/System.Memory/tests/System.Memory.Tests.csproj @@ -14,6 +14,7 @@ + diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Windows.cs index 382021ea055ca6..89e4bc7f45554b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Windows.cs @@ -84,14 +84,12 @@ public static unsafe IDictionary GetEnvironmentVariables() char* currentPtr = stringPtr; while (true) { - int variableLength = string.wcslen(currentPtr); - if (variableLength == 0) + ReadOnlySpan variable = MemoryMarshal.CreateReadOnlySpanFromNullTerminated(currentPtr); + if (variable.IsEmpty) { break; } - var variable = new ReadOnlySpan(currentPtr, variableLength); - // Find the = separating the key and value. We skip entries that begin with =. We also skip entries that don't // have =, which can happen on some older OSes when the environment block gets corrupted. int i = variable.IndexOf('='); @@ -111,7 +109,7 @@ public static unsafe IDictionary GetEnvironmentVariables() } // Move to the end of this variable, after its terminator. - currentPtr += variableLength + 1; + currentPtr += variable.Length + 1; } return results; diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/CalendarData.Icu.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/CalendarData.Icu.cs index af5db2379c4dbc..5a56d731ecbe8a 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Globalization/CalendarData.Icu.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/CalendarData.Icu.cs @@ -433,7 +433,7 @@ private static unsafe void EnumCalendarInfoCallback(char* calendarStringPtr, Int { try { - var calendarStringSpan = new ReadOnlySpan(calendarStringPtr, string.wcslen(calendarStringPtr)); + ReadOnlySpan calendarStringSpan = MemoryMarshal.CreateReadOnlySpanFromNullTerminated(calendarStringPtr); ref IcuEnumCalendarsData callbackContext = ref Unsafe.As(ref *(byte*)context); if (callbackContext.DisallowDuplicates) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/MemoryMarshal.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/MemoryMarshal.cs index 3a76f44da15fb8..51d79d5785a94f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/MemoryMarshal.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/MemoryMarshal.cs @@ -211,27 +211,49 @@ ref Unsafe.As(ref MemoryMarshal.GetReference(span)), } /// - /// Create a new span over a portion of a regular managed object. This can be useful + /// Creates a new span over a portion of a regular managed object. This can be useful /// if part of a managed object represents a "fixed array." This is dangerous because the /// is not checked. /// /// A reference to data. /// The number of elements the memory contains. - /// The lifetime of the returned span will not be validated for safety by span-aware languages. + /// A span representing the specified reference and length. + /// The lifetime of the returned span will not be validated for safety by span-aware languages. [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Span CreateSpan(ref T reference, int length) => new Span(ref reference, length); /// - /// Create a new read-only span over a portion of a regular managed object. This can be useful + /// Creates a new read-only span over a portion of a regular managed object. This can be useful /// if part of a managed object represents a "fixed array." This is dangerous because the /// is not checked. /// /// A reference to data. /// The number of elements the memory contains. - /// The lifetime of the returned span will not be validated for safety by span-aware languages. + /// A read-only span representing the specified reference and length. + /// The lifetime of the returned span will not be validated for safety by span-aware languages. [MethodImpl(MethodImplOptions.AggressiveInlining)] public static ReadOnlySpan CreateReadOnlySpan(ref T reference, int length) => new ReadOnlySpan(ref reference, length); + /// Creates a new read-only span for a null-terminated string. + /// The pointer to the null-terminated string of characters. + /// A read-only span representing the specified null-terminated string, or an empty span if the pointer is null. + /// The returned span does not include the null terminator. + /// The string is longer than . + [CLSCompliant(false)] + public static unsafe ReadOnlySpan CreateReadOnlySpanFromNullTerminated(char* value) => + value != null ? new ReadOnlySpan(value, string.wcslen(value)) : + default; + + /// Creates a new read-only span for a null-terminated UTF8 string. + /// The pointer to the null-terminated string of bytes. + /// A read-only span representing the specified null-terminated string, or an empty span if the pointer is null. + /// The returned span does not include the null terminator, nor does it validate the well-formedness of the UTF8 data. + /// The string is longer than . + [CLSCompliant(false)] + public static unsafe ReadOnlySpan CreateReadOnlySpanFromNullTerminated(byte* value) => + value != null ? new ReadOnlySpan(value, string.strlen(value)) : + default; + /// /// Get an array segment from the underlying memory. /// If unable to get the array segment, return false with a default array segment.