ART: Better IllegalAccessException message

Bug: 17618578
Bug: 17614623
Change-Id: I0e3f15e676acd6ed5844fc86e136f75cc335372d

5 files changed, 64 insertions, 10 deletions

diff --git a/runtime/native/java_lang_reflect_Field.cc b/runtime/native/java_lang_reflect_Field.cc
index 14c6d3879c..d166be030a 100644
--- a/runtime/native/java_lang_reflect_Field.cc
+++ b/runtime/native/java_lang_reflect_Field.cc
@@ -33,13 +33,24 @@ ALWAYS_INLINE inline static bool VerifyFieldAccess(Thread* self, mirror::ArtFiel
                                                    mirror::Object* obj)
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
   if (kIsSet && field->IsFinal()) {
-    ThrowIllegalAccessException(nullptr, StringPrintf("Cannot set final field: %s",
-                                                      PrettyField(field).c_str()).c_str());
+    ThrowIllegalAccessException(nullptr,
+            StringPrintf("Cannot set %s field %s of class %s",
+                PrettyJavaAccessFlags(field->GetAccessFlags()).c_str(),
+                PrettyField(field).c_str(),
+                field->GetDeclaringClass() == nullptr ? "null" :
+                    PrettyClass(field->GetDeclaringClass()).c_str()).c_str());
     return false;
   }
-  if (!VerifyAccess(self, obj, field->GetDeclaringClass(), field->GetAccessFlags())) {
-    ThrowIllegalAccessException(nullptr, StringPrintf("Cannot access field: %s",
-                                                      PrettyField(field).c_str()).c_str());
+  mirror::Class* calling_class = nullptr;
+  if (!VerifyAccess(self, obj, field->GetDeclaringClass(), field->GetAccessFlags(),
+                    &calling_class)) {
+    ThrowIllegalAccessException(nullptr,
+            StringPrintf("Class %s cannot access %s field %s of class %s",
+                calling_class == nullptr ? "null" : PrettyClass(calling_class).c_str(),
+                PrettyJavaAccessFlags(field->GetAccessFlags()).c_str(),
+                PrettyField(field).c_str(),
+                field->GetDeclaringClass() == nullptr ? "null" :
+                    PrettyClass(field->GetDeclaringClass()).c_str()).c_str());
     return false;
   }
   return true;
diff --git a/runtime/reflection.cc b/runtime/reflection.cc
index 0705d402b6..23f8076e0f 100644
--- a/runtime/reflection.cc
+++ b/runtime/reflection.cc
@@ -592,9 +592,16 @@ jobject InvokeMethod(const ScopedObjectAccessAlreadyRunnable& soa, jobject javaM
   }
 
   // If method is not set to be accessible, verify it can be accessed by the caller.
-  if (!accessible && !VerifyAccess(soa.Self(), receiver, declaring_class, m->GetAccessFlags())) {
-    ThrowIllegalAccessException(nullptr, StringPrintf("Cannot access method: %s",
-                                                      PrettyMethod(m).c_str()).c_str());
+  mirror::Class* calling_class = nullptr;
+  if (!accessible && !VerifyAccess(soa.Self(), receiver, declaring_class, m->GetAccessFlags(),
+                                   &calling_class)) {
+    ThrowIllegalAccessException(nullptr,
+        StringPrintf("Class %s cannot access %s method %s of class %s",
+            calling_class == nullptr ? "null" : PrettyClass(calling_class).c_str(),
+            PrettyJavaAccessFlags(m->GetAccessFlags()).c_str(),
+            PrettyMethod(m).c_str(),
+            m->GetDeclaringClass() == nullptr ? "null" :
+                PrettyClass(m->GetDeclaringClass()).c_str()).c_str());
     return nullptr;
   }
 
@@ -815,7 +822,8 @@ bool UnboxPrimitiveForResult(const ThrowLocation& throw_location, mirror::Object
   return UnboxPrimitive(&throw_location, o, dst_class, nullptr, unboxed_value);
 }
 
-bool VerifyAccess(Thread* self, mirror::Object* obj, mirror::Class* declaring_class, uint32_t access_flags) {
+bool VerifyAccess(Thread* self, mirror::Object* obj, mirror::Class* declaring_class,
+                  uint32_t access_flags, mirror::Class** calling_class) {
   if ((access_flags & kAccPublic) != 0) {
     return true;
   }
@@ -829,6 +837,8 @@ bool VerifyAccess(Thread* self, mirror::Object* obj, mirror::Class* declaring_cl
   if (caller_class == declaring_class) {
     return true;
   }
+  ScopedAssertNoThreadSuspension sants(self, "verify-access");
+  *calling_class = caller_class;
   if ((access_flags & kAccPrivate) != 0) {
     return false;
   }
diff --git a/runtime/reflection.h b/runtime/reflection.h
index 00f9d09128..23d8e05452 100644
--- a/runtime/reflection.h
+++ b/runtime/reflection.h
@@ -76,7 +76,7 @@ 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)
+                  uint32_t access_flags, mirror::Class** calling_class)
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
 }  // namespace art
diff --git a/runtime/utils.cc b/runtime/utils.cc
index dbd22139ea..0496d97ae7 100644
--- a/runtime/utils.cc
+++ b/runtime/utils.cc
@@ -474,6 +474,35 @@ std::string PrettyClassAndClassLoader(mirror::Class* c) {
   return result;
 }
 
+std::string PrettyJavaAccessFlags(uint32_t access_flags) {
+  std::string result;
+  if ((access_flags & kAccPublic) != 0) {
+    result += "public ";
+  }
+  if ((access_flags & kAccProtected) != 0) {
+    result += "protected ";
+  }
+  if ((access_flags & kAccPrivate) != 0) {
+    result += "private ";
+  }
+  if ((access_flags & kAccFinal) != 0) {
+    result += "final ";
+  }
+  if ((access_flags & kAccStatic) != 0) {
+    result += "static ";
+  }
+  if ((access_flags & kAccTransient) != 0) {
+    result += "transient ";
+  }
+  if ((access_flags & kAccVolatile) != 0) {
+    result += "volatile ";
+  }
+  if ((access_flags & kAccSynchronized) != 0) {
+    result += "synchronized ";
+  }
+  return result;
+}
+
 std::string PrettySize(int64_t byte_count) {
   // The byte thresholds at which we display amounts.  A byte count is displayed
   // in unit U when kUnitThresholds[U] <= bytes < kUnitThresholds[U+1].
diff --git a/runtime/utils.h b/runtime/utils.h
index 9ec6db1e52..3f2d829b68 100644
--- a/runtime/utils.h
+++ b/runtime/utils.h
@@ -311,6 +311,10 @@ std::string PrettyClass(mirror::Class* c)
 std::string PrettyClassAndClassLoader(mirror::Class* c)
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
+// Returns a human-readable version of the Java part of the access flags, e.g., "private static "
+// (note the trailing whitespace).
+std::string PrettyJavaAccessFlags(uint32_t access_flags);
+
 // Returns a human-readable size string such as "1MB".
 std::string PrettySize(int64_t size_in_bytes);