Use correct index for QLocale::system()'s static

Nothing prevents client code from calling QLocale::setDefault() before
we ever instantiate QLocale::system() - aside from some quirks that
mean setDefault(), currently, does instantiate QLocale::system() to
force initialization of defaultLocalePrivate - so using defaultIndex()
could set the system QLocalePrivate instance's index incorrectly.

In any case, even if the index is initially set correctly, a
subsequent change to the system locale would change the correct index;
and nothing outside QLocale::system() has access to the instance that
would then be remembering an out-of-date index.

Actually tripping over that inconsistency took some deviousness, but
was possible. The index is (currently) only used for month name
lookups and those special-case, for the Roman-derived calendars, the
system locale, to only use the index if the system locale offers no
name for a month. Meanwhile, updateSystemPrivate() uses the fallback
locale's index for its look-up of which CLDR data to copy into the
fallback QLocaleData for the system locale.

None the less, a non-Roman calendar's lookup will go via the index to
get at the CLDR data for that calendar, thereby exposing the system
locale's index to use; and, sure enough, a setDefault() could lead
that to produce wrong answers.

In QLocale::system() there's a cached QLocalePrivate, whose index we
need to ensure stays in sync with the active system locale. So pass
its &m_index to systemData(), which will now (when passed this) ensure
it's up to date. Since we always have called systemData(), to ensure
it is up to date, we can skip that update in the initialization of the
cached private and use m_index = -1 to let systemData() know when it's
in the initial call, thereby making the static cache constinit.

Amended a test to what proved the issue was present.

Change-Id: I8d7ab5830cf0bbb9265c2af2a1edc9396ddef79f
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>

4 files changed, 77 insertions, 39 deletions

diff --git a/src/corelib/text/qlocale.cpp b/src/corelib/text/qlocale.cpp
index d15bdd93cd..c7ecddb484 100644
--- a/src/corelib/text/qlocale.cpp
+++ b/src/corelib/text/qlocale.cpp
@@ -788,25 +788,42 @@ static void updateSystemPrivate()
 }
 #endif // !QT_NO_SYSTEMLOCALE
 
-static const QLocaleData *systemData()
+static const QLocaleData *systemData(qsizetype *sysIndex = nullptr)
 {
 #ifndef QT_NO_SYSTEMLOCALE
     /*
       Copy over the information from the fallback locale and modify.
 
-      This modifies (cross-thread) global state, so take care to only call it in
-      one thread.
+      If sysIndex is passed, it should be the m_index of the system locale's
+      QLocalePrivate, which we'll update if it needs it.
+
+      This modifies (cross-thread) global state, so is mutex-protected.
     */
     {
+        Q_CONSTINIT static QLocaleId sysId;
+        bool updated = false;
+
         Q_CONSTINIT static QBasicMutex systemDataMutex;
         systemDataMutex.lock();
-        if (systemLocaleData.m_language_id == 0)
+        if (systemLocaleData.m_language_id == 0) {
             updateSystemPrivate();
+            updated = true;
+        }
+        // Initialization of system private has *sysIndex == -1 to hit this.
+        if (sysIndex && (updated || *sysIndex < 0)) {
+            const QLocaleId nowId = systemLocaleData.id();
+            if (sysId != nowId || *sysIndex < 0) {
+                // This look-up may be expensive:
+                *sysIndex = QLocaleData::findLocaleIndex(nowId);
+                sysId = nowId;
+            }
+        }
         systemDataMutex.unlock();
     }
 
     return &systemLocaleData;
 #else
+    Q_UNUSED(sysIndex);
     return locale_data;
 #endif
 }
@@ -2749,8 +2766,19 @@ QString QLocale::toString(double f, char format, int precision) const
 
 QLocale QLocale::system()
 {
-    QT_PREPEND_NAMESPACE(systemData)(); // Ensure system data is up to date.
-    static QLocalePrivate locale(systemData(), defaultIndex(), DefaultNumberOptions, 1);
+    constexpr auto sysData = []() {
+        // Same return as systemData(), but leave the setup to the actual call to it.
+#ifdef QT_NO_SYSTEMLOCALE
+        return locale_data;
+#else
+        return &systemLocaleData;
+#endif
+    };
+    Q_CONSTINIT static QLocalePrivate locale(sysData(), -1, DefaultNumberOptions, 1);
+    // Calling systemData() ensures system data is up to date; we also need it
+    // to ensure that locale's index stays up to date:
+    systemData(&locale.m_index);
+    Q_ASSERT(locale.m_index >= 0 && locale.m_index < locale_data_size);
 
     return QLocale(locale);
 }
diff --git a/src/corelib/text/qlocale_p.h b/src/corelib/text/qlocale_p.h
index 6c1826c846..4dcfb0329b 100644
--- a/src/corelib/text/qlocale_p.h
+++ b/src/corelib/text/qlocale_p.h
@@ -532,7 +532,7 @@ public:
     // System locale has an m_data all its own; all others have m_data = locale_data + m_index
     const QLocaleData *const m_data;
     QBasicAtomicInt ref;
-    const qsizetype m_index;
+    qsizetype m_index; // System locale needs this updated when m_data->id() changes.
     QLocale::NumberOptions m_numberOptions;
 
     static QBasicAtomicInt s_generation;
diff --git a/tests/auto/corelib/text/qlocale/syslocaleapp/syslocaleapp.cpp b/tests/auto/corelib/text/qlocale/syslocaleapp/syslocaleapp.cpp
index 1194cf0895..7cfdf161ea 100644
--- a/tests/auto/corelib/text/qlocale/syslocaleapp/syslocaleapp.cpp
+++ b/tests/auto/corelib/text/qlocale/syslocaleapp/syslocaleapp.cpp
@@ -1,15 +1,21 @@
 // Copyright (C) 2016 The Qt Company Ltd.
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
 #include <QLocale>
+#include <QCalendar>
 #include <QCoreApplication>
 #include <QTextStream>
 
 int main(int argc, char** argv)
 {
     QCoreApplication app(argc, argv);
-    QLocale l;
+    // Setting a default locale should not mess up the system one.
+    QLocale::setDefault(QLocale::Persian);
+    QLocale l = QLocale::system();
+    // A non-Roman calendar will use CLDR data instead of system data, so needs
+    // to have got the right locale index to look that up.
+    QCalendar cal = QCalendar(QCalendar::System::Jalali);
     QTextStream str(stdout);
-    str << l.name();
+    str << l.name() << ' ' << cal.standaloneMonthName(l, 2);
 
     return 0;
 }
diff --git a/tests/auto/corelib/text/qlocale/tst_qlocale.cpp b/tests/auto/corelib/text/qlocale/tst_qlocale.cpp
index 912acfd42d..65d7d9ecbf 100644
--- a/tests/auto/corelib/text/qlocale/tst_qlocale.cpp
+++ b/tests/auto/corelib/text/qlocale/tst_qlocale.cpp
@@ -591,45 +591,49 @@ void tst_QLocale::systemLocale_data()
     // Note that the accepted values for fields are implementation-dependent;
     // the template is language[_territory][.codeset][@modifier]
 
+    // "Ordibehesht" is the name (as adapted to English, German or Norsk) of the
+    // second month of the Jalali calendar. If you see anything in Arabic,
+    // setDefault(Persian) has interfered with the system locale setup.
+
     // Vanilla:
-    ADD_CTOR_TEST("C", "C");
+    ADD_CTOR_TEST("C", "C Ordibehesht");
 
     // Standard forms:
-    ADD_CTOR_TEST("en", "en_US");
-    ADD_CTOR_TEST("en_GB", "en_GB");
-    ADD_CTOR_TEST("de", "de_DE");
+    ADD_CTOR_TEST("en", "en_US Ordibehesht");
+    ADD_CTOR_TEST("en_GB", "en_GB Ordibehesht");
+    ADD_CTOR_TEST("de", "de_DE Ordibehescht");
     // Norsk has some quirks:
-    ADD_CTOR_TEST("no", "nb_NO");
-    ADD_CTOR_TEST("nb", "nb_NO");
-    ADD_CTOR_TEST("nn", "nn_NO");
-    ADD_CTOR_TEST("no_NO", "nb_NO");
-    ADD_CTOR_TEST("nb_NO", "nb_NO");
-    ADD_CTOR_TEST("nn_NO", "nn_NO");
+    ADD_CTOR_TEST("no", "nb_NO ordibehesht");
+    ADD_CTOR_TEST("nb", "nb_NO ordibehesht");
+    ADD_CTOR_TEST("nn", "nn_NO ordibehesht");
+    ADD_CTOR_TEST("no_NO", "nb_NO ordibehesht");
+    ADD_CTOR_TEST("nb_NO", "nb_NO ordibehesht");
+    ADD_CTOR_TEST("nn_NO", "nn_NO ordibehesht");
 
     // Not too fussy about case:
-    ADD_CTOR_TEST("DE", "de_DE");
-    ADD_CTOR_TEST("EN", "en_US");
+    ADD_CTOR_TEST("DE", "de_DE Ordibehescht");
+    ADD_CTOR_TEST("EN", "en_US Ordibehesht");
 
     // Invalid fields
-    ADD_CTOR_TEST("bla", "C");
-    ADD_CTOR_TEST("zz", "C");
-    ADD_CTOR_TEST("zz_zz", "C");
-    ADD_CTOR_TEST("zz...", "C");
-    ADD_CTOR_TEST("en.bla", "en_US");
-    ADD_CTOR_TEST("en@bla", "en_US");
-    ADD_CTOR_TEST("en_blaaa", "en_US");
-    ADD_CTOR_TEST("en_zz", "en_US");
-    ADD_CTOR_TEST("en_GB.bla", "en_GB");
-    ADD_CTOR_TEST("en_GB@.bla", "en_GB");
-    ADD_CTOR_TEST("en_GB@bla", "en_GB");
+    ADD_CTOR_TEST("bla", "C Ordibehesht");
+    ADD_CTOR_TEST("zz", "C Ordibehesht");
+    ADD_CTOR_TEST("zz_zz", "C Ordibehesht");
+    ADD_CTOR_TEST("zz...", "C Ordibehesht");
+    ADD_CTOR_TEST("en.bla", "en_US Ordibehesht");
+    ADD_CTOR_TEST("en@bla", "en_US Ordibehesht");
+    ADD_CTOR_TEST("en_blaaa", "en_US Ordibehesht");
+    ADD_CTOR_TEST("en_zz", "en_US Ordibehesht");
+    ADD_CTOR_TEST("en_GB.bla", "en_GB Ordibehesht");
+    ADD_CTOR_TEST("en_GB@.bla", "en_GB Ordibehesht");
+    ADD_CTOR_TEST("en_GB@bla", "en_GB Ordibehesht");
 
     // Empty optional fields, but with punctuators supplied
-    ADD_CTOR_TEST("en.", "en_US");
-    ADD_CTOR_TEST("en@", "en_US");
-    ADD_CTOR_TEST("en.@", "en_US");
-    ADD_CTOR_TEST("en_", "en_US");
-    ADD_CTOR_TEST("en_.", "en_US");
-    ADD_CTOR_TEST("en_.@", "en_US");
+    ADD_CTOR_TEST("en.", "en_US Ordibehesht");
+    ADD_CTOR_TEST("en@", "en_US Ordibehesht");
+    ADD_CTOR_TEST("en.@", "en_US Ordibehesht");
+    ADD_CTOR_TEST("en_", "en_US Ordibehesht");
+    ADD_CTOR_TEST("en_.", "en_US Ordibehesht");
+    ADD_CTOR_TEST("en_.@", "en_US Ordibehesht");
 #undef ADD_CTOR_TEST
 
 #if QT_CONFIG(process) // for runSysApp
@@ -638,7 +642,7 @@ void tst_QLocale::systemLocale_data()
     QString errorMessage;
     if (runSysApp(m_sysapp, QStringList(), cleanEnv, &defaultLoc, &errorMessage)) {
 #if defined(Q_OS_MACOS)
-        QString localeForInvalidLocale = "C";
+        QString localeForInvalidLocale = "C Ordibehesht";
 #else
         QString localeForInvalidLocale = defaultLoc;
 #endif