Skip to content

Commit

Permalink
[libc++] Guard the fix to CityHash behind ABI v2
Browse files Browse the repository at this point in the history
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 0271843)
  • Loading branch information
ldionne authored and tru committed Feb 10, 2023
1 parent 5036912 commit d956ed0
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 0 deletions.
9 changes: 9 additions & 0 deletions libcxx/include/__config
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions libcxx/include/__functional/hash.h
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down

0 comments on commit d956ed0

Please sign in to comment.