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 <fabian.kosmale@qt.io>

1 files 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<uint> 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<int>(requestCodeBase + ReservedForQtOffset);
+    return requestCode;
 }
 
 class QAndroidActivityResultReceiverPrivate: public QtAndroidPrivate::ActivityResultListener