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)

6 files changed, 54 insertions, 37 deletions

diff --git a/libdexfile/dex/hidden_api_access_flags.h b/libdexfile/dex/hidden_api_access_flags.h
index b62d044c6a..1aaeabd783 100644
--- a/libdexfile/dex/hidden_api_access_flags.h
+++ b/libdexfile/dex/hidden_api_access_flags.h
@@ -86,12 +86,10 @@ class HiddenApiAccessFlags {
   }
 
   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 29d71af358..f39af3900c 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 @@ class ArtField FINAL {
     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 4d54ac1d9f..c1fac364bb 100644
--- a/runtime/art_method-inl.h
+++ b/runtime/art_method-inl.h
@@ -384,7 +384,8 @@ inline bool ArtMethod::HasSingleImplementation() {
   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 ae13d86934..acaa4a68a1 100644
--- a/runtime/art_method.h
+++ b/runtime/art_method.h
@@ -341,7 +341,7 @@ class ArtMethod FINAL {
     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 e117c08c25..5762bfefc1 100644
--- a/runtime/hidden_api.h
+++ b/runtime/hidden_api.h
@@ -68,17 +68,17 @@ enum AccessContextFlags {
   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 @@ inline Action GetMemberAction(T* member,
     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 65d6363bfd..1985c6bb19 100644
--- a/runtime/hidden_api_test.cc
+++ b/runtime/hidden_api_test.cc
@@ -86,36 +86,41 @@ class HiddenApiTest : public CommonRuntimeTest {
 };
 
 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) {