Fix array-like methods on V4 sequences

We need to properly convert value type lists on assignment and we need
to add the "length" property to the own properties. Furthermore, the V4
sequence methods were confused about integer type ranges. We teach
them about qsizetype, properly range check everything, and drop the
artificial limitation to INT_MAX.

Pick-to: 6.4
Task-number: QTBUG-82443
Change-Id: Ie5af1130c9e78e412c171e6fa26a28a6a7a5d498
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

6 files changed, 142 insertions, 124 deletions

diff --git a/src/qml/jsruntime/qv4arrayobject_p.h b/src/qml/jsruntime/qv4arrayobject_p.h
index f6b95e2413..b07e27b24f 100644
--- a/src/qml/jsruntime/qv4arrayobject_p.h
+++ b/src/qml/jsruntime/qv4arrayobject_p.h
@@ -20,6 +20,31 @@
 
 QT_BEGIN_NAMESPACE
 
+inline bool qIsAtMostUintLimit(qsizetype length, uint limit = std::numeric_limits<uint>::max())
+{
+    // Use the type with the larger positive range to do the comparison.
+
+    Q_ASSERT(length >= 0);
+    if constexpr (sizeof(qsizetype) > sizeof(uint)) {
+        return length <= qsizetype(limit);
+    } else {
+        return uint(length) <= limit;
+    }
+}
+
+inline bool qIsAtMostSizetypeLimit(uint length, qsizetype limit = std::numeric_limits<qsizetype>::max())
+{
+    // Use the type with the larger positive range to do the comparison.
+
+    Q_ASSERT(limit >= 0);
+    if constexpr (sizeof(qsizetype) > sizeof(uint)) {
+        return qsizetype(length) <= limit;
+    } else {
+        return length <= uint(limit);
+    }
+}
+
+
 namespace QV4 {
 
 namespace Heap {
diff --git a/src/qml/jsruntime/qv4qobjectwrapper.cpp b/src/qml/jsruntime/qv4qobjectwrapper.cpp
index 3fa107ebda..021872b7ad 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper.cpp
+++ b/src/qml/jsruntime/qv4qobjectwrapper.cpp
@@ -597,7 +597,7 @@ void QObjectWrapper::setProperty(
         PROPERTY_STORE(QQmlScriptString, ss);
     } else {
         QVariant v;
-        if (property->isQList())
+        if (property->isQList() && propType.flags().testFlag(QMetaType::IsQmlList))
             v = scope.engine->toVariant(value, QMetaType::fromType<QList<QObject *> >());
         else
             v = scope.engine->toVariant(value, propType);
diff --git a/src/qml/jsruntime/qv4sequenceobject.cpp b/src/qml/jsruntime/qv4sequenceobject.cpp
index 7f4b6057ce..4dc15009c9 100644
--- a/src/qml/jsruntime/qv4sequenceobject.cpp
+++ b/src/qml/jsruntime/qv4sequenceobject.cpp
@@ -77,17 +77,24 @@ struct SequenceOwnPropertyKeyIterator : ObjectOwnPropertyKeyIterator
             s->loadReference();
         }
 
-        if (arrayIndex < quint32(s->size())) {
-            uint index = arrayIndex;
+        const qsizetype size = s->size();
+        if (size > 0 && qIsAtMostSizetypeLimit(arrayIndex, size - 1)) {
+            const uint index = arrayIndex;
             ++arrayIndex;
             if (attrs)
                 *attrs = QV4::Attr_Data;
             if (pd)
-                pd->value = s->engine()->fromVariant(s->at(index));
+                pd->value = s->engine()->fromVariant(s->at(qsizetype(index)));
             return PropertyKey::fromArrayIndex(index);
         }
 
-        return ObjectOwnPropertyKeyIterator::next(o, pd, attrs);
+        if (memberIndex == 0) {
+            ++memberIndex;
+            return o->engine()->id_length()->propertyKey();
+        }
+
+        // You cannot add any own properties via the regular JavaScript interfaces.
+        return PropertyKey::invalid();
     }
 };
 
@@ -182,7 +189,7 @@ qsizetype Sequence::size() const
     return metaSequence(p)->size(p->container);
 }
 
-QVariant Sequence::at(int index) const
+QVariant Sequence::at(qsizetype index) const
 {
     const auto *p = d();
     const QMetaType v = valueMetaType(p);
@@ -196,41 +203,50 @@ QVariant Sequence::at(int index) const
     return result;
 }
 
-void Sequence::append(const QVariant &item)
+
+template<typename Action>
+void convertAndDo(const QVariant &item, const QMetaType v, Action action)
 {
-    const auto *p = d();
-    const auto *m = metaSequence(p);
-    const QMetaType v = valueMetaType(p);
     if (item.metaType() == v) {
-        m->addValueAtEnd(p->container, item.constData());
+        action(item.constData());
     } else if (v == QMetaType::fromType<QVariant>()) {
-        m->addValueAtEnd(p->container, &item);
+        action(&item);
     } else {
         QVariant converted = item;
         if (!converted.convert(v))
             converted = QVariant(v);
-        m->addValueAtEnd(p->container, converted.constData());
+        action(converted.constData());
     }
 }
 
-void Sequence::replace(int index, const QVariant &item)
+void Sequence::append(const QVariant &item)
 {
-    const auto *p = d();
-    const auto *m = metaSequence(p);
-    const QMetaType v = valueMetaType(p);
-    if (item.metaType() == v) {
-        m->setValueAtIndex(p->container, index, item.constData());
-    } else if (v == QMetaType::fromType<QVariant>()) {
-        m->setValueAtIndex(p->container, index, &item);
-    } else {
-        QVariant converted = item;
-        if (!converted.convert(v))
-            converted = QVariant(v);
-        m->setValueAtIndex(p->container, index, converted.constData());
-    }
+    const Heap::Sequence *p = d();
+    convertAndDo(item, valueMetaType(p), [p](const void *data) {
+        metaSequence(p)->addValueAtEnd(p->container, data);
+    });
+}
+
+void Sequence::append(qsizetype num, const QVariant &item)
+{
+    const Heap::Sequence *p = d();
+    convertAndDo(item, valueMetaType(p), [p, num](const void *data) {
+        const QMetaSequence *m = metaSequence(p);
+        void *container = p->container;
+        for (qsizetype i = 0; i < num; ++i)
+            m->addValueAtEnd(container, data);
+    });
 }
 
-void Sequence::removeLast(int num)
+void Sequence::replace(qsizetype index, const QVariant &item)
+{
+    const Heap::Sequence *p = d();
+    convertAndDo(item, valueMetaType(p), [p, index](const void *data) {
+        metaSequence(p)->setValueAtIndex(p->container, index, data);
+    });
+}
+
+void Sequence::removeLast(qsizetype num)
 {
     const auto *p = d();
     const auto *m = metaSequence(p);
@@ -254,15 +270,8 @@ QVariant Sequence::toVariant() const
     return QVariant(p->typePrivate->listId, p->container);
 }
 
-ReturnedValue Sequence::containerGetIndexed(uint index, bool *hasProperty) const
+ReturnedValue Sequence::containerGetIndexed(qsizetype index, bool *hasProperty) const
 {
-    /* Qt containers have int (rather than uint) allowable indexes. */
-    if (index > INT_MAX) {
-        generateWarning(engine(), QLatin1String("Index out of range during indexed get"));
-        if (hasProperty)
-            *hasProperty = false;
-        return Encode::undefined();
-    }
     if (d()->isReference) {
         if (!d()->object) {
             if (hasProperty)
@@ -271,7 +280,7 @@ ReturnedValue Sequence::containerGetIndexed(uint index, bool *hasProperty) const
         }
         loadReference();
     }
-    if (index < quint32(size())) {
+    if (index >= 0 && index < size()) {
         if (hasProperty)
             *hasProperty = true;
         return engine()->fromVariant(at(index));
@@ -281,17 +290,11 @@ ReturnedValue Sequence::containerGetIndexed(uint index, bool *hasProperty) const
     return Encode::undefined();
 }
 
-bool Sequence::containerPutIndexed(uint index, const Value &value)
+bool Sequence::containerPutIndexed(qsizetype index, const Value &value)
 {
     if (internalClass()->engine->hasException)
         return false;
 
-    /* Qt containers have int (rather than uint) allowable indexes. */
-    if (index > INT_MAX) {
-        generateWarning(engine(), QLatin1String("Index out of range during indexed set"));
-        return false;
-    }
-
     if (d()->isReadOnly) {
         engine()->throwTypeError(QLatin1String("Cannot insert into a readonly container"));
         return false;
@@ -303,10 +306,13 @@ bool Sequence::containerPutIndexed(uint index, const Value &value)
         loadReference();
     }
 
-    quint32 count = quint32(size());
+    const qsizetype count = size();
     const QMetaType valueType = valueMetaType(d());
     const QVariant element = engine()->toVariant(value, valueType, false);
 
+    if (index < 0)
+        return false;
+
     if (index == count) {
         append(element);
     } else if (index < count) {
@@ -314,11 +320,8 @@ bool Sequence::containerPutIndexed(uint index, const Value &value)
     } else {
         /* according to ECMA262r3 we need to insert */
         /* the value at the given index, increasing length to index+1. */
-        while (index > count++) {
-            append(valueType == QMetaType::fromType<QVariant>()
-                           ? QVariant()
-                           : QVariant(valueType));
-        }
+        append(index - count,
+               valueType == QMetaType::fromType<QVariant>() ? QVariant() : QVariant(valueType));
         append(element);
     }
 
@@ -327,32 +330,14 @@ bool Sequence::containerPutIndexed(uint index, const Value &value)
     return true;
 }
 
-PropertyAttributes Sequence::containerQueryIndexed(uint index) const
-{
-    /* Qt containers have int (rather than uint) allowable indexes. */
-    if (index > INT_MAX) {
-        generateWarning(engine(), QLatin1String("Index out of range during indexed query"));
-        return QV4::Attr_Invalid;
-    }
-    if (d()->isReference) {
-        if (!d()->object)
-            return QV4::Attr_Invalid;
-        loadReference();
-    }
-    return (index < quint32(size())) ? QV4::Attr_Data : QV4::Attr_Invalid;
-}
-
 SequenceOwnPropertyKeyIterator *containerOwnPropertyKeys(const Object *m, Value *target)
 {
     *target = *m;
     return new SequenceOwnPropertyKeyIterator;
 }
 
-bool Sequence::containerDeleteIndexedProperty(uint index)
+bool Sequence::containerDeleteIndexedProperty(qsizetype index)
 {
-    /* Qt containers have int (rather than uint) allowable indexes. */
-    if (index > INT_MAX)
-        return false;
     if (d()->isReadOnly)
         return false;
     if (d()->isReference) {
@@ -361,7 +346,7 @@ bool Sequence::containerDeleteIndexedProperty(uint index)
         loadReference();
     }
 
-    if (index >= quint32(size()))
+    if (index < 0 || index >= size())
         return false;
 
     /* according to ECMA262r3 it should be Undefined, */
@@ -433,26 +418,40 @@ void Sequence::storeReference()
 
 ReturnedValue Sequence::virtualGet(const Managed *that, PropertyKey id, const Value *receiver, bool *hasProperty)
 {
-    if (!id.isArrayIndex())
-        return Object::virtualGet(that, id, receiver, hasProperty);
-    return static_cast<const Sequence *>(that)->containerGetIndexed(id.asArrayIndex(), hasProperty);
+    if (id.isArrayIndex()) {
+        const uint index = id.asArrayIndex();
+        if (qIsAtMostSizetypeLimit(index))
+            return static_cast<const Sequence *>(that)->containerGetIndexed(qsizetype(index), hasProperty);
+
+        generateWarning(that->engine(), QLatin1String("Index out of range during indexed get"));
+        return false;
+    }
+
+    return Object::virtualGet(that, id, receiver, hasProperty);
 }
 
 bool Sequence::virtualPut(Managed *that, PropertyKey id, const Value &value, Value *receiver)
 {
-    if (id.isArrayIndex())
-        return static_cast<Sequence *>(that)->containerPutIndexed(id.asArrayIndex(), value);
+    if (id.isArrayIndex()) {
+        const uint index = id.asArrayIndex();
+        if (qIsAtMostSizetypeLimit(index))
+            return static_cast<Sequence *>(that)->containerPutIndexed(qsizetype(index), value);
+
+        generateWarning(that->engine(), QLatin1String("Index out of range during indexed set"));
+        return false;
+    }
     return Object::virtualPut(that, id, value, receiver);
 }
 
-PropertyAttributes Sequence::queryIndexed(const Managed *that, uint index)
-{ return static_cast<const Sequence *>(that)->containerQueryIndexed(index); }
-
 bool Sequence::virtualDeleteProperty(Managed *that, PropertyKey id)
 {
     if (id.isArrayIndex()) {
-        uint index = id.asArrayIndex();
-        return static_cast<Sequence *>(that)->containerDeleteIndexedProperty(index);
+        const uint index = id.asArrayIndex();
+        if (qIsAtMostSizetypeLimit(index))
+            return static_cast<Sequence *>(that)->containerDeleteIndexedProperty(qsizetype(index));
+
+        generateWarning(that->engine(), QLatin1String("Index out of range during indexed delete"));
+        return false;
     }
     return Object::virtualDeleteProperty(that, id);
 }
@@ -479,7 +478,12 @@ static QV4::ReturnedValue method_get_length(const FunctionObject *b, const Value
             RETURN_RESULT(Encode(0));
         This->loadReference();
     }
-    RETURN_RESULT(Encode(qint32(This->size())));
+
+    const qsizetype size = This->size();
+    if (qIsAtMostUintLimit(size))
+        RETURN_RESULT(Encode(uint(size)));
+
+    return scope.engine->throwRangeError(QLatin1String("Sequence length out of range"));
 }
 
 static QV4::ReturnedValue method_set_length(const FunctionObject *f, const Value *thisObject, const Value *argv, int argc)
@@ -489,9 +493,9 @@ static QV4::ReturnedValue method_set_length(const FunctionObject *f, const Value
     if (!This)
         THROW_TYPE_ERROR();
 
-    quint32 newLength = argc ? argv[0].toUInt32() : 0;
-    /* Qt containers have int (rather than uint) allowable indexes. */
-    if (newLength > INT_MAX) {
+    bool ok = false;
+    const quint32 argv0 = argc ? argv[0].asArrayLength(&ok) : 0;
+    if (!ok || !qIsAtMostSizetypeLimit(argv0)) {
         generateWarning(scope.engine, QLatin1String("Index out of range during length set"));
         RETURN_UNDEFINED();
     }
@@ -499,15 +503,17 @@ static QV4::ReturnedValue method_set_length(const FunctionObject *f, const Value
     if (This->d()->isReadOnly)
         THROW_TYPE_ERROR();
 
+    const qsizetype newCount = qsizetype(argv0);
+
     /* Read the sequence from the QObject property if we're a reference */
     if (This->d()->isReference) {
         if (!This->d()->object)
             RETURN_UNDEFINED();
         This->loadReference();
     }
+
     /* Determine whether we need to modify the sequence */
-    quint32 newCount = static_cast<quint32>(newLength);
-    quint32 count = static_cast<quint32>(This->size());
+    const qsizetype count = This->size();
     if (newCount == count) {
         RETURN_UNDEFINED();
     } else if (newCount > count) {
@@ -515,19 +521,20 @@ static QV4::ReturnedValue method_set_length(const FunctionObject *f, const Value
         /* according to ECMA262r3 we need to insert */
         /* undefined values increasing length to newLength. */
         /* We cannot, so we insert default-values instead. */
-        while (newCount > count++)
-            This->append(QVariant(valueMetaType));
+        This->append(newCount - count, QVariant(valueMetaType));
     } else {
         /* according to ECMA262r3 we need to remove */
         /* elements until the sequence is the required length. */
-        if (newCount < count)
-            This->removeLast(count - newCount);
+        Q_ASSERT(newCount < count);
+        This->removeLast(count - newCount);
     }
+
     /* write back if required. */
     if (This->d()->isReference) {
         /* write back.  already checked that object is non-null, so skip that check here. */
         This->storeReference();
     }
+
     RETURN_UNDEFINED();
 }
 
@@ -631,9 +638,12 @@ QVariant SequencePrototype::toVariant(const QV4::Value &array, QMetaType typeHin
         const QMetaSequence *meta = priv->extraData.ld;
         const QMetaType containerMetaType(priv->listId);
         QVariant result(containerMetaType);
-        quint32 length = a->getLength();
+        qint64 length = a->getLength();
+        Q_ASSERT(length >= 0);
+        Q_ASSERT(length <= qint64(std::numeric_limits<quint32>::max()));
+
         QV4::ScopedValue v(scope);
-        for (quint32 i = 0; i < length; ++i) {
+        for (quint32 i = 0; i < quint32(length); ++i) {
             const QMetaType valueMetaType = priv->typeId;
             QVariant variant = scope.engine->toVariant(a->get(i), valueMetaType, false);
             if (valueMetaType == QMetaType::fromType<QVariant>()) {
diff --git a/src/qml/jsruntime/qv4sequenceobject_p.h b/src/qml/jsruntime/qv4sequenceobject_p.h
index 51cdc2fc2c..256d324355 100644
--- a/src/qml/jsruntime/qv4sequenceobject_p.h
+++ b/src/qml/jsruntime/qv4sequenceobject_p.h
@@ -75,26 +75,21 @@ public:
     static QV4::ReturnedValue virtualGet(
             const QV4::Managed *that, PropertyKey id, const Value *receiver, bool *hasProperty);
     static bool virtualPut(Managed *that, PropertyKey id, const QV4::Value &value, Value *receiver);
-    static QV4::PropertyAttributes queryIndexed(const QV4::Managed *that, uint index);
     static bool virtualDeleteProperty(QV4::Managed *that, PropertyKey id);
     static bool virtualIsEqualTo(Managed *that, Managed *other);
     static QV4::OwnPropertyKeyIterator *virtualOwnPropertyKeys(const Object *m, Value *target);
 
     qsizetype size() const;
-    QVariant at(int index) const;
+    QVariant at(qsizetype index) const;
     void append(const QVariant &item);
-    void replace(int index, const QVariant &item);
-    void removeLast(int num);
+    void append(qsizetype num, const QVariant &item);
+    void replace(qsizetype index, const QVariant &item);
+    void removeLast(qsizetype num);
     QVariant toVariant() const;
 
-    //  ### Qt 7 use qsizetype instead.
-    QV4::ReturnedValue containerGetIndexed(uint index, bool *hasProperty) const;
-
-    //  ### Qt 7 use qsizetype instead.
-    bool containerPutIndexed(uint index, const QV4::Value &value);
-
-    QV4::PropertyAttributes containerQueryIndexed(uint index) const;
-    bool containerDeleteIndexedProperty(uint index);
+    QV4::ReturnedValue containerGetIndexed(qsizetype index, bool *hasProperty) const;
+    bool containerPutIndexed(qsizetype index, const QV4::Value &value);
+    bool containerDeleteIndexedProperty(qsizetype index);
     bool containerIsEqualTo(Managed *other);
     bool sort(const FunctionObject *f, const Value *, const Value *argv, int argc);
     void *getRawContainerPtr() const;
diff --git a/src/qml/qml/qqmllistwrapper.cpp b/src/qml/qml/qqmllistwrapper.cpp
index 29da1002c4..ba6058ff3d 100644
--- a/src/qml/qml/qqmllistwrapper.cpp
+++ b/src/qml/qml/qqmllistwrapper.cpp
@@ -20,18 +20,6 @@ using namespace Qt::StringLiterals;
 
 DEFINE_OBJECT_VTABLE(QmlListWrapper);
 
-static bool isAtMostUintLimit(qsizetype length, uint limit = std::numeric_limits<uint>::max())
-{
-    // Use the type with the larger positive range to do the comparison.
-
-    Q_ASSERT(length >= 0);
-    if constexpr (sizeof(qsizetype) > sizeof(uint)) {
-        return length <= qsizetype(limit);
-    } else {
-        return uint(length) <= limit;
-    }
-}
-
 void Heap::QmlListWrapper::init()
 {
     Object::init();
@@ -249,7 +237,7 @@ ReturnedValue PropertyListPrototype::method_push(const FunctionObject *b, const
     }
 
     const qsizetype length = property->count(property);
-    if (!isAtMostUintLimit(length, std::numeric_limits<uint>::max() - argc))
+    if (!qIsAtMostUintLimit(length, std::numeric_limits<uint>::max() - argc))
         return scope.engine->throwRangeError(QString::fromLatin1("List length out of range."));
 
     for (int i = 0; i < argc; ++i) {
@@ -334,7 +322,7 @@ ReturnedValue PropertyListPrototype::method_splice(const FunctionObject *b, cons
         return scope.engine->throwTypeError();
     }
 
-    if (!isAtMostUintLimit(deleteCount, std::numeric_limits<uint>::max() - 1))
+    if (!qIsAtMostUintLimit(deleteCount, std::numeric_limits<uint>::max() - 1))
         return scope.engine->throwRangeError(QString::fromLatin1("List length out of range."));
 
     if (!property->at)
@@ -404,7 +392,7 @@ ReturnedValue PropertyListPrototype::method_unshift(const FunctionObject *b, con
         return scope.engine->throwTypeError(u"List doesn't define a Count function"_s);
     const qsizetype len = property->count(property);
 
-    if (std::numeric_limits<qsizetype>::max() - len < argc || !isAtMostUintLimit(len + argc))
+    if (std::numeric_limits<qsizetype>::max() - len < argc || !qIsAtMostUintLimit(len + argc))
         return scope.engine->throwRangeError(QString::fromLatin1("List length out of range."));
 
     if (!property->append)
@@ -490,7 +478,7 @@ ReturnedValue PropertyListPrototype::method_indexOf(const FunctionObject *b, con
 
         for (qsizetype i = fromIndex; i < len; ++i) {
             if (property->at(property, i) == searchValue) {
-                if (isAtMostUintLimit(i))
+                if (qIsAtMostUintLimit(i))
                     return Encode(uint(i));
                 return engine->throwRangeError(QString::fromLatin1("List length out of range."));
             }
@@ -523,7 +511,7 @@ ReturnedValue PropertyListPrototype::method_lastIndexOf(const FunctionObject *b,
 
         for (qsizetype i = fromIndex; i >= 0; --i) {
             if (property->at(property, i) == searchValue) {
-                if (isAtMostUintLimit(i))
+                if (qIsAtMostUintLimit(i))
                     return Encode(uint(i));
                 return engine->throwRangeError(QString::fromLatin1("List length out of range."));
             }
@@ -585,7 +573,7 @@ ReturnedValue PropertyListPrototype::method_get_length(const FunctionObject *b,
         return scope.engine->throwTypeError(u"List doesn't define a Count function"_s);
 
     qsizetype count = property->count(property);
-    if (isAtMostUintLimit(count))
+    if (qIsAtMostUintLimit(count))
         return Encode(uint(count));
 
     return scope.engine->throwRangeError(QString::fromLatin1("List length out of range."));
@@ -618,7 +606,7 @@ ReturnedValue PropertyListPrototype::method_set_length(const FunctionObject *b,
         return scope.engine->throwTypeError(u"List doesn't define a Count function"_s);
 
     qsizetype count = property->count(property);
-    if (!isAtMostUintLimit(count))
+    if (!qIsAtMostUintLimit(count))
         return scope.engine->throwRangeError(QString::fromLatin1("List length out of range."));
 
     if (newLength < uint(count)) {
diff --git a/src/qml/qml/qqmlvmemetaobject.cpp b/src/qml/qml/qqmlvmemetaobject.cpp
index dec7eb63b7..8a0bc1a08e 100644
--- a/src/qml/qml/qqmlvmemetaobject.cpp
+++ b/src/qml/qml/qqmlvmemetaobject.cpp
@@ -829,7 +829,7 @@ int QQmlVMEMetaObject::metaCall(QObject *o, QMetaObject::Call c, int _id, void *
                                             engine, propType, a[0], &success));
                                 if (!success) {
                                     qmlWarning(object)
-                                            << "Could not create a QML sequence object for"
+                                            << "Could not create a QML sequence object for "
                                             << propType.name();
                                 }
                                 needActivate = true;