Merge "Don't allow allocating finalizable objects during transactions."

5 files changed, 35 insertions, 8 deletions

diff --git a/runtime/interpreter/interpreter.cc b/runtime/interpreter/interpreter.cc
index 3c6c225..1e1a8c1 100644
--- a/runtime/interpreter/interpreter.cc
+++ b/runtime/interpreter/interpreter.cc
@@ -98,11 +98,8 @@ static void UnstartedRuntimeJni(Thread* self, ArtMethod* method,
     Primitive::Type primitive_type = component->GetPrimitiveType();
     result->SetI(Primitive::ComponentSize(primitive_type));
   } else {
-    // Throw an exception so we can abort the transaction and undo every change.
-    ThrowLocation throw_location = self->GetCurrentLocationForThrow();
-    self->ThrowNewExceptionF(throw_location, "Ljava/lang/InternalError;",
-                             "Attempt to invoke native method in non-started runtime: %s",
-                             name.c_str());
+    AbortTransaction(self, "Attempt to invoke native method in non-started runtime: %s",
+                     name.c_str());
   }
 }
 
diff --git a/runtime/interpreter/interpreter_common.cc b/runtime/interpreter/interpreter_common.cc
index 297f1a8..5660508 100644
--- a/runtime/interpreter/interpreter_common.cc
+++ b/runtime/interpreter/interpreter_common.cc
@@ -40,6 +40,16 @@ static inline void AssignRegister(ShadowFrame* new_shadow_frame, const ShadowFra
   }
 }
 
+void AbortTransaction(Thread* self, const char* fmt, ...) {
+  CHECK(Runtime::Current()->IsActiveTransaction());
+  // Throw an exception so we can abort the transaction and undo every change.
+  va_list args;
+  va_start(args, fmt);
+  self->ThrowNewExceptionV(self->GetCurrentLocationForThrow(), "Ljava/lang/InternalError;", fmt,
+                           args);
+  va_end(args);
+}
+
 template<bool is_range, bool do_assignability_check>
 bool DoCall(ArtMethod* method, Thread* self, ShadowFrame& shadow_frame,
             const Instruction* inst, uint16_t inst_data, JValue* result) {
diff --git a/runtime/interpreter/interpreter_common.h b/runtime/interpreter/interpreter_common.h
index ce3346e..819b79d 100644
--- a/runtime/interpreter/interpreter_common.h
+++ b/runtime/interpreter/interpreter_common.h
@@ -82,6 +82,9 @@ static inline void DoMonitorExit(Thread* self, Object* ref) NO_THREAD_SAFETY_ANA
   ref->MonitorExit(self);
 }
 
+void AbortTransaction(Thread* self, const char* fmt, ...)
+    SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+
 void RecordArrayElementsInTransaction(mirror::Array* array, int32_t count)
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
diff --git a/runtime/interpreter/interpreter_goto_table_impl.cc b/runtime/interpreter/interpreter_goto_table_impl.cc
index d0bb001..74b7c42 100644
--- a/runtime/interpreter/interpreter_goto_table_impl.cc
+++ b/runtime/interpreter/interpreter_goto_table_impl.cc
@@ -496,7 +496,7 @@ JValue ExecuteGotoImpl(Thread* self, MethodHelper& mh, const DexFile::CodeItem*
   }
   HANDLE_INSTRUCTION_END();
 
-  HANDLE_INSTRUCTION_START(ARRAY_LENGTH)  {
+  HANDLE_INSTRUCTION_START(ARRAY_LENGTH) {
     Object* array = shadow_frame.GetVRegReference(inst->VRegB_12x(inst_data));
     if (UNLIKELY(array == NULL)) {
       ThrowNullPointerExceptionFromDexPC(shadow_frame.GetCurrentLocationForThrow());
@@ -509,12 +509,20 @@ JValue ExecuteGotoImpl(Thread* self, MethodHelper& mh, const DexFile::CodeItem*
   HANDLE_INSTRUCTION_END();
 
   HANDLE_INSTRUCTION_START(NEW_INSTANCE) {
+    Runtime* runtime = Runtime::Current();
     Object* obj = AllocObjectFromCode<do_access_check, true>(
         inst->VRegB_21c(), shadow_frame.GetMethod(), self,
-        Runtime::Current()->GetHeap()->GetCurrentAllocator());
+        runtime->GetHeap()->GetCurrentAllocator());
     if (UNLIKELY(obj == NULL)) {
       HANDLE_PENDING_EXCEPTION();
     } else {
+      // Don't allow finalizable objects to be allocated during a transaction since these can't be
+      // finalized without a started runtime.
+      if (transaction_active && obj->GetClass()->IsFinalizable()) {
+        AbortTransaction(self, "Allocating finalizable object in transcation: %s",
+                         PrettyTypeOf(obj).c_str());
+        HANDLE_PENDING_EXCEPTION();
+      }
       shadow_frame.SetVRegReference(inst->VRegA_21c(inst_data), obj);
       ADVANCE(2);
     }
diff --git a/runtime/interpreter/interpreter_switch_impl.cc b/runtime/interpreter/interpreter_switch_impl.cc
index 77e2a82..0da1445 100644
--- a/runtime/interpreter/interpreter_switch_impl.cc
+++ b/runtime/interpreter/interpreter_switch_impl.cc
@@ -422,12 +422,21 @@ JValue ExecuteSwitchImpl(Thread* self, MethodHelper& mh, const DexFile::CodeItem
       }
       case Instruction::NEW_INSTANCE: {
         PREAMBLE();
+        Runtime* runtime = Runtime::Current();
         Object* obj = AllocObjectFromCode<do_access_check, true>(
             inst->VRegB_21c(), shadow_frame.GetMethod(), self,
-            Runtime::Current()->GetHeap()->GetCurrentAllocator());
+            runtime->GetHeap()->GetCurrentAllocator());
         if (UNLIKELY(obj == NULL)) {
           HANDLE_PENDING_EXCEPTION();
         } else {
+          // Don't allow finalizable objects to be allocated during a transaction since these can't
+          // be finalized without a started runtime.
+          if (transaction_active && obj->GetClass()->IsFinalizable()) {
+            AbortTransaction(self, "Allocating finalizable object in transcation: %s",
+                             PrettyTypeOf(obj).c_str());
+            HANDLE_PENDING_EXCEPTION();
+            break;
+          }
           shadow_frame.SetVRegReference(inst->VRegA_21c(inst_data), obj);
           inst = inst->Next_2xx();
         }