commit aa836f7fa2ef359cf8ec1ef98d924f7971ba8352
author Elliott Hughes <enh@google.com> Tue Aug 20 16:57:23 2013 -0700
committer Elliott Hughes <enh@google.com> Tue Aug 20 17:00:44 2013 -0700
tree d45ecc5798bad35a8513442220203bc860e7a38c
parent 7377fa5cb0feab1b42a87be8009dba7172f65180

Fix PushLocalFrame(0).

Bug: 10395422
Change-Id: Iafef3e496127bfd65db87419ba374a1e5745f148

runtime/jni_internal.cc

diff --git a/runtime/jni_internal.cc b/runtime/jni_internal.cc
index 55c0765..ec0c9be 100644
--- a/runtime/jni_internal.cc
+++ b/runtime/jni_internal.cc
@@ -2333,7 +2333,7 @@
   static jint RegisterNativeMethods(JNIEnv* env, jclass java_class, const JNINativeMethod* methods,
                                     jint method_count, bool return_errors) {
     if (UNLIKELY(method_count < 0)) {
-      JniAbortF("RegisterNatives", "method_cound == %d", method_count);
+      JniAbortF("RegisterNatives", "negative method count: %d", method_count);
       return JNI_ERR;  // Not reached.
     }
     CHECK_NON_NULL_ARGUMENT(RegisterNatives, java_class);
@@ -2512,7 +2512,7 @@
   static jint EnsureLocalCapacity(JNIEnv* env, jint desired_capacity,
                                   const char* caller) {
     // TODO: we should try to expand the table if necessary.
-    if (desired_capacity < 1 || desired_capacity > static_cast<jint>(kLocalsMax)) {
+    if (desired_capacity < 0 || desired_capacity > static_cast<jint>(kLocalsMax)) {
       LOG(ERROR) << "Invalid capacity given to " << caller << ": " << desired_capacity;
       return JNI_ERR;
     }

runtime/jni_internal_test.cc

diff --git a/runtime/jni_internal_test.cc b/runtime/jni_internal_test.cc
index 234e40a..aea2ed3 100644
--- a/runtime/jni_internal_test.cc
+++ b/runtime/jni_internal_test.cc
@@ -1488,6 +1488,21 @@
   env_->DeleteLocalRef(o);
 }
 
+TEST_F(JniInternalTest, PushLocalFrame_10395422) {
+  // The JNI specification is ambiguous about whether the given capacity is to be interpreted as a
+  // maximum or as a minimum, but it seems like it's supposed to be a minimum, and that's how
+  // Android historically treated it, and it's how the RI treats it. It's also the more useful
+  // interpretation!
+  ASSERT_EQ(JNI_OK, env_->PushLocalFrame(0));
+  env_->PopLocalFrame(NULL);
+
+  // Negative capacities are not allowed.
+  ASSERT_EQ(JNI_ERR, env_->PushLocalFrame(-1));
+
+  // And it's okay to have an upper limit. Ours is currently 512.
+  ASSERT_EQ(JNI_ERR, env_->PushLocalFrame(8192));
+}
+
 TEST_F(JniInternalTest, PushLocalFrame_PopLocalFrame) {
   jobject original = env_->NewStringUTF("");
   ASSERT_TRUE(original != NULL);
@@ -1497,11 +1512,11 @@
   ScopedObjectAccess soa(env_);
   mirror::Object* inner2_direct_pointer;
   {
-    env_->PushLocalFrame(4);
+    ASSERT_EQ(JNI_OK, env_->PushLocalFrame(4));
     outer = env_->NewLocalRef(original);
 
     {
-      env_->PushLocalFrame(4);
+      ASSERT_EQ(JNI_OK, env_->PushLocalFrame(4));
       inner1 = env_->NewLocalRef(outer);
       inner2 = env_->NewStringUTF("survivor");
       inner2_direct_pointer = soa.Decode<mirror::Object*>(inner2);