diff options
| author | Louis Dionne <ldionne.2@gmail.com> | 2023-02-09 15:20:20 -0800 |
|---|---|---|
| committer | Tobias Hieta <tobias@hieta.se> | 2023-02-10 08:19:32 +0100 |
| commit | d956ed0d76afd1a0f778faad920cc4a52618b5f8 (patch) | |
| tree | ea9afdcd203a80343819969a87b405b8d381ba10 | |
| parent | 5036912174acb051468611a25f1b43b4e8336be9 (diff) | |
| download | llvm-d956ed0d76afd1a0f778faad920cc4a52618b5f8.tar.gz | |
[libc++] Guard the fix to CityHash behind ABI v2
As explained in a comment in https://reviews.llvm.org/D134124, we tried
landing this unconditionally but this actually bit some users who were
sharing std::unordered_map across an ABI boundary. This shows that the
ABI break is not benign and it should be guarded behind ABI v2.
Differential Revision: https://reviews.llvm.org/D143688
(cherry picked from commit 02718433a0dd3f8d3f3719b97aa1685b699ac785)
| -rw-r--r-- | libcxx/include/__config | 9 | ||||
| -rw-r--r-- | libcxx/include/__functional/hash.h | 4 | ||||
| -rw-r--r-- | libcxx/test/libcxx/utilities/utility/__murmur2_or_cityhash.pass.cpp | 4 |
3 files changed, 17 insertions, 0 deletions
diff --git a/libcxx/include/__config b/libcxx/include/__config index 8846e5146f89..51b100fa5569 100644 --- a/libcxx/include/__config +++ b/libcxx/include/__config @@ -134,6 +134,15 @@ # define _LIBCPP_ABI_DO_NOT_EXPORT_VECTOR_BASE_COMMON // According to the Standard, `bitset::operator[] const` returns bool # define _LIBCPP_ABI_BITSET_VECTOR_BOOL_CONST_SUBSCRIPT_RETURN_BOOL +// Fix the implementation of CityHash used for std::hash<fundamental-type>. +// This is an ABI break because `std::hash` will return a different result, +// which means that hashing the same object in translation units built against +// different versions of libc++ can return inconsistent results. This is especially +// tricky since std::hash is used in the implementation of unordered containers. +// +// The incorrect implementation of CityHash has the problem that it drops some +// bits on the floor. +# define _LIBCPP_ABI_FIX_CITYHASH_IMPLEMENTATION // Remove the base 10 implementation of std::to_chars from the dylib. // The implementation moved to the header, but we still export the symbols from // the dylib for backwards compatibility. diff --git a/libcxx/include/__functional/hash.h b/libcxx/include/__functional/hash.h index dfd8ea2553dc..39382aa9bff4 100644 --- a/libcxx/include/__functional/hash.h +++ b/libcxx/include/__functional/hash.h @@ -140,7 +140,11 @@ struct __murmur2_or_cityhash<_Size, 64> if (__len >= 4) { const uint32_t __a = std::__loadword<uint32_t>(__s); const uint32_t __b = std::__loadword<uint32_t>(__s + __len - 4); +#ifdef _LIBCPP_ABI_FIX_CITYHASH_IMPLEMENTATION return __hash_len_16(__len + (static_cast<_Size>(__a) << 3), __b); +#else + return __hash_len_16(__len + (__a << 3), __b); +#endif } if (__len > 0) { const unsigned char __a = static_cast<unsigned char>(__s[0]); diff --git a/libcxx/test/libcxx/utilities/utility/__murmur2_or_cityhash.pass.cpp b/libcxx/test/libcxx/utilities/utility/__murmur2_or_cityhash.pass.cpp index e2905a450a09..13fc9c33d68f 100644 --- a/libcxx/test/libcxx/utilities/utility/__murmur2_or_cityhash.pass.cpp +++ b/libcxx/test/libcxx/utilities/utility/__murmur2_or_cityhash.pass.cpp @@ -10,6 +10,10 @@ // UNSUPPORTED: c++03 +// In ABI v1, our CityHash implementation is incorrect and fixing it would +// be an ABI break. +// REQUIRES: libcpp-abi-version=2 + #include <cassert> #include <string> #include <utility> |