Skip to content

Commit b935882

Browse files
krasimirggkrasimirgg
committed
Revert "[libc++] Remove UB in list, forward_list and __hash_table"
This reverts commit 0687e4d. Causes LLDB failures: https://reviews.llvm.org/D101206#4653253
1 parent 4e888e2 commit b935882

File tree

8 files changed

+92
-200
lines changed

8 files changed

+92
-200
lines changed

‎libcxx/include/__hash_table

Lines changed: 40 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
#include <__memory/addressof.h>
2222
#include <__memory/allocator_traits.h>
2323
#include <__memory/compressed_pair.h>
24-
#include <__memory/construct_at.h>
2524
#include <__memory/pointer_traits.h>
2625
#include <__memory/swap_allocator.h>
2726
#include <__memory/unique_ptr.h>
@@ -46,7 +45,6 @@
4645
#include <cmath>
4746
#include <cstring>
4847
#include <initializer_list>
49-
#include <new> // __launder
5048

5149
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
5250
# pragma GCC system_header
@@ -109,44 +107,19 @@ struct __hash_node_base
109107
}
110108

111109
_LIBCPP_INLINE_VISIBILITY __hash_node_base() _NOEXCEPT : __next_(nullptr) {}
112-
_LIBCPP_HIDE_FROM_ABI explicit __hash_node_base(__next_pointer __next) _NOEXCEPT : __next_(__next) {}
113110
};
114111

115112
template <class _Tp, class _VoidPtr>
116-
struct __hash_node
113+
struct _LIBCPP_STANDALONE_DEBUG __hash_node
117114
: public __hash_node_base
118115
<
119116
__rebind_pointer_t<_VoidPtr, __hash_node<_Tp, _VoidPtr> >
120117
>
121118
{
122119
typedef _Tp __node_value_type;
123-
using _Base = __hash_node_base<__rebind_pointer_t<_VoidPtr, __hash_node<_Tp, _VoidPtr> > >;
124-
using __next_pointer = typename _Base::__next_pointer;
125120

126121
size_t __hash_;
127-
128-
// We allow starting the lifetime of nodes without initializing the value held by the node,
129-
// since that is handled by the hash table itself in order to be allocator-aware.
130-
#ifndef _LIBCPP_CXX03_LANG
131-
private:
132-
union {
133-
_Tp __value_;
134-
};
135-
136-
public:
137-
_LIBCPP_HIDE_FROM_ABI _Tp& __get_value() { return __value_; }
138-
#else
139-
private:
140-
_ALIGNAS_TYPE(_Tp) char __buffer_[sizeof(_Tp)];
141-
142-
public:
143-
_LIBCPP_HIDE_FROM_ABI _Tp& __get_value() {
144-
return *std::__launder(reinterpret_cast<_Tp*>(&__buffer_));
145-
}
146-
#endif
147-
148-
_LIBCPP_HIDE_FROM_ABI explicit __hash_node(__next_pointer __next, size_t __hash) : _Base(__next), __hash_(__hash) {}
149-
_LIBCPP_HIDE_FROM_ABI ~__hash_node() {}
122+
__node_value_type __value_;
150123
};
151124

152125
inline _LIBCPP_INLINE_VISIBILITY
@@ -338,12 +311,12 @@ public:
338311

339312
_LIBCPP_INLINE_VISIBILITY
340313
reference operator*() const {
341-
return __node_->__upcast()->__get_value();
314+
return __node_->__upcast()->__value_;
342315
}
343316

344317
_LIBCPP_INLINE_VISIBILITY
345318
pointer operator->() const {
346-
return pointer_traits<pointer>::pointer_to(__node_->__upcast()->__get_value());
319+
return pointer_traits<pointer>::pointer_to(__node_->__upcast()->__value_);
347320
}
348321

349322
_LIBCPP_INLINE_VISIBILITY
@@ -414,11 +387,11 @@ public:
414387

415388
_LIBCPP_INLINE_VISIBILITY
416389
reference operator*() const {
417-
return __node_->__upcast()->__get_value();
390+
return __node_->__upcast()->__value_;
418391
}
419392
_LIBCPP_INLINE_VISIBILITY
420393
pointer operator->() const {
421-
return pointer_traits<pointer>::pointer_to(__node_->__upcast()->__get_value());
394+
return pointer_traits<pointer>::pointer_to(__node_->__upcast()->__value_);
422395
}
423396

424397
_LIBCPP_INLINE_VISIBILITY
@@ -480,12 +453,12 @@ public:
480453

481454
_LIBCPP_INLINE_VISIBILITY
482455
reference operator*() const {
483-
return __node_->__upcast()->__get_value();
456+
return __node_->__upcast()->__value_;
484457
}
485458

486459
_LIBCPP_INLINE_VISIBILITY
487460
pointer operator->() const {
488-
return pointer_traits<pointer>::pointer_to(__node_->__upcast()->__get_value());
461+
return pointer_traits<pointer>::pointer_to(__node_->__upcast()->__value_);
489462
}
490463

491464
_LIBCPP_INLINE_VISIBILITY
@@ -570,12 +543,12 @@ public:
570543

571544
_LIBCPP_INLINE_VISIBILITY
572545
reference operator*() const {
573-
return __node_->__upcast()->__get_value();
546+
return __node_->__upcast()->__value_;
574547
}
575548

576549
_LIBCPP_INLINE_VISIBILITY
577550
pointer operator->() const {
578-
return pointer_traits<pointer>::pointer_to(__node_->__upcast()->__get_value());
551+
return pointer_traits<pointer>::pointer_to(__node_->__upcast()->__value_);
579552
}
580553

581554
_LIBCPP_INLINE_VISIBILITY
@@ -697,10 +670,8 @@ public:
697670
_LIBCPP_INLINE_VISIBILITY
698671
void operator()(pointer __p) _NOEXCEPT
699672
{
700-
if (__value_constructed) {
701-
__alloc_traits::destroy(__na_, _NodeTypes::__get_ptr(__p->__get_value()));
702-
std::__destroy_at(std::addressof(*__p));
703-
}
673+
if (__value_constructed)
674+
__alloc_traits::destroy(__na_, _NodeTypes::__get_ptr(__p->__value_));
704675
if (__p)
705676
__alloc_traits::deallocate(__na_, __p, 1);
706677
}
@@ -1394,8 +1365,7 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::__deallocate_node(__next_pointer __np)
13941365
{
13951366
__next_pointer __next = __np->__next_;
13961367
__node_pointer __real_np = __np->__upcast();
1397-
__node_traits::destroy(__na, _NodeTypes::__get_ptr(__real_np->__get_value()));
1398-
std::__destroy_at(std::addressof(*__real_np));
1368+
__node_traits::destroy(__na, _NodeTypes::__get_ptr(__real_np->__value_));
13991369
__node_traits::deallocate(__na, __real_np, 1);
14001370
__np = __next;
14011371
}
@@ -1464,8 +1434,8 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::__move_assign(
14641434
const_iterator __i = __u.begin();
14651435
while (__cache != nullptr && __u.size() != 0)
14661436
{
1467-
__cache->__upcast()->__get_value() =
1468-
_VSTD::move(__u.remove(__i++)->__get_value());
1437+
__cache->__upcast()->__value_ =
1438+
_VSTD::move(__u.remove(__i++)->__value_);
14691439
__next_pointer __next = __cache->__next_;
14701440
__node_insert_multi(__cache->__upcast());
14711441
__cache = __next;
@@ -1483,7 +1453,7 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::__move_assign(
14831453
const_iterator __i = __u.begin();
14841454
while (__u.size() != 0)
14851455
{
1486-
__node_holder __h = __construct_node(_NodeTypes::__move(__u.remove(__i++)->__get_value()));
1456+
__node_holder __h = __construct_node(_NodeTypes::__move(__u.remove(__i++)->__value_));
14871457
__node_insert_multi(__h.get());
14881458
__h.release();
14891459
}
@@ -1525,7 +1495,7 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::__assign_unique(_InputIterator __first
15251495
#endif // _LIBCPP_HAS_NO_EXCEPTIONS
15261496
for (; __cache != nullptr && __first != __last; ++__first)
15271497
{
1528-
__cache->__upcast()->__get_value() = *__first;
1498+
__cache->__upcast()->__value_ = *__first;
15291499
__next_pointer __next = __cache->__next_;
15301500
__node_insert_unique(__cache->__upcast());
15311501
__cache = __next;
@@ -1565,7 +1535,7 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::__assign_multi(_InputIterator __first,
15651535
#endif // _LIBCPP_HAS_NO_EXCEPTIONS
15661536
for (; __cache != nullptr && __first != __last; ++__first)
15671537
{
1568-
__cache->__upcast()->__get_value() = *__first;
1538+
__cache->__upcast()->__value_ = *__first;
15691539
__next_pointer __next = __cache->__next_;
15701540
__node_insert_multi(__cache->__upcast());
15711541
__cache = __next;
@@ -1659,7 +1629,7 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::__node_insert_unique_prepare(
16591629
__ndptr = __ndptr->__next_)
16601630
{
16611631
if ((__ndptr->__hash() == __hash) &&
1662-
key_eq()(__ndptr->__upcast()->__get_value(), __value))
1632+
key_eq()(__ndptr->__upcast()->__value_, __value))
16631633
return __ndptr;
16641634
}
16651635
}
@@ -1708,9 +1678,9 @@ template <class _Tp, class _Hash, class _Equal, class _Alloc>
17081678
pair<typename __hash_table<_Tp, _Hash, _Equal, _Alloc>::iterator, bool>
17091679
__hash_table<_Tp, _Hash, _Equal, _Alloc>::__node_insert_unique(__node_pointer __nd)
17101680
{
1711-
__nd->__hash_ = hash_function()(__nd->__get_value());
1681+
__nd->__hash_ = hash_function()(__nd->__value_);
17121682
__next_pointer __existing_node =
1713-
__node_insert_unique_prepare(__nd->__hash(), __nd->__get_value());
1683+
__node_insert_unique_prepare(__nd->__hash(), __nd->__value_);
17141684

17151685
// Insert the node, unless it already exists in the container.
17161686
bool __inserted = false;
@@ -1756,7 +1726,7 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::__node_insert_multi_prepare(
17561726
// false true set __found to true
17571727
// true false break
17581728
if (__found != (__pn->__next_->__hash() == __cp_hash &&
1759-
key_eq()(__pn->__next_->__upcast()->__get_value(), __cp_val)))
1729+
key_eq()(__pn->__next_->__upcast()->__value_, __cp_val)))
17601730
{
17611731
if (!__found)
17621732
__found = true;
@@ -1810,8 +1780,8 @@ template <class _Tp, class _Hash, class _Equal, class _Alloc>
18101780
typename __hash_table<_Tp, _Hash, _Equal, _Alloc>::iterator
18111781
__hash_table<_Tp, _Hash, _Equal, _Alloc>::__node_insert_multi(__node_pointer __cp)
18121782
{
1813-
__cp->__hash_ = hash_function()(__cp->__get_value());
1814-
__next_pointer __pn = __node_insert_multi_prepare(__cp->__hash(), __cp->__get_value());
1783+
__cp->__hash_ = hash_function()(__cp->__value_);
1784+
__next_pointer __pn = __node_insert_multi_prepare(__cp->__hash(), __cp->__value_);
18151785
__node_insert_multi_perform(__cp, __pn);
18161786

18171787
return iterator(__cp->__ptr());
@@ -1822,7 +1792,7 @@ typename __hash_table<_Tp, _Hash, _Equal, _Alloc>::iterator
18221792
__hash_table<_Tp, _Hash, _Equal, _Alloc>::__node_insert_multi(
18231793
const_iterator __p, __node_pointer __cp)
18241794
{
1825-
if (__p != end() && key_eq()(*__p, __cp->__get_value()))
1795+
if (__p != end() && key_eq()(*__p, __cp->__value_))
18261796
{
18271797
__next_pointer __np = __p.__node_;
18281798
__cp->__hash_ = __np->__hash();
@@ -1869,7 +1839,7 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::__emplace_unique_key_args(_Key const&
18691839
__nd = __nd->__next_)
18701840
{
18711841
if ((__nd->__hash() == __hash) &&
1872-
key_eq()(__nd->__upcast()->__get_value(), __k))
1842+
key_eq()(__nd->__upcast()->__value_, __k))
18731843
goto __done;
18741844
}
18751845
}
@@ -2013,9 +1983,9 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::__node_handle_merge_unique(
20131983
__it != __source.end();)
20141984
{
20151985
__node_pointer __src_ptr = __it.__node_->__upcast();
2016-
size_t __hash = hash_function()(__src_ptr->__get_value());
1986+
size_t __hash = hash_function()(__src_ptr->__value_);
20171987
__next_pointer __existing_node =
2018-
__node_insert_unique_prepare(__hash, __src_ptr->__get_value());
1988+
__node_insert_unique_prepare(__hash, __src_ptr->__value_);
20191989
auto __prev_iter = __it++;
20201990
if (__existing_node == nullptr)
20211991
{
@@ -2067,9 +2037,9 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::__node_handle_merge_multi(
20672037
__it != __source.end();)
20682038
{
20692039
__node_pointer __src_ptr = __it.__node_->__upcast();
2070-
size_t __src_hash = hash_function()(__src_ptr->__get_value());
2040+
size_t __src_hash = hash_function()(__src_ptr->__value_);
20712041
__next_pointer __pn =
2072-
__node_insert_multi_prepare(__src_hash, __src_ptr->__get_value());
2042+
__node_insert_multi_prepare(__src_hash, __src_ptr->__value_);
20732043
(void)__source.remove(__it++).release();
20742044
__src_ptr->__hash_ = __src_hash;
20752045
__node_insert_multi_perform(__src_ptr, __pn);
@@ -2143,8 +2113,8 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::__do_rehash(size_type __nbc)
21432113
if _LIBCPP_CONSTEXPR_SINCE_CXX17 (!_UniqueKeys)
21442114
{
21452115
for (; __np->__next_ != nullptr &&
2146-
key_eq()(__cp->__upcast()->__get_value(),
2147-
__np->__next_->__upcast()->__get_value());
2116+
key_eq()(__cp->__upcast()->__value_,
2117+
__np->__next_->__upcast()->__value_);
21482118
__np = __np->__next_)
21492119
;
21502120
}
@@ -2178,7 +2148,7 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::find(const _Key& __k)
21782148
__nd = __nd->__next_)
21792149
{
21802150
if ((__nd->__hash() == __hash)
2181-
&& key_eq()(__nd->__upcast()->__get_value(), __k))
2151+
&& key_eq()(__nd->__upcast()->__value_, __k))
21822152
return iterator(__nd);
21832153
}
21842154
}
@@ -2205,7 +2175,7 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::find(const _Key& __k) const
22052175
__nd = __nd->__next_)
22062176
{
22072177
if ((__nd->__hash() == __hash)
2208-
&& key_eq()(__nd->__upcast()->__get_value(), __k))
2178+
&& key_eq()(__nd->__upcast()->__value_, __k))
22092179
return const_iterator(__nd);
22102180
}
22112181
}
@@ -2223,20 +2193,10 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::__construct_node(_Args&& ...__args)
22232193
"Construct cannot be called with a hash value type");
22242194
__node_allocator& __na = __node_alloc();
22252195
__node_holder __h(__node_traits::allocate(__na, 1), _Dp(__na));
2226-
2227-
// Begin the lifetime of the node itself. Note that this doesn't begin the lifetime of the value
2228-
// held inside the node, since we need to use the allocator's construct() method for that.
2229-
//
2230-
// We don't use the allocator's construct() method to construct the node itself since the
2231-
// Cpp17FooInsertable named requirements don't require the allocator's construct() method
2232-
// to work on anything other than the value_type.
2233-
std::__construct_at(std::addressof(*__h), /* next = */nullptr, /* hash = */0);
2234-
2235-
// Now construct the value_type using the allocator's construct() method.
2236-
__node_traits::construct(__na, _NodeTypes::__get_ptr(__h->__get_value()), _VSTD::forward<_Args>(__args)...);
2196+
__node_traits::construct(__na, _NodeTypes::__get_ptr(__h->__value_), _VSTD::forward<_Args>(__args)...);
22372197
__h.get_deleter().__value_constructed = true;
2238-
2239-
__h->__hash_ = hash_function()(__h->__get_value());
2198+
__h->__hash_ = hash_function()(__h->__value_);
2199+
__h->__next_ = nullptr;
22402200
return __h;
22412201
}
22422202

@@ -2250,11 +2210,12 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::__construct_node_hash(
22502210
"Construct cannot be called with a hash value type");
22512211
__node_allocator& __na = __node_alloc();
22522212
__node_holder __h(__node_traits::allocate(__na, 1), _Dp(__na));
2253-
std::__construct_at(std::addressof(*__h), /* next = */nullptr, /* hash = */__hash);
2254-
__node_traits::construct(__na, _NodeTypes::__get_ptr(__h->__get_value()),
2213+
__node_traits::construct(__na, _NodeTypes::__get_ptr(__h->__value_),
22552214
_VSTD::forward<_First>(__f),
22562215
_VSTD::forward<_Rest>(__rest)...);
22572216
__h.get_deleter().__value_constructed = true;
2217+
__h->__hash_ = __hash;
2218+
__h->__next_ = nullptr;
22582219
return __h;
22592220
}
22602221

‎libcxx/include/__node_handle

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ struct __set_node_handle_specifics
209209
_LIBCPP_INLINE_VISIBILITY
210210
value_type& value() const
211211
{
212-
return static_cast<_Derived const*>(this)->__ptr_->__get_value();
212+
return static_cast<_Derived const*>(this)->__ptr_->__value_;
213213
}
214214
};
215215

@@ -223,14 +223,14 @@ struct __map_node_handle_specifics
223223
key_type& key() const
224224
{
225225
return static_cast<_Derived const*>(this)->
226-
__ptr_->__get_value().__ref().first;
226+
__ptr_->__value_.__ref().first;
227227
}
228228

229229
_LIBCPP_INLINE_VISIBILITY
230230
mapped_type& mapped() const
231231
{
232232
return static_cast<_Derived const*>(this)->
233-
__ptr_->__get_value().__ref().second;
233+
__ptr_->__value_.__ref().second;
234234
}
235235
};
236236

‎libcxx/include/__tree

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -774,8 +774,6 @@ public:
774774

775775
__node_value_type __value_;
776776

777-
_LIBCPP_HIDE_FROM_ABI _Tp& __get_value() { return __value_; }
778-
779777
private:
780778
~__tree_node() = delete;
781779
__tree_node(__tree_node const&) = delete;

‎libcxx/include/ext/hash_map

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -357,9 +357,9 @@ public:
357357
void operator()(pointer __p)
358358
{
359359
if (__second_constructed)
360-
__alloc_traits::destroy(__na_, _VSTD::addressof(__p->__get_value().second));
360+
__alloc_traits::destroy(__na_, _VSTD::addressof(__p->__value_.second));
361361
if (__first_constructed)
362-
__alloc_traits::destroy(__na_, _VSTD::addressof(__p->__get_value().first));
362+
__alloc_traits::destroy(__na_, _VSTD::addressof(__p->__value_.first));
363363
if (__p)
364364
__alloc_traits::deallocate(__na_, __p, 1);
365365
}
@@ -667,9 +667,9 @@ hash_map<_Key, _Tp, _Hash, _Pred, _Alloc>::__construct_node(const key_type& __k)
667667
{
668668
__node_allocator& __na = __table_.__node_alloc();
669669
__node_holder __h(__node_traits::allocate(__na, 1), _Dp(__na));
670-
__node_traits::construct(__na, _VSTD::addressof(__h->__get_value().first), __k);
670+
__node_traits::construct(__na, _VSTD::addressof(__h->__value_.first), __k);
671671
__h.get_deleter().__first_constructed = true;
672-
__node_traits::construct(__na, _VSTD::addressof(__h->__get_value().second));
672+
__node_traits::construct(__na, _VSTD::addressof(__h->__value_.second));
673673
__h.get_deleter().__second_constructed = true;
674674
return __h;
675675
}

0 commit comments

Comments
 (0)