Skip to content

Fix hash_instantiated_test and hash_test on big-endian - #2120

Closed
miladfarca wants to merge 2 commits into
abseil:masterfrom
miladfarca:63c77e406-fix
Closed

Fix hash_instantiated_test and hash_test on big-endian#2120
miladfarca wants to merge 2 commits into
abseil:masterfrom
miladfarca:63c77e406-fix

Conversation

@miladfarca

Copy link
Copy Markdown
Contributor

The two tests are failing on big-endian platforms after 63c77e4.

The new bit-packing code stores packed bits in the LSB of a uint64_t, then feeds raw bytes via reinterpret_cast. On big-endian, the LSB is at the end of the memory layout, so the partial-word path reads leading zero bytes instead of the actual data, causing hash collisions between distinct inputs.

Fix by byte-swapping the word to little-endian layout before feeding it to the combiner.

@miladfarca

Copy link
Copy Markdown
Contributor Author

/cc @derekmauro

Comment thread absl/hash/internal/hash.h Outdated
for (size_t j = 0; j < 64; ++j) {
word |= static_cast<uint64_t>(vector[i + j]) << j;
}
word = absl::little_endian::FromHost64(word);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix. The new preferred way to do this is something like:

if constexpr (absl::endian::native == absl::endian::big) {
  word = absl::byteswap(word);
}

@miladfarca miladfarca Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for reviewing, I've updated it to use the new method.

@derekmauro derekmauro self-assigned this Jul 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants