From e91a17873ee4ae58d369b8eb70029cf895b31d03 Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Sun, 22 Dec 2024 13:17:05 +0100 Subject: [PATCH] JNI: replace va_arg helper with variadic template Android 15 (at least) seems to have changed how float arguments are passed to native functions, breaking our (conceptually correct) code for processing the va_arg list into a list of static argument types of the function implementation. To fix this, we have to move away from using a va_arg function, and register a function with statically typed arguments instead. Use a template function that we instantiate with variadic arguments deduced from the actual function, using a factory-helper that generates a JNINativeMethod struct with that template instantiation as the function pointer. Move all of that into a struct where we can also declare the signature string as compile-time constant without cluttering the namespace with static objects. We can now remove the helpers that took care of type promotion in va_arg functions, and of the tuple-construction from a va_list. As a drive-by, don't cast function pointers to void *; it's strictly speaking undefined behavior in C and should have generated a compiler warning, if not a hard error [1]. We must initialize the JNINativeMethod::fnPtr member with the address of the function pointer instead. [1] https://port70.net/~nsz/c/c11/n1570.html#6.3.2.3p8 Also, declare the native method as the JNICALL calling convention. That is only defined on Windows, so makes no difference in practice, but it's the correct thing to do anyway. Fixes: QTBUG-132410 Pick-to: 6.9 6.8 Change-Id: I190b95fcbcd07cf99c6765fa426c3c351f91994a Reviewed-by: Volker Krause --- src/corelib/kernel/qjnitypes.h | 144 ++++++------------ .../kernel/qjniobject/tst_qjniobject.cpp | 3 - .../kernel/qjnitypes/tst_qjnitypes.cpp | 7 +- 3 files changed, 54 insertions(+), 100 deletions(-) diff --git a/src/corelib/kernel/qjnitypes.h b/src/corelib/kernel/qjnitypes.h index d9ad6b673fe..367815c6f2d 100644 --- a/src/corelib/kernel/qjnitypes.h +++ b/src/corelib/kernel/qjnitypes.h @@ -127,29 +127,18 @@ QT_OVERLOADED_MACRO(Q_DECLARE_JNI_CLASS_SPECIALIZATION, __VA_ARGS__) namespace QtJniMethods { namespace Detail { -// Various helpers to forward a call from a variadic argument function to -// the real function with proper type conversion. This is needed because we -// want to write functions that take QJniObjects (subclasses), while Java -// can only call functions that take jobjects. +// Various helpers to forward the call to the registered function (with JNI types +// as arguments) to the real function with proper type conversion. This is needed +// because we want to write functions that take QJniObjects (subclasses), while +// Java can only call functions that take jobjects. -// In Var-arg functions, any argument narrower than (unsigned) int or double -// is promoted to (unsigned) int or double. -template struct PromotedType { using Type = Arg; }; -template <> struct PromotedType { using Type = int; }; -template <> struct PromotedType { using Type = int; }; -template <> struct PromotedType { using Type = int; }; -template <> struct PromotedType { using Type = unsigned int; }; -template <> struct PromotedType { using Type = int; }; -template <> struct PromotedType { using Type = unsigned int; }; -template <> struct PromotedType { using Type = double; }; - -// Map any QJniObject type to jobject; that's what's on the va_list +// Map any QJniObject type to jobject template struct JNITypeForArgImpl { using Type = std::conditional_t, std::is_base_of>, - jobject, typename PromotedType::Type>; + jobject, Arg>; static Arg fromVarArg(Type t) { return static_cast(t); @@ -196,93 +185,60 @@ public: template using JNITypeForArg = typename JNITypeForArgImpl>::Type; -template -static inline auto methodArgFromVarArg(Type t) // Type comes from a va_arg, so is always POD -{ - return JNITypeForArgImpl>::fromVarArg(t); -} - -// Turn a va_list into a tuple of typed arguments -template -static constexpr auto makeTupleFromArgsHelper(va_list args) -{ - return std::tuple(methodArgFromVarArg(va_arg(args, JNITypeForArg))...); -} - -template -static constexpr auto makeTupleFromArgs(Ret (*)(JNIEnv *, jobject, Args...), va_list args) -{ - return makeTupleFromArgsHelper(args); -} -template -static constexpr auto makeTupleFromArgs(Ret (*)(JNIEnv *, jclass, Args...), va_list args) -{ - return makeTupleFromArgsHelper(args); -} - -template -struct NativeFunctionReturnType {}; - -template -struct NativeFunctionReturnType -{ - using type = Ret; -}; - } // namespace Detail } // namespace QtJniMethods -// A va_ variadic arguments function that we register with JNI as a proxy -// for the function we have. This function uses the helpers to unpack the -// variadic arguments into a tuple of typed arguments, which we then call -// the actual function with. This then takes care of implicit conversions, -// e.g. a jobject becomes a QJniObject. -#define Q_DECLARE_JNI_NATIVE_METHOD_HELPER(Method) \ -static QtJniMethods::Detail::NativeFunctionReturnType::type \ -va_##Method(JNIEnv *env, jclass thiz, ...) \ -{ \ - va_list args; \ - va_start(args, thiz); \ - auto va_cleanup = qScopeGuard([&args]{ va_end(args); }); \ - auto argTuple = QtJniMethods::Detail::makeTupleFromArgs(Method, args); \ - return std::apply([env, thiz](auto &&... args) { \ - return Method(env, thiz, args...); \ - }, argTuple); \ +// Declaring a JNI method results in a struct with a template function call() that +// gets instantiated with the return type and arguments of the declared method, +// and registered with JNI. That template is implemented to call the declared +// method, with arguments explicitly converted to the types the declared method +// expects (e.g. jobject becomes QJniObject, a QString, a QList etc). +#define Q_DECLARE_JNI_NATIVE_METHOD_HELPER(Method, Postfix, Name) \ +struct Method##_##Postfix { \ +template \ +JNICALL static \ +Ret call(JNIEnv *env, JType thiz, QtJniMethods::Detail::JNITypeForArg ...args) \ +{ \ + return Method(env, thiz, QtJniMethods::Detail::JNITypeForArgImpl< \ + std::decay_t>::fromVarArg(args)... \ + ); \ +} \ +static constexpr auto signature = QtJniTypes::nativeMethodSignature(Method); \ +template \ +static constexpr JNINativeMethod makeJNIMethod(Ret(*)(JNIEnv *, JType, Args...)) \ +{ \ + return JNINativeMethod { \ + #Name, signature.data(), \ + reinterpret_cast(&call) \ + }; \ +} \ +}; \ + +#define Q_DECLARE_JNI_NATIVE_METHOD(...) \ + QT_OVERLOADED_MACRO(QT_DECLARE_JNI_NATIVE_METHOD, __VA_ARGS__) \ + +#define QT_DECLARE_JNI_NATIVE_METHOD_2(Method, Name) \ +namespace QtJniMethods { \ +Q_DECLARE_JNI_NATIVE_METHOD_HELPER(Method, Helper, Name) \ } \ -#define Q_DECLARE_JNI_NATIVE_METHOD(...) \ - QT_OVERLOADED_MACRO(QT_DECLARE_JNI_NATIVE_METHOD, __VA_ARGS__) \ +#define QT_DECLARE_JNI_NATIVE_METHOD_1(Method) \ + QT_DECLARE_JNI_NATIVE_METHOD_2(Method, Method) \ -#define QT_DECLARE_JNI_NATIVE_METHOD_2(Method, Name) \ -namespace QtJniMethods { \ -Q_DECLARE_JNI_NATIVE_METHOD_HELPER(Method) \ -static constexpr auto Method##_signature = \ - QtJniTypes::nativeMethodSignature(Method); \ -static const JNINativeMethod Method##_method = { \ - #Name, Method##_signature.data(), \ - reinterpret_cast(va_##Method) \ -}; \ -} \ +#define Q_JNI_NATIVE_METHOD(Method) \ + QtJniMethods::Method##_Helper::makeJNIMethod(::Method) -#define QT_DECLARE_JNI_NATIVE_METHOD_1(Method) \ - QT_DECLARE_JNI_NATIVE_METHOD_2(Method, Method) \ +#define Q_DECLARE_JNI_NATIVE_METHOD_IN_CURRENT_SCOPE(...) \ + QT_OVERLOADED_MACRO(QT_DECLARE_JNI_NATIVE_METHOD_IN_CURRENT_SCOPE, __VA_ARGS__) \ -#define Q_JNI_NATIVE_METHOD(Method) QtJniMethods::Method##_method +#define QT_DECLARE_JNI_NATIVE_METHOD_IN_CURRENT_SCOPE_2(Method, Name) \ +Q_DECLARE_JNI_NATIVE_METHOD_HELPER(Method, QtJniMethod, Name) \ -#define Q_DECLARE_JNI_NATIVE_METHOD_IN_CURRENT_SCOPE(...) \ - QT_OVERLOADED_MACRO(QT_DECLARE_JNI_NATIVE_METHOD_IN_CURRENT_SCOPE, __VA_ARGS__) \ +#define QT_DECLARE_JNI_NATIVE_METHOD_IN_CURRENT_SCOPE_1(Method) \ + QT_DECLARE_JNI_NATIVE_METHOD_IN_CURRENT_SCOPE_2(Method, Method) \ -#define QT_DECLARE_JNI_NATIVE_METHOD_IN_CURRENT_SCOPE_2(Method, Name) \ - Q_DECLARE_JNI_NATIVE_METHOD_HELPER(Method) \ - static inline constexpr auto Method##_signature = QtJniTypes::nativeMethodSignature(Method); \ - static inline const JNINativeMethod Method##_method = { \ - #Name, Method##_signature.data(), reinterpret_cast(va_##Method) \ - }; - -#define QT_DECLARE_JNI_NATIVE_METHOD_IN_CURRENT_SCOPE_1(Method) \ - QT_DECLARE_JNI_NATIVE_METHOD_IN_CURRENT_SCOPE_2(Method, Method) \ - -#define Q_JNI_NATIVE_SCOPED_METHOD(Method, Scope) Scope::Method##_method +#define Q_JNI_NATIVE_SCOPED_METHOD(Method, Scope) \ + Scope::Method##_QtJniMethod::makeJNIMethod(Scope::Method) // Classes for value types Q_DECLARE_JNI_CLASS(String, "java/lang/String") diff --git a/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp b/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp index 42f113aa09d..6291c70fdeb 100644 --- a/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp +++ b/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp @@ -2192,11 +2192,9 @@ void tst_QJniObject::callback() })); result = testObject.callMethod("callMeBackWithFloat", 1.2345f); QVERIFY(calledWithFloat); - QEXPECT_FAIL("", "QTBUG-132410", Continue); QCOMPARE(calledWithFloat.value(), 1.2345f); result = testObject.callMethod("callMeBackWithFloatStatic", 1.2345f); QVERIFY(calledWithFloatStatic); - QEXPECT_FAIL("", "QTBUG-132410", Continue); QCOMPARE(calledWithFloatStatic.value(), 1.2345f); break; case CallbackParameterType::JniArray: { @@ -2257,7 +2255,6 @@ void tst_QJniObject::callback() })); result = testObject.callMethod("callMeBackWithMany"); QVERIFY(calledWithMany); - QEXPECT_FAIL("", "QTBUG-132410", Continue); QVERIFY(calledWithMany.value()); break; } diff --git a/tests/auto/corelib/kernel/qjnitypes/tst_qjnitypes.cpp b/tests/auto/corelib/kernel/qjnitypes/tst_qjnitypes.cpp index 7a4e89c6e42..3b9092d3df9 100644 --- a/tests/auto/corelib/kernel/qjnitypes/tst_qjnitypes.cpp +++ b/tests/auto/corelib/kernel/qjnitypes/tst_qjnitypes.cpp @@ -181,21 +181,22 @@ void tst_QJniTypes::nativeClassMethod(JNIEnv *, jclass, int) {} void tst_QJniTypes::nativeMethod() { + using namespace QtJniMethods; { const auto method = Q_JNI_NATIVE_METHOD(nativeFunction); - QVERIFY(method.fnPtr == QtJniMethods::va_nativeFunction); + QVERIFY(method.fnPtr == &(nativeFunction_Helper::call)); QCOMPARE(method.name, "nativeFunction"); QCOMPARE(method.signature, "(ILjava/lang/String;J)Z"); } { const auto method = Q_JNI_NATIVE_METHOD(forwardDeclaredNativeFunction); - QVERIFY(method.fnPtr == QtJniMethods::va_forwardDeclaredNativeFunction); + QVERIFY(method.fnPtr == &(forwardDeclaredNativeFunction_Helper::call)); } { const auto method = Q_JNI_NATIVE_SCOPED_METHOD(nativeClassMethod, tst_QJniTypes); - QVERIFY(method.fnPtr == va_nativeClassMethod); + QVERIFY(method.fnPtr == &(nativeClassMethod_QtJniMethod::call)); } }