Refine access analysis in bridge removal
Fixes: b/245882297
Change-Id: Id91b041cd7d28256ced5de829f56e54d215d7367
diff --git a/src/main/java/com/android/tools/r8/graph/Definition.java b/src/main/java/com/android/tools/r8/graph/Definition.java
index 05ad2d1..e03b8e8 100644
--- a/src/main/java/com/android/tools/r8/graph/Definition.java
+++ b/src/main/java/com/android/tools/r8/graph/Definition.java
@@ -155,4 +155,12 @@
default ProgramMethod asProgramMethod() {
return null;
}
+
+ default boolean isSamePackage(Definition definition) {
+ return isSamePackage(definition.getReference());
+ }
+
+ default boolean isSamePackage(DexReference reference) {
+ return getReference().isSamePackage(reference);
+ }
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexReference.java b/src/main/java/com/android/tools/r8/graph/DexReference.java
index 6956170..1e84335 100644
--- a/src/main/java/com/android/tools/r8/graph/DexReference.java
+++ b/src/main/java/com/android/tools/r8/graph/DexReference.java
@@ -83,6 +83,10 @@
return null;
}
+ public boolean isSamePackage(DexReference reference) {
+ return getContextType().isSamePackage(reference.getContextType());
+ }
+
public int referenceTypeOrder() {
if (isDexType()) {
return 1;
diff --git a/src/main/java/com/android/tools/r8/optimize/redundantbridgeremoval/RedundantBridgeRemover.java b/src/main/java/com/android/tools/r8/optimize/redundantbridgeremoval/RedundantBridgeRemover.java
index 44c94c4..ab054e2 100644
--- a/src/main/java/com/android/tools/r8/optimize/redundantbridgeremoval/RedundantBridgeRemover.java
+++ b/src/main/java/com/android/tools/r8/optimize/redundantbridgeremoval/RedundantBridgeRemover.java
@@ -11,6 +11,7 @@
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.ImmediateProgramSubtypingInfo;
+import com.android.tools.r8.graph.MethodAccessFlags;
import com.android.tools.r8.graph.MethodResolutionResult;
import com.android.tools.r8.graph.MethodResolutionResult.FailedResolutionResult;
import com.android.tools.r8.graph.MethodResolutionResult.SingleResolutionResult;
@@ -279,14 +280,7 @@
bridgesToRemoveForClass.add(method);
// Rewrite invokes to the bridge to the target if it is accessible.
- // TODO(b/245882297): Refine these visibility checks so that we also rewrite when
- // the target is not public, but still accessible to call sites.
- boolean isEligibleForRetargeting =
- redundantBridgeRemovalOptions.isRetargetingOfConstructorBridgeCallsEnabled()
- || !method.getDefinition().isInstanceInitializer();
- if (isEligibleForRetargeting
- && target.getAccessFlags().isPublic()
- && target.getHolder().isPublic()) {
+ if (canRetargetInvokesToTargetMethod(method, target)) {
lensBuilder.map(method, target);
}
}
@@ -297,6 +291,39 @@
}
}
+ private boolean canRetargetInvokesToTargetMethod(
+ ProgramMethod method, DexClassAndMethod target) {
+ // Check if constructor retargeting is enabled.
+ if (method.getDefinition().isInstanceInitializer()
+ && !redundantBridgeRemovalOptions.isRetargetingOfConstructorBridgeCallsEnabled()) {
+ return false;
+ }
+ // Check if all possible contexts that have access to the holder of the redundant bridge
+ // method also have access to the holder of the target method.
+ DexProgramClass methodHolder = method.getHolder();
+ DexClass targetHolder = target.getHolder();
+ if (!targetHolder.getAccessFlags().isPublic()) {
+ if (methodHolder.getAccessFlags().isPublic() || !method.isSamePackage(target)) {
+ return false;
+ }
+ }
+ // Check if all possible contexts that have access to the redundant bridge method also have
+ // access to the target method.
+ if (target.getAccessFlags().isPublic()) {
+ return true;
+ }
+ MethodAccessFlags methodAccessFlags = method.getAccessFlags();
+ MethodAccessFlags targetAccessFlags = target.getAccessFlags();
+ if (methodAccessFlags.isPackagePrivate()
+ && !targetAccessFlags.isPrivate()
+ && method.isSamePackage(target)) {
+ return true;
+ }
+ return methodAccessFlags.isProtected()
+ && targetAccessFlags.isProtected()
+ && method.isSamePackage(target);
+ }
+
@Override
public void prune(DexProgramClass clazz) {
// Empty.
