summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorIhor Dutchak <ihor.youw@gmail.com>2017-04-17 00:35:38 +0300
committerGiuseppe D'Angelo <giuseppe.dangelo@kdab.com>2017-04-17 09:56:15 +0000
commitd1210281e41008ce2e3510aa5cfb3ebea1c57734 (patch)
tree4af360e70b787129418ee8029638ad12166d5590
parent5662234afaf23d88e1f3fa4bee2a59b61bd0c267 (diff)
Fix undefined behavior in QSharedPointer::create()5.8
Initialize a deleter for a new object, created by QSharedPointer::create(), only after the object is actually constructed. [ChangeLog][QtCore][QSharedPointer] Fixed undefined behavior when creating an object with QSharedPointer::create() and its conscructor throws an exception. Task-number: QTBUG-49824 Change-Id: I07f77a78ff468d9b45b8ef133278e8cdd96a0647 Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>
-rw-r--r--src/corelib/tools/qsharedpointer_impl.h5
-rw-r--r--tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp50
2 files changed, 54 insertions, 1 deletions
diff --git a/src/corelib/tools/qsharedpointer_impl.h b/src/corelib/tools/qsharedpointer_impl.h
index 5738413bfb..373fc3a662 100644
--- a/src/corelib/tools/qsharedpointer_impl.h
+++ b/src/corelib/tools/qsharedpointer_impl.h
@@ -260,6 +260,7 @@ namespace QtSharedPointer {
internalSafetyCheckRemove(self);
deleter(self);
}
+ static void noDeleter(ExternalRefCountData *) { }
static inline ExternalRefCountData *create(T **ptr, DestroyerFn destroy)
{
@@ -433,11 +434,13 @@ public:
# else
typename Private::DestroyerFn destroy = &Private::deleter;
# endif
+ typename Private::DestroyerFn noDestroy = &Private::noDeleter;
QSharedPointer result(Qt::Uninitialized);
- result.d = Private::create(&result.value, destroy);
+ result.d = Private::create(&result.value, noDestroy);
// now initialize the data
new (result.data()) T(std::forward<Args>(arguments)...);
+ result.d->destroyer = destroy;
result.d->setQObjectShared(result.value, true);
# ifdef QT_SHAREDPOINTER_TRACK_POINTERS
internalSafetyCheckAdd(result.d, result.value);
diff --git a/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp b/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp
index d0a0feb125..7850478602 100644
--- a/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp
+++ b/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp
@@ -97,6 +97,8 @@ private slots:
void qvariantCast();
void sharedFromThis();
+ void constructorThrow();
+
void threadStressTest_data();
void threadStressTest();
void validConstructs();
@@ -2594,6 +2596,54 @@ void tst_QSharedPointer::sharedFromThis()
QCOMPARE(Data::destructorCounter, destructions + 6);
}
+#ifndef QT_NO_EXCEPTIONS
+class ThrowData: public Data
+{
+public:
+ static int childDestructorCounter;
+ static int childGenerationCounter;
+
+ ThrowData()
+ {
+ childGenerationCounter++;
+ throw QStringLiteral("Dummy exception");
+ }
+
+ ~ThrowData()
+ {
+ childDestructorCounter++;
+ }
+};
+int ThrowData::childDestructorCounter = 0;
+int ThrowData::childGenerationCounter = 0;
+#endif // !QT_NO_EXCEPTIONS
+
+void tst_QSharedPointer::constructorThrow()
+{
+#ifndef QT_NO_EXCEPTIONS
+ int generation = Data::generationCounter;
+ int destructorCounter = Data::destructorCounter;
+
+ int childGeneration = ThrowData::childGenerationCounter;
+ int childDestructorCounter = ThrowData::childDestructorCounter;
+
+ QSharedPointer<ThrowData> ptr;
+ QVERIFY_EXCEPTION_THROWN(ptr = QSharedPointer<ThrowData>::create(), QString);
+ QVERIFY(ptr.isNull());
+ QCOMPARE(ThrowData::childGenerationCounter, childGeneration + 1);
+ // destructor should never be called, if a constructor throws
+ // an exception
+ QCOMPARE(ThrowData::childDestructorCounter, childDestructorCounter);
+
+ QCOMPARE(Data::generationCounter, generation + 1);
+ // but base class constructor doesn't throw, so base class destructor
+ // should be called
+ QCOMPARE(Data::destructorCounter, destructorCounter + 1);
+#else
+ QSKIP("Needs exceptions");
+#endif // !QT_NO_EXCEPTIONS
+}
+
namespace ReentrancyWhileDestructing {
struct IB
{