Improve reflection IllegalArgumentException detail messages.

Also add a missing InstanceOf check that was causing CheckJNI to kill us
if someone tried to pass an inappropriate reference type through
Method.invoke. (Amusingly, CheckJNI produced pretty much the exact detail
message that Method.invoke should have.)

Plus a new test for this stuff.

Bug: 6504175
Change-Id: Ice95eecbdba5a0927c6eaf68e56d6500dc52ad2e

8 files changed, 104 insertions, 14 deletions

diff --git a/src/compiler_llvm/runtime_support_llvm.cc b/src/compiler_llvm/runtime_support_llvm.cc
index a31c27eefa..0db31873e6 100644
--- a/src/compiler_llvm/runtime_support_llvm.cc
+++ b/src/compiler_llvm/runtime_support_llvm.cc
@@ -756,7 +756,7 @@ void art_proxy_invoke_handler_from_code(Method* proxy_method, ...) {
     if (result_ref == NULL) {
       result_unboxed->SetL(NULL);
     } else {
-      bool unboxed_okay = UnboxPrimitive(result_ref, proxy_mh.GetReturnType(), *result_unboxed, "result");
+      bool unboxed_okay = UnboxPrimitiveForResult(result_ref, proxy_mh.GetReturnType(), *result_unboxed);
       if (!unboxed_okay) {
         thread->ClearException();
         thread->ThrowNewExceptionF("Ljava/lang/ClassCastException;",
diff --git a/src/native/java_lang_reflect_Field.cc b/src/native/java_lang_reflect_Field.cc
index fe9f79ccc3..3e0c9d7377 100644
--- a/src/native/java_lang_reflect_Field.cc
+++ b/src/native/java_lang_reflect_Field.cc
@@ -213,7 +213,7 @@ static void Field_set(JNIEnv* env, jobject javaField, jobject javaObj, jobject j
   // Unbox the value, if necessary.
   Object* boxed_value = Decode<Object*>(env, javaValue);
   JValue unboxed_value;
-  if (!UnboxPrimitive(boxed_value, FieldHelper(f).GetType(), unboxed_value, "field")) {
+  if (!UnboxPrimitiveForField(boxed_value, FieldHelper(f).GetType(), unboxed_value, f)) {
     return;
   }
 
diff --git a/src/oat/runtime/support_proxy.cc b/src/oat/runtime/support_proxy.cc
index 46f231356c..cc375ad6a4 100644
--- a/src/oat/runtime/support_proxy.cc
+++ b/src/oat/runtime/support_proxy.cc
@@ -168,7 +168,7 @@ extern "C" void artProxyInvokeHandler(Method* proxy_method, Object* receiver,
     Object* result_ref = self->DecodeJObject(result);
     if (result_ref != NULL) {
       JValue result_unboxed;
-      bool unboxed_okay = UnboxPrimitive(result_ref, proxy_mh.GetReturnType(), result_unboxed, "result");
+      bool unboxed_okay = UnboxPrimitiveForResult(result_ref, proxy_mh.GetReturnType(), result_unboxed);
       if (!unboxed_okay) {
         self->ClearException();
         self->ThrowNewExceptionF("Ljava/lang/ClassCastException;",
diff --git a/src/reflection.cc b/src/reflection.cc
index e919ef18a9..6763100b54 100644
--- a/src/reflection.cc
+++ b/src/reflection.cc
@@ -91,10 +91,16 @@ jobject InvokeMethod(JNIEnv* env, jobject javaMethod, jobject javaReceiver, jobj
     Object* arg = objects->Get(i);
     Class* dst_class = mh.GetClassFromTypeIdx(classes->GetTypeItem(i).type_idx_);
     if (dst_class->IsPrimitive()) {
-      std::string what(StringPrintf("argument %d", i + 1)); // Humans count from 1.
-      if (!UnboxPrimitive(arg, dst_class, decoded_args[i], what.c_str())) {
+      if (!UnboxPrimitiveForArgument(arg, dst_class, decoded_args[i], i)) {
         return NULL;
       }
+    } else if (arg != NULL && !arg->InstanceOf(dst_class)) {
+      self->ThrowNewExceptionF("Ljava/lang/IllegalArgumentException;",
+                               "argument %d has type %s, got %s",
+                               i + 1,
+                               PrettyDescriptor(dst_class).c_str(),
+                               PrettyTypeOf(arg).c_str());
+      return NULL;
     } else {
       args[i].l = AddLocalReference<jobject>(env, arg);
     }
@@ -265,12 +271,22 @@ void BoxPrimitive(Primitive::Type src_class, JValue& value) {
   m->Invoke(self, NULL, args, &value);
 }
 
-bool UnboxPrimitive(Object* o, Class* dst_class, JValue& unboxed_value, const char* what) {
+static std::string UnboxingFailureKind(int index, Field* f) {
+  if (index != -1) {
+    return StringPrintf("argument %d", index + 1); // Humans count from 1.
+  }
+  if (f != NULL) {
+    return "field " + PrettyField(f, false);
+  }
+  return "result";
+}
+
+static bool UnboxPrimitive(Object* o, Class* dst_class, JValue& unboxed_value, int index, Field* f) {
   if (!dst_class->IsPrimitive()) {
     if (o != NULL && !o->InstanceOf(dst_class)) {
       Thread::Current()->ThrowNewExceptionF("Ljava/lang/IllegalArgumentException;",
-                                            "boxed object for %s should have type %s, but got %s",
-                                            what,
+                                            "%s has type %s, got %s",
+                                            UnboxingFailureKind(index, f).c_str(),
                                             PrettyDescriptor(dst_class).c_str(),
                                             PrettyTypeOf(o).c_str());
       return false;
@@ -280,14 +296,14 @@ bool UnboxPrimitive(Object* o, Class* dst_class, JValue& unboxed_value, const ch
   } else if (dst_class->GetPrimitiveType() == Primitive::kPrimVoid) {
     Thread::Current()->ThrowNewExceptionF("Ljava/lang/IllegalArgumentException;",
                                           "can't unbox %s to void",
-                                          what);
+                                          UnboxingFailureKind(index, f).c_str());
     return false;
   }
 
   if (o == NULL) {
     Thread::Current()->ThrowNewExceptionF("Ljava/lang/IllegalArgumentException;",
-                                          "%s should have type %s, got null",
-                                          what,
+                                          "%s has type %s, got null",
+                                          UnboxingFailureKind(index, f).c_str(),
                                           PrettyDescriptor(dst_class).c_str());
     return false;
   }
@@ -323,8 +339,8 @@ bool UnboxPrimitive(Object* o, Class* dst_class, JValue& unboxed_value, const ch
     boxed_value.SetS(primitive_field->GetShort(o));
   } else {
     Thread::Current()->ThrowNewExceptionF("Ljava/lang/IllegalArgumentException;",
-                                          "%s should have type %s, got %s",
-                                          what,
+                                          "%s has type %s, got %s",
+                                          UnboxingFailureKind(index, f).c_str(),
                                           PrettyDescriptor(dst_class).c_str(),
                                           PrettyDescriptor(src_descriptor.c_str()).c_str());
     return false;
@@ -334,4 +350,17 @@ bool UnboxPrimitive(Object* o, Class* dst_class, JValue& unboxed_value, const ch
                                boxed_value, unboxed_value);
 }
 
+bool UnboxPrimitiveForArgument(Object* o, Class* dst_class, JValue& unboxed_value, size_t index) {
+  return UnboxPrimitive(o, dst_class, unboxed_value, index, NULL);
+}
+
+bool UnboxPrimitiveForField(Object* o, Class* dst_class, JValue& unboxed_value, Field* f) {
+  CHECK(f != NULL);
+  return UnboxPrimitive(o, dst_class, unboxed_value, -1, f);
+}
+
+bool UnboxPrimitiveForResult(Object* o, Class* dst_class, JValue& unboxed_value) {
+  return UnboxPrimitive(o, dst_class, unboxed_value, -1, NULL);
+}
+
 }  // namespace art
diff --git a/src/reflection.h b/src/reflection.h
index f5173d9599..aabf513060 100644
--- a/src/reflection.h
+++ b/src/reflection.h
@@ -23,12 +23,15 @@
 namespace art {
 
 class Class;
+class Field;
 union JValue;
 class Object;
 
 void InitBoxingMethods();
 void BoxPrimitive(Primitive::Type src_class, JValue& value);
-bool UnboxPrimitive(Object* o, Class* dst_class, JValue& unboxed_value, const char* what);
+bool UnboxPrimitiveForArgument(Object* o, Class* dst_class, JValue& unboxed_value, size_t index);
+bool UnboxPrimitiveForField(Object* o, Class* dst_class, JValue& unboxed_value, Field* f);
+bool UnboxPrimitiveForResult(Object* o, Class* dst_class, JValue& unboxed_value);
 
 bool ConvertPrimitiveValue(Primitive::Type src_class, Primitive::Type dst_class, const JValue& src, JValue& dst);
 
diff --git a/test/200-reflection-errors/expected.txt b/test/200-reflection-errors/expected.txt
new file mode 100644
index 0000000000..b5b201d5de
--- /dev/null
+++ b/test/200-reflection-errors/expected.txt
@@ -0,0 +1,5 @@
+field A.b has type java.lang.String, got java.lang.Integer
+field A.i has type int, got null
+field A.i has type int, got java.lang.String
+argument 2 has type java.lang.String, got java.lang.Integer
+argument 1 has type int, got null
diff --git a/test/200-reflection-errors/info.txt b/test/200-reflection-errors/info.txt
new file mode 100644
index 0000000000..28e3bd4fb0
--- /dev/null
+++ b/test/200-reflection-errors/info.txt
@@ -0,0 +1 @@
+Bug: http://b/6504175
diff --git a/test/200-reflection-errors/src/Main.java b/test/200-reflection-errors/src/Main.java
new file mode 100644
index 0000000000..86bbfd4d2b
--- /dev/null
+++ b/test/200-reflection-errors/src/Main.java
@@ -0,0 +1,52 @@
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+
+public class Main {
+  public static void main(String[] args) throws Exception {
+    // Can't assign Integer to a String field.
+    try {
+      Field field = A.class.getField("b");
+      field.set(new A(), 5);
+    } catch (IllegalArgumentException expected) {
+      System.out.println(expected.getMessage());
+    }
+
+    // Can't unbox null to a primitive.
+    try {
+      Field field = A.class.getField("i");
+      field.set(new A(), null);
+    } catch (IllegalArgumentException expected) {
+      System.out.println(expected.getMessage());
+    }
+
+    // Can't unbox String to a primitive.
+    try {
+      Field field = A.class.getField("i");
+      field.set(new A(), "hello, world!");
+    } catch (IllegalArgumentException expected) {
+      System.out.println(expected.getMessage());
+    }
+
+    // Can't pass an Integer as a String.
+    try {
+      Method m = A.class.getMethod("m", int.class, String.class);
+      m.invoke(new A(), 2, 2);
+    } catch (IllegalArgumentException expected) {
+      System.out.println(expected.getMessage());
+    }
+
+    // Can't pass null as an int.
+    try {
+      Method m = A.class.getMethod("m", int.class, String.class);
+      m.invoke(new A(), null, "");
+    } catch (IllegalArgumentException expected) {
+      System.out.println(expected.getMessage());
+    }
+  }
+}
+
+class A {
+  public String b;
+  public int i;
+  public void m(int i, String s) {}
+}