Revert "Revert "ART: Remove unnecessary SHARED_REQUIRES in ArtMethod""
This reverts commit 38b8287004770e4d20dcc0e0fe4697060917ec72.
Do not always use ScopedObjectAccess. The GC might call this during
sanity checking, at which point the state is not runnable, but the
mutator lock is still held.
Currently needs an ugly NO_THREAD_SAFETY_ANALYSIS helper, as the
assert isn't correctly handled (establishing the held mutator lock).
Change-Id: Ie79e85e2afedc9b989382d88155b09e426fe7f75
diff --git a/runtime/art_method-inl.h b/runtime/art_method-inl.h
index d6b2b7e..632a50f 100644
--- a/runtime/art_method-inl.h
+++ b/runtime/art_method-inl.h
@@ -35,6 +35,8 @@
#include "quick/quick_method_frame_info.h"
#include "read_barrier-inl.h"
#include "runtime-inl.h"
+#include "scoped_thread_state_change.h"
+#include "thread-inl.h"
#include "utils.h"
namespace art {
@@ -75,9 +77,28 @@
expected_root, desired_root);
}
+// AssertSharedHeld doesn't work in GetAccessFlags, so use a NO_THREAD_SAFETY_ANALYSIS helper.
+// TODO: Figure out why ASSERT_SHARED_CAPABILITY doesn't work.
+ALWAYS_INLINE
+static inline void DoGetAccessFlagsHelper(ArtMethod* method) NO_THREAD_SAFETY_ANALYSIS {
+ CHECK(method->IsRuntimeMethod() || method->GetDeclaringClass()->IsIdxLoaded() ||
+ method->GetDeclaringClass()->IsErroneous());
+}
+
inline uint32_t ArtMethod::GetAccessFlags() {
- DCHECK(IsRuntimeMethod() || GetDeclaringClass()->IsIdxLoaded() ||
- GetDeclaringClass()->IsErroneous());
+ if (kIsDebugBuild) {
+ Thread* self = Thread::Current();
+ if (!Locks::mutator_lock_->IsSharedHeld(self)) {
+ ScopedObjectAccess soa(self);
+ CHECK(IsRuntimeMethod() || GetDeclaringClass()->IsIdxLoaded() ||
+ GetDeclaringClass()->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(this);
+ }
+ }
return access_flags_;
}
diff --git a/runtime/art_method.h b/runtime/art_method.h
index f78c827..0315c3a 100644
--- a/runtime/art_method.h
+++ b/runtime/art_method.h
@@ -75,7 +75,9 @@
return MemberOffset(OFFSETOF_MEMBER(ArtMethod, declaring_class_));
}
- ALWAYS_INLINE uint32_t GetAccessFlags() SHARED_REQUIRES(Locks::mutator_lock_);
+ // Note: GetAccessFlags acquires the mutator lock in debug mode to check that it is not called for
+ // a proxy method.
+ ALWAYS_INLINE uint32_t GetAccessFlags();
void SetAccessFlags(uint32_t new_access_flags) {
// Not called within a transaction.
@@ -86,77 +88,78 @@
InvokeType GetInvokeType() SHARED_REQUIRES(Locks::mutator_lock_);
// Returns true if the method is declared public.
- bool IsPublic() SHARED_REQUIRES(Locks::mutator_lock_) {
+ bool IsPublic() {
return (GetAccessFlags() & kAccPublic) != 0;
}
// Returns true if the method is declared private.
- bool IsPrivate() SHARED_REQUIRES(Locks::mutator_lock_) {
+ bool IsPrivate() {
return (GetAccessFlags() & kAccPrivate) != 0;
}
// Returns true if the method is declared static.
- bool IsStatic() SHARED_REQUIRES(Locks::mutator_lock_) {
+ bool IsStatic() {
return (GetAccessFlags() & kAccStatic) != 0;
}
// Returns true if the method is a constructor.
- bool IsConstructor() SHARED_REQUIRES(Locks::mutator_lock_) {
+ bool IsConstructor() {
return (GetAccessFlags() & kAccConstructor) != 0;
}
// Returns true if the method is a class initializer.
- bool IsClassInitializer() SHARED_REQUIRES(Locks::mutator_lock_) {
+ bool IsClassInitializer() {
return IsConstructor() && IsStatic();
}
// Returns true if the method is static, private, or a constructor.
- bool IsDirect() SHARED_REQUIRES(Locks::mutator_lock_) {
+ bool IsDirect() {
return IsDirect(GetAccessFlags());
}
static bool IsDirect(uint32_t access_flags) {
- return (access_flags & (kAccStatic | kAccPrivate | kAccConstructor)) != 0;
+ constexpr uint32_t direct = kAccStatic | kAccPrivate | kAccConstructor;
+ return (access_flags & direct) != 0;
}
// Returns true if the method is declared synchronized.
- bool IsSynchronized() SHARED_REQUIRES(Locks::mutator_lock_) {
- uint32_t synchonized = kAccSynchronized | kAccDeclaredSynchronized;
+ bool IsSynchronized() {
+ constexpr uint32_t synchonized = kAccSynchronized | kAccDeclaredSynchronized;
return (GetAccessFlags() & synchonized) != 0;
}
- bool IsFinal() SHARED_REQUIRES(Locks::mutator_lock_) {
+ bool IsFinal() {
return (GetAccessFlags() & kAccFinal) != 0;
}
- bool IsMiranda() SHARED_REQUIRES(Locks::mutator_lock_) {
+ bool IsMiranda() {
return (GetAccessFlags() & kAccMiranda) != 0;
}
- bool IsNative() SHARED_REQUIRES(Locks::mutator_lock_) {
+ bool IsNative() {
return (GetAccessFlags() & kAccNative) != 0;
}
- bool IsFastNative() SHARED_REQUIRES(Locks::mutator_lock_) {
- uint32_t mask = kAccFastNative | kAccNative;
+ bool IsFastNative() {
+ constexpr uint32_t mask = kAccFastNative | kAccNative;
return (GetAccessFlags() & mask) == mask;
}
- bool IsAbstract() SHARED_REQUIRES(Locks::mutator_lock_) {
+ bool IsAbstract() {
return (GetAccessFlags() & kAccAbstract) != 0;
}
- bool IsSynthetic() SHARED_REQUIRES(Locks::mutator_lock_) {
+ bool IsSynthetic() {
return (GetAccessFlags() & kAccSynthetic) != 0;
}
bool IsProxyMethod() SHARED_REQUIRES(Locks::mutator_lock_);
- bool IsPreverified() SHARED_REQUIRES(Locks::mutator_lock_) {
+ bool IsPreverified() {
return (GetAccessFlags() & kAccPreverified) != 0;
}
- void SetPreverified() SHARED_REQUIRES(Locks::mutator_lock_) {
+ void SetPreverified() {
DCHECK(!IsPreverified());
SetAccessFlags(GetAccessFlags() | kAccPreverified);
}
@@ -404,7 +407,7 @@
return GetNativePointer<void*>(EntryPointFromJniOffset(pointer_size), pointer_size);
}
- void SetEntryPointFromJni(const void* entrypoint) SHARED_REQUIRES(Locks::mutator_lock_) {
+ void SetEntryPointFromJni(const void* entrypoint) {
DCHECK(IsNative());
SetEntryPointFromJniPtrSize(entrypoint, sizeof(void*));
}