summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThiago Macieira <thiago.macieira@intel.com>2021-11-17 14:08:11 -0800
committerThiago Macieira <thiago.macieira@intel.com>2021-11-27 21:38:22 -0800
commit0e72a846d379ba02ff80ecac2526640a05b872b6 (patch)
tree1b64de80427755d2c2a335558e55d549949e740c
parentb41356658e522f8fbc1061c4f4c76f0ba17b6acd (diff)
QObject: Q_ASSERT the object type before calling a PMF
The old-syle signal-slot syntax had the advantage of not delivering signals to slots in derived classes after that derived class's destructor had finished running (because we called via the virtual qt_metacall). The new syntax made no checks, so a conversion from the old to the new syntax may introduce crashes or other data corruptions at runtime if the destructor had completed. This commit introduces a Q_ASSERT to print the class name that the object is not any more. Since this is in inline code, this should get enabled for users' debug modes and does not therefore depend on Qt being built in debug mode. It required some Private classes to be adapted to the new form, by exposing the public q_func() in the public: part. Pick-to: 6.2 Fixes: QTBUG-33908 Change-Id: Iccb47e5527544b6fbd75fffd16b874cdc08c1f3e Reviewed-by: Marc Mutz <marc.mutz@qt.io> Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org> Reviewed-by: Giuseppe D'Angelo <giuseppe.dangelo@kdab.com>
-rw-r--r--src/corelib/kernel/qobject_p.h8
-rw-r--r--src/corelib/kernel/qobjectdefs_impl.h33
-rw-r--r--src/network/socket/qlocalsocket_p.h2
-rw-r--r--src/widgets/widgets/qwidgettextcontrol_p_p.h3
-rw-r--r--tests/auto/corelib/kernel/qobject/tst_qobject.cpp71
5 files changed, 108 insertions, 9 deletions
diff --git a/src/corelib/kernel/qobject_p.h b/src/corelib/kernel/qobject_p.h
index da171f5c7f..ac77d898da 100644
--- a/src/corelib/kernel/qobject_p.h
+++ b/src/corelib/kernel/qobject_p.h
@@ -98,9 +98,9 @@ public:
class Q_CORE_EXPORT QObjectPrivate : public QObjectData
{
+public:
Q_DECLARE_PUBLIC(QObject)
-public:
struct ExtraData
{
ExtraData(QObjectPrivate *ptr) : parent(ptr) { }
@@ -492,6 +492,12 @@ using FunctionStorage = typename std::conditional_t<
FunctionStorageByValue<Func>
>;
+template <typename ObjPrivate> inline void assertObjectType(QObjectPrivate *d)
+{
+ using Obj = std::remove_pointer_t<decltype(std::declval<ObjPrivate *>()->q_func())>;
+ assertObjectType<Obj>(d->q_ptr);
+}
+
template<typename Func, typename Args, typename R>
class QPrivateSlotObject : public QSlotObjectBase, private FunctionStorage<Func>
{
diff --git a/src/corelib/kernel/qobjectdefs_impl.h b/src/corelib/kernel/qobjectdefs_impl.h
index a3068e1712..da9d0f6390 100644
--- a/src/corelib/kernel/qobjectdefs_impl.h
+++ b/src/corelib/kernel/qobjectdefs_impl.h
@@ -50,6 +50,7 @@
QT_BEGIN_NAMESPACE
class QObject;
+class QObjectPrivate;
namespace QtPrivate {
template <typename T> struct RemoveRef { typedef T Type; };
@@ -139,6 +140,22 @@ namespace QtPrivate {
template<typename Func> struct FunctionPointer { enum {ArgumentCount = -1, IsPointerToMemberFunction = false}; };
+ template<typename ObjPrivate> inline void assertObjectType(QObjectPrivate *d);
+ template<typename Obj> inline void assertObjectType(QObject *o)
+ {
+ // ensure all three compile
+ [[maybe_unused]] auto staticcast = [](QObject *obj) { return static_cast<Obj *>(obj); };
+ [[maybe_unused]] auto qobjcast = [](QObject *obj) { return Obj::staticMetaObject.cast(obj); };
+#ifdef __cpp_rtti
+ [[maybe_unused]] auto dyncast = [](QObject *obj) { return dynamic_cast<Obj *>(obj); };
+ auto cast = dyncast;
+#else
+ auto cast = qobjcast;
+#endif
+ Q_ASSERT_X(cast(o), Obj::staticMetaObject.className(),
+ "Called object is not of the correct type (class destructor may have already run)");
+ }
+
template <typename, typename, typename, typename> struct FunctorCall;
template <int... II, typename... SignalArgs, typename R, typename Function>
struct FunctorCall<IndexesList<II...>, List<SignalArgs...>, R, Function> {
@@ -148,25 +165,33 @@ namespace QtPrivate {
};
template <int... II, typename... SignalArgs, typename R, typename... SlotArgs, typename SlotRet, class Obj>
struct FunctorCall<IndexesList<II...>, List<SignalArgs...>, R, SlotRet (Obj::*)(SlotArgs...)> {
- static void call(SlotRet (Obj::*f)(SlotArgs...), Obj *o, void **arg) {
+ static void call(SlotRet (Obj::*f)(SlotArgs...), Obj *o, void **arg)
+ {
+ assertObjectType<Obj>(o);
(o->*f)((*reinterpret_cast<typename RemoveRef<SignalArgs>::Type *>(arg[II+1]))...), ApplyReturnValue<R>(arg[0]);
}
};
template <int... II, typename... SignalArgs, typename R, typename... SlotArgs, typename SlotRet, class Obj>
struct FunctorCall<IndexesList<II...>, List<SignalArgs...>, R, SlotRet (Obj::*)(SlotArgs...) const> {
- static void call(SlotRet (Obj::*f)(SlotArgs...) const, Obj *o, void **arg) {
+ static void call(SlotRet (Obj::*f)(SlotArgs...) const, Obj *o, void **arg)
+ {
+ assertObjectType<Obj>(o);
(o->*f)((*reinterpret_cast<typename RemoveRef<SignalArgs>::Type *>(arg[II+1]))...), ApplyReturnValue<R>(arg[0]);
}
};
template <int... II, typename... SignalArgs, typename R, typename... SlotArgs, typename SlotRet, class Obj>
struct FunctorCall<IndexesList<II...>, List<SignalArgs...>, R, SlotRet (Obj::*)(SlotArgs...) noexcept> {
- static void call(SlotRet (Obj::*f)(SlotArgs...) noexcept, Obj *o, void **arg) {
+ static void call(SlotRet (Obj::*f)(SlotArgs...) noexcept, Obj *o, void **arg)
+ {
+ assertObjectType<Obj>(o);
(o->*f)((*reinterpret_cast<typename RemoveRef<SignalArgs>::Type *>(arg[II+1]))...), ApplyReturnValue<R>(arg[0]);
}
};
template <int... II, typename... SignalArgs, typename R, typename... SlotArgs, typename SlotRet, class Obj>
struct FunctorCall<IndexesList<II...>, List<SignalArgs...>, R, SlotRet (Obj::*)(SlotArgs...) const noexcept> {
- static void call(SlotRet (Obj::*f)(SlotArgs...) const noexcept, Obj *o, void **arg) {
+ static void call(SlotRet (Obj::*f)(SlotArgs...) const noexcept, Obj *o, void **arg)
+ {
+ assertObjectType<Obj>(o);
(o->*f)((*reinterpret_cast<typename RemoveRef<SignalArgs>::Type *>(arg[II+1]))...), ApplyReturnValue<R>(arg[0]);
}
};
diff --git a/src/network/socket/qlocalsocket_p.h b/src/network/socket/qlocalsocket_p.h
index bb1a808911..96ec6931b4 100644
--- a/src/network/socket/qlocalsocket_p.h
+++ b/src/network/socket/qlocalsocket_p.h
@@ -116,9 +116,9 @@ public:
class QLocalSocketPrivate : public QIODevicePrivate
{
+public:
Q_DECLARE_PUBLIC(QLocalSocket)
-public:
QLocalSocketPrivate();
void init();
diff --git a/src/widgets/widgets/qwidgettextcontrol_p_p.h b/src/widgets/widgets/qwidgettextcontrol_p_p.h
index cc4f867e8e..a7a19e748d 100644
--- a/src/widgets/widgets/qwidgettextcontrol_p_p.h
+++ b/src/widgets/widgets/qwidgettextcontrol_p_p.h
@@ -73,8 +73,9 @@ class QAbstractScrollArea;
class QWidgetTextControlPrivate : public QObjectPrivate
{
- Q_DECLARE_PUBLIC(QWidgetTextControl)
public:
+ Q_DECLARE_PUBLIC(QWidgetTextControl)
+
QWidgetTextControlPrivate();
bool cursorMoveKeyEvent(QKeyEvent *e);
diff --git a/tests/auto/corelib/kernel/qobject/tst_qobject.cpp b/tests/auto/corelib/kernel/qobject/tst_qobject.cpp
index 9c27824e36..2e34099e5b 100644
--- a/tests/auto/corelib/kernel/qobject/tst_qobject.cpp
+++ b/tests/auto/corelib/kernel/qobject/tst_qobject.cpp
@@ -1,7 +1,8 @@
/****************************************************************************
**
** Copyright (C) 2021 The Qt Company Ltd.
-** Copyright (C) 2015 Olivier Goffart <ogoffart@woboq.com>
+** Copyright (C) 2020 Olivier Goffart <ogoffart@woboq.com>
+** Copyright (C) 2021 Intel Corporation.
** Contact: https://www.qt.io/licensing/
**
** This file is part of the test suite of the Qt Toolkit.
@@ -169,6 +170,7 @@ private slots:
void disconnectDisconnects();
void singleShotConnection();
void objectNameBinding();
+ void emitToDestroyedClass();
};
struct QObjectCreatedOnShutdown
@@ -5946,8 +5948,8 @@ public:
};
class ConnectToPrivateSlotPrivate : public QObjectPrivate {
- Q_DECLARE_PUBLIC(ConnectToPrivateSlot)
public:
+ Q_DECLARE_PUBLIC(ConnectToPrivateSlot)
int receivedCount;
QVariant receivedValue;
@@ -8140,6 +8142,71 @@ void tst_QObject::objectNameBinding()
"objectName");
}
+namespace EmitToDestroyedClass {
+static int assertionCallCount = 0;
+static int wouldHaveAssertedCount = 0;
+struct WouldAssert : std::exception {};
+class Base : public QObject
+{
+ Q_OBJECT
+public:
+ ~Base()
+ {
+ try {
+ emit theSignal();
+ } catch (const WouldAssert &) {
+ ++wouldHaveAssertedCount;
+ }
+ }
+
+signals:
+ void theSignal();
+};
+
+class Derived : public Base
+{
+ Q_OBJECT
+public:
+ ~Derived() { }
+
+public slots:
+ void doNothing() {}
+};
+} // namespace EmitToDestroyedClass
+
+QT_BEGIN_NAMESPACE
+namespace QtPrivate {
+template<> void assertObjectType<EmitToDestroyedClass::Derived>(QObject *o)
+{
+ // override the assertion so we don't assert and so something does happen
+ // when assertions are disabled. By throwing, we also prevent the UB from
+ // happening.
+ using namespace EmitToDestroyedClass;
+ ++assertionCallCount;
+ if (!qobject_cast<Derived *>(o))
+ throw WouldAssert();
+}
+}
+QT_END_NAMESPACE
+
+void tst_QObject::emitToDestroyedClass()
+{
+ using namespace EmitToDestroyedClass;
+ std::unique_ptr ptr = std::make_unique<Derived>();
+ QObject::connect(ptr.get(), &Base::theSignal, ptr.get(), &Derived::doNothing);
+ QCOMPARE(assertionCallCount, 0);
+ QCOMPARE(wouldHaveAssertedCount, 0);
+
+ // confirm our replacement function did get called
+ emit ptr->theSignal();
+ QCOMPARE(assertionCallCount, 1);
+ QCOMPARE(wouldHaveAssertedCount, 0);
+
+ ptr.reset();
+ QCOMPARE(assertionCallCount, 2);
+ QCOMPARE(wouldHaveAssertedCount, 1);
+}
+
// Test for QtPrivate::HasQ_OBJECT_Macro
static_assert(QtPrivate::HasQ_OBJECT_Macro<tst_QObject>::Value);
static_assert(!QtPrivate::HasQ_OBJECT_Macro<SiblingDeleter>::Value);