Skip to content

[BUG] jni::Class/Object::Call() blindly passes arguments to JNI's variable length argument methods #60

@krwc

Description

@krwc

Passing int to a method that takes jlong likely leads to random stack memory being accessed and thus random trash being passed around.

For example, this is the problematic code:

struct Duration {
    static constexpr auto Name() {
        return "java/time/Duration";
    }
};

auto clazz = jni::Class<Duration>::Find(env);
auto of_seconds = clazz.template GetStaticMethod<jni::Object<Duration>(jni::jlong, jni::jlong)>(env, "ofSeconds");
auto duration = clazz.Call(env, of_seconds, 0, 0); // note the integers rather than jlongs here

auto get_seconds = clazz.template GetMethod<jni::jlong()>(env, "getSeconds");
auto seconds = duration.Call(env, get_seconds); // which is now some random value

Of course jni::Class::Call has some compile-time checks for type convertability, but that's simply not enough, because, some stacks are aligned to 4 bytes, and some are aligned to 8 bytes, and when the alignment is less than sizeof(jlong) things start to get pretty messed up. At least that's the reason of this behavior in my opinion.

Simple fix to the code above is to replace:

auto duration = clazz.Call(env, of_seconds, 0, 0); // note the integers rather than jlongs here

with:

auto duration = clazz.Call(env, of_seconds, jni::jlong{0}, jni::jlong{0});

But that's definitely not ideal. As a temporary workaround I've applied the following patch to class.hpp, which seem to compile and do whatever I wanted (in this specific scenario, but this doesn't solve the problem globally as there are many other Call like functions), but don't know if that's actually the best possible solution:

diff --git a/include/jni/class.hpp b/include/jni/class.hpp
index 6cf96c2..48460a1 100644
--- a/include/jni/class.hpp
+++ b/include/jni/class.hpp
@@ -74,7 +74,7 @@ namespace jni
                -> std::enable_if_t< IsPrimitive<R>::value
                                  && Conjunction<std::is_convertible<const ActualArgs&, const ExpectedArgs&>...>::value, R >
                {
-                return CallStaticMethod<R>(env, *this->get(), method, Untag(args)...);
+                return CallStaticMethod<R>(env, *this->get(), method, Untag(static_cast<const ExpectedArgs&>(args))...);
                }
 
             template < class R, class... ExpectedArgs, class... ActualArgs >
@@ -83,14 +83,14 @@ namespace jni
                                  && !std::is_void<R>::value
                                  && Conjunction<std::is_convertible<const ActualArgs&, const ExpectedArgs&>...>::value, Local<R> >
                {
-                return Local<R>(env, reinterpret_cast<typename R::UntaggedType*>(CallStaticMethod<jobject*>(env, *this->get(), method, Untag(args)...)));
+                return Local<R>(env, reinterpret_cast<typename R::UntaggedType*>(CallStaticMethod<jobject*>(env, *this->get(), method, Untag(static_cast<const ExpectedArgs&>(args))...)));
                }
 
             template < class... ExpectedArgs, class... ActualArgs >
             auto Call(JNIEnv& env, const StaticMethod<TagType, void (ExpectedArgs...)>& method, const ActualArgs&... args) const
                -> std::enable_if_t< Conjunction<std::is_convertible<const ActualArgs&, const ExpectedArgs&>...>::value >
                {
-                CallStaticMethod<void>(env, *this->get(), method, Untag(args)...);
+                CallStaticMethod<void>(env, *this->get(), method, Untag(static_cast<const ExpectedArgs&>(args))...);
                }
 
             static Local<Class> Find(JNIEnv& env)

Anyway, I'll try to make it into pull request some time if nobody's gonna do it before me. ;-)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions