Check dexing of j$ jar does not modify descriptors
Change-Id: I42c92156b7baf5482272e80f567a11b1e76d770c
diff --git a/src/main/java/com/android/tools/r8/D8Command.java b/src/main/java/com/android/tools/r8/D8Command.java
index e2c826a..bd0152e 100644
--- a/src/main/java/com/android/tools/r8/D8Command.java
+++ b/src/main/java/com/android/tools/r8/D8Command.java
@@ -100,6 +100,7 @@
private boolean minimalMainDex = false;
private final List<ProguardConfigurationSource> mainDexRules = new ArrayList<>();
private boolean enableMissingLibraryApiModeling = false;
+ private boolean enableRewritingOfArtProfilesIsNopCheck = false;
private Builder() {
this(new DefaultD8DiagnosticsHandler());
@@ -410,6 +411,11 @@
return self();
}
+ Builder setEnableRewritingOfArtProfilesIsNopCheck() {
+ enableRewritingOfArtProfilesIsNopCheck = true;
+ return self();
+ }
+
@Override
void validate() {
if (isPrintHelp()) {
@@ -523,6 +529,7 @@
proguardMapConsumer,
partitionMapConsumer,
enableMissingLibraryApiModeling,
+ enableRewritingOfArtProfilesIsNopCheck,
getAndroidPlatformBuild(),
getArtProfilesForRewriting(),
getStartupProfileProviders(),
@@ -545,6 +552,7 @@
private final StringConsumer proguardMapConsumer;
private final PartitionMapConsumer partitionMapConsumer;
private final boolean enableMissingLibraryApiModeling;
+ private final boolean enableRewritingOfArtProfilesIsNopCheck;
private final DexItemFactory factory;
public static Builder builder() {
@@ -621,6 +629,7 @@
StringConsumer proguardMapConsumer,
PartitionMapConsumer partitionMapConsumer,
boolean enableMissingLibraryApiModeling,
+ boolean enableRewritingOfArtProfilesIsNopCheck,
boolean isAndroidPlatformBuild,
List<ArtProfileForRewriting> artProfilesForRewriting,
List<StartupProfileProvider> startupProfileProviders,
@@ -662,6 +671,7 @@
this.proguardMapConsumer = proguardMapConsumer;
this.partitionMapConsumer = partitionMapConsumer;
this.enableMissingLibraryApiModeling = enableMissingLibraryApiModeling;
+ this.enableRewritingOfArtProfilesIsNopCheck = enableRewritingOfArtProfilesIsNopCheck;
this.factory = factory;
}
@@ -680,6 +690,7 @@
proguardMapConsumer = null;
partitionMapConsumer = null;
enableMissingLibraryApiModeling = false;
+ enableRewritingOfArtProfilesIsNopCheck = false;
factory = null;
}
@@ -751,6 +762,10 @@
internal.apiModelingOptions().disableOutliningAndStubbing();
}
+ if (enableRewritingOfArtProfilesIsNopCheck) {
+ internal.getArtProfileOptions().setEnableNopCheckForTesting();
+ }
+
// Default is to remove all javac generated assertion code when generating dex.
assert internal.assertionsConfiguration == null;
internal.assertionsConfiguration =
diff --git a/src/main/java/com/android/tools/r8/L8Command.java b/src/main/java/com/android/tools/r8/L8Command.java
index c471c33..679dd95 100644
--- a/src/main/java/com/android/tools/r8/L8Command.java
+++ b/src/main/java/com/android/tools/r8/L8Command.java
@@ -357,9 +357,13 @@
if (proguardMapConsumer != null || partitionMapConsumer != null) {
reporter.error("L8 does not support defining a map consumer when not shrinking");
}
- if (isGeneratingClassFiles && !getArtProfilesForRewriting().isEmpty()) {
- reporter.error(
- "L8 does not support rewriting of ART profiles when generating class files");
+ if (!getArtProfilesForRewriting().isEmpty()) {
+ if (isGeneratingClassFiles) {
+ reporter.error(
+ "L8 does not support rewriting of ART profiles when generating class files");
+ } else {
+ reporter.error("L8 does not impact ART profiles when generating DEX and not shrinking");
+ }
}
}
super.validate();
@@ -432,12 +436,8 @@
.setMode(getMode())
.setIncludeClassesChecksum(getIncludeClassesChecksum())
.setDexClassChecksumFilter(getDexClassChecksumFilter())
- .setProgramConsumer(getProgramConsumer());
- for (ArtProfileForRewriting artProfileForRewriting : getArtProfilesForRewriting()) {
- d8Builder.addArtProfileForRewriting(
- artProfileForRewriting.getArtProfileProvider(),
- artProfileForRewriting.getResidualArtProfileConsumer());
- }
+ .setProgramConsumer(getProgramConsumer())
+ .setEnableRewritingOfArtProfilesIsNopCheck();
for (ClassFileResourceProvider libraryResourceProvider :
inputs.getLibraryResourceProviders()) {
d8Builder.addLibraryResourceProvider(libraryResourceProvider);
diff --git a/src/main/java/com/android/tools/r8/profile/art/ArtProfileCollection.java b/src/main/java/com/android/tools/r8/profile/art/ArtProfileCollection.java
index 2b2f387..e132aed 100644
--- a/src/main/java/com/android/tools/r8/profile/art/ArtProfileCollection.java
+++ b/src/main/java/com/android/tools/r8/profile/art/ArtProfileCollection.java
@@ -36,6 +36,9 @@
if (artProfileOptions.isCompletenessCheckForTestingEnabled()) {
artProfiles.add(createCompleteArtProfile(appInfo));
}
+ if (artProfileOptions.isNopCheckForTestingEnabled()) {
+ assert artProfileOptions.setNopCheckForTestingHashCode(appInfo);
+ }
if (artProfiles.isEmpty()) {
return empty();
}
diff --git a/src/main/java/com/android/tools/r8/profile/art/ArtProfileOptions.java b/src/main/java/com/android/tools/r8/profile/art/ArtProfileOptions.java
index 4d60860..9e78975 100644
--- a/src/main/java/com/android/tools/r8/profile/art/ArtProfileOptions.java
+++ b/src/main/java/com/android/tools/r8/profile/art/ArtProfileOptions.java
@@ -6,8 +6,14 @@
import static com.android.tools.r8.utils.SystemPropertyUtils.parseSystemPropertyOrDefault;
+import com.android.tools.r8.graph.AppInfo;
+import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.ListUtils;
+import com.google.common.hash.Hasher;
+import com.google.common.hash.Hashing;
+import java.nio.charset.StandardCharsets;
import java.util.Collection;
import java.util.Collections;
@@ -19,11 +25,14 @@
private Collection<ArtProfileForRewriting> artProfilesForRewriting = Collections.emptyList();
private boolean enableCompletenessCheckForTesting =
parseSystemPropertyOrDefault(COMPLETENESS_PROPERTY_KEY, false);
+ private boolean enableNopCheckForTesting;
private boolean hasReadArtProfileProviders = false;
private boolean allowReadingEmptyArtProfileProvidersMultipleTimesForTesting = false;
private final InternalOptions options;
+ private String nopCheckForTestingHashCode;
+
public ArtProfileOptions(InternalOptions options) {
this.options = options;
}
@@ -51,6 +60,10 @@
&& !options.getStartupInstrumentationOptions().isStartupInstrumentationEnabled();
}
+ public boolean isNopCheckForTestingEnabled() {
+ return enableNopCheckForTesting;
+ }
+
public boolean isIncludingApiReferenceStubs() {
// We only include API reference stubs in the residual ART profiles for completeness testing.
// This is because the API reference stubs should never be used at runtime except for
@@ -105,4 +118,35 @@
this.enableCompletenessCheckForTesting = enableCompletenessCheckForTesting;
return this;
}
+
+ public ArtProfileOptions setEnableNopCheckForTesting() {
+ this.enableNopCheckForTesting = true;
+ return this;
+ }
+
+ public boolean setNopCheckForTestingHashCode(AppInfo appInfo) {
+ if (nopCheckForTestingHashCode != null) {
+ String hashCode = computeNopCheckForTestingHashCode(appInfo);
+ assert hashCode.equals(nopCheckForTestingHashCode);
+ } else {
+ nopCheckForTestingHashCode = computeNopCheckForTestingHashCode(appInfo);
+ }
+ return true;
+ }
+
+ private static String computeNopCheckForTestingHashCode(AppInfo appInfo) {
+ Hasher hasher = Hashing.sha256().newHasher();
+ for (DexProgramClass clazz : appInfo.classesWithDeterministicOrder()) {
+ hasher.putString(clazz.getType().toDescriptorString(), StandardCharsets.UTF_8);
+ for (DexEncodedMethod method : clazz.methods()) {
+ hasher.putString(method.getReference().toSmaliString(), StandardCharsets.UTF_8);
+ }
+ }
+ return hasher.hash().toString();
+ }
+
+ public boolean verifyHasNopCheckForTestingHashCode() {
+ assert nopCheckForTestingHashCode != null;
+ return true;
+ }
}
diff --git a/src/main/java/com/android/tools/r8/profile/art/NonEmptyArtProfileCollection.java b/src/main/java/com/android/tools/r8/profile/art/NonEmptyArtProfileCollection.java
index bcf3221..a4b8450 100644
--- a/src/main/java/com/android/tools/r8/profile/art/NonEmptyArtProfileCollection.java
+++ b/src/main/java/com/android/tools/r8/profile/art/NonEmptyArtProfileCollection.java
@@ -69,7 +69,12 @@
@Override
public void supplyConsumers(AppView<?> appView) {
- if (appView.options().getArtProfileOptions().isCompletenessCheckForTestingEnabled()) {
+ ArtProfileOptions artProfileOptions = appView.options().getArtProfileOptions();
+ if (artProfileOptions.isNopCheckForTestingEnabled()) {
+ assert artProfileOptions.verifyHasNopCheckForTestingHashCode();
+ assert artProfileOptions.setNopCheckForTestingHashCode(appView.appInfo());
+ }
+ if (artProfileOptions.isCompletenessCheckForTestingEnabled()) {
assert ArtProfileCompletenessChecker.verify(appView);
ListUtils.removeLast(artProfiles);
if (artProfiles.isEmpty()) {
diff --git a/src/test/java/com/android/tools/r8/CommandTestBase.java b/src/test/java/com/android/tools/r8/CommandTestBase.java
index 886aeba..7a71837 100644
--- a/src/test/java/com/android/tools/r8/CommandTestBase.java
+++ b/src/test/java/com/android/tools/r8/CommandTestBase.java
@@ -26,6 +26,7 @@
import com.google.common.collect.ImmutableList;
import java.io.IOException;
import java.nio.file.Path;
+import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import org.junit.Test;
@@ -321,9 +322,17 @@
FileUtils.writeTextFile(profile, profileRuleFlags + profileRuleDescriptor);
// Pass the profile on the command line.
+ List<String> args =
+ new ArrayList<>(
+ ImmutableList.of("--art-profile", profile.toString(), residualProfile.toString()));
+ // L8 only accepts an art profile when shrinking.
+ if (this instanceof L8CommandTest) {
+ Path path = temp.newFile("proguard-rules.pro").toPath();
+ FileUtils.writeTextFile(path, "-dontshrink");
+ args.addAll(ImmutableList.of("--pg-conf", path.toString()));
+ }
List<ArtProfileForRewriting> artProfilesForRewriting =
- parseWithRequiredArgs("--art-profile", profile.toString(), residualProfile.toString())
- .getArtProfilesForRewriting();
+ parseWithRequiredArgs(args.toArray(new String[0])).getArtProfilesForRewriting();
assertEquals(1, artProfilesForRewriting.size());
// Extract inputs.
diff --git a/src/test/java/com/android/tools/r8/profile/art/DesugaredLibraryArtProfileRewritingTest.java b/src/test/java/com/android/tools/r8/profile/art/DesugaredLibraryArtProfileRewritingTest.java
index b812f2d..811cb53 100644
--- a/src/test/java/com/android/tools/r8/profile/art/DesugaredLibraryArtProfileRewritingTest.java
+++ b/src/test/java/com/android/tools/r8/profile/art/DesugaredLibraryArtProfileRewritingTest.java
@@ -9,9 +9,13 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentAndNotRenamed;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentAndRenamed;
+import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.StringContains.containsString;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import com.android.tools.r8.CompilationFailedException;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.desugar.desugaredlibrary.DesugaredLibraryTestBase;
import com.android.tools.r8.desugar.desugaredlibrary.test.CompilationSpecification;
@@ -21,6 +25,8 @@
import com.android.tools.r8.profile.art.utils.ArtProfileInspector;
import com.android.tools.r8.references.MethodReference;
import com.android.tools.r8.references.Reference;
+import com.android.tools.r8.utils.AbortException;
+import com.android.tools.r8.utils.codeinspector.AssertUtils;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
@@ -58,14 +64,28 @@
@Test
public void test() throws Throwable {
Assume.assumeTrue(libraryDesugaringSpecification.hasEmulatedInterfaceDesugaring(parameters));
- testForDesugaredLibrary(parameters, libraryDesugaringSpecification, compilationSpecification)
- .addInnerClasses(getClass())
- .addKeepMainRule(Main.class)
- .addL8ArtProfileForRewriting(getArtProfile())
- .compile()
- .inspectL8ResidualArtProfile(this::inspect)
- .run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines("0");
+ AssertUtils.assertFailsCompilationIf(
+ !compilationSpecification.isL8Shrink(),
+ () ->
+ testForDesugaredLibrary(
+ parameters, libraryDesugaringSpecification, compilationSpecification)
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .addL8ArtProfileForRewriting(getArtProfile())
+ .compile()
+ .inspectL8ResidualArtProfile(this::inspect)
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("0"),
+ exception -> {
+ assertEquals(CompilationFailedException.class, exception.getClass());
+ assertThat(exception.getMessage(), equalTo("Compilation failed to complete"));
+ assertNotNull(exception.getCause());
+ assertEquals(AbortException.class, exception.getCause().getClass());
+ assertThat(
+ exception.getCause().getMessage(),
+ containsString(
+ "L8 does not impact ART profiles when generating DEX and not shrinking"));
+ });
}
private ExternalArtProfile getArtProfile() {
