From 89b6ad3ab5f40a6538105d2fbe52faf9d49c48bb Mon Sep 17 00:00:00 2001
From: Giuseppe D'Angelo <giuseppe.dangelo@kdab.com>
Date: Sat, 23 Sep 2023 18:20:06 +0200
Subject: [PATCH] QWeakPointer: fix the converting constructor from rvalues

When constructing a QWeakPointer<T> from a rvalue QWeakPointer<X>,
even if X* is convertible to T*, actually doing the conversion
requires access to the pointee's vtable in case of virtual inheritance.

For instance:

  class Base { virtual ~Base(); };
  class Derived : public virtual Base {};

Now given a `Derived *ptr`, then a conversion of `ptr` to `Base *` is
implicit (it's a public base), but the compiler needs to dereference
`ptr` to find out where the Base sub-object is.

This access to the pointee requires protection, because by the time we
attempt the cast the pointee may have already been destroyed, or it's
being destroyed by another thread. Do that by going through a shared
pointer. (This matches the existing code for the converting assignment.)

This requires changing the private assign() method, used by QPointer, to
avoid going through a converting move assignment/construction, because
one can't upgrade a QWeakPointer tracking a QObject to a QSharedPointer.
Given it's the caller's responsibility to guard the lifetime of the
pointee passed into assign(), I can simply build a QWeakPointer<T> and
use ordinary (i.e. non-converting) move assignment instead.

Change-Id: I7743b334d479de7cefa6999395a33df06814c8f1
Pick-to: 6.5 6.6
Fixes: QTBUG-117483
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
---
 src/corelib/tools/qsharedpointer_impl.h          |  4 ++--
 .../tools/qsharedpointer/tst_qsharedpointer.cpp  | 16 ++++++++++++++++
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/corelib/tools/qsharedpointer_impl.h b/src/corelib/tools/qsharedpointer_impl.h
index 7927a6dd2b9..a9c6babbf58 100644
--- a/src/corelib/tools/qsharedpointer_impl.h
+++ b/src/corelib/tools/qsharedpointer_impl.h
@@ -569,7 +569,7 @@ public:
     template <class X, IfCompatible<X> = true>
     Q_NODISCARD_CTOR
     QWeakPointer(QWeakPointer<X> &&other) noexcept
-        : d(other.d), value(other.value)
+        : d(other.d), value(other.toStrongRef().get()) // must go through QSharedPointer, see below
     {
         other.d = nullptr;
         other.value = nullptr;
@@ -677,7 +677,7 @@ private:
 
     template <class X>
     inline QWeakPointer &assign(X *ptr)
-    { return *this = QWeakPointer<X>(ptr, true); }
+    { return *this = QWeakPointer<T>(ptr, true); }
 
 #ifndef QT_NO_QOBJECT
     template <class X, IfCompatible<X> = true>
diff --git a/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp b/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp
index 4b1354dc419..5c4514eeb49 100644
--- a/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp
+++ b/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp
@@ -1275,6 +1275,22 @@ void tst_QSharedPointer::virtualBaseDifferentPointers()
         QVERIFY(baseptr == aBase);
     }
     safetyCheck();
+    {
+        VirtualDerived *aData = new VirtualDerived;
+
+        QSharedPointer<VirtualDerived> ptr = QSharedPointer<VirtualDerived>(aData);
+        QWeakPointer<VirtualDerived> wptr = ptr;
+
+        ptr.reset();
+        QVERIFY(wptr.toStrongRef().isNull());
+
+        QWeakPointer<Data> wptr2 = wptr;
+        QVERIFY(wptr2.toStrongRef().isNull());
+
+        QWeakPointer<Data> wptr3 = std::move(wptr);
+        QVERIFY(wptr3.toStrongRef().isNull());
+    }
+    safetyCheck();
 }
 
 #ifndef QTEST_NO_RTTI