From 9baf1d525625a60ce9fa6b9c0e6788d0d4d93e6e Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Fri, 19 Nov 2021 11:05:45 +0100 Subject: [PATCH] QAndroidActivityResultReceiver: make uniqueActivityRequestCode() lock- and UB-free The old code attempted to detect the overflow of the requestCode variable, but because it didn't do anything about it except warn, it still ran into the overflow, and, since the variable is signed, into UB. That means that a clever compiler will just eliminate the warning as dead code because it can backtrack and see that the condition guarding it is never true, because otherwise UB happens. Fix that problem by using a uint counter (unsigned overflow is defined to wrap) and only convert to int after the check. Also fix two inefficiencies with the old code: 1. We don't need a mutex just to up a counter in a thread-safe way. Upping a shared counter is the prototypical use-case for relaxed atomics, so use that. That also solves the problem of the non-POD static object at function scope (QMutex) that forced compilers to emit thread-safe static initialization code. 2. We had a static variable whose initial value isn't 0, which means it can't be in stored in the BSS, but only in the DATA segment. Do the trivial transformation of subtracting the offset so the variable starts at 0. The offset can be added afterwards. Pick-to: 6.2 Change-Id: I3560df21d6b4e4201cb6772237780cc8b400631d Reviewed-by: Fabian Kosmale --- src/corelib/platform/android/qandroidextras.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/corelib/platform/android/qandroidextras.cpp b/src/corelib/platform/android/qandroidextras.cpp index 1da8bcf265..d9bcc76a1a 100644 --- a/src/corelib/platform/android/qandroidextras.cpp +++ b/src/corelib/platform/android/qandroidextras.cpp @@ -478,18 +478,19 @@ QJniObject QAndroidServiceConnection::handle() const */ +static QBasicAtomicInteger nextUniqueActivityRequestCode = Q_BASIC_ATOMIC_INITIALIZER(0); // Get a unique activity request code. static int uniqueActivityRequestCode() { - static QMutex mutex; - static int requestCode = 0x1000; // Reserve all request codes under 0x1000 for Qt. + constexpr uint ReservedForQtOffset = 0x1000; // Reserve all request codes under 0x1000 for Qt - QMutexLocker locker(&mutex); - if (requestCode == INT_MAX) + const uint requestCodeBase = nextUniqueActivityRequestCode.fetchAndAddRelaxed(1); + if (requestCodeBase == uint(INT_MAX) - ReservedForQtOffset) qWarning("Unique activity request code has wrapped. Unexpected behavior may occur."); - return requestCode++; + const int requestCode = static_cast(requestCodeBase + ReservedForQtOffset); + return requestCode; } class QAndroidActivityResultReceiverPrivate: public QtAndroidPrivate::ActivityResultListener