Version 1.6.50 Cherry pick: Always initialize and write checksum encodings when requested. CL: https://r8-review.googlesource.com/c/r8/+/45195 Bug: 143949125 Cherry pick: Add information on checksums present in marker CL: https://r8-review.googlesource.com/c/r8/+/45114 Change-Id: I0704fa0f2dc6f789242cb1f69d558a28412c23db
diff --git a/src/main/java/com/android/tools/r8/ExtractMarker.java b/src/main/java/com/android/tools/r8/ExtractMarker.java index c9f2032..bda3597 100644 --- a/src/main/java/com/android/tools/r8/ExtractMarker.java +++ b/src/main/java/com/android/tools/r8/ExtractMarker.java
@@ -101,7 +101,7 @@ options.minApiLevel = AndroidApiLevel.P.getLevel(); DexApplication dexApp = new ApplicationReader(app, options, new Timing("ExtractMarker")).read(); - return dexApp.dexItemFactory.extractMarker(); + return dexApp.dexItemFactory.extractMarkers(); } public static void main(String[] args)
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java index db6daa1..ad2811c 100644 --- a/src/main/java/com/android/tools/r8/Version.java +++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@ // This field is accessed from release scripts using simple pattern matching. // Therefore, changing this field could break our release scripts. - public static final String LABEL = "1.6.49"; + public static final String LABEL = "1.6.50"; private Version() { }
diff --git a/src/main/java/com/android/tools/r8/dex/ApplicationWriter.java b/src/main/java/com/android/tools/r8/dex/ApplicationWriter.java index 6cdea02..c62df61 100644 --- a/src/main/java/com/android/tools/r8/dex/ApplicationWriter.java +++ b/src/main/java/com/android/tools/r8/dex/ApplicationWriter.java
@@ -244,7 +244,7 @@ application.dexItemFactory.sort(namingLens); assert markers == null || markers.isEmpty() - || application.dexItemFactory.extractMarker() != null; + || application.dexItemFactory.extractMarkers() != null; SortAnnotations sortAnnotations = new SortAnnotations(); application.classes().forEach((clazz) -> clazz.addDependencies(sortAnnotations));
diff --git a/src/main/java/com/android/tools/r8/dex/ClassesChecksum.java b/src/main/java/com/android/tools/r8/dex/ClassesChecksum.java index eaff74e..ffdb9a4 100644 --- a/src/main/java/com/android/tools/r8/dex/ClassesChecksum.java +++ b/src/main/java/com/android/tools/r8/dex/ClassesChecksum.java
@@ -18,7 +18,7 @@ private static final char PREFIX_CHAR1 = '~'; private static final char PREFIX_CHAR2 = '~'; - private Object2LongMap<String> dictionary = null; + private final Object2LongMap<String> dictionary = new Object2LongOpenHashMap<>(); public ClassesChecksum() { assert PREFIX.length() == 3; @@ -27,14 +27,7 @@ assert PREFIX.charAt(2) == PREFIX_CHAR2; } - private void ensureMap() { - if (dictionary == null) { - dictionary = new Object2LongOpenHashMap<>(); - } - } - private void append(JsonObject json) { - ensureMap(); json.entrySet() .forEach( entry -> @@ -42,7 +35,6 @@ } public void addChecksum(String classDescriptor, long crc) { - ensureMap(); dictionary.put(classDescriptor, crc); }
diff --git a/src/main/java/com/android/tools/r8/dex/Marker.java b/src/main/java/com/android/tools/r8/dex/Marker.java index 85d3568..7ab2a56 100644 --- a/src/main/java/com/android/tools/r8/dex/Marker.java +++ b/src/main/java/com/android/tools/r8/dex/Marker.java
@@ -21,6 +21,7 @@ public static final String MIN_API = "min-api"; public static final String SHA1 = "sha-1"; public static final String COMPILATION_MODE = "compilation-mode"; + public static final String HAS_CHECKSUMS = "has-checksums"; public static final String PG_MAP_ID = "pg-map-id"; public enum Tool { @@ -107,6 +108,16 @@ return this; } + public boolean getHasChecksums() { + return jsonObject.get(HAS_CHECKSUMS).getAsBoolean(); + } + + public Marker setHasChecksums(boolean hasChecksums) { + assert !jsonObject.has(HAS_CHECKSUMS); + jsonObject.addProperty(HAS_CHECKSUMS, hasChecksums); + return this; + } + public String getPgMapId() { return jsonObject.get(PG_MAP_ID).getAsString(); }
diff --git a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java index b9df2a1..c99cf51 100644 --- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java +++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -1155,19 +1155,6 @@ } // Debugging support to extract marking string. - public synchronized Collection<Marker> extractMarker() { - // This is slow but it is not needed for any production code yet. - List<Marker> markers = new ArrayList<>(); - for (DexString dexString : strings.keySet()) { - Marker result = Marker.parse(dexString); - if (result != null) { - markers.add(result); - } - } - return markers; - } - - // Debugging support to extract marking string. // Find all markers. public synchronized List<Marker> extractMarkers() { // This is slow but it is not needed for any production code yet.
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 218baef..0098813 100644 --- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java +++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -282,7 +282,8 @@ Marker marker = new Marker(tool) .setVersion(Version.LABEL) - .setCompilationMode(debug ? CompilationMode.DEBUG : CompilationMode.RELEASE); + .setCompilationMode(debug ? CompilationMode.DEBUG : CompilationMode.RELEASE) + .setHasChecksums(encodeChecksums); if (!isGeneratingClassFiles()) { marker.setMinApi(minApiLevel); }
diff --git a/src/test/java/com/android/tools/r8/ExtractMarkerTest.java b/src/test/java/com/android/tools/r8/ExtractMarkerTest.java index 948ab96..008ff1f 100644 --- a/src/test/java/com/android/tools/r8/ExtractMarkerTest.java +++ b/src/test/java/com/android/tools/r8/ExtractMarkerTest.java
@@ -4,32 +4,57 @@ package com.android.tools.r8; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import com.android.tools.r8.dex.Marker; import com.android.tools.r8.dex.Marker.Tool; +import com.android.tools.r8.utils.BooleanUtils; import java.nio.file.Paths; import java.util.Collection; import java.util.Set; +import org.junit.Assume; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; -public class ExtractMarkerTest { +@RunWith(Parameterized.class) +public class ExtractMarkerTest extends TestBase { private static final String CLASS_FILE = ToolHelper.EXAMPLES_BUILD_DIR + "classes/trivial/Trivial.class"; - private static void verifyMarker(Marker marker, Tool tool) { + private final TestParameters parameters; + private boolean includeClassesChecksum; + + @Parameterized.Parameters(name = "{0}, includeClassesChecksum: {1}") + public static Collection<Object[]> data() { + return buildParameters( + getTestParameters().withAllRuntimes().withAllApiLevels().build(), BooleanUtils.values()); + } + + public ExtractMarkerTest(TestParameters parameters, boolean includeClassesChecksum) { + this.parameters = parameters; + this.includeClassesChecksum = includeClassesChecksum; + } + + private void verifyMarkerDex(Marker marker, Tool tool) { assertEquals(tool, marker.getTool()); assertEquals(Version.LABEL, marker.getVersion()); assertEquals(CompilationMode.DEBUG.toString().toLowerCase(), marker.getCompilationMode()); + assertEquals(parameters.getApiLevel().getLevel(), marker.getMinApi().intValue()); + assertEquals(includeClassesChecksum, marker.getHasChecksums()); } @Test public void extractMarkerTestDex() throws CompilationFailedException { - boolean[] testExecuted = {false}; + Assume.assumeTrue(parameters.getRuntime().isDex()); + boolean[] testExecuted = {false}; D8.run( D8Command.builder() .addProgramFiles(Paths.get(CLASS_FILE)) + .setMinApiLevel(parameters.getApiLevel().getLevel()) + .setIncludeClassesChecksum(includeClassesChecksum) .setProgramConsumer( new DexIndexedConsumer.ForwardingConsumer(null) { @Override @@ -47,7 +72,7 @@ } catch (Exception e) { throw new RuntimeException(e); } - verifyMarker(marker, Tool.D8); + verifyMarkerDex(marker, Tool.D8); testExecuted[0] = true; } }) @@ -55,8 +80,18 @@ assertTrue(testExecuted[0]); } + private static void verifyMarkerCf(Marker marker, Tool tool) { + assertEquals(tool, marker.getTool()); + assertEquals(Version.LABEL, marker.getVersion()); + assertEquals(CompilationMode.DEBUG.toString().toLowerCase(), marker.getCompilationMode()); + assertFalse(marker.getHasChecksums()); + } + @Test public void extractMarkerTestCf() throws CompilationFailedException { + Assume.assumeTrue(parameters.getRuntime().isCf()); + Assume.assumeFalse(includeClassesChecksum); + boolean[] testExecuted = {false}; R8.run( R8Command.builder() @@ -78,7 +113,7 @@ } catch (Exception e) { throw new RuntimeException(e); } - verifyMarker(marker, Tool.R8); + verifyMarkerCf(marker, Tool.R8); testExecuted[0] = true; } })
diff --git a/src/test/java/com/android/tools/r8/d8/D8FrameworkDexPassthroughMarkerTest.java b/src/test/java/com/android/tools/r8/d8/D8FrameworkDexPassthroughMarkerTest.java index 93c0323..866dcf5 100644 --- a/src/test/java/com/android/tools/r8/d8/D8FrameworkDexPassthroughMarkerTest.java +++ b/src/test/java/com/android/tools/r8/d8/D8FrameworkDexPassthroughMarkerTest.java
@@ -71,7 +71,7 @@ new ApplicationReader( app, options, new Timing("D8FrameworkDexPassthroughMarkerTest")) .read(); - Collection<Marker> markers = dexApp.dexItemFactory.extractMarker(); + Collection<Marker> markers = dexApp.dexItemFactory.extractMarkers(); assertEquals(1, markers.size()); Marker readMarker = markers.iterator().next(); assertEquals(marker, readMarker);
diff --git a/src/test/java/com/android/tools/r8/dexfilemerger/DexMergeChecksumsFileWithNoClassesTest.java b/src/test/java/com/android/tools/r8/dexfilemerger/DexMergeChecksumsFileWithNoClassesTest.java new file mode 100644 index 0000000..57fe4b3 --- /dev/null +++ b/src/test/java/com/android/tools/r8/dexfilemerger/DexMergeChecksumsFileWithNoClassesTest.java
@@ -0,0 +1,77 @@ +// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +package com.android.tools.r8.dexfilemerger; + +import static org.junit.Assert.assertTrue; + +import com.android.tools.r8.CompilationMode; +import com.android.tools.r8.TestBase; +import com.android.tools.r8.TestParameters; +import com.android.tools.r8.TestParametersCollection; +import com.android.tools.r8.utils.codeinspector.CodeInspector; +import java.nio.file.Path; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +@RunWith(Parameterized.class) +public class DexMergeChecksumsFileWithNoClassesTest extends TestBase { + + private final TestParameters parameters; + + @Parameterized.Parameters(name = "{0}") + public static TestParametersCollection data() { + return getTestParameters().withDexRuntimes().withAllApiLevels().build(); + } + + public DexMergeChecksumsFileWithNoClassesTest(TestParameters parameters) { + this.parameters = parameters; + } + + @Test + public void testChecksumWithNoClasses() throws Exception { + Path out1 = + testForD8() + .setMinApi(parameters.getApiLevel()) + .setMode(CompilationMode.DEBUG) + .setIncludeClassesChecksum(true) + .setIntermediate(true) + .compile() + .inspect(this::checkContainsChecksums) + .writeToZip(); + + Path out2 = + testForD8() + .setMinApi(parameters.getApiLevel()) + .setMode(CompilationMode.DEBUG) + .setIncludeClassesChecksum(true) + .addProgramClasses(TestClass.class) + .setIntermediate(true) + .compile() + .inspect(this::checkContainsChecksums) + .writeToZip(); + + testForD8() + .addProgramFiles(out1, out2) + .setIncludeClassesChecksum(true) + .setMinApi(parameters.getApiLevel()) + .run(parameters.getRuntime(), TestClass.class) + .inspect(this::checkContainsChecksums); + } + + private void checkContainsChecksums(CodeInspector inspector) { + inspector.getMarkers().forEach(m -> assertTrue(m.getHasChecksums())); + // It may be prudent to check that the dex file also has the encoding string, but that is + // not easily accessed. + inspector.allClasses().forEach(c -> c.getDexClass().asProgramClass().getChecksum()); + } + + public static class TestClass { + + public static void main(String[] args) { + System.out.println("Hello world!"); + } + } +}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java b/src/test/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java index 2a91e4b..2bc1dd4 100644 --- a/src/test/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java +++ b/src/test/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java
@@ -10,6 +10,7 @@ import com.android.tools.r8.cf.code.CfTryCatch; import com.android.tools.r8.code.Instruction; import com.android.tools.r8.dex.ApplicationReader; +import com.android.tools.r8.dex.Marker; import com.android.tools.r8.errors.Unimplemented; import com.android.tools.r8.graph.CfCode; import com.android.tools.r8.graph.Code; @@ -48,6 +49,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; @@ -391,6 +393,10 @@ } } + public Collection<Marker> getMarkers() { + return dexItemFactory.extractMarkers(); + } + // Build the generic signature using the current mapping if any. class GenericSignatureGenerator implements GenericSignatureAction<String> {