Spot NULL jobjects passed to MonitorEnter/MonitorExit.
(Rather than aborting inside the monitor implementation.)
Change-Id: I1dd30726c56d9d5707616f1c998ab2bff170a649
diff --git a/src/check_jni.cc b/src/check_jni.cc
index 0189ba3..554b56a 100644
--- a/src/check_jni.cc
+++ b/src/check_jni.cc
@@ -568,6 +568,83 @@
}
}
+ enum InstanceKind {
+ kClass,
+ kDirectByteBuffer,
+ kObject,
+ kString,
+ kThrowable,
+ };
+
+ /*
+ * Verify that "jobj" is a valid non-NULL object reference, and points to
+ * an instance of expectedClass.
+ *
+ * Because we're looking at an object on the GC heap, we have to switch
+ * to "running" mode before doing the checks.
+ */
+ bool CheckInstance(InstanceKind kind, jobject java_object) {
+ const char* what = NULL;
+ switch (kind) {
+ case kClass:
+ what = "jclass";
+ break;
+ case kDirectByteBuffer:
+ what = "direct ByteBuffer";
+ break;
+ case kObject:
+ what = "jobject";
+ break;
+ case kString:
+ what = "jstring";
+ break;
+ case kThrowable:
+ what = "jthrowable";
+ break;
+ default:
+ CHECK(false) << static_cast<int>(kind);
+ }
+
+ if (java_object == NULL) {
+ LOG(ERROR) << "JNI ERROR: " << function_name_ << " received null " << what;
+ JniAbort();
+ return false;
+ }
+
+ ScopedJniThreadState ts(env_);
+ Object* obj = Decode<Object*>(ts, java_object);
+ if (!Heap::IsHeapAddress(obj)) {
+ LOG(ERROR) << "JNI ERROR: " << what << " is an invalid " << GetIndirectRefKind(java_object) << ": " << java_object;
+ JniAbort();
+ return false;
+ }
+
+ bool okay = true;
+ switch (kind) {
+ case kClass:
+ okay = obj->IsClass();
+ break;
+ case kDirectByteBuffer:
+ UNIMPLEMENTED(FATAL);
+ break;
+ case kString:
+ okay = obj->GetClass()->IsStringClass();
+ break;
+ case kThrowable:
+ okay = obj->GetClass()->IsThrowableClass();
+ break;
+ case kObject:
+ break;
+ }
+ if (!okay) {
+ LOG(ERROR) << "JNI ERROR: " << what << " has wrong type: " << PrettyTypeOf(obj);
+ JniAbort();
+ return false;
+ }
+
+ return true;
+ }
+
private:
void Init(JNIEnv* env, JavaVM* vm, int flags, const char* functionName, bool hasMethod) {
env_ = reinterpret_cast<JNIEnvExt*>(env);
@@ -768,74 +845,6 @@
}
}
- enum InstanceKind {
- kClass,
- kDirectByteBuffer,
- kString,
- kThrowable,
- };
-
- /*
- * Verify that "jobj" is a valid non-NULL object reference, and points to
- * an instance of expectedClass.
- *
- * Because we're looking at an object on the GC heap, we have to switch
- * to "running" mode before doing the checks.
- */
- void CheckInstance(InstanceKind kind, jobject java_object) {
- const char* what = NULL;
- switch (kind) {
- case kClass:
- what = "jclass";
- break;
- case kDirectByteBuffer:
- what = "direct ByteBuffer";
- break;
- case kString:
- what = "jstring";
- break;
- case kThrowable:
- what = "jthrowable";
- break;
- default:
- CHECK(false) << static_cast<int>(kind);
- }
-
- if (java_object == NULL) {
- LOG(ERROR) << "JNI ERROR: received null " << what;
- JniAbort();
- return;
- }
-
- ScopedJniThreadState ts(env_);
- Object* obj = Decode<Object*>(ts, java_object);
- if (!Heap::IsHeapAddress(obj)) {
- LOG(ERROR) << "JNI ERROR: " << what << " is an invalid " << GetIndirectRefKind(java_object) << ": " << java_object;
- JniAbort();
- return;
- }
-
- bool okay = true;
- switch (kind) {
- case kClass:
- okay = obj->IsClass();
- break;
- case kDirectByteBuffer:
- // TODO
- break;
- case kString:
- okay = obj->GetClass()->IsStringClass();
- break;
- case kThrowable:
- okay = obj->GetClass()->IsThrowableClass();
- break;
- }
- if (!okay) {
- LOG(ERROR) << "JNI ERROR: " << what << " has wrong type: " << PrettyTypeOf(obj);
- JniAbort();
- }
- }
-
static uint8_t CheckUtfBytes(const char* bytes, const char** errorKind) {
while (*bytes != '\0') {
uint8_t utf8 = *(bytes++);
@@ -1680,11 +1689,17 @@
static jint MonitorEnter(JNIEnv* env, jobject obj) {
CHECK_JNI_ENTRY(kFlag_Default, "EL", env, obj);
+ if (!sc.CheckInstance(ScopedCheck::kObject, obj)) {
+ return JNI_ERR; // Only for jni_internal_test. Real code will have aborted already.
+ }
return CHECK_JNI_EXIT("I", baseEnv(env)->MonitorEnter(env, obj));
}
static jint MonitorExit(JNIEnv* env, jobject obj) {
CHECK_JNI_ENTRY(kFlag_Default | kFlag_ExcepOkay, "EL", env, obj);
+ if (!sc.CheckInstance(ScopedCheck::kObject, obj)) {
+ return JNI_ERR; // Only for jni_internal_test. Real code will have aborted already.
+ }
return CHECK_JNI_EXIT("I", baseEnv(env)->MonitorExit(env, obj));
}
diff --git a/src/jni_internal_test.cc b/src/jni_internal_test.cc
index 97cd969..d492deb 100644
--- a/src/jni_internal_test.cc
+++ b/src/jni_internal_test.cc
@@ -1505,6 +1505,14 @@
thrown_exception = env_->ExceptionOccurred();
env_->ExceptionClear();
EXPECT_TRUE(env_->IsInstanceOf(thrown_exception, imse_class));
+
+ // It's an error to call MonitorEnter or MonitorExit on NULL.
+ vm_->check_jni_abort_hook = TestCheckJniAbortHook;
+ env_->MonitorEnter(NULL);
+ EXPECT_TRUE(gCheckJniAbortMessage.find("in call to MonitorEnter") != std::string::npos);
+ env_->MonitorExit(NULL);
+ EXPECT_TRUE(gCheckJniAbortMessage.find("in call to MonitorExit") != std::string::npos);
+ vm_->check_jni_abort_hook = NULL;
}
} // namespace art