Reland [libcxx]Put clang::trivial_abi on smart pointers

Reviewed By: ldionne,EricWF

    Tags: #libcxx

    Differential Revision: https://reviews.llvm.org/D82490

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<Foo> smart_arg) { ... }
+
+    Foo* raw_ptr_retval() { ... }
+    std::unique_ptr<Foo*> 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<B>, A) {} };
+    C c{{}, make_unique<B>, {}};
+
+
+  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<Foo> 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<Foo>(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<T>``. The following code will no longer compile.
+
+   .. code-block:: cpp
+
+       class Foo;
+       void func(std::unique_ptr<Foo> 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<Foo>);
+     void func() {
+        std::unique_ptr<Foo> 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)));
+                  ^
+                 // <<<Crash expected here
+     }
+
+ - Lifetime of local *returned* ``std::unique_ptr<>`` ends earlier.
+
+   Spot the bug:
+
+    .. code-block:: cpp
+
+     std::unique_ptr<Foo> create_and_subscribe(Bar* subscriber) {
+       auto foo = std::make_unique<Foo>();
+       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.

libcxx/docs/index.rst

@@ -164,6 +164,7 @@ Design Documents
    DesignDocs/FileTimeType
    DesignDocs/FeatureTestMacros
    DesignDocs/ExtendedCXX03Support
+   DesignDocs/UniquePtrTrivialAbi
 
 * `<atomic> design <http://libcxx.llvm.org/atomic_design.html>`_
 * `<type_traits> design <http://libcxx.llvm.org/type_traits_design.html>`_

libcxx/include/__config

@@ -105,6 +105,10 @@
 // Re-worked external template instantiations for std::string with a focus on
 // performance and fast-path inlining.
 #  define _LIBCPP_ABI_STRING_OPTIMIZED_EXTERNAL_INSTANTIATION
+// 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
 #elif _LIBCPP_ABI_VERSION == 1
 #  if !defined(_LIBCPP_OBJECT_FORMAT_COFF)
 // Enable compiling copies of now inline methods into the dylib to support

libcxx/include/memory

@@ -338,7 +338,7 @@ public:
     pointer release() noexcept;
     void reset(pointer p = pointer()) noexcept;
     void reset(nullptr_t) noexcept;
-    template <class U> void reset(U) = delete;
+  template <class U> void reset(U) = delete;
     void swap(unique_ptr& u) noexcept;
 };
 
@@ -2316,8 +2316,14 @@ struct __unique_ptr_deleter_sfinae<_Deleter&> {
   typedef false_type __enable_rval_overload;
 };
 
+#if defined(_LIBCPP_ABI_ENABLE_UNIQUE_PTR_TRIVIAL_ABI)
+#  define _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI __attribute__((trivial_abi))
+#else
+#  define _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI
+#endif
+
 template <class _Tp, class _Dp = default_delete<_Tp> >
-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 @@ public:
 
 
 template <class _Tp, class _Dp>
-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 @@ struct __compatible_with
     : 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 _Tp>
-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 @@ get_deleter(const shared_ptr<_Tp>& __p) _NOEXCEPT
 #endif  // _LIBCPP_NO_RTTI
 
 template<class _Tp>
-class _LIBCPP_TEMPLATE_VIS weak_ptr
+class _LIBCPP_SHARED_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS weak_ptr
 {
 public:
     typedef _Tp element_type;

libcxx/test/libcxx/memory/trivial_abi/shared_ptr_arg.pass.cpp

@@ -0,0 +1,51 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// <memory>
+
+// Test shared_ptr<T> with trivial_abi as parameter type.
+
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ABI_ENABLE_SHARED_PTR_TRIVIAL_ABI
+
+// There were assertion failures in both parse and codegen, which are fixed in clang 11.
+// UNSUPPORTED: clang-4, clang-5, clang-6, clang-7, clang-8, clang-9, clang-10
+
+#include <memory>
+#include <cassert>
+
+__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> /*unused*/) {
+  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<Node>(&shared)));
+
+  return 0;
+}

libcxx/test/libcxx/memory/trivial_abi/unique_ptr_arg.pass.cpp

@@ -0,0 +1,53 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// <memory>
+
+// Test unique_ptr<T> with trivial_abi as parameter type.
+
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ABI_ENABLE_UNIQUE_PTR_TRIVIAL_ABI
+
+// There were assertion failures in both parse and codegen, which are fixed in clang 11.
+// UNSUPPORTED: clang-4, clang-5, clang-6, clang-7, clang-8, clang-9, clang-10
+
+#include <memory>
+#include <cassert>
+
+__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> /*unused*/) {
+  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<Node>(&shared)));
+
+  // Check that the shared-value is still 1.
+  expect_1(&shared, true);
+  return 0;
+}

libcxx/test/libcxx/memory/trivial_abi/unique_ptr_array.pass.cpp

@@ -0,0 +1,55 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// <memory>
+
+// Test unique_ptr<T[]> with trivial_abi as parameter type.
+
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ABI_ENABLE_UNIQUE_PTR_TRIVIAL_ABI
+
+// There were assertion failures in both parse and codegen, which are fixed in clang 11.
+// UNSUPPORTED: clang-4, clang-5, clang-6, clang-7, clang-8, clang-9, clang-10
+
+#include <memory>
+#include <cassert>
+
+__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[]> /*unused*/) {
+  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<Node[]>(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;
+}

libcxx/test/libcxx/memory/trivial_abi/unique_ptr_destruction_order.pass.cpp

@@ -0,0 +1,62 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// <memory>
+
+// Test arguments destruction order involving unique_ptr<T> with trivial_abi.
+// Note: Unlike other tests in this directory, this is the only test that
+// exhibits a difference between the two modes in Microsft ABI.
+
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ABI_ENABLE_UNIQUE_PTR_TRIVIAL_ABI
+
+// There were assertion failures in both parse and codegen, which are fixed in clang 11.
+// UNSUPPORTED: clang-4, clang-5, clang-6, clang-7, clang-8, clang-9, clang-10
+
+#include <memory>
+#include <cassert>
+
+__attribute__((noinline)) void call_something() { asm volatile(""); }
+
+struct Base {
+  char* shared_buff;
+  int* cur_idx;
+  const char id;
+
+  explicit Base(char* buf, int* idx, char ch)
+      : shared_buff(buf), cur_idx(idx), id(ch) {}
+  ~Base() { shared_buff[(*cur_idx)++] = id; }
+};
+
+struct A : Base {
+  explicit A(char* buf, int* idx) : Base(buf, idx, 'A') {}
+};
+
+struct B : Base {
+  explicit B(char* buf, int* idx) : Base(buf, idx, 'B') {}
+};
+
+struct C : Base {
+  explicit C(char* buf, int* idx) : Base(buf, idx, 'C') {}
+};
+
+__attribute__((noinline)) void func(A /*unused*/, std::unique_ptr<B> /*unused*/,
+                                    C /*unused*/) {
+  call_something();
+}
+
+int main(int, char**) {
+  char shared_buf[3] = {'0', '0', '0'};
+  int cur_idx = 0;
+
+  func(A(shared_buf, &cur_idx), std::make_unique<B>(shared_buf, &cur_idx),
+       C(shared_buf, &cur_idx));
+
+  // With trivial_abi, the std::unique_ptr<B> arg is always destructed first.
+  assert(shared_buf[0] == 'B');
+  return 0;
+}

libcxx/test/libcxx/memory/trivial_abi/unique_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
+//
+//===----------------------------------------------------------------------===//
+
+// <memory>
+
+// Test unique_ptr<T> with trivial_abi as return-type.
+
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ABI_ENABLE_UNIQUE_PTR_TRIVIAL_ABI
+
+// There were assertion failures in both parse and codegen, which are fixed in clang 11.
+// UNSUPPORTED: clang-4, clang-5, clang-6, clang-7, clang-8, clang-9, clang-10
+
+#include <memory>
+#include <cassert>
+
+__attribute__((noinline)) void call_something() { asm volatile(""); }
+
+struct Node {
+  explicit Node() {}
+  ~Node() {}
+};
+
+__attribute__((noinline)) std::unique_ptr<Node> make_val(void** local_addr) {
+  call_something();
+
+  auto ret = std::make_unique<Node>();
+
+  // 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;
+}

libcxx/test/libcxx/memory/trivial_abi/weak_ptr_ret.pass.cpp

@@ -0,0 +1,55 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// <memory>
+
+// Test weak_ptr<T> with trivial_abi as return-type.
+
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ABI_ENABLE_SHARED_PTR_TRIVIAL_ABI
+
+// There were assertion failures in both parse and codegen, which are fixed in clang 11.
+// UNSUPPORTED: clang-4, clang-5, clang-6, clang-7, clang-8, clang-9, clang-10
+
+#include <memory>
+#include <cassert>
+
+__attribute__((noinline)) void call_something() { asm volatile(""); }
+
+struct Node {
+  explicit Node() {}
+  ~Node() {}
+};
+
+__attribute__((noinline)) std::weak_ptr<Node>
+make_val(std::shared_ptr<Node>& sptr, void** local_addr) {
+  call_something();
+
+  std::weak_ptr<Node> 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<Node>();
+  std::weak_ptr<Node> 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;
+}