summaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorAllan Sandfeld Jensen <allan.jensen@qt.io>2023-03-15 12:18:54 +0100
committerAllan Sandfeld Jensen <allan.jensen@qt.io>2023-03-17 00:41:31 +0100
commit0cab9b56e99712c6985eee5af72e53e1fb04113d (patch)
tree05a2832fb2fc94e4d706db9c74f2fada56f80f9e /src
parentf6d70650933c5c1e9b238d8aaa06419223908183 (diff)
Fix UBSAN warnings
Convert tagged pointer to an encapsulated tagged pointer type, and clean up where it is used. Change-Id: I9cf5b8d1dfa345eebaa185e44d762d5008769480 Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Diffstat (limited to 'src')
-rw-r--r--src/corelib/kernel/qobject.cpp18
-rw-r--r--src/corelib/kernel/qobject_p.h1
-rw-r--r--src/corelib/kernel/qobject_p_p.h49
3 files changed, 39 insertions, 29 deletions
diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp
index d3dead9d8f1..6b7a5dafcb4 100644
--- a/src/corelib/kernel/qobject.cpp
+++ b/src/corelib/kernel/qobject.cpp
@@ -369,16 +369,16 @@ void QObjectPrivate::ConnectionData::removeConnection(QObjectPrivate::Connection
c->prevConnectionList->nextConnectionList.storeRelaxed(n);
c->prevConnectionList = nullptr;
- Q_ASSERT(c != orphaned.loadRelaxed());
+ Q_ASSERT(c != static_cast<Connection *>(orphaned.load(std::memory_order_relaxed)));
// add c to orphanedConnections
- Connection *o = nullptr;
+ TaggedSignalVector o = nullptr;
/* No ABA issue here: When adding a node, we only care about the list head, it doesn't
* matter if the tail changes.
*/
+ o = orphaned.load(std::memory_order_acquire);
do {
- o = orphaned.loadRelaxed();
c->nextInOrphanList = o;
- } while (!orphaned.testAndSetRelease(o, c));
+ } while (!orphaned.compare_exchange_strong(o, TaggedSignalVector(c), std::memory_order_release));
#ifndef QT_NO_DEBUG
found = false;
@@ -396,7 +396,7 @@ void QObjectPrivate::ConnectionData::removeConnection(QObjectPrivate::Connection
void QObjectPrivate::ConnectionData::cleanOrphanedConnectionsImpl(QObject *sender, LockPolicy lockPolicy)
{
QBasicMutex *senderMutex = signalSlotLock(sender);
- ConnectionOrSignalVector *c = nullptr;
+ TaggedSignalVector c = nullptr;
{
std::unique_lock<QBasicMutex> lock(*senderMutex, std::defer_lock_t{});
if (lockPolicy == NeedToLock)
@@ -407,7 +407,7 @@ void QObjectPrivate::ConnectionData::cleanOrphanedConnectionsImpl(QObject *sende
// Since ref == 1, no activate() is in process since we locked the mutex. That implies,
// that nothing can reference the orphaned connection objects anymore and they can
// be safely deleted
- c = orphaned.fetchAndStoreRelaxed(nullptr);
+ c = orphaned.exchange(nullptr, std::memory_order_relaxed);
}
if (c) {
// Deleting c might run arbitrary user code, so we must not hold the lock
@@ -421,11 +421,11 @@ void QObjectPrivate::ConnectionData::cleanOrphanedConnectionsImpl(QObject *sende
}
}
-inline void QObjectPrivate::ConnectionData::deleteOrphaned(QObjectPrivate::ConnectionOrSignalVector *o)
+inline void QObjectPrivate::ConnectionData::deleteOrphaned(TaggedSignalVector o)
{
while (o) {
- QObjectPrivate::ConnectionOrSignalVector *next = nullptr;
- if (SignalVector *v = ConnectionOrSignalVector::asSignalVector(o)) {
+ TaggedSignalVector next = nullptr;
+ if (SignalVector *v = static_cast<SignalVector *>(o)) {
next = v->nextInOrphanList;
free(v);
} else {
diff --git a/src/corelib/kernel/qobject_p.h b/src/corelib/kernel/qobject_p.h
index 8d382c78ba7..f9bb93f9608 100644
--- a/src/corelib/kernel/qobject_p.h
+++ b/src/corelib/kernel/qobject_p.h
@@ -111,6 +111,7 @@ public:
struct ConnectionOrSignalVector;
struct SignalVector;
struct Sender;
+ struct TaggedSignalVector;
/*
This contains the all connections from and to an object.
diff --git a/src/corelib/kernel/qobject_p_p.h b/src/corelib/kernel/qobject_p_p.h
index d79ce7e2b93..a9bd290d37f 100644
--- a/src/corelib/kernel/qobject_p_p.h
+++ b/src/corelib/kernel/qobject_p_p.h
@@ -34,25 +34,35 @@ struct QObjectPrivate::ConnectionList
static_assert(std::is_trivially_destructible_v<QObjectPrivate::ConnectionList>);
Q_DECLARE_TYPEINFO(QObjectPrivate::ConnectionList, Q_RELOCATABLE_TYPE);
-struct QObjectPrivate::ConnectionOrSignalVector
+struct QObjectPrivate::TaggedSignalVector
{
- union {
- // linked list of orphaned connections that need cleaning up
- ConnectionOrSignalVector *nextInOrphanList;
- // linked list of connections connected to slots in this object
- Connection *next;
- };
+ quintptr c;
- static SignalVector *asSignalVector(ConnectionOrSignalVector *c)
+ TaggedSignalVector() = default;
+ TaggedSignalVector(std::nullptr_t) noexcept : c(0) {}
+ TaggedSignalVector(Connection *v) noexcept : c(reinterpret_cast<quintptr>(v)) { Q_ASSERT(v && (reinterpret_cast<quintptr>(v) & 0x1) == 0); }
+ TaggedSignalVector(SignalVector *v) noexcept : c(reinterpret_cast<quintptr>(v) | quintptr(1u)) { Q_ASSERT(v); }
+ explicit operator SignalVector *() const noexcept
{
- if (reinterpret_cast<quintptr>(c) & 1)
- return reinterpret_cast<SignalVector *>(reinterpret_cast<quintptr>(c) & ~quintptr(1u));
+ if (c & 0x1)
+ return reinterpret_cast<SignalVector *>(c & ~quintptr(1u));
return nullptr;
}
- static Connection *fromSignalVector(SignalVector *v)
+ explicit operator Connection *() const noexcept
{
- return reinterpret_cast<Connection *>(reinterpret_cast<quintptr>(v) | quintptr(1u));
+ return reinterpret_cast<Connection *>(c);
}
+ operator uintptr_t() const noexcept { return c; }
+};
+
+struct QObjectPrivate::ConnectionOrSignalVector
+{
+ union {
+ // linked list of orphaned connections that need cleaning up
+ TaggedSignalVector nextInOrphanList;
+ // linked list of connections connected to slots in this object
+ Connection *next;
+ };
};
static_assert(std::is_trivial_v<QObjectPrivate::ConnectionOrSignalVector>);
@@ -132,12 +142,12 @@ struct QObjectPrivate::ConnectionData
QAtomicPointer<SignalVector> signalVector;
Connection *senders = nullptr;
Sender *currentSender = nullptr; // object currently activating the object
- QAtomicPointer<Connection> orphaned;
+ std::atomic<TaggedSignalVector> orphaned = {};
~ConnectionData()
{
Q_ASSERT(ref.loadRelaxed() == 0);
- auto *c = orphaned.fetchAndStoreRelaxed(nullptr);
+ TaggedSignalVector c = orphaned.exchange(nullptr, std::memory_order_relaxed);
if (c)
deleteOrphaned(c);
SignalVector *v = signalVector.loadRelaxed();
@@ -159,7 +169,7 @@ struct QObjectPrivate::ConnectionData
};
void cleanOrphanedConnections(QObject *sender, LockPolicy lockPolicy = NeedToLock)
{
- if (orphaned.loadRelaxed() && ref.loadAcquire() == 1)
+ if (orphaned.load(std::memory_order_relaxed) && ref.loadAcquire() == 1)
cleanOrphanedConnectionsImpl(sender, lockPolicy);
}
void cleanOrphanedConnectionsImpl(QObject *sender, LockPolicy lockPolicy);
@@ -194,15 +204,14 @@ struct QObjectPrivate::ConnectionData
signalVector.storeRelaxed(newVector);
if (vector) {
- Connection *o = nullptr;
+ TaggedSignalVector o = nullptr;
/* No ABA issue here: When adding a node, we only care about the list head, it doesn't
* matter if the tail changes.
*/
+ o = orphaned.load(std::memory_order_acquire);
do {
- o = orphaned.loadRelaxed();
vector->nextInOrphanList = o;
- } while (!orphaned.testAndSetRelease(
- o, ConnectionOrSignalVector::fromSignalVector(vector)));
+ } while (!orphaned.compare_exchange_strong(o, TaggedSignalVector(vector), std::memory_order_release));
}
}
int signalVectorCount() const
@@ -210,7 +219,7 @@ struct QObjectPrivate::ConnectionData
return signalVector.loadAcquire() ? signalVector.loadRelaxed()->count() : -1;
}
- static void deleteOrphaned(ConnectionOrSignalVector *c);
+ static void deleteOrphaned(TaggedSignalVector o);
};
struct QObjectPrivate::Sender
generated by cgit v1.2.3 (git 2.25.1) at 2025-12-18 10:30:09 +0000