Update instrumentation stubs on resolved classes

We cannot update methods of a class in the process of being loaded.
We need it to be fully resolved (kStatusResolved) so we can access
its complete structure (including method index) and the compiled
code from the oat file.

We ensure that by skipping classes that are not resolved yet when we
update instrumentation (with all threads suspended). The entrypoints
will be updated when the class gets resolved by the ClassLinker. We
also do not update method entrypoints of erroneous classes
(kStatusError) because we cannot execute code for these methods.

This situation can happen when the debugger requests an event that
will cause a full deoptimization (like a METHOD_ENTRY event) while we
are loading a new class. Because we suspend all threads to update
instrumentation, we may visit a class that is being loaded but not
yet resolved.

Bug: 19012386
Bug: 18766029
Change-Id: I5a645dfaf5c25dcf4282c1aaeb24f1b6333baa37
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 05b6b1d..643eaaa 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -2354,6 +2354,18 @@
 
   Handle<mirror::Class> new_class_h(hs.NewHandle(new_class));
 
+  // Instrumentation may have updated entrypoints for all methods of all
+  // classes. However it could not update methods of this class while we
+  // were loading it. Now the class is resolved, we can update entrypoints
+  // as required by instrumentation.
+  if (Runtime::Current()->GetInstrumentation()->AreExitStubsInstalled()) {
+    // We must be in the kRunnable state to prevent instrumentation from
+    // suspending all threads to update entrypoints while we are doing it
+    // for this class.
+    DCHECK_EQ(self->GetState(), kRunnable);
+    Runtime::Current()->GetInstrumentation()->InstallStubsForClass(new_class_h.Get());
+  }
+
   /*
    * We send CLASS_PREPARE events to the debugger from here.  The
    * definition of "preparation" is creating the static fields for a
@@ -2671,10 +2683,6 @@
       DCHECK(IsQuickGenericJniStub(entry_point) || IsQuickResolutionStub(entry_point));
     }
   }
-
-  // Allow instrumentation its chance to hijack code.
-  runtime->GetInstrumentation()->UpdateMethodsCode(method.Get(),
-                                                   method->GetEntryPointFromQuickCompiledCode());
 }
 
 
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index 1548cfd..14834dd 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -54,7 +54,7 @@
 static constexpr bool kDeoptimizeForAccurateMethodEntryExitListeners = true;
 
 static bool InstallStubsClassVisitor(mirror::Class* klass, void* arg)
-    SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
+    EXCLUSIVE_LOCKS_REQUIRED(Locks::mutator_lock_) {
   Instrumentation* instrumentation = reinterpret_cast<Instrumentation*>(arg);
   return instrumentation->InstallStubsForClass(klass);
 }
@@ -74,11 +74,18 @@
 }
 
 bool Instrumentation::InstallStubsForClass(mirror::Class* klass) {
-  for (size_t i = 0, e = klass->NumDirectMethods(); i < e; i++) {
-    InstallStubsForMethod(klass->GetDirectMethod(i));
-  }
-  for (size_t i = 0, e = klass->NumVirtualMethods(); i < e; i++) {
-    InstallStubsForMethod(klass->GetVirtualMethod(i));
+  if (klass->IsErroneous()) {
+    // We can't execute code in a erroneous class: do nothing.
+  } else if (!klass->IsResolved()) {
+    // We need the class to be resolved to install/uninstall stubs. Otherwise its methods
+    // could not be initialized or linked with regards to class inheritance.
+  } else {
+    for (size_t i = 0, e = klass->NumDirectMethods(); i < e; i++) {
+      InstallStubsForMethod(klass->GetDirectMethod(i));
+    }
+    for (size_t i = 0, e = klass->NumVirtualMethods(); i < e; i++) {
+      InstallStubsForMethod(klass->GetVirtualMethod(i));
+    }
   }
   return true;
 }
@@ -541,6 +548,7 @@
   }
   Thread* const self = Thread::Current();
   Runtime* runtime = Runtime::Current();
+  Locks::mutator_lock_->AssertExclusiveHeld(self);
   Locks::thread_list_lock_->AssertNotHeld(self);
   if (desired_level > 0) {
     if (require_interpreter) {
@@ -631,6 +639,7 @@
 }
 
 void Instrumentation::UpdateMethodsCode(mirror::ArtMethod* method, const void* quick_code) {
+  DCHECK(method->GetDeclaringClass()->IsResolved());
   const void* new_quick_code;
   if (LIKELY(!instrumentation_stubs_installed_)) {
     new_quick_code = quick_code;