ART: Refactor IRT:Add

Do not abort on overflow. Return null and an error message. The
caller is responsible for handling this, e.g., by aborting. In a
future CL, this may be used for driving additional GCs.

Additional side effect is the removal of a frame from an abortion
stack trace.

Test: m
Test: m test-art-host
Change-Id: I80b1e0ee396fc69906d051f1b661d7dba222fc6f

6 files changed, 102 insertions, 70 deletions

diff --git a/runtime/indirect_reference_table.cc b/runtime/indirect_reference_table.cc
index cff3ea7ecd..2dd4db3895 100644
--- a/runtime/indirect_reference_table.cc
+++ b/runtime/indirect_reference_table.cc
@@ -237,7 +237,8 @@ bool IndirectReferenceTable::Resize(size_t new_size, std::string* error_msg) {
 }
 
 IndirectRef IndirectReferenceTable::Add(IRTSegmentState previous_state,
-                                        ObjPtr<mirror::Object> obj) {
+                                        ObjPtr<mirror::Object> obj,
+                                        std::string* error_msg) {
   if (kDebugIRT) {
     LOG(INFO) << "+++ Add: previous_state=" << previous_state.top_index
               << " top_index=" << segment_state_.top_index
@@ -253,28 +254,34 @@ IndirectRef IndirectReferenceTable::Add(IRTSegmentState previous_state,
 
   if (top_index == max_entries_) {
     if (resizable_ == ResizableCapacity::kNo) {
-      LOG(FATAL) << "JNI ERROR (app bug): " << kind_ << " table overflow "
-                 << "(max=" << max_entries_ << ")\n"
-                 << MutatorLockedDumpable<IndirectReferenceTable>(*this);
-      UNREACHABLE();
+      std::ostringstream oss;
+      oss << "JNI ERROR (app bug): " << kind_ << " table overflow "
+          << "(max=" << max_entries_ << ")"
+          << MutatorLockedDumpable<IndirectReferenceTable>(*this);
+      *error_msg = oss.str();
+      return nullptr;
     }
 
     // Try to double space.
     if (std::numeric_limits<size_t>::max() / 2 < max_entries_) {
-      LOG(FATAL) << "JNI ERROR (app bug): " << kind_ << " table overflow "
-                 << "(max=" << max_entries_ << ")" << std::endl
-                 << MutatorLockedDumpable<IndirectReferenceTable>(*this)
-                << " Resizing failed: exceeds size_t";
-      UNREACHABLE();
+      std::ostringstream oss;
+      oss << "JNI ERROR (app bug): " << kind_ << " table overflow "
+          << "(max=" << max_entries_ << ")" << std::endl
+          << MutatorLockedDumpable<IndirectReferenceTable>(*this)
+          << " Resizing failed: exceeds size_t";
+      *error_msg = oss.str();
+      return nullptr;
     }
 
-    std::string error_msg;
-    if (!Resize(max_entries_ * 2, &error_msg)) {
-      LOG(FATAL) << "JNI ERROR (app bug): " << kind_ << " table overflow "
-                 << "(max=" << max_entries_ << ")" << std::endl
-                 << MutatorLockedDumpable<IndirectReferenceTable>(*this)
-                 << " Resizing failed: " << error_msg;
-      UNREACHABLE();
+    std::string inner_error_msg;
+    if (!Resize(max_entries_ * 2, &inner_error_msg)) {
+      std::ostringstream oss;
+      oss << "JNI ERROR (app bug): " << kind_ << " table overflow "
+          << "(max=" << max_entries_ << ")" << std::endl
+          << MutatorLockedDumpable<IndirectReferenceTable>(*this)
+          << " Resizing failed: " << inner_error_msg;
+      *error_msg = oss.str();
+      return nullptr;
     }
   }
 
diff --git a/runtime/indirect_reference_table.h b/runtime/indirect_reference_table.h
index 6d52d959cb..bf287b1ac9 100644
--- a/runtime/indirect_reference_table.h
+++ b/runtime/indirect_reference_table.h
@@ -244,8 +244,10 @@ class IndirectReferenceTable {
   bool IsValid() const;
 
   // Add a new entry. "obj" must be a valid non-null object reference. This function will
-  // abort if the table is full (max entries reached, or expansion failed).
-  IndirectRef Add(IRTSegmentState previous_state, ObjPtr<mirror::Object> obj)
+  // return null if an error happened (with an appropriate error message set).
+  IndirectRef Add(IRTSegmentState previous_state,
+                  ObjPtr<mirror::Object> obj,
+                  std::string* error_msg)
       REQUIRES_SHARED(Locks::mutator_lock_);
 
   // Given an IndirectRef in the table, return the Object it refers to.
diff --git a/runtime/indirect_reference_table_test.cc b/runtime/indirect_reference_table_test.cc
index 6aefe239a9..92785090f5 100644
--- a/runtime/indirect_reference_table_test.cc
+++ b/runtime/indirect_reference_table_test.cc
@@ -80,13 +80,13 @@ TEST_F(IndirectReferenceTableTest, BasicTest) {
   EXPECT_FALSE(irt.Remove(cookie, iref0)) << "unexpectedly successful removal";
 
   // Add three, check, remove in the order in which they were added.
-  iref0 = irt.Add(cookie, obj0.Get());
+  iref0 = irt.Add(cookie, obj0.Get(), &error_msg);
   EXPECT_TRUE(iref0 != nullptr);
   CheckDump(&irt, 1, 1);
-  IndirectRef iref1 = irt.Add(cookie, obj1.Get());
+  IndirectRef iref1 = irt.Add(cookie, obj1.Get(), &error_msg);
   EXPECT_TRUE(iref1 != nullptr);
   CheckDump(&irt, 2, 2);
-  IndirectRef iref2 = irt.Add(cookie, obj2.Get());
+  IndirectRef iref2 = irt.Add(cookie, obj2.Get(), &error_msg);
   EXPECT_TRUE(iref2 != nullptr);
   CheckDump(&irt, 3, 3);
 
@@ -108,11 +108,11 @@ TEST_F(IndirectReferenceTableTest, BasicTest) {
   EXPECT_TRUE(irt.Get(iref0) == nullptr);
 
   // Add three, remove in the opposite order.
-  iref0 = irt.Add(cookie, obj0.Get());
+  iref0 = irt.Add(cookie, obj0.Get(), &error_msg);
   EXPECT_TRUE(iref0 != nullptr);
-  iref1 = irt.Add(cookie, obj1.Get());
+  iref1 = irt.Add(cookie, obj1.Get(), &error_msg);
   EXPECT_TRUE(iref1 != nullptr);
-  iref2 = irt.Add(cookie, obj2.Get());
+  iref2 = irt.Add(cookie, obj2.Get(), &error_msg);
   EXPECT_TRUE(iref2 != nullptr);
   CheckDump(&irt, 3, 3);
 
@@ -128,11 +128,11 @@ TEST_F(IndirectReferenceTableTest, BasicTest) {
 
   // Add three, remove middle / middle / bottom / top.  (Second attempt
   // to remove middle should fail.)
-  iref0 = irt.Add(cookie, obj0.Get());
+  iref0 = irt.Add(cookie, obj0.Get(), &error_msg);
   EXPECT_TRUE(iref0 != nullptr);
-  iref1 = irt.Add(cookie, obj1.Get());
+  iref1 = irt.Add(cookie, obj1.Get(), &error_msg);
   EXPECT_TRUE(iref1 != nullptr);
-  iref2 = irt.Add(cookie, obj2.Get());
+  iref2 = irt.Add(cookie, obj2.Get(), &error_msg);
   EXPECT_TRUE(iref2 != nullptr);
   CheckDump(&irt, 3, 3);
 
@@ -157,20 +157,20 @@ TEST_F(IndirectReferenceTableTest, BasicTest) {
   // Add four entries.  Remove #1, add new entry, verify that table size
   // is still 4 (i.e. holes are getting filled).  Remove #1 and #3, verify
   // that we delete one and don't hole-compact the other.
-  iref0 = irt.Add(cookie, obj0.Get());
+  iref0 = irt.Add(cookie, obj0.Get(), &error_msg);
   EXPECT_TRUE(iref0 != nullptr);
-  iref1 = irt.Add(cookie, obj1.Get());
+  iref1 = irt.Add(cookie, obj1.Get(), &error_msg);
   EXPECT_TRUE(iref1 != nullptr);
-  iref2 = irt.Add(cookie, obj2.Get());
+  iref2 = irt.Add(cookie, obj2.Get(), &error_msg);
   EXPECT_TRUE(iref2 != nullptr);
-  IndirectRef iref3 = irt.Add(cookie, obj3.Get());
+  IndirectRef iref3 = irt.Add(cookie, obj3.Get(), &error_msg);
   EXPECT_TRUE(iref3 != nullptr);
   CheckDump(&irt, 4, 4);
 
   ASSERT_TRUE(irt.Remove(cookie, iref1));
   CheckDump(&irt, 3, 3);
 
-  iref1 = irt.Add(cookie, obj1.Get());
+  iref1 = irt.Add(cookie, obj1.Get(), &error_msg);
   EXPECT_TRUE(iref1 != nullptr);
 
   ASSERT_EQ(4U, irt.Capacity()) << "hole not filled";
@@ -193,12 +193,12 @@ TEST_F(IndirectReferenceTableTest, BasicTest) {
   // Add an entry, remove it, add a new entry, and try to use the original
   // iref.  They have the same slot number but are for different objects.
   // With the extended checks in place, this should fail.
-  iref0 = irt.Add(cookie, obj0.Get());
+  iref0 = irt.Add(cookie, obj0.Get(), &error_msg);
   EXPECT_TRUE(iref0 != nullptr);
   CheckDump(&irt, 1, 1);
   ASSERT_TRUE(irt.Remove(cookie, iref0));
   CheckDump(&irt, 0, 0);
-  iref1 = irt.Add(cookie, obj1.Get());
+  iref1 = irt.Add(cookie, obj1.Get(), &error_msg);
   EXPECT_TRUE(iref1 != nullptr);
   CheckDump(&irt, 1, 1);
   ASSERT_FALSE(irt.Remove(cookie, iref0)) << "mismatched del succeeded";
@@ -209,12 +209,12 @@ TEST_F(IndirectReferenceTableTest, BasicTest) {
 
   // Same as above, but with the same object.  A more rigorous checker
   // (e.g. with slot serialization) will catch this.
-  iref0 = irt.Add(cookie, obj0.Get());
+  iref0 = irt.Add(cookie, obj0.Get(), &error_msg);
   EXPECT_TRUE(iref0 != nullptr);
   CheckDump(&irt, 1, 1);
   ASSERT_TRUE(irt.Remove(cookie, iref0));
   CheckDump(&irt, 0, 0);
-  iref1 = irt.Add(cookie, obj0.Get());
+  iref1 = irt.Add(cookie, obj0.Get(), &error_msg);
   EXPECT_TRUE(iref1 != nullptr);
   CheckDump(&irt, 1, 1);
   if (iref0 != iref1) {
@@ -229,7 +229,7 @@ TEST_F(IndirectReferenceTableTest, BasicTest) {
   ASSERT_TRUE(irt.Get(nullptr) == nullptr);
 
   // Stale lookup.
-  iref0 = irt.Add(cookie, obj0.Get());
+  iref0 = irt.Add(cookie, obj0.Get(), &error_msg);
   EXPECT_TRUE(iref0 != nullptr);
   CheckDump(&irt, 1, 1);
   ASSERT_TRUE(irt.Remove(cookie, iref0));
@@ -241,12 +241,12 @@ TEST_F(IndirectReferenceTableTest, BasicTest) {
   static const size_t kTableInitial = kTableMax / 2;
   IndirectRef manyRefs[kTableInitial];
   for (size_t i = 0; i < kTableInitial; i++) {
-    manyRefs[i] = irt.Add(cookie, obj0.Get());
+    manyRefs[i] = irt.Add(cookie, obj0.Get(), &error_msg);
     ASSERT_TRUE(manyRefs[i] != nullptr) << "Failed adding " << i;
     CheckDump(&irt, i + 1, 1);
   }
   // ...this one causes overflow.
-  iref0 = irt.Add(cookie, obj0.Get());
+  iref0 = irt.Add(cookie, obj0.Get(), &error_msg);
   ASSERT_TRUE(iref0 != nullptr);
   ASSERT_EQ(kTableInitial + 1, irt.Capacity());
   CheckDump(&irt, kTableInitial + 1, 1);
@@ -306,16 +306,16 @@ TEST_F(IndirectReferenceTableTest, Holes) {
 
     CheckDump(&irt, 0, 0);
 
-    IndirectRef iref0 = irt.Add(cookie0, obj0.Get());
-    IndirectRef iref1 = irt.Add(cookie0, obj1.Get());
-    IndirectRef iref2 = irt.Add(cookie0, obj2.Get());
+    IndirectRef iref0 = irt.Add(cookie0, obj0.Get(), &error_msg);
+    IndirectRef iref1 = irt.Add(cookie0, obj1.Get(), &error_msg);
+    IndirectRef iref2 = irt.Add(cookie0, obj2.Get(), &error_msg);
 
     EXPECT_TRUE(irt.Remove(cookie0, iref1));
 
     // New segment.
     const IRTSegmentState cookie1 = irt.GetSegmentState();
 
-    IndirectRef iref3 = irt.Add(cookie1, obj3.Get());
+    IndirectRef iref3 = irt.Add(cookie1, obj3.Get(), &error_msg);
 
     // Must not have filled the previous hole.
     EXPECT_EQ(irt.Capacity(), 4u);
@@ -337,21 +337,21 @@ TEST_F(IndirectReferenceTableTest, Holes) {
 
     CheckDump(&irt, 0, 0);
 
-    IndirectRef iref0 = irt.Add(cookie0, obj0.Get());
+    IndirectRef iref0 = irt.Add(cookie0, obj0.Get(), &error_msg);
 
     // New segment.
     const IRTSegmentState cookie1 = irt.GetSegmentState();
 
-    IndirectRef iref1 = irt.Add(cookie1, obj1.Get());
-    IndirectRef iref2 = irt.Add(cookie1, obj2.Get());
-    IndirectRef iref3 = irt.Add(cookie1, obj3.Get());
+    IndirectRef iref1 = irt.Add(cookie1, obj1.Get(), &error_msg);
+    IndirectRef iref2 = irt.Add(cookie1, obj2.Get(), &error_msg);
+    IndirectRef iref3 = irt.Add(cookie1, obj3.Get(), &error_msg);
 
     EXPECT_TRUE(irt.Remove(cookie1, iref2));
 
     // Pop segment.
     irt.SetSegmentState(cookie1);
 
-    IndirectRef iref4 = irt.Add(cookie1, obj4.Get());
+    IndirectRef iref4 = irt.Add(cookie1, obj4.Get(), &error_msg);
 
     EXPECT_EQ(irt.Capacity(), 2u);
     EXPECT_TRUE(irt.Get(iref2) == nullptr);
@@ -373,25 +373,25 @@ TEST_F(IndirectReferenceTableTest, Holes) {
 
     CheckDump(&irt, 0, 0);
 
-    IndirectRef iref0 = irt.Add(cookie0, obj0.Get());
+    IndirectRef iref0 = irt.Add(cookie0, obj0.Get(), &error_msg);
 
     // New segment.
     const IRTSegmentState cookie1 = irt.GetSegmentState();
 
-    IndirectRef iref1 = irt.Add(cookie1, obj1.Get());
-    IndirectRef iref2 = irt.Add(cookie1, obj2.Get());
+    IndirectRef iref1 = irt.Add(cookie1, obj1.Get(), &error_msg);
+    IndirectRef iref2 = irt.Add(cookie1, obj2.Get(), &error_msg);
 
     EXPECT_TRUE(irt.Remove(cookie1, iref1));
 
     // New segment.
     const IRTSegmentState cookie2 = irt.GetSegmentState();
 
-    IndirectRef iref3 = irt.Add(cookie2, obj3.Get());
+    IndirectRef iref3 = irt.Add(cookie2, obj3.Get(), &error_msg);
 
     // Pop segment.
     irt.SetSegmentState(cookie2);
 
-    IndirectRef iref4 = irt.Add(cookie1, obj4.Get());
+    IndirectRef iref4 = irt.Add(cookie1, obj4.Get(), &error_msg);
 
     EXPECT_EQ(irt.Capacity(), 3u);
     EXPECT_TRUE(irt.Get(iref1) == nullptr);
@@ -412,20 +412,20 @@ TEST_F(IndirectReferenceTableTest, Holes) {
 
     CheckDump(&irt, 0, 0);
 
-    IndirectRef iref0 = irt.Add(cookie0, obj0.Get());
+    IndirectRef iref0 = irt.Add(cookie0, obj0.Get(), &error_msg);
 
     // New segment.
     const IRTSegmentState cookie1 = irt.GetSegmentState();
 
-    IndirectRef iref1 = irt.Add(cookie1, obj1.Get());
+    IndirectRef iref1 = irt.Add(cookie1, obj1.Get(), &error_msg);
     EXPECT_TRUE(irt.Remove(cookie1, iref1));
 
     // Emptied segment, push new one.
     const IRTSegmentState cookie2 = irt.GetSegmentState();
 
-    IndirectRef iref2 = irt.Add(cookie1, obj1.Get());
-    IndirectRef iref3 = irt.Add(cookie1, obj2.Get());
-    IndirectRef iref4 = irt.Add(cookie1, obj3.Get());
+    IndirectRef iref2 = irt.Add(cookie1, obj1.Get(), &error_msg);
+    IndirectRef iref3 = irt.Add(cookie1, obj2.Get(), &error_msg);
+    IndirectRef iref4 = irt.Add(cookie1, obj3.Get(), &error_msg);
 
     EXPECT_TRUE(irt.Remove(cookie1, iref3));
 
@@ -433,7 +433,7 @@ TEST_F(IndirectReferenceTableTest, Holes) {
     UNUSED(cookie2);
     irt.SetSegmentState(cookie1);
 
-    IndirectRef iref5 = irt.Add(cookie1, obj4.Get());
+    IndirectRef iref5 = irt.Add(cookie1, obj4.Get(), &error_msg);
 
     EXPECT_EQ(irt.Capacity(), 2u);
     EXPECT_TRUE(irt.Get(iref3) == nullptr);
@@ -455,14 +455,14 @@ TEST_F(IndirectReferenceTableTest, Holes) {
 
     CheckDump(&irt, 0, 0);
 
-    IndirectRef iref0 = irt.Add(cookie0, obj0.Get());
+    IndirectRef iref0 = irt.Add(cookie0, obj0.Get(), &error_msg);
 
     // New segment.
     const IRTSegmentState cookie1 = irt.GetSegmentState();
 
-    IndirectRef iref1 = irt.Add(cookie1, obj1.Get());
-    IndirectRef iref2 = irt.Add(cookie1, obj1.Get());
-    IndirectRef iref3 = irt.Add(cookie1, obj2.Get());
+    IndirectRef iref1 = irt.Add(cookie1, obj1.Get(), &error_msg);
+    IndirectRef iref2 = irt.Add(cookie1, obj1.Get(), &error_msg);
+    IndirectRef iref3 = irt.Add(cookie1, obj2.Get(), &error_msg);
 
     EXPECT_TRUE(irt.Remove(cookie1, iref2));
 
@@ -473,7 +473,7 @@ TEST_F(IndirectReferenceTableTest, Holes) {
     const IRTSegmentState cookie1_second = irt.GetSegmentState();
     UNUSED(cookie1_second);
 
-    IndirectRef iref4 = irt.Add(cookie1, obj3.Get());
+    IndirectRef iref4 = irt.Add(cookie1, obj3.Get(), &error_msg);
 
     EXPECT_EQ(irt.Capacity(), 2u);
     EXPECT_TRUE(irt.Get(iref3) == nullptr);
@@ -504,7 +504,7 @@ TEST_F(IndirectReferenceTableTest, Resize) {
   const IRTSegmentState cookie = kIRTFirstSegment;
 
   for (size_t i = 0; i != kTableMax + 1; ++i) {
-    irt.Add(cookie, obj0.Get());
+    irt.Add(cookie, obj0.Get(), &error_msg);
   }
 
   EXPECT_EQ(irt.Capacity(), kTableMax + 1);
diff --git a/runtime/java_vm_ext.cc b/runtime/java_vm_ext.cc
index c0d1861a8e..5a1605323e 100644
--- a/runtime/java_vm_ext.cc
+++ b/runtime/java_vm_ext.cc
@@ -589,7 +589,12 @@ jobject JavaVMExt::AddGlobalRef(Thread* self, ObjPtr<mirror::Object> obj) {
     return nullptr;
   }
   WriterMutexLock mu(self, *Locks::jni_globals_lock_);
-  IndirectRef ref = globals_.Add(kIRTFirstSegment, obj);
+  std::string error_msg;
+  IndirectRef ref = globals_.Add(kIRTFirstSegment, obj, &error_msg);
+  if (UNLIKELY(ref == nullptr)) {
+    LOG(FATAL) << error_msg;
+    UNREACHABLE();
+  }
   return reinterpret_cast<jobject>(ref);
 }
 
@@ -607,7 +612,12 @@ jweak JavaVMExt::AddWeakGlobalRef(Thread* self, ObjPtr<mirror::Object> obj) {
     self->CheckEmptyCheckpointFromWeakRefAccess(Locks::jni_weak_globals_lock_);
     weak_globals_add_condition_.WaitHoldingLocks(self);
   }
-  IndirectRef ref = weak_globals_.Add(kIRTFirstSegment, obj);
+  std::string error_msg;
+  IndirectRef ref = weak_globals_.Add(kIRTFirstSegment, obj, &error_msg);
+  if (UNLIKELY(ref == nullptr)) {
+    LOG(FATAL) << error_msg;
+    UNREACHABLE();
+  }
   return reinterpret_cast<jweak>(ref);
 }
 
diff --git a/runtime/jni_env_ext-inl.h b/runtime/jni_env_ext-inl.h
index 25893b7eda..d66df081c6 100644
--- a/runtime/jni_env_ext-inl.h
+++ b/runtime/jni_env_ext-inl.h
@@ -25,7 +25,13 @@ namespace art {
 
 template<typename T>
 inline T JNIEnvExt::AddLocalReference(ObjPtr<mirror::Object> obj) {
-  IndirectRef ref = locals.Add(local_ref_cookie, obj);
+  std::string error_msg;
+  IndirectRef ref = locals.Add(local_ref_cookie, obj, &error_msg);
+  if (UNLIKELY(ref == nullptr)) {
+    // This is really unexpected if we allow resizing local IRTs...
+    LOG(FATAL) << error_msg;
+    UNREACHABLE();
+  }
 
   // TODO: fix this to understand PushLocalFrame, so we can turn it on.
   if (false) {
diff --git a/runtime/jni_env_ext.cc b/runtime/jni_env_ext.cc
index 3ff94f995d..8352657f28 100644
--- a/runtime/jni_env_ext.cc
+++ b/runtime/jni_env_ext.cc
@@ -99,7 +99,14 @@ jobject JNIEnvExt::NewLocalRef(mirror::Object* obj) {
   if (obj == nullptr) {
     return nullptr;
   }
-  return reinterpret_cast<jobject>(locals.Add(local_ref_cookie, obj));
+  std::string error_msg;
+  jobject ref = reinterpret_cast<jobject>(locals.Add(local_ref_cookie, obj, &error_msg));
+  if (UNLIKELY(ref == nullptr)) {
+    // This is really unexpected if we allow resizing local IRTs...
+    LOG(FATAL) << error_msg;
+    UNREACHABLE();
+  }
+  return ref;
 }
 
 void JNIEnvExt::DeleteLocalRef(jobject obj) {