commit b8c6619f16348403c3933d9e425b4b8c80af5389
author David Brazdil <dbrazdil@google.com> Mon Apr 23 13:51:16 2018 +0100
committer David Brazdil <dbrazdil@google.com> Mon Apr 23 16:08:12 2018 +0100
tree 38642b8514fd9b036731c0a02112ec62e80b3e91
parent 14c212a44ac9a3ad12025ebf30836129669fa949

Fix race in hidden_api.h

The hidden API decision logic can dedupe warnings by changing the
access flags of a previously warned about method to whitelist, thus
exiting early on the next attempt to access it. This logic had a race
in it, because it would decode the flags, make a decision and then
decode them again when printing a warning. If another thread changed
the flags in between, the warning would say "whitelist".

Change the code so that the hidden API flags are decoded only once.
This may still result in multiple warnings about the same API, but at
least the messages will be consistent.

Bug: 78327881
Bug: 64382372
Test: none
Change-Id: I956dd56536bdfce492845e6a90fdcbe29c2676b5

runtime/hidden_api.cc

diff --git a/runtime/hidden_api.cc b/runtime/hidden_api.cc
index 612a930..bc6a8f2 100644
--- a/runtime/hidden_api.cc
+++ b/runtime/hidden_api.cc
@@ -172,7 +172,10 @@
 }
 
 template<typename T>
-Action GetMemberActionImpl(T* member, Action action, AccessMethod access_method) {
+Action GetMemberActionImpl(T* member,
+                           HiddenApiAccessFlags::ApiList api_list,
+                           Action action,
+                           AccessMethod access_method) {
   DCHECK_NE(action, kAllow);
 
   // Get the signature, we need it later.
@@ -202,8 +205,7 @@
     if (access_method != kNone) {
       // Print a log message with information about this class member access.
       // We do this if we're about to block access, or the app is debuggable.
-      member_signature.WarnAboutAccess(access_method,
-          HiddenApiAccessFlags::DecodeFromRuntime(member->GetAccessFlags()));
+      member_signature.WarnAboutAccess(access_method, api_list);
     }
   }
 
@@ -244,9 +246,11 @@
 
 // Need to instantiate this.
 template Action GetMemberActionImpl<ArtField>(ArtField* member,
+                                              HiddenApiAccessFlags::ApiList api_list,
                                               Action action,
                                               AccessMethod access_method);
 template Action GetMemberActionImpl<ArtMethod>(ArtMethod* member,
+                                               HiddenApiAccessFlags::ApiList api_list,
                                                Action action,
                                                AccessMethod access_method);
 }  // namespace detail

runtime/hidden_api.h

diff --git a/runtime/hidden_api.h b/runtime/hidden_api.h
index 5762bfe..65c6406 100644
--- a/runtime/hidden_api.h
+++ b/runtime/hidden_api.h
@@ -134,7 +134,10 @@
 };
 
 template<typename T>
-Action GetMemberActionImpl(T* member, Action action, AccessMethod access_method)
+Action GetMemberActionImpl(T* member,
+                           HiddenApiAccessFlags::ApiList api_list,
+                           Action action,
+                           AccessMethod access_method)
     REQUIRES_SHARED(Locks::mutator_lock_);
 
 // Returns true if the caller is either loaded by the boot strap class loader or comes from
@@ -191,7 +194,7 @@
   }
 
   // Member is hidden and caller is not in the platform.
-  return detail::GetMemberActionImpl(member, action, access_method);
+  return detail::GetMemberActionImpl(member, api_list, action, access_method);
 }
 
 inline bool IsCallerInPlatformDex(ObjPtr<mirror::Class> caller)