Re-enable dex verifier after dexlayout
Disabled class def ordering since it violates the spec regarding
superclasses occurring before subclasses. This fixes a dex verifier
failure.
Adjust the data section size based on string data and code item
diff. This fixes dex2oat_image_test failing dex file verification.
Fixed handling of unreference annotations. Previously, these would
not get written back out. This resulted the dex file verifier
prematurely ending for annotations during CheckIntraSectionIterate
and then complaining about non zero padding (that was actually an
annotation).
Test: test-art-host
Bug: 63756964
Bug: 68208404
Change-Id: Id2b9b360640c360ac562826e9193971f7bb30eea
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index ebd4afe..affe639 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -449,7 +449,7 @@
UsageError(" The image writer will group them together.");
UsageError("");
UsageError(" --compact-dex-level=none|fast: None avoids generating compact dex, fast");
- UsageError(" generates compact dex with fast optimizations.");
+ UsageError(" generates compact dex with low compile time.");
UsageError("");
std::cerr << "See log for usage error information\n";
exit(EXIT_FAILURE);
diff --git a/dex2oat/dex2oat_test.cc b/dex2oat/dex2oat_test.cc
index 21768d3..cb91978 100644
--- a/dex2oat/dex2oat_test.cc
+++ b/dex2oat/dex2oat_test.cc
@@ -825,13 +825,13 @@
ASSERT_LT(class_def_count, std::numeric_limits<uint16_t>::max());
ASSERT_GE(class_def_count, 2U);
- // The new layout swaps the classes at indexes 0 and 1.
+ // Make sure the indexes stay the same.
std::string old_class0 = old_dex_file->PrettyType(old_dex_file->GetClassDef(0).class_idx_);
std::string old_class1 = old_dex_file->PrettyType(old_dex_file->GetClassDef(1).class_idx_);
std::string new_class0 = new_dex_file->PrettyType(new_dex_file->GetClassDef(0).class_idx_);
std::string new_class1 = new_dex_file->PrettyType(new_dex_file->GetClassDef(1).class_idx_);
- EXPECT_EQ(old_class0, new_class1);
- EXPECT_EQ(old_class1, new_class0);
+ EXPECT_EQ(old_class0, new_class0);
+ EXPECT_EQ(old_class1, new_class1);
}
EXPECT_EQ(odex_file->GetCompilerFilter(), CompilerFilter::kSpeedProfile);
diff --git a/dexlayout/dex_ir.cc b/dexlayout/dex_ir.cc
index f75eacc..8c4ee6e 100644
--- a/dexlayout/dex_ir.cc
+++ b/dexlayout/dex_ir.cc
@@ -403,8 +403,23 @@
return encoded_array_item;
}
-AnnotationItem* Collections::CreateAnnotationItem(const DexFile::AnnotationItem* annotation,
- uint32_t offset) {
+void Collections::AddAnnotationsFromMapListSection(const DexFile& dex_file,
+ uint32_t start_offset,
+ uint32_t count) {
+ uint32_t current_offset = start_offset;
+ for (size_t i = 0; i < count; ++i) {
+ // Annotation that we didn't process already, add it to the set.
+ const DexFile::AnnotationItem* annotation = dex_file.GetAnnotationItemAtOffset(current_offset);
+ AnnotationItem* annotation_item = CreateAnnotationItem(dex_file, annotation);
+ DCHECK(annotation_item != nullptr);
+ current_offset += annotation_item->GetSize();
+ }
+}
+
+AnnotationItem* Collections::CreateAnnotationItem(const DexFile& dex_file,
+ const DexFile::AnnotationItem* annotation) {
+ const uint8_t* const start_data = reinterpret_cast<const uint8_t*>(annotation);
+ const uint32_t offset = start_data - dex_file.Begin();
auto found_annotation_item = AnnotationItems().find(offset);
if (found_annotation_item != AnnotationItems().end()) {
return found_annotation_item->second.get();
@@ -413,10 +428,11 @@
const uint8_t* annotation_data = annotation->annotation_;
std::unique_ptr<EncodedValue> encoded_value(
ReadEncodedValue(&annotation_data, DexFile::kDexAnnotationAnnotation, 0));
- // TODO: Calculate the size of the annotation.
AnnotationItem* annotation_item =
new AnnotationItem(visibility, encoded_value->ReleaseEncodedAnnotation());
- annotation_items_.AddItem(annotation_item, offset);
+ annotation_item->SetOffset(offset);
+ annotation_item->SetSize(annotation_data - start_data);
+ annotation_items_.AddItem(annotation_item, annotation_item->GetOffset());
return annotation_item;
}
@@ -437,8 +453,7 @@
if (annotation == nullptr) {
continue;
}
- AnnotationItem* annotation_item =
- CreateAnnotationItem(annotation, disk_annotations_item->entries_[i]);
+ AnnotationItem* annotation_item = CreateAnnotationItem(dex_file, annotation);
items->push_back(annotation_item);
}
AnnotationSetItem* annotation_set_item = new AnnotationSetItem(items);
diff --git a/dexlayout/dex_ir.h b/dexlayout/dex_ir.h
index df3484c..99a66f3 100644
--- a/dexlayout/dex_ir.h
+++ b/dexlayout/dex_ir.h
@@ -208,7 +208,8 @@
TypeList* CreateTypeList(const DexFile::TypeList* type_list, uint32_t offset);
EncodedArrayItem* CreateEncodedArrayItem(const uint8_t* static_data, uint32_t offset);
- AnnotationItem* CreateAnnotationItem(const DexFile::AnnotationItem* annotation, uint32_t offset);
+ AnnotationItem* CreateAnnotationItem(const DexFile& dex_file,
+ const DexFile::AnnotationItem* annotation);
AnnotationSetItem* CreateAnnotationSetItem(const DexFile& dex_file,
const DexFile::AnnotationSetItem* disk_annotations_item, uint32_t offset);
AnnotationsDirectoryItem* CreateAnnotationsDirectoryItem(const DexFile& dex_file,
@@ -216,6 +217,9 @@
CodeItem* CreateCodeItem(
const DexFile& dex_file, const DexFile::CodeItem& disk_code_item, uint32_t offset);
ClassData* CreateClassData(const DexFile& dex_file, const uint8_t* encoded_data, uint32_t offset);
+ void AddAnnotationsFromMapListSection(const DexFile& dex_file,
+ uint32_t start_offset,
+ uint32_t count);
StringId* GetStringId(uint32_t index) {
CHECK_LT(index, StringIdsSize());
diff --git a/dexlayout/dex_ir_builder.cc b/dexlayout/dex_ir_builder.cc
index 8eb726a..bd3e1fa 100644
--- a/dexlayout/dex_ir_builder.cc
+++ b/dexlayout/dex_ir_builder.cc
@@ -152,6 +152,7 @@
break;
case DexFile::kDexTypeAnnotationItem:
collections->SetAnnotationItemsOffset(item->offset_);
+ collections->AddAnnotationsFromMapListSection(dex_file, item->offset_, item->size_);
break;
case DexFile::kDexTypeEncodedArrayItem:
collections->SetEncodedArrayItemsOffset(item->offset_);
diff --git a/dexlayout/dexlayout.cc b/dexlayout/dexlayout.cc
index 731040b..1c966c1 100644
--- a/dexlayout/dexlayout.cc
+++ b/dexlayout/dexlayout.cc
@@ -52,6 +52,11 @@
using android::base::StringPrintf;
+// Setting this to false disables class def layout entirely, which is stronger than strictly
+// necessary to ensure the partial order w.r.t. class derivation. TODO: Re-enable (b/68317550).
+static constexpr bool kChangeClassDefOrder = false;
+
+static constexpr uint32_t kDataSectionAlignment = sizeof(uint32_t) * 2;
static constexpr uint32_t kDexCodeItemAlignment = 4;
/*
@@ -1581,9 +1586,13 @@
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 (kChangeClassDefOrder) {
+ // This produces dex files that violate the spec since the super class class_def is supposed
+ // to occur before any subclasses.
+ class_def->SetIndex(i);
+ class_def->SetOffset(class_defs_offset);
+ class_defs_offset += dex_ir::ClassDef::ItemSize();
+ }
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);
@@ -1595,7 +1604,7 @@
return new_class_data_order;
}
-void DexLayout::LayoutStringData(const DexFile* dex_file) {
+int32_t DexLayout::LayoutStringData(const DexFile* dex_file) {
const size_t num_strings = header_->GetCollections().StringIds().size();
std::vector<bool> is_shorty(num_strings, false);
std::vector<bool> from_hot_method(num_strings, false);
@@ -1708,13 +1717,11 @@
offset += data->GetSize() + 1; // Add one extra for null.
}
if (offset > max_offset) {
- const uint32_t diff = offset - max_offset;
+ return offset - max_offset;
// If we expanded the string data section, we need to update the offsets or else we will
// corrupt the next section when writing out.
- FixupSections(header_->GetCollections().StringDatasOffset(), diff);
- // Update file size.
- header_->SetFileSize(header_->FileSize() + diff);
}
+ return 0;
}
// Orders code items according to specified class data ordering.
@@ -1954,13 +1961,23 @@
}
void DexLayout::LayoutOutputFile(const DexFile* dex_file) {
- LayoutStringData(dex_file);
- std::vector<dex_ir::ClassData*> new_class_data_order = LayoutClassDefsAndClassData(dex_file);
- int32_t diff = LayoutCodeItems(dex_file, new_class_data_order);
- // Move sections after ClassData by diff bytes.
- FixupSections(header_->GetCollections().ClassDatasOffset(), diff);
+ const int32_t string_diff = LayoutStringData(dex_file);
+ // If we expanded the string data section, we need to update the offsets or else we will
+ // corrupt the next section when writing out.
+ FixupSections(header_->GetCollections().StringDatasOffset(), string_diff);
// Update file size.
- header_->SetFileSize(header_->FileSize() + diff);
+ header_->SetFileSize(header_->FileSize() + string_diff);
+
+ std::vector<dex_ir::ClassData*> new_class_data_order = LayoutClassDefsAndClassData(dex_file);
+ const int32_t code_item_diff = LayoutCodeItems(dex_file, new_class_data_order);
+ // Move sections after ClassData by diff bytes.
+ FixupSections(header_->GetCollections().ClassDatasOffset(), code_item_diff);
+
+ // Update file and data size.
+ // The data size must be aligned to kDataSectionAlignment.
+ const int32_t total_diff = code_item_diff + string_diff;
+ header_->SetDataSize(RoundUp(header_->DataSize() + total_diff, kDataSectionAlignment));
+ header_->SetFileSize(header_->FileSize() + total_diff);
}
void DexLayout::OutputDexFile(const DexFile* dex_file) {
@@ -2016,8 +2033,7 @@
/*verify*/ true,
/*verify_checksum*/ false,
&error_msg));
- // Disabled since it is currently failing for dex2oat_image_test.
- // DCHECK(output_dex_file != nullptr) << "Failed to re-open output file:" << error_msg;
+ DCHECK(output_dex_file != nullptr) << " Failed to re-open output file:" << error_msg;
}
// Do IR-level comparison between input and output. This check ignores potential differences
// due to layout, so offsets are not checked. Instead, it checks the data contents of each item.
diff --git a/dexlayout/dexlayout.h b/dexlayout/dexlayout.h
index eaa7cac..57d273e 100644
--- a/dexlayout/dexlayout.h
+++ b/dexlayout/dexlayout.h
@@ -123,7 +123,7 @@
std::vector<dex_ir::ClassData*> LayoutClassDefsAndClassData(const DexFile* dex_file);
int32_t LayoutCodeItems(const DexFile* dex_file,
std::vector<dex_ir::ClassData*> new_class_data_order);
- void LayoutStringData(const DexFile* dex_file);
+ int32_t LayoutStringData(const DexFile* dex_file);
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/runtime/dex_file.h b/runtime/dex_file.h
index d7ea745..53e1f9e 100644
--- a/runtime/dex_file.h
+++ b/runtime/dex_file.h
@@ -853,14 +853,18 @@
: reinterpret_cast<const AnnotationSetRefList*>(begin_ + offset);
}
- const AnnotationItem* GetAnnotationItem(const AnnotationSetItem* set_item, uint32_t index) const {
- DCHECK_LE(index, set_item->size_);
- uint32_t offset = set_item->entries_[index];
+ ALWAYS_INLINE const AnnotationItem* GetAnnotationItemAtOffset(uint32_t offset) const {
+ DCHECK_LE(offset, Size());
return (offset == 0)
? nullptr
: reinterpret_cast<const AnnotationItem*>(begin_ + offset);
}
+ const AnnotationItem* GetAnnotationItem(const AnnotationSetItem* set_item, uint32_t index) const {
+ DCHECK_LE(index, set_item->size_);
+ return GetAnnotationItemAtOffset(set_item->entries_[index]);
+ }
+
const AnnotationSetItem* GetSetRefItemItem(const AnnotationSetRefItem* anno_item) const {
uint32_t offset = anno_item->annotations_off_;
return (offset == 0)
diff --git a/runtime/dex_file_verifier.cc b/runtime/dex_file_verifier.cc
index 8fdd470..50f56c7 100644
--- a/runtime/dex_file_verifier.cc
+++ b/runtime/dex_file_verifier.cc
@@ -721,14 +721,19 @@
return true;
}
-bool DexFileVerifier::CheckPadding(size_t offset, uint32_t aligned_offset) {
+bool DexFileVerifier::CheckPadding(size_t offset,
+ uint32_t aligned_offset,
+ DexFile::MapItemType type) {
if (offset < aligned_offset) {
if (!CheckListSize(begin_ + offset, aligned_offset - offset, sizeof(uint8_t), "section")) {
return false;
}
while (offset < aligned_offset) {
if (UNLIKELY(*ptr_ != '\0')) {
- ErrorStringPrintf("Non-zero padding %x before section start at %zx", *ptr_, offset);
+ ErrorStringPrintf("Non-zero padding %x before section of type %zu at offset 0x%zx",
+ *ptr_,
+ static_cast<size_t>(type),
+ offset);
return false;
}
ptr_++;
@@ -1615,7 +1620,7 @@
size_t aligned_offset = (offset + alignment_mask) & ~alignment_mask;
// Check the padding between items.
- if (!CheckPadding(offset, aligned_offset)) {
+ if (!CheckPadding(offset, aligned_offset, type)) {
return false;
}
@@ -1837,7 +1842,10 @@
size_t next_offset = ptr_ - begin_;
if (next_offset > data_end) {
- ErrorStringPrintf("Out-of-bounds end of data subsection: %zx", next_offset);
+ ErrorStringPrintf("Out-of-bounds end of data subsection: %zu data_off=%u data_size=%u",
+ next_offset,
+ header_->data_off_,
+ header_->data_size_);
return false;
}
@@ -1859,7 +1867,7 @@
DexFile::MapItemType type = static_cast<DexFile::MapItemType>(item->type_);
// Check for padding and overlap between items.
- if (!CheckPadding(offset, section_offset)) {
+ if (!CheckPadding(offset, section_offset, type)) {
return false;
} else if (UNLIKELY(offset > section_offset)) {
ErrorStringPrintf("Section overlap or out-of-order map: %zx, %x", offset, section_offset);
diff --git a/runtime/dex_file_verifier.h b/runtime/dex_file_verifier.h
index 74f8225..23089fa 100644
--- a/runtime/dex_file_verifier.h
+++ b/runtime/dex_file_verifier.h
@@ -97,7 +97,7 @@
const DexFile::ClassDef** class_def);
bool CheckStaticFieldTypes(const DexFile::ClassDef* class_def);
- bool CheckPadding(size_t offset, uint32_t aligned_offset);
+ bool CheckPadding(size_t offset, uint32_t aligned_offset, DexFile::MapItemType type);
bool CheckEncodedValue();
bool CheckEncodedArray();
bool CheckEncodedAnnotation();