Delay entrypoint update until visibly initialized.

Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing --jit
Test: aosp_taimen-userdebug boots.
Test: run-gtests.sh
Test: testrunner.py --target --optimizing --jit
Bug: 18161648
Change-Id: Idde0cbc848f19236319426bc82ac10b8b8bb9ee9

runtime/art_method-inl.h

@@ -432,12 +432,12 @@ inline void ArtMethod::CalculateAndSetImtIndex() {
 
 template <ReadBarrierOption kReadBarrierOption>
 bool ArtMethod::NeedsInitializationCheck() {
-  // Knowing if the class is visibly initialized is only for JIT/AOT compiled
-  // code to avoid the memory barrier. For callers of `NeedsInitializationCheck`
-  // it's enough to just check whether the class is initialized.
+  // The class needs to be visibly initialized before we can use entrypoints to
+  // compiled code for static methods. See b/18161648 . The class initializer is
+  // special as it is invoked during initialization and does not need the check.
   return IsStatic() &&
       !IsConstructor() &&
-      !GetDeclaringClass<kReadBarrierOption>()->IsInitialized();
+      !GetDeclaringClass<kReadBarrierOption>()->IsVisiblyInitialized();
 }
 
 }  // namespace art

runtime/class_linker.cc

@@ -303,6 +303,7 @@ class ClassLinker::VisiblyInitializedCallback final
           vm->DeleteWeakGlobalRef(self, classes_[i]);
           if (klass != nullptr) {
             mirror::Class::SetStatus(klass, ClassStatus::kVisiblyInitialized, self);
+            class_linker_->FixupStaticTrampolines(klass.Get());
           }
         }
         num_classes_ = 0u;
@@ -402,12 +403,14 @@ ClassLinker::VisiblyInitializedCallback* ClassLinker::MarkClassInitialized(
     // Thanks to the x86 memory model, we do not need any memory fences and
     // we can immediately mark the class as visibly initialized.
     mirror::Class::SetStatus(klass, ClassStatus::kVisiblyInitialized, self);
+    FixupStaticTrampolines(klass.Get());
     return nullptr;
   }
   if (Runtime::Current()->IsActiveTransaction()) {
     // Transactions are single-threaded, so we can mark the class as visibly intialized.
     // (Otherwise we'd need to track the callback's entry in the transaction for rollback.)
     mirror::Class::SetStatus(klass, ClassStatus::kVisiblyInitialized, self);
+    FixupStaticTrampolines(klass.Get());
     return nullptr;
   }
   mirror::Class::SetStatus(klass, ClassStatus::kInitialized, self);
@@ -3697,10 +3700,13 @@ bool ClassLinker::ShouldUseInterpreterEntrypoint(ArtMethod* method, const void*
 
 void ClassLinker::FixupStaticTrampolines(ObjPtr<mirror::Class> klass) {
   ScopedAssertNoThreadSuspension sants(__FUNCTION__);
-  DCHECK(klass->IsInitialized()) << klass->PrettyDescriptor();
+  DCHECK(klass->IsVisiblyInitialized()) << klass->PrettyDescriptor();
   if (klass->NumDirectMethods() == 0) {
     return;  // No direct methods => no static methods.
   }
+  if (UNLIKELY(klass->IsProxyClass())) {
+    return;
+  }
   Runtime* runtime = Runtime::Current();
   if (!runtime->IsStarted()) {
     if (runtime->IsAotCompiler() || runtime->GetHeap()->HasBootImageSpace()) {
@@ -3735,7 +3741,7 @@ void ClassLinker::FixupStaticTrampolines(ObjPtr<mirror::Class> klass) {
       quick_code = oat_method.GetQuickCode();
     }
     // Check if we have JIT compiled code for it.
-    jit::Jit* jit = Runtime::Current()->GetJit();
+    jit::Jit* jit = runtime->GetJit();
     if (quick_code == nullptr && jit != nullptr) {
       quick_code = jit->GetCodeCache()->GetSavedEntryPointOfPreCompiledMethod(method);
     }
@@ -5671,8 +5677,6 @@ bool ClassLinker::InitializeClass(Thread* self, Handle<mirror::Class> klass,
         LOG(INFO) << "Initialized class " << klass->GetDescriptor(&temp) << " from " <<
             klass->GetLocation();
       }
-      // Opportunistically set static method trampolines to their destination.
-      FixupStaticTrampolines(klass.Get());
     }
   }
   if (callback != nullptr) {

runtime/jit/jit_code_cache.cc

@@ -119,6 +119,21 @@ class JitCodeCache::JniStubData {
     code_ = code;
   }
 
+  void UpdateEntryPoints(const void* entrypoint) REQUIRES_SHARED(Locks::mutator_lock_) {
+    DCHECK(IsCompiled());
+    DCHECK(entrypoint == OatQuickMethodHeader::FromCodePointer(GetCode())->GetEntryPoint());
+    instrumentation::Instrumentation* instrum = Runtime::Current()->GetInstrumentation();
+    for (ArtMethod* m : GetMethods()) {
+      // Because `m` might be in the process of being deleted:
+      // - Call the dedicated method instead of the more generic UpdateMethodsCode
+      // - Check the class status without a read barrier.
+      // TODO: Use ReadBarrier::IsMarked() if not null to check the class status.
+      if (!m->NeedsInitializationCheck<kWithoutReadBarrier>()) {
+        instrum->UpdateNativeMethodsCodeToJitCode(m, entrypoint);
+      }
+    }
+  }
+
   const void* GetCode() const {
     return code_;
   }
@@ -705,15 +720,7 @@ bool JitCodeCache::Commit(Thread* self,
       DCHECK(ContainsElement(data->GetMethods(), method))
           << "Entry inserted in NotifyCompilationOf() should contain this method.";
       data->SetCode(code_ptr);
-      instrumentation::Instrumentation* instrum = Runtime::Current()->GetInstrumentation();
-      for (ArtMethod* m : data->GetMethods()) {
-        // Because `m` might be in the process of being deleted:
-        // - Call the dedicated method instead of the more generic UpdateMethodsCode
-        // - Check the class status without a read barrier.
-        if (!m->NeedsInitializationCheck<kWithoutReadBarrier>()) {
-          instrum->UpdateNativeMethodsCodeToJitCode(m, method_header->GetEntryPoint());
-        }
-      }
+      data->UpdateEntryPoints(method_header->GetEntryPoint());
     } else {
       if (method->IsPreCompiled() && IsSharedRegion(*region)) {
         zygote_map_.Put(code_ptr, method);
@@ -1555,6 +1562,12 @@ bool JitCodeCache::NotifyCompilationOf(ArtMethod* method,
   if (method->NeedsInitializationCheck() && !prejit) {
     // Unless we're pre-jitting, we currently don't save the JIT compiled code if we cannot
     // update the entrypoint due to needing an initialization check.
+    if (method->GetDeclaringClass()->IsInitialized()) {
+      // Request visible initialization but do not block to allow compiling other methods.
+      // Hopefully, this will complete by the time the method becomes hot again.
+      Runtime::Current()->GetClassLinker()->MakeInitializedClassesVisiblyInitialized(
+          self, /*wait=*/ false);
+    }
     VLOG(jit) << "Not compiling "
               << method->PrettyMethod()
               << " because it has the resolution stub";
@@ -1587,15 +1600,7 @@ bool JitCodeCache::NotifyCompilationOf(ArtMethod* method,
       // changed these entrypoints to GenericJNI in preparation for a full GC, we may
       // as well change them back as this stub shall not be collected anyway and this
       // can avoid a few expensive GenericJNI calls.
-      instrumentation::Instrumentation* instrumentation = Runtime::Current()->GetInstrumentation();
-      for (ArtMethod* m : data->GetMethods()) {
-        // Because `m` might be in the process of being deleted:
-        // - Call the dedicated method instead of the more generic UpdateMethodsCode
-        // - Check the class status without a read barrier.
-        if (!m->NeedsInitializationCheck<kWithoutReadBarrier>()) {
-          instrumentation->UpdateNativeMethodsCodeToJitCode(m, entrypoint);
-        }
-      }
+      data->UpdateEntryPoints(entrypoint);
       if (collection_in_progress_) {
         if (!IsInZygoteExecSpace(data->GetCode())) {
           GetLiveBitmap()->AtomicTestAndSet(FromCodeToAllocation(data->GetCode()));

test/common/runtime_state.cc

@@ -221,14 +221,28 @@ static void ForceJitCompiled(Thread* self, ArtMethod* method) REQUIRES(!Locks::m
       ThrowIllegalStateException(msg.c_str());
       return;
     }
-    // We force initialization of the declaring class to make sure the method doesn't keep
-    // the resolution stub as entrypoint.
+    // We force visible initialization of the declaring class to make sure the method
+    // doesn't keep the resolution stub as entrypoint.
     StackHandleScope<1> hs(self);
     Handle<mirror::Class> h_klass(hs.NewHandle(method->GetDeclaringClass()));
-    if (!Runtime::Current()->GetClassLinker()->EnsureInitialized(self, h_klass, true, true)) {
+    ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
+    if (!class_linker->EnsureInitialized(self, h_klass, true, true)) {
       self->AssertPendingException();
       return;
     }
+    if (UNLIKELY(!h_klass->IsInitialized())) {
+      // Must be initializing in this thread.
+      CHECK_EQ(h_klass->GetStatus(), ClassStatus::kInitializing);
+      CHECK_EQ(h_klass->GetClinitThreadId(), self->GetTid());
+      std::string msg(method->PrettyMethod());
+      msg += ": is not safe to jit because the class is being initialized in this thread!";
+      ThrowIllegalStateException(msg.c_str());
+      return;
+    }
+    if (!h_klass->IsVisiblyInitialized()) {
+      ScopedThreadSuspension sts(self, ThreadState::kNative);
+      class_linker->MakeInitializedClassesVisiblyInitialized(self, /*wait=*/ true);
+    }
   }
   jit::Jit* jit = GetJitIfEnabled();
   jit::JitCodeCache* code_cache = jit->GetCodeCache();