Add tests of DEX with identifiers with white space characters
Bug: b/269089718
Change-Id: If8a9e928c0caea3e7f718cd535f1b58521829607
diff --git a/src/main/java/com/android/tools/r8/dex/FileWriter.java b/src/main/java/com/android/tools/r8/dex/FileWriter.java
index 6d43e68..5d8832f 100644
--- a/src/main/java/com/android/tools/r8/dex/FileWriter.java
+++ b/src/main/java/com/android/tools/r8/dex/FileWriter.java
@@ -795,6 +795,12 @@
private byte[] dexVersionBytes() {
if (options.testing.dexContainerExperiment) {
+ return DexVersion.V41.getBytes();
+ }
+ // TODO(b/269089718): Remove this testing option and always emit DEX version 040 if DEX contains
+ // identifiers with whitespace.
+ if (options.testing.dexVersion40FromApiLevel30
+ && options.getMinApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.R)) {
return DexVersion.V40.getBytes();
}
return options.testing.forceDexVersionBytes != null
diff --git a/src/main/java/com/android/tools/r8/utils/AndroidApiLevel.java b/src/main/java/com/android/tools/r8/utils/AndroidApiLevel.java
index 5be322e..2e7e2e3 100644
--- a/src/main/java/com/android/tools/r8/utils/AndroidApiLevel.java
+++ b/src/main/java/com/android/tools/r8/utils/AndroidApiLevel.java
@@ -99,6 +99,8 @@
case V39:
return AndroidApiLevel.P;
case V40:
+ return AndroidApiLevel.R;
+ case V41:
return AndroidApiLevel.ANDROID_PLATFORM;
default:
throw new Unreachable();
diff --git a/src/main/java/com/android/tools/r8/utils/DexVersion.java b/src/main/java/com/android/tools/r8/utils/DexVersion.java
index d094cf7..e1b4588 100644
--- a/src/main/java/com/android/tools/r8/utils/DexVersion.java
+++ b/src/main/java/com/android/tools/r8/utils/DexVersion.java
@@ -13,7 +13,8 @@
V37(37, new byte[] {'0', '3', '7'}),
V38(38, new byte[] {'0', '3', '8'}),
V39(39, new byte[] {'0', '3', '9'}),
- V40(40, new byte[] {'0', '4', '0'});
+ V40(40, new byte[] {'0', '4', '0'}),
+ V41(41, new byte[] {'0', '4', '1'});
private final int dexVersion;
@@ -47,6 +48,8 @@
case Sv2:
case S:
case R:
+ // Dex version should have been V40 starting from API level 30, see b/269089718.
+ // return DexVersion.V40;
case Q:
case P:
return DexVersion.V39;
@@ -97,6 +100,8 @@
return Optional.of(V39);
case 40:
return Optional.of(V40);
+ case 41:
+ return Optional.of(V41);
default:
return Optional.empty();
}
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 e7dfeee..ce1acd6 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -154,7 +154,7 @@
public static final int SUPPORTED_DEX_VERSION =
AndroidApiLevel.LATEST.getDexVersion().getIntValue();
- public static final int EXPERIMENTAL_DEX_VERSION = DexVersion.V40.getIntValue();
+ public static final int EXPERIMENTAL_DEX_VERSION = DexVersion.V41.getIntValue();
public static final int ASM_VERSION = Opcodes.ASM9;
@@ -1978,6 +1978,8 @@
private boolean hasReadCheckDeterminism = false;
private DeterminismChecker determinismChecker = null;
public boolean usePcEncodingInCfForTesting = false;
+ public boolean dexVersion40FromApiLevel30 =
+ System.getProperty("com.android.tools.r8.dexVersion40ForApiLevel30") != null;
public boolean dexContainerExperiment =
System.getProperty("com.android.tools.r8.dexContainerExperiment") != null;
diff --git a/src/test/java/com/android/tools/r8/dex/container/DexContainerFormatBasicTest.java b/src/test/java/com/android/tools/r8/dex/container/DexContainerFormatBasicTest.java
index d1f909d..72f856f 100644
--- a/src/test/java/com/android/tools/r8/dex/container/DexContainerFormatBasicTest.java
+++ b/src/test/java/com/android/tools/r8/dex/container/DexContainerFormatBasicTest.java
@@ -6,12 +6,14 @@
import static com.android.tools.r8.dex.Constants.CHECKSUM_OFFSET;
import static com.android.tools.r8.dex.Constants.DATA_OFF_OFFSET;
import static com.android.tools.r8.dex.Constants.DATA_SIZE_OFFSET;
+import static com.android.tools.r8.dex.Constants.DEX_MAGIC_SIZE;
import static com.android.tools.r8.dex.Constants.FILE_SIZE_OFFSET;
import static com.android.tools.r8.dex.Constants.MAP_OFF_OFFSET;
import static com.android.tools.r8.dex.Constants.SIGNATURE_OFFSET;
import static com.android.tools.r8.dex.Constants.STRING_IDS_OFF_OFFSET;
import static com.android.tools.r8.dex.Constants.STRING_IDS_SIZE_OFFSET;
import static com.android.tools.r8.dex.Constants.TYPE_STRING_ID_ITEM;
+import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
@@ -27,6 +29,7 @@
import com.android.tools.r8.transformers.ClassTransformer;
import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.DescriptorUtils;
+import com.android.tools.r8.utils.DexVersion;
import com.android.tools.r8.utils.ZipUtils;
import com.google.common.collect.ImmutableList;
import com.google.common.io.ByteStreams;
@@ -80,7 +83,7 @@
.setMinApi(AndroidApiLevel.L)
.compile()
.writeToZip();
- validateDex(outputA, 2);
+ validateDex(outputA, 2, AndroidApiLevel.L.getDexVersion());
Path outputB =
testForD8(Backend.DEX)
@@ -88,7 +91,7 @@
.setMinApi(AndroidApiLevel.L)
.compile()
.writeToZip();
- validateDex(outputB, 2);
+ validateDex(outputB, 2, AndroidApiLevel.L.getDexVersion());
Path outputMerged =
testForD8(Backend.DEX)
@@ -96,7 +99,7 @@
.setMinApi(AndroidApiLevel.L)
.compile()
.writeToZip();
- validateDex(outputMerged, 4);
+ validateDex(outputMerged, 4, AndroidApiLevel.L.getDexVersion());
}
@Test
@@ -135,21 +138,22 @@
validateSingleContainerDex(outputB);
}
- private void validateDex(Path output, int expectedDexes) throws Exception {
+ private void validateDex(Path output, int expectedDexes, DexVersion expectedVersion)
+ throws Exception {
List<byte[]> dexes = unzipContent(output);
assertEquals(expectedDexes, dexes.size());
for (byte[] dex : dexes) {
- validate(dex);
+ validate(dex, expectedVersion);
}
}
private void validateSingleContainerDex(Path output) throws Exception {
List<byte[]> dexes = unzipContent(output);
assertEquals(1, dexes.size());
- validate(dexes.get(0));
+ validate(dexes.get(0), DexVersion.V41);
}
- private void validate(byte[] dex) throws Exception {
+ private void validate(byte[] dex, DexVersion expectedVersion) throws Exception {
CompatByteBuffer buffer = CompatByteBuffer.wrap(dex);
setByteOrder(buffer);
@@ -163,21 +167,47 @@
}
assertEquals(buffer.capacity(), offset);
- int lastOffset = sections.getInt(sections.size() - 1);
- int stringIdsSize = buffer.getInt(lastOffset + STRING_IDS_SIZE_OFFSET);
- int stringIdsOffset = buffer.getInt(lastOffset + STRING_IDS_OFF_OFFSET);
-
for (Integer sectionOffset : sections) {
- assertEquals(stringIdsSize, buffer.getInt(sectionOffset + STRING_IDS_SIZE_OFFSET));
- assertEquals(stringIdsOffset, buffer.getInt(sectionOffset + STRING_IDS_OFF_OFFSET));
- assertEquals(stringIdsSize, getSizeFromMap(TYPE_STRING_ID_ITEM, buffer, sectionOffset));
- assertEquals(stringIdsOffset, getOffsetFromMap(TYPE_STRING_ID_ITEM, buffer, sectionOffset));
+ validateHeader(sections, buffer, sectionOffset, expectedVersion);
validateMap(buffer, sectionOffset);
validateSignature(buffer, sectionOffset);
validateChecksum(buffer, sectionOffset);
}
}
+ private byte[] magicBytes(DexVersion version) {
+ byte[] magic = new byte[DEX_MAGIC_SIZE];
+ System.arraycopy(
+ Constants.DEX_FILE_MAGIC_PREFIX, 0, magic, 0, Constants.DEX_FILE_MAGIC_PREFIX.length);
+ System.arraycopy(
+ version.getBytes(),
+ 0,
+ magic,
+ Constants.DEX_FILE_MAGIC_PREFIX.length,
+ version.getBytes().length);
+ magic[Constants.DEX_FILE_MAGIC_PREFIX.length + version.getBytes().length] =
+ Constants.DEX_FILE_MAGIC_SUFFIX;
+ assertEquals(
+ DEX_MAGIC_SIZE, Constants.DEX_FILE_MAGIC_PREFIX.length + version.getBytes().length + 1);
+ return magic;
+ }
+
+ private void validateHeader(
+ IntList sections, CompatByteBuffer buffer, int offset, DexVersion expectedVersion) {
+ int lastOffset = sections.getInt(sections.size() - 1);
+ int stringIdsSize = buffer.getInt(lastOffset + STRING_IDS_SIZE_OFFSET);
+ int stringIdsOffset = buffer.getInt(lastOffset + STRING_IDS_OFF_OFFSET);
+
+ byte[] magic = new byte[DEX_MAGIC_SIZE];
+ buffer.get(magic);
+ assertArrayEquals(magicBytes(expectedVersion), magic);
+
+ assertEquals(stringIdsSize, buffer.getInt(offset + STRING_IDS_SIZE_OFFSET));
+ assertEquals(stringIdsOffset, buffer.getInt(offset + STRING_IDS_OFF_OFFSET));
+ assertEquals(stringIdsSize, getSizeFromMap(TYPE_STRING_ID_ITEM, buffer, offset));
+ assertEquals(stringIdsOffset, getOffsetFromMap(TYPE_STRING_ID_ITEM, buffer, offset));
+ }
+
private void validateMap(CompatByteBuffer buffer, int offset) {
int mapOffset = buffer.getInt(offset + MAP_OFF_OFFSET);
buffer.position(mapOffset);
diff --git a/src/test/java/com/android/tools/r8/dex/whitespaceinidentifiers/WhiteSpaceInIdentifiersTest.java b/src/test/java/com/android/tools/r8/dex/whitespaceinidentifiers/WhiteSpaceInIdentifiersTest.java
new file mode 100644
index 0000000..9092ea4
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/dex/whitespaceinidentifiers/WhiteSpaceInIdentifiersTest.java
@@ -0,0 +1,263 @@
+// Copyright (c) 2023, 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.dex.whitespaceinidentifiers;
+
+import static com.android.tools.r8.DiagnosticsMatcher.diagnosticException;
+import static com.android.tools.r8.DiagnosticsMatcher.diagnosticMessage;
+import static org.hamcrest.CoreMatchers.allOf;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.junit.Assume.assumeTrue;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestCompilerBuilder;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.naming.ProguardMapReader.ParseException;
+import com.android.tools.r8.transformers.ClassFileTransformer.MethodPredicate;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.DexVersion;
+import com.android.tools.r8.utils.StringUtils;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class WhiteSpaceInIdentifiersTest extends TestBase {
+
+ @Parameter() public TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static List<Object[]> data() {
+ return buildParameters(getTestParameters().withDexRuntimesAndAllApiLevels().build());
+ }
+
+ private static final String EXPECTED_OUTPUT =
+ StringUtils.lines(
+ "0x20", "0xa0", "0x1680", "0x2000", "0x2001", "0x2002", "0x2003", "0x2004", "0x2005",
+ "0x2006", "0x2007", "0x2008", "0x2009", "0x200a", "0x202f", "0x205f", "0x3000");
+
+ public void test(TestCompilerBuilder<?, ?, ?, ?, ?> testBuilder) throws Exception {
+ testBuilder
+ .addProgramClassFileData(getTransformed())
+ .setMinApi(parameters.getApiLevel())
+ .applyIf(
+ parameters.getApiLevel().isLessThan(AndroidApiLevel.R),
+ b -> {
+ try {
+ b.compileWithExpectedDiagnostics(
+ diagnostics ->
+ diagnostics.assertErrorsMatch(
+ diagnosticMessage(
+ containsString("are not allowed prior to DEX version 040"))));
+ fail("Unexpected success");
+ } catch (CompilationFailedException e) {
+ // Expected.
+ }
+ },
+ b ->
+ b.run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(EXPECTED_OUTPUT));
+ }
+
+ @Test
+ public void testD8() throws Exception {
+ test(testForD8(parameters.getBackend()));
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ try {
+ test(testForR8(parameters.getBackend()).addKeepMainRule(TestClass.class));
+ } catch (RuntimeException e) {
+ // TODO(b/141287396): Proguard maps with spaces in identifiers are not supported. Running
+ // on R8 always creates an inspector to find the name of the potentially renamed main class.
+ assertTrue(e.getCause() instanceof ExecutionException);
+ assertTrue(e.getCause().getCause() instanceof ParseException);
+ }
+ }
+
+ @Test
+ public void testD8MergeWithSpaces() throws Exception {
+ assumeTrue(parameters.getApiLevel().getDexVersion().isGreaterThanOrEqualTo(DexVersion.V39));
+
+ // Compile with API level 30 to allow white space in identifiers. This generates DEX
+ // version 039.
+ Path dex =
+ testForD8(parameters.getBackend())
+ .addProgramClassFileData(getTransformed())
+ .setMinApi(AndroidApiLevel.R)
+ .compile()
+ .writeToZip();
+
+ // Run merge step with DEX with white space in input (not forcing min API level of R).
+ testForD8(parameters.getBackend())
+ .addProgramFiles(dex)
+ .setMinApi(parameters.getApiLevel())
+ .applyIf(
+ parameters.getApiLevel().isLessThan(AndroidApiLevel.R),
+ b -> {
+ try {
+ // TODO(b/269089718): This should not be an AssertionError but a compilation error.
+ b.compileWithExpectedDiagnostics(
+ diagnostics ->
+ diagnostics.assertErrorsMatch(diagnosticException(AssertionError.class)));
+ fail("Unexpected success");
+ } catch (CompilationFailedException e) {
+ // Expected.
+ }
+ },
+ b ->
+ b.run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(EXPECTED_OUTPUT));
+ }
+
+ @Test
+ public void testD8RunWithSpaces() throws Exception {
+ assumeTrue(parameters.getApiLevel().getDexVersion().isGreaterThanOrEqualTo(DexVersion.V39));
+ testForD8(parameters.getBackend())
+ .addProgramClassFileData(getTransformed())
+ .setMinApi(AndroidApiLevel.R)
+ .run(parameters.getRuntime(), TestClass.class)
+ .applyIf(
+ parameters.getApiLevel().isLessThan(AndroidApiLevel.R),
+ b -> b.assertFailureWithErrorThatMatches(containsString("Failure to verify dex file")),
+ b -> b.assertSuccessWithOutput(EXPECTED_OUTPUT));
+ }
+
+ @Test
+ public void testD8RunWithSpacesUsingDexV40() throws Exception {
+ assumeTrue(parameters.getApiLevel().getDexVersion().isGreaterThanOrEqualTo(DexVersion.V39));
+ testForD8(parameters.getBackend())
+ .addOptionsModification(options -> options.testing.dexVersion40FromApiLevel30 = true)
+ .addProgramClassFileData(getTransformed())
+ .setMinApi(AndroidApiLevel.R)
+ .run(parameters.getRuntime(), TestClass.class)
+ .applyIf(
+ parameters.getApiLevel().isLessThan(AndroidApiLevel.R),
+ b ->
+ b.assertFailureWithErrorThatMatches(
+ allOf(containsString("Unrecognized version"), containsString("0 4 0"))),
+ b -> b.assertSuccessWithOutput(EXPECTED_OUTPUT));
+ }
+
+ private String rename(String name) {
+ return new String(Character.toChars(Integer.parseInt(name.substring(1), 16)));
+ }
+
+ private byte[] getTransformed() throws Exception {
+ return transformer(TestClass.class)
+ .renameMethod(MethodPredicate.onName(name -> name.startsWith("t")), this::rename)
+ .transformMethodInsnInMethod(
+ "main",
+ ((opcode, owner, name, descriptor, isInterface, continuation) -> {
+ continuation.visitMethodInsn(
+ opcode,
+ owner,
+ name.startsWith("t") ? rename(name) : name,
+ descriptor,
+ isInterface);
+ }))
+ .transform();
+ }
+
+ // Test with white space characters added in https://r8-review.git.corp.google.com/c/r8/+/42269.
+ // Supported on Art in https://android-review.git.corp.google.com/c/platform/art/+/1106719.
+ static class TestClass {
+
+ private static void t20() {
+ System.out.println("0x20");
+ }
+
+ private static void ta0() {
+ System.out.println("0xa0");
+ }
+
+ private static void t1680() {
+ System.out.println("0x1680");
+ }
+
+ private static void t2000() {
+ System.out.println("0x2000");
+ }
+
+ private static void t2001() {
+ System.out.println("0x2001");
+ }
+
+ private static void t2002() {
+ System.out.println("0x2002");
+ }
+
+ private static void t2003() {
+ System.out.println("0x2003");
+ }
+
+ private static void t2004() {
+ System.out.println("0x2004");
+ }
+
+ private static void t2005() {
+ System.out.println("0x2005");
+ }
+
+ private static void t2006() {
+ System.out.println("0x2006");
+ }
+
+ private static void t2007() {
+ System.out.println("0x2007");
+ }
+
+ private static void t2008() {
+ System.out.println("0x2008");
+ }
+
+ private static void t2009() {
+ System.out.println("0x2009");
+ }
+
+ private static void t200a() {
+ System.out.println("0x200a");
+ }
+
+ private static void t202f() {
+ System.out.println("0x202f");
+ }
+
+ private static void t205f() {
+ System.out.println("0x205f");
+ }
+
+ private static void t3000() {
+ System.out.println("0x3000");
+ }
+
+ public static void main(String[] args) {
+ t20();
+ ta0();
+ t1680();
+ t2000();
+ t2001();
+ t2002();
+ t2003();
+ t2004();
+ t2005();
+ t2006();
+ t2007();
+ t2008();
+ t2009();
+ t200a();
+ t202f();
+ t205f();
+ t3000();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/transformers/ClassFileTransformer.java b/src/test/java/com/android/tools/r8/transformers/ClassFileTransformer.java
index f9f8f13..0ee68d2 100644
--- a/src/test/java/com/android/tools/r8/transformers/ClassFileTransformer.java
+++ b/src/test/java/com/android/tools/r8/transformers/ClassFileTransformer.java
@@ -39,6 +39,7 @@
import java.util.Objects;
import java.util.function.Consumer;
import java.util.function.Function;
+import java.util.function.Predicate;
import java.util.stream.Collectors;
import org.objectweb.asm.AnnotationVisitor;
import org.objectweb.asm.ClassReader;
@@ -610,11 +611,15 @@
}
static MethodPredicate onName(String name) {
- return (access, otherName, descriptor, signature, exceptions) -> name.equals(otherName);
+ return onName(name::equals);
+ }
+
+ static MethodPredicate onName(Predicate<String> predicate) {
+ return (access, otherName, descriptor, signature, exceptions) -> predicate.test(otherName);
}
static MethodPredicate onNames(Collection<String> names) {
- return (access, otherName, descriptor, signature, exceptions) -> names.contains(otherName);
+ return onName(names::contains);
}
static MethodPredicate onNames(String... names) {
@@ -758,13 +763,19 @@
}
public ClassFileTransformer renameMethod(MethodPredicate predicate, String newName) {
+ return renameMethod(predicate, name -> newName);
+ }
+
+ public ClassFileTransformer renameMethod(
+ MethodPredicate predicate, Function<String, String> newName) {
return addClassTransformer(
new ClassTransformer() {
@Override
public MethodVisitor visitMethod(
int access, String name, String descriptor, String signature, String[] exceptions) {
if (predicate.test(access, name, descriptor, signature, exceptions)) {
- return super.visitMethod(access, newName, descriptor, signature, exceptions);
+ return super.visitMethod(
+ access, newName.apply(name), descriptor, signature, exceptions);
}
return super.visitMethod(access, name, descriptor, signature, exceptions);
}
