Clean up initialization checks for entrypoints.
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: Ia3c2fdb616a5bb289e5afeccd4e6fe3eaf7ed697
diff --git a/dex2oat/linker/image_writer.cc b/dex2oat/linker/image_writer.cc
index dd29fa2..ede5ef7 100644
--- a/dex2oat/linker/image_writer.cc
+++ b/dex2oat/linker/image_writer.cc
@@ -42,6 +42,7 @@
#include "driver/compiler_options.h"
#include "elf/elf_utils.h"
#include "elf_file.h"
+#include "entrypoints/entrypoint_utils-inl.h"
#include "gc/accounting/card_table-inl.h"
#include "gc/accounting/heap_bitmap.h"
#include "gc/accounting/space_bitmap-inl.h"
@@ -3368,12 +3369,14 @@
if (quick_code == nullptr) {
// If we don't have code, use generic jni / interpreter bridge.
+ // Both perform class initialization check if needed.
quick_code = method->IsNative()
? GetOatAddress(StubType::kQuickGenericJNITrampoline)
: GetOatAddress(StubType::kQuickToInterpreterBridge);
- } else if (method->NeedsInitializationCheck()) {
- // If we do have code and the method needs an initialization check, go through
- // the resolution stub.
+ } else if (NeedsClinitCheckBeforeCall(method) &&
+ !method->GetDeclaringClass()->IsVisiblyInitialized()) {
+ // If we do have code but the method needs a class initialization check before calling
+ // that code, install the resolution stub that will perform the check.
quick_code = GetOatAddress(StubType::kQuickResolutionTrampoline);
}
return quick_code;
diff --git a/runtime/art_method-inl.h b/runtime/art_method-inl.h
index 2e0f1eb..3aec1c3 100644
--- a/runtime/art_method-inl.h
+++ b/runtime/art_method-inl.h
@@ -430,16 +430,6 @@
imt_index_ = ~ImTable::GetImtIndex(this);
}
-template <ReadBarrierOption kReadBarrierOption>
-bool ArtMethod::NeedsInitializationCheck() {
- // 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>()->IsVisiblyInitialized();
-}
-
} // namespace art
#endif // ART_RUNTIME_ART_METHOD_INL_H_
diff --git a/runtime/art_method.h b/runtime/art_method.h
index 612c566..d84ea7c 100644
--- a/runtime/art_method.h
+++ b/runtime/art_method.h
@@ -365,11 +365,6 @@
AddAccessFlags(kAccMustCountLocks);
}
- // Returns whether the method can be invoked directly or needs a
- // class initialization check.
- template <ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
- bool NeedsInitializationCheck() REQUIRES_SHARED(Locks::mutator_lock_);
-
// Returns true if this method could be overridden by a default method.
bool IsOverridableByDefaultMethod() REQUIRES_SHARED(Locks::mutator_lock_);
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 650f907..9c61386 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -69,7 +69,7 @@
#include "dex/dex_file_loader.h"
#include "dex/signature-inl.h"
#include "dex/utf.h"
-#include "entrypoints/entrypoint_utils.h"
+#include "entrypoints/entrypoint_utils-inl.h"
#include "entrypoints/runtime_asm_entrypoints.h"
#include "experimental_flags.h"
#include "gc/accounting/card_table-inl.h"
@@ -3804,9 +3804,10 @@
method->IsNative() ? GetQuickGenericJniStub() : GetQuickToInterpreterBridge());
} else if (enter_interpreter) {
method->SetEntryPointFromQuickCompiledCode(GetQuickToInterpreterBridge());
- } else if (method->NeedsInitializationCheck()) {
- // If there is compiled code, and the method needs to make sure the class is
- // initialized before execution, install the resolution stub.
+ } else if (NeedsClinitCheckBeforeCall(method)) {
+ DCHECK(!method->GetDeclaringClass()->IsVisiblyInitialized()); // Actually ClassStatus::Idx.
+ // If we do have code but the method needs a class initialization check before calling
+ // that code, install the resolution stub that will perform the check.
// It will be replaced by the proper entry point by ClassLinker::FixupStaticTrampolines
// after initializing class (see ClassLinker::InitializeClass method).
method->SetEntryPointFromQuickCompiledCode(GetQuickResolutionStub());
diff --git a/runtime/entrypoints/entrypoint_utils-inl.h b/runtime/entrypoints/entrypoint_utils-inl.h
index fe12a7c..cc1a7f8 100644
--- a/runtime/entrypoints/entrypoint_utils-inl.h
+++ b/runtime/entrypoints/entrypoint_utils-inl.h
@@ -771,6 +771,13 @@
}
}
+inline bool NeedsClinitCheckBeforeCall(ArtMethod* method) {
+ // 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 method->IsStatic() && !method->IsConstructor();
+}
+
} // namespace art
#endif // ART_RUNTIME_ENTRYPOINTS_ENTRYPOINT_UTILS_INL_H_
diff --git a/runtime/entrypoints/entrypoint_utils.h b/runtime/entrypoints/entrypoint_utils.h
index 6e75a1e..6403d90 100644
--- a/runtime/entrypoints/entrypoint_utils.h
+++ b/runtime/entrypoints/entrypoint_utils.h
@@ -212,6 +212,10 @@
ArtMethod* GetCalleeSaveOuterMethod(Thread* self, CalleeSaveType type)
REQUIRES_SHARED(Locks::mutator_lock_);
+// Returns whether we need to do class initialization check before invoking the method.
+// The caller is responsible for performing that check.
+bool NeedsClinitCheckBeforeCall(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_);
+
} // namespace art
#endif // ART_RUNTIME_ENTRYPOINTS_ENTRYPOINT_UTILS_H_
diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
index 4a127eb..34a165f 100644
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -774,7 +774,7 @@
self->PushShadowFrame(shadow_frame);
self->EndAssertNoThreadSuspension(old_cause);
- if (method->IsStatic()) {
+ if (NeedsClinitCheckBeforeCall(method)) {
ObjPtr<mirror::Class> declaring_class = method->GetDeclaringClass();
if (UNLIKELY(!declaring_class->IsVisiblyInitialized())) {
// Ensure static method's class is initialized.
@@ -1456,10 +1456,13 @@
<< invoke_type << " " << orig_called->GetVtableIndex();
}
- // Ensure that the called method's class is initialized.
- StackHandleScope<1> hs(soa.Self());
- Handle<mirror::Class> called_class(hs.NewHandle(called->GetDeclaringClass()));
- linker->EnsureInitialized(soa.Self(), called_class, true, true);
+ ObjPtr<mirror::Class> called_class = called->GetDeclaringClass();
+ if (NeedsClinitCheckBeforeCall(called) && !called_class->IsVisiblyInitialized()) {
+ // Ensure that the called method's class is initialized.
+ StackHandleScope<1> hs(soa.Self());
+ HandleWrapperObjPtr<mirror::Class> h_called_class(hs.NewHandleWrapper(&called_class));
+ linker->EnsureInitialized(soa.Self(), h_called_class, true, true);
+ }
bool force_interpreter = self->IsForceInterpreter() && !called->IsNative();
if (called_class->IsInitialized() || called_class->IsInitializing()) {
if (UNLIKELY(force_interpreter ||
@@ -2379,15 +2382,18 @@
// We can set the entrypoint of a native method to generic JNI even when the
// class hasn't been initialized, so we need to do the initialization check
// before invoking the native code.
- if (called->NeedsInitializationCheck()) {
- // Ensure static method's class is initialized.
- StackHandleScope<1> hs(self);
- Handle<mirror::Class> h_class(hs.NewHandle(called->GetDeclaringClass()));
- if (!Runtime::Current()->GetClassLinker()->EnsureInitialized(self, h_class, true, true)) {
- DCHECK(Thread::Current()->IsExceptionPending()) << called->PrettyMethod();
- self->PopHandleScope();
- // A negative value denotes an error.
- return GetTwoWordFailureValue();
+ if (NeedsClinitCheckBeforeCall(called)) {
+ ObjPtr<mirror::Class> declaring_class = called->GetDeclaringClass();
+ if (UNLIKELY(!declaring_class->IsVisiblyInitialized())) {
+ // Ensure static method's class is initialized.
+ StackHandleScope<1> hs(self);
+ Handle<mirror::Class> h_class(hs.NewHandle(declaring_class));
+ if (!Runtime::Current()->GetClassLinker()->EnsureInitialized(self, h_class, true, true)) {
+ DCHECK(Thread::Current()->IsExceptionPending()) << called->PrettyMethod();
+ self->PopHandleScope();
+ // A negative value denotes an error.
+ return GetTwoWordFailureValue();
+ }
}
}
diff --git a/runtime/jit/jit.cc b/runtime/jit/jit.cc
index c6e0702..11619c4 100644
--- a/runtime/jit/jit.cc
+++ b/runtime/jit/jit.cc
@@ -31,6 +31,7 @@
#include "debugger.h"
#include "dex/type_lookup_table.h"
#include "gc/space/image_space.h"
+#include "entrypoints/entrypoint_utils-inl.h"
#include "entrypoints/runtime_asm_entrypoints.h"
#include "image-inl.h"
#include "interpreter/interpreter.h"
@@ -1249,7 +1250,8 @@
return false;
}
if (UNLIKELY(method->IsPreCompiled()) && !with_backedges /* don't check for OSR */) {
- if (!method->NeedsInitializationCheck()) {
+ if (!NeedsClinitCheckBeforeCall(method) ||
+ method->GetDeclaringClass()->IsVisiblyInitialized()) {
const void* entry_point = code_cache_->GetSavedEntryPointOfPreCompiledMethod(method);
if (entry_point != nullptr) {
Runtime::Current()->GetInstrumentation()->UpdateMethodsCode(method, entry_point);
diff --git a/runtime/jit/jit_code_cache.cc b/runtime/jit/jit_code_cache.cc
index e1db1f6..6a13d59 100644
--- a/runtime/jit/jit_code_cache.cc
+++ b/runtime/jit/jit_code_cache.cc
@@ -37,6 +37,7 @@
#include "debugger_interface.h"
#include "dex/dex_file_loader.h"
#include "dex/method_reference.h"
+#include "entrypoints/entrypoint_utils-inl.h"
#include "entrypoints/runtime_asm_entrypoints.h"
#include "gc/accounting/bitmap-inl.h"
#include "gc/allocator/dlmalloc.h"
@@ -126,9 +127,29 @@
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>()) {
+ // - Check the class status without a full read barrier; use ReadBarrier::IsMarked().
+ bool can_set_entrypoint = true;
+ if (NeedsClinitCheckBeforeCall(m)) {
+ // To avoid resurrecting an unreachable object, we must not use a full read
+ // barrier but we do not want to miss updating an entrypoint under common
+ // circumstances, i.e. during a GC the class becomes visibly initialized,
+ // the method becomes hot, we compile the thunk and want to update the
+ // entrypoint while the method's declaring class field still points to the
+ // from-space class object with the old status. Therefore we read the
+ // declaring class without a read barrier and check if it's already marked.
+ // If yes, we check the status of the to-space class object as intended.
+ // Otherwise, there is no to-space object and the from-space class object
+ // contains the most recent value of the status field; even if this races
+ // with another thread doing a read barrier and updating the status, that's
+ // no different from a race with a thread that just updates the status.
+ // Such race can happen only for the zygote method pre-compilation, as we
+ // otherwise compile only thunks for methods of visibly initialized classes.
+ ObjPtr<mirror::Class> klass = m->GetDeclaringClass<kWithoutReadBarrier>();
+ ObjPtr<mirror::Class> marked = ReadBarrier::IsMarked(klass.Ptr());
+ ObjPtr<mirror::Class> checked_klass = (marked != nullptr) ? marked : klass;
+ can_set_entrypoint = checked_klass->IsVisiblyInitialized();
+ }
+ if (can_set_entrypoint) {
instrum->UpdateNativeMethodsCodeToJitCode(m, entrypoint);
}
}
@@ -730,7 +751,8 @@
if (osr) {
number_of_osr_compilations_++;
osr_code_map_.Put(method, code_ptr);
- } else if (method->NeedsInitializationCheck()) {
+ } else if (NeedsClinitCheckBeforeCall(method) &&
+ !method->GetDeclaringClass()->IsVisiblyInitialized()) {
// This situation currently only occurs in the jit-zygote mode.
DCHECK(Runtime::Current()->IsUsingApexBootImageLocation());
DCHECK(!garbage_collect_code_);
@@ -1559,21 +1581,27 @@
return false;
}
- 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);
+ if (NeedsClinitCheckBeforeCall(method) && !prejit) {
+ // We do not need a synchronization barrier for checking the visibly initialized status
+ // or checking the initialized status just for requesting visible initialization.
+ ClassStatus status = method->GetDeclaringClass()
+ ->GetStatus<kDefaultVerifyFlags, /*kWithSynchronizationBarrier=*/ false>();
+ if (status != ClassStatus::kVisiblyInitialized) {
+ // 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 (status == ClassStatus::kInitialized) {
+ // 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";
+ // Give it a new chance to be hot.
+ ClearMethodCounter(method, /*was_warm=*/ false);
+ return false;
}
- VLOG(jit) << "Not compiling "
- << method->PrettyMethod()
- << " because it has the resolution stub";
- // Give it a new chance to be hot.
- ClearMethodCounter(method, /*was_warm=*/ false);
- return false;
}
MutexLock mu(self, *Locks::jit_lock_);