From ca239af73e512df5eeb80fe6c09c2ca614649e06 Mon Sep 17 00:00:00 2001 From: Mathieu Chartier Date: Sun, 29 Mar 2015 18:27:50 -0700 Subject: [PATCH] Fix some reflection errors Fixed incorrectly using 2nd frame instead of 1st in VerifyAccess. Added regression test to ART. Fixed broken setShort, getDeclaredFieldInternal. Change-Id: I4b21d52d998cb768fe9503b8bccec506b7b972e5 --- runtime/native/java_lang_Class.cc | 5 ++- runtime/native/java_lang_reflect_Field.cc | 6 +-- runtime/reflection.cc | 6 +-- runtime/reflection.h | 2 +- test/046-reflect/src/Main.java | 65 ++++++++++++++++++++----------- 5 files changed, 53 insertions(+), 31 deletions(-) diff --git a/runtime/native/java_lang_Class.cc b/runtime/native/java_lang_Class.cc index 3cb6b367f..0ca9d2482 100644 --- a/runtime/native/java_lang_Class.cc +++ b/runtime/native/java_lang_Class.cc @@ -253,7 +253,10 @@ static jobject Class_getDeclaredField(JNIEnv* env, jobject javaThis, jstring nam mirror::Field* result = GetDeclaredField(soa.Self(), klass, name_string); if (result == nullptr) { std::string name_str = name_string->ToModifiedUtf8(); - soa.Self()->ThrowNewException("Ljava/lang/NoSuchFieldException;", name_str.c_str()); + // We may have a pending exception if we failed to resolve. + if (!soa.Self()->IsExceptionPending()) { + soa.Self()->ThrowNewException("Ljava/lang/NoSuchFieldException;", name_str.c_str()); + } return nullptr; } return soa.AddLocalReference(result); diff --git a/runtime/native/java_lang_reflect_Field.cc b/runtime/native/java_lang_reflect_Field.cc index 0fe78b328..721b7a3b7 100644 --- a/runtime/native/java_lang_reflect_Field.cc +++ b/runtime/native/java_lang_reflect_Field.cc @@ -44,7 +44,7 @@ ALWAYS_INLINE inline static bool VerifyFieldAccess(Thread* self, mirror::Field* } mirror::Class* calling_class = nullptr; if (!VerifyAccess(self, obj, field->GetDeclaringClass(), field->GetAccessFlags(), - &calling_class)) { + &calling_class, 1)) { ThrowIllegalAccessException( StringPrintf("Class %s cannot access %s field %s of class %s", calling_class == nullptr ? "null" : PrettyClass(calling_class).c_str(), @@ -276,9 +276,9 @@ ALWAYS_INLINE inline static void SetFieldValue(mirror::Object* o, mirror::Field* break; case Primitive::kPrimShort: if (is_volatile) { - o->SetFieldShortVolatile(offset, new_value.GetZ()); + o->SetFieldShortVolatile(offset, new_value.GetS()); } else { - o->SetFieldShort(offset, new_value.GetZ()); + o->SetFieldShort(offset, new_value.GetS()); } break; case Primitive::kPrimNot: diff --git a/runtime/reflection.cc b/runtime/reflection.cc index d845e402e..4e94de413 100644 --- a/runtime/reflection.cc +++ b/runtime/reflection.cc @@ -587,7 +587,7 @@ jobject InvokeMethod(const ScopedObjectAccessAlreadyRunnable& soa, jobject javaM // If method is not set to be accessible, verify it can be accessed by the caller. mirror::Class* calling_class = nullptr; if (!accessible && !VerifyAccess(soa.Self(), receiver, declaring_class, m->GetAccessFlags(), - &calling_class)) { + &calling_class, 2)) { ThrowIllegalAccessException( StringPrintf("Class %s cannot access %s method %s of class %s", calling_class == nullptr ? "null" : PrettyClass(calling_class).c_str(), @@ -794,11 +794,11 @@ bool UnboxPrimitiveForResult(mirror::Object* o, } bool VerifyAccess(Thread* self, mirror::Object* obj, mirror::Class* declaring_class, - uint32_t access_flags, mirror::Class** calling_class) { + uint32_t access_flags, mirror::Class** calling_class, size_t num_frames) { if ((access_flags & kAccPublic) != 0) { return true; } - NthCallerVisitor visitor(self, 2); + NthCallerVisitor visitor(self, num_frames); visitor.WalkStack(); if (UNLIKELY(visitor.caller == nullptr)) { // The caller is an attached native thread. diff --git a/runtime/reflection.h b/runtime/reflection.h index 6bef664ca..ff970e550 100644 --- a/runtime/reflection.h +++ b/runtime/reflection.h @@ -73,7 +73,7 @@ ALWAYS_INLINE bool VerifyObjectIsClass(mirror::Object* o, mirror::Class* c) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); bool VerifyAccess(Thread* self, mirror::Object* obj, mirror::Class* declaring_class, - uint32_t access_flags, mirror::Class** calling_class) + uint32_t access_flags, mirror::Class** calling_class, size_t num_frames) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); void InvalidReceiverError(mirror::Object* o, mirror::Class* c) diff --git a/test/046-reflect/src/Main.java b/test/046-reflect/src/Main.java index 5c6ca135b..59f7001a5 100644 --- a/test/046-reflect/src/Main.java +++ b/test/046-reflect/src/Main.java @@ -720,6 +720,10 @@ public class Main { } } + static void checkPrivateFieldAccess() { + (new OtherClass()).test(); + } + public static void main(String[] args) throws Exception { Main test = new Main(); test.run(); @@ -733,6 +737,7 @@ public class Main { checkUnique(); checkParametrizedTypeEqualsAndHashCode(); checkGenericArrayTypeEqualsAndHashCode(); + checkPrivateFieldAccess(); } } @@ -804,41 +809,41 @@ class Target extends SuperTarget { } class FieldNoisyInit { - static { - System.out.println("FieldNoisyInit is initializing"); - //Throwable th = new Throwable(); - //th.printStackTrace(); - } + static { + System.out.println("FieldNoisyInit is initializing"); + //Throwable th = new Throwable(); + //th.printStackTrace(); + } } class FieldNoisyInitUser { - static { - System.out.println("FieldNoisyInitUser is initializing"); - } - public static int staticField; - public static FieldNoisyInit noisy; + static { + System.out.println("FieldNoisyInitUser is initializing"); + } + public static int staticField; + public static FieldNoisyInit noisy; } class MethodNoisyInit { - static { - System.out.println("MethodNoisyInit is initializing"); - //Throwable th = new Throwable(); - //th.printStackTrace(); - } + static { + System.out.println("MethodNoisyInit is initializing"); + //Throwable th = new Throwable(); + //th.printStackTrace(); + } } class MethodNoisyInitUser { - static { - System.out.println("MethodNoisyInitUser is initializing"); - } - public static void staticMethod() {} - public void createMethodNoisyInit(MethodNoisyInit ni) {} + static { + System.out.println("MethodNoisyInitUser is initializing"); + } + public static void staticMethod() {} + public void createMethodNoisyInit(MethodNoisyInit ni) {} } class Thrower { - public Thrower() throws UnsupportedOperationException { - throw new UnsupportedOperationException(); - } + public Thrower() throws UnsupportedOperationException { + throw new UnsupportedOperationException(); + } } class ParametrizedTypeTest { @@ -850,3 +855,17 @@ class GenericArrayTypeTest { public void aMethod(T[] names) {} public void aMethodIdentical(T[] names) {} } + +class OtherClass { + private static final long LONG = 1234; + public void test() { + try { + Field field = getClass().getDeclaredField("LONG"); + if (1234 != field.getLong(null)) { + System.out.println("ERROR: values don't match"); + } + } catch (Exception e) { + System.out.println(e); + } + } +} \ No newline at end of file -- 2.11.0