commit 289da82ae82438ab3aed1898c82fd87c16846e74
author Elliott Hughes <enh@google.com> Tue Aug 16 10:11:20 2011 -0700
committer Elliott Hughes <enh@google.com> Tue Aug 16 10:35:27 2011 -0700
tree 31cff59beae91556fb85aa2e11da1697b6feea67
parent 04b63fd60de0d9e6f07763511a1a9622accfeed7

Add bounds checking to our internal array get/set methods.

Change-Id: Ia570615e6b4ea81d796b6bf720444c62830a4f89

src/jni_internal.cc

diff --git a/src/jni_internal.cc b/src/jni_internal.cc
index 81e2fdd..87decd7 100644
--- a/src/jni_internal.cc
+++ b/src/jni_internal.cc
@@ -1302,7 +1302,6 @@
   // TODO: DecodeReference
   ObjectArray<Object>* array = reinterpret_cast<ObjectArray<Object>*>(java_array);
   Object* value = reinterpret_cast<Object*>(java_value);
-  // TODO: who should throw? JNI or ObjectArray?
   array->Set(index, value);
 }
 

src/jni_internal_test.cc

diff --git a/src/jni_internal_test.cc b/src/jni_internal_test.cc
index e5ba913..9394ff6 100644
--- a/src/jni_internal_test.cc
+++ b/src/jni_internal_test.cc
@@ -92,7 +92,8 @@
 
   // TODO: check non-NULL initial elements.
 
-  jclass c = env_->FindClass("[Ljava.lang.String;");
+  jclass c = env_->FindClass("[Ljava/lang/String;");
+  ASSERT_TRUE(c != NULL);
 
   EXPECT_TRUE(env_->NewObjectArray(0, c, NULL) != NULL);
 
@@ -106,6 +107,18 @@
   // TODO: check some non-ASCII strings.
 }
 
+TEST_F(JniInternalTest, SetObjectArrayElement) {
+  jclass c = env_->FindClass("[Ljava/lang/Object;");
+  ASSERT_TRUE(c != NULL);
+
+  jobjectArray array = env_->NewObjectArray(1, c, NULL);
+  EXPECT_TRUE(array != NULL);
+  env_->SetObjectArrayElement(array, 0, c);
+  // TODO: check reading value back
+  // TODO: check IndexOutOfBoundsExceptions thrown for bad indexes.
+  // TODO: check ArrayStoreException thrown for bad types.
+}
+
 bool EnsureInvokeStub(Method* method);
 
 byte* AllocateCode(void* code, size_t length) {

src/object.h

diff --git a/src/object.h b/src/object.h
index 6fe2ecf..4f32cf9 100644
--- a/src/object.h
+++ b/src/object.h
@@ -625,9 +625,21 @@
     length_ = length;
   }
 
+ protected:
+  bool IsValidIndex(int32_t index) const {
+    if (index < 0 || index >= length_) {
+      // TODO: throw ArrayIndexOutOfBoundsException with the detail message
+      // "length=%d; index=%d", length_, index;
+      CHECK(false) << "ArrayIndexOutOfBoundsException: length="
+                   << length_ << "; index=" << index;
+      return false;
+    }
+    return true;
+  }
+
  private:
   // The number of array elements.
-  uint32_t length_;
+  int32_t length_;
   // Padding to ensure the first member defined by a subclass begins on a 8-byte boundary
   int32_t padding_;
 
@@ -651,13 +663,17 @@
   }
 
   T* Get(int32_t i) const {
-    CHECK_LT(i, GetLength());
+    if (!IsValidIndex(i)) {
+      return NULL;
+    }
     return GetData()[i];
   }
 
   void Set(int32_t i, T* object) {
-    CHECK_LT(i, GetLength());
-    GetData()[i] = object;  // TODO: write barrier
+    if (IsValidIndex(i)) {
+      // TODO: ArrayStoreException
+      GetData()[i] = object;  // TODO: write barrier
+    }
   }
 
   static void Copy(ObjectArray<T>* src, int src_pos,
@@ -1207,13 +1223,17 @@
   }
 
   T Get(int32_t i) const {
-    CHECK_LT(i, GetLength());
+    if (!IsValidIndex(i)) {
+      return T();
+    }
     return GetData()[i];
   }
 
   void Set(int32_t i, T value) {
-    CHECK_LT(i, GetLength());
-    GetData()[i] = value;
+    // TODO: ArrayStoreException
+    if (IsValidIndex(i)) {
+      GetData()[i] = value;
+    }
   }
 
   static void SetArrayClass(Class* array_class) {
@@ -1249,9 +1269,14 @@
     return count_;
   }
 
-  uint16_t CharAt(uint32_t index) const {
-    DCHECK_LE(index, GetLength());
-    return GetCharArray()->Get(index + GetOffset());
+  uint16_t CharAt(int32_t index) const {
+    if (index < 0 || index >= count_) {
+      // TODO: throw new StringIndexOutOfBounds(this, index);
+      CHECK(false) << "StringIndexOutOfBounds: " << index;
+      return 0;
+    } else {
+      return GetCharArray()->Get(index + GetOffset());
+    }
   }
 
   static String* AllocFromUtf16(int32_t utf16_length,
@@ -1429,9 +1454,9 @@
 
   uint32_t hash_code_;
 
-  uint32_t offset_;
+  int32_t offset_;
 
-  uint32_t count_;
+  int32_t count_;
 
   static Class* GetJavaLangString() {
     DCHECK(java_lang_String_ != NULL);