Add support for unknown accessibility in access control
Change-Id: I1ad41b291aa95011b854bc2b41bcb738bdc979d7
diff --git a/src/main/java/com/android/tools/r8/graph/AccessControl.java b/src/main/java/com/android/tools/r8/graph/AccessControl.java
index 0f219bd..dd6dd27 100644
--- a/src/main/java/com/android/tools/r8/graph/AccessControl.java
+++ b/src/main/java/com/android/tools/r8/graph/AccessControl.java
@@ -3,6 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.graph;
+import com.android.tools.r8.utils.OptionalBool;
+
/**
* Definitions of access control routines.
*
@@ -11,14 +13,14 @@
*/
public class AccessControl {
- public static boolean isClassAccessible(DexClass clazz, DexProgramClass context) {
+ public static OptionalBool isClassAccessible(DexClass clazz, DexProgramClass context) {
if (clazz.accessFlags.isPublic()) {
- return true;
+ return OptionalBool.TRUE;
}
- return clazz.getType().isSamePackage(context.getType());
+ return OptionalBool.of(clazz.getType().isSamePackage(context.getType()));
}
- public static boolean isMethodAccessible(
+ public static OptionalBool isMethodAccessible(
DexEncodedMethod method,
DexClass holder,
DexProgramClass context,
@@ -26,7 +28,7 @@
return isMemberAccessible(method.accessFlags, holder, context, appInfo);
}
- public static boolean isFieldAccessible(
+ public static OptionalBool isFieldAccessible(
DexEncodedField field,
DexClass holder,
DexProgramClass context,
@@ -34,27 +36,31 @@
return isMemberAccessible(field.accessFlags, holder, context, appInfo);
}
- private static boolean isMemberAccessible(
+ private static OptionalBool isMemberAccessible(
AccessFlags<?> memberFlags,
DexClass holder,
DexProgramClass context,
AppInfoWithClassHierarchy appInfo) {
- if (!isClassAccessible(holder, context)) {
- return false;
+ OptionalBool classAccessibility = isClassAccessible(holder, context);
+ if (classAccessibility.isFalse()) {
+ return OptionalBool.FALSE;
}
if (memberFlags.isPublic()) {
- return true;
+ return classAccessibility;
}
if (memberFlags.isPrivate()) {
- return isNestMate(holder, context);
+ if (!isNestMate(holder, context)) {
+ return OptionalBool.FALSE;
+ }
+ return classAccessibility;
}
if (holder.getType().isSamePackage(context.getType())) {
- return true;
+ return classAccessibility;
}
- if (!memberFlags.isProtected()) {
- return false;
+ if (memberFlags.isProtected() && appInfo.isSubtype(context.getType(), holder.getType())) {
+ return classAccessibility;
}
- return appInfo.isSubtype(context.getType(), holder.getType());
+ return OptionalBool.FALSE;
}
private static boolean isNestMate(DexClass clazz, DexProgramClass context) {
diff --git a/src/main/java/com/android/tools/r8/graph/ResolutionResult.java b/src/main/java/com/android/tools/r8/graph/ResolutionResult.java
index d164bf1..7b388a3 100644
--- a/src/main/java/com/android/tools/r8/graph/ResolutionResult.java
+++ b/src/main/java/com/android/tools/r8/graph/ResolutionResult.java
@@ -8,6 +8,7 @@
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.shaking.InstantiatedObject;
import com.android.tools.r8.utils.Box;
+import com.android.tools.r8.utils.OptionalBool;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
@@ -58,10 +59,10 @@
return isSingleResolution() ? asSingleResolution().getResolvedMethod() : null;
}
- public abstract boolean isAccessibleFrom(
+ public abstract OptionalBool isAccessibleFrom(
DexProgramClass context, AppInfoWithClassHierarchy appInfo);
- public abstract boolean isAccessibleForVirtualDispatchFrom(
+ public abstract OptionalBool isAccessibleForVirtualDispatchFrom(
DexProgramClass context, AppInfoWithClassHierarchy appInfo);
public abstract boolean isVirtualTarget();
@@ -153,15 +154,19 @@
}
@Override
- public boolean isAccessibleFrom(DexProgramClass context, AppInfoWithClassHierarchy appInfo) {
+ public OptionalBool isAccessibleFrom(
+ DexProgramClass context, AppInfoWithClassHierarchy appInfo) {
return AccessControl.isMethodAccessible(
resolvedMethod, initialResolutionHolder, context, appInfo);
}
@Override
- public boolean isAccessibleForVirtualDispatchFrom(
+ public OptionalBool isAccessibleForVirtualDispatchFrom(
DexProgramClass context, AppInfoWithClassHierarchy appInfo) {
- return resolvedMethod.isVirtualMethod() && isAccessibleFrom(context, appInfo);
+ if (resolvedMethod.isVirtualMethod()) {
+ return isAccessibleFrom(context, appInfo);
+ }
+ return OptionalBool.FALSE;
}
@Override
@@ -179,7 +184,7 @@
public DexEncodedMethod lookupInvokeSpecialTarget(
DexProgramClass context, AppInfoWithClassHierarchy appInfo) {
// If the resolution is non-accessible then no target exists.
- if (isAccessibleFrom(context, appInfo)) {
+ if (isAccessibleFrom(context, appInfo).isPossiblyTrue()) {
return internalInvokeSpecialOrSuper(
context, appInfo, (sup, sub) -> isSuperclass(sup, sub, appInfo));
}
@@ -210,7 +215,7 @@
// If the target is <init> or not on a super class then the call is invalid.
return null;
}
- if (isAccessibleFrom(context, appInfo)) {
+ if (isAccessibleFrom(context, appInfo).isPossiblyTrue()) {
return internalInvokeSpecialOrSuper(context, appInfo, (sup, sub) -> true);
}
return null;
@@ -229,7 +234,7 @@
@Override
public DexEncodedMethod lookupInvokeStaticTarget(DexProgramClass context,
AppInfoWithClassHierarchy appInfo) {
- if (!isAccessibleFrom(context, appInfo)) {
+ if (isAccessibleFrom(context, appInfo).isFalse()) {
return null;
}
if (resolvedMethod.isStatic()) {
@@ -250,7 +255,7 @@
@Override
public DexEncodedMethod lookupInvokeDirectTarget(
DexProgramClass context, AppInfoWithClassHierarchy appInfo) {
- if (!isAccessibleFrom(context, appInfo)) {
+ if (isAccessibleFrom(context, appInfo).isFalse()) {
return null;
}
if (resolvedMethod.isDirectMethod()) {
@@ -349,7 +354,7 @@
// Check that the initial resolution holder is accessible from the context.
assert appInfo.isSubtype(initialResolutionHolder.type, resolvedHolder.type)
: initialResolutionHolder.type + " is not a subtype of " + resolvedHolder.type;
- if (context != null && !isAccessibleFrom(context, appInfo)) {
+ if (context != null && isAccessibleFrom(context, appInfo).isFalse()) {
return LookupResult.createFailedResult();
}
if (resolvedMethod.isPrivateMethod()) {
@@ -722,14 +727,15 @@
}
@Override
- public boolean isAccessibleFrom(DexProgramClass context, AppInfoWithClassHierarchy appInfo) {
- return true;
+ public OptionalBool isAccessibleFrom(
+ DexProgramClass context, AppInfoWithClassHierarchy appInfo) {
+ return OptionalBool.TRUE;
}
@Override
- public boolean isAccessibleForVirtualDispatchFrom(
+ public OptionalBool isAccessibleForVirtualDispatchFrom(
DexProgramClass context, AppInfoWithClassHierarchy appInfo) {
- return true;
+ return OptionalBool.TRUE;
}
@Override
@@ -756,14 +762,15 @@
}
@Override
- public boolean isAccessibleFrom(DexProgramClass context, AppInfoWithClassHierarchy appInfo) {
- return false;
+ public OptionalBool isAccessibleFrom(
+ DexProgramClass context, AppInfoWithClassHierarchy appInfo) {
+ return OptionalBool.FALSE;
}
@Override
- public boolean isAccessibleForVirtualDispatchFrom(
+ public OptionalBool isAccessibleForVirtualDispatchFrom(
DexProgramClass context, AppInfoWithClassHierarchy appInfo) {
- return false;
+ return OptionalBool.FALSE;
}
@Override
diff --git a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
index ada10b8..3fdfbb1 100644
--- a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
+++ b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
@@ -1129,7 +1129,7 @@
SingleResolutionResult resolution =
resolveMethod(initialResolutionHolder, method).asSingleResolution();
if (resolution == null
- || !resolution.isAccessibleForVirtualDispatchFrom(invocationClass, this)) {
+ || resolution.isAccessibleForVirtualDispatchFrom(invocationClass, this).isFalse()) {
return null;
}
// If the method is modeled, return the resolution.
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index 5540e34..4d91ba3 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -2437,7 +2437,8 @@
markMethodAsTargeted(resolvedHolder, resolvedMethod, reason);
DexProgramClass context = contextOrNull == null ? null : contextOrNull.getHolder();
- if (contextOrNull != null && !resolution.isAccessibleForVirtualDispatchFrom(context, appInfo)) {
+ if (contextOrNull != null
+ && resolution.isAccessibleForVirtualDispatchFrom(context, appInfo).isFalse()) {
// Not accessible from this context, so this call will cause a runtime exception.
return;
}
diff --git a/src/test/java/com/android/tools/r8/resolution/access/NestInvokeSpecialMethodPublicAccessWithIntermediateTest.java b/src/test/java/com/android/tools/r8/resolution/access/NestInvokeSpecialMethodPublicAccessWithIntermediateTest.java
index db75b6e..4f013bb 100644
--- a/src/test/java/com/android/tools/r8/resolution/access/NestInvokeSpecialMethodPublicAccessWithIntermediateTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/access/NestInvokeSpecialMethodPublicAccessWithIntermediateTest.java
@@ -20,6 +20,7 @@
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.transformers.ClassFileTransformer;
import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.OptionalBool;
import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
@@ -103,7 +104,8 @@
definingClassDefinition, resolutionResult.asSingleResolution().getResolvedHolder());
// Verify that the resolved method is accessible (it is public).
- assertTrue(resolutionResult.isAccessibleFrom(callerClassDefinition, appInfo));
+ assertEquals(
+ OptionalBool.TRUE, resolutionResult.isAccessibleFrom(callerClassDefinition, appInfo));
// Verify that looking up the dispatch target returns the defining method.
DexEncodedMethod targetSpecial =
diff --git a/src/test/java/com/android/tools/r8/resolution/access/SelfVirtualMethodAccessTest.java b/src/test/java/com/android/tools/r8/resolution/access/SelfVirtualMethodAccessTest.java
index f2f3309..020ec02 100644
--- a/src/test/java/com/android/tools/r8/resolution/access/SelfVirtualMethodAccessTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/access/SelfVirtualMethodAccessTest.java
@@ -3,7 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.resolution.access;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertEquals;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
@@ -13,6 +13,7 @@
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.ResolutionResult;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.android.tools.r8.utils.OptionalBool;
import com.android.tools.r8.utils.StringUtils;
import com.google.common.collect.ImmutableList;
import java.util.Collection;
@@ -56,7 +57,7 @@
appInfo.definitionFor(buildType(A.class, appInfo.dexItemFactory())).asProgramClass();
DexMethod bar = buildMethod(A.class.getDeclaredMethod("bar"), appInfo.dexItemFactory());
ResolutionResult resolutionResult = appInfo.resolveMethod(bar.holder, bar);
- assertTrue(resolutionResult.isAccessibleFrom(aClass, appInfo));
+ assertEquals(OptionalBool.TRUE, resolutionResult.isAccessibleFrom(aClass, appInfo));
}
@Test
diff --git a/src/test/java/com/android/tools/r8/resolution/access/indirectfield/IndirectFieldAccessTest.java b/src/test/java/com/android/tools/r8/resolution/access/indirectfield/IndirectFieldAccessTest.java
index 94b3c45..4616af8 100644
--- a/src/test/java/com/android/tools/r8/resolution/access/indirectfield/IndirectFieldAccessTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/access/indirectfield/IndirectFieldAccessTest.java
@@ -3,7 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.resolution.access.indirectfield;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertEquals;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
@@ -18,6 +18,7 @@
import com.android.tools.r8.references.Reference;
import com.android.tools.r8.resolution.access.indirectfield.pkg.C;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.android.tools.r8.utils.OptionalBool;
import com.android.tools.r8.utils.StringUtils;
import com.google.common.collect.ImmutableList;
import java.util.List;
@@ -60,7 +61,8 @@
DexClass initialResolutionHolder = appInfo.definitionFor(f.holder);
DexEncodedField resolutionTarget = appInfo.resolveField(f);
// TODO(b/145723539): Test access via the resolution result once possible.
- assertTrue(
+ assertEquals(
+ OptionalBool.TRUE,
AccessControl.isFieldAccessible(
resolutionTarget, initialResolutionHolder, cClass, appInfo));
}
diff --git a/src/test/java/com/android/tools/r8/resolution/access/indirectmethod/IndirectMethodAccessTest.java b/src/test/java/com/android/tools/r8/resolution/access/indirectmethod/IndirectMethodAccessTest.java
index 34af60e..bdd8484 100644
--- a/src/test/java/com/android/tools/r8/resolution/access/indirectmethod/IndirectMethodAccessTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/access/indirectmethod/IndirectMethodAccessTest.java
@@ -3,7 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.resolution.access.indirectmethod;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertEquals;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
@@ -15,6 +15,7 @@
import com.android.tools.r8.graph.ResolutionResult;
import com.android.tools.r8.resolution.access.indirectmethod.pkg.C;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.android.tools.r8.utils.OptionalBool;
import com.android.tools.r8.utils.StringUtils;
import com.google.common.collect.ImmutableList;
import java.io.IOException;
@@ -61,7 +62,8 @@
appInfo.definitionFor(buildType(C.class, appInfo.dexItemFactory())).asProgramClass();
DexMethod bar = buildMethod(B.class.getMethod("foo"), appInfo.dexItemFactory());
ResolutionResult resolutionResult = appInfo.resolveMethod(bar.holder, bar);
- assertTrue(resolutionResult.isAccessibleForVirtualDispatchFrom(cClass, appInfo));
+ assertEquals(
+ OptionalBool.TRUE, resolutionResult.isAccessibleForVirtualDispatchFrom(cClass, appInfo));
}
@Test
