Move hidden API warnings into resolvers
Checks inserted to maybe warn about accessing a hidden API member
are proving too expensive. Move them into the resolvers.
Bug: 64382372
Bug: 72649186
Test: art/test.py --host -b -r -t 674-hiddenapi -v
Change-Id: I11bceccb8fe647e829c491be38ba0af72f1b3996
diff --git a/runtime/hidden_api.h b/runtime/hidden_api.h
index de3a51a..7e41c1d 100644
--- a/runtime/hidden_api.h
+++ b/runtime/hidden_api.h
@@ -24,144 +24,104 @@
namespace art {
namespace hiddenapi {
-// Returns true if member with `access flags` should only be accessed from
-// boot class path.
-inline bool IsMemberHidden(uint32_t access_flags) {
- if (!Runtime::Current()->AreHiddenApiChecksEnabled()) {
- return false;
- }
+enum Action {
+ kAllow,
+ kAllowButWarn,
+ kDeny
+};
+inline Action GetMemberAction(uint32_t access_flags) {
switch (HiddenApiAccessFlags::DecodeFromRuntime(access_flags)) {
case HiddenApiAccessFlags::kWhitelist:
+ return kAllow;
case HiddenApiAccessFlags::kLightGreylist:
case HiddenApiAccessFlags::kDarkGreylist:
- return false;
+ return kAllowButWarn;
case HiddenApiAccessFlags::kBlacklist:
- return true;
+ return kDeny;
}
}
-// Returns true if we should warn about non-boot class path accessing member
-// with `access_flags`.
-inline bool ShouldWarnAboutMember(uint32_t access_flags) {
- if (!Runtime::Current()->AreHiddenApiChecksEnabled()) {
- return false;
- }
-
- switch (HiddenApiAccessFlags::DecodeFromRuntime(access_flags)) {
- case HiddenApiAccessFlags::kWhitelist:
- return false;
- case HiddenApiAccessFlags::kLightGreylist:
- case HiddenApiAccessFlags::kDarkGreylist:
- return true;
- case HiddenApiAccessFlags::kBlacklist:
- // We should never access a blacklisted member from non-boot class path,
- // but this function is called before we establish the origin of the access.
- // Return false here, we do not want to warn when boot class path accesses
- // a blacklisted member.
- return false;
- }
-}
-
-// Returns true if caller `num_frames` up the stack is in boot class path.
-inline bool IsCallerInBootClassPath(Thread* self, size_t num_frames)
- REQUIRES_SHARED(Locks::mutator_lock_) {
- ObjPtr<mirror::Class> klass = GetCallingClass(self, num_frames);
- if (klass == nullptr) {
- // Unattached native thread. Assume that this is *not* boot class path.
- return false;
- }
- return klass->IsBootStrapClassLoaded();
-}
-
-// Returns true if `caller` should not be allowed to access member with `access_flags`.
-inline bool ShouldBlockAccessToMember(uint32_t access_flags, mirror::Class* caller)
- REQUIRES_SHARED(Locks::mutator_lock_) {
- return IsMemberHidden(access_flags) &&
- !caller->IsBootStrapClassLoaded();
-}
-
-// Returns true if `caller` should not be allowed to access `member`.
-template<typename T>
-inline bool ShouldBlockAccessToMember(T* member, ArtMethod* caller)
- REQUIRES_SHARED(Locks::mutator_lock_) {
- DCHECK(member != nullptr);
- DCHECK(!caller->IsRuntimeMethod());
- return ShouldBlockAccessToMember(member->GetAccessFlags(), caller->GetDeclaringClass());
-}
-
-// Returns true if the caller `num_frames` up the stack should not be allowed
-// to access `member`.
-template<typename T>
-inline bool ShouldBlockAccessToMember(T* member, Thread* self, size_t num_frames)
- REQUIRES_SHARED(Locks::mutator_lock_) {
- DCHECK(member != nullptr);
- return IsMemberHidden(member->GetAccessFlags()) &&
- !IsCallerInBootClassPath(self, num_frames); // This is expensive. Save it for last.
-}
-
// Issue a warning about field access.
inline void WarnAboutMemberAccess(ArtField* field) REQUIRES_SHARED(Locks::mutator_lock_) {
- Runtime::Current()->SetPendingHiddenApiWarning(true);
LOG(WARNING) << "Access to hidden field " << field->PrettyField();
}
// Issue a warning about method access.
inline void WarnAboutMemberAccess(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_) {
- Runtime::Current()->SetPendingHiddenApiWarning(true);
LOG(WARNING) << "Access to hidden method " << method->PrettyMethod();
}
-// Set access flags of `member` to be in hidden API whitelist. This can be disabled
-// with a Runtime::SetDedupHiddenApiWarnings.
+// Returns true if access to `member` should be denied to the caller of the
+// reflective query. The decision is based on whether the caller is in boot
+// class path or not. Because different users of this function determine this
+// in a different way, `fn_caller_in_boot(self)` is called and should return
+// true if the caller is in boot class path.
+// This function might print warnings into the log if the member is greylisted.
template<typename T>
-inline void MaybeWhitelistMember(T* member) REQUIRES_SHARED(Locks::mutator_lock_) {
- if (Runtime::Current()->ShouldDedupeHiddenApiWarnings()) {
- member->SetAccessFlags(HiddenApiAccessFlags::EncodeForRuntime(
- member->GetAccessFlags(), HiddenApiAccessFlags::kWhitelist));
- DCHECK(!ShouldWarnAboutMember(member->GetAccessFlags()));
- }
-}
-
-// Check if `caller` should be allowed to access `member` and warn if not.
-template<typename T>
-inline void MaybeWarnAboutMemberAccess(T* member, ArtMethod* caller)
+inline bool ShouldBlockAccessToMember(T* member,
+ Thread* self,
+ std::function<bool(Thread*)> fn_caller_in_boot)
REQUIRES_SHARED(Locks::mutator_lock_) {
DCHECK(member != nullptr);
- DCHECK(!caller->IsRuntimeMethod());
- if (!Runtime::Current()->AreHiddenApiChecksEnabled() ||
- member == nullptr ||
- !ShouldWarnAboutMember(member->GetAccessFlags()) ||
- caller->GetDeclaringClass()->IsBootStrapClassLoaded()) {
- return;
+
+ if (!Runtime::Current()->AreHiddenApiChecksEnabled()) {
+ // Exit early. Nothing to enforce.
+ return false;
}
- WarnAboutMember(member);
- MaybeWhitelistMember(member);
+ Action action = GetMemberAction(member->GetAccessFlags());
+ if (action == kAllow) {
+ // Nothing to do.
+ return false;
+ }
+
+ // Member is hidden. Walk the stack to find the caller.
+ // This can be *very* expensive. Save it for last.
+ if (fn_caller_in_boot(self)) {
+ // Caller in boot class path. Exit.
+ return false;
+ }
+
+ // Member is hidden and we are not in the boot class path. Act accordingly.
+ if (action == kAllowButWarn) {
+ // Allow access to this member but print a warning. Depending on a runtime
+ // flag, we might move the member into whitelist and skip the warning the
+ // next time the member is used.
+ Runtime::Current()->SetPendingHiddenApiWarning(true);
+ if (Runtime::Current()->ShouldDedupeHiddenApiWarnings()) {
+ member->SetAccessFlags(HiddenApiAccessFlags::EncodeForRuntime(
+ member->GetAccessFlags(), HiddenApiAccessFlags::kWhitelist));
+ }
+ WarnAboutMemberAccess(member);
+ return false;
+ } else {
+ DCHECK_EQ(action, hiddenapi::kDeny);
+ return true;
+ }
}
-// Check if the caller `num_frames` up the stack should be allowed to access
-// `member` and warn if not.
-template<typename T>
-inline void MaybeWarnAboutMemberAccess(T* member, Thread* self, size_t num_frames)
+// Returns true if access to member with `access_flags` should be denied to `caller`.
+// This function should be called on statically linked uses of hidden API.
+inline bool ShouldBlockAccessToMember(uint32_t access_flags, mirror::Class* caller)
REQUIRES_SHARED(Locks::mutator_lock_) {
- if (!Runtime::Current()->AreHiddenApiChecksEnabled() ||
- member == nullptr ||
- !ShouldWarnAboutMember(member->GetAccessFlags())) {
- return;
+ if (!Runtime::Current()->AreHiddenApiChecksEnabled()) {
+ // Exit early. Nothing to enforce.
+ return false;
}
- // Walk the stack to find the caller. This is *very* expensive. Save it for last.
- ObjPtr<mirror::Class> klass = GetCallingClass(self, num_frames);
- if (klass == nullptr) {
- // Unattached native thread, assume that this is *not* boot class path
- // and enforce the rules.
- } else if (klass->IsBootStrapClassLoaded()) {
- return;
+ // Only continue if we want to deny access. Warnings are *not* printed.
+ if (GetMemberAction(access_flags) != kDeny) {
+ return false;
}
- WarnAboutMemberAccess(member);
- MaybeWhitelistMember(member);
+ // Member is hidden. Check if the caller is in boot class path.
+ if (caller == nullptr) {
+ // The caller is unknown. We assume that this is *not* boot class path.
+ return true;
+ }
+
+ return !caller->IsBootStrapClassLoaded();
}
} // namespace hiddenapi
diff --git a/runtime/interpreter/unstarted_runtime.cc b/runtime/interpreter/unstarted_runtime.cc
index b9b0051..85acc71 100644
--- a/runtime/interpreter/unstarted_runtime.cc
+++ b/runtime/interpreter/unstarted_runtime.cc
@@ -176,6 +176,13 @@
return param->AsString();
}
+template<typename T>
+static ALWAYS_INLINE bool ShouldBlockAccessToMember(T* member, ShadowFrame* frame)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ return hiddenapi::ShouldBlockAccessToMember(
+ member->GetAccessFlags(), frame->GetMethod()->GetDeclaringClass());
+}
+
void UnstartedRuntime::UnstartedClassForNameCommon(Thread* self,
ShadowFrame* shadow_frame,
JValue* result,
@@ -267,8 +274,7 @@
auto* cl = Runtime::Current()->GetClassLinker();
if (cl->EnsureInitialized(self, h_klass, true, true)) {
ArtMethod* cons = h_klass->FindConstructor("()V", cl->GetImagePointerSize());
- if (cons != nullptr &&
- hiddenapi::ShouldBlockAccessToMember(cons, shadow_frame->GetMethod())) {
+ if (cons != nullptr && ShouldBlockAccessToMember(cons, shadow_frame)) {
cons = nullptr;
}
if (cons != nullptr) {
@@ -313,8 +319,7 @@
}
}
}
- if (found != nullptr &&
- hiddenapi::ShouldBlockAccessToMember(found, shadow_frame->GetMethod())) {
+ if (found != nullptr && ShouldBlockAccessToMember(found, shadow_frame)) {
found = nullptr;
}
if (found == nullptr) {
@@ -379,8 +384,7 @@
self, klass, name, args);
}
}
- if (method != nullptr &&
- hiddenapi::ShouldBlockAccessToMember(method->GetArtMethod(), shadow_frame->GetMethod())) {
+ if (method != nullptr && ShouldBlockAccessToMember(method->GetArtMethod(), shadow_frame)) {
method = nullptr;
}
result->SetL(method);
@@ -418,8 +422,7 @@
}
}
if (constructor != nullptr &&
- hiddenapi::ShouldBlockAccessToMember(
- constructor->GetArtMethod(), shadow_frame->GetMethod())) {
+ ShouldBlockAccessToMember(constructor->GetArtMethod(), shadow_frame)) {
constructor = nullptr;
}
result->SetL(constructor);
diff --git a/runtime/jni_internal.cc b/runtime/jni_internal.cc
index c40360f..158a182 100644
--- a/runtime/jni_internal.cc
+++ b/runtime/jni_internal.cc
@@ -80,17 +80,28 @@
// things not rendering correctly. E.g. b/16858794
static constexpr bool kWarnJniAbort = false;
-// Helpers to check if we need to warn about accessing hidden API fields and to call instrumentation
-// functions for them. These take jobjects so we don't need to set up handles for the rare case
-// where these actually do something. Once these functions return it is possible there will be
-// a pending exception if the instrumentation happens to throw one.
+static bool IsCallerInBootClassPath(Thread* self) REQUIRES_SHARED(Locks::mutator_lock_) {
+ ObjPtr<mirror::Class> klass = GetCallingClass(self, /* num_frames */ 1);
+ // If `klass` is null, it is an unattached native thread. Assume this is
+ // *not* boot class path.
+ return klass != nullptr && klass->IsBootStrapClassLoaded();
+}
+
+template<typename T>
+ALWAYS_INLINE static bool ShouldBlockAccessToMember(T* member, Thread* self)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ return hiddenapi::ShouldBlockAccessToMember(member, self, IsCallerInBootClassPath);
+}
+
+// Helpers to call instrumentation functions for fields. These take jobjects so we don't need to set
+// up handles for the rare case where these actually do something. Once these functions return it is
+// possible there will be a pending exception if the instrumentation happens to throw one.
static void NotifySetObjectField(ArtField* field, jobject obj, jobject jval)
REQUIRES_SHARED(Locks::mutator_lock_) {
DCHECK_EQ(field->GetTypeAsPrimitiveType(), Primitive::kPrimNot);
- Thread* self = Thread::Current();
- hiddenapi::MaybeWarnAboutMemberAccess(field, self, /* num_frames */ 1);
instrumentation::Instrumentation* instrumentation = Runtime::Current()->GetInstrumentation();
if (UNLIKELY(instrumentation->HasFieldWriteListeners())) {
+ Thread* self = Thread::Current();
ArtMethod* cur_method = self->GetCurrentMethod(/*dex_pc*/ nullptr,
/*check_suspended*/ true,
/*abort_on_error*/ false);
@@ -115,10 +126,9 @@
static void NotifySetPrimitiveField(ArtField* field, jobject obj, JValue val)
REQUIRES_SHARED(Locks::mutator_lock_) {
DCHECK_NE(field->GetTypeAsPrimitiveType(), Primitive::kPrimNot);
- Thread* self = Thread::Current();
- hiddenapi::MaybeWarnAboutMemberAccess(field, self, /* num_frames */ 1);
instrumentation::Instrumentation* instrumentation = Runtime::Current()->GetInstrumentation();
if (UNLIKELY(instrumentation->HasFieldWriteListeners())) {
+ Thread* self = Thread::Current();
ArtMethod* cur_method = self->GetCurrentMethod(/*dex_pc*/ nullptr,
/*check_suspended*/ true,
/*abort_on_error*/ false);
@@ -140,10 +150,9 @@
static void NotifyGetField(ArtField* field, jobject obj)
REQUIRES_SHARED(Locks::mutator_lock_) {
- Thread* self = Thread::Current();
- hiddenapi::MaybeWarnAboutMemberAccess(field, self, /* num_frames */ 1);
instrumentation::Instrumentation* instrumentation = Runtime::Current()->GetInstrumentation();
if (UNLIKELY(instrumentation->HasFieldReadListeners())) {
+ Thread* self = Thread::Current();
ArtMethod* cur_method = self->GetCurrentMethod(/*dex_pc*/ nullptr,
/*check_suspended*/ true,
/*abort_on_error*/ false);
@@ -243,8 +252,7 @@
} else {
method = c->FindClassMethod(name, sig, pointer_size);
}
- if (method != nullptr &&
- hiddenapi::ShouldBlockAccessToMember(method, soa.Self(), /* num_frames */ 1)) {
+ if (method != nullptr && ShouldBlockAccessToMember(method, soa.Self())) {
method = nullptr;
}
if (method == nullptr || method->IsStatic() != is_static) {
@@ -323,8 +331,7 @@
} else {
field = c->FindInstanceField(name, field_type->GetDescriptor(&temp));
}
- if (field != nullptr &&
- hiddenapi::ShouldBlockAccessToMember(field, soa.Self(), /* num_frames */ 1)) {
+ if (field != nullptr && ShouldBlockAccessToMember(field, soa.Self())) {
field = nullptr;
}
if (field == nullptr) {
diff --git a/runtime/native/java_lang_Class.cc b/runtime/native/java_lang_Class.cc
index 5544275..1a3867f 100644
--- a/runtime/native/java_lang_Class.cc
+++ b/runtime/native/java_lang_Class.cc
@@ -48,12 +48,8 @@
namespace art {
-ALWAYS_INLINE static bool ShouldEnforceHiddenApi(Thread* self)
- REQUIRES_SHARED(Locks::mutator_lock_) {
- if (!Runtime::Current()->AreHiddenApiChecksEnabled()) {
- return false;
- }
-
+// Returns true if the first non-ClassClass caller up the stack is in boot class path.
+static bool IsCallerInBootClassPath(Thread* self) REQUIRES_SHARED(Locks::mutator_lock_) {
// Walk the stack and find the first frame not from java.lang.Class.
// This is very expensive. Save this till the last.
struct FirstNonClassClassCallerVisitor : public StackVisitor {
@@ -84,8 +80,16 @@
FirstNonClassClassCallerVisitor visitor(self);
visitor.WalkStack();
- return visitor.caller == nullptr ||
- !visitor.caller->GetDeclaringClass()->IsBootStrapClassLoaded();
+ return visitor.caller != nullptr &&
+ visitor.caller->GetDeclaringClass()->IsBootStrapClassLoaded();
+}
+
+// Returns true if the first non-ClassClass caller up the stack is not allowed to
+// access hidden APIs. This can be *very* expensive. Never call this in a loop.
+ALWAYS_INLINE static bool ShouldEnforceHiddenApi(Thread* self)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ return Runtime::Current()->AreHiddenApiChecksEnabled() &&
+ !IsCallerInBootClassPath(self);
}
// Returns true if the first non-ClassClass caller up the stack should not be
@@ -93,9 +97,7 @@
template<typename T>
ALWAYS_INLINE static bool ShouldBlockAccessToMember(T* member, Thread* self)
REQUIRES_SHARED(Locks::mutator_lock_) {
- DCHECK(member != nullptr);
- return hiddenapi::IsMemberHidden(member->GetAccessFlags()) &&
- ShouldEnforceHiddenApi(self);
+ return hiddenapi::ShouldBlockAccessToMember(member, self, IsCallerInBootClassPath);
}
// Returns true if a class member should be discoverable with reflection given
@@ -109,14 +111,13 @@
return false;
}
- if (enforce_hidden_api && hiddenapi::IsMemberHidden(access_flags)) {
+ if (enforce_hidden_api && hiddenapi::GetMemberAction(access_flags) == hiddenapi::kDeny) {
return false;
}
return true;
}
-
ALWAYS_INLINE static inline ObjPtr<mirror::Class> DecodeClass(
const ScopedFastNativeObjectAccess& soa, jobject java_class)
REQUIRES_SHARED(Locks::mutator_lock_) {
@@ -831,7 +832,6 @@
return nullptr;
}
}
- hiddenapi::MaybeWarnAboutMemberAccess(constructor, soa.Self(), /* num_frames */ 1);
// Invoke the constructor.
JValue result;
uint32_t args[1] = { static_cast<uint32_t>(reinterpret_cast<uintptr_t>(receiver.Get())) };
diff --git a/runtime/native/java_lang_reflect_Field.cc b/runtime/native/java_lang_reflect_Field.cc
index db7f4bb..f990c04 100644
--- a/runtime/native/java_lang_reflect_Field.cc
+++ b/runtime/native/java_lang_reflect_Field.cc
@@ -25,7 +25,6 @@
#include "common_throws.h"
#include "dex/dex_file-inl.h"
#include "dex/dex_file_annotations.h"
-#include "hidden_api.h"
#include "jni_internal.h"
#include "mirror/class-inl.h"
#include "mirror/field-inl.h"
@@ -162,9 +161,6 @@
DCHECK(soa.Self()->IsExceptionPending());
return nullptr;
}
-
- hiddenapi::MaybeWarnAboutMemberAccess(f->GetArtField(), soa.Self(), /* num_frames */ 1);
-
// We now don't expect suspension unless an exception is thrown.
// Get the field's value, boxing if necessary.
Primitive::Type field_type = f->GetTypeAsPrimitiveType();
@@ -187,14 +183,13 @@
DCHECK(soa.Self()->IsExceptionPending());
return JValue();
}
+
// If field is not set to be accessible, verify it can be accessed by the caller.
if (!f->IsAccessible() && !VerifyFieldAccess<false>(soa.Self(), f, o)) {
DCHECK(soa.Self()->IsExceptionPending());
return JValue();
}
- hiddenapi::MaybeWarnAboutMemberAccess(f->GetArtField(), soa.Self(), /* num_frames */ 1);
-
// We now don't expect suspension unless an exception is thrown.
// Read the value.
Primitive::Type field_type = f->GetTypeAsPrimitiveType();
@@ -356,15 +351,11 @@
DCHECK(soa.Self()->IsExceptionPending());
return;
}
-
// If field is not set to be accessible, verify it can be accessed by the caller.
if (!f->IsAccessible() && !VerifyFieldAccess<true>(soa.Self(), f, o)) {
DCHECK(soa.Self()->IsExceptionPending());
return;
}
-
- hiddenapi::MaybeWarnAboutMemberAccess(f->GetArtField(), soa.Self(), /* num_frames */ 1);
-
SetFieldValue(o, f, field_prim_type, true, unboxed_value);
}
@@ -400,8 +391,6 @@
return;
}
- hiddenapi::MaybeWarnAboutMemberAccess(f->GetArtField(), soa.Self(), /* num_frames */ 1);
-
// Write the value.
SetFieldValue(o, f, field_type, false, wide_value);
}
diff --git a/runtime/reflection.cc b/runtime/reflection.cc
index 6ffafe0..635a03a 100644
--- a/runtime/reflection.cc
+++ b/runtime/reflection.cc
@@ -465,9 +465,6 @@
}
ArtMethod* method = jni::DecodeArtMethod(mid);
-
- hiddenapi::MaybeWarnAboutMemberAccess(method, soa.Self(), /* num_frames */ 1);
-
bool is_string_init = method->GetDeclaringClass()->IsStringClass() && method->IsConstructor();
if (is_string_init) {
// Replace calls to String.<init> with equivalent StringFactory call.
@@ -499,9 +496,6 @@
}
ArtMethod* method = jni::DecodeArtMethod(mid);
-
- hiddenapi::MaybeWarnAboutMemberAccess(method, soa.Self(), /* num_frames */ 1);
-
bool is_string_init = method->GetDeclaringClass()->IsStringClass() && method->IsConstructor();
if (is_string_init) {
// Replace calls to String.<init> with equivalent StringFactory call.
@@ -534,9 +528,6 @@
ObjPtr<mirror::Object> receiver = soa.Decode<mirror::Object>(obj);
ArtMethod* method = FindVirtualMethod(receiver, jni::DecodeArtMethod(mid));
-
- hiddenapi::MaybeWarnAboutMemberAccess(method, soa.Self(), /* num_frames */ 1);
-
bool is_string_init = method->GetDeclaringClass()->IsStringClass() && method->IsConstructor();
if (is_string_init) {
// Replace calls to String.<init> with equivalent StringFactory call.
@@ -569,9 +560,6 @@
ObjPtr<mirror::Object> receiver = soa.Decode<mirror::Object>(obj);
ArtMethod* method = FindVirtualMethod(receiver, jni::DecodeArtMethod(mid));
-
- hiddenapi::MaybeWarnAboutMemberAccess(method, soa.Self(), /* num_frames */ 1);
-
bool is_string_init = method->GetDeclaringClass()->IsStringClass() && method->IsConstructor();
if (is_string_init) {
// Replace calls to String.<init> with equivalent StringFactory call.
@@ -616,8 +604,6 @@
}
}
- hiddenapi::MaybeWarnAboutMemberAccess(m, soa.Self(), num_frames);
-
ObjPtr<mirror::Object> receiver;
if (!m->IsStatic()) {
// Replace calls to String.<init> with equivalent StringFactory call.