QArrayData: fix potential UB in QBasicAtomic initializationHEADdev

Until C++17 (inclusive), a default-constructed std::atomic object can,
officially, only be initialized with a call to std::atomic_init, for
which QBasicAtomic doesn't have API. It is even unclear whether
zero-initialization of static and thread-local objects will cause the
object to be initialized.

Seeing as we can't rely on malloc's zero-initialization to properly
initialize std::atomic objects (and therefore QBasicAtomic ones), and
because it's the right thing to do, from a [basic.life] POV, anyway,
port to placement new instead.

The realloc() feels fishy, seeing as it reallocs a struct that
contains an atomic variable (which we don't mark as Q_RELOCATABLE_TYPE
even for our own types), but assume that part it ok, and in-place
construct only if realloc() was passed nullptr (so is equivalen to
malloc()). A final solution for this can probably only be implemented
when we can depend on C++20's atomic_ref, which decouples the
underlying object from the atomic operations performed on it.

Rename the ref_ member to m_ref to catch any uses of the variable,
incl. since removed ones in older branches.

Amends 812a611dc05e5facd036856625ccb9274fdcb117 (which ported from
QtPrivate::RefCount to QBasicAtomicInt; I didn't check whether
QtPrivate::RefCount had a similar problem, because that commit is
already much older than what I can pick back to at this point).

Task-number: QTBUG-137465
Pick-to: 6.10 6.9 6.8 6.5
Change-Id: I188e05d09e20e0820af7cf1cbb651afa860bb9d6
Reviewed-by: Thiago Macieira <[email protected]>

4 files changed, 23 insertions, 20 deletions

diff --git a/src/corelib/tools/qarraydata.cpp b/src/corelib/tools/qarraydata.cpp
index 37d6dea35f9..54bd971cb36 100644
--- a/src/corelib/tools/qarraydata.cpp
+++ b/src/corelib/tools/qarraydata.cpp
@@ -181,13 +181,12 @@ allocateHelper(qsizetype objectSize, qsizetype alignment, qsizetype capacity,
         return {};
 
     void *data = nullptr;
-    QArrayData *header = static_cast<QArrayData *>(::malloc(size_t(allocSize)));
-    if (Q_LIKELY(header)) {
-        header->ref_.storeRelaxed(1);
-        header->flags = {};
+    void *mem = ::malloc(size_t(allocSize));
+    QArrayData *header = nullptr;
+    if (Q_LIKELY(mem)) {
+        header = new (mem) QArrayData{1, {}, capacity};
         // find where offset should point to so that data() is aligned to alignment bytes
         data = QTypedArrayData<void>::dataStart(header, alignment);
-        header->alloc = capacity;
     }
 
     return { data, header };
@@ -245,8 +244,12 @@ QArrayData::reallocateUnaligned(QArrayData *data, void *dataPointer,
     Q_ASSERT(offset > 0);
     Q_ASSERT(offset <= allocSize); // equals when all free space is at the beginning
 
-    QArrayData *header = static_cast<QArrayData *>(::realloc(data, size_t(allocSize)));
-    if (header) {
+    const bool hadData = data;
+    void *mem = ::realloc(data, size_t(allocSize));
+    QArrayData *header = static_cast<QArrayData *>(mem);
+    if (mem) {
+        if (!hadData)
+            header = new (mem) QArrayData{0, {}, {}};
         header->alloc = capacity;
         dataPointer = reinterpret_cast<char *>(header) + offset;
     } else {
diff --git a/src/corelib/tools/qarraydata.h b/src/corelib/tools/qarraydata.h
index 38d1091ac1f..71e183e646e 100644
--- a/src/corelib/tools/qarraydata.h
+++ b/src/corelib/tools/qarraydata.h
@@ -39,7 +39,7 @@ struct QArrayData
     };
     Q_DECLARE_FLAGS(ArrayOptions, ArrayOption)
 
-    QBasicAtomicInt ref_;
+    QBasicAtomicInt m_ref;
     ArrayOptions flags;
     qsizetype alloc;
 
@@ -56,19 +56,19 @@ struct QArrayData
     /// Returns true if sharing took place
     bool ref() noexcept
     {
-        ref_.ref();
+        m_ref.ref();
         return true;
     }
 
     /// Returns false if deallocation is necessary
     bool deref() noexcept
     {
-        return ref_.deref();
+        return m_ref.deref();
     }
 
     bool isShared() const noexcept
     {
-        return ref_.loadRelaxed() != 1;
+        return m_ref.loadRelaxed() != 1;
     }
 
     // Returns true if a detach is necessary before modifying the data
@@ -76,7 +76,7 @@ struct QArrayData
     // detaching is necessary, you should be in a non-const function already
     bool needsDetach() noexcept
     {
-        return ref_.loadRelaxed() > 1;
+        return m_ref.loadRelaxed() > 1;
     }
 
     qsizetype detachCapacity(qsizetype newSize) const noexcept
diff --git a/src/corelib/tools/qarraydataops.h b/src/corelib/tools/qarraydataops.h
index c20abd12c23..419585b0260 100644
--- a/src/corelib/tools/qarraydataops.h
+++ b/src/corelib/tools/qarraydataops.h
@@ -82,7 +82,7 @@ public:
     void destroyAll() noexcept // Call from destructors, ONLY!
     {
         Q_ASSERT(this->d);
-        Q_ASSERT(this->d->ref_.loadRelaxed() == 0);
+        Q_ASSERT(this->d->m_ref.loadRelaxed() == 0);
 
         // As this is to be called only from destructor, it doesn't need to be
         // exception safe; size not updated.
@@ -345,7 +345,7 @@ public:
         // As this is to be called only from destructor, it doesn't need to be
         // exception safe; size not updated.
 
-        Q_ASSERT(this->d->ref_.loadRelaxed() == 0);
+        Q_ASSERT(this->d->m_ref.loadRelaxed() == 0);
 
         std::destroy(this->begin(), this->end());
     }
diff --git a/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp b/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp
index 5804aeeb1e5..87d48d56e6b 100644
--- a/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp
+++ b/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp
@@ -73,22 +73,22 @@ void tst_QArrayData::referenceCounting()
         // Reference counting initialized to 1 (owned)
         QArrayData array = { Q_BASIC_ATOMIC_INITIALIZER(1), {}, 0 };
 
-        QCOMPARE(array.ref_.loadRelaxed(), 1);
+        QCOMPARE(array.m_ref.loadRelaxed(), 1);
 
         QVERIFY(array.ref());
-        QCOMPARE(array.ref_.loadRelaxed(), 2);
+        QCOMPARE(array.m_ref.loadRelaxed(), 2);
 
         QVERIFY(array.deref());
-        QCOMPARE(array.ref_.loadRelaxed(), 1);
+        QCOMPARE(array.m_ref.loadRelaxed(), 1);
 
         QVERIFY(array.ref());
-        QCOMPARE(array.ref_.loadRelaxed(), 2);
+        QCOMPARE(array.m_ref.loadRelaxed(), 2);
 
         QVERIFY(array.deref());
-        QCOMPARE(array.ref_.loadRelaxed(), 1);
+        QCOMPARE(array.m_ref.loadRelaxed(), 1);
 
         QVERIFY(!array.deref());
-        QCOMPARE(array.ref_.loadRelaxed(), 0);
+        QCOMPARE(array.m_ref.loadRelaxed(), 0);
 
         // Now would be a good time to free/release allocated data
     }