Revert "[libcxx] Put clang::trivial_abi on std::unique_ptr, std::shared_ptr, and std::weak_ptr"

This reverts commit 5cde3c9633.

The tests were reported failing on clang10

libcxx/docs/DesignDocs/UniquePtrTrivialAbi.rst

@@ -1,149 +0,0 @@
-=============================================
-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,7 +164,6 @@ 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,10 +105,6 @@
 // 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,14 +2316,8 @@ 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 __atribute__((trivial_abi))
-#else
-#  define _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI
-#endif
-
 template <class _Tp, class _Dp = default_delete<_Tp> >
-class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr {
+class _LIBCPP_TEMPLATE_VIS unique_ptr {
 public:
   typedef _Tp element_type;
   typedef _Dp deleter_type;
@@ -2531,7 +2525,7 @@ public:
 
 
 template <class _Tp, class _Dp>
-class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp[], _Dp> {
+class _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp[], _Dp> {
 public:
   typedef _Tp element_type;
   typedef _Dp deleter_type;
@@ -3543,14 +3537,8 @@ 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_SHARED_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS shared_ptr
+class _LIBCPP_TEMPLATE_VIS shared_ptr
 {
 public:
 #if _LIBCPP_STD_VER > 14
@@ -4538,7 +4526,7 @@ get_deleter(const shared_ptr<_Tp>& __p) _NOEXCEPT
 #endif  // _LIBCPP_NO_RTTI
 
 template<class _Tp>
-class _LIBCPP_SHARED_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS weak_ptr
+class _LIBCPP_TEMPLATE_VIS weak_ptr
 {
 public:
     typedef _Tp element_type;

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

@@ -1,48 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// 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
-
-#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> 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<Node>(&shared)));
-
-  return 0;
-}

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

@@ -1,50 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// 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
-
-#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> 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<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

@@ -1,52 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// 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
-
-#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[]> 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<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

@@ -1,59 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// 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
-
-#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 a_struct, std::unique_ptr<B> b,
-                                    C c_struct) {
-  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

@@ -1,49 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// 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
-
-#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

@@ -1,52 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// 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
-
-#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>(&shared);
-  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;
-}