Synthesize rules to disallow shrinking of Fragment/ZygotePreload inits
Bug: b/281468010
Change-Id: I86dfb7f4e5a3ae3cd013790cb58a239122832e14
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 634a532..d44a39b 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -317,21 +317,13 @@
appView.getSyntheticItems().commit(appView.appInfo().app())));
}
- List<ProguardConfigurationRule> synthesizedProguardRules = new ArrayList<>();
timing.begin("Strip unused code");
timing.begin("Before enqueuer");
RuntimeTypeCheckInfo.Builder classMergingEnqueuerExtensionBuilder =
new RuntimeTypeCheckInfo.Builder(appView);
+ List<ProguardConfigurationRule> synthesizedProguardRules;
try {
- // Add synthesized -assumenosideeffects from min api if relevant.
- if (options.isGeneratingDex()) {
- if (!ProguardConfigurationUtils.hasExplicitAssumeValuesOrAssumeNoSideEffectsRuleForMinSdk(
- options.itemFactory, options.getProguardConfiguration().getRules())) {
- synthesizedProguardRules.add(
- ProguardConfigurationUtils.buildAssumeNoSideEffectsRuleForApiLevel(
- options.itemFactory, options.getMinApiLevel()));
- }
- }
+ synthesizedProguardRules = ProguardConfigurationUtils.synthesizeRules(appView);
ProfileCollectionAdditions profileCollectionAdditions =
ProfileCollectionAdditions.create(appView);
AssumeInfoCollection.Builder assumeInfoCollectionBuilder = AssumeInfoCollection.builder();
diff --git a/src/main/java/com/android/tools/r8/optimize/redundantbridgeremoval/RedundantBridgeRemovalOptions.java b/src/main/java/com/android/tools/r8/optimize/redundantbridgeremoval/RedundantBridgeRemovalOptions.java
index c290822..0ca1265 100644
--- a/src/main/java/com/android/tools/r8/optimize/redundantbridgeremoval/RedundantBridgeRemovalOptions.java
+++ b/src/main/java/com/android/tools/r8/optimize/redundantbridgeremoval/RedundantBridgeRemovalOptions.java
@@ -4,42 +4,9 @@
package com.android.tools.r8.optimize.redundantbridgeremoval;
-import com.android.tools.r8.graph.DexItemFactory;
-import com.android.tools.r8.graph.DexLibraryClass;
-import com.android.tools.r8.graph.DexType;
-import com.android.tools.r8.utils.InternalOptions;
-import com.android.tools.r8.utils.SetUtils;
-import java.util.Collections;
-import java.util.Set;
-
public class RedundantBridgeRemovalOptions {
- private final InternalOptions options;
-
private boolean enableRetargetingOfConstructorBridgeCalls = true;
- private Set<DexType> noConstructorShrinkingHierarchies;
-
- public RedundantBridgeRemovalOptions(InternalOptions options) {
- this.options = options;
- }
-
- public void clearNoConstructorShrinkingHierarchiesForTesting() {
- noConstructorShrinkingHierarchies = Collections.emptySet();
- }
-
- public RedundantBridgeRemovalOptions ensureInitialized() {
- if (noConstructorShrinkingHierarchies == null) {
- DexItemFactory dexItemFactory = options.dexItemFactory();
- noConstructorShrinkingHierarchies =
- SetUtils.newIdentityHashSet(
- dexItemFactory.androidAppFragment, dexItemFactory.androidAppZygotePreload);
- }
- return this;
- }
-
- public boolean isPlatformReflectingOnDefaultConstructorInSubclasses(DexLibraryClass clazz) {
- return noConstructorShrinkingHierarchies.contains(clazz.getType());
- }
public boolean isRetargetingOfConstructorBridgeCallsEnabled() {
return enableRetargetingOfConstructorBridgeCalls;
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 ab054e2..7ae9ca2 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
@@ -4,7 +4,6 @@
package com.android.tools.r8.optimize.redundantbridgeremoval;
import com.android.tools.r8.graph.AppView;
-import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexClassAndMethod;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexMethod;
@@ -25,20 +24,13 @@
import com.android.tools.r8.optimize.argumentpropagation.utils.ProgramClassesBidirectedGraph;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.shaking.KeepMethodInfo;
-import com.android.tools.r8.utils.IterableUtils;
import com.android.tools.r8.utils.ThreadUtils;
-import com.android.tools.r8.utils.WorkList;
import com.android.tools.r8.utils.collections.ProgramMethodSet;
-import com.google.common.collect.Iterables;
import java.util.Collection;
-import java.util.Collections;
import java.util.List;
-import java.util.Map;
import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
-import java.util.function.Consumer;
public class RedundantBridgeRemover {
@@ -46,16 +38,13 @@
private final ImmediateProgramSubtypingInfo immediateSubtypingInfo;
private final RedundantBridgeRemovalOptions redundantBridgeRemovalOptions;
- private final InvokedReflectivelyFromPlatformAnalysis invokedReflectivelyFromPlatformAnalysis =
- new InvokedReflectivelyFromPlatformAnalysis();
private final RedundantBridgeRemovalLens.Builder lensBuilder =
new RedundantBridgeRemovalLens.Builder();
public RedundantBridgeRemover(AppView<AppInfoWithLiveness> appView) {
this.appView = appView;
this.immediateSubtypingInfo = ImmediateProgramSubtypingInfo.create(appView);
- this.redundantBridgeRemovalOptions =
- appView.options().getRedundantBridgeRemovalOptions().ensureInitialized();
+ this.redundantBridgeRemovalOptions = appView.options().getRedundantBridgeRemovalOptions();
}
private DexClassAndMethod getTargetForRedundantBridge(ProgramMethod method) {
@@ -76,9 +65,6 @@
if (!isTargetingSuperMethod(method, targetExtractor.getKind(), target)) {
return null;
}
- if (invokedReflectivelyFromPlatformAnalysis.isMaybeInvokedReflectivelyFromPlatform(method)) {
- return null;
- }
// This is a visibility forward, so check for the direct target.
DexClassAndMethod targetMethod =
appView.appInfo().unsafeResolveMethodDueToDexFormatLegacy(target).getResolutionPair();
@@ -329,88 +315,4 @@
// Empty.
}
}
-
- class InvokedReflectivelyFromPlatformAnalysis {
-
- // Maps each class to a boolean indicating if the class inherits from android.app.Fragment or
- // android.app.ZygotePreload.
- private final Map<DexClass, Boolean> cache = new ConcurrentHashMap<>();
-
- boolean isMaybeInvokedReflectivelyFromPlatform(ProgramMethod method) {
- return method.getDefinition().isDefaultInstanceInitializer()
- && !method.getHolder().isAbstract()
- && computeIsPlatformReflectingOnDefaultConstructor(method.getHolder());
- }
-
- private boolean computeIsPlatformReflectingOnDefaultConstructor(DexProgramClass clazz) {
- Boolean cacheResult = cache.get(clazz);
- if (cacheResult != null) {
- return cacheResult;
- }
- WorkList.<WorklistItem>newIdentityWorkList(new NotProcessedWorklistItem(clazz))
- .process(WorklistItem::accept);
- assert cache.containsKey(clazz);
- return cache.get(clazz);
- }
-
- abstract class WorklistItem implements Consumer<WorkList<WorklistItem>> {
-
- protected final DexClass clazz;
-
- WorklistItem(DexClass clazz) {
- this.clazz = clazz;
- }
-
- Iterable<DexClass> getImmediateSupertypes() {
- return IterableUtils.flatMap(
- clazz.allImmediateSupertypes(),
- supertype -> {
- DexClass definition = appView.definitionFor(supertype);
- return definition != null
- ? Collections.singletonList(definition)
- : Collections.emptyList();
- });
- }
- }
-
- class NotProcessedWorklistItem extends WorklistItem {
-
- NotProcessedWorklistItem(DexClass clazz) {
- super(clazz);
- }
-
- @Override
- public void accept(WorkList<WorklistItem> worklist) {
- // Enqueue a worklist item to process the current class after processing its super classes.
- worklist.addFirstIgnoringSeenSet(new ProcessedWorklistItem(clazz));
- // Enqueue all superclasses for processing.
- for (DexClass supertype : getImmediateSupertypes()) {
- if (!cache.containsKey(supertype)) {
- worklist.addFirstIgnoringSeenSet(new NotProcessedWorklistItem(supertype));
- }
- }
- }
- }
-
- class ProcessedWorklistItem extends WorklistItem {
-
- ProcessedWorklistItem(DexClass clazz) {
- super(clazz);
- }
-
- @Override
- public void accept(WorkList<WorklistItem> worklist) {
- cache.put(
- clazz,
- Iterables.any(
- getImmediateSupertypes(),
- supertype ->
- cache.get(supertype)
- || (supertype.isLibraryClass()
- && redundantBridgeRemovalOptions
- .isPlatformReflectingOnDefaultConstructorInSubclasses(
- supertype.asLibraryClass()))));
- }
- }
- }
}
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardAccessFlags.java b/src/main/java/com/android/tools/r8/shaking/ProguardAccessFlags.java
index 9bd37a5..cdf0264 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardAccessFlags.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardAccessFlags.java
@@ -132,8 +132,9 @@
return isSet(Constants.ACC_FINAL);
}
- public void setAbstract() {
+ public ProguardAccessFlags setAbstract() {
set(Constants.ACC_ABSTRACT);
+ return this;
}
public boolean isAbstract() {
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
index b050658..2c51922 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
@@ -2384,6 +2384,10 @@
this.wildcards = wildcards;
}
+ static IdentifierPatternWithWildcards init() {
+ return withoutWildcards("<init>");
+ }
+
static IdentifierPatternWithWildcards withoutWildcards(String pattern) {
return new IdentifierPatternWithWildcards(pattern, ImmutableList.of());
}
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationUtils.java b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationUtils.java
index 94ab239..9b0ed2e 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationUtils.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationUtils.java
@@ -4,17 +4,53 @@
package com.android.tools.r8.shaking;
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.shaking.ProguardConfigurationParser.IdentifierPatternWithWildcards;
import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.LongInterval;
import com.google.common.collect.ImmutableList;
+import java.util.ArrayList;
+import java.util.Collections;
import java.util.List;
public class ProguardConfigurationUtils {
- public static ProguardAssumeNoSideEffectRule buildAssumeNoSideEffectsRuleForApiLevel(
+ public static List<ProguardConfigurationRule> synthesizeRules(AppView<?> appView) {
+ List<ProguardConfigurationRule> synthesizedRules = new ArrayList<>();
+ DexItemFactory factory = appView.dexItemFactory();
+ InternalOptions options = appView.options();
+ // Add synthesized -assumenosideeffects from min api if relevant.
+ if (options.isGeneratingDex()) {
+ if (!hasExplicitAssumeValuesOrAssumeNoSideEffectsRuleForMinSdk(
+ factory, options.getProguardConfiguration().getRules())) {
+ synthesizedRules.add(
+ buildAssumeNoSideEffectsRuleForApiLevel(factory, options.getMinApiLevel()));
+ }
+ }
+ // Add synthesized -keepclassmembers rules for the default initializer of classes that inherit
+ // from android.app.Fragment and android.app.ZygotePreload. This is needed since the Android
+ // Platform may reflectively access these instance initializers.
+ DexClass androidAppFragment =
+ appView.appInfo().definitionForWithoutExistenceAssert(factory.androidAppFragment);
+ if (androidAppFragment != null) {
+ synthesizedRules.add(
+ buildKeepClassMembersNoShrinkingOfInitializerOnSubclasses(factory, androidAppFragment));
+ }
+ DexClass androidAppZygotePreload =
+ appView.appInfo().definitionForWithoutExistenceAssert(factory.androidAppZygotePreload);
+ if (androidAppZygotePreload != null) {
+ synthesizedRules.add(
+ buildKeepClassMembersNoShrinkingOfInitializerOnSubclasses(
+ factory, androidAppZygotePreload));
+ }
+ return synthesizedRules;
+ }
+
+ private static ProguardAssumeNoSideEffectRule buildAssumeNoSideEffectsRuleForApiLevel(
DexItemFactory factory, AndroidApiLevel apiLevel) {
Origin synthesizedFromApiLevel =
new Origin(Origin.root()) {
@@ -53,7 +89,7 @@
* Check if an explicit rule matching the field public static final int
* android.os.Build$VERSION.SDK_INT is present.
*/
- public static boolean hasExplicitAssumeValuesOrAssumeNoSideEffectsRuleForMinSdk(
+ private static boolean hasExplicitAssumeValuesOrAssumeNoSideEffectsRuleForMinSdk(
DexItemFactory factory, List<ProguardConfigurationRule> rules) {
for (ProguardConfigurationRule rule : rules) {
if (!(rule instanceof ProguardAssumeValuesRule
@@ -108,4 +144,28 @@
}
return false;
}
+
+ // -keepclassmembers,allow* !abstract class * extends T { void <init>(); }
+ private static ProguardKeepRule buildKeepClassMembersNoShrinkingOfInitializerOnSubclasses(
+ DexItemFactory factory, DexClass clazz) {
+ return ProguardKeepRule.builder()
+ .setClassNames(ProguardClassNameList.singletonList(ProguardTypeMatcher.allClassesMatcher()))
+ .setClassType(ProguardClassType.CLASS)
+ .setInheritanceClassName(ProguardTypeMatcher.create(clazz.getType()))
+ .setInheritanceIsExtends(!clazz.isInterface())
+ .setMemberRules(
+ Collections.singletonList(
+ ProguardMemberRule.builder()
+ .setRuleType(ProguardMemberType.INIT)
+ .setName(IdentifierPatternWithWildcards.init())
+ .setArguments(Collections.emptyList())
+ .setTypeMatcher(ProguardTypeMatcher.create(factory.voidType))
+ .build()))
+ .setNegatedClassAccessFlags(new ProguardAccessFlags().setAbstract())
+ .setOrigin(clazz.getOrigin())
+ .setType(ProguardKeepRuleType.KEEP_CLASS_MEMBERS)
+ .updateModifiers(
+ modifiersBuilder -> modifiersBuilder.setAllowsAll().setAllowsShrinking(false).build())
+ .build();
+ }
}
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardKeepRuleModifiers.java b/src/main/java/com/android/tools/r8/shaking/ProguardKeepRuleModifiers.java
index a2619f7..ec825c3 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardKeepRuleModifiers.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardKeepRuleModifiers.java
@@ -18,6 +18,16 @@
private Builder() {}
+ public Builder setAllowsAll() {
+ setAllowsAccessModification(true);
+ setAllowsAnnotationRemoval(true);
+ setAllowsObfuscation(true);
+ setAllowsOptimization(true);
+ setAllowsRepackaging(true);
+ setAllowsShrinking(true);
+ return this;
+ }
+
public Builder setAllowsAccessModification(boolean allowsAccessModification) {
this.allowsAccessModification = allowsAccessModification;
return this;
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardTypeMatcher.java b/src/main/java/com/android/tools/r8/shaking/ProguardTypeMatcher.java
index 19bd876..fee5278 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardTypeMatcher.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardTypeMatcher.java
@@ -125,6 +125,10 @@
return new MatchSpecificType(type);
}
+ public static ProguardTypeMatcher allClassesMatcher() {
+ return MatchClassTypes.MATCH_CLASS_TYPES;
+ }
+
public static ProguardTypeMatcher defaultAllMatcher() {
return MatchAllTypes.MATCH_ALL_TYPES;
}
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index 0f8c7cf..8a733a5 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -890,7 +890,7 @@
new OpenClosedInterfacesOptions();
private final ProtoShrinkingOptions protoShrinking = new ProtoShrinkingOptions();
private final RedundantBridgeRemovalOptions redundantBridgeRemovalOptions =
- new RedundantBridgeRemovalOptions(this);
+ new RedundantBridgeRemovalOptions();
private final KotlinOptimizationOptions kotlinOptimizationOptions =
new KotlinOptimizationOptions();
private final ApiModelTestingOptions apiModelTestingOptions = new ApiModelTestingOptions();
diff --git a/src/test/java/com/android/tools/r8/shaking/constructor/ForwardingConstructorUsedFromPlatformShakingOnDexTest.java b/src/test/java/com/android/tools/r8/shaking/constructor/ForwardingConstructorUsedFromPlatformShakingOnDexTest.java
index 7dacf64..f4d629f 100644
--- a/src/test/java/com/android/tools/r8/shaking/constructor/ForwardingConstructorUsedFromPlatformShakingOnDexTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/constructor/ForwardingConstructorUsedFromPlatformShakingOnDexTest.java
@@ -5,15 +5,13 @@
package com.android.tools.r8.shaking.constructor;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
-import static com.android.tools.r8.utils.codeinspector.Matchers.onlyIf;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assume.assumeFalse;
import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.apimodel.ApiModelingTestHelper;
-import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
@@ -41,15 +39,11 @@
private static List<byte[]> transformedLibraryClassFileData;
@Parameter(0)
- public boolean enableModeling;
-
- @Parameter(1)
public TestParameters parameters;
- @Parameters(name = "{1}, modeling: {0}")
- public static List<Object[]> data() {
- return buildParameters(
- BooleanUtils.values(), getTestParameters().withAllRuntimesAndApiLevels().build());
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
}
@BeforeClass
@@ -60,7 +54,6 @@
@Test
public void testRuntime() throws Exception {
- assumeFalse(enableModeling);
testForRuntime(parameters)
.addProgramClassFileData(transformedProgramClassFileData)
.addProgramClassFileData(transformedLibraryClassFileData)
@@ -75,14 +68,6 @@
.addLibraryClassFileData(transformedLibraryClassFileData)
.addLibraryFiles(parameters.getDefaultRuntimeLibrary())
.addKeepMainRule(Main.class)
- .applyIf(
- !enableModeling,
- testBuilder ->
- testBuilder.addOptionsModification(
- options ->
- options
- .getRedundantBridgeRemovalOptions()
- .clearNoConstructorShrinkingHierarchiesForTesting()))
// Since Fragment is first defined in API 11.
.apply(ApiModelingTestHelper::disableStubbingOfClasses)
.enableNeverClassInliningAnnotations()
@@ -91,10 +76,7 @@
.inspect(this::inspect)
.addRunClasspathClassFileData(transformedLibraryClassFileData)
.run(parameters.getRuntime(), Main.class)
- .applyIf(
- enableModeling || !parameters.canHaveNonReboundConstructorInvoke(),
- runResult -> runResult.assertSuccessWithOutput(EXPECTED_OUTPUT),
- runResult -> runResult.assertFailureWithErrorThatThrows(NoSuchMethodException.class));
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
}
private static List<byte[]> getTransformedProgramClasses() throws Exception {
@@ -144,15 +126,11 @@
private void inspect(CodeInspector inspector) {
ClassSubject myFragmentClassSubject = inspector.clazz(MyFragment.class);
assertThat(myFragmentClassSubject, isPresent());
- assertThat(
- myFragmentClassSubject.init(),
- onlyIf(enableModeling || !parameters.canHaveNonReboundConstructorInvoke(), isPresent()));
+ assertThat(myFragmentClassSubject.init(), isPresent());
ClassSubject myZygotePreloadClassSubject = inspector.clazz(MyZygotePreload.class);
assertThat(myZygotePreloadClassSubject, isPresent());
- assertThat(
- myZygotePreloadClassSubject.init(),
- onlyIf(enableModeling || !parameters.canHaveNonReboundConstructorInvoke(), isPresent()));
+ assertThat(myZygotePreloadClassSubject.init(), isPresent());
}
// Library classes.
