Skip to content

Commit 91a4e94

Browse files
committed
[libcxx] Remove empty ~__no_destroy
Primary motivation is that after #84651 msan will complain if fields accessed after ~__no_destroy. Previously msan assumed that __obj_ will have own destructor, were we will poison the field, but it never happened before. After #84651 msan will complain on any field access after `~__no_destroy`. As is Msan does validate fields destruction order for classes with trivial destructor. Additionally 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. Reviewers: #reviewers-libcxx Pull Request: #89882
1 parent 21b8492 commit 91a4e94

File tree

2 files changed

+38
-19
lines changed

2 files changed

+38
-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) {}
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) unsigned char __obj_[sizeof(_Tp)] = {};
5950
};
6051

6152
_LIBCPP_END_NAMESPACE_STD
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
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+
struct DestroyLast {
15+
~DestroyLast() { assert(*ptr == 5); }
16+
17+
int* ptr;
18+
} last;
19+
20+
static std::__no_destroy<int> nd_int(5);
21+
22+
void test() { last.ptr = &nd_int.__get(); }
23+
24+
int main(int, char**) {
25+
test();
26+
27+
return 0;
28+
}

0 commit comments

Comments
 (0)