diff --git a/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs b/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs index d208eb3bedf63a..74c0c243d480bc 100644 --- a/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs +++ b/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Numerics; using System.Runtime.InteropServices; @@ -1846,36 +1847,50 @@ private static void CheckStringSplitOptions(StringSplitOptions options) // Returns a substring of this string. // - public string Substring(int startIndex) => Substring(startIndex, Length - startIndex); - - public string Substring(int startIndex, int length) + public string Substring(int startIndex) { - if (startIndex < 0) + if (startIndex == 0) { - throw new ArgumentOutOfRangeException(nameof(startIndex), SR.ArgumentOutOfRange_StartIndex); + return this; } - if (startIndex > Length) + int thisLength = Length; + if (startIndex == thisLength) { - throw new ArgumentOutOfRangeException(nameof(startIndex), SR.ArgumentOutOfRange_StartIndexLargerThanLength); + return string.Empty; } - if (length < 0) - { - throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_NegativeLength); - } + // Copy Span.Slice checks for perf + if ((uint)startIndex > (uint)thisLength) + ThrowSubstringArgumentOutRangeException(startIndex, thisLength - startIndex); - if (startIndex > Length - length) - { - throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_IndexLength); - } + return InternalSubString(startIndex, thisLength - startIndex); + } + + public string Substring(int startIndex, int length) + { + // Copy Span.Slice checks for perf + int thisLength = Length; +#if TARGET_64BIT + // Since start and length are both 32-bit, their sum can be computed across a 64-bit domain + // without loss of fidelity. The cast to uint before the cast to ulong ensures that the + // extension from 32- to 64-bit is zero-extending rather than sign-extending. The end result + // of this is that if either input is negative or if the input sum overflows past Int32.MaxValue, + // that information is captured correctly in the comparison against the backing _length field. + // We don't use this same mechanism in a 32-bit process due to the overhead of 64-bit arithmetic. + if ((ulong)(uint)startIndex + (ulong)(uint)length > (ulong)(uint)thisLength) + ThrowSubstringArgumentOutRangeException(startIndex, length); +#else + if ((uint)startIndex > (uint)thisLength || (uint)length > (uint)(thisLength - startIndex)) + ThrowSubstringArgumentOutRangeException(startIndex, length); +#endif if (length == 0) { return string.Empty; } - if (startIndex == 0 && length == this.Length) + if (length == thisLength) { return this; } @@ -1883,6 +1898,26 @@ public string Substring(int startIndex, int length) return InternalSubString(startIndex, length); } + [DoesNotReturn] + private void ThrowSubstringArgumentOutRangeException(int startIndex, int length) + { + if (startIndex < 0) + { + throw new ArgumentOutOfRangeException(nameof(startIndex), SR.ArgumentOutOfRange_StartIndex); + } + if (startIndex > Length) + { + throw new ArgumentOutOfRangeException(nameof(startIndex), SR.ArgumentOutOfRange_StartIndexLargerThanLength); + } + if (length < 0) + { + throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_NegativeLength); + } + // Check not needed since we only call this when startIndex, length are out of range + //if (startIndex > Length - length) + throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_IndexLength); + } + private string InternalSubString(int startIndex, int length) { Debug.Assert(startIndex >= 0 && startIndex <= this.Length, "StartIndex is out of range!");