diff options
author | 2023-11-14 11:49:03 -0800 | |
---|---|---|
committer | 2023-11-14 11:49:49 -0800 | |
commit | 1415840502f38952e63c1c6e4892f6e69ce7bc13 (patch) | |
tree | a433572dc9519ad396388955dd921a9e948c3c8f | |
parent | 4a0255b16e85233849ec34c48f428ad8d2936eda (diff) |
Don't allow AbslHashValue() to take a C-style array parameter. The
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
-rw-r--r-- | absl/hash/internal/hash.h | 16 |
1 files changed, 15 insertions, 1 deletions
diff --git a/absl/hash/internal/hash.h b/absl/hash/internal/hash.h index 300c63af..e97cb315 100644 --- a/absl/hash/internal/hash.h +++ b/absl/hash/internal/hash.h @@ -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. |