Fixes to writing the DEX conatiner format
* Sort the map by offset
* Calculate signature and checksum correctly
Before the entries in the map was written as for non-container DEX,
but as the string section can now be after the other sections (as it
is shared with a later section) the map needs to be sorted after the
location of the shared string table is known.
Changing the map also affects the signature and checksum, so these
has to be calculated after all offsets has been finalized.
Bug: b/249922554
Change-Id: I1c7e8ac8c735c03cc70d88ff6824f16af7f72625
diff --git a/src/main/java/com/android/tools/r8/dex/ApplicationWriterExperimental.java b/src/main/java/com/android/tools/r8/dex/ApplicationWriterExperimental.java
index 826b8dc..77f653a 100644
--- a/src/main/java/com/android/tools/r8/dex/ApplicationWriterExperimental.java
+++ b/src/main/java/com/android/tools/r8/dex/ApplicationWriterExperimental.java
@@ -11,6 +11,7 @@
import com.android.tools.r8.debuginfo.DebugRepresentation;
import com.android.tools.r8.dex.FileWriter.ByteBufferResult;
import com.android.tools.r8.dex.FileWriter.DexContainerSection;
+import com.android.tools.r8.dex.FileWriter.MapItem;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.ObjectToOffsetMapping;
@@ -176,31 +177,31 @@
DexOutputBuffer dexOutputBuffer, List<DexContainerSection> sections) {
// The last section has the shared string_ids table. Now it is written the final size and
// offset is known and the remaining sections can be updated to point to the shared table.
- // This updates both the size and offset in the header and in the map.
DexContainerSection lastSection = ListUtils.last(sections);
int stringIdsSize = lastSection.getFileWriter().getMixedSectionOffsets().getStringData().size();
int stringIdsOffset = lastSection.getLayout().stringIdsOffset;
for (DexContainerSection section : sections) {
if (section != lastSection) {
+ // Update the string_ids size and offset in the header.
dexOutputBuffer.moveTo(section.getLayout().headerOffset + Constants.STRING_IDS_SIZE_OFFSET);
dexOutputBuffer.putInt(stringIdsSize);
dexOutputBuffer.putInt(stringIdsOffset);
+ // Write the map. The map is sorted by offset, so write all entries after setting
+ // string_ids and sorting.
dexOutputBuffer.moveTo(section.getLayout().getMapOffset());
- // Skip size.
- dexOutputBuffer.getInt();
- while (dexOutputBuffer.position() < section.getLayout().getEndOfFile()) {
- int sectionType = dexOutputBuffer.getShort();
- dexOutputBuffer.getShort(); // Skip unused.
- if (sectionType == Constants.TYPE_STRING_ID_ITEM) {
- dexOutputBuffer.putInt(stringIdsSize);
- dexOutputBuffer.putInt(stringIdsOffset);
- break;
- } else {
- // Skip size and offset for this type.
- dexOutputBuffer.getInt();
- dexOutputBuffer.getInt();
- }
+ List<MapItem> mapItems =
+ section
+ .getLayout()
+ .generateMapInfo(section.getFileWriter(), stringIdsSize, stringIdsOffset);
+ int originalSize = dexOutputBuffer.getInt();
+ int size = 0;
+ for (MapItem mapItem : mapItems) {
+ size += mapItem.write(dexOutputBuffer);
}
+ assert originalSize == size;
+ // Calculate signature and checksum after the map is written.
+ section.getFileWriter().writeSignature(section.getLayout(), dexOutputBuffer);
+ section.getFileWriter().writeChecksum(section.getLayout(), dexOutputBuffer);
}
}
}
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 9ddc2d2..fc9c037 100644
--- a/src/main/java/com/android/tools/r8/dex/FileWriter.java
+++ b/src/main/java/com/android/tools/r8/dex/FileWriter.java
@@ -66,6 +66,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
+import java.util.Comparator;
import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.List;
@@ -292,8 +293,7 @@
this::writeAnnotationDirectory,
4);
- // Add the map at the end
- layout.setMapOffset(dest.align(4));
+ // Add the map at the end.
writeMap(layout);
layout.setEndOfFile(dest.position());
@@ -315,8 +315,10 @@
// Fill in the header information.
writeHeader(layout);
- writeSignature(layout);
- writeChecksum(layout);
+ if (includeStringData) {
+ writeSignature(layout);
+ writeChecksum(layout);
+ }
// Wrap backing buffer with actual length.
return new DexContainerSection(this, dest, layout);
@@ -777,70 +779,15 @@
}
}
- private int writeMapItem(int type, int offset, int length) {
- if (length == 0) {
- return 0;
- }
- if (Log.ENABLED) {
- Log.debug(getClass(), "Map entry 0x%04x @ 0x%08x # %08d.", type, offset, length);
- }
- dest.putShort((short) type);
- dest.putShort((short) 0);
- dest.putInt(length);
- dest.putInt(offset);
- return 1;
- }
-
- private void writeMap(Layout layout) {
+ public void writeMap(Layout layout) {
int startOfMap = dest.align(4);
+ layout.setMapOffset(startOfMap);
dest.forward(4); // Leave space for size;
+ List<MapItem> mapItems = layout.generateMapInfo(this);
int size = 0;
- size += writeMapItem(Constants.TYPE_HEADER_ITEM, 0, 1);
- size += writeMapItem(Constants.TYPE_STRING_ID_ITEM, layout.stringIdsOffset,
- mapping.getStrings().size());
- size += writeMapItem(Constants.TYPE_TYPE_ID_ITEM, layout.typeIdsOffset,
- mapping.getTypes().size());
- size += writeMapItem(Constants.TYPE_PROTO_ID_ITEM, layout.protoIdsOffset,
- mapping.getProtos().size());
- size += writeMapItem(Constants.TYPE_FIELD_ID_ITEM, layout.fieldIdsOffset,
- mapping.getFields().size());
- size += writeMapItem(Constants.TYPE_METHOD_ID_ITEM, layout.methodIdsOffset,
- mapping.getMethods().size());
- size += writeMapItem(Constants.TYPE_CLASS_DEF_ITEM, layout.classDefsOffset,
- mapping.getClasses().length);
- size += writeMapItem(Constants.TYPE_CALL_SITE_ID_ITEM, layout.callSiteIdsOffset,
- mapping.getCallSites().size());
- size += writeMapItem(Constants.TYPE_METHOD_HANDLE_ITEM, layout.methodHandleIdsOffset,
- mapping.getMethodHandles().size());
- size += writeMapItem(Constants.TYPE_CODE_ITEM, layout.getCodesOffset(),
- mixedSectionOffsets.getCodes().size());
- size += writeMapItem(Constants.TYPE_DEBUG_INFO_ITEM, layout.getDebugInfosOffset(),
- mixedSectionOffsets.getDebugInfos().size());
- size += writeMapItem(Constants.TYPE_TYPE_LIST, layout.getTypeListsOffset(),
- mixedSectionOffsets.getTypeLists().size());
- size +=
- writeMapItem(
- Constants.TYPE_STRING_DATA_ITEM,
- layout.getStringDataOffsets(),
- layout.getStringDataOffsets() == 0 ? 0 : mixedSectionOffsets.getStringData().size());
- size += writeMapItem(Constants.TYPE_ANNOTATION_ITEM, layout.getAnnotationsOffset(),
- mixedSectionOffsets.getAnnotations().size());
- size += writeMapItem(Constants.TYPE_CLASS_DATA_ITEM, layout.getClassDataOffset(),
- mixedSectionOffsets.getClassesWithData().size());
- size +=
- writeMapItem(
- Constants.TYPE_ENCODED_ARRAY_ITEM,
- layout.getEncodedArraysOffset(),
- mixedSectionOffsets.getEncodedArrays().size());
- size += writeMapItem(Constants.TYPE_ANNOTATION_SET_ITEM, layout.getAnnotationSetsOffset(),
- mixedSectionOffsets.getAnnotationSets().size());
- size += writeMapItem(Constants.TYPE_ANNOTATION_SET_REF_LIST,
- layout.getAnnotationSetRefListsOffset(),
- mixedSectionOffsets.getAnnotationSetRefLists().size());
- size += writeMapItem(Constants.TYPE_ANNOTATIONS_DIRECTORY_ITEM,
- layout.getAnnotationDirectoriesOffset(),
- mixedSectionOffsets.getAnnotationDirectories().size());
- size += writeMapItem(Constants.TYPE_MAP_LIST, layout.getMapOffset(), 1);
+ for (MapItem mapItem : mapItems) {
+ size += includeStringData ? mapItem.write(dest) : mapItem.size();
+ }
dest.moveTo(startOfMap);
dest.putInt(size);
dest.forward(size * Constants.TYPE_MAP_LIST_ITEM_SIZE);
@@ -892,26 +839,34 @@
}
private void writeSignature(Layout layout) {
+ writeSignature(layout, dest);
+ }
+
+ public void writeSignature(Layout layout, DexOutputBuffer dexOutputBuffer) {
try {
MessageDigest md = MessageDigest.getInstance("SHA-1");
md.update(
- dest.asArray(),
+ dexOutputBuffer.asArray(),
layout.headerOffset + Constants.FILE_SIZE_OFFSET,
layout.getEndOfFile() - layout.headerOffset - Constants.FILE_SIZE_OFFSET);
- md.digest(dest.asArray(), layout.headerOffset + Constants.SIGNATURE_OFFSET, 20);
+ md.digest(dexOutputBuffer.asArray(), layout.headerOffset + Constants.SIGNATURE_OFFSET, 20);
} catch (Exception e) {
throw new RuntimeException(e);
}
}
private void writeChecksum(Layout layout) {
+ writeChecksum(layout, dest);
+ }
+
+ public void writeChecksum(Layout layout, DexOutputBuffer dexOutputBuffer) {
Adler32 adler = new Adler32();
adler.update(
- dest.asArray(),
+ dexOutputBuffer.asArray(),
layout.headerOffset + Constants.SIGNATURE_OFFSET,
layout.getEndOfFile() - layout.headerOffset - Constants.SIGNATURE_OFFSET);
- dest.moveTo(layout.headerOffset + Constants.CHECKSUM_OFFSET);
- dest.putInt((int) adler.getValue());
+ dexOutputBuffer.moveTo(layout.headerOffset + Constants.CHECKSUM_OFFSET);
+ dexOutputBuffer.putInt((int) adler.getValue());
}
private static int alignSize(int bytes, int value) {
@@ -919,6 +874,48 @@
return (value + mask) & ~mask;
}
+ public static class MapItem {
+ final int type;
+ final int offset;
+ final int length;
+
+ public MapItem(int type, int offset, int size) {
+ this.type = type;
+ this.offset = offset;
+ this.length = size;
+ }
+
+ public int getType() {
+ return type;
+ }
+
+ public int getOffset() {
+ return offset;
+ }
+
+ public int getLength() {
+ return length;
+ }
+
+ public int write(DexOutputBuffer dest) {
+ if (length == 0) {
+ return 0;
+ }
+ if (Log.ENABLED) {
+ Log.debug(getClass(), "Map entry 0x%04x @ 0x%08x # %08d.", type, offset, length);
+ }
+ dest.putShort((short) type);
+ dest.putShort((short) 0);
+ dest.putInt(length);
+ dest.putInt(offset);
+ return 1;
+ }
+
+ public int size() {
+ return length == 0 ? 0 : 1;
+ }
+ }
+
public static class Layout {
private static final int NOT_SET = -1;
@@ -1124,6 +1121,104 @@
this.mapOffset = mapOffset;
}
+ public List<MapItem> generateMapInfo(FileWriter fileWriter) {
+ return generateMapInfo(
+ fileWriter, fileWriter.mixedSectionOffsets.getStringData().size(), stringIdsOffset);
+ }
+
+ public List<MapItem> generateMapInfo(
+ FileWriter fileWriter, int stringIdsSize, int stringIdsOffset) {
+ List<MapItem> mapItems = new ArrayList<>();
+ mapItems.add(new MapItem(Constants.TYPE_HEADER_ITEM, 0, 1));
+ mapItems.add(
+ new MapItem(
+ Constants.TYPE_STRING_ID_ITEM,
+ stringIdsOffset,
+ fileWriter.mapping.getStrings().size()));
+ mapItems.add(
+ new MapItem(
+ Constants.TYPE_TYPE_ID_ITEM, typeIdsOffset, fileWriter.mapping.getTypes().size()));
+ mapItems.add(
+ new MapItem(
+ Constants.TYPE_PROTO_ID_ITEM, protoIdsOffset, fileWriter.mapping.getProtos().size()));
+ mapItems.add(
+ new MapItem(
+ Constants.TYPE_FIELD_ID_ITEM, fieldIdsOffset, fileWriter.mapping.getFields().size()));
+ mapItems.add(
+ new MapItem(
+ Constants.TYPE_METHOD_ID_ITEM,
+ methodIdsOffset,
+ fileWriter.mapping.getMethods().size()));
+ mapItems.add(
+ new MapItem(
+ Constants.TYPE_CLASS_DEF_ITEM,
+ classDefsOffset,
+ fileWriter.mapping.getClasses().length));
+ mapItems.add(
+ new MapItem(
+ Constants.TYPE_CALL_SITE_ID_ITEM,
+ callSiteIdsOffset,
+ fileWriter.mapping.getCallSites().size()));
+ mapItems.add(
+ new MapItem(
+ Constants.TYPE_METHOD_HANDLE_ITEM,
+ methodHandleIdsOffset,
+ fileWriter.mapping.getMethodHandles().size()));
+ mapItems.add(
+ new MapItem(
+ Constants.TYPE_CODE_ITEM,
+ getCodesOffset(),
+ fileWriter.mixedSectionOffsets.getCodes().size()));
+ mapItems.add(
+ new MapItem(
+ Constants.TYPE_DEBUG_INFO_ITEM,
+ getDebugInfosOffset(),
+ fileWriter.mixedSectionOffsets.getDebugInfos().size()));
+ mapItems.add(
+ new MapItem(
+ Constants.TYPE_TYPE_LIST,
+ getTypeListsOffset(),
+ fileWriter.mixedSectionOffsets.getTypeLists().size()));
+ mapItems.add(
+ new MapItem(
+ Constants.TYPE_STRING_DATA_ITEM,
+ getStringDataOffsets(),
+ getStringDataOffsets() == 0 ? 0 : stringIdsSize));
+ mapItems.add(
+ new MapItem(
+ Constants.TYPE_ANNOTATION_ITEM,
+ getAnnotationsOffset(),
+ fileWriter.mixedSectionOffsets.getAnnotations().size()));
+ mapItems.add(
+ new MapItem(
+ Constants.TYPE_CLASS_DATA_ITEM,
+ getClassDataOffset(),
+ fileWriter.mixedSectionOffsets.getClassesWithData().size()));
+ mapItems.add(
+ new MapItem(
+ Constants.TYPE_ENCODED_ARRAY_ITEM,
+ getEncodedArraysOffset(),
+ fileWriter.mixedSectionOffsets.getEncodedArrays().size()));
+ mapItems.add(
+ new MapItem(
+ Constants.TYPE_ANNOTATION_SET_ITEM,
+ getAnnotationSetsOffset(),
+ fileWriter.mixedSectionOffsets.getAnnotationSets().size()));
+ mapItems.add(
+ new MapItem(
+ Constants.TYPE_ANNOTATION_SET_REF_LIST,
+ getAnnotationSetRefListsOffset(),
+ fileWriter.mixedSectionOffsets.getAnnotationSetRefLists().size()));
+ mapItems.add(
+ new MapItem(
+ Constants.TYPE_ANNOTATIONS_DIRECTORY_ITEM,
+ getAnnotationDirectoriesOffset(),
+ fileWriter.mixedSectionOffsets.getAnnotationDirectories().size()));
+ mapItems.add(new MapItem(Constants.TYPE_MAP_LIST, getMapOffset(), 1));
+ mapItems.sort(Comparator.comparingInt(MapItem::getOffset));
+ return mapItems;
+ }
+
public int getEndOfFile() {
return endOfFile;
}
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 6f68f53..d1f909d 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
@@ -3,13 +3,17 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.dex.container;
+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.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.assertEquals;
+import static org.junit.Assert.assertTrue;
import com.android.tools.r8.ByteDataView;
import com.android.tools.r8.ClassFileConsumer.ArchiveConsumer;
@@ -31,8 +35,10 @@
import java.io.IOException;
import java.nio.ByteOrder;
import java.nio.file.Path;
+import java.security.MessageDigest;
import java.util.ArrayList;
import java.util.List;
+import java.util.zip.Adler32;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -74,7 +80,7 @@
.setMinApi(AndroidApiLevel.L)
.compile()
.writeToZip();
- assertEquals(2, unzipContent(outputA).size());
+ validateDex(outputA, 2);
Path outputB =
testForD8(Backend.DEX)
@@ -82,7 +88,7 @@
.setMinApi(AndroidApiLevel.L)
.compile()
.writeToZip();
- assertEquals(2, unzipContent(outputB).size());
+ validateDex(outputB, 2);
Path outputMerged =
testForD8(Backend.DEX)
@@ -90,7 +96,7 @@
.setMinApi(AndroidApiLevel.L)
.compile()
.writeToZip();
- assertEquals(4, unzipContent(outputMerged).size());
+ validateDex(outputMerged, 4);
}
@Test
@@ -129,13 +135,21 @@
validateSingleContainerDex(outputB);
}
- private void validateSingleContainerDex(Path output) throws IOException {
+ private void validateDex(Path output, int expectedDexes) throws Exception {
List<byte[]> dexes = unzipContent(output);
- assertEquals(1, dexes.size());
- validateStringIdsSizeAndOffsets(dexes.get(0));
+ assertEquals(expectedDexes, dexes.size());
+ for (byte[] dex : dexes) {
+ validate(dex);
+ }
}
- private void validateStringIdsSizeAndOffsets(byte[] dex) {
+ private void validateSingleContainerDex(Path output) throws Exception {
+ List<byte[]> dexes = unzipContent(output);
+ assertEquals(1, dexes.size());
+ validate(dexes.get(0));
+ }
+
+ private void validate(byte[] dex) throws Exception {
CompatByteBuffer buffer = CompatByteBuffer.wrap(dex);
setByteOrder(buffer);
@@ -158,9 +172,53 @@
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));
+ validateMap(buffer, sectionOffset);
+ validateSignature(buffer, sectionOffset);
+ validateChecksum(buffer, sectionOffset);
}
}
+ private void validateMap(CompatByteBuffer buffer, int offset) {
+ int mapOffset = buffer.getInt(offset + MAP_OFF_OFFSET);
+ buffer.position(mapOffset);
+ int mapSize = buffer.getInt();
+ int previousOffset = Integer.MAX_VALUE;
+ for (int i = 0; i < mapSize; i++) {
+ buffer.getShort(); // Skip section type.
+ buffer.getShort(); // Skip unused.
+ buffer.getInt(); // Skip section size.
+ int o = buffer.getInt();
+ if (i > 0) {
+ assertTrue("" + i + ": " + o + " " + previousOffset, o > previousOffset);
+ }
+ previousOffset = o;
+ }
+ }
+
+ private void validateSignature(CompatByteBuffer buffer, int offset) throws Exception {
+ int sectionSize = buffer.getInt(offset + FILE_SIZE_OFFSET);
+ MessageDigest md = MessageDigest.getInstance("SHA-1");
+ md.update(
+ buffer.asByteBuffer().array(),
+ offset + FILE_SIZE_OFFSET,
+ sectionSize - offset - FILE_SIZE_OFFSET);
+ byte[] expectedSignature = new byte[20];
+ md.digest(expectedSignature, 0, 20);
+ for (int i = 0; i < expectedSignature.length; i++) {
+ assertEquals(expectedSignature[i], buffer.get(offset + SIGNATURE_OFFSET + i));
+ }
+ }
+
+ private void validateChecksum(CompatByteBuffer buffer, int offset) {
+ int sectionSize = buffer.getInt(offset + FILE_SIZE_OFFSET);
+ Adler32 adler = new Adler32();
+ adler.update(
+ buffer.asByteBuffer().array(),
+ offset + SIGNATURE_OFFSET,
+ sectionSize - offset - SIGNATURE_OFFSET);
+ assertEquals((int) adler.getValue(), buffer.getInt(offset + CHECKSUM_OFFSET));
+ }
+
private int getSizeFromMap(int type, CompatByteBuffer buffer, int offset) {
int mapOffset = buffer.getInt(offset + MAP_OFF_OFFSET);
buffer.position(mapOffset);
