Fix field access.
There was a bug in the implementation where the native methods weren't
necessarily getting a null receiver but art::Object insists that you only
get/set static fields on a null receiver.
Also, these tests assume that fields are in the setAccessible(false) state
by default. In our system, they're not.
Change-Id: I935614fec013be465fc0bc30d3c717f12476e09b
diff --git a/src/java_lang_reflect_Field.cc b/src/java_lang_reflect_Field.cc
index 8e09adb..2937241 100644
--- a/src/java_lang_reflect_Field.cc
+++ b/src/java_lang_reflect_Field.cc
@@ -71,17 +71,25 @@
return false;
}
+bool CheckReceiver(JNIEnv* env, jobject javaObj, jclass javaDeclaringClass, Field* f, Object*& o) {
+ if (f->IsStatic()) {
+ o = NULL;
+ return true;
+ }
+
+ o = Decode<Object*>(env, javaObj);
+ Class* declaringClass = Decode<Class*>(env, javaDeclaringClass);
+ if (!VerifyObjectInClass(env, o, declaringClass)) {
+ return false;
+ }
+ return true;
+}
+
JValue GetPrimitiveField(JNIEnv* env, jobject javaField, jobject javaObj, jclass javaDeclaringClass, jchar targetDescriptor) {
Field* f = DecodeField(env->FromReflectedField(javaField));
-
- // Check that the receiver is non-null and an instance of the field's declaring class.
- Object* o = Decode<Object*>(env, javaObj);
- bool isStatic = (javaObj == NULL);
- if (!isStatic) {
- Class* declaringClass = Decode<Class*>(env, javaDeclaringClass);
- if (!VerifyObjectInClass(env, o, declaringClass)) {
- return JValue();
- }
+ Object* o = NULL;
+ if (!CheckReceiver(env, javaObj, javaDeclaringClass, f, o)) {
+ return JValue();
}
// Read the value.
@@ -179,15 +187,9 @@
void SetPrimitiveField(JNIEnv* env, jobject javaField, jobject javaObj, jclass javaDeclaringClass, jchar targetDescriptor, const JValue& new_value) {
Field* f = DecodeField(env->FromReflectedField(javaField));
-
- // Check that the receiver is non-null and an instance of the field's declaring class.
- Object* o = Decode<Object*>(env, javaObj);
- bool isStatic = (javaObj == NULL);
- if (!isStatic) {
- Class* declaringClass = Decode<Class*>(env, javaDeclaringClass);
- if (!VerifyObjectInClass(env, o, declaringClass)) {
- return;
- }
+ Object* o = NULL;
+ if (!CheckReceiver(env, javaObj, javaDeclaringClass, f, o)) {
+ return;
}
// Widen the value if necessary (and possible).
@@ -260,13 +262,9 @@
}
// Check that the receiver is non-null and an instance of the field's declaring class.
- Object* o = Decode<Object*>(env, javaObj);
- bool isStatic = (javaObj == NULL);
- if (!isStatic) {
- Class* declaringClass = Decode<Class*>(env, javaDeclaringClass);
- if (!VerifyObjectInClass(env, o, declaringClass)) {
- return;
- }
+ Object* o = NULL;
+ if (!CheckReceiver(env, javaObj, javaDeclaringClass, f, o)) {
+ return;
}
SetFieldValue(o, f, unboxed_value, true);
@@ -274,15 +272,9 @@
jobject Field_getField(JNIEnv* env, jobject javaField, jobject javaObj, jclass javaDeclaringClass, jclass, jint, jboolean) {
Field* f = DecodeField(env->FromReflectedField(javaField));
-
- // Check that the receiver is non-null and an instance of the field's declaring class.
- Object* o = Decode<Object*>(env, javaObj);
- bool isStatic = (javaObj == NULL);
- if (!isStatic) {
- Class* declaringClass = Decode<Class*>(env, javaDeclaringClass);
- if (!VerifyObjectInClass(env, o, declaringClass)) {
- return NULL;
- }
+ Object* o = NULL;
+ if (!CheckReceiver(env, javaObj, javaDeclaringClass, f, o)) {
+ return NULL;
}
// Get the field's value, boxing if necessary.
diff --git a/test/064-field-access/src/Main.java b/test/064-field-access/src/Main.java
index c068d23..cbf6b71 100644
--- a/test/064-field-access/src/Main.java
+++ b/test/064-field-access/src/Main.java
@@ -175,6 +175,8 @@
throw new RuntimeException(nsfe);
}
+ Class<?> accessExceptionClass = null; // art deliberately doesn't throw IllegalAccessException.
+
/*
* Get a public field from a class in the same package.
*/
@@ -188,8 +190,7 @@
/*
* Get a private field from a class in the same package.
*/
- this.getValue(localPrivFloatField, samePkgInst, 'F',
- IllegalAccessException.class);
+ this.getValue(localPrivFloatField, samePkgInst, 'F', accessExceptionClass);
/*
* Get a protected field from otherInst's superclass.
@@ -200,78 +201,51 @@
* of OtherPackage does not allow us to modify protected fields in
* all other subclasses of OtherPackage.
*/
- this.getValue(otherProtShortField, this, 'S',
- null);
- this.getValue(otherProtShortField, otherPkgInst, 'S',
- IllegalAccessException.class);
- this.getValue(otherPkgDoubleField, otherPkgInst, 'D',
- IllegalAccessException.class);
+ this.getValue(otherProtShortField, this, 'S', null);
+ this.getValue(otherProtShortField, otherPkgInst, 'S', accessExceptionClass);
+ this.getValue(otherPkgDoubleField, otherPkgInst, 'D', accessExceptionClass);
/*
- * Null object. Different exceptions based on which package
- * we would be trying to access and whether or not our object
- * has the correct type.
+ * Null object.
*/
- localInst.getValue(localPubByteField, null, 'B',
- NullPointerException.class);
+ localInst.getValue(localPubByteField, null, 'B', NullPointerException.class);
- this.getValue(subProtLongField, null, 'J',
- NullPointerException.class);
+ this.getValue(subProtLongField, null, 'J', NullPointerException.class);
- this.getValue(localPrivFloatField, null, 'F',
- IllegalAccessException.class);
+ this.getValue(localPrivFloatField, null, 'F', NullPointerException.class);
- localInst.getValue(otherProtShortField, null, 'S',
- IllegalAccessException.class);
- this.getValue(otherProtShortField, null, 'S',
- IllegalAccessException.class);
- this.getValue(otherPkgDoubleField, null, 'D',
- IllegalAccessException.class);
+ localInst.getValue(otherProtShortField, null, 'S', NullPointerException.class);
+ this.getValue(otherProtShortField, null, 'S', NullPointerException.class);
+ this.getValue(otherPkgDoubleField, null, 'D', NullPointerException.class);
- localInst.getValue(otherProtShortField, null, 'Z',
- IllegalAccessException.class);
- /* -- Dalvik VM currently throws NPE
- this.getValue(subProtLongField, null, 'Z',
- IllegalArgumentException.class);
- */
+ localInst.getValue(otherProtShortField, null, 'Z', NullPointerException.class);
+ this.getValue(subProtLongField, null, 'Z', NullPointerException.class);
/*
* Valid object, wrong field type.
*/
- this.getValue(subProtLongField, this, 'J',
- null);
- this.getValue(localProtByteField, samePkgInst, 'Z',
- IllegalArgumentException.class);
- this.getValue(subProtLongField, this, 'Z',
- IllegalArgumentException.class);
- this.getValue(localPrivFloatField, this, 'Z',
- IllegalAccessException.class);
- this.getValue(localPrivFloatField, this, 'Z',
- IllegalAccessException.class);
- localInst.getValue(otherProtShortField, otherPkgInst, 'Z',
- IllegalAccessException.class);
- this.getValue(otherProtShortField, otherPkgInst, 'Z',
- IllegalAccessException.class);
+ this.getValue(subProtLongField, this, 'J', null);
+ this.getValue(localProtByteField, samePkgInst, 'Z', IllegalArgumentException.class);
+ this.getValue(subProtLongField, this, 'Z', IllegalArgumentException.class);
+ this.getValue(localPrivFloatField, this, 'Z', IllegalArgumentException.class);
+ this.getValue(localPrivFloatField, this, 'Z', IllegalArgumentException.class);
+ localInst.getValue(otherProtShortField, otherPkgInst, 'Z', IllegalArgumentException.class);
+ this.getValue(otherProtShortField, otherPkgInst, 'Z', IllegalArgumentException.class);
/*
* Wrong object.
*/
- this.getValue(subProtLongField, plainObj, 'J',
- IllegalArgumentException.class);
+ this.getValue(subProtLongField, plainObj, 'J', IllegalArgumentException.class);
/* wrong object + private field */
- this.getValue(localPrivFloatField, plainObj, 'F',
- IllegalAccessException.class);
+ this.getValue(localPrivFloatField, plainObj, 'F', IllegalArgumentException.class);
/* wrong object + wrong field type */
- this.getValue(subProtLongField, plainObj, 'Z',
- IllegalArgumentException.class);
+ this.getValue(subProtLongField, plainObj, 'Z', IllegalArgumentException.class);
/* wrong object + invalid access */
- localInst.getValue(otherProtShortField, plainObj, 'S',
- IllegalAccessException.class);
- this.getValue(otherProtShortField, plainObj, 'S',
- IllegalAccessException.class);
+ localInst.getValue(otherProtShortField, plainObj, 'S', IllegalArgumentException.class);
+ this.getValue(otherProtShortField, plainObj, 'S', IllegalArgumentException.class);
System.out.println("good");
}