[ApiModel] Enable api modeling
This CL enables api-modeling for all references to ensure no inlining of
methods into other methods with a lower api level. The CL also ensure
that merged classes will have the same api level.
RELNOTES: R8 prevents soft-verification errors by modeling all references to android api's introduced later than min-api. Concretely, R8 disables inlining of methods with a higher api-level into methods with a lower level and merging of classes with different levels.
Bug: 138781768
Bug: 187496508
Fixes: 188388130
Fixes: 204063897
Change-Id: I3e0008384d719eaaf09cde0b12702ec9bed658d3
diff --git a/src/main/java/com/android/tools/r8/androidapi/AndroidApiReferenceLevelCache.java b/src/main/java/com/android/tools/r8/androidapi/AndroidApiReferenceLevelCache.java
index df4cefd..9a4afa3 100644
--- a/src/main/java/com/android/tools/r8/androidapi/AndroidApiReferenceLevelCache.java
+++ b/src/main/java/com/android/tools/r8/androidapi/AndroidApiReferenceLevelCache.java
@@ -88,10 +88,12 @@
// of the program.
return appView.options().minApiLevel;
}
- return reference.apply(
- androidApiLevelDatabase::getTypeApiLevel,
- androidApiLevelDatabase::getFieldApiLevel,
- androidApiLevelDatabase::getMethodApiLevel);
+ return reference
+ .apply(
+ androidApiLevelDatabase::getTypeApiLevel,
+ androidApiLevelDatabase::getFieldApiLevel,
+ androidApiLevelDatabase::getMethodApiLevel)
+ .max(appView.options().minApiLevel);
}
private boolean isReferenceToJavaLangObject(DexReference reference) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
index 0b7b139..8007b17 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
@@ -127,8 +127,7 @@
WhyAreYouNotInliningReporter whyAreYouNotInliningReporter) {
// Do not inline if the inlinee is greater than the api caller level.
// TODO(b/188498051): We should not force inline lower api method calls.
- if (reason != Reason.FORCE
- && isApiSafeForInlining(method, singleTarget, appView.options()).isPossiblyFalse()) {
+ if (reason != Reason.FORCE && !isApiSafeForInlining(method, singleTarget, appView.options())) {
whyAreYouNotInliningReporter.reportInlineeHigherApiCall();
return false;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
index 5f05d8d..228c259 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
@@ -880,7 +880,7 @@
return null;
}
// Check the api level is allowed to be inlined.
- if (isApiSafeForInlining(method, singleTarget, appView.options()).isPossiblyFalse()) {
+ if (!isApiSafeForInlining(method, singleTarget, appView.options())) {
return null;
}
// Check that the entire constructor chain can be inlined into the current context.
@@ -910,7 +910,7 @@
return null;
}
// Check the api level is allowed to be inlined.
- if (isApiSafeForInlining(method, encodedParent, appView.options()).isPossiblyFalse()) {
+ if (!isApiSafeForInlining(method, encodedParent, appView.options())) {
return null;
}
parent =
diff --git a/src/main/java/com/android/tools/r8/utils/AndroidApiLevelUtils.java b/src/main/java/com/android/tools/r8/utils/AndroidApiLevelUtils.java
index 96c4a2b..73ee4de 100644
--- a/src/main/java/com/android/tools/r8/utils/AndroidApiLevelUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/AndroidApiLevelUtils.java
@@ -8,18 +8,17 @@
public class AndroidApiLevelUtils {
- public static OptionalBool isApiSafeForInlining(
+ public static boolean isApiSafeForInlining(
ProgramMethod caller, ProgramMethod inlinee, InternalOptions options) {
if (!options.apiModelingOptions().enableApiCallerIdentification) {
- return OptionalBool.TRUE;
+ return true;
}
if (caller.getHolderType() == inlinee.getHolderType()) {
- return OptionalBool.TRUE;
+ return true;
}
- return OptionalBool.of(
- caller
- .getDefinition()
- .getApiLevel()
- .isGreaterThanOrEqualTo(inlinee.getDefinition().getApiLevelForCode()));
+ return caller
+ .getDefinition()
+ .getApiLevel()
+ .isGreaterThanOrEqualTo(inlinee.getDefinition().getApiLevelForCode());
}
}
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 0fdc11c..88c96ef 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -1473,7 +1473,7 @@
public Map<ClassReference, AndroidApiLevel> classApiMapping = new HashMap<>();
public BiConsumer<MethodReference, AndroidApiLevel> tracedMethodApiLevelCallback = null;
- public boolean enableApiCallerIdentification = false;
+ public boolean enableApiCallerIdentification = true;
public boolean checkAllApiReferencesAreSet = true;
public boolean useHashingDatabase = true;
diff --git a/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergerTest.java b/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergerTest.java
index 46ecc50..699bb50 100644
--- a/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergerTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergerTest.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.classmerging.vertical;
import static com.android.tools.r8.smali.SmaliBuilder.buildCode;
+import static com.android.tools.r8.utils.AndroidApiLevel.K;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.MatcherAssert.assertThat;
@@ -315,10 +316,13 @@
CF_DIR.resolve("FieldCollisionTest$A.class"),
CF_DIR.resolve("FieldCollisionTest$B.class")
};
- Set<String> preservedClassNames =
- ImmutableSet.of(
- "classmerging.FieldCollisionTest",
- "classmerging.FieldCollisionTest$B");
+ ImmutableSet.Builder<String> preservedNamesBuilder = ImmutableSet.builder();
+ preservedNamesBuilder.add("classmerging.FieldCollisionTest");
+ preservedNamesBuilder.add("classmerging.FieldCollisionTest$B");
+ if (parameters.isCfRuntime() || parameters.getApiLevel().isLessThan(K)) {
+ preservedNamesBuilder.add("classmerging.FieldCollisionTest$A");
+ }
+ Set<String> preservedClassNames = preservedNamesBuilder.build();
runTest(
testForR8(parameters.getBackend())
.addKeepRules(getProguardConfig(EXAMPLE_KEEP))
diff --git a/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/NestAttributesUpdateTest.java b/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/NestAttributesUpdateTest.java
index 00575cd..bc389c9 100644
--- a/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/NestAttributesUpdateTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/NestAttributesUpdateTest.java
@@ -12,10 +12,12 @@
import static junit.framework.TestCase.assertTrue;
import com.android.tools.r8.Jdk9TestUtils;
+import com.android.tools.r8.R8FullTestBuilder;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.TestRuntime.CfVm;
+import com.android.tools.r8.ThrowableConsumer;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.NestMemberClassAttribute;
import com.android.tools.r8.utils.StringUtils;
@@ -51,33 +53,72 @@
@Test
public void testClassMergingNestMemberRemoval() throws Exception {
- testNestAttributesCorrect(MERGING_OUTER_CLASS, MERGING_OUTER_CLASS, MERGING_EXPECTED_RESULT, 3);
+ testNestAttributesCorrect(
+ MERGING_OUTER_CLASS,
+ MERGING_OUTER_CLASS,
+ MERGING_EXPECTED_RESULT,
+ 3,
+ ThrowableConsumer.empty());
}
@Test
public void testClassMergingNestHostRemoval() throws Exception {
testNestAttributesCorrect(
- MERGING_OUTER_CLASS + "$MiddleOuter", MERGING_OUTER_CLASS, MERGING_EXPECTED_RESULT, 2);
+ MERGING_OUTER_CLASS + "$MiddleOuter",
+ MERGING_OUTER_CLASS,
+ MERGING_EXPECTED_RESULT,
+ 2,
+ builder -> {
+ builder.addOptionsModification(
+ internalOptions -> {
+ // The test makes an invoke to StringConcatFactory which is not known to DEX and
+ // we therefore fail to merge the classes.
+ internalOptions.apiModelingOptions().enableApiCallerIdentification = false;
+ });
+ });
}
@Test
public void testTreePruningNestMemberRemoval() throws Exception {
- testNestAttributesCorrect(PRUNING_OUTER_CLASS, PRUNING_OUTER_CLASS, PRUNING_EXPECTED_RESULT, 2);
+ testNestAttributesCorrect(
+ PRUNING_OUTER_CLASS,
+ PRUNING_OUTER_CLASS,
+ PRUNING_EXPECTED_RESULT,
+ 2,
+ ThrowableConsumer.empty());
}
@Test
public void testTreePruningNestHostRemoval() throws Exception {
testNestAttributesCorrect(
- PRUNING_OUTER_CLASS + "$Pruned", PRUNING_OUTER_CLASS, PRUNING_EXPECTED_RESULT, 1);
+ PRUNING_OUTER_CLASS + "$Pruned",
+ PRUNING_OUTER_CLASS,
+ PRUNING_EXPECTED_RESULT,
+ 1,
+ ThrowableConsumer.empty());
}
public void testNestAttributesCorrect(
- String mainClassName, String outerNestName, String expectedResult, int expectedNumClassesLeft)
+ String mainClassName,
+ String outerNestName,
+ String expectedResult,
+ int expectedNumClassesLeft,
+ ThrowableConsumer<R8FullTestBuilder> testBuilderConsumer)
throws Exception {
testNestAttributesCorrect(
- mainClassName, outerNestName, expectedResult, true, expectedNumClassesLeft);
+ mainClassName,
+ outerNestName,
+ expectedResult,
+ true,
+ expectedNumClassesLeft,
+ testBuilderConsumer);
testNestAttributesCorrect(
- mainClassName, outerNestName, expectedResult, false, expectedNumClassesLeft);
+ mainClassName,
+ outerNestName,
+ expectedResult,
+ false,
+ expectedNumClassesLeft,
+ testBuilderConsumer);
}
public void testNestAttributesCorrect(
@@ -85,7 +126,8 @@
String outerNestName,
String expectedResult,
boolean minification,
- int expectedNumClassesLeft)
+ int expectedNumClassesLeft,
+ ThrowableConsumer<R8FullTestBuilder> testBuilderConsumer)
throws Exception {
String actualMainClassName = PACKAGE_NAME + mainClassName;
testForR8(parameters.getBackend())
@@ -101,6 +143,7 @@
.applyIf(parameters.isCfRuntime(), Jdk9TestUtils.addJdk9LibraryFiles(temp))
.addKeepPackageNamesRule("nesthostexample")
.addInliningAnnotations()
+ .apply(testBuilderConsumer)
.compile()
.inspect(
inspector -> {
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ConstructorWithNonTrivialControlFlowTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ConstructorWithNonTrivialControlFlowTest.java
index 693b9bf..26bbf63 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ConstructorWithNonTrivialControlFlowTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ConstructorWithNonTrivialControlFlowTest.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.NeverPropagateValue;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
@@ -56,9 +57,9 @@
private void verifyClassInliningRemovesCandidate(CodeInspector inspector) {
ClassSubject candidateClassSubject = inspector.clazz(Candidate.class);
- if (enableClassInlining) {
- // TODO(b/188388130): String.empty was added in AndroidApiLevel.G, so if this fails after
- // enabling api modeling then check for api level.
+ if (enableClassInlining
+ && (parameters.isDexRuntime()
+ && parameters.getApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.G))) {
assertThat(candidateClassSubject, not(isPresent()));
} else {
assertThat(candidateClassSubject, isPresent());
diff --git a/tools/chrome_data.py b/tools/chrome_data.py
index 0d4329d..ebae542 100644
--- a/tools/chrome_data.py
+++ b/tools/chrome_data.py
@@ -248,6 +248,7 @@
'inputs': [os.path.join(V180917_BASE, path) for path in INPUT_JARS],
'pgconf': [os.path.join(V180917_BASE, path) for path in PG_CONFS],
'libraries': [os.path.join(V180917_BASE, path) for path in LIBRARIES],
+ 'min-api': '21',
},
},
'200430': {
