Skip to content

[libc++] Fix a segfault in weak_ptr(const weak_ptr<Y>&) #67956

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 2, 2023

Conversation

AMP999
Copy link
Contributor

@AMP999 AMP999 commented Oct 2, 2023

Fixes #40459

@AMP999 AMP999 requested a review from a team as a code owner October 2, 2023 07:35
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Oct 2, 2023
@llvmbot
Copy link
Member

llvmbot commented Oct 2, 2023

@llvm/pr-subscribers-libcxx

Changes

Fixes #40459


Full diff: https://github.com/llvm/llvm-project/pull/67956.diff

2 Files Affected:

  • (modified) libcxx/include/__memory/shared_ptr.h (+9-8)
  • (added) libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.weak/util.smartptr.weak.const/pr40459.pass.cpp (+61)
diff --git a/libcxx/include/__memory/shared_ptr.h b/libcxx/include/__memory/shared_ptr.h
index 6be2f22184590ae..33a1b95a31ddbd5 100644
--- a/libcxx/include/__memory/shared_ptr.h
+++ b/libcxx/include/__memory/shared_ptr.h
@@ -1727,11 +1727,11 @@ template<class _Yp, __enable_if_t<__compatible_with<_Yp, _Tp>::value, int> >
 inline
 weak_ptr<_Tp>::weak_ptr(weak_ptr<_Yp> const& __r)
          _NOEXCEPT
-    : __ptr_(__r.__ptr_),
-      __cntrl_(__r.__cntrl_)
+    : __ptr_(nullptr),
+      __cntrl_(nullptr)
 {
-    if (__cntrl_)
-        __cntrl_->__add_weak();
+    shared_ptr<_Yp> __s = __r.lock();
+    *this = weak_ptr<_Tp>(__s);
 }
 
 template<class _Tp>
@@ -1749,11 +1749,12 @@ template<class _Yp, __enable_if_t<__compatible_with<_Yp, _Tp>::value, int> >
 inline
 weak_ptr<_Tp>::weak_ptr(weak_ptr<_Yp>&& __r)
          _NOEXCEPT
-    : __ptr_(__r.__ptr_),
-      __cntrl_(__r.__cntrl_)
+    : __ptr_(nullptr),
+      __cntrl_(nullptr)
 {
-    __r.__ptr_ = nullptr;
-    __r.__cntrl_ = nullptr;
+    shared_ptr<_Yp> __s = __r.lock();
+    *this = weak_ptr<_Tp>(__s);
+    __r.reset();
 }
 
 template<class _Tp>
diff --git a/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.weak/util.smartptr.weak.const/pr40459.pass.cpp b/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.weak/util.smartptr.weak.const/pr40459.pass.cpp
new file mode 100644
index 000000000000000..25141ad2de6c992
--- /dev/null
+++ b/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.weak/util.smartptr.weak.const/pr40459.pass.cpp
@@ -0,0 +1,61 @@
+//===----------------------------------------------------------------------===//
+//
+// 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>
+
+// weak_ptr
+
+// template<class Y> weak_ptr(const weak_ptr<Y>& r);
+// template<class Y> weak_ptr(weak_ptr<Y>&& r);
+//
+// Regression test for https://github.com/llvm/llvm-project/issues/40459
+// Verify that these constructors never attempt a derived-to-virtual-base
+// conversion on a dangling weak_ptr.
+
+#include <cassert>
+#include <cstring>
+#include <memory>
+#include <utility>
+
+#include "test_macros.h"
+
+struct A {
+  int i;
+  virtual ~A() {}
+};
+struct B : public virtual A {
+  int j;
+};
+struct Deleter {
+  void operator()(void*) const {
+    // do nothing
+  }
+};
+
+int main(int, char**) {
+#if TEST_STD_VER >= 11
+  alignas(B) char buffer[sizeof(B)];
+#else
+  std::aligned_storage<sizeof(B), std::alignment_of<B>::value>::type buffer;
+#endif
+  B *pb = ::new ((void*)&buffer) B();
+  std::shared_ptr<B> sp = std::shared_ptr<B>(pb, Deleter());
+  std::weak_ptr<B> wp = sp;
+  sp = nullptr;
+  assert(wp.expired());
+
+  // Overwrite the B object with junk.
+  std::memset(&buffer, '*', sizeof(buffer));
+
+  std::weak_ptr<A> wq = wp;
+  assert(wq.expired());
+  std::weak_ptr<A> wr = std::move(wp);
+  assert(wr.expired());
+
+  return 0;
+}

@github-actions
Copy link

github-actions bot commented Oct 2, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

@AMP999 AMP999 force-pushed the pr-weak-ptr-segfault branch from 6d7d0d3 to 7440baa Compare October 2, 2023 08:22
@AMP999 AMP999 force-pushed the pr-weak-ptr-segfault branch from 7440baa to d2eab55 Compare October 2, 2023 08:45
@ldionne
Copy link
Member

ldionne commented Oct 2, 2023

Thanks for fixing! You can ignore the failure in https://buildkite.com/llvm-project/github-pull-requests/builds/5233#018aef91-8930-4221-a325-d88dfa640bf8, it would go away if you rebased.

@ldionne ldionne merged commit b861457 into llvm:main Oct 2, 2023
@AMP999 AMP999 deleted the pr-weak-ptr-segfault branch October 4, 2023 06:47
Guzhu-AMD pushed a commit to GPUOpen-Drivers/llvm-project that referenced this pull request Oct 12, 2023
Local branch amd-gfx 6cf360c Merged main:451255b207c8 into amd-gfx:03a4dc920122
Remote branch main b861457 [libc++] Fix a segfault in weak_ptr(const weak_ptr<Y>&) (llvm#67956)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

weak_ptr(weak_ptr const &) can segfault in the presence of virtual bases
3 participants