Loosen check:jni around GetStatic[...]Field
-Xcheck:jni was requiring that the jclass being passed to
GetStatic[...]Field was the exact same class as the jfieldID's
declaring class. This was stricter than is really needed and causes
some issues when dealing with fields where the declaring class cannot
be easily determined.
Bug: 70532839
Test: ./test.py --host -j50
Change-Id: I0987de09af956ed9a8dde37c50606604fdd94b87
diff --git a/runtime/check_jni.cc b/runtime/check_jni.cc
index 0b9bf22..90f478f 100644
--- a/runtime/check_jni.cc
+++ b/runtime/check_jni.cc
@@ -373,7 +373,7 @@
if (f == nullptr) {
return false;
}
- if (c != f->GetDeclaringClass()) {
+ if (!f->GetDeclaringClass()->IsAssignableFrom(c)) {
AbortF("static jfieldID %p not valid for class %s", fid,
mirror::Class::PrettyClass(c).c_str());
return false;
@@ -710,7 +710,7 @@
return false;
}
ObjPtr<mirror::Class> c = o->AsClass();
- if (c != field->GetDeclaringClass()) {
+ if (!field->GetDeclaringClass()->IsAssignableFrom(c)) {
AbortF("attempt to access static field %s with an incompatible class argument of %s: %p",
field->PrettyField().c_str(), mirror::Class::PrettyDescriptor(c).c_str(), fid);
return false;
diff --git a/runtime/jni_internal_test.cc b/runtime/jni_internal_test.cc
index 1ecfe7c..efeff0a 100644
--- a/runtime/jni_internal_test.cc
+++ b/runtime/jni_internal_test.cc
@@ -1783,66 +1783,115 @@
EXPECT_TRUE(vm_->SetCheckJniEnabled(old_check_jni)); \
} while (false)
+#define TEST_PRIMITIVE_FIELD_FOR_CLASS(cname) \
+ do { \
+ Thread::Current()->TransitionFromSuspendedToRunnable(); \
+ LoadDex("AllFields"); \
+ bool started = runtime_->Start(); \
+ ASSERT_TRUE(started); \
+ jclass c = env_->FindClass(cname); \
+ ASSERT_NE(c, nullptr); \
+ jobject o = env_->AllocObject(c); \
+ ASSERT_NE(o, nullptr); \
+ \
+ EXPECT_STATIC_PRIMITIVE_FIELD(EXPECT_EQ, Boolean, "sZ", "Z", JNI_TRUE, JNI_FALSE); \
+ EXPECT_STATIC_PRIMITIVE_FIELD(EXPECT_EQ, Byte, "sB", "B", 1, 2); \
+ EXPECT_STATIC_PRIMITIVE_FIELD(EXPECT_EQ, Char, "sC", "C", 'a', 'b'); \
+ EXPECT_STATIC_PRIMITIVE_FIELD(EXPECT_DOUBLE_EQ, Double, "sD", "D", 1.0, 2.0); \
+ EXPECT_STATIC_PRIMITIVE_FIELD(EXPECT_FLOAT_EQ, Float, "sF", "F", 1.0, 2.0); \
+ EXPECT_STATIC_PRIMITIVE_FIELD(EXPECT_EQ, Int, "sI", "I", 1, 2); \
+ EXPECT_STATIC_PRIMITIVE_FIELD(EXPECT_EQ, Long, "sJ", "J", 1, 2); \
+ EXPECT_STATIC_PRIMITIVE_FIELD(EXPECT_EQ, Short, "sS", "S", 1, 2); \
+ \
+ EXPECT_PRIMITIVE_FIELD(EXPECT_EQ, o, Boolean, "iZ", "Z", JNI_TRUE, JNI_FALSE); \
+ EXPECT_PRIMITIVE_FIELD(EXPECT_EQ, o, Byte, "iB", "B", 1, 2); \
+ EXPECT_PRIMITIVE_FIELD(EXPECT_EQ, o, Char, "iC", "C", 'a', 'b'); \
+ EXPECT_PRIMITIVE_FIELD(EXPECT_DOUBLE_EQ, o, Double, "iD", "D", 1.0, 2.0); \
+ EXPECT_PRIMITIVE_FIELD(EXPECT_FLOAT_EQ, o, Float, "iF", "F", 1.0, 2.0); \
+ EXPECT_PRIMITIVE_FIELD(EXPECT_EQ, o, Int, "iI", "I", 1, 2); \
+ EXPECT_PRIMITIVE_FIELD(EXPECT_EQ, o, Long, "iJ", "J", 1, 2); \
+ EXPECT_PRIMITIVE_FIELD(EXPECT_EQ, o, Short, "iS", "S", 1, 2); \
+ } while (false)
TEST_F(JniInternalTest, GetPrimitiveField_SetPrimitiveField) {
+ TEST_PRIMITIVE_FIELD_FOR_CLASS("AllFields");
+}
+
+TEST_F(JniInternalTest, GetPrimitiveField_SetPrimitiveField_Subclass) {
+ TEST_PRIMITIVE_FIELD_FOR_CLASS("AllFieldsSub");
+}
+
+#define EXPECT_UNRELATED_FIELD_FAILURE(type, field_name, sig, value1) \
+ do { \
+ jfieldID fid = env_->GetStaticFieldID(c, field_name, sig); \
+ EXPECT_NE(fid, nullptr); \
+ CheckJniAbortCatcher jni_abort_catcher; \
+ env_->Get ## type ## Field(uc, fid); \
+ jni_abort_catcher.Check("not valid for an object of class"); \
+ env_->Set ## type ## Field(uc, fid, value1); \
+ jni_abort_catcher.Check("not valid for an object of class"); \
+ } while (false)
+
+TEST_F(JniInternalTest, GetField_SetField_unrelated) {
Thread::Current()->TransitionFromSuspendedToRunnable();
LoadDex("AllFields");
bool started = runtime_->Start();
ASSERT_TRUE(started);
-
jclass c = env_->FindClass("AllFields");
ASSERT_NE(c, nullptr);
- jobject o = env_->AllocObject(c);
- ASSERT_NE(o, nullptr);
-
- EXPECT_STATIC_PRIMITIVE_FIELD(EXPECT_EQ, Boolean, "sZ", "Z", JNI_TRUE, JNI_FALSE);
- EXPECT_STATIC_PRIMITIVE_FIELD(EXPECT_EQ, Byte, "sB", "B", 1, 2);
- EXPECT_STATIC_PRIMITIVE_FIELD(EXPECT_EQ, Char, "sC", "C", 'a', 'b');
- EXPECT_STATIC_PRIMITIVE_FIELD(EXPECT_DOUBLE_EQ, Double, "sD", "D", 1.0, 2.0);
- EXPECT_STATIC_PRIMITIVE_FIELD(EXPECT_FLOAT_EQ, Float, "sF", "F", 1.0, 2.0);
- EXPECT_STATIC_PRIMITIVE_FIELD(EXPECT_EQ, Int, "sI", "I", 1, 2);
- EXPECT_STATIC_PRIMITIVE_FIELD(EXPECT_EQ, Long, "sJ", "J", 1, 2);
- EXPECT_STATIC_PRIMITIVE_FIELD(EXPECT_EQ, Short, "sS", "S", 1, 2);
-
- EXPECT_PRIMITIVE_FIELD(EXPECT_EQ, o, Boolean, "iZ", "Z", JNI_TRUE, JNI_FALSE);
- EXPECT_PRIMITIVE_FIELD(EXPECT_EQ, o, Byte, "iB", "B", 1, 2);
- EXPECT_PRIMITIVE_FIELD(EXPECT_EQ, o, Char, "iC", "C", 'a', 'b');
- EXPECT_PRIMITIVE_FIELD(EXPECT_DOUBLE_EQ, o, Double, "iD", "D", 1.0, 2.0);
- EXPECT_PRIMITIVE_FIELD(EXPECT_FLOAT_EQ, o, Float, "iF", "F", 1.0, 2.0);
- EXPECT_PRIMITIVE_FIELD(EXPECT_EQ, o, Int, "iI", "I", 1, 2);
- EXPECT_PRIMITIVE_FIELD(EXPECT_EQ, o, Long, "iJ", "J", 1, 2);
- EXPECT_PRIMITIVE_FIELD(EXPECT_EQ, o, Short, "iS", "S", 1, 2);
+ jclass uc = env_->FindClass("AllFieldsUnrelated");
+ ASSERT_NE(uc, nullptr);
+ bool old_check_jni = vm_->SetCheckJniEnabled(true);
+ EXPECT_UNRELATED_FIELD_FAILURE(Boolean, "sZ", "Z", JNI_TRUE);
+ EXPECT_UNRELATED_FIELD_FAILURE(Byte, "sB", "B", 1);
+ EXPECT_UNRELATED_FIELD_FAILURE(Char, "sC", "C", 'a');
+ EXPECT_UNRELATED_FIELD_FAILURE(Double, "sD", "D", 1.0);
+ EXPECT_UNRELATED_FIELD_FAILURE(Float, "sF", "F", 1.0);
+ EXPECT_UNRELATED_FIELD_FAILURE(Int, "sI", "I", 1);
+ EXPECT_UNRELATED_FIELD_FAILURE(Long, "sJ", "J", 1);
+ EXPECT_UNRELATED_FIELD_FAILURE(Short, "sS", "S", 1);
+ EXPECT_UNRELATED_FIELD_FAILURE(Object, "sObject", "Ljava/lang/Object;", c);
+ EXPECT_TRUE(vm_->SetCheckJniEnabled(old_check_jni));
}
+#define TEST_OBJECT_FIELD_FOR_CLASS(cname) \
+ do { \
+ Thread::Current()->TransitionFromSuspendedToRunnable(); \
+ LoadDex("AllFields"); \
+ runtime_->Start(); \
+ \
+ jclass c = env_->FindClass(cname); \
+ ASSERT_NE(c, nullptr); \
+ jobject o = env_->AllocObject(c); \
+ ASSERT_NE(o, nullptr); \
+ \
+ jstring s1 = env_->NewStringUTF("hello"); \
+ ASSERT_NE(s1, nullptr); \
+ jstring s2 = env_->NewStringUTF("world"); \
+ ASSERT_NE(s2, nullptr); \
+ \
+ jfieldID s_fid = env_->GetStaticFieldID(c, "sObject", "Ljava/lang/Object;"); \
+ ASSERT_NE(s_fid, nullptr); \
+ jfieldID i_fid = env_->GetFieldID(c, "iObject", "Ljava/lang/Object;"); \
+ ASSERT_NE(i_fid, nullptr); \
+ \
+ env_->SetStaticObjectField(c, s_fid, s1); \
+ ASSERT_TRUE(env_->IsSameObject(s1, env_->GetStaticObjectField(c, s_fid))); \
+ env_->SetStaticObjectField(c, s_fid, s2); \
+ ASSERT_TRUE(env_->IsSameObject(s2, env_->GetStaticObjectField(c, s_fid))); \
+ \
+ env_->SetObjectField(o, i_fid, s1); \
+ ASSERT_TRUE(env_->IsSameObject(s1, env_->GetObjectField(o, i_fid))); \
+ env_->SetObjectField(o, i_fid, s2); \
+ ASSERT_TRUE(env_->IsSameObject(s2, env_->GetObjectField(o, i_fid))); \
+ } while (false)
+
TEST_F(JniInternalTest, GetObjectField_SetObjectField) {
- Thread::Current()->TransitionFromSuspendedToRunnable();
- LoadDex("AllFields");
- runtime_->Start();
+ TEST_OBJECT_FIELD_FOR_CLASS("AllFields");
+}
- jclass c = env_->FindClass("AllFields");
- ASSERT_NE(c, nullptr);
- jobject o = env_->AllocObject(c);
- ASSERT_NE(o, nullptr);
-
- jstring s1 = env_->NewStringUTF("hello");
- ASSERT_NE(s1, nullptr);
- jstring s2 = env_->NewStringUTF("world");
- ASSERT_NE(s2, nullptr);
-
- jfieldID s_fid = env_->GetStaticFieldID(c, "sObject", "Ljava/lang/Object;");
- ASSERT_NE(s_fid, nullptr);
- jfieldID i_fid = env_->GetFieldID(c, "iObject", "Ljava/lang/Object;");
- ASSERT_NE(i_fid, nullptr);
-
- env_->SetStaticObjectField(c, s_fid, s1);
- ASSERT_TRUE(env_->IsSameObject(s1, env_->GetStaticObjectField(c, s_fid)));
- env_->SetStaticObjectField(c, s_fid, s2);
- ASSERT_TRUE(env_->IsSameObject(s2, env_->GetStaticObjectField(c, s_fid)));
-
- env_->SetObjectField(o, i_fid, s1);
- ASSERT_TRUE(env_->IsSameObject(s1, env_->GetObjectField(o, i_fid)));
- env_->SetObjectField(o, i_fid, s2);
- ASSERT_TRUE(env_->IsSameObject(s2, env_->GetObjectField(o, i_fid)));
+TEST_F(JniInternalTest, GetObjectField_SetObjectField_subclass) {
+ TEST_OBJECT_FIELD_FOR_CLASS("AllFieldsSub");
}
TEST_F(JniInternalTest, NewLocalRef_nullptr) {
diff --git a/test/004-JniTest/expected.txt b/test/004-JniTest/expected.txt
index 1d05160..b09b9a2 100644
--- a/test/004-JniTest/expected.txt
+++ b/test/004-JniTest/expected.txt
@@ -1,4 +1,5 @@
JNI_OnLoad called
+ABC.XYZ = 12, GetStaticIntField(DEF.class, 'XYZ') = 12
Super.<init>
Super.<init>
Subclass.<init>
diff --git a/test/004-JniTest/jni_test.cc b/test/004-JniTest/jni_test.cc
index 4561895..33a8f5b 100644
--- a/test/004-JniTest/jni_test.cc
+++ b/test/004-JniTest/jni_test.cc
@@ -90,6 +90,14 @@
CHECK(!env->ExceptionCheck());
}
+extern "C" JNIEXPORT jint JNICALL Java_Main_getFieldSubclass(JNIEnv* env,
+ jclass,
+ jobject f_obj,
+ jclass sub) {
+ jfieldID f = env->FromReflectedField(f_obj);
+ return env->GetStaticIntField(sub, f);
+}
+
// http://b/10994325
extern "C" JNIEXPORT void JNICALL Java_Main_testFindClassOnAttachedNativeThread(JNIEnv*, jclass) {
PthreadHelper(&testFindClassOnAttachedNativeThread);
diff --git a/test/004-JniTest/src/Main.java b/test/004-JniTest/src/Main.java
index 871107c..f94dcf6 100644
--- a/test/004-JniTest/src/Main.java
+++ b/test/004-JniTest/src/Main.java
@@ -18,6 +18,7 @@
import java.lang.reflect.InvocationHandler;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
+import java.lang.reflect.Field;
import java.lang.reflect.Proxy;
import java.util.regex.Pattern;
@@ -32,6 +33,7 @@
throw new RuntimeException("Slow-debug flags unexpectedly off.");
}
+ testFieldSubclass();
testFindClassOnAttachedNativeThread();
testFindFieldOnAttachedNativeThread();
testReflectFieldGetFromAttachedNativeThreadNative();
@@ -65,6 +67,19 @@
testDoubleLoad(args[0]);
}
+ static class ABC { public static int XYZ = 12; }
+ static class DEF extends ABC {}
+ public static void testFieldSubclass() {
+ try {
+ System.out.println("ABC.XYZ = " + ABC.XYZ + ", GetStaticIntField(DEF.class, 'XYZ') = " +
+ getFieldSubclass(ABC.class.getDeclaredField("XYZ"), DEF.class));
+ } catch (Exception e) {
+ throw new RuntimeException("Failed to test get static field on a subclass", e);
+ }
+ }
+
+ public static native int getFieldSubclass(Field f, Class sub);
+
private static native boolean registerNativesJniTest();
private static native void testCallDefaultMethods();
diff --git a/test/AllFields/AllFields.java b/test/AllFields/AllFields.java
index d5eac8f..24f8ba1 100644
--- a/test/AllFields/AllFields.java
+++ b/test/AllFields/AllFields.java
@@ -14,7 +14,7 @@
* limitations under the License.
*/
-class AllFields {
+public class AllFields {
static boolean sZ;
static byte sB;
static char sC;
diff --git a/test/AllFields/AllFieldsSub.java b/test/AllFields/AllFieldsSub.java
new file mode 100644
index 0000000..d5f933f
--- /dev/null
+++ b/test/AllFields/AllFieldsSub.java
@@ -0,0 +1,17 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+public class AllFieldsSub extends AllFields { }
diff --git a/test/AllFields/AllFieldsUnrelated.java b/test/AllFields/AllFieldsUnrelated.java
new file mode 100644
index 0000000..4db66b1
--- /dev/null
+++ b/test/AllFields/AllFieldsUnrelated.java
@@ -0,0 +1,17 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+public class AllFieldsUnrelated { }