Skip to content

Commit

Permalink
Remove unsafe from BytesOrdinalEqualsStringAndAscii (#31201)
Browse files Browse the repository at this point in the history
  • Loading branch information
BrennanConroy authored Mar 24, 2021
1 parent 3535b83 commit dda8967
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 14 deletions.
4 changes: 2 additions & 2 deletions src/Servers/Kestrel/Core/test/AsciiDecoding.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ private void ExceptionThrownForZeroOrNonAscii(byte b)
{
var byteRange = Enumerable.Range(1, length).Select(x => (byte)x).ToArray();
byteRange[position] = b;

Assert.Throws<InvalidOperationException>(() => new Span<byte>(byteRange).GetAsciiStringNonNullCharacters());
}
}
Expand Down Expand Up @@ -147,7 +147,7 @@ static void Test(Span<byte> asciiBytes)

// Change byte back for next iteration, ensure is equal again
asciiBytes[i] = b;
Assert.True(StringUtilities.BytesOrdinalEqualsStringAndAscii(s, asciiBytes));
Assert.True(StringUtilities.BytesOrdinalEqualsStringAndAscii(s, asciiBytes), s);
}
}
}
Expand Down
24 changes: 12 additions & 12 deletions src/Shared/ServerInfrastructure/StringUtilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ out Unsafe.AsRef<Vector<ushort>>(output),
}

[MethodImpl(MethodImplOptions.AggressiveOptimization)]
public unsafe static bool BytesOrdinalEqualsStringAndAscii(string previousValue, ReadOnlySpan<byte> newValue)
public static bool BytesOrdinalEqualsStringAndAscii(string previousValue, ReadOnlySpan<byte> newValue)
{
// previousValue is a previously materialized string which *must* have already passed validation.
Debug.Assert(IsValidHeaderString(previousValue));
Expand All @@ -412,8 +412,8 @@ public unsafe static bool BytesOrdinalEqualsStringAndAscii(string previousValue,
// This isn't problematic as we know the maximum length is max string length (from test above)
// which is a signed value so half the size of the unsigned pointer value so we can safely add
// a Vector<byte>.Count to it without overflowing.
var count = (IntPtr)newValue.Length;
var offset = (IntPtr)0;
var count = (nint)newValue.Length;
var offset = (nint)0;

// Get references to the first byte in the span, and the first char in the string.
ref var bytes = ref MemoryMarshal.GetReference(newValue);
Expand All @@ -422,12 +422,12 @@ public unsafe static bool BytesOrdinalEqualsStringAndAscii(string previousValue,
do
{
// If Vector not-accelerated or remaining less than vector size
if (!Vector.IsHardwareAccelerated || (byte*)(offset + Vector<byte>.Count) > (byte*)count)
if (!Vector.IsHardwareAccelerated || (offset + Vector<byte>.Count) > count)
{
if (IntPtr.Size == 8) // Use Intrinsic switch for branch elimination
{
// 64-bit: Loop longs by default
while ((byte*)(offset + sizeof(long)) <= (byte*)count)
while ((offset + sizeof(long)) <= count)
{
if (!WidenFourAsciiBytesToUtf16AndCompareToChars(
ref Unsafe.Add(ref str, offset),
Expand All @@ -441,7 +441,7 @@ ref Unsafe.Add(ref str, offset + 4),

offset += sizeof(long);
}
if ((byte*)(offset + sizeof(int)) <= (byte*)count)
if ((offset + sizeof(int)) <= count)
{
if (!WidenFourAsciiBytesToUtf16AndCompareToChars(
ref Unsafe.Add(ref str, offset),
Expand All @@ -456,7 +456,7 @@ ref Unsafe.Add(ref str, offset),
else
{
// 32-bit: Loop ints by default
while ((byte*)(offset + sizeof(int)) <= (byte*)count)
while ((offset + sizeof(int)) <= count)
{
if (!WidenFourAsciiBytesToUtf16AndCompareToChars(
ref Unsafe.Add(ref str, offset),
Expand All @@ -468,7 +468,7 @@ ref Unsafe.Add(ref str, offset),
offset += sizeof(int);
}
}
if ((byte*)(offset + sizeof(short)) <= (byte*)count)
if ((offset + sizeof(short)) <= count)
{
if (!WidenTwoAsciiBytesToUtf16AndCompareToChars(
ref Unsafe.Add(ref str, offset),
Expand All @@ -479,7 +479,7 @@ ref Unsafe.Add(ref str, offset),

offset += sizeof(short);
}
if ((byte*)offset < (byte*)count)
if (offset < count)
{
var ch = (char)Unsafe.Add(ref bytes, offset);
if (((ch & 0x80) != 0) || Unsafe.Add(ref str, offset) != ch)
Expand Down Expand Up @@ -527,11 +527,11 @@ ref Unsafe.Add(ref str, offset),
}

offset += Vector<byte>.Count;
} while ((byte*)(offset + Vector<byte>.Count) <= (byte*)count);
} while ((offset + Vector<byte>.Count) <= count);

// Vector path done, loop back to do non-Vector
// If is a exact multiple of vector size, bail now
} while ((byte*)offset < (byte*)count);
} while (offset < count);

// If we get here (input is exactly a multiple of Vector length) then there are no inequalities via widening;
// so the input bytes are both ascii and a match to the string if it was converted via Encoding.ASCII.GetString(...)
Expand Down Expand Up @@ -651,7 +651,7 @@ private static bool AllBytesInUInt16AreAscii(ushort value)
return ((value & 0x8080u) == 0);
}

private unsafe static bool IsValidHeaderString(string value)
private static bool IsValidHeaderString(string value)
{
// Method for Debug.Assert to ensure BytesOrdinalEqualsStringAndAscii
// is not called with an unvalidated string comparitor.
Expand Down

0 comments on commit dda8967

Please sign in to comment.