libandroidfw: Remove pre-verification
This added more up-front cost to loading an APK and didn't provide
a significant benefit to resource retrieval.
Test: make libandroidfw_tests
Change-Id: Idbf993abc433fa8c8950d106c66469b310b66f7f
diff --git a/libs/androidfw/LoadedArsc.cpp b/libs/androidfw/LoadedArsc.cpp
index 6ee5e86..28548e2 100644
--- a/libs/androidfw/LoadedArsc.cpp
+++ b/libs/androidfw/LoadedArsc.cpp
@@ -129,9 +129,14 @@
// Precondition: The header passed in has already been verified, so reading any fields and trusting
// the ResChunk_header is safe.
static bool VerifyResTableType(const ResTable_type* header) {
+ if (header->id == 0) {
+ LOG(ERROR) << "RES_TABLE_TYPE_TYPE has invalid ID 0.";
+ return false;
+ }
+
const size_t entry_count = dtohl(header->entryCount);
if (entry_count > std::numeric_limits<uint16_t>::max()) {
- LOG(ERROR) << "Too many entries in RES_TABLE_TYPE_TYPE.";
+ LOG(ERROR) << "RES_TABLE_TYPE_TYPE has too many entries (" << entry_count << ").";
return false;
}
@@ -141,17 +146,17 @@
const size_t offsets_length = sizeof(uint32_t) * entry_count;
if (offsets_offset > entries_offset || entries_offset - offsets_offset < offsets_length) {
- LOG(ERROR) << "Entry offsets overlap actual entry data.";
+ LOG(ERROR) << "RES_TABLE_TYPE_TYPE entry offsets overlap actual entry data.";
return false;
}
if (entries_offset > dtohl(header->header.size)) {
- LOG(ERROR) << "Entry offsets extend beyond chunk.";
+ LOG(ERROR) << "RES_TABLE_TYPE_TYPE entry offsets extend beyond chunk.";
return false;
}
if (entries_offset & 0x03) {
- LOG(ERROR) << "Entries start at unaligned address.";
+ LOG(ERROR) << "RES_TABLE_TYPE_TYPE entries start at unaligned address.";
return false;
}
return true;
@@ -236,7 +241,6 @@
return true;
}
-template <bool Verified>
bool LoadedPackage::FindEntry(const TypeSpecPtr& type_spec_ptr, uint16_t entry_idx,
const ResTable_config& config, FindEntryResult* out_entry) const {
const ResTable_config* best_config = nullptr;
@@ -254,13 +258,6 @@
const size_t entry_count = dtohl(type_chunk->entryCount);
const size_t offsets_offset = dtohs(type_chunk->header.headerSize);
- // If the package hasn't been verified, do bounds checking.
- if (!Verified) {
- if (!VerifyResTableType(type_chunk)) {
- continue;
- }
- }
-
// Check if there is the desired entry in this type.
if (type_chunk->flags & ResTable_type::FLAG_SPARSE) {
@@ -282,7 +279,7 @@
// Extract the offset from the entry. Each offset must be a multiple of 4 so we store it as
// the real offset divided by 4.
- best_offset = dtohs(result->offset) * 4u;
+ best_offset = uint32_t{dtohs(result->offset)} * 4u;
} else {
if (entry_idx >= entry_count) {
// This entry cannot be here.
@@ -309,10 +306,8 @@
return false;
}
- if (!Verified) {
- if (!VerifyResTableEntry(best_type, best_offset, entry_idx)) {
- return false;
- }
+ if (UNLIKELY(!VerifyResTableEntry(best_type, best_offset, entry_idx))) {
+ return false;
}
const ResTable_entry* best_entry = reinterpret_cast<const ResTable_entry*>(
@@ -334,7 +329,7 @@
// If the type IDs are offset in this package, we need to take that into account when searching
// for a type.
const TypeSpecPtr& ptr = type_specs_[type_idx - type_id_offset_];
- if (ptr == nullptr) {
+ if (UNLIKELY(ptr == nullptr)) {
return false;
}
@@ -345,54 +340,7 @@
return false;
}
}
-
- // Don't bother checking if the entry ID is larger than the number of entries.
- if (entry_idx >= dtohl(ptr->type_spec->entryCount)) {
- return false;
- }
-
- if (verified_) {
- return FindEntry<true>(ptr, entry_idx, config, out_entry);
- }
- return FindEntry<false>(ptr, entry_idx, config, out_entry);
-}
-
-static bool VerifyType(const Chunk& chunk) {
- ATRACE_CALL();
- const ResTable_type* header = chunk.header<ResTable_type, kResTableTypeMinSize>();
-
- if (!VerifyResTableType(header)) {
- return false;
- }
-
- const size_t entry_count = dtohl(header->entryCount);
- const size_t offsets_offset = chunk.header_size();
-
- if (header->flags & ResTable_type::FLAG_SPARSE) {
- // This is encoded as a sparse map, so perform a binary search.
- const ResTable_sparseTypeEntry* sparse_indices =
- reinterpret_cast<const ResTable_sparseTypeEntry*>(reinterpret_cast<const uint8_t*>(header) +
- offsets_offset);
- for (size_t i = 0; i < entry_count; i++) {
- const uint32_t offset = uint32_t{dtohs(sparse_indices[i].offset)} * 4u;
- if (!VerifyResTableEntry(header, offset, i)) {
- return false;
- }
- }
- } else {
- // Check each entry offset.
- const uint32_t* offsets = reinterpret_cast<const uint32_t*>(
- reinterpret_cast<const uint8_t*>(header) + offsets_offset);
- for (size_t i = 0; i < entry_count; i++) {
- uint32_t offset = dtohl(offsets[i]);
- if (offset != ResTable_type::NO_ENTRY) {
- if (!VerifyResTableEntry(header, offset, i)) {
- return false;
- }
- }
- }
- }
- return true;
+ return FindEntry(ptr, entry_idx, config, out_entry);
}
void LoadedPackage::CollectConfigurations(bool exclude_mipmap,
@@ -507,7 +455,7 @@
sizeof(ResTable_package) - sizeof(ResTable_package::typeIdOffset);
const ResTable_package* header = chunk.header<ResTable_package, kMinPackageSize>();
if (header == nullptr) {
- LOG(ERROR) << "Chunk RES_TABLE_PACKAGE_TYPE is too small.";
+ LOG(ERROR) << "RES_TABLE_PACKAGE_TYPE too small.";
return {};
}
@@ -529,7 +477,7 @@
if (header->header.headerSize >= sizeof(ResTable_package)) {
uint32_t type_id_offset = dtohl(header->typeIdOffset);
if (type_id_offset > std::numeric_limits<uint8_t>::max()) {
- LOG(ERROR) << "Type ID offset in RES_TABLE_PACKAGE_TYPE is too large.";
+ LOG(ERROR) << "RES_TABLE_PACKAGE_TYPE type ID offset too large.";
return {};
}
loaded_package->type_id_offset_ = static_cast<int>(type_id_offset);
@@ -560,7 +508,7 @@
status_t err = loaded_package->type_string_pool_.setTo(
child_chunk.header<ResStringPool_header>(), child_chunk.size());
if (err != NO_ERROR) {
- LOG(ERROR) << "Corrupt package type string pool.";
+ LOG(ERROR) << "RES_STRING_POOL_TYPE for types corrupt.";
return {};
}
} else if (pool_address == header_address + dtohl(header->keyStrings)) {
@@ -568,11 +516,11 @@
status_t err = loaded_package->key_string_pool_.setTo(
child_chunk.header<ResStringPool_header>(), child_chunk.size());
if (err != NO_ERROR) {
- LOG(ERROR) << "Corrupt package key string pool.";
+ LOG(ERROR) << "RES_STRING_POOL_TYPE for keys corrupt.";
return {};
}
} else {
- LOG(WARNING) << "Too many string pool chunks found in package.";
+ LOG(WARNING) << "Too many RES_STRING_POOL_TYPEs found in RES_TABLE_PACKAGE_TYPE.";
}
} break;
@@ -603,18 +551,18 @@
const ResTable_typeSpec* type_spec = child_chunk.header<ResTable_typeSpec>();
if (type_spec == nullptr) {
- LOG(ERROR) << "Chunk RES_TABLE_TYPE_SPEC_TYPE is too small.";
+ LOG(ERROR) << "RES_TABLE_TYPE_SPEC_TYPE too small.";
return {};
}
if (type_spec->id == 0) {
- LOG(ERROR) << "Chunk RES_TABLE_TYPE_SPEC_TYPE has invalid ID 0.";
+ LOG(ERROR) << "RES_TABLE_TYPE_SPEC_TYPE has invalid ID 0.";
return {};
}
if (loaded_package->type_id_offset_ + static_cast<int>(type_spec->id) >
std::numeric_limits<uint8_t>::max()) {
- LOG(ERROR) << "Chunk RES_TABLE_TYPE_SPEC_TYPE has out of range ID.";
+ LOG(ERROR) << "RES_TABLE_TYPE_SPEC_TYPE has out of range ID.";
return {};
}
@@ -626,12 +574,12 @@
// There can only be 2^16 entries in a type, because that is the ID
// space for entries (EEEE) in the resource ID 0xPPTTEEEE.
if (entry_count > std::numeric_limits<uint16_t>::max()) {
- LOG(ERROR) << "Too many entries in RES_TABLE_TYPE_SPEC_TYPE: " << entry_count << ".";
+ LOG(ERROR) << "RES_TABLE_TYPE_SPEC_TYPE has too many entries (" << entry_count << ").";
return {};
}
if (entry_count * sizeof(uint32_t) > chunk.data_size()) {
- LOG(ERROR) << "Chunk too small to hold entries in RES_TABLE_TYPE_SPEC_TYPE.";
+ LOG(ERROR) << "RES_TABLE_TYPE_SPEC_TYPE too small to hold entries.";
return {};
}
@@ -650,41 +598,32 @@
case RES_TABLE_TYPE_TYPE: {
const ResTable_type* type = child_chunk.header<ResTable_type, kResTableTypeMinSize>();
if (type == nullptr) {
- LOG(ERROR) << "Chunk RES_TABLE_TYPE_TYPE is too small.";
+ LOG(ERROR) << "RES_TABLE_TYPE_TYPE too small.";
return {};
}
- if (type->id == 0) {
- LOG(ERROR) << "Chunk RES_TABLE_TYPE_TYPE has invalid ID 0.";
+ if (!VerifyResTableType(type)) {
return {};
}
// Type chunks must be preceded by their TypeSpec chunks.
if (!types_builder || type->id - 1 != last_type_idx) {
- LOG(ERROR) << "Found RES_TABLE_TYPE_TYPE chunk without RES_TABLE_TYPE_SPEC_TYPE.";
+ LOG(ERROR) << "RES_TABLE_TYPE_TYPE found without preceding RES_TABLE_TYPE_SPEC_TYPE.";
return {};
}
- // Only verify the type if we haven't already failed verification.
- if (loaded_package->verified_) {
- if (!VerifyType(child_chunk)) {
- LOG(WARNING) << "Package failed verification, resource retrieval may be slower";
- loaded_package->verified_ = false;
- }
- }
-
types_builder->AddType(type);
} break;
case RES_TABLE_LIBRARY_TYPE: {
const ResTable_lib_header* lib = child_chunk.header<ResTable_lib_header>();
if (lib == nullptr) {
- LOG(ERROR) << "Chunk RES_TABLE_LIBRARY_TYPE is too small.";
+ LOG(ERROR) << "RES_TABLE_LIBRARY_TYPE too small.";
return {};
}
if (child_chunk.data_size() / sizeof(ResTable_lib_entry) < dtohl(lib->count)) {
- LOG(ERROR) << "Chunk too small to hold entries in RES_TABLE_LIBRARY_TYPE.";
+ LOG(ERROR) << "RES_TABLE_LIBRARY_TYPE too small to hold entries.";
return {};
}
@@ -751,7 +690,7 @@
const uint8_t type_id = get_type_id(resid);
const uint16_t entry_id = get_entry_id(resid);
- if (type_id == 0) {
+ if (UNLIKELY(type_id == 0)) {
LOG(ERROR) << base::StringPrintf("Invalid ID 0x%08x.", resid);
return false;
}
@@ -769,7 +708,7 @@
ATRACE_CALL();
const ResTable_header* header = chunk.header<ResTable_header>();
if (header == nullptr) {
- LOG(ERROR) << "Chunk RES_TABLE_TYPE is too small.";
+ LOG(ERROR) << "RES_TABLE_TYPE too small.";
return false;
}
@@ -788,11 +727,11 @@
status_t err = global_string_pool_.setTo(child_chunk.header<ResStringPool_header>(),
child_chunk.size());
if (err != NO_ERROR) {
- LOG(ERROR) << "Corrupt string pool.";
+ LOG(ERROR) << "RES_STRING_POOL_TYPE corrupt.";
return false;
}
} else {
- LOG(WARNING) << "Multiple string pool chunks found in resource table.";
+ LOG(WARNING) << "Multiple RES_STRING_POOL_TYPEs found in RES_TABLE_TYPE.";
}
break;