Skip to content

Commit daa88b3

Browse files
[libc++] Remove UB from std::__tree_node construction (#153908)
This patch also updates `__hash_table` to match what we do in `__tree` now. Fixes #102547 Fixes llvm/llvm-project#134330 (comment)
1 parent 94d5c54 commit daa88b3

File tree

3 files changed

+35
-20
lines changed

3 files changed

+35
-20
lines changed

libcxx/include/__config

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1176,12 +1176,6 @@ typedef __char32_t char32_t;
11761176
# define _LIBCPP_NO_SPECIALIZATIONS
11771177
# endif
11781178

1179-
# if __has_cpp_attribute(_Clang::__standalone_debug__)
1180-
# define _LIBCPP_STANDALONE_DEBUG [[_Clang::__standalone_debug__]]
1181-
# else
1182-
# define _LIBCPP_STANDALONE_DEBUG
1183-
# endif
1184-
11851179
# if __has_cpp_attribute(_Clang::__preferred_name__)
11861180
# define _LIBCPP_PREFERRED_NAME(x) [[_Clang::__preferred_name__(x)]]
11871181
# else

libcxx/include/__hash_table

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,12 @@ public:
166166
}
167167
#endif
168168

169-
_LIBCPP_HIDE_FROM_ABI explicit __hash_node(__next_pointer __next, size_t __hash) : _Base(__next), __hash_(__hash) {}
169+
template <class _Alloc, class... _Args>
170+
_LIBCPP_HIDE_FROM_ABI explicit __hash_node(size_t __hash, _Alloc& __na, _Args&&... __args)
171+
: _Base(nullptr), __hash_(__hash) {
172+
allocator_traits<_Alloc>::construct(__na, std::addressof(__get_value()), std::forward<_Args>(__args)...);
173+
}
174+
170175
_LIBCPP_HIDE_FROM_ABI ~__hash_node() {}
171176
};
172177

@@ -1855,16 +1860,13 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::__construct_node(_Args&&... __args) {
18551860
__node_allocator& __na = __node_alloc();
18561861
__node_holder __h(__node_traits::allocate(__na, 1), _Dp(__na));
18571862

1858-
// Begin the lifetime of the node itself. Note that this doesn't begin the lifetime of the value
1859-
// held inside the node, since we need to use the allocator's construct() method for that.
1863+
// Begin the lifetime of the node itself and the value_type contained within.
18601864
//
18611865
// We don't use the allocator's construct() method to construct the node itself since the
18621866
// Cpp17FooInsertable named requirements don't require the allocator's construct() method
18631867
// to work on anything other than the value_type.
1864-
std::__construct_at(std::addressof(*__h), /* next = */ nullptr, /* hash = */ 0);
1868+
std::__construct_at(std::addressof(*__h), /* hash = */ 0, __na, std::forward<_Args>(__args)...);
18651869

1866-
// Now construct the value_type using the allocator's construct() method.
1867-
__node_traits::construct(__na, std::addressof(__h->__get_value()), std::forward<_Args>(__args)...);
18681870
__h.get_deleter().__value_constructed = true;
18691871

18701872
__h->__hash_ = hash_function()(__h->__get_value());
@@ -1878,8 +1880,7 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::__construct_node_hash(size_t __hash, _
18781880
static_assert(!__is_hash_value_type<_Args...>::value, "Construct cannot be called with a hash value type");
18791881
__node_allocator& __na = __node_alloc();
18801882
__node_holder __h(__node_traits::allocate(__na, 1), _Dp(__na));
1881-
std::__construct_at(std::addressof(*__h), /* next = */ nullptr, /* hash = */ __hash);
1882-
__node_traits::construct(__na, std::addressof(__h->__get_value()), std::forward<_Args>(__args)...);
1883+
std::__construct_at(std::addressof(*__h), /* hash = */ __hash, __na, std::forward<_Args>(__args)...);
18831884
__h.get_deleter().__value_constructed = true;
18841885
return __h;
18851886
}

libcxx/include/__tree

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@
2020
#include <__memory/addressof.h>
2121
#include <__memory/allocator_traits.h>
2222
#include <__memory/compressed_pair.h>
23+
#include <__memory/construct_at.h>
2324
#include <__memory/pointer_traits.h>
2425
#include <__memory/swap_allocator.h>
2526
#include <__memory/unique_ptr.h>
27+
#include <__new/launder.h>
2628
#include <__type_traits/copy_cvref.h>
2729
#include <__type_traits/enable_if.h>
2830
#include <__type_traits/invoke.h>
@@ -559,8 +561,7 @@ public:
559561
};
560562

561563
template <class _VoidPtr>
562-
class _LIBCPP_STANDALONE_DEBUG
563-
__tree_node_base : public __tree_end_node<__rebind_pointer_t<_VoidPtr, __tree_node_base<_VoidPtr> > > {
564+
class __tree_node_base : public __tree_end_node<__rebind_pointer_t<_VoidPtr, __tree_node_base<_VoidPtr> > > {
564565
public:
565566
using pointer = __rebind_pointer_t<_VoidPtr, __tree_node_base>;
566567
using __end_node_pointer _LIBCPP_NODEBUG = __rebind_pointer_t<_VoidPtr, __tree_end_node<pointer> >;
@@ -573,22 +574,41 @@ public:
573574

574575
_LIBCPP_HIDE_FROM_ABI void __set_parent(pointer __p) { __parent_ = static_cast<__end_node_pointer>(__p); }
575576

576-
~__tree_node_base() = delete;
577+
_LIBCPP_HIDE_FROM_ABI __tree_node_base() = default;
577578
__tree_node_base(__tree_node_base const&) = delete;
578579
__tree_node_base& operator=(__tree_node_base const&) = delete;
579580
};
580581

581582
template <class _Tp, class _VoidPtr>
582-
class _LIBCPP_STANDALONE_DEBUG __tree_node : public __tree_node_base<_VoidPtr> {
583+
class __tree_node : public __tree_node_base<_VoidPtr> {
583584
public:
584585
using __node_value_type _LIBCPP_NODEBUG = __get_node_value_type_t<_Tp>;
585586

587+
// We use a union to avoid initialization during member initialization, which allows us
588+
// to use the allocator from the container to construct the `__node_value_type` in the
589+
// memory provided by the union member
590+
#ifndef _LIBCPP_CXX03_LANG
591+
586592
private:
587-
__node_value_type __value_;
593+
union {
594+
__node_value_type __value_;
595+
};
588596

589597
public:
590598
_LIBCPP_HIDE_FROM_ABI __node_value_type& __get_value() { return __value_; }
599+
#else
600+
601+
private:
602+
_ALIGNAS_TYPE(__node_value_type) unsigned char __buffer_[sizeof(__node_value_type)];
591603

604+
public:
605+
_LIBCPP_HIDE_FROM_ABI __node_value_type& __get_value() { return *reinterpret_cast<__node_value_type*>(__buffer_); }
606+
#endif
607+
608+
template <class _Alloc, class... _Args>
609+
_LIBCPP_HIDE_FROM_ABI explicit __tree_node(_Alloc& __na, _Args&&... __args) {
610+
allocator_traits<_Alloc>::construct(__na, std::addressof(__get_value()), std::forward<_Args>(__args)...);
611+
}
592612
~__tree_node() = delete;
593613
__tree_node(__tree_node const&) = delete;
594614
__tree_node& operator=(__tree_node const&) = delete;
@@ -1816,7 +1836,7 @@ typename __tree<_Tp, _Compare, _Allocator>::__node_holder
18161836
__tree<_Tp, _Compare, _Allocator>::__construct_node(_Args&&... __args) {
18171837
__node_allocator& __na = __node_alloc();
18181838
__node_holder __h(__node_traits::allocate(__na, 1), _Dp(__na));
1819-
__node_traits::construct(__na, std::addressof(__h->__get_value()), std::forward<_Args>(__args)...);
1839+
std::__construct_at(std::addressof(*__h), __na, std::forward<_Args>(__args)...);
18201840
__h.get_deleter().__value_constructed = true;
18211841
return __h;
18221842
}

0 commit comments

Comments
 (0)