Remove span pinning associated with string.Create (#78914)

* Remove span pinning associated with string.Create

This is now possible with C# 11.

* Address PR feedback

Author: stephentoub

src/libraries/Common/src/System/HexConverter.cs

@@ -208,14 +208,10 @@ public static unsafe string ToString(ReadOnlySpan<byte> bytes, Casing casing = C
             }
             return result.ToString();
 #else
-            fixed (byte* bytesPtr = bytes)
-            {
-                return string.Create(bytes.Length * 2, (Ptr: (IntPtr)bytesPtr, bytes.Length, casing), static (chars, args) =>
-                {
-                    var ros = new ReadOnlySpan<byte>((byte*)args.Ptr, args.Length);
-                    EncodeToUtf16(ros, chars, args.casing);
-                });
-            }
+#pragma warning disable CS8500 // takes address of managed type
+            return string.Create(bytes.Length * 2, (RosPtr: (IntPtr)(&bytes), casing), static (chars, args) =>
+                EncodeToUtf16(*(ReadOnlySpan<byte>*)args.RosPtr, chars, args.casing));
+#pragma warning restore CS8500
 #endif
         }
 

src/libraries/Common/src/System/IO/Archiving.Utils.Windows.cs

@@ -61,22 +61,22 @@ public static unsafe string EntryFromPath(ReadOnlySpan<char> path, bool appendPa
                     string.Empty;
             }
 
-            fixed (char* pathPtr = &MemoryMarshal.GetReference(path))
+#pragma warning disable CS8500 // takes address of managed type
+            ReadOnlySpan<char> tmpPath = path; // avoid address exposing the span and impacting the other code in the method that uses it
+            return string.Create(appendPathSeparator ? tmpPath.Length + 1 : tmpPath.Length, (appendPathSeparator, RosPtr: (IntPtr)(&tmpPath)), static (dest, state) =>
             {
-                return string.Create(appendPathSeparator ? path.Length + 1 : path.Length, (appendPathSeparator, (IntPtr)pathPtr, path.Length), static (dest, state) =>
+                var path = *(ReadOnlySpan<char>*)state.RosPtr;
+                path.CopyTo(dest);
+                if (state.appendPathSeparator)
                 {
-                    ReadOnlySpan<char> path = new ReadOnlySpan<char>((char*)state.Item2, state.Length);
-                    path.CopyTo(dest);
-                    if (state.appendPathSeparator)
-                    {
-                        dest[^1] = '/';
-                    }
+                    dest[^1] = '/';
+                }
 
-                    // To ensure tar files remain compatible with Unix, and per the ZIP File Format Specification 4.4.17.1,
-                    // all slashes should be forward slashes.
-                    dest.Replace('\\', '/');
-                });
-            }
+                // To ensure tar files remain compatible with Unix, and per the ZIP File Format Specification 4.4.17.1,
+                // all slashes should be forward slashes.
+                dest.Replace('\\', '/');
+            });
+#pragma warning restore CS8500
         }
     }
 }

src/libraries/System.Private.CoreLib/src/System/IO/Path.cs

@@ -686,132 +686,91 @@ private static unsafe string JoinInternal(ReadOnlySpan<char> first, ReadOnlySpan
         {
             Debug.Assert(first.Length > 0 && second.Length > 0, "should have dealt with empty paths");
 
-            bool hasSeparator = PathInternal.IsDirectorySeparator(first[first.Length - 1])
-                || PathInternal.IsDirectorySeparator(second[0]);
+            bool hasSeparator = PathInternal.IsDirectorySeparator(first[^1]) || PathInternal.IsDirectorySeparator(second[0]);
 
             return hasSeparator ?
                 string.Concat(first, second) :
                 string.Concat(first, PathInternal.DirectorySeparatorCharAsString, second);
         }
 
-        private readonly unsafe struct Join3Payload
-        {
-            public Join3Payload(char* first, int firstLength, char* second, int secondLength, char* third, int thirdLength, byte separators)
-            {
-                First = first;
-                FirstLength = firstLength;
-                Second = second;
-                SecondLength = secondLength;
-                Third = third;
-                ThirdLength = thirdLength;
-                Separators = separators;
-            }
-
-            public readonly char* First;
-            public readonly int FirstLength;
-            public readonly char* Second;
-            public readonly int SecondLength;
-            public readonly char* Third;
-            public readonly int ThirdLength;
-            public readonly byte Separators;
-        }
-
         private static unsafe string JoinInternal(ReadOnlySpan<char> first, ReadOnlySpan<char> second, ReadOnlySpan<char> third)
         {
             Debug.Assert(first.Length > 0 && second.Length > 0 && third.Length > 0, "should have dealt with empty paths");
 
-            byte firstNeedsSeparator = PathInternal.IsDirectorySeparator(first[first.Length - 1])
-                || PathInternal.IsDirectorySeparator(second[0]) ? (byte)0 : (byte)1;
-            byte secondNeedsSeparator = PathInternal.IsDirectorySeparator(second[second.Length - 1])
-                || PathInternal.IsDirectorySeparator(third[0]) ? (byte)0 : (byte)1;
+            bool firstHasSeparator = PathInternal.IsDirectorySeparator(first[^1]) || PathInternal.IsDirectorySeparator(second[0]);
+            bool secondHasSeparator = PathInternal.IsDirectorySeparator(second[^1]) || PathInternal.IsDirectorySeparator(third[0]);
 
-            fixed (char* f = &MemoryMarshal.GetReference(first), s = &MemoryMarshal.GetReference(second), t = &MemoryMarshal.GetReference(third))
+            return (firstHasSeparator, secondHasSeparator) switch
             {
-                var payload = new Join3Payload(
-                    f, first.Length, s, second.Length, t, third.Length,
-                    (byte)(firstNeedsSeparator | secondNeedsSeparator << 1));
-
-                return string.Create(
-                    first.Length + second.Length + third.Length + firstNeedsSeparator + secondNeedsSeparator,
-                    (IntPtr)(&payload),
-                    static (destination, statePtr) =>
-                    {
-                        ref Join3Payload state = ref *(Join3Payload*)statePtr;
-                        new Span<char>(state.First, state.FirstLength).CopyTo(destination);
-                        if ((state.Separators & 0b1) != 0)
-                            destination[state.FirstLength] = PathInternal.DirectorySeparatorChar;
-                        new Span<char>(state.Second, state.SecondLength).CopyTo(destination.Slice(state.FirstLength + (state.Separators & 0b1)));
-                        if ((state.Separators & 0b10) != 0)
-                            destination[destination.Length - state.ThirdLength - 1] = PathInternal.DirectorySeparatorChar;
-                        new Span<char>(state.Third, state.ThirdLength).CopyTo(destination.Slice(destination.Length - state.ThirdLength));
-                    });
-            }
+                (false, false) => string.Concat(first, PathInternal.DirectorySeparatorCharAsString, second, PathInternal.DirectorySeparatorCharAsString, third),
+                (false, true) => string.Concat(first, PathInternal.DirectorySeparatorCharAsString, second, third),
+                (true, false) => string.Concat(first, second, PathInternal.DirectorySeparatorCharAsString, third),
+                (true, true) => string.Concat(first, second, third),
+            };
         }
 
-        private readonly unsafe struct Join4Payload
+        private static unsafe string JoinInternal(ReadOnlySpan<char> first, ReadOnlySpan<char> second, ReadOnlySpan<char> third, ReadOnlySpan<char> fourth)
         {
-            public Join4Payload(char* first, int firstLength, char* second, int secondLength, char* third, int thirdLength, char* fourth, int fourthLength, byte separators)
+            Debug.Assert(first.Length > 0 && second.Length > 0 && third.Length > 0 && fourth.Length > 0, "should have dealt with empty paths");
+
+#pragma warning disable CS8500 // This takes the address of, gets the size of, or declares a pointer to a managed type
+            var state = new JoinInternalState
             {
-                First = first;
-                FirstLength = firstLength;
-                Second = second;
-                SecondLength = secondLength;
-                Third = third;
-                ThirdLength = thirdLength;
-                Fourth = fourth;
-                FourthLength = fourthLength;
-                Separators = separators;
-            }
+                ReadOnlySpanPtr1 = (IntPtr)(&first),
+                ReadOnlySpanPtr2 = (IntPtr)(&second),
+                ReadOnlySpanPtr3 = (IntPtr)(&third),
+                ReadOnlySpanPtr4 = (IntPtr)(&fourth),
+                NeedSeparator1 = PathInternal.IsDirectorySeparator(first[^1]) || PathInternal.IsDirectorySeparator(second[0]) ? (byte)0 : (byte)1,
+                NeedSeparator2 = PathInternal.IsDirectorySeparator(second[^1]) || PathInternal.IsDirectorySeparator(third[0]) ? (byte)0 : (byte)1,
+                NeedSeparator3 = PathInternal.IsDirectorySeparator(third[^1]) || PathInternal.IsDirectorySeparator(fourth[0]) ? (byte)0 : (byte)1,
+            };
 
-            public readonly char* First;
-            public readonly int FirstLength;
-            public readonly char* Second;
-            public readonly int SecondLength;
-            public readonly char* Third;
-            public readonly int ThirdLength;
-            public readonly char* Fourth;
-            public readonly int FourthLength;
-            public readonly byte Separators;
-        }
+            return string.Create(
+                first.Length + second.Length + third.Length + fourth.Length + state.NeedSeparator1 + state.NeedSeparator2 + state.NeedSeparator3,
+                state,
+                static (destination, state) =>
+                {
+                    ReadOnlySpan<char> first = *(ReadOnlySpan<char>*)state.ReadOnlySpanPtr1;
+                    first.CopyTo(destination);
+                    destination = destination.Slice(first.Length);
 
-        private static unsafe string JoinInternal(ReadOnlySpan<char> first, ReadOnlySpan<char> second, ReadOnlySpan<char> third, ReadOnlySpan<char> fourth)
-        {
-            Debug.Assert(first.Length > 0 && second.Length > 0 && third.Length > 0 && fourth.Length > 0, "should have dealt with empty paths");
+                    if (state.NeedSeparator1 != 0)
+                    {
+                        destination[0] = PathInternal.DirectorySeparatorChar;
+                        destination = destination.Slice(1);
+                    }
 
-            byte firstNeedsSeparator = PathInternal.IsDirectorySeparator(first[first.Length - 1])
-                || PathInternal.IsDirectorySeparator(second[0]) ? (byte)0 : (byte)1;
-            byte secondNeedsSeparator = PathInternal.IsDirectorySeparator(second[second.Length - 1])
-                || PathInternal.IsDirectorySeparator(third[0]) ? (byte)0 : (byte)1;
-            byte thirdNeedsSeparator = PathInternal.IsDirectorySeparator(third[third.Length - 1])
-                || PathInternal.IsDirectorySeparator(fourth[0]) ? (byte)0 : (byte)1;
+                    ReadOnlySpan<char> second = *(ReadOnlySpan<char>*)state.ReadOnlySpanPtr2;
+                    second.CopyTo(destination);
+                    destination = destination.Slice(second.Length);
 
-            fixed (char* f = &MemoryMarshal.GetReference(first), s = &MemoryMarshal.GetReference(second), t = &MemoryMarshal.GetReference(third), u = &MemoryMarshal.GetReference(fourth))
-            {
-                var payload = new Join4Payload(
-                    f, first.Length, s, second.Length, t, third.Length, u, fourth.Length,
-                    (byte)(firstNeedsSeparator | secondNeedsSeparator << 1 | thirdNeedsSeparator << 2));
-
-                return string.Create(
-                    first.Length + second.Length + third.Length + fourth.Length + firstNeedsSeparator + secondNeedsSeparator + thirdNeedsSeparator,
-                    (IntPtr)(&payload),
-                    static (destination, statePtr) =>
+                    if (state.NeedSeparator2 != 0)
                     {
-                        ref Join4Payload state = ref *(Join4Payload*)statePtr;
-                        new Span<char>(state.First, state.FirstLength).CopyTo(destination);
-                        int insertionPoint = state.FirstLength;
-                        if ((state.Separators & 0b1) != 0)
-                            destination[insertionPoint++] = PathInternal.DirectorySeparatorChar;
-                        new Span<char>(state.Second, state.SecondLength).CopyTo(destination.Slice(insertionPoint));
-                        insertionPoint += state.SecondLength;
-                        if ((state.Separators & 0b10) != 0)
-                            destination[insertionPoint++] = PathInternal.DirectorySeparatorChar;
-                        new Span<char>(state.Third, state.ThirdLength).CopyTo(destination.Slice(insertionPoint));
-                        insertionPoint += state.ThirdLength;
-                        if ((state.Separators & 0b100) != 0)
-                            destination[insertionPoint++] = PathInternal.DirectorySeparatorChar;
-                        new Span<char>(state.Fourth, state.FourthLength).CopyTo(destination.Slice(insertionPoint));
-                    });
-            }
+                        destination[0] = PathInternal.DirectorySeparatorChar;
+                        destination = destination.Slice(1);
+                    }
+
+                    ReadOnlySpan<char> third = *(ReadOnlySpan<char>*)state.ReadOnlySpanPtr3;
+                    third.CopyTo(destination);
+                    destination = destination.Slice(third.Length);
+
+                    if (state.NeedSeparator3 != 0)
+                    {
+                        destination[0] = PathInternal.DirectorySeparatorChar;
+                        destination = destination.Slice(1);
+                    }
+
+                    ReadOnlySpan<char> fourth = *(ReadOnlySpan<char>*)state.ReadOnlySpanPtr4;
+                    Debug.Assert(fourth.Length == destination.Length);
+                    fourth.CopyTo(destination);
+                });
+#pragma warning restore CS8500
+        }
+
+        private struct JoinInternalState // used to avoid rooting ValueTuple`7
+        {
+            public IntPtr ReadOnlySpanPtr1, ReadOnlySpanPtr2, ReadOnlySpanPtr3, ReadOnlySpanPtr4;
+            public byte NeedSeparator1, NeedSeparator2, NeedSeparator3;
         }
 
         private static ReadOnlySpan<byte> Base32Char => "abcdefghijklmnopqrstuvwxyz012345"u8;

src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs

@@ -379,6 +379,34 @@ public static string Concat(ReadOnlySpan<char> str0, ReadOnlySpan<char> str1, Re
             return result;
         }
 
+        internal static string Concat(ReadOnlySpan<char> str0, ReadOnlySpan<char> str1, ReadOnlySpan<char> str2, ReadOnlySpan<char> str3, ReadOnlySpan<char> str4)
+        {
+            int length = checked(str0.Length + str1.Length + str2.Length + str3.Length + str4.Length);
+            if (length == 0)
+            {
+                return Empty;
+            }
+
+            string result = FastAllocateString(length);
+            Span<char> resultSpan = new Span<char>(ref result._firstChar, result.Length);
+
+            str0.CopyTo(resultSpan);
+            resultSpan = resultSpan.Slice(str0.Length);
+
+            str1.CopyTo(resultSpan);
+            resultSpan = resultSpan.Slice(str1.Length);
+
+            str2.CopyTo(resultSpan);
+            resultSpan = resultSpan.Slice(str2.Length);
+
+            str3.CopyTo(resultSpan);
+            resultSpan = resultSpan.Slice(str3.Length);
+
+            str4.CopyTo(resultSpan);
+
+            return result;
+        }
+
         public static string Concat(params string?[] values)
         {
             ArgumentNullException.ThrowIfNull(values);

src/libraries/System.Private.Uri/src/System/Uri.cs

@@ -3840,15 +3840,14 @@ private static unsafe ParsingError CheckSchemeSyntax(ReadOnlySpan<char> span, re
             }
 
             // Then look up the syntax in a string-based table.
-            string str;
-            fixed (char* pSpan = span)
+#pragma warning disable CS8500 // takes address of managed type
+            ReadOnlySpan<char> tmpSpan = span; // avoid address exposing the span and impacting the other code in the method that uses it
+            string str = string.Create(tmpSpan.Length, (IntPtr)(&tmpSpan), (buffer, spanPtr) =>
             {
-                str = string.Create(span.Length, (ip: (IntPtr)pSpan, length: span.Length), (buffer, state) =>
-                {
-                    int charsWritten = new ReadOnlySpan<char>((char*)state.ip, state.length).ToLowerInvariant(buffer);
-                    Debug.Assert(charsWritten == buffer.Length);
-                });
-            }
+                int charsWritten = (*(ReadOnlySpan<char>*)spanPtr).ToLowerInvariant(buffer);
+                Debug.Assert(charsWritten == buffer.Length);
+            });
+#pragma warning restore CS8500
             syntax = UriParser.FindOrFetchAsUnknownV1Syntax(str);
             return ParsingError.None;
         }

src/libraries/System.Private.Uri/src/System/UriHelper.cs

@@ -543,22 +543,21 @@ internal static unsafe string StripBidiControlCharacters(ReadOnlySpan<char> strT
                 return backingString ?? new string(strToClean);
             }
 
-            fixed (char* pStrToClean = &MemoryMarshal.GetReference(strToClean))
+#pragma warning disable CS8500 // takes address of managed type
+            ReadOnlySpan<char> tmpStrToClean = strToClean; // avoid address exposing the span and impacting the other code in the method that uses it
+            return string.Create(tmpStrToClean.Length - charsToRemove, (IntPtr)(&tmpStrToClean), static (buffer, strToCleanPtr) =>
             {
-                return string.Create(strToClean.Length - charsToRemove, (StrToClean: (IntPtr)pStrToClean, strToClean.Length), static (buffer, state) =>
+                int destIndex = 0;
+                foreach (char c in *(ReadOnlySpan<char>*)strToCleanPtr)
                 {
-                    var strToClean = new ReadOnlySpan<char>((char*)state.StrToClean, state.Length);
-                    int destIndex = 0;
-                    foreach (char c in strToClean)
+                    if (!IsBidiControlCharacter(c))
                     {
-                        if (!IsBidiControlCharacter(c))
-                        {
-                            buffer[destIndex++] = c;
-                        }
+                        buffer[destIndex++] = c;
                     }
-                    Debug.Assert(buffer.Length == destIndex);
-                });
-            }
+                }
+                Debug.Assert(buffer.Length == destIndex);
+            });
+#pragma warning restore CS8500
         }
     }
 }