Remove ArtMethod's declaring class state checks.
This check was not very useful because the Class is already
in a state to pass the check when we're constructing the
ArtMethod and it can never revert to an earlier state, so
the check is essentially a weak protection against GC bugs.
Besides not being very useful, the check had the ability to
invalidate ObjPtr<> cookies (when called in non-runnable
state), making it difficult to fully ObjPtr<>-ify the code.
Also remove a lot of kReadBarrierOption template parameters
which were needed specifically for this check. This removes
unnecessary maintence burden as shown by past bugs dealing
with carefully adding those parameters where necessary.
Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing
Bug: 74373650
Bug: 31113334
Change-Id: I87f2999fc4e7c27b5c2307139269b4b5f6649d16
diff --git a/compiler/optimizing/stack_map_stream.cc b/compiler/optimizing/stack_map_stream.cc
index a65fbcc..d74d7b6 100644
--- a/compiler/optimizing/stack_map_stream.cc
+++ b/compiler/optimizing/stack_map_stream.cc
@@ -179,7 +179,7 @@
ScopedObjectAccess soa(Thread::Current());
DCHECK(IsSameDexFile(*outer_dex_file, *method->GetDexFile()));
}
- uint32_t dex_method_index = method->GetDexMethodIndexUnchecked();
+ uint32_t dex_method_index = method->GetDexMethodIndex();
entry[InlineInfo::kMethodInfoIndex] = method_infos_.Dedup({dex_method_index});
}
current_inline_infos_.push_back(entry);
@@ -196,8 +196,7 @@
if (encode_art_method) {
CHECK_EQ(inline_info.GetArtMethod(), method);
} else {
- CHECK_EQ(method_infos_[inline_info.GetMethodInfoIndex()][0],
- method->GetDexMethodIndexUnchecked());
+ CHECK_EQ(method_infos_[inline_info.GetMethodInfoIndex()][0], method->GetDexMethodIndex());
}
});
}
diff --git a/dex2oat/linker/oat_writer.cc b/dex2oat/linker/oat_writer.cc
index 22e7024..065e31a 100644
--- a/dex2oat/linker/oat_writer.cc
+++ b/dex2oat/linker/oat_writer.cc
@@ -1570,9 +1570,7 @@
// Find origin method. Declaring class and dex_method_idx
// in the copied method should be the same as in the origin
// method.
- // FIXME: Cannot use ObjPtr<> for declaring class because origin->IsDirect()
- // invalidates ObjPtr<> cookie for kCheckDeclaringClassState == true.
- mirror::Class* declaring_class = method.GetDeclaringClass().Ptr();
+ ObjPtr<mirror::Class> declaring_class = method.GetDeclaringClass();
ArtMethod* origin = declaring_class->FindClassMethod(
declaring_class->GetDexCache(),
method.GetDexMethodIndex(),
diff --git a/runtime/art_method-inl.h b/runtime/art_method-inl.h
index 9da4a39..ac22f07 100644
--- a/runtime/art_method-inl.h
+++ b/runtime/art_method-inl.h
@@ -60,12 +60,6 @@
if (kIsDebugBuild) {
if (!IsRuntimeMethod()) {
CHECK(result != nullptr) << this;
- if (kCheckDeclaringClassState) {
- if (!(result->IsIdxLoaded() || result->IsErroneous())) {
- LOG(FATAL_WITHOUT_ABORT) << "Class status: " << result->GetStatus();
- LOG(FATAL) << result->PrettyClass();
- }
- }
} else {
CHECK(result == nullptr) << this;
}
@@ -94,16 +88,6 @@
return method_index_;
}
-template <ReadBarrierOption kReadBarrierOption>
-inline uint32_t ArtMethod::GetDexMethodIndex() {
- if (kCheckDeclaringClassState) {
- CHECK(IsRuntimeMethod() ||
- GetDeclaringClass<kReadBarrierOption>()->IsIdxLoaded() ||
- GetDeclaringClass<kReadBarrierOption>()->IsErroneous());
- }
- return GetDexMethodIndexUnchecked();
-}
-
inline ObjPtr<mirror::Class> ArtMethod::LookupResolvedClassFromTypeIndex(dex::TypeIndex type_idx) {
ScopedAssertNoThreadSuspension ants(__FUNCTION__);
ObjPtr<mirror::Class> type =
@@ -196,14 +180,7 @@
inline const char* ArtMethod::GetShorty(uint32_t* out_length) {
DCHECK(!IsProxyMethod());
const DexFile* dex_file = GetDexFile();
- // Don't do a read barrier in the DCHECK() inside GetDexMethodIndex() as GetShorty()
- // can be called when the declaring class is about to be unloaded and cannot be added
- // to the mark stack (subsequent GC assertion would fail).
- // It is safe to avoid the read barrier as the ArtMethod is constructed with a declaring
- // Class already satisfying the DCHECK() inside GetDexMethodIndex(), so even if that copy
- // of declaring class becomes a from-space object, it shall satisfy the DCHECK().
- return dex_file->GetMethodShorty(dex_file->GetMethodId(GetDexMethodIndex<kWithoutReadBarrier>()),
- out_length);
+ return dex_file->GetMethodShorty(dex_file->GetMethodId(GetDexMethodIndex()), out_length);
}
inline const Signature ArtMethod::GetSignature() {
@@ -329,7 +306,7 @@
template <ReadBarrierOption kReadBarrierOption>
inline mirror::DexCache* ArtMethod::GetDexCache() {
- if (LIKELY(!IsObsolete<kReadBarrierOption>())) {
+ if (LIKELY(!IsObsolete())) {
ObjPtr<mirror::Class> klass = GetDeclaringClass<kReadBarrierOption>();
return klass->GetDexCache<kDefaultVerifyFlags, kReadBarrierOption>();
} else {
@@ -382,12 +359,12 @@
template <ReadBarrierOption kReadBarrierOption>
inline bool ArtMethod::HasSingleImplementation() {
- if (IsFinal<kReadBarrierOption>() || GetDeclaringClass<kReadBarrierOption>()->IsFinal()) {
+ if (IsFinal() || GetDeclaringClass<kReadBarrierOption>()->IsFinal()) {
// We don't set kAccSingleImplementation for these cases since intrinsic
// can use the flag also.
return true;
}
- return (GetAccessFlags<kReadBarrierOption>() & kAccSingleImplementation) != 0;
+ return (GetAccessFlags() & kAccSingleImplementation) != 0;
}
inline HiddenApiAccessFlags::ApiList ArtMethod::GetHiddenApiAccessFlags()
@@ -529,9 +506,9 @@
}
}
-template <ReadBarrierOption kReadBarrierOption, typename Visitor>
+template <typename Visitor>
inline void ArtMethod::UpdateEntrypoints(const Visitor& visitor, PointerSize pointer_size) {
- if (IsNative<kReadBarrierOption>()) {
+ if (IsNative()) {
const void* old_native_code = GetEntryPointFromJniPtrSize(pointer_size);
const void* new_native_code = visitor(old_native_code);
if (old_native_code != new_native_code) {
diff --git a/runtime/art_method.cc b/runtime/art_method.cc
index 80b6921..68ccfee 100644
--- a/runtime/art_method.cc
+++ b/runtime/art_method.cc
@@ -59,8 +59,6 @@
extern "C" void art_quick_invoke_static_stub(ArtMethod*, uint32_t*, uint32_t, Thread*, JValue*,
const char*);
-DEFINE_RUNTIME_DEBUG_FLAG(ArtMethod, kCheckDeclaringClassState);
-
// Enforce that we he have the right index for runtime methods.
static_assert(ArtMethod::kRuntimeMethodDexMethodIndex == dex::kDexNoIndex,
"Wrong runtime-method dex method index");
@@ -90,18 +88,13 @@
}
}
-template <ReadBarrierOption kReadBarrierOption>
ArtMethod* ArtMethod::GetSingleImplementation(PointerSize pointer_size) {
- if (!IsAbstract<kReadBarrierOption>()) {
+ if (!IsAbstract()) {
// A non-abstract's single implementation is itself.
return this;
}
return reinterpret_cast<ArtMethod*>(GetDataPtrSize(pointer_size));
}
-template ArtMethod* ArtMethod::GetSingleImplementation<ReadBarrierOption::kWithReadBarrier>(
- PointerSize pointer_size);
-template ArtMethod* ArtMethod::GetSingleImplementation<ReadBarrierOption::kWithoutReadBarrier>(
- PointerSize pointer_size);
ArtMethod* ArtMethod::FromReflectedMethod(const ScopedObjectAccessAlreadyRunnable& soa,
jobject jlr_method) {
@@ -801,25 +794,4 @@
method->GetDeclaringClass<kReadBarrierOption>()->IsErroneous());
}
-template <ReadBarrierOption kReadBarrierOption> void ArtMethod::GetAccessFlagsDCheck() {
- if (kCheckDeclaringClassState) {
- Thread* self = Thread::Current();
- if (!Locks::mutator_lock_->IsSharedHeld(self)) {
- if (self->IsThreadSuspensionAllowable()) {
- ScopedObjectAccess soa(self);
- CHECK(IsRuntimeMethod() ||
- GetDeclaringClass<kReadBarrierOption>()->IsIdxLoaded() ||
- GetDeclaringClass<kReadBarrierOption>()->IsErroneous());
- }
- } else {
- // We cannot use SOA in this case. We might be holding the lock, but may not be in the
- // runnable state (e.g., during GC).
- Locks::mutator_lock_->AssertSharedHeld(self);
- DoGetAccessFlagsHelper<kReadBarrierOption>(this);
- }
- }
-}
-template void ArtMethod::GetAccessFlagsDCheck<ReadBarrierOption::kWithReadBarrier>();
-template void ArtMethod::GetAccessFlagsDCheck<ReadBarrierOption::kWithoutReadBarrier>();
-
} // namespace art
diff --git a/runtime/art_method.h b/runtime/art_method.h
index f5ef3fa..ce08cb0 100644
--- a/runtime/art_method.h
+++ b/runtime/art_method.h
@@ -107,13 +107,7 @@
return MemberOffset(OFFSETOF_MEMBER(ArtMethod, declaring_class_));
}
- // Note: GetAccessFlags acquires the mutator lock in debug mode to check that it is not called for
- // a proxy method.
- template <ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
uint32_t GetAccessFlags() {
- if (kCheckDeclaringClassState) {
- GetAccessFlagsDCheck<kReadBarrierOption>();
- }
return access_flags_.load(std::memory_order_relaxed);
}
@@ -172,14 +166,12 @@
return (GetAccessFlags() & synchonized) != 0;
}
- template <ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
bool IsFinal() {
- return (GetAccessFlags<kReadBarrierOption>() & kAccFinal) != 0;
+ return (GetAccessFlags() & kAccFinal) != 0;
}
- template <ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
bool IsIntrinsic() {
- return (GetAccessFlags<kReadBarrierOption>() & kAccIntrinsic) != 0;
+ return (GetAccessFlags() & kAccIntrinsic) != 0;
}
ALWAYS_INLINE void SetIntrinsic(uint32_t intrinsic) REQUIRES_SHARED(Locks::mutator_lock_);
@@ -241,25 +233,22 @@
}
// This is set by the class linker.
- template <ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
bool IsDefault() {
static_assert((kAccDefault & (kAccIntrinsic | kAccIntrinsicBits)) == 0,
"kAccDefault conflicts with intrinsic modifier");
- return (GetAccessFlags<kReadBarrierOption>() & kAccDefault) != 0;
+ return (GetAccessFlags() & kAccDefault) != 0;
}
- template <ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
bool IsObsolete() {
- return (GetAccessFlags<kReadBarrierOption>() & kAccObsoleteMethod) != 0;
+ return (GetAccessFlags() & kAccObsoleteMethod) != 0;
}
void SetIsObsolete() {
AddAccessFlags(kAccObsoleteMethod);
}
- template <ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
bool IsNative() {
- return (GetAccessFlags<kReadBarrierOption>() & kAccNative) != 0;
+ return (GetAccessFlags() & kAccNative) != 0;
}
// Checks to see if the method was annotated with @dalvik.annotation.optimization.FastNative.
@@ -280,9 +269,8 @@
return (GetAccessFlags() & mask) == mask;
}
- template <ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
bool IsAbstract() {
- return (GetAccessFlags<kReadBarrierOption>() & kAccAbstract) != 0;
+ return (GetAccessFlags() & kAccAbstract) != 0;
}
bool IsSynthetic() {
@@ -305,7 +293,7 @@
void SetSkipAccessChecks() {
// SkipAccessChecks() is applicable only to non-native methods.
- DCHECK(!IsNative<kWithoutReadBarrier>());
+ DCHECK(!IsNative());
AddAccessFlags(kAccSkipAccessChecks);
}
@@ -317,13 +305,12 @@
return (GetAccessFlags() & kAccPreviouslyWarm) != 0;
}
- template <ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
void SetPreviouslyWarm() {
- if (IsIntrinsic<kReadBarrierOption>()) {
+ if (IsIntrinsic()) {
// kAccPreviouslyWarm overlaps with kAccIntrinsicBits.
return;
}
- AddAccessFlags<kReadBarrierOption>(kAccPreviouslyWarm);
+ AddAccessFlags(kAccPreviouslyWarm);
}
// Should this method be run in the interpreter and count locks (e.g., failed structured-
@@ -384,11 +371,9 @@
// Number of 32bit registers that would be required to hold all the arguments
static size_t NumArgRegisters(const StringPiece& shorty);
- ALWAYS_INLINE uint32_t GetDexMethodIndexUnchecked() {
+ ALWAYS_INLINE uint32_t GetDexMethodIndex() {
return dex_method_index_;
}
- template <ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
- ALWAYS_INLINE uint32_t GetDexMethodIndex() REQUIRES_SHARED(Locks::mutator_lock_);
void SetDexMethodIndex(uint32_t new_idx) {
// Not called within a transaction.
@@ -472,11 +457,7 @@
}
ProfilingInfo* GetProfilingInfo(PointerSize pointer_size) REQUIRES_SHARED(Locks::mutator_lock_) {
- // Don't do a read barrier in the DCHECK() inside GetAccessFlags() called by IsNative(),
- // as GetProfilingInfo is called in places where the declaring class is treated as a weak
- // reference (accessing it with a read barrier would either prevent unloading the class,
- // or crash the runtime if the GC wants to unload it).
- if (UNLIKELY(IsNative<kWithoutReadBarrier>()) || UNLIKELY(IsProxyMethod())) {
+ if (UNLIKELY(IsNative()) || UNLIKELY(IsProxyMethod())) {
return nullptr;
}
return reinterpret_cast<ProfilingInfo*>(GetDataPtrSize(pointer_size));
@@ -513,15 +494,12 @@
ArtMethod* GetCanonicalMethod(PointerSize pointer_size = kRuntimePointerSize)
REQUIRES_SHARED(Locks::mutator_lock_);
- template <ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
- ArtMethod* GetSingleImplementation(PointerSize pointer_size)
- REQUIRES_SHARED(Locks::mutator_lock_);
+ ArtMethod* GetSingleImplementation(PointerSize pointer_size);
- template <ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
ALWAYS_INLINE void SetSingleImplementation(ArtMethod* method, PointerSize pointer_size) {
- DCHECK(!IsNative<kReadBarrierOption>());
+ DCHECK(!IsNative());
// Non-abstract method's single implementation is just itself.
- DCHECK(IsAbstract<kReadBarrierOption>());
+ DCHECK(IsAbstract());
SetDataPtrSize(method, pointer_size);
}
@@ -713,7 +691,7 @@
REQUIRES_SHARED(Locks::mutator_lock_);
// Update entry points by passing them through the visitor.
- template <ReadBarrierOption kReadBarrierOption = kWithReadBarrier, typename Visitor>
+ template <typename Visitor>
ALWAYS_INLINE void UpdateEntrypoints(const Visitor& visitor, PointerSize pointer_size);
// Visit the individual members of an ArtMethod. Used by imgdiag.
@@ -833,8 +811,6 @@
}
}
- template <ReadBarrierOption kReadBarrierOption> void GetAccessFlagsDCheck();
-
static inline bool IsValidIntrinsicUpdate(uint32_t modifier) {
return (((modifier & kAccIntrinsic) == kAccIntrinsic) &&
(((modifier & ~(kAccIntrinsic | kAccIntrinsicBits)) == 0)));
@@ -845,9 +821,8 @@
}
// This setter guarantees atomicity.
- template <ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
void AddAccessFlags(uint32_t flag) {
- DCHECK(!IsIntrinsic<kReadBarrierOption>() ||
+ DCHECK(!IsIntrinsic() ||
!OverlapsIntrinsicBits(flag) ||
IsValidIntrinsicUpdate(flag));
uint32_t old_access_flags;
diff --git a/runtime/cha.cc b/runtime/cha.cc
index f2e6a73..ccbe066 100644
--- a/runtime/cha.cc
+++ b/runtime/cha.cc
@@ -142,12 +142,12 @@
ArtMethod* super_method = super_it->
GetVTableEntry<kDefaultVerifyFlags, kWithoutReadBarrier>(vtbl_index, pointer_size);
- if (super_method->IsAbstract<kWithoutReadBarrier>() &&
+ if (super_method->IsAbstract() &&
super_method->HasSingleImplementation<kWithoutReadBarrier>() &&
- super_method->GetSingleImplementation<kWithoutReadBarrier>(pointer_size) == method) {
+ super_method->GetSingleImplementation(pointer_size) == method) {
// Do like there was no single implementation defined previously
// for this method of the superclass.
- super_method->SetSingleImplementation<kWithoutReadBarrier>(nullptr, pointer_size);
+ super_method->SetSingleImplementation(nullptr, pointer_size);
} else {
// No related SingleImplementations could possibly be found any further.
DCHECK(!super_method->HasSingleImplementation<kWithoutReadBarrier>());
@@ -168,11 +168,10 @@
++j) {
ArtMethod* method = interface->GetVirtualMethod(j, pointer_size);
if (method->HasSingleImplementation<kWithoutReadBarrier>() &&
- alloc->ContainsUnsafe(
- method->GetSingleImplementation<kWithoutReadBarrier>(pointer_size)) &&
- !method->IsDefault<kWithoutReadBarrier>()) {
+ alloc->ContainsUnsafe(method->GetSingleImplementation(pointer_size)) &&
+ !method->IsDefault()) {
// Do like there was no single implementation defined previously for this method.
- method->SetSingleImplementation<kWithoutReadBarrier>(nullptr, pointer_size);
+ method->SetSingleImplementation(nullptr, pointer_size);
}
}
}
diff --git a/runtime/gc/space/image_space.cc b/runtime/gc/space/image_space.cc
index cbce940..0936a53 100644
--- a/runtime/gc/space/image_space.cc
+++ b/runtime/gc/space/image_space.cc
@@ -1163,7 +1163,7 @@
if (fixup_heap_objects_) {
method->UpdateObjectsForImageRelocation(ForwardObjectAdapter(this));
}
- method->UpdateEntrypoints<kWithoutReadBarrier>(ForwardCodeAdapter(this), pointer_size_);
+ method->UpdateEntrypoints(ForwardCodeAdapter(this), pointer_size_);
}
}
diff --git a/runtime/jit/jit_code_cache.cc b/runtime/jit/jit_code_cache.cc
index 70a7171..9d8dab4 100644
--- a/runtime/jit/jit_code_cache.cc
+++ b/runtime/jit/jit_code_cache.cc
@@ -759,11 +759,7 @@
static void ClearMethodCounter(ArtMethod* method, bool was_warm) {
if (was_warm) {
- // Don't do any read barrier, as the declaring class of `method` may
- // be in the process of being GC'ed (reading the declaring class is done
- // when DCHECKing the declaring class is resolved, which we know it is
- // at this point).
- method->SetPreviouslyWarm<kWithoutReadBarrier>();
+ method->SetPreviouslyWarm();
}
// We reset the counter to 1 so that the profile knows that the method was executed at least once.
// This is required for layout purposes.
