summaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorMarc Mutz <[email protected]>2025-06-06 16:40:48 +0200
committerMarc Mutz <[email protected]>2025-07-04 01:10:09 +0200
commit3f61f736266ece40d627dcf6214618a22a009fd1 (patch)
treeb48fa2420b0acd9966249cc28890e84dffdfc0a4 /src
parent22effeae2ca4565ad537ac95f638754a92afcf72 (diff)
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]>
Diffstat (limited to 'src')
-rw-r--r--src/corelib/tools/qarraydata.cpp17
-rw-r--r--src/corelib/tools/qarraydata.h10
-rw-r--r--src/corelib/tools/qarraydataops.h4
3 files changed, 17 insertions, 14 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());
}