diff --git a/libcxx/docs/DesignDocs/UniquePtrTrivialAbi.rst b/libcxx/docs/DesignDocs/UniquePtrTrivialAbi.rst new file mode 100644 --- /dev/null +++ b/libcxx/docs/DesignDocs/UniquePtrTrivialAbi.rst @@ -0,0 +1,149 @@ +============================================= +Enable std::unique_ptr [[clang::trivial_abi]] +============================================= + +Background +========== + +Consider the follow snippets + + +.. code-block:: cpp + + void raw_func(Foo* raw_arg) { ... } + void smart_func(std::unique_ptr smart_arg) { ... } + + Foo* raw_ptr_retval() { ... } + std::unique_ptr smart_ptr_retval() { ... } + + + +The argument ``raw_arg`` could be passed in a register but ``smart_arg`` could not, due to current +implementation. + +Specifically, in the ``smart_arg`` case, the caller secretly constructs a temporary ``std::unique_ptr`` +in its stack-frame, and then passes a pointer to it to the callee in a hidden parameter. +Similarly, the return value from ``smart_ptr_retval`` is secretly allocated in the caller and +passed as a secret reference to the callee. + + +Goal +=================== + +``std::unique_ptr`` is passed directly in a register. + +Design +====== + +* Annotate the two definitions of ``std::unique_ptr`` with ``clang::trivial_abi`` attribute. +* Put the attribuate behind a flag because this change has potential compilation and runtime breakages. + + +This comes with some side effects: + +* ``std::unique_ptr`` parameters will now be destroyed by callees, rather than callers. + It is worth noting that destruction by callee is not unique to the use of trivial_abi attribute. + In most Microsoft's ABIs, arguments are always destroyed by the callee. + + Consequently, this may change the destruction order for function parameters to an order that is non-conforming to the standard. + For example: + + + .. code-block:: cpp + + struct A { ~A(); }; + struct B { ~B(); }; + struct C { C(A, unique_ptr, A) {} }; + C c{{}, make_unique, {}}; + + + In a conforming implementation, the destruction order for C::C's parameters is required to be ``~A(), ~B(), ~A()`` but with this mode enabled, we'll instead see ``~B(), ~A(), ~A()``. + +* Reduced code-size. + + +Performance impact +------------------ + +Google has measured performance improvements of up to 1.6% on some large server macrobenchmarks, and a small reduction in binary sizes. + +This also affects null pointer optimization + +Clang's optimizer can now figure out when a `std::unique_ptr` is known to contain *non*-null. +(Actually, this has been a *missed* optimization all along.) + + +.. code-block:: cpp + + struct Foo { + ~Foo(); + }; + std::unique_ptr make_foo(); + void do_nothing(const Foo&) + + void bar() { + auto x = make_foo(); + do_nothing(*x); + } + + +With this change, ``~Foo()`` will be called even if ``make_foo`` returns ``unique_ptr(nullptr)``. +The compiler can now assume that ``x.get()`` cannot be null by the end of ``bar()``, because +the deference of ``x`` would be UB if it were ``nullptr``. (This dereference would not have caused +a segfault, because no load is generated for dereferencing a pointer to a reference. This can be detected with ``-fsanitize=null``). + + +Potential breakages +------------------- + +The following breakages were discovered by enabling this change and fixing the resulting issues in a large code base. + +- Compilation failures + + - Function definitions now require complete type ``T`` for parameters with type ``std::unique_ptr``. The following code will no longer compile. + + .. code-block:: cpp + + class Foo; + void func(std::unique_ptr arg) { /* never use `arg` directly */ } + + - Fix: Remove forward-declaration of ``Foo`` and include its proper header. + +- Runtime Failures + + - Lifetime of ``std::unique_ptr<>`` arguments end earlier (at the end of the callee's body, rather than at the end of the full expression containing the call). + + .. code-block:: cpp + + util::Status run_worker(std::unique_ptr); + void func() { + std::unique_ptr smart_foo = ...; + Foo* owned_foo = smart_foo.get(); + // Currently, the following would "work" because the argument to run_worker() is deleted at the end of func() + // With the new calling convention, it will be deleted at the end of run_worker(), + // making this an access to freed memory. + owned_foo->Bar(run_worker(std::move(smart_foo))); + ^ + // <<`` ends earlier. + + Spot the bug: + + .. code-block:: cpp + + std::unique_ptr create_and_subscribe(Bar* subscriber) { + auto foo = std::make_unique(); + subscriber->sub([&foo] { foo->do_thing();} ); + return foo; + } + + One could point out this is an obvious stack-use-after return bug. + With the current calling convention, running this code with ASAN enabled, however, would not yield any "issue". + So is this a bug in ASAN? (Spoiler: No) + + This currently would "work" only because the storage for ``foo`` is in the caller's stackframe. + In other words, ``&foo`` in callee and ``&foo`` in the caller are the same address. + +ASAN can be used to detect both of these. diff --git a/libcxx/docs/index.rst b/libcxx/docs/index.rst --- a/libcxx/docs/index.rst +++ b/libcxx/docs/index.rst @@ -164,6 +164,7 @@ DesignDocs/FileTimeType DesignDocs/FeatureTestMacros DesignDocs/ExtendedCXX03Support + DesignDocs/UniquePtrTrivialAbi * ` design `_ * ` design `_ diff --git a/libcxx/include/__config b/libcxx/include/__config --- a/libcxx/include/__config +++ b/libcxx/include/__config @@ -105,6 +105,12 @@ // Re-worked external template instantiations for std::string with a focus on // performance and fast-path inlining. # define _LIBCPP_ABI_STRING_OPTIMIZED_EXTERNAL_INSTANTIATION +# if __has_cpp_attribute(clang::trivial_abi) +// Enable clang::trivial_abi on std::unique_ptr. +# define _LIBCPP_ABI_ENABLE_UNIQUE_PTR_TRIVIAL_ABI +// Enable clang::trivial_abi on std::shared_ptr and std::weak_ptr +# define _LIBCPP_ABI_ENABLE_SHARED_PTR_TRIVIAL_ABI +# endif #elif _LIBCPP_ABI_VERSION == 1 # if !defined(_LIBCPP_OBJECT_FORMAT_COFF) // Enable compiling copies of now inline methods into the dylib to support diff --git a/libcxx/include/memory b/libcxx/include/memory --- a/libcxx/include/memory +++ b/libcxx/include/memory @@ -338,7 +338,7 @@ pointer release() noexcept; void reset(pointer p = pointer()) noexcept; void reset(nullptr_t) noexcept; - template void reset(U) = delete; + template void reset(U) = delete; void swap(unique_ptr& u) noexcept; }; @@ -2316,8 +2316,14 @@ typedef false_type __enable_rval_overload; }; +#if defined(_LIBCPP_ABI_ENABLE_UNIQUE_PTR_TRIVIAL_ABI) +# define _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI __atribute__((trivial_abi)) +#else +# define _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI +#endif + template > -class _LIBCPP_TEMPLATE_VIS unique_ptr { +class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr { public: typedef _Tp element_type; typedef _Dp deleter_type; @@ -2525,7 +2531,7 @@ template -class _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp[], _Dp> { +class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp[], _Dp> { public: typedef _Tp element_type; typedef _Dp deleter_type; @@ -3537,8 +3543,14 @@ : is_convertible<_Tp*, _Up*> {}; #endif // _LIBCPP_STD_VER > 14 +#if defined(_LIBCPP_ABI_ENABLE_SHARED_PTR_TRIVIAL_ABI) +# define _LIBCPP_SHARED_PTR_TRIVIAL_ABI __attribute__((trivial_abi)) +#else +# define _LIBCPP_SHARED_PTR_TRIVIAL_ABI +#endif + template -class _LIBCPP_TEMPLATE_VIS shared_ptr +class _LIBCPP_SHARED_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS shared_ptr { public: #if _LIBCPP_STD_VER > 14 @@ -4526,7 +4538,7 @@ #endif // _LIBCPP_NO_RTTI template -class _LIBCPP_TEMPLATE_VIS weak_ptr +class _LIBCPP_SHARED_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS weak_ptr { public: typedef _Tp element_type; diff --git a/libcxx/test/libcxx/memory/trivial_abi/shared_ptr_arg.pass.cpp b/libcxx/test/libcxx/memory/trivial_abi/shared_ptr_arg.pass.cpp new file mode 100644 --- /dev/null +++ b/libcxx/test/libcxx/memory/trivial_abi/shared_ptr_arg.pass.cpp @@ -0,0 +1,48 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// + +// Test shared_ptr with trivial_abi as parameter type. + +// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ABI_ENABLE_SHARED_PTR_TRIVIAL_ABI + +#include +#include + +__attribute__((noinline)) void call_something() { asm volatile(""); } + +struct Node { + int* shared_val; + + explicit Node(int* ptr) : shared_val(ptr) {} + ~Node() { ++(*shared_val); } +}; + +__attribute__((noinline)) bool get_val(std::shared_ptr node) { + call_something(); + return true; +} + +__attribute__((noinline)) void expect_1(int* shared, bool /*unused*/) { + assert(*shared == 1); +} + +int main(int, char**) { + int shared = 0; + + // Without trivial-abi, the shared_ptr is deleted at the end of this + // statement; expect_1 will see shared == 0 because it's not incremented (in + // ~Node()) until expect_1 returns. + // + // With trivial-abi, expect_1 will see shared == 1 because shared_val is + // incremented before get_val returns. + expect_1(&shared, get_val(std::make_shared(&shared))); + + return 0; +} diff --git a/libcxx/test/libcxx/memory/trivial_abi/unique_ptr_arg.pass.cpp b/libcxx/test/libcxx/memory/trivial_abi/unique_ptr_arg.pass.cpp new file mode 100644 --- /dev/null +++ b/libcxx/test/libcxx/memory/trivial_abi/unique_ptr_arg.pass.cpp @@ -0,0 +1,50 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// + +// Test unique_ptr with trivial_abi as parameter type. + +// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ABI_ENABLE_UNIQUE_PTR_TRIVIAL_ABI + +#include +#include + +__attribute__((noinline)) void call_something() { asm volatile(""); } + +struct Node { + int* shared_val; + + explicit Node(int* ptr) : shared_val(ptr) {} + ~Node() { ++(*shared_val); } +}; + +__attribute__((noinline)) bool get_val(std::unique_ptr node) { + call_something(); + return true; +} + +__attribute__((noinline)) void expect_1(int* shared, bool /*unused*/) { + assert(*shared == 1); +} + +int main(int, char**) { + int shared = 0; + + // Without trivial-abi, the unique_ptr is deleted at the end of this + // statement; expect_1 will see shared == 0 because it's not incremented (in + // ~Node()) until expect_1 returns. + // + // With trivial-abi, expect_1 will see shared == 1 because shared_val is + // incremented before get_val returns. + expect_1(&shared, get_val(std::make_unique(&shared))); + + // Check that the shared-value is still 1. + expect_1(&shared, true); + return 0; +} diff --git a/libcxx/test/libcxx/memory/trivial_abi/unique_ptr_array.pass.cpp b/libcxx/test/libcxx/memory/trivial_abi/unique_ptr_array.pass.cpp new file mode 100644 --- /dev/null +++ b/libcxx/test/libcxx/memory/trivial_abi/unique_ptr_array.pass.cpp @@ -0,0 +1,52 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// + +// Test unique_ptr with trivial_abi as parameter type. + +// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ABI_ENABLE_UNIQUE_PTR_TRIVIAL_ABI + +#include +#include + +__attribute__((noinline)) void call_something() { asm volatile(""); } + +struct Node { + int* shared_val; + + explicit Node(int* ptr) : shared_val(ptr) {} + ~Node() { ++(*shared_val); } +}; + +__attribute__((noinline)) bool get_val(std::unique_ptr node) { + call_something(); + return true; +} + +__attribute__((noinline)) void expect_3(int* shared, bool /*unused*/) { + assert(*shared == 3); +} + +int main(int, char**) { + int shared = 0; + + // Without trivial-abi, the unique_ptr is deleted at the end of this + // statement, expect_3 will see shared == 0 because it's not incremented (in + // ~Node()) until the end of this statement. + // + // With trivial-abi, shared_val is incremented 3 times before get_val returns + // because ~Node() was called 3 times. + expect_3(&shared, get_val(std::unique_ptr(new Node[3]{ + Node(&shared), Node(&shared), Node(&shared)}))); + + // Check that shared_value is still 3 (ie., ~Node() isn't called again by the end of the full-expression above) + expect_3(&shared, true); + + return 0; +} diff --git a/libcxx/test/libcxx/memory/trivial_abi/unique_ptr_ret.pass.cpp b/libcxx/test/libcxx/memory/trivial_abi/unique_ptr_ret.pass.cpp new file mode 100644 --- /dev/null +++ b/libcxx/test/libcxx/memory/trivial_abi/unique_ptr_ret.pass.cpp @@ -0,0 +1,49 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// + +// Test unique_ptr with trivial_abi as return-type. + +// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ABI_ENABLE_UNIQUE_PTR_TRIVIAL_ABI + +#include +#include + +__attribute__((noinline)) void call_something() { asm volatile(""); } + +struct Node { + explicit Node() {} + ~Node() {} +}; + +__attribute__((noinline)) std::unique_ptr make_val(void** local_addr) { + call_something(); + + auto ret = std::make_unique(); + + // Capture the local address of ret. + *local_addr = &ret; + + return ret; +} + +int main(int, char**) { + void* local_addr = nullptr; + auto ret = make_val(&local_addr); + assert(local_addr != nullptr); + + // Without trivial_abi, &ret == local_addr because the return value + // is allocated here in main's stackframe. + // + // With trivial_abi, local_addr is the address of a local variable in + // make_val, and hence different from &ret. + assert((void*)&ret != local_addr); + + return 0; +} diff --git a/libcxx/test/libcxx/memory/trivial_abi/weak_ptr_ret.pass.cpp b/libcxx/test/libcxx/memory/trivial_abi/weak_ptr_ret.pass.cpp new file mode 100644 --- /dev/null +++ b/libcxx/test/libcxx/memory/trivial_abi/weak_ptr_ret.pass.cpp @@ -0,0 +1,52 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// + +// Test weak_ptr with trivial_abi as return-type. + +// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ABI_ENABLE_SHARED_PTR_TRIVIAL_ABI + +#include +#include + +__attribute__((noinline)) void call_something() { asm volatile(""); } + +struct Node { + explicit Node() {} + ~Node() {} +}; + +__attribute__((noinline)) std::weak_ptr +make_val(std::shared_ptr& sptr, void** local_addr) { + call_something(); + + std::weak_ptr ret; + ret = sptr; + + // Capture the local address of ret. + *local_addr = &ret; + + return ret; +} + +int main(int, char**) { + void* local_addr = nullptr; + auto sptr = std::make_shared(&shared); + std::weak_ptr ret = make_val(sptr, &local_addr); + assert(local_addr != nullptr); + + // Without trivial_abi, &ret == local_addr because the return value + // is allocated here in main's stackframe. + // + // With trivial_abi, local_addr is the address of a local variable in + // make_val, and hence different from &ret. + assert((void*)&ret != local_addr); + + return 0; +}