Consider whitelist when listing class members.
Previously, only the enforcement policy was considered when getting
declared fields or members, meaning whitelisted APIs would still not be
discoverable. Fix this by calling hiddenapi::GetMemberAction from within
IsDiscoverable.
Bug: 77787686
Bug: 64382372
Test: cts/tests/signature/runSignatureTests.sh (with ag/3863796)
Test: art/test.py --host -t 674-hiddenapi
Merged-In: I234d274f47f377e3e105c81aae2d49072287992a
Change-Id: I234d274f47f377e3e105c81aae2d49072287992a
(cherry picked from commit 64ee8aeaeb70aa2d5d1c3ff57a682a5001869653)
diff --git a/runtime/hidden_api.cc b/runtime/hidden_api.cc
index 02b4f53..98feb4d 100644
--- a/runtime/hidden_api.cc
+++ b/runtime/hidden_api.cc
@@ -137,8 +137,10 @@
// Avoid re-examining the exemption list next time.
// Note this results in no warning for the member, which seems like what one would expect.
// Exemptions effectively adds new members to the whitelist.
- member->SetAccessFlags(HiddenApiAccessFlags::EncodeForRuntime(
- member->GetAccessFlags(), HiddenApiAccessFlags::kWhitelist));
+ if (runtime->ShouldDedupeHiddenApiWarnings()) {
+ member->SetAccessFlags(HiddenApiAccessFlags::EncodeForRuntime(
+ member->GetAccessFlags(), HiddenApiAccessFlags::kWhitelist));
+ }
return kAllow;
}
diff --git a/runtime/native/java_lang_Class.cc b/runtime/native/java_lang_Class.cc
index bfd7f69..2c1c963 100644
--- a/runtime/native/java_lang_Class.cc
+++ b/runtime/native/java_lang_Class.cc
@@ -128,19 +128,20 @@
// the criteria. Some reflection calls only return public members
// (public_only == true), some members should be hidden from non-boot class path
// callers (enforce_hidden_api == true).
+template<typename T>
ALWAYS_INLINE static bool IsDiscoverable(bool public_only,
bool enforce_hidden_api,
- uint32_t access_flags) {
- if (public_only && ((access_flags & kAccPublic) == 0)) {
+ T* member)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ if (public_only && ((member->GetAccessFlags() & kAccPublic) == 0)) {
return false;
}
- if (enforce_hidden_api &&
- hiddenapi::GetActionFromAccessFlags(access_flags) == hiddenapi::kDeny) {
- return false;
- }
-
- return true;
+ return hiddenapi::GetMemberAction(member,
+ nullptr,
+ [enforce_hidden_api] (Thread*) { return !enforce_hidden_api; },
+ hiddenapi::kNone)
+ != hiddenapi::kDeny;
}
ALWAYS_INLINE static inline ObjPtr<mirror::Class> DecodeClass(
@@ -269,12 +270,12 @@
bool enforce_hidden_api = ShouldEnforceHiddenApi(self);
// Lets go subtract all the non discoverable fields.
for (ArtField& field : ifields) {
- if (!IsDiscoverable(public_only, enforce_hidden_api, field.GetAccessFlags())) {
+ if (!IsDiscoverable(public_only, enforce_hidden_api, &field)) {
--array_size;
}
}
for (ArtField& field : sfields) {
- if (!IsDiscoverable(public_only, enforce_hidden_api, field.GetAccessFlags())) {
+ if (!IsDiscoverable(public_only, enforce_hidden_api, &field)) {
--array_size;
}
}
@@ -285,7 +286,7 @@
return nullptr;
}
for (ArtField& field : ifields) {
- if (IsDiscoverable(public_only, enforce_hidden_api, field.GetAccessFlags())) {
+ if (IsDiscoverable(public_only, enforce_hidden_api, &field)) {
auto* reflect_field = mirror::Field::CreateFromArtField<kRuntimePointerSize>(self,
&field,
force_resolve);
@@ -300,7 +301,7 @@
}
}
for (ArtField& field : sfields) {
- if (IsDiscoverable(public_only, enforce_hidden_api, field.GetAccessFlags())) {
+ if (IsDiscoverable(public_only, enforce_hidden_api, &field)) {
auto* reflect_field = mirror::Field::CreateFromArtField<kRuntimePointerSize>(self,
&field,
force_resolve);
@@ -521,7 +522,7 @@
DCHECK(m != nullptr);
return m->IsConstructor() &&
!m->IsStatic() &&
- IsDiscoverable(public_only, enforce_hidden_api, m->GetAccessFlags());
+ IsDiscoverable(public_only, enforce_hidden_api, m);
}
static jobjectArray Class_getDeclaredConstructorsInternal(
@@ -591,7 +592,7 @@
uint32_t modifiers = m.GetAccessFlags();
// Add non-constructor declared methods.
if ((modifiers & kAccConstructor) == 0 &&
- IsDiscoverable(public_only, enforce_hidden_api, modifiers)) {
+ IsDiscoverable(public_only, enforce_hidden_api, &m)) {
++num_methods;
}
}
@@ -605,7 +606,7 @@
for (ArtMethod& m : klass->GetDeclaredMethods(kRuntimePointerSize)) {
uint32_t modifiers = m.GetAccessFlags();
if ((modifiers & kAccConstructor) == 0 &&
- IsDiscoverable(public_only, enforce_hidden_api, modifiers)) {
+ IsDiscoverable(public_only, enforce_hidden_api, &m)) {
DCHECK_EQ(Runtime::Current()->GetClassLinker()->GetImagePointerSize(), kRuntimePointerSize);
DCHECK(!Runtime::Current()->IsActiveTransaction());
auto* method =
diff --git a/test/674-hiddenapi/src-art/Main.java b/test/674-hiddenapi/src-art/Main.java
index a808e94..782748c 100644
--- a/test/674-hiddenapi/src-art/Main.java
+++ b/test/674-hiddenapi/src-art/Main.java
@@ -16,6 +16,7 @@
import dalvik.system.InMemoryDexClassLoader;
import dalvik.system.PathClassLoader;
+import dalvik.system.VMRuntime;
import java.io.File;
import java.io.InputStream;
import java.lang.reflect.Constructor;
@@ -34,26 +35,41 @@
// Enable hidden API checks in case they are disabled by default.
init();
+ // TODO there are sequential depencies between these test cases, and bugs
+ // in the production code may lead to subsequent tests to erroneously pass,
+ // or test the wrong thing. We rely on not deduping hidden API warnings
+ // here for the same reasons), meaning the code under test and production
+ // code are running in different configurations. Each test should be run in
+ // a fresh process to ensure that they are working correcting and not
+ // accidentally interfering with eachother.
+
// Run test with both parent and child dex files loaded with class loaders.
// The expectation is that hidden members in parent should be visible to
// the child.
- doTest(false, false);
+ doTest(false, false, false);
doUnloading();
// Now append parent dex file to boot class path and run again. This time
- // the child dex file should not be able to access private APIs of the parent.
+ // the child dex file should not be able to access private APIs of the
+ // parent.
appendToBootClassLoader(DEX_PARENT_BOOT);
- doTest(true, false);
+ doTest(true, false, false);
+ doUnloading();
+
+ // Now run the same test again, but with the blacklist exmemptions list set
+ // to "L" which matches everything.
+ doTest(true, false, true);
doUnloading();
// And finally append to child to boot class path as well. With both in the
// boot class path, access should be granted.
appendToBootClassLoader(DEX_CHILD);
- doTest(true, true);
+ doTest(true, true, false);
doUnloading();
}
- private static void doTest(boolean parentInBoot, boolean childInBoot) throws Exception {
+ private static void doTest(boolean parentInBoot, boolean childInBoot, boolean whitelistAllApis)
+ throws Exception {
// Load parent dex if it is not in boot class path.
ClassLoader parentLoader = null;
if (parentInBoot) {
@@ -78,12 +94,18 @@
// be loaded once, but for some reason even classes from a class loader
// cannot register their native methods against symbols in a shared library
// loaded by their parent class loader.
- String nativeLibCopy = createNativeLibCopy(parentInBoot, childInBoot);
+ String nativeLibCopy = createNativeLibCopy(parentInBoot, childInBoot, whitelistAllApis);
+
+ if (whitelistAllApis) {
+ VMRuntime.getRuntime().setHiddenApiExemptions(new String[]{"L"});
+ }
// Invoke ChildClass.runTest
Class.forName("ChildClass", true, childLoader)
- .getDeclaredMethod("runTest", String.class, Boolean.TYPE, Boolean.TYPE)
- .invoke(null, nativeLibCopy, parentInBoot, childInBoot);
+ .getDeclaredMethod("runTest", String.class, Boolean.TYPE, Boolean.TYPE, Boolean.TYPE)
+ .invoke(null, nativeLibCopy, parentInBoot, childInBoot, whitelistAllApis);
+
+ VMRuntime.getRuntime().setHiddenApiExemptions(new String[0]);
}
// Routine which tries to figure out the absolute path of our native library.
@@ -122,20 +144,21 @@
return buffer;
}
- // Copy native library to a new file with a unique name so it does not conflict
- // with other loaded instance of the same binary file.
- private static String createNativeLibCopy(boolean parentInBoot, boolean childInBoot)
- throws Exception {
+ // Copy native library to a new file with a unique name so it does not
+ // conflict with other loaded instance of the same binary file.
+ private static String createNativeLibCopy(
+ boolean parentInBoot, boolean childInBoot, boolean whitelistAllApis) throws Exception {
String tempFileName = System.mapLibraryName(
- "hiddenapitest_" + (parentInBoot ? "1" : "0") + (childInBoot ? "1" : "0"));
+ "hiddenapitest_" + (parentInBoot ? "1" : "0") + (childInBoot ? "1" : "0") +
+ (whitelistAllApis ? "1" : "0"));
File tempFile = new File(System.getenv("DEX_LOCATION"), tempFileName);
Files.copy(new File(nativeLibFileName).toPath(), tempFile.toPath());
return tempFile.getAbsolutePath();
}
private static void doUnloading() {
- // Do multiple GCs to prevent rare flakiness if some other thread is keeping the
- // classloader live.
+ // Do multiple GCs to prevent rare flakiness if some other thread is
+ // keeping the classloader live.
for (int i = 0; i < 5; ++i) {
Runtime.getRuntime().gc();
}
diff --git a/test/674-hiddenapi/src-ex/ChildClass.java b/test/674-hiddenapi/src-ex/ChildClass.java
index e3d3e69..da0c36f 100644
--- a/test/674-hiddenapi/src-ex/ChildClass.java
+++ b/test/674-hiddenapi/src-ex/ChildClass.java
@@ -70,7 +70,7 @@
private static final boolean booleanValues[] = new boolean[] { false, true };
public static void runTest(String libFileName, boolean expectedParentInBoot,
- boolean expectedChildInBoot) throws Exception {
+ boolean expectedChildInBoot, boolean everythingWhitelisted) throws Exception {
System.load(libFileName);
// Check expectations about loading into boot class path.
@@ -84,13 +84,15 @@
throw new RuntimeException("Expected ChildClass " + (expectedChildInBoot ? "" : "not ") +
"in boot class path");
}
+ ChildClass.everythingWhitelisted = everythingWhitelisted;
boolean isSameBoot = (isParentInBoot == isChildInBoot);
// Run meaningful combinations of access flags.
for (Hiddenness hiddenness : Hiddenness.values()) {
final Behaviour expected;
- if (isSameBoot || hiddenness == Hiddenness.Whitelist) {
+ if (isSameBoot || hiddenness == Hiddenness.Whitelist ||
+ (hiddenness == Hiddenness.Blacklist && everythingWhitelisted)) {
expected = Behaviour.Granted;
} else if (hiddenness == Hiddenness.Blacklist) {
expected = Behaviour.Denied;
@@ -510,14 +512,16 @@
String fn, boolean canAccess) {
throw new RuntimeException("Expected " + (isField ? "field " : "method ") + klass.getName() +
"." + name + " to " + (canAccess ? "" : "not ") + "be discoverable with " + fn + ". " +
- "isParentInBoot = " + isParentInBoot + ", " + "isChildInBoot = " + isChildInBoot);
+ "isParentInBoot = " + isParentInBoot + ", " + "isChildInBoot = " + isChildInBoot + ", " +
+ "everythingWhitelisted = " + everythingWhitelisted);
}
private static void throwAccessException(Class<?> klass, String name, boolean isField,
String fn) {
throw new RuntimeException("Expected to be able to access " + (isField ? "field " : "method ") +
klass.getName() + "." + name + " using " + fn + ". " +
- "isParentInBoot = " + isParentInBoot + ", " + "isChildInBoot = " + isChildInBoot);
+ "isParentInBoot = " + isParentInBoot + ", " + "isChildInBoot = " + isChildInBoot + ", " +
+ "everythingWhitelisted = " + everythingWhitelisted);
}
private static void throwWarningException(Class<?> klass, String name, boolean isField,
@@ -525,7 +529,8 @@
throw new RuntimeException("Expected access to " + (isField ? "field " : "method ") +
klass.getName() + "." + name + " using " + fn + " to " + (setsWarning ? "" : "not ") +
"set the warning flag. " +
- "isParentInBoot = " + isParentInBoot + ", " + "isChildInBoot = " + isChildInBoot);
+ "isParentInBoot = " + isParentInBoot + ", " + "isChildInBoot = " + isChildInBoot + ", " +
+ "everythingWhitelisted = " + everythingWhitelisted);
}
private static void throwModifiersException(Class<?> klass, String name, boolean isField) {
@@ -535,6 +540,7 @@
private static boolean isParentInBoot;
private static boolean isChildInBoot;
+ private static boolean everythingWhitelisted;
private static native boolean hasPendingWarning();
private static native void clearWarning();
