Skip to content

Commit

Permalink
Don't allow AbslHashValue() to take a C-style array parameter. The
Browse files Browse the repository at this point in the history
current behavior of decaying the array to a pointer and hashing the
pointer can lead to subtle bugs.

The most common potential error is passing a C-string literal. Hashing
the char pointer in those cases is correct only if the string literals
are guaranteed to be deduplicated, which is dangerous to rely on even
if true (and the call sites in header files require deduplication
across translation units). After this change, these call-sites
requires wrapping the literal in absl::string_view.

This is a breaking change for code doing something like
absl::HashOf("string");
Instead, this should be changed to
absl::HashOf(absl::string_view("string"));
PiperOrigin-RevId: 582393585
Change-Id: I3810c07b5b74bf153cb62a7beedce243be5a69ee
  • Loading branch information
derekmauro authored and copybara-github committed Nov 14, 2023
1 parent 4a0255b commit 1415840
Showing 1 changed file with 15 additions and 1 deletion.
16 changes: 15 additions & 1 deletion absl/hash/internal/hash.h
Original file line number Diff line number Diff line change
Expand Up @@ -409,9 +409,23 @@ AbslHashValue(H hash_state, LongDouble value) {
return H::combine(std::move(hash_state), category);
}

// Without this overload, an array decays to a pointer and we hash that, which
// is not likely to be what the caller intended.
template <typename H, typename T, size_t N>
H AbslHashValue(H hash_state, T (&)[N]) {
static_assert(
sizeof(T) == -1,
"Hashing C arrays is not allowed. For string literals, wrap the literal "
"in absl::string_view(). To hash the array contents, use "
"absl::MakeSpan() or make the array an std::array. To hash the array "
"address, use &array[0].");
return hash_state;
}

// AbslHashValue() for hashing pointers
template <typename H, typename T>
H AbslHashValue(H hash_state, T* ptr) {
std::enable_if_t<std::is_pointer<T>::value, H> AbslHashValue(H hash_state,
T ptr) {
auto v = reinterpret_cast<uintptr_t>(ptr);
// Due to alignment, pointers tend to have low bits as zero, and the next few
// bits follow a pattern since they are also multiples of some base value.
Expand Down

0 comments on commit 1415840

Please sign in to comment.