woboq / qmetaobject-rs

Integrate Qml and Rust by building the QMetaObject at compile time.
MIT License
620 stars 89 forks source link

QmlEngine::invoke_method is broken with Qt ≥6.5 #286

Closed alois31 closed 1 year ago

alois31 commented 1 year ago

In Qt 6.5, there have been some changes around QMetaObject::invokeMethod. Compilation has been fixed by https://github.com/woboq/qmetaobject-rs/pull/284, however this change was incorrect. QMetaObject::invokeMethod apparently now checks the signature of the method, and prints an error message on mismatches. Since we pass in a bunch of extra arguments which are not properly initialized, this segfaults trying to dereference a null pointer, probably when trying to print the type name of the extra arguments. I'm not sure how to fix this short of providing implementing each arity separately. Even then, only QVariants can be passed with the current code, so I'm not sure how useful that is.

ogoffart commented 1 year ago

cc @AdrianEddy

AdrianEddy commented 1 year ago

Hmm, everything works fine for me, do you have a reproducible example?

alois31 commented 1 year ago

A simple cargo test segfaults reproducibly for me in the mentioned codepath.

AdrianEddy commented 1 year ago

Indeed, thanks for noticing this.

@ogoffart I confirmed it works when the method is called explicitly with the correct number of arguments, but it's not pretty:

QVariant ret;
#if QT_VERSION >= QT_VERSION_CHECK(6,5,0)
    #define INVOKE_METHOD_WITH_ARGS(...) QMetaObject::invokeMethod(robjs.first(), name, qReturnArg(ret), __VA_ARGS__);
#else
    #define INVOKE_METHOD_WITH_ARGS(...) QMetaObject::invokeMethod(robjs.first(), name, Q_RETURN_ARG(QVariant, ret), __VA_ARGS__);
#endif
switch (args_size) {
    case 0: INVOKE_METHOD_WITH_ARGS() break;
    case 1: INVOKE_METHOD_WITH_ARGS(Q_ARG(QVariant, args_ptr[0])); break;
    case 2: INVOKE_METHOD_WITH_ARGS(Q_ARG(QVariant, args_ptr[0]), Q_ARG(QVariant, args_ptr[1])); break;
    case 3: INVOKE_METHOD_WITH_ARGS(Q_ARG(QVariant, args_ptr[0]), Q_ARG(QVariant, args_ptr[1]), Q_ARG(QVariant, args_ptr[2])); break;
    case 4: INVOKE_METHOD_WITH_ARGS(Q_ARG(QVariant, args_ptr[0]), Q_ARG(QVariant, args_ptr[1]), Q_ARG(QVariant, args_ptr[2]), Q_ARG(QVariant, args_ptr[3])); break;
    case 5: INVOKE_METHOD_WITH_ARGS(Q_ARG(QVariant, args_ptr[0]), Q_ARG(QVariant, args_ptr[1]), Q_ARG(QVariant, args_ptr[2]), Q_ARG(QVariant, args_ptr[3]), Q_ARG(QVariant, args_ptr[4])); break;
    case 6: INVOKE_METHOD_WITH_ARGS(Q_ARG(QVariant, args_ptr[0]), Q_ARG(QVariant, args_ptr[1]), Q_ARG(QVariant, args_ptr[2]), Q_ARG(QVariant, args_ptr[3]), Q_ARG(QVariant, args_ptr[4]), Q_ARG(QVariant, args_ptr[5])); break;
    case 7: INVOKE_METHOD_WITH_ARGS(Q_ARG(QVariant, args_ptr[0]), Q_ARG(QVariant, args_ptr[1]), Q_ARG(QVariant, args_ptr[2]), Q_ARG(QVariant, args_ptr[3]), Q_ARG(QVariant, args_ptr[4]), Q_ARG(QVariant, args_ptr[5]), Q_ARG(QVariant, args_ptr[6])); break;
    case 8: INVOKE_METHOD_WITH_ARGS(Q_ARG(QVariant, args_ptr[0]), Q_ARG(QVariant, args_ptr[1]), Q_ARG(QVariant, args_ptr[2]), Q_ARG(QVariant, args_ptr[3]), Q_ARG(QVariant, args_ptr[4]), Q_ARG(QVariant, args_ptr[5]), Q_ARG(QVariant, args_ptr[6]), Q_ARG(QVariant, args_ptr[7])); break;
    case 9: INVOKE_METHOD_WITH_ARGS(Q_ARG(QVariant, args_ptr[0]), Q_ARG(QVariant, args_ptr[1]), Q_ARG(QVariant, args_ptr[2]), Q_ARG(QVariant, args_ptr[3]), Q_ARG(QVariant, args_ptr[4]), Q_ARG(QVariant, args_ptr[5]), Q_ARG(QVariant, args_ptr[6]), Q_ARG(QVariant, args_ptr[7]), Q_ARG(QVariant, args_ptr[8])); break;
}
return ret;

Do you want me to submit this PR, or you have a better idea?

alois31 commented 1 year ago

We should probably also assert that there are at most 9 arguments instead of writing extra arguments out of bounds and then silently not passing them on (the same is actually the case for the old codepath). It's a pity that invokeMethodImpl is private, otherwise we could properly pass on the variadics.