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.8
Change-Id: I190b95fcbcd07cf99c6765fa426c3c351f91994a
Reviewed-by: Volker Krause <vkrause@kde.org>
(cherry picked from commit e91a17873ee4ae58d369b8eb70029cf895b31d03)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

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
+// Various helpers to forward the call to the registered function (with JNI types
-// the real function with proper type conversion. This is needed because we
+// as arguments) to the real function with proper type conversion. This is needed
-// want to write functions that take QJniObjects (subclasses), while Java
+// because we want to write functions that take QJniObjects (subclasses), while
-// can only call functions that take jobjects.
+// Java can only call functions that take jobjects.
 
-// In Var-arg functions, any argument narrower than (unsigned) int or double
+// Map any QJniObject type to jobject
-// is promoted to (unsigned) int or double.
-template <typename Arg> struct PromotedType { using Type = Arg; };
-template <> struct PromotedType<bool> { using Type = int; };
-template <> struct PromotedType<char> { using Type = int; };
-template <> struct PromotedType<signed char> { using Type = int; };
-template <> struct PromotedType<unsigned char> { using Type = unsigned int; };
-template <> struct PromotedType<short> { using Type = int; };
-template <> struct PromotedType<unsigned short> { using Type = unsigned int; };
-template <> struct PromotedType<float> { using Type = double; };
-
-// Map any QJniObject type to jobject; that's what's on the va_list
 template <typename Arg>
 struct JNITypeForArgImpl
 {
     using Type = std::conditional_t<std::disjunction_v<std::is_base_of<QJniObject, Arg>,
                                                        std::is_base_of<QtJniTypes::JObjectBase, Arg>>,
-                                    jobject, typename PromotedType<Arg>::Type>;
+                                    jobject, Arg>;
     static Arg fromVarArg(Type t)
     {
         return static_cast<Arg>(t);
@@ -196,93 +185,60 @@ public:
 
 template <typename Arg>
 using JNITypeForArg = typename JNITypeForArgImpl<std::decay_t<Arg>>::Type;
-template <typename Arg, typename Type>
-static inline auto methodArgFromVarArg(Type t) // Type comes from a va_arg, so is always POD
-{
-    return JNITypeForArgImpl<std::decay_t<Arg>>::fromVarArg(t);
-}
-
-// Turn a va_list into a tuple of typed arguments
-template <typename ...Args>
-static constexpr auto makeTupleFromArgsHelper(va_list args)
-{
-    return std::tuple(methodArgFromVarArg<Args>(va_arg(args, JNITypeForArg<Args>))...);
-}
-
-template <typename Ret, typename ...Args>
-static constexpr auto makeTupleFromArgs(Ret (*)(JNIEnv *, jobject, Args...), va_list args)
-{
-    return makeTupleFromArgsHelper<Args...>(args);
-}
-template <typename Ret, typename ...Args>
-static constexpr auto makeTupleFromArgs(Ret (*)(JNIEnv *, jclass, Args...), va_list args)
-{
-    return makeTupleFromArgsHelper<Args...>(args);
-}
-
-template <typename>
-struct NativeFunctionReturnType {};
-
-template<typename Ret, typename... Args>
-struct NativeFunctionReturnType<Ret(Args...)>
-{
-  using type = Ret;
-};
-
 } // namespace Detail
 } // namespace QtJniMethods
 
-// A va_ variadic arguments function that we register with JNI as a proxy
+// Declaring a JNI method results in a struct with a template function call() that
-// for the function we have. This function uses the helpers to unpack the
+// gets instantiated with the return type and arguments of the declared method,
-// variadic arguments into a tuple of typed arguments, which we then call
+// and registered with JNI. That template is implemented to call the declared
-// the actual function with. This then takes care of implicit conversions,
+// method, with arguments explicitly converted to the types the declared method
-// e.g. a jobject becomes a QJniObject.
+// expects (e.g. jobject becomes QJniObject, a QString, a QList etc).
-#define Q_DECLARE_JNI_NATIVE_METHOD_HELPER(Method)                              \
+#define Q_DECLARE_JNI_NATIVE_METHOD_HELPER(Method, Postfix, Name)                           \
-static QtJniMethods::Detail::NativeFunctionReturnType<decltype(Method)>::type   \
+struct Method##_##Postfix {                                                                 \
-va_##Method(JNIEnv *env, jclass thiz, ...)                                      \
+template<typename Ret, typename JType, typename... Args>                                    \
+JNICALL static                                                                              \
+Ret call(JNIEnv *env, JType thiz, QtJniMethods::Detail::JNITypeForArg<Args> ...args)        \
 {                                                                                           \
-    va_list args;                                                               \
+    return Method(env, thiz, QtJniMethods::Detail::JNITypeForArgImpl<                       \
-    va_start(args, thiz);                                                       \
+                                                std::decay_t<Args>>::fromVarArg(args)...    \
-    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);                                                               \
 }                                                                                           \
+static constexpr auto signature = QtJniTypes::nativeMethodSignature(Method);                \
+template<typename Ret, typename JType, typename ...Args>                                    \
+static constexpr JNINativeMethod makeJNIMethod(Ret(*)(JNIEnv *, JType, Args...))            \
+{                                                                                           \
+    return JNINativeMethod {                                                                \
+        #Name, signature.data(),                                                            \
+        reinterpret_cast<void *>(&call<Ret, JType, Args...>)                                \
+    };                                                                                      \
+}                                                                                           \
+};                                                                                          \
 
 #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)                      \
+Q_DECLARE_JNI_NATIVE_METHOD_HELPER(Method, Helper, Name)                        \
-static constexpr auto Method##_signature =                      \
-    QtJniTypes::nativeMethodSignature(Method);                  \
-static const JNINativeMethod Method##_method = {                \
-    #Name, Method##_signature.data(),                           \
-    reinterpret_cast<void *>(va_##Method)                       \
-};                                                              \
 }                                                                               \
 
 #define QT_DECLARE_JNI_NATIVE_METHOD_1(Method)                                  \
     QT_DECLARE_JNI_NATIVE_METHOD_2(Method, Method)                              \
 
-#define Q_JNI_NATIVE_METHOD(Method) QtJniMethods::Method##_method
+#define Q_JNI_NATIVE_METHOD(Method)                                             \
+    QtJniMethods::Method##_Helper::makeJNIMethod(::Method)
 
 #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_2(Method, Name)                       \
-    Q_DECLARE_JNI_NATIVE_METHOD_HELPER(Method)                                                   \
+Q_DECLARE_JNI_NATIVE_METHOD_HELPER(Method, QtJniMethod, Name)                               \
-    static inline constexpr auto Method##_signature = QtJniTypes::nativeMethodSignature(Method); \
-    static inline const JNINativeMethod Method##_method = {                                      \
-        #Name, Method##_signature.data(), reinterpret_cast<void *>(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")

tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp

@@ -2192,11 +2192,9 @@ void tst_QJniObject::callback()
         }));
         result = testObject.callMethod<int>("callMeBackWithFloat", 1.2345f);
         QVERIFY(calledWithFloat);
-        QEXPECT_FAIL("", "QTBUG-132410", Continue);
         QCOMPARE(calledWithFloat.value(), 1.2345f);
         result = testObject.callMethod<int>("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<int>("callMeBackWithMany");
         QVERIFY(calledWithMany);
-        QEXPECT_FAIL("", "QTBUG-132410", Continue);
         QVERIFY(calledWithMany.value());
         break;
     }

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<bool, jclass, int, jstring, quint64>));
         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<int, jobject, bool>));
     }
 
     {
         const auto method = Q_JNI_NATIVE_SCOPED_METHOD(nativeClassMethod, tst_QJniTypes);
-        QVERIFY(method.fnPtr == va_nativeClassMethod);
+        QVERIFY(method.fnPtr == &(nativeClassMethod_QtJniMethod::call<void, jclass, int>));
     }
 }