diff options
| author | Fabian Kosmale <fabian.kosmale@qt.io> | 2024-12-12 14:39:37 +0100 |
|---|---|---|
| committer | Fabian Kosmale <fabian.kosmale@qt.io> | 2024-12-18 11:55:52 +0100 |
| commit | a5feec81934ab0b074d6a8c7621b591851f6b544 (patch) | |
| tree | ca409667609e35a3a5dec64ee2cdd14cf59f634f /src/qml/jsruntime/qv4urlobject.cpp | |
| parent | ac2d9bf0f2c32bdd6a64b8421c414a28369cbe2e (diff) | |
QtQml: Avoid potential gc issues
Implicitly constructing a value from a ReturnedValue muddies the
responsibility for ensuring that the gc can find the object.
With this change, we disable the implicit conversion. The expectation
for lifetime management is now:
- If a ReturnedValue is stored on the C++ stack, it must be put into a
QV4::Scoped class (or there should be a comment why not doing so is
safe). Passing a ReturnedValue to a function should no longer be
possible, unless the function takes a ReturnedValue, in which case the
expectation is that it stores the value in a place where it can be
seen by the gc, before doing anything that could trigger a gc run.
Using Value::fromReturnedValue can still be used to pass a Value on,
but in that case, the expectation is that there is a comment which
explains why this is safe.
- If a QV4::Value is obtained from a function call, it ought to be
stored in a ScopedValue, too. We currently can't enforce this easily,
so this should be checked during code review. A possible way forward
would be to disallow returning Values, but that would be a larger
change, and is deferred to the future.
- If a functions has a QV4::Value parameter, it's the callers'
responsibilty to ensure that the gc can find it.
Pick-to: 6.9 6.8 6.5
Fixes: QTBUG-131961
Change-Id: Iea055589d35a5f1ac36fe376d4389eb81de87961
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
Diffstat (limited to 'src/qml/jsruntime/qv4urlobject.cpp')
| -rw-r--r-- | src/qml/jsruntime/qv4urlobject.cpp | 31 |
1 files changed, 18 insertions, 13 deletions
diff --git a/src/qml/jsruntime/qv4urlobject.cpp b/src/qml/jsruntime/qv4urlobject.cpp index 89c8b9cda2..ff67af84a1 100644 --- a/src/qml/jsruntime/qv4urlobject.cpp +++ b/src/qml/jsruntime/qv4urlobject.cpp @@ -806,7 +806,8 @@ ReturnedValue UrlSearchParamsCtor::virtualCallAsConstructor(const FunctionObject uint len = argArray->getLength(); for (uint i = 0; i < len; i++) { - QV4::Value pair = argArray->get(i); + // safe to user fromReturnedValue: Normal array, which will ensure marking + QV4::Value pair = Value::fromReturnedValue(argArray->get(i)); auto *pairArrayObject = pair.as<ArrayObject>(); if (pairArrayObject == nullptr) { @@ -895,11 +896,14 @@ void UrlSearchParamsObject::initializeParams(ScopedArrayObject& params) for (uint i = 0; i < len; i++) { - QV4::Value pair = params->get(i); + // fromReturnedValue is safe; everything is reachable via params + // so the gc won't collect it; we control params, so there can't be + // any weird proxy magic + QV4::Value pair = Value::fromReturnedValue(params->get(i)); auto *pairArrayObject = pair.as<ArrayObject>(); - QV4::Value key = pairArrayObject->get(uint(0)); - QV4::Value value = pairArrayObject->get(uint(1)); + QV4::Value key = Value::fromReturnedValue(pairArrayObject->get(uint(0))); + QV4::Value value = Value::fromReturnedValue(pairArrayObject->get(uint(1))); scopedKeys->put(i, key); scopedValues->put(i, value); @@ -1014,11 +1018,11 @@ QList<QStringList> UrlSearchParamsObject::params() const uint len = scopedArray->getLength(); for (uint i = 0; i < len; i++) { - QV4::Value pair = scopedArray->get(i); + QV4::Value pair = Value::fromReturnedValue(scopedArray->get(i)); auto *pairArrayObject = pair.as<ArrayObject>(); - QV4::Value key = pairArrayObject->get(uint(0)); - QV4::Value value = pairArrayObject->get(uint(1)); + QV4::Value key = Value::fromReturnedValue(pairArrayObject->get(uint(0))); + QV4::Value value = Value::fromReturnedValue(pairArrayObject->get(uint(1))); result << QStringList { key.toQString(), value.toQString() }; } @@ -1063,10 +1067,11 @@ int UrlSearchParamsObject::indexOf(QString name, int last) const int len = scopedArray->getLength(); for (int i = last + 1; i < len; i++) { - QV4::Value pair = scopedArray->get(i); + // fromReturnedValue is safe, scopedArray is a normal array and takes care of marking + QV4::Value pair = Value::fromReturnedValue(scopedArray->get(i)); auto *pairArrayObject = pair.as<ArrayObject>(); - QV4::Value key = pairArrayObject->get(uint(0)); + QV4::Value key = Value::fromReturnedValue(pairArrayObject->get(uint(0))); if (key.toQString() == name) return i; @@ -1084,10 +1089,10 @@ QString UrlSearchParamsObject::stringAt(int index, int pairIndex) const if (index >= scopedArray->getLength()) return {}; - QV4::Value pair = scopedArray->get(index); + QV4::Value pair = Value::fromReturnedValue(scopedArray->get(index)); auto *pairArrayObject = pair.as<ArrayObject>(); - QV4::Value value = pairArrayObject->get(pairIndex); + QV4::Value value = Value::fromReturnedValue(pairArrayObject->get(pairIndex)); return value.toQString(); } @@ -1101,10 +1106,10 @@ QV4::Heap::String * UrlSearchParamsObject::stringAtRaw(int index, int pairIndex) if (index >= scopedArray->getLength()) return nullptr; - QV4::Value pair = scopedArray->get(index); + QV4::Value pair = Value::fromReturnedValue(scopedArray->get(index)); auto *pairArrayObject = pair.as<ArrayObject>(); - QV4::Value value = pairArrayObject->get(pairIndex); + QV4::Value value = Value::fromReturnedValue(pairArrayObject->get(pairIndex)); return value.as<String>()->d(); } |