Skip to content

Commit 85a9528

Browse files
authored
[libcxx] Remove empty ~__no_destroy (#89882)
Primary motivation: is that after #84651 msan will complain if fields accessed after ~__no_destroy. My understanding of the https://eel.is/c++draft/basic.life#10 Static object with trivial destruction has program lifetime. Static object with empty destuctor has implicit lifetime, and accessing the object after lifetime is UB. It was UB before #84651, it's just msan ignored union members. Existing code with unions uses empty destructor, so accessing after the main() can cause UB. "placement new" version can have trivial destructor, so there is no end of lifetime. Secondary motivation: empty destructor will register __cxa_atexit with -O0. https://gcc.godbolt.org/z/hce587b65 We can not remove the destructor with union where _Tp can have non-trivial destructor. But we can remove destructor if we use in-place new instead of union. https://gcc.godbolt.org/z/Yqxx57eEd - empty even with -O0. New test fails without the patch on https://lab.llvm.org/buildbot/#/builders/sanitizer-x86_64-linux-bootstrap-msan
1 parent 90a959a commit 85a9528

File tree

3 files changed

+42
-19
lines changed

3 files changed

+42
-19
lines changed

libcxx/include/__utility/no_destroy.h

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include <__config>
1313
#include <__type_traits/is_constant_evaluated.h>
1414
#include <__utility/forward.h>
15+
#include <new>
1516

1617
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
1718
# pragma GCC system_header
@@ -29,33 +30,23 @@ struct __uninitialized_tag {};
2930
// initialization using __emplace.
3031
template <class _Tp>
3132
struct __no_destroy {
32-
_LIBCPP_CONSTEXPR_SINCE_CXX14 _LIBCPP_HIDE_FROM_ABI explicit __no_destroy(__uninitialized_tag) : __dummy_() {
33-
if (__libcpp_is_constant_evaluated()) {
34-
__dummy_ = char();
35-
}
36-
}
37-
_LIBCPP_HIDE_FROM_ABI ~__no_destroy() {
38-
// nothing
39-
}
33+
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR explicit __no_destroy(__uninitialized_tag) : __obj_() {}
4034

4135
template <class... _Args>
42-
_LIBCPP_CONSTEXPR _LIBCPP_HIDE_FROM_ABI explicit __no_destroy(_Args&&... __args)
43-
: __obj_(std::forward<_Args>(__args)...) {}
36+
_LIBCPP_HIDE_FROM_ABI explicit __no_destroy(_Args&&... __args) {
37+
::new ((void*)__obj_) _Tp(std::forward<_Args>(__args)...);
38+
}
4439

4540
template <class... _Args>
46-
_LIBCPP_CONSTEXPR_SINCE_CXX14 _LIBCPP_HIDE_FROM_ABI _Tp& __emplace(_Args&&... __args) {
47-
new (&__obj_) _Tp(std::forward<_Args>(__args)...);
48-
return __obj_;
41+
_LIBCPP_HIDE_FROM_ABI _Tp& __emplace(_Args&&... __args) {
42+
return *(::new ((void*)__obj_) _Tp(std::forward<_Args>(__args)...));
4943
}
5044

51-
_LIBCPP_CONSTEXPR_SINCE_CXX14 _LIBCPP_HIDE_FROM_ABI _Tp& __get() { return __obj_; }
52-
_LIBCPP_CONSTEXPR_SINCE_CXX14 _LIBCPP_HIDE_FROM_ABI _Tp const& __get() const { return __obj_; }
45+
_LIBCPP_HIDE_FROM_ABI _Tp& __get() { return *reinterpret_cast<_Tp*>(__obj_); }
46+
_LIBCPP_HIDE_FROM_ABI _Tp const& __get() const { return *reinterpret_cast<const _Tp*>(__obj_); }
5347

5448
private:
55-
union {
56-
_Tp __obj_;
57-
char __dummy_; // so we can initialize a member even with __uninitialized_tag for constexpr-friendliness
58-
};
49+
_ALIGNAS_TYPE(_Tp) char __obj_[sizeof(_Tp)];
5950
};
6051

6152
_LIBCPP_END_NAMESPACE_STD

libcxx/test/libcxx/transitive_includes/cxx20.csv

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ chrono cwchar
129129
chrono forward_list
130130
chrono limits
131131
chrono locale
132+
chrono new
132133
chrono optional
133134
chrono ostream
134135
chrono ratio
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
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+
#include <__utility/no_destroy.h>
10+
#include <cassert>
11+
12+
#include "test_macros.h"
13+
14+
#if TEST_STD_VER > 17
15+
// Test constexpr-constructibility.
16+
constinit std::__no_destroy<int> nd_int_const(std::__uninitialized_tag{});
17+
#endif
18+
19+
struct DestroyLast {
20+
~DestroyLast() { assert(*ptr == 5); }
21+
22+
int* ptr;
23+
} last;
24+
25+
static std::__no_destroy<int> nd_int(5);
26+
27+
int main(int, char**) {
28+
last.ptr = &nd_int.__get();
29+
30+
return 0;
31+
}

0 commit comments

Comments
 (0)