QtQml: Fix return type constructions when calling methods

We now expect the return type to be initialized in all cases. This is in
line with what the various metacall() methods expect. Unifying this
behavior makes it much easier to reason about and avoids complicated
bugs and memory leaks. The code generated by QmlCompiler always passes
initialized values for the return type anyway.

Amends commit 4f1b9156a4
Amends commit 02c4c817fe

Change-Id: I26c016676dd82c91d6ef81762b5c4b599f6f7f72
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
This commit is contained in:
Ulf Hermann 2023-10-03 17:44:20 +02:00
parent bb698b7f2e
commit 1d8859ce3a
5 changed files with 46 additions and 31 deletions

View File

@ -125,6 +125,7 @@ ReturnedValue convertAndCall(
types[i + 1] = argumentType;
if (const qsizetype argumentSize = argumentType.sizeOf()) {
Q_ALLOCA_VAR(void, argument, argumentSize);
if (argumentType.flags() & QMetaType::NeedsConstruction)
argumentType.construct(argument);
if (i < argc)
ExecutionEngine::metaTypeFromJS(argv[i], argumentType, argument);
@ -139,6 +140,8 @@ ReturnedValue convertAndCall(
if (const qsizetype returnSize = types[0].sizeOf()) {
Q_ALLOCA_ASSIGN(void, returnValue, returnSize);
values[0] = returnValue;
if (types[0].flags() & QMetaType::NeedsConstruction)
types[0].construct(returnValue);
} else {
values[0] = nullptr;
}
@ -151,13 +154,16 @@ ReturnedValue convertAndCall(
ReturnedValue result;
if (values[0]) {
result = engine->metaTypeToJS(types[0], values[0]);
if (types[0].flags() & QMetaType::NeedsDestruction)
types[0].destruct(values[0]);
} else {
result = Encode::undefined();
}
for (qsizetype i = 1, end = numFunctionArguments + 1; i < end; ++i)
for (qsizetype i = 1, end = numFunctionArguments + 1; i < end; ++i) {
if (types[i].flags() & QMetaType::NeedsDestruction)
types[i].destruct(values[i]);
}
return result;
}
@ -477,10 +483,9 @@ void coerceAndCall(
memcpy(transformedArguments, argv, (argc + 1) * sizeof(void *));
if (frameReturn == QMetaType::fromType<QVariant>()) {
void *returnValue = argv[0];
new (returnValue) QVariant(returnType);
transformedResult = transformedArguments[0]
= static_cast<QVariant *>(returnValue)->data();
QVariant *returnValue = static_cast<QVariant *>(argv[0]);
*returnValue = QVariant(returnType);
transformedResult = transformedArguments[0] = returnValue->data();
returnsQVariantWrapper = true;
} else if (returnType.sizeOf() > 0) {
Q_ALLOCA_ASSIGN(void, transformedResult, returnType.sizeOf());
@ -545,8 +550,11 @@ void coerceAndCall(
call(transformedArguments, numFunctionArguments);
if (transformedResult && !returnsQVariantWrapper) {
if (frameReturn.sizeOf() > 0)
if (frameReturn.sizeOf() > 0) {
if (frameReturn.flags() & QMetaType::NeedsDestruction)
frameReturn.destruct(argv[0]);
coerce(engine, returnType, transformedResult, frameReturn, argv[0]);
}
if (returnType.flags() & QMetaType::NeedsDestruction)
returnType.destruct(transformedResult);
}

View File

@ -675,19 +675,20 @@ void QQmlBinding::doUpdate(const DeleteWatcher &watcher, QQmlPropertyData::Write
if (v4Function && v4Function->kind == QV4::Function::AotCompiled && !hasBoundFunction()) {
const auto returnType = v4Function->aotCompiledFunction->returnType;
if (returnType == QMetaType::fromType<QVariant>()) {
// It expects uninitialized memory
Q_ALLOCA_VAR(QVariant, result, sizeof(QVariant));
const bool isUndefined = !evaluate(result, returnType);
QVariant result;
const bool isUndefined = !evaluate(&result, returnType);
if (canWrite())
error = !write(result->data(), result->metaType(), isUndefined, flags);
result->~QVariant();
error = !write(result.data(), result.metaType(), isUndefined, flags);
} else {
const auto size = returnType.sizeOf();
if (Q_LIKELY(size > 0)) {
Q_ALLOCA_VAR(void, result, size);
if (returnType.flags() & QMetaType::NeedsConstruction)
returnType.construct(result);
const bool isUndefined = !evaluate(result, returnType);
if (canWrite())
error = !write(result, returnType, isUndefined, flags);
if (returnType.flags() & QMetaType::NeedsDestruction)
returnType.destruct(result);
} else if (canWrite()) {
error = !write(QV4::Encode::undefined(), true, flags);

View File

@ -286,19 +286,27 @@ bool QQmlPropertyBinding::evaluate(QMetaType metaType, void *dataPtr)
if (!hasBoundFunction()) {
Q_ASSERT(metaType.sizeOf() > 0);
// No need to construct here. evaluate() expects uninitialized memory.
const auto size = [&]() -> qsizetype {
using Tuple = std::tuple<qsizetype, bool, bool>;
const auto [size, needsConstruction, needsDestruction] = [&]() -> Tuple {
switch (type) {
case QMetaType::QObjectStar: return sizeof(QObject *);
case QMetaType::Bool: return sizeof(bool);
case QMetaType::Int: return (sizeof(int));
case QMetaType::Double: return (sizeof(double));
case QMetaType::Float: return (sizeof(float));
case QMetaType::QString: return (sizeof(QString));
default: return metaType.sizeOf();
case QMetaType::QObjectStar: return Tuple(sizeof(QObject *), false, false);
case QMetaType::Bool: return Tuple(sizeof(bool), false, false);
case QMetaType::Int: return Tuple(sizeof(int), false, false);
case QMetaType::Double: return Tuple(sizeof(double), false, false);
case QMetaType::Float: return Tuple(sizeof(float), false, false);
case QMetaType::QString: return Tuple(sizeof(QString), true, true);
default: {
const auto flags = metaType.flags();
return Tuple(
metaType.sizeOf(),
flags & QMetaType::NeedsConstruction,
flags & QMetaType::NeedsDestruction);
}
}
}();
Q_ALLOCA_VAR(void, result, size);
if (needsConstruction)
metaType.construct(result);
const bool evaluatedToUndefined = !jsExpression()->evaluate(&result, &metaType, 0);
if (!handleErrorAndUndefined(evaluatedToUndefined))
@ -326,9 +334,11 @@ bool QQmlPropertyBinding::evaluate(QMetaType metaType, void *dataPtr)
const bool hasChanged = !metaType.equals(result, dataPtr);
if (hasChanged) {
if (needsDestruction)
metaType.destruct(dataPtr);
metaType.construct(dataPtr, result);
}
if (needsDestruction)
metaType.destruct(result);
return hasChanged;
}

View File

@ -1119,14 +1119,10 @@ int QQmlVMEMetaObject::metaCall(QObject *o, QMetaObject::Call c, int _id, void *
if (arguments && arguments->names) {
const quint32 parameterCount = arguments->names->size();
Q_ASSERT(parameterCount == function->formalParameterCount());
if (void *result = a[0])
arguments->types[0].destruct(result);
function->call(nullptr, a, arguments->types, parameterCount);
} else {
Q_ASSERT(function->formalParameterCount() == 0);
const QMetaType returnType = methodData->propType();
if (void *result = a[0])
returnType.destruct(result);
function->call(nullptr, a, &returnType, 0);
}

View File

@ -468,7 +468,7 @@ void wrapCall(const QQmlPrivate::AOTCompiledContext *aotContext, void *dataPtr,
binding(aotContext, argumentsPtr);
} else {
if (dataPtr) {
new (dataPtr) return_type(binding(aotContext, argumentsPtr));
*static_cast<return_type *>(dataPtr) = binding(aotContext, argumentsPtr);
} else {
binding(aotContext, argumentsPtr);
}