-
Notifications
You must be signed in to change notification settings - Fork 12.7k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
SipHasher128: fix platform-independence confusion
StableHasher is supposed to ensure platform independence by converting integers to little-endian and extending isize and usize to 64 bits as necessary, but in fact, much of that work is already handled by SipHasher128. In particular, SipHasher128 implements short_write in an endian-independent way, yet both StableHasher and SipHasher128 additionally attempt to achieve endian-independence by byte swapping on BE hardware before invoking short writes. This double swap has no effect, so let's remove it. Because short_write is endian-independent, SipHasher128 is already handling part of the platform-independence, and it would be somewhat difficult to make it *not* handle that part with the current implementation. As splitting platform-independence responsibilities between StableHasher and SipHasher128 would be confusing, let's make SipHasher128 handle all of it. Finally, update some incorrect comments and increase test coverage. Unit tests pass on both LE and BE systems.
- Loading branch information
1 parent
fc2daaa
commit eb0a88f
Showing
4 changed files
with
183 additions
and
51 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
use super::*; | ||
|
||
// The tests below compare the computed hashes to particular expected values | ||
// in order to test that we produce the same results on different platforms, | ||
// regardless of endianness and `usize` and `isize` size differences (this | ||
// of course assumes we run these tests on platforms that differ in those | ||
// ways). The expected values depend on the hashing algorithm used, so they | ||
// need to be updated whenever StableHasher changes its hashing algorithm. | ||
|
||
#[test] | ||
fn test_hash_integers() { | ||
// Test that integers are handled consistently across platforms. | ||
let test_u8 = 0xAB_u8; | ||
let test_u16 = 0xFFEE_u16; | ||
let test_u32 = 0x445577AA_u32; | ||
let test_u64 = 0x01234567_13243546_u64; | ||
let test_u128 = 0x22114433_66557788_99AACCBB_EEDDFF77_u128; | ||
let test_usize = 0xD0C0B0A0_usize; | ||
|
||
let test_i8 = -100_i8; | ||
let test_i16 = -200_i16; | ||
let test_i32 = -300_i32; | ||
let test_i64 = -400_i64; | ||
let test_i128 = -500_i128; | ||
let test_isize = -600_isize; | ||
|
||
let mut h = StableHasher::new(); | ||
test_u8.hash(&mut h); | ||
test_u16.hash(&mut h); | ||
test_u32.hash(&mut h); | ||
test_u64.hash(&mut h); | ||
test_u128.hash(&mut h); | ||
test_usize.hash(&mut h); | ||
test_i8.hash(&mut h); | ||
test_i16.hash(&mut h); | ||
test_i32.hash(&mut h); | ||
test_i64.hash(&mut h); | ||
test_i128.hash(&mut h); | ||
test_isize.hash(&mut h); | ||
|
||
// This depends on the hashing algorithm. See note at top of file. | ||
let expected = (2736651863462566372, 8121090595289675650); | ||
|
||
assert_eq!(h.finalize(), expected); | ||
} | ||
|
||
#[test] | ||
fn test_hash_usize() { | ||
// Test that usize specifically is handled consistently across platforms. | ||
let test_usize = 0xABCDEF01_usize; | ||
|
||
let mut h = StableHasher::new(); | ||
test_usize.hash(&mut h); | ||
|
||
// This depends on the hashing algorithm. See note at top of file. | ||
let expected = (5798740672699530587, 11186240177685111648); | ||
|
||
assert_eq!(h.finalize(), expected); | ||
} | ||
|
||
#[test] | ||
fn test_hash_isize() { | ||
// Test that isize specifically is handled consistently across platforms. | ||
let test_isize = -7_isize; | ||
|
||
let mut h = StableHasher::new(); | ||
test_isize.hash(&mut h); | ||
|
||
// This depends on the hashing algorithm. See note at top of file. | ||
let expected = (14721296605626097289, 11385941877786388409); | ||
|
||
assert_eq!(h.finalize(), expected); | ||
} |