Fix hidden API flags decoding for intrinsics
Hidden API decision logic would try to decode the access flags of
intrinsics directly, bypassing the override in ArtMethod. This patch
get hidden_api.h to use the same code path.
This also fixes CtsHiddenApiDiscoveryTestCases where the access flags
of blacklisted APIs are tested. VarHandle intrinsics would not pass.
Bug: 64382372
Bug: 72430785
Bug: 78230396
Test: cts-tradefed run cts --module CtsHiddenApiDiscoveryTestCases
Merged-In: I080313dd91bbee2d7d98b00c02e224974b344c01
Change-Id: I080313dd91bbee2d7d98b00c02e224974b344c01
(cherry picked from commit 14c212a44ac9a3ad12025ebf30836129669fa949)
diff --git a/libdexfile/dex/hidden_api_access_flags.h b/libdexfile/dex/hidden_api_access_flags.h
index b62d044..1aaeabd 100644
--- a/libdexfile/dex/hidden_api_access_flags.h
+++ b/libdexfile/dex/hidden_api_access_flags.h
@@ -86,12 +86,10 @@
}
static ALWAYS_INLINE ApiList DecodeFromRuntime(uint32_t runtime_access_flags) {
- if ((runtime_access_flags & kAccIntrinsic) != 0) {
- return kWhitelist;
- } else {
- uint32_t int_value = (runtime_access_flags & kAccHiddenApiBits) >> kAccFlagsShift;
- return static_cast<ApiList>(int_value);
- }
+ // This is used in the fast path, only DCHECK here.
+ DCHECK_EQ(runtime_access_flags & kAccIntrinsic, 0u);
+ uint32_t int_value = (runtime_access_flags & kAccHiddenApiBits) >> kAccFlagsShift;
+ return static_cast<ApiList>(int_value);
}
static ALWAYS_INLINE uint32_t EncodeForRuntime(uint32_t runtime_access_flags, ApiList value) {
diff --git a/runtime/art_field.h b/runtime/art_field.h
index 29d71af..f39af39 100644
--- a/runtime/art_field.h
+++ b/runtime/art_field.h
@@ -20,6 +20,7 @@
#include <jni.h>
#include "dex/dex_file_types.h"
+#include "dex/hidden_api_access_flags.h"
#include "dex/modifiers.h"
#include "dex/primitive.h"
#include "gc_root.h"
@@ -179,6 +180,10 @@
return (GetAccessFlags() & kAccVolatile) != 0;
}
+ HiddenApiAccessFlags::ApiList GetHiddenApiAccessFlags() REQUIRES_SHARED(Locks::mutator_lock_) {
+ return HiddenApiAccessFlags::DecodeFromRuntime(GetAccessFlags());
+ }
+
// Returns an instance field with this offset in the given class or null if not found.
// If kExactOffset is true then we only find the matching offset, not the field containing the
// offset.
diff --git a/runtime/art_method-inl.h b/runtime/art_method-inl.h
index 4d54ac1..c1fac36 100644
--- a/runtime/art_method-inl.h
+++ b/runtime/art_method-inl.h
@@ -384,7 +384,8 @@
return (GetAccessFlags<kReadBarrierOption>() & kAccSingleImplementation) != 0;
}
-inline HiddenApiAccessFlags::ApiList ArtMethod::GetHiddenApiAccessFlags() {
+inline HiddenApiAccessFlags::ApiList ArtMethod::GetHiddenApiAccessFlags()
+ REQUIRES_SHARED(Locks::mutator_lock_) {
if (UNLIKELY(IsIntrinsic())) {
switch (static_cast<Intrinsics>(GetIntrinsic())) {
case Intrinsics::kSystemArrayCopyChar:
diff --git a/runtime/art_method.h b/runtime/art_method.h
index ae13d86..acaa4a6 100644
--- a/runtime/art_method.h
+++ b/runtime/art_method.h
@@ -341,7 +341,7 @@
AddAccessFlags(kAccMustCountLocks);
}
- HiddenApiAccessFlags::ApiList GetHiddenApiAccessFlags();
+ HiddenApiAccessFlags::ApiList GetHiddenApiAccessFlags() 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/hidden_api.h b/runtime/hidden_api.h
index e117c08..5762bfe 100644
--- a/runtime/hidden_api.h
+++ b/runtime/hidden_api.h
@@ -68,17 +68,17 @@
kAccessDenied = 1 << 1,
};
-inline Action GetActionFromAccessFlags(uint32_t access_flags) {
+inline Action GetActionFromAccessFlags(HiddenApiAccessFlags::ApiList api_list) {
+ if (api_list == HiddenApiAccessFlags::kWhitelist) {
+ return kAllow;
+ }
+
EnforcementPolicy policy = Runtime::Current()->GetHiddenApiEnforcementPolicy();
if (policy == EnforcementPolicy::kNoChecks) {
// Exit early. Nothing to enforce.
return kAllow;
}
- HiddenApiAccessFlags::ApiList api_list = HiddenApiAccessFlags::DecodeFromRuntime(access_flags);
- if (api_list == HiddenApiAccessFlags::kWhitelist) {
- return kAllow;
- }
// if policy is "just warn", always warn. We returned above for whitelist APIs.
if (policy == EnforcementPolicy::kJustWarn) {
return kAllowButWarn;
@@ -169,7 +169,15 @@
REQUIRES_SHARED(Locks::mutator_lock_) {
DCHECK(member != nullptr);
- Action action = GetActionFromAccessFlags(member->GetAccessFlags());
+ // Decode hidden API access flags.
+ // NB Multiple threads might try to access (and overwrite) these simultaneously,
+ // causing a race. We only do that if access has not been denied, so the race
+ // cannot change Java semantics. We should, however, decode the access flags
+ // once and use it throughout this function, otherwise we may get inconsistent
+ // results, e.g. print whitelist warnings (b/78327881).
+ HiddenApiAccessFlags::ApiList api_list = member->GetHiddenApiAccessFlags();
+
+ Action action = GetActionFromAccessFlags(member->GetHiddenApiAccessFlags());
if (action == kAllow) {
// Nothing to do.
return action;
diff --git a/runtime/hidden_api_test.cc b/runtime/hidden_api_test.cc
index 65d6363..1985c6b 100644
--- a/runtime/hidden_api_test.cc
+++ b/runtime/hidden_api_test.cc
@@ -86,36 +86,41 @@
};
TEST_F(HiddenApiTest, CheckGetActionFromRuntimeFlags) {
- uint32_t whitelist = HiddenApiAccessFlags::EncodeForRuntime(0, HiddenApiAccessFlags::kWhitelist);
- uint32_t lightgreylist =
- HiddenApiAccessFlags::EncodeForRuntime(0, HiddenApiAccessFlags::kLightGreylist);
- uint32_t darkgreylist =
- HiddenApiAccessFlags::EncodeForRuntime(0, HiddenApiAccessFlags::kDarkGreylist);
- uint32_t blacklist = HiddenApiAccessFlags::EncodeForRuntime(0, HiddenApiAccessFlags::kBlacklist);
-
runtime_->SetHiddenApiEnforcementPolicy(hiddenapi::EnforcementPolicy::kNoChecks);
- ASSERT_EQ(GetActionFromAccessFlags(whitelist), hiddenapi::kAllow);
- ASSERT_EQ(GetActionFromAccessFlags(lightgreylist), hiddenapi::kAllow);
- ASSERT_EQ(GetActionFromAccessFlags(darkgreylist), hiddenapi::kAllow);
- ASSERT_EQ(GetActionFromAccessFlags(blacklist), hiddenapi::kAllow);
+ ASSERT_EQ(GetActionFromAccessFlags(HiddenApiAccessFlags::kWhitelist), hiddenapi::kAllow);
+ ASSERT_EQ(GetActionFromAccessFlags(HiddenApiAccessFlags::kLightGreylist), hiddenapi::kAllow);
+ ASSERT_EQ(GetActionFromAccessFlags(HiddenApiAccessFlags::kDarkGreylist), hiddenapi::kAllow);
+ ASSERT_EQ(GetActionFromAccessFlags(HiddenApiAccessFlags::kBlacklist), hiddenapi::kAllow);
runtime_->SetHiddenApiEnforcementPolicy(hiddenapi::EnforcementPolicy::kJustWarn);
- ASSERT_EQ(GetActionFromAccessFlags(whitelist), hiddenapi::kAllow);
- ASSERT_EQ(GetActionFromAccessFlags(lightgreylist), hiddenapi::kAllowButWarn);
- ASSERT_EQ(GetActionFromAccessFlags(darkgreylist), hiddenapi::kAllowButWarn);
- ASSERT_EQ(GetActionFromAccessFlags(blacklist), hiddenapi::kAllowButWarn);
+ ASSERT_EQ(GetActionFromAccessFlags(HiddenApiAccessFlags::kWhitelist),
+ hiddenapi::kAllow);
+ ASSERT_EQ(GetActionFromAccessFlags(HiddenApiAccessFlags::kLightGreylist),
+ hiddenapi::kAllowButWarn);
+ ASSERT_EQ(GetActionFromAccessFlags(HiddenApiAccessFlags::kDarkGreylist),
+ hiddenapi::kAllowButWarn);
+ ASSERT_EQ(GetActionFromAccessFlags(HiddenApiAccessFlags::kBlacklist),
+ hiddenapi::kAllowButWarn);
runtime_->SetHiddenApiEnforcementPolicy(hiddenapi::EnforcementPolicy::kDarkGreyAndBlackList);
- ASSERT_EQ(GetActionFromAccessFlags(whitelist), hiddenapi::kAllow);
- ASSERT_EQ(GetActionFromAccessFlags(lightgreylist), hiddenapi::kAllowButWarn);
- ASSERT_EQ(GetActionFromAccessFlags(darkgreylist), hiddenapi::kDeny);
- ASSERT_EQ(GetActionFromAccessFlags(blacklist), hiddenapi::kDeny);
+ ASSERT_EQ(GetActionFromAccessFlags(HiddenApiAccessFlags::kWhitelist),
+ hiddenapi::kAllow);
+ ASSERT_EQ(GetActionFromAccessFlags(HiddenApiAccessFlags::kLightGreylist),
+ hiddenapi::kAllowButWarn);
+ ASSERT_EQ(GetActionFromAccessFlags(HiddenApiAccessFlags::kDarkGreylist),
+ hiddenapi::kDeny);
+ ASSERT_EQ(GetActionFromAccessFlags(HiddenApiAccessFlags::kBlacklist),
+ hiddenapi::kDeny);
runtime_->SetHiddenApiEnforcementPolicy(hiddenapi::EnforcementPolicy::kBlacklistOnly);
- ASSERT_EQ(GetActionFromAccessFlags(whitelist), hiddenapi::kAllow);
- ASSERT_EQ(GetActionFromAccessFlags(lightgreylist), hiddenapi::kAllowButWarn);
- ASSERT_EQ(GetActionFromAccessFlags(darkgreylist), hiddenapi::kAllowButWarnAndToast);
- ASSERT_EQ(GetActionFromAccessFlags(blacklist), hiddenapi::kDeny);
+ ASSERT_EQ(GetActionFromAccessFlags(HiddenApiAccessFlags::kWhitelist),
+ hiddenapi::kAllow);
+ ASSERT_EQ(GetActionFromAccessFlags(HiddenApiAccessFlags::kLightGreylist),
+ hiddenapi::kAllowButWarn);
+ ASSERT_EQ(GetActionFromAccessFlags(HiddenApiAccessFlags::kDarkGreylist),
+ hiddenapi::kAllowButWarnAndToast);
+ ASSERT_EQ(GetActionFromAccessFlags(HiddenApiAccessFlags::kBlacklist),
+ hiddenapi::kDeny);
}
TEST_F(HiddenApiTest, CheckMembersRead) {