XXHash64/32 incorrect one-shot implementation

[Tornhoof]

tl;dr: I upgraded one of my apps to .NET 6 and .NET 6 ships their own xxhash64 implementation, which is great. But the output was not consistent with my previous implementation. (beside mine not outputting bigendian, but that's a spec thingie).

runtime/src/libraries/System.IO.Hashing/src/System/IO/Hashing/XxHash64.cs

Line 218 in 69b5d67

while (source.Length > StripeSize)

This should be >=, you're not hashing all available data in the bulk/stride block.

Funny thing, the incremental version is correct:

runtime/src/libraries/System.IO.Hashing/src/System/IO/Hashing/XxHash64.cs

Line 92 in 69b5d67

while (source.Length >= StripeSize)

Looking through the test cases, I see cases which should test boundaries, but only like 33 and 63, but not testing the power of two case and I don't see any test case (maybe I missed those, the tests are slightly convoluted for these algorithms) which compare one-shot with incremental.

The same problem is in XxHash32.

Repro:

// See https://aka.ms/new-console-template for more information

using System.Security.Cryptography;

for (int i = 127; i <= 129; i++)
{
	Console.WriteLine("Bytes-Length: {0}", i);
	var bytes = new byte[i];
	RandomNumberGenerator.Fill(bytes);

	var h = new System.IO.Hashing.XxHash64();
	h.Append(bytes);
	Console.WriteLine("System.IO.Hashing.XxHash64 Incremental: {0:X}", BitConverter.ToUInt64(h.GetHashAndReset()));

	var netOutput = System.IO.Hashing.XxHash64.Hash(bytes);
	Console.WriteLine("System.IO.Hashing.XxHash64: {0:X}", BitConverter.ToUInt64(netOutput));

	var h2 = new System.IO.Hashing.XxHash32();
	h2.Append(bytes);
	Console.WriteLine("System.IO.Hashing.XxHash32 Incremental: {0:X}", BitConverter.ToUInt32(h2.GetHashAndReset()));

	var netOutput2 = System.IO.Hashing.XxHash32.Hash(bytes);
	Console.WriteLine("System.IO.Hashing.XxHash32: {0:X}", BitConverter.ToUInt32(netOutput2));
}

Output is:

Bytes-Length: 127
System.IO.Hashing.XxHash64 Incremental: DAF756640FB36470
System.IO.Hashing.XxHash64: DAF756640FB36470
System.IO.Hashing.XxHash32 Incremental: 582BE2A5
System.IO.Hashing.XxHash32: 582BE2A5
Bytes-Length: 128
System.IO.Hashing.XxHash64 Incremental: 3E389AA2FCAFBF14
System.IO.Hashing.XxHash64: F3F710BA03289AD5
System.IO.Hashing.XxHash32 Incremental: 26D01A09
System.IO.Hashing.XxHash32: B643E2D7
Bytes-Length: 129
System.IO.Hashing.XxHash64 Incremental: 6C6BA559540903BF
System.IO.Hashing.XxHash64: 6C6BA559540903BF
System.IO.Hashing.XxHash32 Incremental: E2B400A0
System.IO.Hashing.XxHash32: E2B400A0

(note: I don't like the new multi part entry bug template, it is not possible to prepare the report in an external editor anymore or even back it up to an editor, which I usually do, so I don't accidentially lose the content when I close my browser).

@bartonjs might be of interest for you.

I'll throw up a PR to fix this, if I'll understand the test cases.

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

---

tl;dr: I upgraded one of my apps to .NET 6 and .NET 6 ships their own xxhash64 implementation, which is great. But the output was not consistent with my previous implementation. (beside my not outputting bigendian, but that's a spec thingie).

runtime/src/libraries/System.IO.Hashing/src/System/IO/Hashing/XxHash64.cs

Line 218 in 69b5d67

while (source.Length > StripeSize)

This should be >=, you're not hashing all data in the bulk block.

Funny thing, the incremental version is correct:

runtime/src/libraries/System.IO.Hashing/src/System/IO/Hashing/XxHash64.cs

Line 92 in 69b5d67

while (source.Length >= StripeSize)

Looking through the test cases, I see cases which should test boundaries, but only like 33 and 63, but not testing the power of two case and I don't see any test case (maybe I missed those, the tests are slightly convoluted for these algorithms) which compare one-shot with incremental.

The same problem is in XxHash32.

Repro:

// See https://aka.ms/new-console-template for more information

using System.Security.Cryptography;

for (int i = 127; i <= 129; i++)
{
	Console.WriteLine("Bytes-Length: {0}", i);
	var bytes = new byte[i];
	RandomNumberGenerator.Fill(bytes);

	var h = new System.IO.Hashing.XxHash64();
	h.Append(bytes);
	Console.WriteLine("System.IO.Hashing.XxHash64 Incremental: {0:X}", BitConverter.ToUInt64(h.GetHashAndReset()));

	var netOutput = System.IO.Hashing.XxHash64.Hash(bytes);
	Console.WriteLine("System.IO.Hashing.XxHash64: {0:X}", BitConverter.ToUInt64(netOutput));

	var h2 = new System.IO.Hashing.XxHash32();
	h2.Append(bytes);
	Console.WriteLine("System.IO.Hashing.XxHash32 Incremental: {0:X}", BitConverter.ToUInt32(h2.GetHashAndReset()));

	var netOutput2 = System.IO.Hashing.XxHash32.Hash(bytes);
	Console.WriteLine("System.IO.Hashing.XxHash32: {0:X}", BitConverter.ToUInt32(netOutput2));
}

Output is:

Bytes-Length: 127
System.IO.Hashing.XxHash64 Incremental: DAF756640FB36470
System.IO.Hashing.XxHash64: DAF756640FB36470
System.IO.Hashing.XxHash32 Incremental: 582BE2A5
System.IO.Hashing.XxHash32: 582BE2A5
Bytes-Length: 128
System.IO.Hashing.XxHash64 Incremental: 3E389AA2FCAFBF14
System.IO.Hashing.XxHash64: F3F710BA03289AD5
System.IO.Hashing.XxHash32 Incremental: 26D01A09
System.IO.Hashing.XxHash32: B643E2D7
Bytes-Length: 129
System.IO.Hashing.XxHash64 Incremental: 6C6BA559540903BF
System.IO.Hashing.XxHash64: 6C6BA559540903BF
System.IO.Hashing.XxHash32 Incremental: E2B400A0
System.IO.Hashing.XxHash32: E2B400A0

(note: I don't like the new multi part entry bug template, it is not possible to prepare the report in an external editor anymore or even back it up to an editor, which I usually do, so I don't accidentially loose the content when I close my browser).

@bartonjs might be of interest for you.

Author:	Tornhoof
Assignees:	-
Labels:	area-System.IO, untriaged
Milestone:	-

[danmoseley]

@Tornhoof

(note: I don't like the new multi part entry bug template, it is not possible to prepare the report in an external editor anymore or even back it up to an editor, which I usually do, so I don't accidentially lose the content when I close my browser).

Feel free to pick "blank" from the list of buttons when you open an issue. (I have the same preference)