[libcxx] func.wrap.func.con: Unset function before destroying anything

Be defensive against a reentrant std::function::operator=(nullptr_t), in case
the held function object has a non-trivial destructor.  Destroying the function
object in-place can lead to the destructor being called twice.

Patch by Duncan P. N. Exon Smith. C++03 support by Volodymyr Sapsai.

rdar://problem/32836603

Reviewers: EricWF, mclow.lists

Reviewed By: mclow.lists

Subscribers: cfe-commits, arphaman

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


git-svn-id: https://llvm.org/svn/llvm-project/libcxx/trunk@330885 91177308-0d34-0410-b5e6-96231b3b80d8

include/__functional_03

@@ -600,7 +600,10 @@ template<class _Rp>
 function<_Rp()>&
 function<_Rp()>::operator=(const function& __f)
 {
+    if (__f)
         function(__f).swap(*this);
+    else
+        *this = nullptr;
     return *this;
 }
 
@@ -608,11 +611,12 @@ template<class _Rp>
 function<_Rp()>&
 function<_Rp()>::operator=(nullptr_t)
 {
-    if (__f_ == (__base*)&__buf_)
+    __base* __t = __f_;
-        __f_->destroy();
-    else if (__f_)
-        __f_->destroy_deallocate();
     __f_ = 0;
+    if (__t == (__base*)&__buf_)
+        __t->destroy();
+    else if (__t)
+        __t->destroy_deallocate();
     return *this;
 }
 
@@ -876,7 +880,10 @@ template<class _Rp, class _A0>
 function<_Rp(_A0)>&
 function<_Rp(_A0)>::operator=(const function& __f)
 {
+    if (__f)
         function(__f).swap(*this);
+    else
+        *this = nullptr;
     return *this;
 }
 
@@ -884,11 +891,12 @@ template<class _Rp, class _A0>
 function<_Rp(_A0)>&
 function<_Rp(_A0)>::operator=(nullptr_t)
 {
-    if (__f_ == (__base*)&__buf_)
+    __base* __t = __f_;
-        __f_->destroy();
-    else if (__f_)
-        __f_->destroy_deallocate();
     __f_ = 0;
+    if (__t == (__base*)&__buf_)
+        __t->destroy();
+    else if (__t)
+        __t->destroy_deallocate();
     return *this;
 }
 
@@ -1152,7 +1160,10 @@ template<class _Rp, class _A0, class _A1>
 function<_Rp(_A0, _A1)>&
 function<_Rp(_A0, _A1)>::operator=(const function& __f)
 {
+    if (__f)
         function(__f).swap(*this);
+    else
+        *this = nullptr;
     return *this;
 }
 
@@ -1160,11 +1171,12 @@ template<class _Rp, class _A0, class _A1>
 function<_Rp(_A0, _A1)>&
 function<_Rp(_A0, _A1)>::operator=(nullptr_t)
 {
-    if (__f_ == (__base*)&__buf_)
+    __base* __t = __f_;
-        __f_->destroy();
-    else if (__f_)
-        __f_->destroy_deallocate();
     __f_ = 0;
+    if (__t == (__base*)&__buf_)
+        __t->destroy();
+    else if (__t)
+        __t->destroy_deallocate();
     return *this;
 }
 
@@ -1428,7 +1440,10 @@ template<class _Rp, class _A0, class _A1, class _A2>
 function<_Rp(_A0, _A1, _A2)>&
 function<_Rp(_A0, _A1, _A2)>::operator=(const function& __f)
 {
+    if (__f)
         function(__f).swap(*this);
+    else
+        *this = nullptr;
     return *this;
 }
 
@@ -1436,11 +1451,12 @@ template<class _Rp, class _A0, class _A1, class _A2>
 function<_Rp(_A0, _A1, _A2)>&
 function<_Rp(_A0, _A1, _A2)>::operator=(nullptr_t)
 {
-    if (__f_ == (__base*)&__buf_)
+    __base* __t = __f_;
-        __f_->destroy();
-    else if (__f_)
-        __f_->destroy_deallocate();
     __f_ = 0;
+    if (__t == (__base*)&__buf_)
+        __t->destroy();
+    else if (__t)
+        __t->destroy_deallocate();
     return *this;
 }
 

include/functional

@@ -1818,11 +1818,7 @@ template<class _Rp, class ..._ArgTypes>
 function<_Rp(_ArgTypes...)>&
 function<_Rp(_ArgTypes...)>::operator=(function&& __f) _NOEXCEPT
 {
-    if ((void *)__f_ == &__buf_)
+    *this = nullptr;
-        __f_->destroy();
-    else if (__f_)
-        __f_->destroy_deallocate();
-    __f_ = 0;
     if (__f.__f_ == 0)
         __f_ = 0;
     else if ((void *)__f.__f_ == &__f.__buf_)
@@ -1842,11 +1838,12 @@ template<class _Rp, class ..._ArgTypes>
 function<_Rp(_ArgTypes...)>&
 function<_Rp(_ArgTypes...)>::operator=(nullptr_t) _NOEXCEPT
 {
-    if ((void *)__f_ == &__buf_)
+    __base* __t = __f_;
-        __f_->destroy();
-    else if (__f_)
-        __f_->destroy_deallocate();
     __f_ = 0;
+    if ((void *)__t == &__buf_)
+        __t->destroy();
+    else if (__t)
+        __t->destroy_deallocate();
     return *this;
 }
 

test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/move_reentrant.pass.cpp

@@ -0,0 +1,46 @@
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// <functional>
+
+// class function<R(ArgTypes...)>
+
+// function& operator=(function &&);
+
+#include <functional>
+#include <cassert>
+
+#include "test_macros.h"
+
+struct A
+{
+  static std::function<void()> global;
+  static bool cancel;
+
+  ~A() {
+    DoNotOptimize(cancel);
+    if (cancel)
+      global = std::function<void()>(nullptr);
+  }
+  void operator()() {}
+};
+
+std::function<void()> A::global;
+bool A::cancel = false;
+
+int main()
+{
+  A::global = A();
+  assert(A::global.target<A>());
+
+  // Check that we don't recurse in A::~A().
+  A::cancel = true;
+  A::global = std::function<void()>(nullptr);
+  assert(!A::global.target<A>());
+}

test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/nullptr_t_assign_reentrant.pass.cpp

@@ -0,0 +1,46 @@
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// <functional>
+
+// class function<R(ArgTypes...)>
+
+// function& operator=(nullptr_t);
+
+#include <functional>
+#include <cassert>
+
+#include "test_macros.h"
+
+struct A
+{
+  static std::function<void()> global;
+  static bool cancel;
+
+  ~A() {
+    DoNotOptimize(cancel);
+    if (cancel)
+      global = nullptr;
+  }
+  void operator()() {}
+};
+
+std::function<void()> A::global;
+bool A::cancel = false;
+
+int main()
+{
+  A::global = A();
+  assert(A::global.target<A>());
+
+  // Check that we don't recurse in A::~A().
+  A::cancel = true;
+  A::global = nullptr;
+  assert(!A::global.target<A>());
+}