QmlCompiler: Consider lengths to be qsizetype

This is what we get as "source material" from all the containers. In
JavaScript, arrays sizes are up to 32bit unsigned, but we need to
represent -1 in a few places. Therefore, we cannot clamp the result to
32bit signed if the underlying container supports 64bit indices.

We can change qjslist.h here because this header is only supposed to be
used from generated code. Therefore, if you rebuild any users of this
code, you will also re-generate them, adapting to the new API.

Since we check for QJSNumberCoercion::isArrayIndex() before we use any
number as an array index when generating code, this is actually safe. We
just need to adapt isArrayIndex() to consider numbers greater than
INT_MAX as possible values.

Fixes: QTBUG-122582
Change-Id: I1147978a05bfedb63037c7f4437921f85a6aabb1
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

7 files changed, 101 insertions, 75 deletions

diff --git a/src/qml/common/qjsnumbercoercion.cpp b/src/qml/common/qjsnumbercoercion.cpp
index ba76c12bb0..8cd96a4e25 100644
--- a/src/qml/common/qjsnumbercoercion.cpp
+++ b/src/qml/common/qjsnumbercoercion.cpp
@@ -24,10 +24,26 @@ QT_BEGIN_NAMESPACE
   \internal
 
   Checks whether \a d contains a value that can serve as an index into an array.
-  For that, \a d must be a non-negative value representable as an int.
+  For that, \a d must be a non-negative value representable as an unsigned 32bit int.
  */
 
 /*!
+  \fn bool QJSNumberCoercion::isArrayIndex(qint64 i)
+  \internal
+
+  Checks whether \a i contains a value that can serve as an index into an array.
+  For that, \a d must be a non-negative value representable as an unsigned 32bit int.
+*/
+
+/*!
+  \fn bool QJSNumberCoercion::isArrayIndex(quint64 i)
+  \internal
+
+  Checks whether \a i contains a value that can serve as an index into an array.
+  For that, \a d must be a value representable as an unsigned 32bit int.
+*/
+
+/*!
   \fn int QJSNumberCoercion::toInteger(double d)
   \internal
 
diff --git a/src/qml/common/qjsnumbercoercion.h b/src/qml/common/qjsnumbercoercion.h
index 569976454f..0023bff6e8 100644
--- a/src/qml/common/qjsnumbercoercion.h
+++ b/src/qml/common/qjsnumbercoercion.h
@@ -27,11 +27,20 @@ public:
 
     static constexpr bool isArrayIndex(double d)
     {
-        if (d < 0 || !equals(d, d) || d > (std::numeric_limits<int>::max)()) {
-            return false;
-        }
+        return d >= 0
+                && equals(d, d)
+                && d <= (std::numeric_limits<uint>::max)()
+                && equals(static_cast<uint>(d), d);
+    }
 
-        return equals(static_cast<int>(d), d);
+    static constexpr bool isArrayIndex(qint64 i)
+    {
+        return i >= 0 && i <= (std::numeric_limits<uint>::max)();
+    }
+
+    static constexpr bool isArrayIndex(quint64 i)
+    {
+        return i <= (std::numeric_limits<uint>::max)();
     }
 
     static constexpr int toInteger(double d) {
diff --git a/src/qml/jsapi/qjslist.h b/src/qml/jsapi/qjslist.h
index f55d669a41..d604e266f2 100644
--- a/src/qml/jsapi/qjslist.h
+++ b/src/qml/jsapi/qjslist.h
@@ -28,7 +28,7 @@ QT_BEGIN_NAMESPACE
 
 struct QJSListIndexClamp
 {
-    static qsizetype clamp(int start, qsizetype max, qsizetype min = 0)
+    static qsizetype clamp(qsizetype start, qsizetype max, qsizetype min = 0)
     {
         Q_ASSERT(min >= 0);
         Q_ASSERT(min <= max);
@@ -43,18 +43,15 @@ struct QJSList : private QJSListIndexClamp
 
     QJSList(List *list, QJSEngine *engine) : m_list(list), m_engine(engine) {}
 
-    Value at(int index) const
+    Value at(qsizetype index) const
     {
         Q_ASSERT(index >= 0 && index < size());
         return *(m_list->cbegin() + index);
     }
 
-    int size() const
-    {
-        return int(std::min(m_list->size(), qsizetype(std::numeric_limits<int>::max())));
-    }
+    qsizetype size() const { return m_list->size(); }
 
-    void resize(int size)
+    void resize(qsizetype size)
     {
         m_list->resize(size);
     }
@@ -64,7 +61,7 @@ struct QJSList : private QJSListIndexClamp
         return std::find(m_list->cbegin(), m_list->cend(), value) != m_list->cend();
     }
 
-    bool includes(const Value &value, int start) const
+    bool includes(const Value &value, qsizetype start) const
     {
         return std::find(m_list->cbegin() + clamp(start, m_list->size()), m_list->cend(), value)
                 != m_list->cend();
@@ -88,14 +85,14 @@ struct QJSList : private QJSListIndexClamp
     {
         return *m_list;
     }
-    List slice(int start) const
+    List slice(qsizetype start) const
     {
         List result;
         std::copy(m_list->cbegin() + clamp(start, m_list->size()), m_list->cend(),
                   std::back_inserter(result));
         return result;
     }
-    List slice(int start, int end) const
+    List slice(qsizetype start, qsizetype end) const
     {
         const qsizetype size = m_list->size();
         const qsizetype clampedStart = clamp(start, size);
@@ -107,7 +104,7 @@ struct QJSList : private QJSListIndexClamp
         return result;
     }
 
-    int indexOf(const Value &value) const
+    qsizetype indexOf(const Value &value) const
     {
         const auto begin = m_list->cbegin();
         const auto end = m_list->cend();
@@ -116,9 +113,9 @@ struct QJSList : private QJSListIndexClamp
             return -1;
         const qsizetype result = it - begin;
         Q_ASSERT(result >= 0);
-        return result > std::numeric_limits<int>::max() ? -1 : int(result);
+        return result;
     }
-    int indexOf(const Value &value, int start) const
+    qsizetype indexOf(const Value &value, qsizetype start) const
     {
         const auto begin = m_list->cbegin();
         const auto end = m_list->cend();
@@ -127,18 +124,17 @@ struct QJSList : private QJSListIndexClamp
             return -1;
         const qsizetype result = it - begin;
         Q_ASSERT(result >= 0);
-        return result > std::numeric_limits<int>::max() ? -1 : int(result);
+        return result;
     }
 
-    int lastIndexOf(const Value &value) const
+    qsizetype lastIndexOf(const Value &value) const
     {
         const auto begin = std::make_reverse_iterator(m_list->cend());
         const auto end = std::make_reverse_iterator(m_list->cbegin());
         const auto it = std::find(begin, end, value);
-        const qsizetype result = (end - it) - 1;
-        return result > std::numeric_limits<int>::max() ? -1 : int(result);
+        return (end - it) - 1;
     }
-    int lastIndexOf(const Value &value, int start) const
+    qsizetype lastIndexOf(const Value &value, qsizetype start) const
     {
         const qsizetype size = m_list->size();
         if (size == 0)
@@ -150,8 +146,7 @@ struct QJSList : private QJSListIndexClamp
 
         const auto end = std::make_reverse_iterator(m_list->cbegin());
         const auto it = std::find(begin, end, value);
-        const qsizetype result = (end - it) - 1;
-        return result > std::numeric_limits<int>::max() ? -1 : int(result);
+        return (end - it) - 1;
     }
 
     QString toString() const { return join(); }
@@ -168,18 +163,18 @@ struct QJSList<QQmlListProperty<QObject>, QObject *>  : private QJSListIndexClam
 
     QJSList(QQmlListProperty<QObject> *list, QJSEngine *engine) : m_list(list), m_engine(engine) {}
 
-    QObject *at(int index) const
+    QObject *at(qsizetype index) const
     {
         Q_ASSERT(index >= 0 && index < size());
         return m_list->at(m_list, index);
     }
 
-    int size() const
+    qsizetype size() const
     {
-        return int(std::min(m_list->count(m_list), qsizetype(std::numeric_limits<int>::max())));
+        return m_list->count(m_list);
     }
 
-    void resize(int size)
+    void resize(qsizetype size)
     {
         qsizetype current = m_list->count(m_list);
         if (current < size && m_list->append) {
@@ -206,7 +201,7 @@ struct QJSList<QQmlListProperty<QObject>, QObject *>  : private QJSListIndexClam
 
         return false;
     }
-    bool includes(const QObject *value, int start) const
+    bool includes(const QObject *value, qsizetype start) const
     {
         if (!m_list->count || !m_list->at)
             return false;
@@ -239,7 +234,7 @@ struct QJSList<QQmlListProperty<QObject>, QObject *>  : private QJSListIndexClam
     {
         return m_list->toList<QObjectList>();
     }
-    QObjectList slice(int start) const
+    QObjectList slice(qsizetype start) const
     {
         if (!m_list->count || !m_list->at)
             return QObjectList();
@@ -252,7 +247,7 @@ struct QJSList<QQmlListProperty<QObject>, QObject *>  : private QJSListIndexClam
             result.append(m_list->at(m_list, i));
         return result;
     }
-    QObjectList slice(int start, int end) const
+    QObjectList slice(qsizetype start, qsizetype end) const
     {
         if (!m_list->count || !m_list->at)
             return QObjectList();
@@ -267,46 +262,43 @@ struct QJSList<QQmlListProperty<QObject>, QObject *>  : private QJSListIndexClam
         return result;
     }
 
-    int indexOf(const QObject *value) const
+    qsizetype indexOf(const QObject *value) const
     {
         if (!m_list->count || !m_list->at)
             return -1;
 
-        const qsizetype end
-                = std::min(m_list->count(m_list), qsizetype(std::numeric_limits<int>::max()));
+        const qsizetype end = m_list->count(m_list);
         for (qsizetype i = 0; i < end; ++i) {
             if (m_list->at(m_list, i) == value)
-                return int(i);
+                return i;
         }
         return -1;
     }
-    int indexOf(const QObject *value, int start) const
+    qsizetype indexOf(const QObject *value, qsizetype start) const
     {
         if (!m_list->count || !m_list->at)
             return -1;
 
         const qsizetype size = m_list->count(m_list);
-        for (qsizetype i = clamp(start, size),
-             end = std::min(size, qsizetype(std::numeric_limits<int>::max()));
-             i < end; ++i) {
+        for (qsizetype i = clamp(start, size); i < size; ++i) {
             if (m_list->at(m_list, i) == value)
-                return int(i);
+                return i;
         }
         return -1;
     }
 
-    int lastIndexOf(const QObject *value) const
+    qsizetype lastIndexOf(const QObject *value) const
     {
         if (!m_list->count || !m_list->at)
             return -1;
 
         for (qsizetype i = m_list->count(m_list) - 1; i >= 0; --i) {
             if (m_list->at(m_list, i) == value)
-                return i > std::numeric_limits<int>::max() ? -1 : int(i);
+                return i;
         }
         return -1;
     }
-    int lastIndexOf(const QObject *value, int start) const
+    qsizetype lastIndexOf(const QObject *value, qsizetype start) const
     {
         if (!m_list->count || !m_list->at)
             return -1;
@@ -318,7 +310,7 @@ struct QJSList<QQmlListProperty<QObject>, QObject *>  : private QJSListIndexClam
         qsizetype clampedStart = std::min(clamp(start, size), size - 1);
         for (qsizetype i = clampedStart; i >= 0; --i) {
             if (m_list->at(m_list, i) == value)
-                return i > std::numeric_limits<int>::max() ? -1 : int(i);
+                return i;
         }
         return -1;
     }
@@ -342,11 +334,11 @@ public:
     }
 
     bool hasNext() const { return m_index < m_size; }
-    int next() { return m_index++; }
+    qsizetype next() { return m_index++; }
 
 private:
-    int m_index;
-    int m_size;
+    qsizetype m_index;
+    qsizetype m_size;
 };
 
 // QJSListForInIterator must not require initialization so that we can jump over it with goto.
@@ -365,7 +357,7 @@ public:
     Value next(const QJSList<List, Value> &list) { return list.at(m_index++); }
 
 private:
-    int m_index;
+    qsizetype m_index;
 };
 
 // QJSListForOfIterator must not require initialization so that we can jump over it with goto.
diff --git a/src/qmlcompiler/qqmljscodegenerator.cpp b/src/qmlcompiler/qqmljscodegenerator.cpp
index ce2334b935..7b0664f918 100644
--- a/src/qmlcompiler/qqmljscodegenerator.cpp
+++ b/src/qmlcompiler/qqmljscodegenerator.cpp
@@ -723,13 +723,11 @@ void QQmlJSCodeGenerator::generate_LoadElement(int base)
     AccumulatorConverter registers(this);
     const QString baseName = registerVariable(base);
 
-    if (!m_typeResolver->isIntegral(indexType)) {
+    if (!m_typeResolver->isNativeArrayIndex(indexType)) {
         m_body += u"if (!QJSNumberCoercion::isArrayIndex("_s + indexName + u"))\n"_s
                 + voidAssignment
                 + u"else "_s;
-    }
-
-    if (!m_typeResolver->isUnsignedInteger(indexType)) {
+    } else if (!m_typeResolver->isUnsignedInteger(indexType)) {
         m_body += u"if ("_s + indexName + u" < 0)\n"_s
                 + voidAssignment
                 + u"else "_s;
@@ -774,9 +772,9 @@ void QQmlJSCodeGenerator::generate_StoreElement(int base, int index)
     INJECT_TRACE_INFO(generate_StoreElement);
 
     const QQmlJSRegisterContent baseType = registerType(base);
-    const QQmlJSRegisterContent indexType = registerType(index);
+    const QQmlJSScope::ConstPtr indexType = m_typeResolver->containedType(registerType(index));
 
-    if (!m_typeResolver->isNumeric(registerType(index)) || !baseType.isList()) {
+    if (!m_typeResolver->isNumeric(indexType) || !baseType.isList()) {
         reject(u"StoreElement with non-list base type or non-numeric arguments"_s);
         return;
     }
@@ -794,26 +792,27 @@ void QQmlJSCodeGenerator::generate_StoreElement(int base, int index)
                                           m_typeResolver->containedType(valueType)));
 
     addInclude(u"QtQml/qjslist.h"_s);
-    m_body += u"if ("_s;
-    if (!m_typeResolver->isIntegral(indexType))
-        m_body += u"QJSNumberCoercion::isArrayIndex("_s + indexName + u") && "_s;
-    if (!m_typeResolver->isUnsignedInteger(m_typeResolver->containedType(indexType)))
-        m_body += indexName + u" >= 0"_s;
+    if (!m_typeResolver->isNativeArrayIndex(indexType))
+        m_body += u"if (QJSNumberCoercion::isArrayIndex("_s + indexName + u")) {\n"_s;
+    else if (!m_typeResolver->isUnsignedInteger(indexType))
+        m_body += u"if ("_s + indexName + u" >= 0) {\n"_s;
+    else
+        m_body += u"{\n"_s;
 
     if (m_typeResolver->registerIsStoredIn(baseType, m_typeResolver->listPropertyType())) {
-        m_body += u" && "_s + indexName + u" < "_s + baseName + u".count(&"_s + baseName
+        m_body += u"    if ("_s + indexName + u" < "_s + baseName + u".count(&"_s + baseName
                 + u"))\n"_s;
-        m_body += u"    "_s + baseName + u".replace(&"_s + baseName
+        m_body += u"        "_s + baseName + u".replace(&"_s + baseName
                 + u", "_s + indexName + u", "_s;
         m_body += conversion(m_state.accumulatorIn(), elementType, m_state.accumulatorVariableIn)
                 + u");\n"_s;
+        m_body += u"}\n"_s;
         return;
     }
 
     if (m_state.isRegisterAffectedBySideEffects(base))
         reject(u"LoadElement on a sequence potentially affected by side effects"_s);
 
-    m_body += u") {\n"_s;
     m_body += u"    if ("_s + indexName + u" >= " + baseName + u".size())\n"_s;
     m_body += u"        QJSList(&"_s + baseName + u", aotContext->engine).resize("_s
             + indexName + u" + 1);\n"_s;
@@ -1381,7 +1380,7 @@ void QQmlJSCodeGenerator::generate_GetLookupHelper(int index)
         if (stored->isListProperty()) {
             m_body += m_state.accumulatorVariableOut + u" = "_s;
             m_body += conversion(
-                        m_typeResolver->globalType(m_typeResolver->int32Type()),
+                        m_typeResolver->globalType(m_typeResolver->sizeType()),
                         m_state.accumulatorOut(),
                         m_state.accumulatorVariableIn + u".count("_s + u'&'
                             + m_state.accumulatorVariableIn + u')');
@@ -1389,7 +1388,7 @@ void QQmlJSCodeGenerator::generate_GetLookupHelper(int index)
         } else if (stored->accessSemantics() == QQmlJSScope::AccessSemantics::Sequence
                    || m_typeResolver->equals(stored, m_typeResolver->stringType())) {
             m_body += m_state.accumulatorVariableOut + u" = "_s
-                    + conversion(m_typeResolver->globalType(m_typeResolver->int32Type()),
+                    + conversion(m_typeResolver->globalType(m_typeResolver->sizeType()),
                                  m_state.accumulatorOut(),
                                  m_state.accumulatorVariableIn + u".length()"_s)
                     + u";\n"_s;
@@ -2321,14 +2320,13 @@ void QQmlJSCodeGenerator::generate_Construct(int func, int argc, int argv)
                     + u"QLatin1String(\"Invalid array length\"));\n"_s;
 
             const QString indexName = registerVariable(argv);
-            const auto indexType = registerType(argv);
-            if (!m_typeResolver->isIntegral(indexType)) {
+            const auto indexType = m_typeResolver->containedType(registerType(argv));
+            if (!m_typeResolver->isNativeArrayIndex(indexType)) {
                 m_body += u"if (!QJSNumberCoercion::isArrayIndex("_s + indexName + u")) {\n"_s
                         + error;
                 generateReturnError();
                 m_body += u"}\n"_s;
-            } else if (!m_typeResolver->isUnsignedInteger(
-                               m_typeResolver->containedType(indexType))) {
+            } else if (!m_typeResolver->isUnsignedInteger(indexType)) {
                 m_body += u"if ("_s + indexName + u" < 0) {\n"_s
                         + error;
                 generateReturnError();
@@ -2340,7 +2338,7 @@ void QQmlJSCodeGenerator::generate_Construct(int func, int argc, int argv)
             m_body += u"QJSList(&"_s + m_state.accumulatorVariableOut
                     + u", aotContext->engine).resize("_s
                     + convertStored(
-                              registerType(argv).storedType(), m_typeResolver->int32Type(),
+                              registerType(argv).storedType(), m_typeResolver->sizeType(),
                               consumedRegisterVariable(argv))
                     + u");\n"_s;
         } else if (!m_error->isValid()) {
diff --git a/src/qmlcompiler/qqmljstypepropagator.cpp b/src/qmlcompiler/qqmljstypepropagator.cpp
index 0ff2e18684..baa6eab694 100644
--- a/src/qmlcompiler/qqmljstypepropagator.cpp
+++ b/src/qmlcompiler/qqmljstypepropagator.cpp
@@ -742,7 +742,7 @@ void QQmlJSTypePropagator::generate_LoadElement(int base)
     if (m_typeResolver->isNumeric(m_state.accumulatorIn())) {
         const auto contained = m_typeResolver->containedType(m_state.accumulatorIn());
         if (m_typeResolver->isSignedInteger(contained))
-            addReadAccumulator(m_typeResolver->globalType(m_typeResolver->int32Type()));
+            addReadAccumulator(m_typeResolver->globalType(m_typeResolver->sizeType()));
         else if (m_typeResolver->isUnsignedInteger(contained))
             addReadAccumulator(m_typeResolver->globalType(m_typeResolver->uint32Type()));
         else
diff --git a/src/qmlcompiler/qqmljstyperesolver.cpp b/src/qmlcompiler/qqmljstyperesolver.cpp
index f136120f75..93f9c7f7f4 100644
--- a/src/qmlcompiler/qqmljstyperesolver.cpp
+++ b/src/qmlcompiler/qqmljstyperesolver.cpp
@@ -376,6 +376,16 @@ bool QQmlJSTypeResolver::isUnsignedInteger(const QQmlJSScope::ConstPtr &type) co
             || equals(type, m_uint64Type);
 }
 
+bool QQmlJSTypeResolver::isNativeArrayIndex(const QQmlJSScope::ConstPtr &type) const
+{
+    return (equals(type, m_uint8Type)
+            || equals(type, m_int8Type)
+            || equals(type, m_uint16Type)
+            || equals(type, m_int16Type)
+            || equals(type, m_uint32Type)
+            || equals(type, m_int32Type));
+}
+
 QQmlJSScope::ConstPtr
 QQmlJSTypeResolver::containedType(const QQmlJSRegisterContent &container) const
 {
@@ -1395,11 +1405,11 @@ QQmlJSRegisterContent QQmlJSTypeResolver::lengthProperty(
 {
     QQmlJSMetaProperty prop;
     prop.setPropertyName(u"length"_s);
-    prop.setTypeName(u"int"_s);
-    prop.setType(int32Type());
+    prop.setTypeName(u"qsizetype"_s);
+    prop.setType(sizeType());
     prop.setIsWritable(isWritable);
     return QQmlJSRegisterContent::create(
-            int32Type(), prop, QQmlJSRegisterContent::InvalidLookupIndex,
+            sizeType(), prop, QQmlJSRegisterContent::InvalidLookupIndex,
             QQmlJSRegisterContent::InvalidLookupIndex, QQmlJSRegisterContent::Builtin, scope);
 }
 
@@ -1612,7 +1622,7 @@ QQmlJSRegisterContent QQmlJSTypeResolver::valueType(const QQmlJSRegisterContent
             return scope->valueType();
 
         if (equals(scope, m_forInIteratorPtr))
-            return m_int32Type;
+            return m_sizeType;
 
         if (equals(scope, m_forOfIteratorPtr))
             return list.scopeType()->valueType();
diff --git a/src/qmlcompiler/qqmljstyperesolver_p.h b/src/qmlcompiler/qqmljstyperesolver_p.h
index e72e7e78f0..6a5ea8cf9b 100644
--- a/src/qmlcompiler/qqmljstyperesolver_p.h
+++ b/src/qmlcompiler/qqmljstyperesolver_p.h
@@ -201,6 +201,7 @@ public:
     bool isIntegral(const QQmlJSScope::ConstPtr &type) const;
     bool isSignedInteger(const QQmlJSScope::ConstPtr &type) const;
     bool isUnsignedInteger(const QQmlJSScope::ConstPtr &type) const;
+    bool isNativeArrayIndex(const QQmlJSScope::ConstPtr &type) const;
 
     bool canHold(const QQmlJSScope::ConstPtr &container,
                  const QQmlJSScope::ConstPtr &contained) const;