Merge "Don't generate compat rules when tracing reflective use"
diff --git a/src/main/java/com/android/tools/r8/experimental/graphinfo/GraphEdgeInfo.java b/src/main/java/com/android/tools/r8/experimental/graphinfo/GraphEdgeInfo.java
index 0c838ac..e3fa01b 100644
--- a/src/main/java/com/android/tools/r8/experimental/graphinfo/GraphEdgeInfo.java
+++ b/src/main/java/com/android/tools/r8/experimental/graphinfo/GraphEdgeInfo.java
@@ -16,9 +16,11 @@
InvokedFrom,
InvokedFromLambdaCreatedIn,
ReferencedFrom,
+ ReflectiveUseFrom,
ReachableFromLiveType,
ReferencedInAnnotation,
IsLibraryMethod,
+ MethodHandleUseFrom
}
private final EdgeKind kind;
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 f10d000..673314d 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -604,7 +604,7 @@
if (methodHandle.isMethodHandle() && use != MethodHandleUse.ARGUMENT_TO_LAMBDA_METAFACTORY) {
DexClass holder = appInfo.definitionFor(methodHandle.asMethod().holder);
if (holder != null) {
- markClassAsInstantiatedWithMethodHandleRule(holder);
+ markInstantiated(holder.type, KeepReason.methodHandleReferencedIn(currentMethod));
}
}
}
@@ -1189,8 +1189,10 @@
private void markVirtualMethodAsLive(DexEncodedMethod method, KeepReason reason) {
assert method != null;
- // Only explicit keep rules should make abstract methods live.
- assert !method.accessFlags.isAbstract() || reason.isDueToKeepRule();
+ // Only explicit keep rules or reflective use should make abstract methods live.
+ assert !method.accessFlags.isAbstract()
+ || reason.isDueToKeepRule()
+ || reason.isDueToReflectiveUse();
if (!liveMethods.contains(method)) {
if (Log.ENABLED) {
Log.verbose(getClass(), "Adding virtual method `%s` to live set.", method.method);
@@ -1662,16 +1664,8 @@
collectReachedFields(staticFields, this::tryLookupStaticField)));
}
- private void markClassAsInstantiatedWithMethodHandleRule(DexClass clazz) {
- ProguardKeepRule rule =
- ProguardConfigurationUtils.buildMethodHandleKeepRule(clazz);
- proguardCompatibilityWorkList.add(
- Action.markInstantiated(clazz, KeepReason.dueToProguardCompatibilityKeepRule(rule)));
- }
-
private void markClassAsInstantiatedWithCompatRule(DexClass clazz) {
- ProguardKeepRule rule =
- ProguardConfigurationUtils.buildDefaultInitializerKeepRule(clazz);
+ ProguardKeepRule rule = ProguardConfigurationUtils.buildDefaultInitializerKeepRule(clazz);
proguardCompatibilityWorkList.add(
Action.markInstantiated(clazz, KeepReason.dueToProguardCompatibilityKeepRule(rule)));
if (clazz.hasDefaultInitializer()) {
@@ -1681,19 +1675,6 @@
}
}
- private void markFieldAsKeptWithCompatRule(DexEncodedField field, boolean keepClass) {
- DexClass holderClass = appInfo.definitionFor(field.field.getHolder());
- ProguardKeepRule rule =
- ProguardConfigurationUtils.buildFieldKeepRule(holderClass, field, keepClass);
- if (keepClass) {
- proguardCompatibilityWorkList.add(
- Action.markInstantiated(
- holderClass, KeepReason.dueToProguardCompatibilityKeepRule(rule)));
- }
- proguardCompatibilityWorkList.add(
- Action.markFieldKept(field, KeepReason.dueToProguardCompatibilityKeepRule(rule)));
- }
-
private void markMethodAsKeptWithCompatRule(DexEncodedMethod method) {
DexClass holderClass = appInfo.definitionFor(method.method.getHolder());
ProguardKeepRule rule =
@@ -1733,7 +1714,11 @@
if (identifierItem.isDexType()) {
DexClass clazz = appInfo.definitionFor(identifierItem.asDexType());
if (clazz != null) {
- markClassAsInstantiatedWithCompatRule(clazz);
+ markInstantiated(clazz.type, KeepReason.reflectiveUseIn(method));
+ if (clazz.hasDefaultInitializer()) {
+ markDirectStaticOrConstructorMethodAsLive(
+ clazz.getDefaultInitializer(), KeepReason.reflectiveUseIn(method));
+ }
}
} else if (identifierItem.isDexField()) {
DexEncodedField encodedField = appInfo.definitionFor(identifierItem.asDexField());
@@ -1746,13 +1731,22 @@
boolean keepClass =
!encodedField.accessFlags.isStatic()
&& appInfo.dexItemFactory.atomicFieldUpdaterMethods.isFieldUpdater(invokedMethod);
- markFieldAsKeptWithCompatRule(encodedField, keepClass);
+ if (keepClass) {
+ DexClass holderClass = appInfo.definitionFor(encodedField.field.getHolder());
+ markInstantiated(holderClass.type, KeepReason.reflectiveUseIn(method));
+ }
+ markFieldAsKept(encodedField, KeepReason.reflectiveUseIn(method));
}
} else {
assert identifierItem.isDexMethod();
DexEncodedMethod encodedMethod = appInfo.definitionFor(identifierItem.asDexMethod());
if (encodedMethod != null) {
- markMethodAsKeptWithCompatRule(encodedMethod);
+ if (encodedMethod.accessFlags.isStatic() || encodedMethod.accessFlags.isConstructor()) {
+ markDirectStaticOrConstructorMethodAsLive(
+ encodedMethod, KeepReason.reflectiveUseIn(method));
+ } else {
+ markVirtualMethodAsLive(encodedMethod, KeepReason.reflectiveUseIn(method));
+ }
}
}
}
diff --git a/src/main/java/com/android/tools/r8/shaking/KeepReason.java b/src/main/java/com/android/tools/r8/shaking/KeepReason.java
index 16d78a7..a797742 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepReason.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepReason.java
@@ -61,6 +61,10 @@
return false;
}
+ public boolean isDueToReflectiveUse() {
+ return false;
+ }
+
public boolean isDueToProguardCompatibility() {
return false;
}
@@ -73,6 +77,14 @@
return new TargetedBySuper(from);
}
+ public static KeepReason reflectiveUseIn(DexEncodedMethod method) {
+ return new ReflectiveUseFrom(method);
+ }
+
+ public static KeepReason methodHandleReferencedIn(DexEncodedMethod method) {
+ return new MethodHandleReferencedFrom(method);
+ }
+
private static class DueToKeepRule extends KeepReason {
final ProguardKeepRule keepRule;
@@ -289,4 +301,43 @@
return enqueuer.getAnnotationGraphNode(holder);
}
}
+
+ private static class ReflectiveUseFrom extends BasedOnOtherMethod {
+
+ private ReflectiveUseFrom(DexEncodedMethod method) {
+ super(method);
+ }
+
+ @Override
+ public boolean isDueToReflectiveUse() {
+ return true;
+ }
+
+ @Override
+ public EdgeKind edgeKind() {
+ return EdgeKind.ReflectiveUseFrom;
+ }
+
+ @Override
+ String getKind() {
+ return "reflective use in";
+ }
+ }
+
+ private static class MethodHandleReferencedFrom extends BasedOnOtherMethod {
+
+ private MethodHandleReferencedFrom(DexEncodedMethod method) {
+ super(method);
+ }
+
+ @Override
+ public EdgeKind edgeKind() {
+ return EdgeKind.MethodHandleUseFrom;
+ }
+
+ @Override
+ String getKind() {
+ return "method handle referenced from";
+ }
+ }
}
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 163c60b..f9846b8 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationUtils.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationUtils.java
@@ -5,13 +5,8 @@
package com.android.tools.r8.shaking;
import com.android.tools.r8.graph.DexClass;
-import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
-import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItemFactory;
-import com.android.tools.r8.graph.DexMethod;
-import com.android.tools.r8.graph.DexReference;
-import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.shaking.ProguardConfigurationParser.IdentifierPatternWithWildcards;
import com.android.tools.r8.utils.AndroidApiLevel;
@@ -31,19 +26,6 @@
}
};
- public static ProguardKeepRule buildMethodHandleKeepRule(DexClass clazz) {
- ProguardKeepRule.Builder builder = ProguardKeepRule.builder();
- builder.setOrigin(proguardCompatOrigin);
- builder.setType(ProguardKeepRuleType.KEEP);
- builder.getModifiersBuilder().setAllowsObfuscation(true);
- builder.getModifiersBuilder().setAllowsOptimization(true);
- builder.setClassType(
- clazz.isInterface() ? ProguardClassType.INTERFACE : ProguardClassType.CLASS);
- builder.setClassNames(
- ProguardClassNameList.singletonList(ProguardTypeMatcher.create(clazz.type)));
- return builder.build();
- }
-
public static ProguardKeepRule buildDefaultInitializerKeepRule(DexClass clazz) {
ProguardKeepRule.Builder builder = ProguardKeepRule.builder();
builder.setOrigin(proguardCompatOrigin);
@@ -64,36 +46,6 @@
return builder.build();
}
- public static ProguardKeepRule buildFieldKeepRule(
- DexClass clazz, DexEncodedField field, boolean keepClass) {
- assert clazz.type == field.field.getHolder();
- ProguardKeepRule.Builder builder = ProguardKeepRule.builder();
- builder.setOrigin(proguardCompatOrigin);
- if (keepClass) {
- builder.setType(ProguardKeepRuleType.KEEP);
- } else {
- builder.setType(ProguardKeepRuleType.KEEP_CLASS_MEMBERS);
- }
- builder.getModifiersBuilder().setAllowsObfuscation(true);
- builder.getModifiersBuilder().setAllowsOptimization(true);
- builder.getClassAccessFlags().setVisibility(clazz.accessFlags);
- if (clazz.isInterface()) {
- builder.setClassType(ProguardClassType.INTERFACE);
- } else {
- builder.setClassType(ProguardClassType.CLASS);
- }
- builder.setClassNames(
- ProguardClassNameList.singletonList(ProguardTypeMatcher.create(clazz.type)));
- ProguardMemberRule.Builder memberRuleBuilder = ProguardMemberRule.builder();
- memberRuleBuilder.setRuleType(ProguardMemberType.FIELD);
- memberRuleBuilder.getAccessFlags().setFlags(field.accessFlags);
- memberRuleBuilder.setName(
- IdentifierPatternWithWildcards.withoutWildcards(field.field.name.toString()));
- memberRuleBuilder.setTypeMatcher(ProguardTypeMatcher.create(field.field.type));
- builder.getMemberRules().add(memberRuleBuilder.build());
- return builder.build();
- }
-
public static ProguardKeepRule buildMethodKeepRule(DexClass clazz, DexEncodedMethod method) {
// TODO(b/122295241): These generated rules should be linked into the graph, eg, the method
// using identified reflection should be the source keeping the target alive.
@@ -125,41 +77,6 @@
return builder.build();
}
- public static ProguardIdentifierNameStringRule buildIdentifierNameStringRule(DexReference item) {
- assert item.isDexField() || item.isDexMethod();
- ProguardIdentifierNameStringRule.Builder builder = ProguardIdentifierNameStringRule.builder();
- ProguardMemberRule.Builder memberRuleBuilder = ProguardMemberRule.builder();
- DexType holderType;
- if (item.isDexField()) {
- DexField field = item.asDexField();
- holderType = field.getHolder();
- memberRuleBuilder.setRuleType(ProguardMemberType.FIELD);
- memberRuleBuilder.setName(
- IdentifierPatternWithWildcards.withoutWildcards(field.name.toString()));
- memberRuleBuilder.setTypeMatcher(ProguardTypeMatcher.create(field.type));
- } else {
- DexMethod method = item.asDexMethod();
- holderType = method.getHolder();
- memberRuleBuilder.setRuleType(ProguardMemberType.METHOD);
- memberRuleBuilder.setName(
- IdentifierPatternWithWildcards.withoutWildcards(method.name.toString()));
- memberRuleBuilder.setTypeMatcher(ProguardTypeMatcher.create(method.proto.returnType));
- List<ProguardTypeMatcher> arguments = Arrays.stream(method.proto.parameters.values)
- .map(ProguardTypeMatcher::create)
- .collect(Collectors.toList());
- memberRuleBuilder.setArguments(arguments);
- }
- if (holderType.isInterface()) {
- builder.setClassType(ProguardClassType.INTERFACE);
- } else {
- builder.setClassType(ProguardClassType.CLASS);
- }
- builder.setClassNames(
- ProguardClassNameList.singletonList(ProguardTypeMatcher.create(holderType)));
- builder.getMemberRules().add(memberRuleBuilder.build());
- return builder.build();
- }
-
public static ProguardAssumeValuesRule buildAssumeValuesForApiLevel(
DexItemFactory factory, AndroidApiLevel apiLevel) {
Origin synthesizedFromApiLevel =
diff --git a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ForceProguardCompatibilityTest.java b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ForceProguardCompatibilityTest.java
index 6504169..906aee9 100644
--- a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ForceProguardCompatibilityTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ForceProguardCompatibilityTest.java
@@ -23,8 +23,6 @@
import com.android.tools.r8.shaking.ProguardConfigurationParser;
import com.android.tools.r8.shaking.ProguardConfigurationRule;
import com.android.tools.r8.shaking.ProguardKeepAttributes;
-import com.android.tools.r8.shaking.ProguardKeepRule;
-import com.android.tools.r8.shaking.ProguardKeepRuleType;
import com.android.tools.r8.shaking.ProguardMemberRule;
import com.android.tools.r8.shaking.ProguardMemberType;
import com.android.tools.r8.shaking.forceproguardcompatibility.defaultmethods.ClassImplementingInterface;
@@ -40,8 +38,6 @@
import com.android.tools.r8.utils.codeinspector.FieldSubject;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
import com.google.common.collect.ImmutableList;
-import it.unimi.dsi.fastutil.objects.Object2BooleanArrayMap;
-import it.unimi.dsi.fastutil.objects.Object2BooleanMap;
import java.io.File;
import java.nio.file.Path;
import java.util.ArrayList;
@@ -277,39 +273,6 @@
assertEquals(subject.isPresent() && allowObfuscation, subject.isRenamed());
});
- // Check the Proguard compatibility rules generated.
- ProguardConfigurationParser parser =
- new ProguardConfigurationParser(new DexItemFactory(), new Reporter());
- parser.parse(proguardCompatibilityRules);
- ProguardConfiguration configuration = parser.getConfigRawForTesting();
- if (forceProguardCompatibility) {
- List<ProguardConfigurationRule> rules = configuration.getRules();
- assertEquals(2, rules.size());
- for (ProguardConfigurationRule r : rules) {
- assertTrue(r instanceof ProguardKeepRule);
- ProguardKeepRule rule = (ProguardKeepRule) r;
- assertEquals(ProguardKeepRuleType.KEEP, rule.getType());
- assertTrue(rule.getModifiers().allowsObfuscation);
- assertTrue(rule.getModifiers().allowsOptimization);
- List<ProguardMemberRule> memberRules = rule.getMemberRules();
- ProguardClassNameList classNames = rule.getClassNames();
- assertEquals(1, classNames.size());
- DexType type = classNames.asSpecificDexTypes().get(0);
- if (type.toSourceString().equals(forNameClass1.getCanonicalName())) {
- assertEquals(1, memberRules.size());
- assertEquals(ProguardMemberType.INIT, memberRules.iterator().next().getRuleType());
- } else {
- assertTrue(type.toSourceString().equals(forNameClass2.getCanonicalName()));
- // During parsing we add in the default constructor if there are otherwise no single
- // member rule.
- assertEquals(1, memberRules.size());
- assertEquals(ProguardMemberType.INIT, memberRules.iterator().next().getRuleType());
- }
- }
- } else {
- assertEquals(0, configuration.getRules().size());
- }
-
if (isRunProguard()) {
Path proguardedJar = File.createTempFile("proguarded", ".jar", temp.getRoot()).toPath();
Path proguardConfigFile = File.createTempFile("proguard", ".config", temp.getRoot()).toPath();
@@ -376,38 +339,6 @@
assertTrue(bar.isPresent());
assertEquals(bar.isPresent() && allowObfuscation, bar.isRenamed());
- // Check the Proguard compatibility rules generated.
- ProguardConfigurationParser parser =
- new ProguardConfigurationParser(new DexItemFactory(), new Reporter());
- parser.parse(proguardCompatibilityRules);
- ProguardConfiguration configuration = parser.getConfigRawForTesting();
- if (forceProguardCompatibility) {
- List<ProguardConfigurationRule> rules = configuration.getRules();
- assertEquals(2, rules.size());
- for (ProguardConfigurationRule r : rules) {
- assertTrue(r instanceof ProguardKeepRule);
- ProguardKeepRule rule = (ProguardKeepRule) r;
- assertEquals(ProguardKeepRuleType.KEEP_CLASS_MEMBERS, rule.getType());
- assertTrue(rule.getModifiers().allowsObfuscation);
- assertTrue(rule.getModifiers().allowsOptimization);
- List<ProguardMemberRule> memberRules = rule.getMemberRules();
- ProguardClassNameList classNames = rule.getClassNames();
- assertEquals(1, classNames.size());
- DexType type = classNames.asSpecificDexTypes().get(0);
- assertEquals(withMemberClass.getCanonicalName(), type.toSourceString());
- assertEquals(1, memberRules.size());
- ProguardMemberRule memberRule = memberRules.iterator().next();
- if (memberRule.getRuleType() == ProguardMemberType.FIELD) {
- assertTrue(memberRule.getName().matches("foo"));
- } else {
- assertEquals(ProguardMemberType.METHOD, memberRule.getRuleType());
- assertTrue(memberRule.getName().matches("bar"));
- }
- }
- } else {
- assertEquals(0, configuration.getRules().size());
- }
-
if (isRunProguard()) {
Path proguardedJar = File.createTempFile("proguarded", ".jar", temp.getRoot()).toPath();
Path proguardConfigFile = File.createTempFile("proguard", ".config", temp.getRoot()).toPath();
@@ -481,39 +412,6 @@
assertTrue(f.isPresent());
assertEquals(f.isPresent() && allowObfuscation, f.isRenamed());
- // Check the Proguard compatibility rules generated.
- ProguardConfigurationParser parser =
- new ProguardConfigurationParser(new DexItemFactory(), new Reporter());
- parser.parse(proguardCompatibilityRules);
- ProguardConfiguration configuration = parser.getConfigRawForTesting();
- if (forceProguardCompatibility) {
- List<ProguardConfigurationRule> rules = configuration.getRules();
- assertEquals(3, rules.size());
- Object2BooleanMap<String> keptFields = new Object2BooleanArrayMap<>();
- for (ProguardConfigurationRule r : rules) {
- assertTrue(r instanceof ProguardKeepRule);
- ProguardKeepRule rule = (ProguardKeepRule) r;
- assertEquals(ProguardKeepRuleType.KEEP, rule.getType());
- assertTrue(rule.getModifiers().allowsObfuscation);
- assertTrue(rule.getModifiers().allowsOptimization);
- List<ProguardMemberRule> memberRules = rule.getMemberRules();
- ProguardClassNameList classNames = rule.getClassNames();
- assertEquals(1, classNames.size());
- DexType type = classNames.asSpecificDexTypes().get(0);
- assertEquals(withVolatileFields.getCanonicalName(), type.toSourceString());
- assertEquals(1, memberRules.size());
- ProguardMemberRule memberRule = memberRules.iterator().next();
- assertEquals(ProguardMemberType.FIELD, memberRule.getRuleType());
- keptFields.put(memberRule.getName().toString(), true);
- }
- assertEquals(3, keptFields.size());
- assertTrue(keptFields.containsKey("intField"));
- assertTrue(keptFields.containsKey("longField"));
- assertTrue(keptFields.containsKey("objField"));
- } else {
- assertEquals(0, configuration.getRules().size());
- }
-
if (isRunProguard()) {
Path proguardedJar = File.createTempFile("proguarded", ".jar", temp.getRoot()).toPath();
Path proguardConfigFile = File.createTempFile("proguard", ".config", temp.getRoot()).toPath();
