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

1 files changed, 15 insertions, 6 deletions

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 @@ const bool kVerboseInstrumentation = false;
 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 @@ Instrumentation::Instrumentation()
 }
 
 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 @@ void Instrumentation::ConfigureStubs(bool require_entry_exit_stubs, bool require
   }
   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::ResetQuickAllocEntryPoints() {
 }
 
 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;