[libc++] Fix unintended ABI break in associative containers with reference comparators (#118685) While reference comparators are a terrible idea and it's not entirely clear whether they are supported, fixing the unintended ABI break is straightforward so we should do it as a first step. Fixes #118559 GitOrigin-RevId: 2393ab65edda14cc416068ebee0b512228723877
diff --git a/include/__memory/compressed_pair.h b/include/__memory/compressed_pair.h index a7acaaf..38798a2 100644 --- a/include/__memory/compressed_pair.h +++ b/include/__memory/compressed_pair.h
@@ -50,9 +50,19 @@ // member1 - offset: 0, size: 4 // member2 - offset: 4, size: 4 // member3 - offset: 8, size: 8 +// +// Furthermore, that alignment must be the same as what was used in the old __compressed_pair layout, so we must +// handle reference types specially since alignof(T&) == alignof(T). +// See https://github.com/llvm/llvm-project/issues/118559. #ifndef _LIBCPP_ABI_NO_COMPRESSED_PAIR_PADDING +template <class _Tp> +inline const size_t __compressed_pair_alignment = _LIBCPP_ALIGNOF(_Tp); + +template <class _Tp> +inline const size_t __compressed_pair_alignment<_Tp&> = _LIBCPP_ALIGNOF(void*); + template <class _ToPad, bool _Empty = ((is_empty<_ToPad>::value && !__libcpp_is_final<_ToPad>::value) || is_reference<_ToPad>::value || sizeof(_ToPad) == __datasizeof_v<_ToPad>)> @@ -64,14 +74,15 @@ class __compressed_pair_padding<_ToPad, true> {}; # define _LIBCPP_COMPRESSED_PAIR(T1, Initializer1, T2, Initializer2) \ - _LIBCPP_NO_UNIQUE_ADDRESS __attribute__((__aligned__(_LIBCPP_ALIGNOF(T2)))) T1 Initializer1; \ + _LIBCPP_NO_UNIQUE_ADDRESS __attribute__((__aligned__(::std::__compressed_pair_alignment<T2>))) T1 Initializer1; \ _LIBCPP_NO_UNIQUE_ADDRESS ::std::__compressed_pair_padding<T1> _LIBCPP_CONCAT3(__padding1_, __LINE__, _); \ _LIBCPP_NO_UNIQUE_ADDRESS T2 Initializer2; \ _LIBCPP_NO_UNIQUE_ADDRESS ::std::__compressed_pair_padding<T2> _LIBCPP_CONCAT3(__padding2_, __LINE__, _) # define _LIBCPP_COMPRESSED_TRIPLE(T1, Initializer1, T2, Initializer2, T3, Initializer3) \ _LIBCPP_NO_UNIQUE_ADDRESS \ - __attribute__((__aligned__(_LIBCPP_ALIGNOF(T2)), __aligned__(_LIBCPP_ALIGNOF(T3)))) T1 Initializer1; \ + __attribute__((__aligned__(::std::__compressed_pair_alignment<T2>), \ + __aligned__(::std::__compressed_pair_alignment<T3>))) T1 Initializer1; \ _LIBCPP_NO_UNIQUE_ADDRESS ::std::__compressed_pair_padding<T1> _LIBCPP_CONCAT3(__padding1_, __LINE__, _); \ _LIBCPP_NO_UNIQUE_ADDRESS T2 Initializer2; \ _LIBCPP_NO_UNIQUE_ADDRESS ::std::__compressed_pair_padding<T2> _LIBCPP_CONCAT3(__padding2_, __LINE__, _); \
diff --git a/test/libcxx/containers/associative/reference_comparator_abi.compile.pass.cpp b/test/libcxx/containers/associative/reference_comparator_abi.compile.pass.cpp new file mode 100644 index 0000000..f364fc8 --- /dev/null +++ b/test/libcxx/containers/associative/reference_comparator_abi.compile.pass.cpp
@@ -0,0 +1,57 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// Pin down the ABI of associative containers with respect to their size and alignment +// when passed a comparator that is a reference. +// +// While it's not even clear that reference comparators are legal in containers, an +// unintended ABI break was discovered after implementing the new compressed pair +// mechanism based on [[no_unique_address]], and this is a regression test for that. +// If we decide to make reference comparators ill-formed, this test would become +// unnecessary. +// +// See https://github.com/llvm/llvm-project/issues/118559 for more details. + +#include <set> +#include <map> + +#include "test_macros.h" + +struct TEST_ALIGNAS(16) Cmp { + bool operator()(int, int) const; +}; + +template <class Compare> +struct Set { + char b; + std::set<int, Compare> s; +}; + +template <class Compare> +struct Multiset { + char b; + std::multiset<int, Compare> s; +}; + +template <class Compare> +struct Map { + char b; + std::map<int, char, Compare> s; +}; + +template <class Compare> +struct Multimap { + char b; + std::multimap<int, char, Compare> s; +}; + +static_assert(sizeof(Set<Cmp&>) == sizeof(Set<bool (*)(int, int)>), ""); +static_assert(sizeof(Multiset<Cmp&>) == sizeof(Multiset<bool (*)(int, int)>), ""); + +static_assert(sizeof(Map<Cmp&>) == sizeof(Map<bool (*)(int, int)>), ""); +static_assert(sizeof(Multimap<Cmp&>) == sizeof(Multimap<bool (*)(int, int)>), "");