Skip to content

Commit 2393ab6

Browse files
authored
[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
1 parent 6018820 commit 2393ab6

File tree

2 files changed

+70
-2
lines changed

2 files changed

+70
-2
lines changed

libcxx/include/__memory/compressed_pair.h

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,19 @@ _LIBCPP_BEGIN_NAMESPACE_STD
5050
// member1 - offset: 0, size: 4
5151
// member2 - offset: 4, size: 4
5252
// member3 - offset: 8, size: 8
53+
//
54+
// Furthermore, that alignment must be the same as what was used in the old __compressed_pair layout, so we must
55+
// handle reference types specially since alignof(T&) == alignof(T).
56+
// See https://github.com/llvm/llvm-project/issues/118559.
5357

5458
#ifndef _LIBCPP_ABI_NO_COMPRESSED_PAIR_PADDING
5559

60+
template <class _Tp>
61+
inline const size_t __compressed_pair_alignment = _LIBCPP_ALIGNOF(_Tp);
62+
63+
template <class _Tp>
64+
inline const size_t __compressed_pair_alignment<_Tp&> = _LIBCPP_ALIGNOF(void*);
65+
5666
template <class _ToPad,
5767
bool _Empty = ((is_empty<_ToPad>::value && !__libcpp_is_final<_ToPad>::value) ||
5868
is_reference<_ToPad>::value || sizeof(_ToPad) == __datasizeof_v<_ToPad>)>
@@ -64,14 +74,15 @@ template <class _ToPad>
6474
class __compressed_pair_padding<_ToPad, true> {};
6575

6676
# define _LIBCPP_COMPRESSED_PAIR(T1, Initializer1, T2, Initializer2) \
67-
_LIBCPP_NO_UNIQUE_ADDRESS __attribute__((__aligned__(_LIBCPP_ALIGNOF(T2)))) T1 Initializer1; \
77+
_LIBCPP_NO_UNIQUE_ADDRESS __attribute__((__aligned__(::std::__compressed_pair_alignment<T2>))) T1 Initializer1; \
6878
_LIBCPP_NO_UNIQUE_ADDRESS ::std::__compressed_pair_padding<T1> _LIBCPP_CONCAT3(__padding1_, __LINE__, _); \
6979
_LIBCPP_NO_UNIQUE_ADDRESS T2 Initializer2; \
7080
_LIBCPP_NO_UNIQUE_ADDRESS ::std::__compressed_pair_padding<T2> _LIBCPP_CONCAT3(__padding2_, __LINE__, _)
7181

7282
# define _LIBCPP_COMPRESSED_TRIPLE(T1, Initializer1, T2, Initializer2, T3, Initializer3) \
7383
_LIBCPP_NO_UNIQUE_ADDRESS \
74-
__attribute__((__aligned__(_LIBCPP_ALIGNOF(T2)), __aligned__(_LIBCPP_ALIGNOF(T3)))) T1 Initializer1; \
84+
__attribute__((__aligned__(::std::__compressed_pair_alignment<T2>), \
85+
__aligned__(::std::__compressed_pair_alignment<T3>))) T1 Initializer1; \
7586
_LIBCPP_NO_UNIQUE_ADDRESS ::std::__compressed_pair_padding<T1> _LIBCPP_CONCAT3(__padding1_, __LINE__, _); \
7687
_LIBCPP_NO_UNIQUE_ADDRESS T2 Initializer2; \
7788
_LIBCPP_NO_UNIQUE_ADDRESS ::std::__compressed_pair_padding<T2> _LIBCPP_CONCAT3(__padding2_, __LINE__, _); \
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
// Pin down the ABI of associative containers with respect to their size and alignment
10+
// when passed a comparator that is a reference.
11+
//
12+
// While it's not even clear that reference comparators are legal in containers, an
13+
// unintended ABI break was discovered after implementing the new compressed pair
14+
// mechanism based on [[no_unique_address]], and this is a regression test for that.
15+
// If we decide to make reference comparators ill-formed, this test would become
16+
// unnecessary.
17+
//
18+
// See https://github.com/llvm/llvm-project/issues/118559 for more details.
19+
20+
#include <set>
21+
#include <map>
22+
23+
#include "test_macros.h"
24+
25+
struct TEST_ALIGNAS(16) Cmp {
26+
bool operator()(int, int) const;
27+
};
28+
29+
template <class Compare>
30+
struct Set {
31+
char b;
32+
std::set<int, Compare> s;
33+
};
34+
35+
template <class Compare>
36+
struct Multiset {
37+
char b;
38+
std::multiset<int, Compare> s;
39+
};
40+
41+
template <class Compare>
42+
struct Map {
43+
char b;
44+
std::map<int, char, Compare> s;
45+
};
46+
47+
template <class Compare>
48+
struct Multimap {
49+
char b;
50+
std::multimap<int, char, Compare> s;
51+
};
52+
53+
static_assert(sizeof(Set<Cmp&>) == sizeof(Set<bool (*)(int, int)>), "");
54+
static_assert(sizeof(Multiset<Cmp&>) == sizeof(Multiset<bool (*)(int, int)>), "");
55+
56+
static_assert(sizeof(Map<Cmp&>) == sizeof(Map<bool (*)(int, int)>), "");
57+
static_assert(sizeof(Multimap<Cmp&>) == sizeof(Multimap<bool (*)(int, int)>), "");

0 commit comments

Comments
 (0)