Preserve IAE across isolated split boundary
Bug: b/300247439
Change-Id: I78fb2205884b0d170868a78e713da1a03a05aee2
diff --git a/src/main/java/com/android/tools/r8/features/FeatureSplitBoundaryOptimizationUtils.java b/src/main/java/com/android/tools/r8/features/FeatureSplitBoundaryOptimizationUtils.java
index 9de2dad..4c5c33b 100644
--- a/src/main/java/com/android/tools/r8/features/FeatureSplitBoundaryOptimizationUtils.java
+++ b/src/main/java/com/android/tools/r8/features/FeatureSplitBoundaryOptimizationUtils.java
@@ -9,75 +9,45 @@
import com.android.tools.r8.FeatureSplit;
import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
import com.android.tools.r8.graph.AppView;
-import com.android.tools.r8.graph.DexClass;
-import com.android.tools.r8.graph.DexClassAndMember;
+import com.android.tools.r8.graph.Definition;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.ProgramDefinition;
-import com.android.tools.r8.graph.ProgramMember;
import com.android.tools.r8.graph.ProgramMethod;
-import com.android.tools.r8.ir.optimize.Inliner.ConstraintWithTarget;
-import com.android.tools.r8.synthesis.SyntheticItems;
public class FeatureSplitBoundaryOptimizationUtils {
- public static ConstraintWithTarget getInliningConstraintForResolvedMember(
- DexClassAndMember<?, ?> resolvedMember,
- DexClass initialResolutionHolder,
- ProgramMethod context,
- AppView<? extends AppInfoWithClassHierarchy> appView) {
- if (!appView.options().hasFeatureSplitConfiguration()) {
- return ConstraintWithTarget.ALWAYS;
- }
- // If we resolve to something outside the program then everything is fine.
- if (!initialResolutionHolder.isProgramClass()) {
- assert !resolvedMember.isProgramMember();
- return ConstraintWithTarget.ALWAYS;
- }
- ProgramMember<?, ?> resolvedProgramMember = resolvedMember.asProgramMember();
- DexProgramClass initialProgramResolutionHolder = initialResolutionHolder.asProgramClass();
- return getInliningConstraintForResolvedMember(
- resolvedProgramMember, initialProgramResolutionHolder, context, appView);
- }
-
- private static ConstraintWithTarget getInliningConstraintForResolvedMember(
- ProgramMember<?, ?> resolvedMember,
- DexProgramClass initialResolutionHolder,
- ProgramMethod context,
- AppView<? extends AppInfoWithClassHierarchy> appView) {
- assert appView.options().hasFeatureSplitConfiguration();
- ClassToFeatureSplitMap features = appView.appInfo().getClassToFeatureSplitMap();
- // Check that whatever we resolve to is in the same feature or in base.
- if (!features.isInBaseOrSameFeatureAs(initialResolutionHolder, context, appView)
- || !features.isInBaseOrSameFeatureAs(resolvedMember, context, appView)) {
- return ConstraintWithTarget.NEVER;
- }
- // If isolated splits are enabled then the resolved method must be in the same feature split or
- // it must be public.
- if (appView.options().getFeatureSplitConfiguration().isIsolatedSplitsEnabled()) {
- if (!initialResolutionHolder.isPublic()
- && !features.isInSameFeature(initialResolutionHolder, context, appView)) {
- return ConstraintWithTarget.NEVER;
- }
- if (!resolvedMember.getAccessFlags().isPublic()
- && !features.isInSameFeature(resolvedMember, context, appView)) {
- return ConstraintWithTarget.NEVER;
- }
- }
- return ConstraintWithTarget.ALWAYS;
- }
-
public static FeatureSplit getMergeKeyForHorizontalClassMerging(
DexProgramClass clazz, AppView<? extends AppInfoWithClassHierarchy> appView) {
- ClassToFeatureSplitMap classToFeatureSplitMap = appView.appInfo().getClassToFeatureSplitMap();
- return classToFeatureSplitMap.getFeatureSplit(clazz, appView);
+ return appView.appInfo().getClassToFeatureSplitMap().getFeatureSplit(clazz, appView);
}
public static boolean isSafeForAccess(
- DexProgramClass accessedClass,
- ProgramDefinition accessor,
- ClassToFeatureSplitMap classToFeatureSplitMap,
- SyntheticItems syntheticItems) {
- return classToFeatureSplitMap.isInBaseOrSameFeatureAs(accessedClass, accessor, syntheticItems);
+ Definition definition,
+ ProgramDefinition context,
+ AppView<? extends AppInfoWithClassHierarchy> appView) {
+ return !appView.options().hasFeatureSplitConfiguration()
+ || !definition.isProgramDefinition()
+ || isSafeForAccess(definition.asProgramDefinition(), context, appView);
+ }
+
+ private static boolean isSafeForAccess(
+ ProgramDefinition definition,
+ ProgramDefinition context,
+ AppView<? extends AppInfoWithClassHierarchy> appView) {
+ assert appView.options().hasFeatureSplitConfiguration();
+ ClassToFeatureSplitMap classToFeatureSplitMap = appView.appInfo().getClassToFeatureSplitMap();
+ if (classToFeatureSplitMap.isInSameFeature(definition, context, appView)) {
+ return true;
+ }
+ if (!classToFeatureSplitMap.isInBase(definition, appView)) {
+ return false;
+ }
+ // If isolated splits are enabled then the resolved method must be public.
+ if (appView.options().getFeatureSplitConfiguration().isIsolatedSplitsEnabled()
+ && !definition.getAccessFlags().isPublic()) {
+ return false;
+ }
+ return true;
}
public static boolean isSafeForInlining(
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 2d5b68a..3ed6de7 100644
--- a/src/main/java/com/android/tools/r8/graph/AccessControl.java
+++ b/src/main/java/com/android/tools/r8/graph/AccessControl.java
@@ -3,9 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.graph;
-import com.android.tools.r8.features.ClassToFeatureSplitMap;
import com.android.tools.r8.features.FeatureSplitBoundaryOptimizationUtils;
-import com.android.tools.r8.synthesis.SyntheticItems;
import com.android.tools.r8.utils.OptionalBool;
/**
@@ -17,31 +15,20 @@
public class AccessControl {
public static OptionalBool isClassAccessible(
- DexClass clazz,
- ProgramDefinition context,
- AppView<? extends AppInfoWithClassHierarchy> appView) {
- return isClassAccessible(
- clazz,
- context,
- appView.appInfo().getClassToFeatureSplitMap(),
- appView.getSyntheticItems());
+ DexClass clazz, Definition context, AppView<? extends AppInfoWithClassHierarchy> appView) {
+ return isClassAccessible(clazz, context, appView, appView.appInfo());
}
public static OptionalBool isClassAccessible(
- DexClass clazz,
- Definition context,
- ClassToFeatureSplitMap classToFeatureSplitMap,
- SyntheticItems syntheticItems) {
+ DexClass clazz, Definition context, AppView<?> appView, AppInfoWithClassHierarchy appInfo) {
+ assert appInfo != null;
if (!clazz.isPublic() && !clazz.getType().isSamePackage(context.getContextType())) {
return OptionalBool.FALSE;
}
- if (clazz.isProgramClass()
+ if (appView.hasClassHierarchy()
&& context.isProgramDefinition()
&& !FeatureSplitBoundaryOptimizationUtils.isSafeForAccess(
- clazz.asProgramClass(),
- context.asProgramDefinition(),
- classToFeatureSplitMap,
- syntheticItems)) {
+ clazz, context.asProgramDefinition(), appView.withClassHierarchy())) {
return OptionalBool.UNKNOWN;
}
return OptionalBool.TRUE;
@@ -51,11 +38,13 @@
static OptionalBool isMemberAccessible(
SuccessfulMemberResolutionResult<?, ?> resolutionResult,
Definition context,
+ AppView<?> appView,
AppInfoWithClassHierarchy appInfo) {
return isMemberAccessible(
resolutionResult.getResolutionPair(),
resolutionResult.getInitialResolutionHolder(),
context,
+ appView,
appInfo);
}
@@ -64,21 +53,19 @@
Definition initialResolutionContext,
Definition context,
AppView<? extends AppInfoWithClassHierarchy> appView) {
- return isMemberAccessible(member, initialResolutionContext, context, appView.appInfo());
+ return isMemberAccessible(
+ member, initialResolutionContext, context, appView, appView.appInfo());
}
static OptionalBool isMemberAccessible(
DexClassAndMember<?, ?> member,
Definition initialResolutionContext,
Definition context,
+ AppView<?> appView,
AppInfoWithClassHierarchy appInfo) {
AccessFlags<?> memberFlags = member.getDefinition().getAccessFlags();
OptionalBool classAccessibility =
- isClassAccessible(
- initialResolutionContext.getContextClass(),
- context,
- appInfo.getClassToFeatureSplitMap(),
- appInfo.getSyntheticItems());
+ isClassAccessible(initialResolutionContext.getContextClass(), context, appView, appInfo);
if (classAccessibility.isFalse()) {
return OptionalBool.FALSE;
}
@@ -91,6 +78,12 @@
}
return classAccessibility;
}
+ if (appView.hasClassHierarchy()
+ && context.isProgramDefinition()
+ && !FeatureSplitBoundaryOptimizationUtils.isSafeForAccess(
+ member, context.asProgramDefinition(), appView.withClassHierarchy())) {
+ return OptionalBool.UNKNOWN;
+ }
if (member.getHolderType().isSamePackage(context.getContextType())) {
return classAccessibility;
}
@@ -101,7 +94,6 @@
return OptionalBool.FALSE;
}
- @SuppressWarnings("ReferenceEquality")
private static boolean isNestMate(DexClass clazz, DexClass context) {
if (clazz == context) {
return true;
@@ -113,6 +105,6 @@
if (!clazz.isInANest() || !context.isInANest()) {
return false;
}
- return clazz.getNestHost() == context.getNestHost();
+ return clazz.getNestHost().isIdenticalTo(context.getNestHost());
}
}
diff --git a/src/main/java/com/android/tools/r8/graph/FieldResolutionResult.java b/src/main/java/com/android/tools/r8/graph/FieldResolutionResult.java
index 6bae976..a383253 100644
--- a/src/main/java/com/android/tools/r8/graph/FieldResolutionResult.java
+++ b/src/main/java/com/android/tools/r8/graph/FieldResolutionResult.java
@@ -203,7 +203,7 @@
@Override
public OptionalBool isAccessibleFrom(
ProgramDefinition context, AppView<?> appView, AppInfoWithClassHierarchy appInfo) {
- return AccessControl.isMemberAccessible(this, context, appInfo);
+ return AccessControl.isMemberAccessible(this, context, appView, appInfo);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/graph/MethodResolutionResult.java b/src/main/java/com/android/tools/r8/graph/MethodResolutionResult.java
index 03b3932..bdc3a2a 100644
--- a/src/main/java/com/android/tools/r8/graph/MethodResolutionResult.java
+++ b/src/main/java/com/android/tools/r8/graph/MethodResolutionResult.java
@@ -501,7 +501,7 @@
@Override
public OptionalBool isAccessibleFrom(
ProgramDefinition context, AppView<?> appView, AppInfoWithClassHierarchy appInfo) {
- return AccessControl.isMemberAccessible(this, context, appInfo);
+ return AccessControl.isMemberAccessible(this, context, appView, appInfo);
}
@Override
@@ -1480,18 +1480,14 @@
.forEachClassResolutionResult(
clazz ->
seenNoAccess.or(
- AccessControl.isClassAccessible(
- clazz,
- context,
- appInfo.getClassToFeatureSplitMap(),
- appView.getSyntheticItems())
+ AccessControl.isClassAccessible(clazz, context, appView, appInfo)
.isPossiblyFalse())),
method -> {
DexClass holder = appView.definitionFor(method.getHolderType());
DexClassAndMethod classAndMethod = DexClassAndMethod.create(holder, method);
seenNoAccess.or(
AccessControl.isMemberAccessible(
- classAndMethod, initialResolutionHolder, context, appInfo)
+ classAndMethod, initialResolutionHolder, context, appView, appInfo)
.isPossiblyFalse());
});
return seenNoAccess.get();
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java b/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
index b3ebdc6..a3ea046 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
@@ -346,13 +346,11 @@
DexClass initialResolutionHolder,
ProgramMethod context,
DexClassAndMember<?, ?> resolvedMember) {
- ConstraintWithTarget featureSplitInliningConstraint =
- FeatureSplitBoundaryOptimizationUtils.getInliningConstraintForResolvedMember(
- resolvedMember, initialResolutionHolder, context, appView);
- assert featureSplitInliningConstraint == ConstraintWithTarget.ALWAYS
- || featureSplitInliningConstraint.isNever();
- if (featureSplitInliningConstraint.isNever()) {
- return featureSplitInliningConstraint;
+ if (!FeatureSplitBoundaryOptimizationUtils.isSafeForAccess(
+ initialResolutionHolder, context, appView)
+ || !FeatureSplitBoundaryOptimizationUtils.isSafeForAccess(
+ resolvedMember, context, appView)) {
+ return ConstraintWithTarget.NEVER;
}
DexType resolvedHolder = resolvedMember.getHolderType();
assert initialResolutionHolder != null;
diff --git a/src/test/java/com/android/tools/r8/features/PreserveIllegalAccessAcrossIsolatedSplitBoundaryTest.java b/src/test/java/com/android/tools/r8/features/PreserveIllegalAccessAcrossIsolatedSplitBoundaryTest.java
index d6edc0b..ecae36b 100644
--- a/src/test/java/com/android/tools/r8/features/PreserveIllegalAccessAcrossIsolatedSplitBoundaryTest.java
+++ b/src/test/java/com/android/tools/r8/features/PreserveIllegalAccessAcrossIsolatedSplitBoundaryTest.java
@@ -4,8 +4,9 @@
package com.android.tools.r8.features;
import static com.android.tools.r8.utils.codeinspector.CodeMatchers.invokesMethod;
-import static com.android.tools.r8.utils.codeinspector.Matchers.isAbsent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPackagePrivate;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentIf;
import static org.hamcrest.MatcherAssert.assertThat;
import com.android.tools.r8.NeverInline;
@@ -58,10 +59,12 @@
ClassSubject baseClassSubject = baseInspector.clazz(Base.class);
assertThat(baseClassSubject, isPresent());
- // TODO(b/300247439): Illegal access error should be preserved.
MethodSubject nonPublicMethodSubject =
baseClassSubject.uniqueMethodWithOriginalName("nonPublicMethod");
- assertThat(nonPublicMethodSubject, isAbsent());
+ assertThat(nonPublicMethodSubject, isPresentIf(enableIsolatedSplits));
+ if (enableIsolatedSplits) {
+ assertThat(nonPublicMethodSubject, isPackagePrivate());
+ }
MethodSubject otherMethodSubject =
baseClassSubject.uniqueMethodWithOriginalName("otherMethod");
@@ -73,7 +76,10 @@
MethodSubject featureMethodSubject =
featureClassSubject.uniqueMethodWithOriginalName("test");
assertThat(featureMethodSubject, isPresent());
- assertThat(featureMethodSubject, invokesMethod(otherMethodSubject));
+ assertThat(
+ featureMethodSubject,
+ invokesMethod(
+ enableIsolatedSplits ? nonPublicMethodSubject : otherMethodSubject));
});
}
