Add more checking to ReleasePrimitiveArray.

When we ReleasePrimitiveArray, we now check that the elements pointer
is not a heap address if it is not equal to the java array's data.

Bug: 12845603
Change-Id: I458862f4dc586ba1c414647c7eb81b978c4ccb7e

4 files changed, 25 insertions, 7 deletions

diff --git a/runtime/entrypoints/jni/jni_entrypoints.cc b/runtime/entrypoints/jni/jni_entrypoints.cc
index 4d1e531..c0304eb 100644
--- a/runtime/entrypoints/jni/jni_entrypoints.cc
+++ b/runtime/entrypoints/jni/jni_entrypoints.cc
@@ -46,7 +46,8 @@ extern "C" void* artFindNativeMethod() {
   }
 }
 
-static void WorkAroundJniBugsForJobject(intptr_t* arg_ptr) {
+static void WorkAroundJniBugsForJobject(intptr_t* arg_ptr)
+    SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
   intptr_t value = *arg_ptr;
   mirror::Object** value_as_jni_rep = reinterpret_cast<mirror::Object**>(value);
   mirror::Object* value_as_work_around_rep = value_as_jni_rep != NULL ? *value_as_jni_rep : NULL;
diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index 9c828b2..d98fe37 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -800,11 +800,12 @@ bool Heap::IsValidObjectAddress(const mirror::Object* obj) const {
   return IsAligned<kObjectAlignment>(obj) && IsHeapAddress(obj);
 }
 
+bool Heap::IsNonDiscontinuousSpaceHeapAddress(const mirror::Object* obj) const {
+  return FindContinuousSpaceFromObject(obj, true) != nullptr;
+}
+
 bool Heap::IsHeapAddress(const mirror::Object* obj) const {
-  if (kMovingCollector && bump_pointer_space_ && bump_pointer_space_->HasAddress(obj)) {
-    return true;
-  }
-  // TODO: This probably doesn't work for large objects.
+  // TODO: This might not work for large objects.
   return FindSpaceFromObject(obj, true) != nullptr;
 }
 
diff --git a/runtime/gc/heap.h b/runtime/gc/heap.h
index 368a687..e416c0e 100644
--- a/runtime/gc/heap.h
+++ b/runtime/gc/heap.h
@@ -226,10 +226,16 @@ class Heap {
   // A weaker test than IsLiveObject or VerifyObject that doesn't require the heap lock,
   // and doesn't abort on error, allowing the caller to report more
   // meaningful diagnostics.
-  bool IsValidObjectAddress(const mirror::Object* obj) const;
+  bool IsValidObjectAddress(const mirror::Object* obj) const
+      SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
   // Returns true if the address passed in is a heap address, doesn't need to be aligned.
-  bool IsHeapAddress(const mirror::Object* obj) const;
+  bool IsHeapAddress(const mirror::Object* obj) const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+
+  // Faster alternative to IsHeapAddress since finding if an object is in the large object space is
+  // very slow.
+  bool IsNonDiscontinuousSpaceHeapAddress(const mirror::Object* obj) const
+      SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
   // Returns true if 'obj' is a live heap object, false otherwise (including for invalid addresses).
   // Requires the heap lock to be held.
diff --git a/runtime/jni_internal.cc b/runtime/jni_internal.cc
index 030b213..fbaadfb 100644
--- a/runtime/jni_internal.cc
+++ b/runtime/jni_internal.cc
@@ -2650,6 +2650,16 @@ class JNI {
     size_t bytes = array->GetLength() * component_size;
     VLOG(heap) << "Release primitive array " << env << " array_data " << array_data
                << " elements " << reinterpret_cast<void*>(elements);
+    if (is_copy) {
+      // Sanity check: If elements is not the same as the java array's data, it better not be a
+      // heap address. TODO: This might be slow to check, may be worth keeping track of which
+      // copies we make?
+      if (heap->IsNonDiscontinuousSpaceHeapAddress(reinterpret_cast<mirror::Object*>(elements))) {
+        JniAbortF("ReleaseArrayElements", "invalid element pointer %p, array elements are %p",
+                  reinterpret_cast<void*>(elements), array_data);
+        return;
+      }
+    }
     // Don't need to copy if we had a direct pointer.
     if (mode != JNI_ABORT && is_copy) {
       memcpy(array_data, elements, bytes);