Dexlayout fix for duplicate class data and preserving last code item.
This change fixes a dexlayout issue when multiple class defs reference
the same class data. A set is used to tell what class data has already
been visited.
This change also makes dexlayout preserve the offset of the last code
item in the dex file if the following section is not 4-byte aligned.
Due to limitations in dexlayout, it is difficult to adjust the offsets
of sections if more space is needed. An overhaul of dexlayout should
be done to properly fix this.
Bug: 35451910
Test: mm test-art-host-gtest-dexlayout_test
Change-Id: I8d066c2151a1a57c382ce35f12bf53a519da89f6
diff --git a/dexlayout/dex_ir.cc b/dexlayout/dex_ir.cc
index 609068f..131f4b9 100644
--- a/dexlayout/dex_ir.cc
+++ b/dexlayout/dex_ir.cc
@@ -653,7 +653,7 @@
if (has_catch_all) {
size = -size;
}
- if (already_added == true) {
+ if (already_added) {
for (int32_t i = 0; i < size; i++) {
DecodeUnsignedLeb128(&handlers_data);
DecodeUnsignedLeb128(&handlers_data);
diff --git a/dexlayout/dexlayout.cc b/dexlayout/dexlayout.cc
index 1add6bf..22619b9 100644
--- a/dexlayout/dexlayout.cc
+++ b/dexlayout/dexlayout.cc
@@ -46,6 +46,8 @@
using android::base::StringPrintf;
+static constexpr uint32_t kDexCodeItemAlignment = 4;
+
/*
* Flags for use with createAccessFlagStr().
*/
@@ -1489,7 +1491,7 @@
}
}
-std::vector<dex_ir::ClassDef*> DexLayout::LayoutClassDefsAndClassData(const DexFile* dex_file) {
+std::vector<dex_ir::ClassData*> DexLayout::LayoutClassDefsAndClassData(const DexFile* dex_file) {
std::vector<dex_ir::ClassDef*> new_class_def_order;
for (std::unique_ptr<dex_ir::ClassDef>& class_def : header_->GetCollections().ClassDefs()) {
dex::TypeIndex type_idx(class_def->ClassType()->GetIndex());
@@ -1505,46 +1507,93 @@
}
uint32_t class_defs_offset = header_->GetCollections().ClassDefsOffset();
uint32_t class_data_offset = header_->GetCollections().ClassDatasOffset();
+ std::unordered_set<dex_ir::ClassData*> visited_class_data;
+ std::vector<dex_ir::ClassData*> new_class_data_order;
for (uint32_t i = 0; i < new_class_def_order.size(); ++i) {
dex_ir::ClassDef* class_def = new_class_def_order[i];
class_def->SetIndex(i);
class_def->SetOffset(class_defs_offset);
class_defs_offset += dex_ir::ClassDef::ItemSize();
- if (class_def->GetClassData() != nullptr) {
- class_def->GetClassData()->SetOffset(class_data_offset);
- class_data_offset += class_def->GetClassData()->GetSize();
+ dex_ir::ClassData* class_data = class_def->GetClassData();
+ if (class_data != nullptr && visited_class_data.find(class_data) == visited_class_data.end()) {
+ class_data->SetOffset(class_data_offset);
+ class_data_offset += class_data->GetSize();
+ visited_class_data.insert(class_data);
+ new_class_data_order.push_back(class_data);
}
}
- return new_class_def_order;
+ return new_class_data_order;
}
-int32_t DexLayout::LayoutCodeItems(std::vector<dex_ir::ClassDef*> new_class_def_order) {
- int32_t diff = 0;
+// Orders code items according to specified class data ordering.
+// NOTE: If the section following the code items is byte aligned, the last code item is left in
+// place to preserve alignment. Layout needs an overhaul to handle movement of other sections.
+int32_t DexLayout::LayoutCodeItems(std::vector<dex_ir::ClassData*> new_class_data_order) {
+ // Find the last code item so we can leave it in place if the next section is not 4 byte aligned.
+ std::unordered_set<dex_ir::CodeItem*> visited_code_items;
uint32_t offset = header_->GetCollections().CodeItemsOffset();
- for (dex_ir::ClassDef* class_def : new_class_def_order) {
- dex_ir::ClassData* class_data = class_def->GetClassData();
- if (class_data != nullptr) {
- class_data->SetOffset(class_data->GetOffset() + diff);
- for (auto& method : *class_data->DirectMethods()) {
- dex_ir::CodeItem* code_item = method->GetCodeItem();
- if (code_item != nullptr) {
- diff += UnsignedLeb128Size(offset) - UnsignedLeb128Size(code_item->GetOffset());
- code_item->SetOffset(offset);
- offset += RoundUp(code_item->GetSize(), 4);
- }
+ bool is_code_item_aligned = IsNextSectionCodeItemAligned(offset);
+ if (!is_code_item_aligned) {
+ dex_ir::CodeItem* last_code_item = nullptr;
+ for (auto& code_item_pair : header_->GetCollections().CodeItems()) {
+ std::unique_ptr<dex_ir::CodeItem>& code_item = code_item_pair.second;
+ if (last_code_item == nullptr || last_code_item->GetOffset() < code_item->GetOffset()) {
+ last_code_item = code_item.get();
}
- for (auto& method : *class_data->VirtualMethods()) {
- dex_ir::CodeItem* code_item = method->GetCodeItem();
- if (code_item != nullptr) {
- diff += UnsignedLeb128Size(offset) - UnsignedLeb128Size(code_item->GetOffset());
- code_item->SetOffset(offset);
- offset += RoundUp(code_item->GetSize(), 4);
- }
+ }
+ // Preserve the last code item by marking it already visited.
+ visited_code_items.insert(last_code_item);
+ }
+
+ int32_t diff = 0;
+ for (dex_ir::ClassData* class_data : new_class_data_order) {
+ class_data->SetOffset(class_data->GetOffset() + diff);
+ for (auto& method : *class_data->DirectMethods()) {
+ dex_ir::CodeItem* code_item = method->GetCodeItem();
+ if (code_item != nullptr && visited_code_items.find(code_item) == visited_code_items.end()) {
+ visited_code_items.insert(code_item);
+ diff += UnsignedLeb128Size(offset) - UnsignedLeb128Size(code_item->GetOffset());
+ code_item->SetOffset(offset);
+ offset += RoundUp(code_item->GetSize(), kDexCodeItemAlignment);
+ }
+ }
+ for (auto& method : *class_data->VirtualMethods()) {
+ dex_ir::CodeItem* code_item = method->GetCodeItem();
+ if (code_item != nullptr && visited_code_items.find(code_item) == visited_code_items.end()) {
+ visited_code_items.insert(code_item);
+ diff += UnsignedLeb128Size(offset) - UnsignedLeb128Size(code_item->GetOffset());
+ code_item->SetOffset(offset);
+ offset += RoundUp(code_item->GetSize(), kDexCodeItemAlignment);
}
}
}
+ // Adjust diff to be 4-byte aligned.
+ return RoundUp(diff, kDexCodeItemAlignment);
+}
- return diff;
+bool DexLayout::IsNextSectionCodeItemAligned(uint32_t offset) {
+ dex_ir::Collections& collections = header_->GetCollections();
+ std::set<uint32_t> section_offsets;
+ section_offsets.insert(collections.MapListOffset());
+ section_offsets.insert(collections.TypeListsOffset());
+ section_offsets.insert(collections.AnnotationSetRefListsOffset());
+ section_offsets.insert(collections.AnnotationSetItemsOffset());
+ section_offsets.insert(collections.ClassDatasOffset());
+ section_offsets.insert(collections.CodeItemsOffset());
+ section_offsets.insert(collections.StringDatasOffset());
+ section_offsets.insert(collections.DebugInfoItemsOffset());
+ section_offsets.insert(collections.AnnotationItemsOffset());
+ section_offsets.insert(collections.EncodedArrayItemsOffset());
+ section_offsets.insert(collections.AnnotationsDirectoryItemsOffset());
+
+ auto found = section_offsets.find(offset);
+ if (found != section_offsets.end()) {
+ found++;
+ if (found != section_offsets.end()) {
+ return *found % kDexCodeItemAlignment == 0;
+ }
+ }
+ return false;
}
// Adjust offsets of every item in the specified section by diff bytes.
@@ -1626,10 +1675,8 @@
}
void DexLayout::LayoutOutputFile(const DexFile* dex_file) {
- std::vector<dex_ir::ClassDef*> new_class_def_order = LayoutClassDefsAndClassData(dex_file);
- int32_t diff = LayoutCodeItems(new_class_def_order);
- // Adjust diff to be 4-byte aligned.
- diff = RoundUp(diff, 4);
+ std::vector<dex_ir::ClassData*> new_class_data_order = LayoutClassDefsAndClassData(dex_file);
+ int32_t diff = LayoutCodeItems(new_class_data_order);
// Move sections after ClassData by diff bytes.
FixupSections(header_->GetCollections().ClassDatasOffset(), diff);
// Update file size.
diff --git a/dexlayout/dexlayout.h b/dexlayout/dexlayout.h
index ac1a4a6..3918706 100644
--- a/dexlayout/dexlayout.h
+++ b/dexlayout/dexlayout.h
@@ -105,8 +105,9 @@
void DumpSField(uint32_t idx, uint32_t flags, int i, dex_ir::EncodedValue* init);
void DumpDexFile();
- std::vector<dex_ir::ClassDef*> LayoutClassDefsAndClassData(const DexFile* dex_file);
- int32_t LayoutCodeItems(std::vector<dex_ir::ClassDef*> new_class_def_order);
+ std::vector<dex_ir::ClassData*> LayoutClassDefsAndClassData(const DexFile* dex_file);
+ int32_t LayoutCodeItems(std::vector<dex_ir::ClassData*> new_class_data_order);
+ bool IsNextSectionCodeItemAligned(uint32_t offset);
template<class T> void FixupSection(std::map<uint32_t, std::unique_ptr<T>>& map, uint32_t diff);
void FixupSections(uint32_t offset, uint32_t diff);
diff --git a/dexlayout/dexlayout_test.cc b/dexlayout/dexlayout_test.cc
index 9881e28..9f0593a 100644
--- a/dexlayout/dexlayout_test.cc
+++ b/dexlayout/dexlayout_test.cc
@@ -55,6 +55,26 @@
"qAAAAAYAAAACAAAAwAAAAAEgAAACAAAAAAEAAAIgAAAHAAAAMAEAAAMgAAACAAAAaQEAAAAgAAAC"
"AAAAdQEAAAAQAAABAAAAjAEAAA==";
+// Dex file with catch handler unreferenced by try blocks.
+// Constructed by building a dex file with try/catch blocks and hex editing.
+static const char kUnreferencedCatchHandlerInputDex[] =
+ "ZGV4CjAzNQD+exd52Y0f9nY5x5GmInXq5nXrO6Kl2RV4AwAAcAAAAHhWNBIAAAAAAAAAANgCAAAS"
+ "AAAAcAAAAAgAAAC4AAAAAwAAANgAAAABAAAA/AAAAAQAAAAEAQAAAQAAACQBAAA0AgAARAEAANYB"
+ "AADeAQAA5gEAAO4BAAAAAgAADwIAACYCAAA9AgAAUQIAAGUCAAB5AgAAfwIAAIUCAACIAgAAjAIA"
+ "AKECAACnAgAArAIAAAQAAAAFAAAABgAAAAcAAAAIAAAACQAAAAwAAAAOAAAADAAAAAYAAAAAAAAA"
+ "DQAAAAYAAADIAQAADQAAAAYAAADQAQAABQABABAAAAAAAAAAAAAAAAAAAgAPAAAAAQABABEAAAAD"
+ "AAAAAAAAAAAAAAABAAAAAwAAAAAAAAADAAAAAAAAAMgCAAAAAAAAAQABAAEAAAC1AgAABAAAAHAQ"
+ "AwAAAA4AAwABAAIAAgC6AgAAIQAAAGIAAAAaAQoAbiACABAAYgAAABoBCwBuIAIAEAAOAA0AYgAA"
+ "ABoBAQBuIAIAEAAo8A0AYgAAABoBAgBuIAIAEAAo7gAAAAAAAAcAAQAHAAAABwABAAIBAg8BAhgA"
+ "AQAAAAQAAAABAAAABwAGPGluaXQ+AAZDYXRjaDEABkNhdGNoMgAQSGFuZGxlclRlc3QuamF2YQAN"
+ "TEhhbmRsZXJUZXN0OwAVTGphdmEvaW8vUHJpbnRTdHJlYW07ABVMamF2YS9sYW5nL0V4Y2VwdGlv"
+ "bjsAEkxqYXZhL2xhbmcvT2JqZWN0OwASTGphdmEvbGFuZy9TdHJpbmc7ABJMamF2YS9sYW5nL1N5"
+ "c3RlbTsABFRyeTEABFRyeTIAAVYAAlZMABNbTGphdmEvbGFuZy9TdHJpbmc7AARtYWluAANvdXQA"
+ "B3ByaW50bG4AAQAHDgAEAQAHDn17AncdHoseAAAAAgAAgYAExAIBCdwCAAANAAAAAAAAAAEAAAAA"
+ "AAAAAQAAABIAAABwAAAAAgAAAAgAAAC4AAAAAwAAAAMAAADYAAAABAAAAAEAAAD8AAAABQAAAAQA"
+ "AAAEAQAABgAAAAEAAAAkAQAAASAAAAIAAABEAQAAARAAAAIAAADIAQAAAiAAABIAAADWAQAAAyAA"
+ "AAIAAAC1AgAAACAAAAEAAADIAgAAABAAAAEAAADYAgAA";
+
// Dex file with multiple code items that have the same debug_info_off_. Constructed by a modified
// dexlayout on XandY.
static const char kDexFileDuplicateOffset[] =
@@ -100,25 +120,30 @@
"ASAAAAIAAACEAQAABiAAAAIAAACwAQAAARAAAAIAAADYAQAAAiAAABIAAADoAQAAAyAAAAIAAADw"
"AgAABCAAAAIAAAD8AgAAACAAAAIAAAAIAwAAABAAAAEAAAAgAwAA";
-// Dex file with catch handler unreferenced by try blocks.
-// Constructed by building a dex file with try/catch blocks and hex editing.
-static const char kUnreferencedCatchHandlerInputDex[] =
- "ZGV4CjAzNQD+exd52Y0f9nY5x5GmInXq5nXrO6Kl2RV4AwAAcAAAAHhWNBIAAAAAAAAAANgCAAAS"
- "AAAAcAAAAAgAAAC4AAAAAwAAANgAAAABAAAA/AAAAAQAAAAEAQAAAQAAACQBAAA0AgAARAEAANYB"
- "AADeAQAA5gEAAO4BAAAAAgAADwIAACYCAAA9AgAAUQIAAGUCAAB5AgAAfwIAAIUCAACIAgAAjAIA"
- "AKECAACnAgAArAIAAAQAAAAFAAAABgAAAAcAAAAIAAAACQAAAAwAAAAOAAAADAAAAAYAAAAAAAAA"
- "DQAAAAYAAADIAQAADQAAAAYAAADQAQAABQABABAAAAAAAAAAAAAAAAAAAgAPAAAAAQABABEAAAAD"
- "AAAAAAAAAAAAAAABAAAAAwAAAAAAAAADAAAAAAAAAMgCAAAAAAAAAQABAAEAAAC1AgAABAAAAHAQ"
- "AwAAAA4AAwABAAIAAgC6AgAAIQAAAGIAAAAaAQoAbiACABAAYgAAABoBCwBuIAIAEAAOAA0AYgAA"
- "ABoBAQBuIAIAEAAo8A0AYgAAABoBAgBuIAIAEAAo7gAAAAAAAAcAAQAHAAAABwABAAIBAg8BAhgA"
- "AQAAAAQAAAABAAAABwAGPGluaXQ+AAZDYXRjaDEABkNhdGNoMgAQSGFuZGxlclRlc3QuamF2YQAN"
- "TEhhbmRsZXJUZXN0OwAVTGphdmEvaW8vUHJpbnRTdHJlYW07ABVMamF2YS9sYW5nL0V4Y2VwdGlv"
- "bjsAEkxqYXZhL2xhbmcvT2JqZWN0OwASTGphdmEvbGFuZy9TdHJpbmc7ABJMamF2YS9sYW5nL1N5"
- "c3RlbTsABFRyeTEABFRyeTIAAVYAAlZMABNbTGphdmEvbGFuZy9TdHJpbmc7AARtYWluAANvdXQA"
- "B3ByaW50bG4AAQAHDgAEAQAHDn17AncdHoseAAAAAgAAgYAExAIBCdwCAAANAAAAAAAAAAEAAAAA"
- "AAAAAQAAABIAAABwAAAAAgAAAAgAAAC4AAAAAwAAAAMAAADYAAAABAAAAAEAAAD8AAAABQAAAAQA"
- "AAAEAQAABgAAAAEAAAAkAQAAASAAAAIAAABEAQAAARAAAAIAAADIAQAAAiAAABIAAADWAQAAAyAA"
- "AAIAAAC1AgAAACAAAAEAAADIAgAAABAAAAEAAADYAgAA";
+// Dex file with shared empty class data item for multiple class defs.
+// Constructing by building a dex file with multiple classes and hex editing.
+static const char kMultiClassDataInputDex[] =
+ "ZGV4CjAzNQALJgF9TtnLq748xVe/+wyxETrT9lTEiW6YAQAAcAAAAHhWNBIAAAAAAAAAADQBAAAI"
+ "AAAAcAAAAAQAAACQAAAAAAAAAAAAAAACAAAAoAAAAAAAAAAAAAAAAgAAALAAAACoAAAA8AAAAPAA"
+ "AAD4AAAAAAEAAAMBAAAIAQAADQEAACEBAAAkAQAAAgAAAAMAAAAEAAAABQAAAAEAAAAGAAAAAgAA"
+ "AAcAAAABAAAAAQYAAAMAAAAAAAAAAAAAAAAAAAAnAQAAAAAAAAIAAAABBgAAAwAAAAAAAAABAAAA"
+ "AAAAACcBAAAAAAAABkEuamF2YQAGQi5qYXZhAAFJAANMQTsAA0xCOwASTGphdmEvbGFuZy9PYmpl"
+ "Y3Q7AAFhAAFiAAAAAAABAAAAARkAAAAIAAAAAAAAAAEAAAAAAAAAAQAAAAgAAABwAAAAAgAAAAQA"
+ "AACQAAAABAAAAAIAAACgAAAABgAAAAIAAACwAAAAAiAAAAgAAADwAAAAACAAAAIAAAAnAQAAABAA"
+ "AAEAAAA0AQAA";
+
+// Dex file with code info followed by non 4-byte aligned section.
+// Constructed a dex file with code info followed by string data and hex edited.
+static const char kUnalignedCodeInfoInputDex[] =
+ "ZGV4CjAzNQDXJzXNb4iWn2SLhmLydW/8h1K9moERIw7UAQAAcAAAAHhWNBIAAAAAAAAAAEwBAAAG"
+ "AAAAcAAAAAMAAACIAAAAAQAAAJQAAAAAAAAAAAAAAAMAAACgAAAAAQAAALgAAAD8AAAA2AAAAAIB"
+ "AAAKAQAAEgEAABcBAAArAQAALgEAAAIAAAADAAAABAAAAAQAAAACAAAAAAAAAAAAAAAAAAAAAAAA"
+ "AAUAAAABAAAAAAAAAAAAAAABAAAAAQAAAAAAAAABAAAAAAAAADsBAAAAAAAAAQABAAEAAAAxAQAA"
+ "BAAAAHAQAgAAAA4AAQABAAAAAAA2AQAAAQAAAA4ABjxpbml0PgAGQS5qYXZhAANMQTsAEkxqYXZh"
+ "L2xhbmcvT2JqZWN0OwABVgABYQABAAcOAAMABw4AAAABAQCBgATYAQEB8AEAAAALAAAAAAAAAAEA"
+ "AAAAAAAAAQAAAAYAAABwAAAAAgAAAAMAAACIAAAAAwAAAAEAAACUAAAABQAAAAMAAACgAAAABgAA"
+ "AAEAAAC4AAAAASAAAAIAAADYAAAAAiAAAAYAAAACAQAAAyAAAAIAAAAxAQAAACAAAAEAAAA7AQAA"
+ "ABAAAAEAAABMAQAA";
static void WriteBase64ToFile(const char* base64, File* file) {
// Decode base64.
@@ -314,6 +339,12 @@
ASSERT_TRUE(DexFileLayoutExec(&error_msg)) << error_msg;
}
+TEST_F(DexLayoutTest, UnreferencedCatchHandler) {
+ // Disable test on target.
+ TEST_DISABLED_FOR_TARGET();
+ std::string error_msg;
+ ASSERT_TRUE(UnreferencedCatchHandlerExec(&error_msg)) << error_msg;
+}
TEST_F(DexLayoutTest, DuplicateOffset) {
ScratchFile temp;
WriteBase64ToFile(kDexFileDuplicateOffset, temp.GetFile());
@@ -351,11 +382,40 @@
}
}
-TEST_F(DexLayoutTest, UnreferencedCatchHandler) {
- // Disable test on target.
- TEST_DISABLED_FOR_TARGET();
+TEST_F(DexLayoutTest, MultiClassData) {
+ ScratchFile temp;
+ WriteBase64ToFile(kMultiClassDataInputDex, temp.GetFile());
+ ScratchFile temp2;
+ WriteBase64ToFile(kDexFileLayoutInputProfile, temp2.GetFile());
+ EXPECT_EQ(temp.GetFile()->Flush(), 0);
+ std::string dexlayout = GetTestAndroidRoot() + "/bin/dexlayout";
+ EXPECT_TRUE(OS::FileExists(dexlayout.c_str())) << dexlayout << " should be a valid file path";
+ std::vector<std::string> dexlayout_exec_argv =
+ { dexlayout, "-p", temp2.GetFilename(), "-o", "/dev/null", temp.GetFilename() };
std::string error_msg;
- ASSERT_TRUE(UnreferencedCatchHandlerExec(&error_msg)) << error_msg;
+ const bool result = ::art::Exec(dexlayout_exec_argv, &error_msg);
+ EXPECT_TRUE(result);
+ if (!result) {
+ LOG(ERROR) << "Error " << error_msg;
+ }
+}
+
+TEST_F(DexLayoutTest, UnalignedCodeInfo) {
+ ScratchFile temp;
+ WriteBase64ToFile(kUnalignedCodeInfoInputDex, temp.GetFile());
+ ScratchFile temp2;
+ WriteBase64ToFile(kDexFileLayoutInputProfile, temp2.GetFile());
+ EXPECT_EQ(temp.GetFile()->Flush(), 0);
+ std::string dexlayout = GetTestAndroidRoot() + "/bin/dexlayout";
+ EXPECT_TRUE(OS::FileExists(dexlayout.c_str())) << dexlayout << " should be a valid file path";
+ std::vector<std::string> dexlayout_exec_argv =
+ { dexlayout, "-p", temp2.GetFilename(), "-o", "/dev/null", temp.GetFilename() };
+ std::string error_msg;
+ const bool result = ::art::Exec(dexlayout_exec_argv, &error_msg);
+ EXPECT_TRUE(result);
+ if (!result) {
+ LOG(ERROR) << "Error " << error_msg;
+ }
}
} // namespace art