From 6587102ec24d1d11943e3e898a85479629077182 Mon Sep 17 00:00:00 2001 From: Jeremy Meyer Date: Tue, 12 Jul 2022 22:45:08 +0000 Subject: Store string frros in a string pool and copy that pool to the idmap Bug: 232940948 Test: manual with test app and added automated tests Change-Id: I30f5efefff7cf4fb0869188be78cc08e8efe7f04 --- libs/androidfw/LoadedArsc.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'libs/androidfw/LoadedArsc.cpp') diff --git a/libs/androidfw/LoadedArsc.cpp b/libs/androidfw/LoadedArsc.cpp index 35b6170fae5b..5b69cca2d747 100644 --- a/libs/androidfw/LoadedArsc.cpp +++ b/libs/androidfw/LoadedArsc.cpp @@ -820,6 +820,13 @@ bool LoadedArsc::LoadTable(const Chunk& chunk, const LoadedIdmap* loaded_idmap, return true; } +bool LoadedArsc::LoadStringPool(const LoadedIdmap* loaded_idmap) { + if (loaded_idmap != nullptr) { + global_string_pool_ = util::make_unique(loaded_idmap); + } + return true; +} + std::unique_ptr LoadedArsc::Load(incfs::map_ptr data, const size_t length, const LoadedIdmap* loaded_idmap, @@ -855,6 +862,16 @@ std::unique_ptr LoadedArsc::Load(incfs::map_ptr data, return loaded_arsc; } +std::unique_ptr LoadedArsc::Load(const LoadedIdmap* loaded_idmap) { + ATRACE_NAME("LoadedArsc::Load"); + + // Not using make_unique because the constructor is private. + std::unique_ptr loaded_arsc(new LoadedArsc()); + loaded_arsc->LoadStringPool(loaded_idmap); + return loaded_arsc; +} + + std::unique_ptr LoadedArsc::CreateEmpty() { return std::unique_ptr(new LoadedArsc()); } -- cgit v1.2.3-59-g8ed1b From 368cd19d031f5a0b219ea92531d11ccb0ee66c4d Mon Sep 17 00:00:00 2001 From: Eric Miao Date: Fri, 9 Sep 2022 15:46:14 -0700 Subject: androidfw: Add support for compact resource entries Bug: 237583012 Given the large number of simple resources such as strings in Android resources, their ResTable_entry and Res_value can be encoded together in a compact way. This allows a significant saving in both storage and memory footprint. The basic observations for simple resources are: * ResTable_entry.size will always be sizeof(ResTable_entry) unless it's a complex entry * ResTable_entry.key is unlikely to exceed 16-bit * ResTable_entry.flags only uses 3 bits for now * Res_value.size will always be sizeof(Res_value) Given the above, we could well encode the information into a compact/compatible structure. struct compact { uint16_t key; uint16_t flags; uint32_t data; }; The layout of this structure will allow maximum backward compatibility. e.g. the flags will be at the same offset, and a `dtohs((ResTable_entry *)entry->flags) & FLAG_COMPACT` would tell if this entry is a compact one or not. For a compact entry: struct compact *entry; entry_size == sizeof(*entry) entry_key == static_cast(dtohs(entry->key)) entry_flags == dtohs(entry->flags) & 0xff // low 8-bit data_type == dtohs(entry->flags) >> 8 // high 8-bit data_size == sizeof(Res_value) data_value == dtohl(entry->data) To allow minimum code change and backward compatibility, we change 'struct ResTable_entry' to 'union ResTable_entry', with an anonymous structure inside that's fully backward compatible. Thus, any existing reference such as: ResTable_entry *entry = ... if (dtohs(entry->flags) & ResTable_entry::FLAG_COMPLEX) ... would still work. However, special care needs to be taken after an entry is obtained, and when value needs to be extracted. A compact entry will not encode a complex value, and hence complex entries/values are handled the same way. Change-Id: I15d97a4f5e85fab28c075496f7f0cf6b1fcd73e3 --- libs/androidfw/AssetManager2.cpp | 38 +++----- libs/androidfw/LoadedArsc.cpp | 24 +++-- libs/androidfw/ResourceTypes.cpp | 46 ++++----- libs/androidfw/TypeWrappers.cpp | 6 +- libs/androidfw/include/androidfw/LoadedArsc.h | 8 +- libs/androidfw/include/androidfw/ResourceTypes.h | 93 ++++++++++++++++-- libs/androidfw/tests/TypeWrappers_test.cpp | 12 +-- tools/aapt/ResourceTable.cpp | 8 +- tools/aapt2/Debug.cpp | 39 ++++---- tools/aapt2/cmd/Link.h | 3 + tools/aapt2/format/binary/BinaryResourceParser.cpp | 15 +-- tools/aapt2/format/binary/ResEntryWriter.cpp | 100 ++++++++----------- tools/aapt2/format/binary/ResEntryWriter.h | 97 ++++++++++++++----- tools/aapt2/format/binary/ResEntryWriter_test.cpp | 107 +++++++++++++++------ tools/aapt2/format/binary/TableFlattener.cpp | 49 ++++++++-- tools/aapt2/format/binary/TableFlattener.h | 3 + 16 files changed, 407 insertions(+), 241 deletions(-) (limited to 'libs/androidfw/LoadedArsc.cpp') diff --git a/libs/androidfw/AssetManager2.cpp b/libs/androidfw/AssetManager2.cpp index 1381bdd6a50d..06ffb72d183b 100644 --- a/libs/androidfw/AssetManager2.cpp +++ b/libs/androidfw/AssetManager2.cpp @@ -43,28 +43,19 @@ namespace { using EntryValue = std::variant>; +/* NOTE: table_entry has been verified in LoadedPackage::GetEntryFromOffset(), + * and so access to ->value() and ->map_entry() are safe here + */ base::expected GetEntryValue( incfs::verified_map_ptr table_entry) { - const uint16_t entry_size = dtohs(table_entry->size); + const uint16_t entry_size = table_entry->size(); // Check if the entry represents a bag value. - if (entry_size >= sizeof(ResTable_map_entry) && - (dtohs(table_entry->flags) & ResTable_entry::FLAG_COMPLEX)) { - const auto map_entry = table_entry.convert(); - if (!map_entry) { - return base::unexpected(IOError::PAGES_MISSING); - } - return map_entry.verified(); + if (entry_size >= sizeof(ResTable_map_entry) && table_entry->is_complex()) { + return table_entry.convert().verified(); } - // The entry represents a non-bag value. - const auto entry_value = table_entry.offset(entry_size).convert(); - if (!entry_value) { - return base::unexpected(IOError::PAGES_MISSING); - } - Res_value value; - value.copyFrom_dtoh(entry_value.value()); - return value; + return table_entry->value(); } } // namespace @@ -814,17 +805,12 @@ base::expected AssetManager2::FindEntryInternal( return base::unexpected(std::nullopt); } - auto best_entry_result = LoadedPackage::GetEntryFromOffset(best_type, best_offset); - if (!best_entry_result.has_value()) { - return base::unexpected(best_entry_result.error()); - } - - const incfs::map_ptr best_entry = *best_entry_result; - if (!best_entry) { - return base::unexpected(IOError::PAGES_MISSING); + auto best_entry_verified = LoadedPackage::GetEntryFromOffset(best_type, best_offset); + if (!best_entry_verified.has_value()) { + return base::unexpected(best_entry_verified.error()); } - const auto entry = GetEntryValue(best_entry.verified()); + const auto entry = GetEntryValue(*best_entry_verified); if (!entry.has_value()) { return base::unexpected(entry.error()); } @@ -837,7 +823,7 @@ base::expected AssetManager2::FindEntryInternal( .package_name = &best_package->GetPackageName(), .type_string_ref = StringPoolRef(best_package->GetTypeStringPool(), best_type->id - 1), .entry_string_ref = StringPoolRef(best_package->GetKeyStringPool(), - best_entry->key.index), + (*best_entry_verified)->key()), .dynamic_ref_table = package_group.dynamic_ref_table.get(), }; } diff --git a/libs/androidfw/LoadedArsc.cpp b/libs/androidfw/LoadedArsc.cpp index 5b69cca2d747..e78f91ee3f46 100644 --- a/libs/androidfw/LoadedArsc.cpp +++ b/libs/androidfw/LoadedArsc.cpp @@ -107,8 +107,8 @@ static bool VerifyResTableType(incfs::map_ptr header) { return true; } -static base::expected VerifyResTableEntry( - incfs::verified_map_ptr type, uint32_t entry_offset) { +static base::expected, NullOrIOError> +VerifyResTableEntry(incfs::verified_map_ptr type, uint32_t entry_offset) { // Check that the offset is aligned. if (UNLIKELY(entry_offset & 0x03U)) { LOG(ERROR) << "Entry at offset " << entry_offset << " is not 4-byte aligned."; @@ -136,7 +136,7 @@ static base::expected VerifyResTableEntry( return base::unexpected(IOError::PAGES_MISSING); } - const size_t entry_size = dtohs(entry->size); + const size_t entry_size = entry->size(); if (UNLIKELY(entry_size < sizeof(entry.value()))) { LOG(ERROR) << "ResTable_entry size " << entry_size << " at offset " << entry_offset << " is too small."; @@ -149,6 +149,11 @@ static base::expected VerifyResTableEntry( return base::unexpected(std::nullopt); } + // If entry is compact, value is already encoded, and a compact entry + // cannot be a map_entry, we are done verifying + if (entry->is_compact()) + return entry.verified(); + if (entry_size < sizeof(ResTable_map_entry)) { // There needs to be room for one Res_value struct. if (UNLIKELY(entry_offset + entry_size > chunk_size - sizeof(Res_value))) { @@ -192,7 +197,7 @@ static base::expected VerifyResTableEntry( return base::unexpected(std::nullopt); } } - return {}; + return entry.verified(); } LoadedPackage::iterator::iterator(const LoadedPackage* lp, size_t ti, size_t ei) @@ -228,7 +233,7 @@ uint32_t LoadedPackage::iterator::operator*() const { entryIndex_); } -base::expected, NullOrIOError> LoadedPackage::GetEntry( +base::expected, NullOrIOError> LoadedPackage::GetEntry( incfs::verified_map_ptr type_chunk, uint16_t entry_index) { base::expected entry_offset = GetEntryOffset(type_chunk, entry_index); if (UNLIKELY(!entry_offset.has_value())) { @@ -297,13 +302,14 @@ base::expected LoadedPackage::GetEntryOffset( return value; } -base::expected, NullOrIOError> LoadedPackage::GetEntryFromOffset( - incfs::verified_map_ptr type_chunk, uint32_t offset) { +base::expected, NullOrIOError> +LoadedPackage::GetEntryFromOffset(incfs::verified_map_ptr type_chunk, + uint32_t offset) { auto valid = VerifyResTableEntry(type_chunk, offset); if (UNLIKELY(!valid.has_value())) { return base::unexpected(valid.error()); } - return type_chunk.offset(offset + dtohl(type_chunk->entriesStart)).convert(); + return valid; } base::expected LoadedPackage::CollectConfigurations( @@ -400,7 +406,7 @@ base::expected LoadedPackage::FindEntryByName( return base::unexpected(IOError::PAGES_MISSING); } - if (dtohl(entry->key.index) == static_cast(*key_idx)) { + if (entry->key() == static_cast(*key_idx)) { // The package ID will be overridden by the caller (due to runtime assignment of package // IDs for shared libraries). return make_resid(0x00, *type_idx + type_id_offset_ + 1, res_idx); diff --git a/libs/androidfw/ResourceTypes.cpp b/libs/androidfw/ResourceTypes.cpp index 46fbe7c4d981..aac52b46f909 100644 --- a/libs/androidfw/ResourceTypes.cpp +++ b/libs/androidfw/ResourceTypes.cpp @@ -4415,20 +4415,14 @@ ssize_t ResTable::getResource(uint32_t resID, Res_value* outValue, bool mayBeBag return err; } - if ((dtohs(entry.entry->flags) & ResTable_entry::FLAG_COMPLEX) != 0) { + if (entry.entry->map_entry()) { if (!mayBeBag) { ALOGW("Requesting resource 0x%08x failed because it is complex\n", resID); } return BAD_VALUE; } - const Res_value* value = reinterpret_cast( - reinterpret_cast(entry.entry) + entry.entry->size); - - outValue->size = dtohs(value->size); - outValue->res0 = value->res0; - outValue->dataType = value->dataType; - outValue->data = dtohl(value->data); + *outValue = entry.entry->value(); // The reference may be pointing to a resource in a shared library. These // references have build-time generated package IDs. These ids may not match @@ -4619,11 +4613,10 @@ ssize_t ResTable::getBagLocked(uint32_t resID, const bag_entry** outBag, return err; } - const uint16_t entrySize = dtohs(entry.entry->size); - const uint32_t parent = entrySize >= sizeof(ResTable_map_entry) - ? dtohl(((const ResTable_map_entry*)entry.entry)->parent.ident) : 0; - const uint32_t count = entrySize >= sizeof(ResTable_map_entry) - ? dtohl(((const ResTable_map_entry*)entry.entry)->count) : 0; + const uint16_t entrySize = entry.entry->size(); + const ResTable_map_entry* map_entry = entry.entry->map_entry(); + const uint32_t parent = map_entry ? dtohl(map_entry->parent.ident) : 0; + const uint32_t count = map_entry ? dtohl(map_entry->count) : 0; size_t N = count; @@ -4687,7 +4680,7 @@ ssize_t ResTable::getBagLocked(uint32_t resID, const bag_entry** outBag, // Now merge in the new attributes... size_t curOff = (reinterpret_cast(entry.entry) - reinterpret_cast(entry.type)) - + dtohs(entry.entry->size); + + entrySize; const ResTable_map* map; bag_entry* entries = (bag_entry*)(set+1); size_t curEntry = 0; @@ -5065,7 +5058,7 @@ uint32_t ResTable::findEntry(const PackageGroup* group, ssize_t typeIndex, const continue; } - if (dtohl(entry->key.index) == (size_t) *ei) { + if (entry->key() == (size_t) *ei) { uint32_t resId = Res_MAKEID(group->id - 1, typeIndex, iter.index()); if (outTypeSpecFlags) { Entry result; @@ -6579,8 +6572,8 @@ status_t ResTable::getEntry( const ResTable_entry* const entry = reinterpret_cast( reinterpret_cast(bestType) + bestOffset); - if (dtohs(entry->size) < sizeof(*entry)) { - ALOGW("ResTable_entry size 0x%x is too small", dtohs(entry->size)); + if (entry->size() < sizeof(*entry)) { + ALOGW("ResTable_entry size 0x%zx is too small", entry->size()); return BAD_TYPE; } @@ -6591,7 +6584,7 @@ status_t ResTable::getEntry( outEntry->specFlags = specFlags; outEntry->package = bestPackage; outEntry->typeStr = StringPoolRef(&bestPackage->typeStrings, actualTypeIndex - bestPackage->typeIdOffset); - outEntry->keyStr = StringPoolRef(&bestPackage->keyStrings, dtohl(entry->key.index)); + outEntry->keyStr = StringPoolRef(&bestPackage->keyStrings, entry->key()); } return NO_ERROR; } @@ -7662,7 +7655,7 @@ void ResTable::print(bool inclValues) const continue; } - uintptr_t esize = dtohs(ent->size); + uintptr_t esize = ent->size(); if ((esize&0x3) != 0) { printf("NON-INTEGER ResTable_entry SIZE: %p\n", (void *)esize); continue; @@ -7674,30 +7667,27 @@ void ResTable::print(bool inclValues) const } const Res_value* valuePtr = NULL; - const ResTable_map_entry* bagPtr = NULL; + const ResTable_map_entry* bagPtr = ent->map_entry(); Res_value value; - if ((dtohs(ent->flags)&ResTable_entry::FLAG_COMPLEX) != 0) { + if (bagPtr) { printf(""); - bagPtr = (const ResTable_map_entry*)ent; } else { - valuePtr = (const Res_value*) - (((const uint8_t*)ent) + esize); - value.copyFrom_dtoh(*valuePtr); + value = ent->value(); printf("t=0x%02x d=0x%08x (s=0x%04x r=0x%02x)", (int)value.dataType, (int)value.data, (int)value.size, (int)value.res0); } - if ((dtohs(ent->flags)&ResTable_entry::FLAG_PUBLIC) != 0) { + if (ent->flags() & ResTable_entry::FLAG_PUBLIC) { printf(" (PUBLIC)"); } printf("\n"); if (inclValues) { - if (valuePtr != NULL) { + if (bagPtr == NULL) { printf(" "); print_value(typeConfigs->package, value); - } else if (bagPtr != NULL) { + } else { const int N = dtohl(bagPtr->count); const uint8_t* baseMapPtr = (const uint8_t*)ent; size_t mapOffset = esize; diff --git a/libs/androidfw/TypeWrappers.cpp b/libs/androidfw/TypeWrappers.cpp index 647aa197a94d..2c39a81aca3e 100644 --- a/libs/androidfw/TypeWrappers.cpp +++ b/libs/androidfw/TypeWrappers.cpp @@ -91,11 +91,11 @@ const ResTable_entry* TypeVariant::iterator::operator*() const { if (reinterpret_cast(entry) > containerEnd - sizeof(*entry)) { ALOGE("Entry offset at index %u points outside the Type's boundaries", mIndex); return NULL; - } else if (reinterpret_cast(entry) + dtohs(entry->size) > containerEnd) { + } else if (reinterpret_cast(entry) + entry->size() > containerEnd) { ALOGE("Entry at index %u extends beyond Type's boundaries", mIndex); return NULL; - } else if (dtohs(entry->size) < sizeof(*entry)) { - ALOGE("Entry at index %u is too small (%u)", mIndex, dtohs(entry->size)); + } else if (entry->size() < sizeof(*entry)) { + ALOGE("Entry at index %u is too small (%zu)", mIndex, entry->size()); return NULL; } return entry; diff --git a/libs/androidfw/include/androidfw/LoadedArsc.h b/libs/androidfw/include/androidfw/LoadedArsc.h index e45963950b04..79d962829046 100644 --- a/libs/androidfw/include/androidfw/LoadedArsc.h +++ b/libs/androidfw/include/androidfw/LoadedArsc.h @@ -166,14 +166,14 @@ class LoadedPackage { base::expected FindEntryByName(const std::u16string& type_name, const std::u16string& entry_name) const; - static base::expected, NullOrIOError> GetEntry( - incfs::verified_map_ptr type_chunk, uint16_t entry_index); + static base::expected, NullOrIOError> + GetEntry(incfs::verified_map_ptr type_chunk, uint16_t entry_index); static base::expected GetEntryOffset( incfs::verified_map_ptr type_chunk, uint16_t entry_index); - static base::expected, NullOrIOError> GetEntryFromOffset( - incfs::verified_map_ptr type_chunk, uint32_t offset); + static base::expected, NullOrIOError> + GetEntryFromOffset(incfs::verified_map_ptr type_chunk, uint32_t offset); // Returns the string pool where type names are stored. const ResStringPool* GetTypeStringPool() const { diff --git a/libs/androidfw/include/androidfw/ResourceTypes.h b/libs/androidfw/include/androidfw/ResourceTypes.h index 24628cd36ba5..42d8cbeb8dfd 100644 --- a/libs/androidfw/include/androidfw/ResourceTypes.h +++ b/libs/androidfw/include/androidfw/ResourceTypes.h @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -1490,6 +1491,8 @@ union ResTable_sparseTypeEntry { static_assert(sizeof(ResTable_sparseTypeEntry) == sizeof(uint32_t), "ResTable_sparseTypeEntry must be 4 bytes in size"); +struct ResTable_map_entry; + /** * This is the beginning of information about an entry in the resource * table. It holds the reference to the name of this entry, and is @@ -1497,12 +1500,11 @@ static_assert(sizeof(ResTable_sparseTypeEntry) == sizeof(uint32_t), * * A Res_value structure, if FLAG_COMPLEX is -not- set. * * An array of ResTable_map structures, if FLAG_COMPLEX is set. * These supply a set of name/value mappings of data. + * * If FLAG_COMPACT is set, this entry is a compact entry for + * simple values only */ -struct ResTable_entry +union ResTable_entry { - // Number of bytes in this structure. - uint16_t size; - enum { // If set, this is a complex entry, holding a set of name/value // mappings. It is followed by an array of ResTable_map structures. @@ -1514,18 +1516,91 @@ struct ResTable_entry // resources of the same name/type. This is only useful during // linking with other resource tables. FLAG_WEAK = 0x0004, + // If set, this is a compact entry with data type and value directly + // encoded in the this entry, see ResTable_entry::compact + FLAG_COMPACT = 0x0008, }; - uint16_t flags; - - // Reference into ResTable_package::keyStrings identifying this entry. - struct ResStringPool_ref key; + + struct Full { + // Number of bytes in this structure. + uint16_t size; + + uint16_t flags; + + // Reference into ResTable_package::keyStrings identifying this entry. + struct ResStringPool_ref key; + } full; + + /* A compact entry is indicated by FLAG_COMPACT, with flags at the same + * offset as a normal entry. This is only for simple data values where + * + * - size for entry or value can be inferred (both being 8 bytes). + * - key index is encoded in 16-bit + * - dataType is encoded as the higher 8-bit of flags + * - data is encoded directly in this entry + */ + struct Compact { + uint16_t key; + uint16_t flags; + uint32_t data; + } compact; + + uint16_t flags() const { return dtohs(full.flags); }; + bool is_compact() const { return flags() & FLAG_COMPACT; } + bool is_complex() const { return flags() & FLAG_COMPLEX; } + + size_t size() const { + return is_compact() ? sizeof(ResTable_entry) : dtohs(this->full.size); + } + + uint32_t key() const { + return is_compact() ? dtohs(this->compact.key) : dtohl(this->full.key.index); + } + + /* Always verify the memory associated with this entry and its value + * before calling value() or map_entry() + */ + Res_value value() const { + Res_value v; + if (is_compact()) { + v.size = sizeof(Res_value); + v.res0 = 0; + v.data = dtohl(this->compact.data); + v.dataType = dtohs(compact.flags) >> 8; + } else { + auto vaddr = reinterpret_cast(this) + dtohs(this->full.size); + auto value = reinterpret_cast(vaddr); + v.size = dtohs(value->size); + v.res0 = value->res0; + v.data = dtohl(value->data); + v.dataType = value->dataType; + } + return v; + } + + const ResTable_map_entry* map_entry() const { + return is_complex() && !is_compact() ? + reinterpret_cast(this) : nullptr; + } }; +/* Make sure size of ResTable_entry::Full and ResTable_entry::Compact + * be the same as ResTable_entry. This is to allow iteration of entries + * to work in either cases. + * + * The offset of flags must be at the same place for both structures, + * to ensure the correct reading to decide whether this is a full entry + * or a compact entry. + */ +static_assert(sizeof(ResTable_entry) == sizeof(ResTable_entry::Full)); +static_assert(sizeof(ResTable_entry) == sizeof(ResTable_entry::Compact)); +static_assert(offsetof(ResTable_entry, full.flags) == offsetof(ResTable_entry, compact.flags)); + /** * Extended form of a ResTable_entry for map entries, defining a parent map * resource from which to inherit values. */ -struct ResTable_map_entry : public ResTable_entry +struct ResTable_map_entry : public ResTable_entry::Full { // Resource identifier of the parent mapping, or 0 if there is none. // This is always treated as a TYPE_DYNAMIC_REFERENCE. diff --git a/libs/androidfw/tests/TypeWrappers_test.cpp b/libs/androidfw/tests/TypeWrappers_test.cpp index d69abe5d0f11..aba3ab3a06a2 100644 --- a/libs/androidfw/tests/TypeWrappers_test.cpp +++ b/libs/androidfw/tests/TypeWrappers_test.cpp @@ -37,8 +37,8 @@ void* createTypeData() { offsets[0] = 0; ResTable_entry e1; memset(&e1, 0, sizeof(e1)); - e1.size = sizeof(e1); - e1.key.index = 0; + e1.full.size = sizeof(e1); + e1.full.key.index = 0; t.header.size += sizeof(e1); Res_value v1; @@ -50,8 +50,8 @@ void* createTypeData() { offsets[2] = sizeof(e1) + sizeof(v1); ResTable_entry e2; memset(&e2, 0, sizeof(e2)); - e2.size = sizeof(e2); - e2.key.index = 1; + e2.full.size = sizeof(e2); + e2.full.key.index = 1; t.header.size += sizeof(e2); Res_value v2; @@ -83,7 +83,7 @@ TEST(TypeVariantIteratorTest, shouldIterateOverTypeWithoutErrors) { TypeVariant::iterator iter = v.beginEntries(); ASSERT_EQ(uint32_t(0), iter.index()); ASSERT_TRUE(NULL != *iter); - ASSERT_EQ(uint32_t(0), iter->key.index); + ASSERT_EQ(uint32_t(0), iter->full.key.index); ASSERT_NE(v.endEntries(), iter); iter++; @@ -96,7 +96,7 @@ TEST(TypeVariantIteratorTest, shouldIterateOverTypeWithoutErrors) { ASSERT_EQ(uint32_t(2), iter.index()); ASSERT_TRUE(NULL != *iter); - ASSERT_EQ(uint32_t(1), iter->key.index); + ASSERT_EQ(uint32_t(1), iter->full.key.index); ASSERT_NE(v.endEntries(), iter); iter++; diff --git a/tools/aapt/ResourceTable.cpp b/tools/aapt/ResourceTable.cpp index 47750fc11a6e..4e597fb3b30a 100644 --- a/tools/aapt/ResourceTable.cpp +++ b/tools/aapt/ResourceTable.cpp @@ -3743,15 +3743,15 @@ ssize_t ResourceTable::Entry::flatten(Bundle* /* bundle */, const sp& size_t amt = 0; ResTable_entry header; memset(&header, 0, sizeof(header)); - header.size = htods(sizeof(header)); + header.full.size = htods(sizeof(header)); const type ty = mType; if (ty == TYPE_BAG) { - header.flags |= htods(header.FLAG_COMPLEX); + header.full.flags |= htods(header.FLAG_COMPLEX); } if (isPublic) { - header.flags |= htods(header.FLAG_PUBLIC); + header.full.flags |= htods(header.FLAG_PUBLIC); } - header.key.index = htodl(mNameIndex); + header.full.key.index = htodl(mNameIndex); if (ty != TYPE_BAG) { status_t err = data->writeData(&header, sizeof(header)); if (err != NO_ERROR) { diff --git a/tools/aapt2/Debug.cpp b/tools/aapt2/Debug.cpp index f9e52b491413..df878899fa28 100644 --- a/tools/aapt2/Debug.cpp +++ b/tools/aapt2/Debug.cpp @@ -687,32 +687,32 @@ class ChunkPrinter { continue; } - printer_->Print((entry->flags & ResTable_entry::FLAG_COMPLEX) ? "[ResTable_map_entry]" - : "[ResTable_entry]"); + if (entry->is_complex()) { + printer_->Print("[ResTable_map_entry]"); + } else if (entry->is_compact()) { + printer_->Print("[ResTable_entry_compact]"); + } else { + printer_->Print("[ResTable_entry]"); + } + printer_->Print(StringPrintf(" id: 0x%04x", it.index())); printer_->Print(StringPrintf( - " name: %s", - android::util::GetString(key_pool_, android::util::DeviceToHost32(entry->key.index)) - .c_str())); - printer_->Print( - StringPrintf(" keyIndex: %u", android::util::DeviceToHost32(entry->key.index))); - printer_->Print(StringPrintf(" size: %u", android::util::DeviceToHost32(entry->size))); - printer_->Print(StringPrintf(" flags: 0x%04x", android::util::DeviceToHost32(entry->flags))); + " name: %s", android::util::GetString(key_pool_, entry->key()).c_str())); + printer_->Print(StringPrintf(" keyIndex: %u", entry->key())); + printer_->Print(StringPrintf(" size: %zu", entry->size())); + printer_->Print(StringPrintf(" flags: 0x%04x", entry->flags())); printer_->Indent(); - if (entry->flags & ResTable_entry::FLAG_COMPLEX) { - auto map_entry = (const ResTable_map_entry*)entry; - printer_->Print( - StringPrintf(" count: 0x%04x", android::util::DeviceToHost32(map_entry->count))); + if (auto map_entry = entry->map_entry()) { + uint32_t map_entry_count = android::util::DeviceToHost32(map_entry->count); + printer_->Print(StringPrintf(" count: 0x%04x", map_entry_count)); printer_->Print(StringPrintf(" parent: 0x%08x\n", android::util::DeviceToHost32(map_entry->parent.ident))); // Print the name and value mappings - auto maps = (const ResTable_map*)((const uint8_t*)entry + - android::util::DeviceToHost32(entry->size)); - for (size_t i = 0, count = android::util::DeviceToHost32(map_entry->count); i < count; - i++) { + auto maps = (const ResTable_map*)((const uint8_t*)entry + entry->size()); + for (size_t i = 0; i < map_entry_count; i++) { PrintResValue(&(maps[i].value), config, type); printer_->Print(StringPrintf( @@ -725,9 +725,8 @@ class ChunkPrinter { printer_->Print("\n"); // Print the value of the entry - auto value = - (const Res_value*)((const uint8_t*)entry + android::util::DeviceToHost32(entry->size)); - PrintResValue(value, config, type); + Res_value value = entry->value(); + PrintResValue(&value, config, type); } printer_->Undent(); diff --git a/tools/aapt2/cmd/Link.h b/tools/aapt2/cmd/Link.h index cffcdf2f1710..5fdfb66bdf4e 100644 --- a/tools/aapt2/cmd/Link.h +++ b/tools/aapt2/cmd/Link.h @@ -159,6 +159,9 @@ class LinkCommand : public Command { AddOptionalSwitch("--enable-sparse-encoding", "This decreases APK size at the cost of resource retrieval performance.", &options_.use_sparse_encoding); + AddOptionalSwitch("--enable-compact-entries", + "This decreases APK size by using compact resource entries for simple data types.", + &options_.table_flattener_options.use_compact_entries); AddOptionalSwitch("-x", "Legacy flag that specifies to use the package identifier 0x01.", &legacy_x_flag_); AddOptionalSwitch("-z", "Require localization of strings marked 'suggested'.", diff --git a/tools/aapt2/format/binary/BinaryResourceParser.cpp b/tools/aapt2/format/binary/BinaryResourceParser.cpp index d9e379db84b7..82918629f1f4 100644 --- a/tools/aapt2/format/binary/BinaryResourceParser.cpp +++ b/tools/aapt2/format/binary/BinaryResourceParser.cpp @@ -384,21 +384,16 @@ bool BinaryResourceParser::ParseType(const ResourceTablePackage* package, continue; } - const ResourceName name( - package->name, *parsed_type, - android::util::GetString(key_pool_, android::util::DeviceToHost32(entry->key.index))); + const ResourceName name(package->name, *parsed_type, + android::util::GetString(key_pool_, entry->key())); const ResourceId res_id(package_id, type->id, static_cast(it.index())); std::unique_ptr resource_value; - if (entry->flags & ResTable_entry::FLAG_COMPLEX) { - const ResTable_map_entry* mapEntry = static_cast(entry); - + if (auto mapEntry = entry->map_entry()) { // TODO(adamlesinski): Check that the entry count is valid. resource_value = ParseMapEntry(name, config, mapEntry); } else { - const Res_value* value = - (const Res_value*)((const uint8_t*)entry + android::util::DeviceToHost32(entry->size)); - resource_value = ParseValue(name, config, *value); + resource_value = ParseValue(name, config, entry->value()); } if (!resource_value) { @@ -419,7 +414,7 @@ bool BinaryResourceParser::ParseType(const ResourceTablePackage* package, .SetId(res_id, OnIdConflict::CREATE_ENTRY) .SetAllowMangled(true); - if (entry->flags & ResTable_entry::FLAG_PUBLIC) { + if (entry->flags() & ResTable_entry::FLAG_PUBLIC) { Visibility visibility{Visibility::Level::kPublic}; auto spec_flags = entry_type_spec_flags_.find(res_id); diff --git a/tools/aapt2/format/binary/ResEntryWriter.cpp b/tools/aapt2/format/binary/ResEntryWriter.cpp index 8832c24842ee..9dc205f4c1ba 100644 --- a/tools/aapt2/format/binary/ResEntryWriter.cpp +++ b/tools/aapt2/format/binary/ResEntryWriter.cpp @@ -24,11 +24,6 @@ namespace aapt { -using android::BigBuffer; -using android::Res_value; -using android::ResTable_entry; -using android::ResTable_map; - struct less_style_entries { bool operator()(const Style::Entry* a, const Style::Entry* b) const { if (a->key.id) { @@ -189,26 +184,40 @@ class MapFlattenVisitor : public ConstValueVisitor { }; template -void WriteEntry(const FlatEntry* entry, T* out_result) { +void WriteEntry(const FlatEntry* entry, T* out_result, bool compact = false) { static_assert(std::is_same_v || std::is_same_v, "T must be ResTable_entry or ResTable_entry_ext"); ResTable_entry* out_entry = (ResTable_entry*)out_result; + uint16_t flags = 0; + if (entry->entry->visibility.level == Visibility::Level::kPublic) { - out_entry->flags |= ResTable_entry::FLAG_PUBLIC; + flags |= ResTable_entry::FLAG_PUBLIC; } if (entry->value->IsWeak()) { - out_entry->flags |= ResTable_entry::FLAG_WEAK; + flags |= ResTable_entry::FLAG_WEAK; } if constexpr (std::is_same_v) { - out_entry->flags |= ResTable_entry::FLAG_COMPLEX; + flags |= ResTable_entry::FLAG_COMPLEX; } - out_entry->flags = android::util::HostToDevice16(out_entry->flags); - out_entry->key.index = android::util::HostToDevice32(entry->entry_key); - out_entry->size = android::util::HostToDevice16(sizeof(T)); + if (!compact) { + out_entry->full.flags = android::util::HostToDevice16(flags); + out_entry->full.key.index = android::util::HostToDevice32(entry->entry_key); + out_entry->full.size = android::util::HostToDevice16(sizeof(T)); + } else { + Res_value value; + CHECK(entry->entry_key < 0xffffu) << "cannot encode key in 16-bit"; + CHECK(compact && (std::is_same_v)) << "cannot encode complex entry"; + CHECK(ValueCast(entry->value)->Flatten(&value)) << "flatten failed"; + + flags |= ResTable_entry::FLAG_COMPACT | (value.dataType << 8); + out_entry->compact.flags = android::util::HostToDevice16(flags); + out_entry->compact.key = android::util::HostToDevice16(entry->entry_key); + out_entry->compact.data = value.data; + } } int32_t WriteMapToBuffer(const FlatEntry* map_entry, BigBuffer* buffer) { @@ -222,57 +231,26 @@ int32_t WriteMapToBuffer(const FlatEntry* map_entry, BigBuffer* buffer) { return offset; } -void WriteItemToPair(const FlatEntry* item_entry, ResEntryValuePair* out_pair) { - static_assert(sizeof(ResEntryValuePair) == sizeof(ResTable_entry) + sizeof(Res_value), - "ResEntryValuePair must not have padding between entry and value."); - - WriteEntry(item_entry, &out_pair->entry); - - CHECK(ValueCast(item_entry->value)->Flatten(&out_pair->value)) << "flatten failed"; - out_pair->value.size = android::util::HostToDevice16(sizeof(out_pair->value)); -} - -int32_t SequentialResEntryWriter::WriteMap(const FlatEntry* entry) { - return WriteMapToBuffer(entry, entries_buffer_); -} - -int32_t SequentialResEntryWriter::WriteItem(const FlatEntry* entry) { - int32_t offset = entries_buffer_->size(); - auto* out_pair = entries_buffer_->NextBlock(); - WriteItemToPair(entry, out_pair); - return offset; -} - -std::size_t ResEntryValuePairContentHasher::operator()(const ResEntryValuePairRef& ref) const { - return android::JenkinsHashMixBytes(0, ref.ptr, sizeof(ResEntryValuePair)); -} - -bool ResEntryValuePairContentEqualTo::operator()(const ResEntryValuePairRef& a, - const ResEntryValuePairRef& b) const { - return std::memcmp(a.ptr, b.ptr, sizeof(ResEntryValuePair)) == 0; -} +template +std::pair WriteItemToBuffer(const FlatEntry* item_entry, BigBuffer* buffer) { + int32_t offset = buffer->size(); + T* out_entry = buffer->NextBlock(); -int32_t DeduplicateItemsResEntryWriter::WriteMap(const FlatEntry* entry) { - return WriteMapToBuffer(entry, entries_buffer_); + if constexpr (compact_entry) { + WriteEntry(item_entry, out_entry, true); + } else { + WriteEntry(item_entry, &out_entry->entry); + CHECK(ValueCast(item_entry->value)->Flatten(&out_entry->value)) << "flatten failed"; + out_entry->value.size = android::util::HostToDevice16(sizeof(out_entry->value)); + } + return {offset, out_entry}; } -int32_t DeduplicateItemsResEntryWriter::WriteItem(const FlatEntry* entry) { - int32_t initial_offset = entries_buffer_->size(); - - auto* out_pair = entries_buffer_->NextBlock(); - WriteItemToPair(entry, out_pair); +// explicitly specialize both versions +template std::pair*> WriteItemToBuffer( + const FlatEntry* item_entry, BigBuffer* buffer); - auto ref = ResEntryValuePairRef{*out_pair}; - auto [it, inserted] = entry_offsets.insert({ref, initial_offset}); - if (inserted) { - // If inserted just return a new offset as this is a first time we store - // this entry. - return initial_offset; - } - // If not inserted this means that this is a duplicate, backup allocated block to the buffer - // and return offset of previously stored entry. - entries_buffer_->BackUp(sizeof(ResEntryValuePair)); - return it->second; -} +template std::pair*> WriteItemToBuffer( + const FlatEntry* item_entry, BigBuffer* buffer); -} // namespace aapt \ No newline at end of file +} // namespace aapt diff --git a/tools/aapt2/format/binary/ResEntryWriter.h b/tools/aapt2/format/binary/ResEntryWriter.h index a36ceec2613b..c11598ec12f7 100644 --- a/tools/aapt2/format/binary/ResEntryWriter.h +++ b/tools/aapt2/format/binary/ResEntryWriter.h @@ -27,6 +27,11 @@ namespace aapt { +using android::BigBuffer; +using android::Res_value; +using android::ResTable_entry; +using android::ResTable_map; + struct FlatEntry { const ResourceTableEntryView* entry; const Value* value; @@ -39,28 +44,42 @@ struct FlatEntry { // We introduce this structure for ResEntryWriter to a have single allocation using // BigBuffer::NextBlock which allows to return it back with BigBuffer::Backup. struct ResEntryValuePair { - android::ResTable_entry entry; - android::Res_value value; + ResTable_entry entry; + Res_value value; }; -// References ResEntryValuePair object stored in BigBuffer used as a key in std::unordered_map. -// Allows access to memory address where ResEntryValuePair is stored. -union ResEntryValuePairRef { - const std::reference_wrapper pair; +static_assert(sizeof(ResEntryValuePair) == sizeof(ResTable_entry) + sizeof(Res_value), + "ResEntryValuePair must not have padding between entry and value."); + +template +using ResEntryValue = std::conditional_t; + +// References ResEntryValue object stored in BigBuffer used as a key in std::unordered_map. +// Allows access to memory address where ResEntryValue is stored. +template +union ResEntryValueRef { + using T = ResEntryValue; + const std::reference_wrapper ref; const u_char* ptr; - explicit ResEntryValuePairRef(const ResEntryValuePair& ref) : pair(ref) { + explicit ResEntryValueRef(const T& rev) : ref(rev) { } }; -// Hasher which computes hash of ResEntryValuePair using its bytes representation in memory. -struct ResEntryValuePairContentHasher { - std::size_t operator()(const ResEntryValuePairRef& ref) const; +// Hasher which computes hash of ResEntryValue using its bytes representation in memory. +struct ResEntryValueContentHasher { + template + std::size_t operator()(const R& ref) const { + return android::JenkinsHashMixBytes(0, ref.ptr, sizeof(typename R::T)); + } }; // Equaler which compares ResEntryValuePairs using theirs bytes representation in memory. -struct ResEntryValuePairContentEqualTo { - bool operator()(const ResEntryValuePairRef& a, const ResEntryValuePairRef& b) const; +struct ResEntryValueContentEqualTo { + template + bool operator()(const R& a, const R& b) const { + return std::memcmp(a.ptr, b.ptr, sizeof(typename R::T)) == 0; + } }; // Base class that allows to write FlatEntries into entries_buffer. @@ -79,9 +98,9 @@ class ResEntryWriter { } protected: - ResEntryWriter(android::BigBuffer* entries_buffer) : entries_buffer_(entries_buffer) { + ResEntryWriter(BigBuffer* entries_buffer) : entries_buffer_(entries_buffer) { } - android::BigBuffer* entries_buffer_; + BigBuffer* entries_buffer_; virtual int32_t WriteItem(const FlatEntry* entry) = 0; @@ -91,18 +110,29 @@ class ResEntryWriter { DISALLOW_COPY_AND_ASSIGN(ResEntryWriter); }; +int32_t WriteMapToBuffer(const FlatEntry* map_entry, BigBuffer* buffer); + +template > +std::pair WriteItemToBuffer(const FlatEntry* item_entry, BigBuffer* buffer); + // ResEntryWriter which writes FlatEntries sequentially into entries_buffer. // Next entry is always written right after previous one in the buffer. +template class SequentialResEntryWriter : public ResEntryWriter { public: - explicit SequentialResEntryWriter(android::BigBuffer* entries_buffer) + explicit SequentialResEntryWriter(BigBuffer* entries_buffer) : ResEntryWriter(entries_buffer) { } ~SequentialResEntryWriter() override = default; - int32_t WriteItem(const FlatEntry* entry) override; + int32_t WriteItem(const FlatEntry* entry) override { + auto result = WriteItemToBuffer(entry, entries_buffer_); + return result.first; + } - int32_t WriteMap(const FlatEntry* entry) override; + int32_t WriteMap(const FlatEntry* entry) override { + return WriteMapToBuffer(entry, entries_buffer_); + } private: DISALLOW_COPY_AND_ASSIGN(SequentialResEntryWriter); @@ -111,25 +141,44 @@ class SequentialResEntryWriter : public ResEntryWriter { // ResEntryWriter that writes only unique entry and value pairs into entries_buffer. // Next entry is written into buffer only if there is no entry with the same bytes representation // in memory written before. Otherwise returns offset of already written entry. +template class DeduplicateItemsResEntryWriter : public ResEntryWriter { public: - explicit DeduplicateItemsResEntryWriter(android::BigBuffer* entries_buffer) + explicit DeduplicateItemsResEntryWriter(BigBuffer* entries_buffer) : ResEntryWriter(entries_buffer) { } ~DeduplicateItemsResEntryWriter() override = default; - int32_t WriteItem(const FlatEntry* entry) override; + int32_t WriteItem(const FlatEntry* entry) override { + const auto& [offset, out_entry] = WriteItemToBuffer(entry, entries_buffer_); + + auto [it, inserted] = entry_offsets.insert({Ref{*out_entry}, offset}); + if (inserted) { + // If inserted just return a new offset as this is a first time we store + // this entry + return offset; + } - int32_t WriteMap(const FlatEntry* entry) override; + // If not inserted this means that this is a duplicate, backup allocated block to the buffer + // and return offset of previously stored entry + entries_buffer_->BackUp(sizeof(*out_entry)); + return it->second; + } + + int32_t WriteMap(const FlatEntry* entry) override { + return WriteMapToBuffer(entry, entries_buffer_); + } private: DISALLOW_COPY_AND_ASSIGN(DeduplicateItemsResEntryWriter); - std::unordered_map - entry_offsets; + using Ref = ResEntryValueRef; + using Map = std::unordered_map; + Map entry_offsets; }; } // namespace aapt -#endif \ No newline at end of file +#endif diff --git a/tools/aapt2/format/binary/ResEntryWriter_test.cpp b/tools/aapt2/format/binary/ResEntryWriter_test.cpp index 56ca1332ec5d..4cb17c33e64a 100644 --- a/tools/aapt2/format/binary/ResEntryWriter_test.cpp +++ b/tools/aapt2/format/binary/ResEntryWriter_test.cpp @@ -56,14 +56,28 @@ TEST_F(SequentialResEntryWriterTest, WriteEntriesOneByOne) { .AddSimple("com.app.test:id/id3", ResourceId(0x7f010002)) .Build(); - BigBuffer out(512); - SequentialResEntryWriter writer(&out); - auto offsets = WriteAllEntries(table->GetPartitionedView(), writer); - - std::vector expected_offsets{0, sizeof(ResEntryValuePair), - 2 * sizeof(ResEntryValuePair)}; - EXPECT_EQ(out.size(), 3 * sizeof(ResEntryValuePair)); - EXPECT_EQ(offsets, expected_offsets); + { + BigBuffer out(512); + SequentialResEntryWriter writer(&out); + auto offsets = WriteAllEntries(table->GetPartitionedView(), writer); + + std::vector expected_offsets{0, sizeof(ResEntryValuePair), + 2 * sizeof(ResEntryValuePair)}; + EXPECT_EQ(out.size(), 3 * sizeof(ResEntryValuePair)); + EXPECT_EQ(offsets, expected_offsets); + } + + { + /* expect a compact entry to only take sizeof(ResTable_entry) */ + BigBuffer out(512); + SequentialResEntryWriter writer(&out); + auto offsets = WriteAllEntries(table->GetPartitionedView(), writer); + + std::vector expected_offsets{0, sizeof(ResTable_entry), + 2 * sizeof(ResTable_entry)}; + EXPECT_EQ(out.size(), 3 * sizeof(ResTable_entry)); + EXPECT_EQ(offsets, expected_offsets); + } }; TEST_F(SequentialResEntryWriterTest, WriteMapEntriesOneByOne) { @@ -83,13 +97,26 @@ TEST_F(SequentialResEntryWriterTest, WriteMapEntriesOneByOne) { .AddValue("com.app.test:array/arr2", std::move(array2)) .Build(); - BigBuffer out(512); - SequentialResEntryWriter writer(&out); - auto offsets = WriteAllEntries(table->GetPartitionedView(), writer); + { + BigBuffer out(512); + SequentialResEntryWriter writer(&out); + auto offsets = WriteAllEntries(table->GetPartitionedView(), writer); - std::vector expected_offsets{0, sizeof(ResTable_entry_ext) + 2 * sizeof(ResTable_map)}; - EXPECT_EQ(out.size(), 2 * (sizeof(ResTable_entry_ext) + 2 * sizeof(ResTable_map))); - EXPECT_EQ(offsets, expected_offsets); + std::vector expected_offsets{0, sizeof(ResTable_entry_ext) + 2 * sizeof(ResTable_map)}; + EXPECT_EQ(out.size(), 2 * (sizeof(ResTable_entry_ext) + 2 * sizeof(ResTable_map))); + EXPECT_EQ(offsets, expected_offsets); + } + + { + /* compact_entry should have no impact to map items */ + BigBuffer out(512); + SequentialResEntryWriter writer(&out); + auto offsets = WriteAllEntries(table->GetPartitionedView(), writer); + + std::vector expected_offsets{0, sizeof(ResTable_entry_ext) + 2 * sizeof(ResTable_map)}; + EXPECT_EQ(out.size(), 2 * (sizeof(ResTable_entry_ext) + 2 * sizeof(ResTable_map))); + EXPECT_EQ(offsets, expected_offsets); + } }; TEST_F(DeduplicateItemsResEntryWriterTest, DeduplicateItemEntries) { @@ -100,13 +127,26 @@ TEST_F(DeduplicateItemsResEntryWriterTest, DeduplicateItemEntries) { .AddSimple("com.app.test:id/id3", ResourceId(0x7f010002)) .Build(); - BigBuffer out(512); - DeduplicateItemsResEntryWriter writer(&out); - auto offsets = WriteAllEntries(table->GetPartitionedView(), writer); + { + BigBuffer out(512); + DeduplicateItemsResEntryWriter writer(&out); + auto offsets = WriteAllEntries(table->GetPartitionedView(), writer); - std::vector expected_offsets{0, 0, 0}; - EXPECT_EQ(out.size(), sizeof(ResEntryValuePair)); - EXPECT_EQ(offsets, expected_offsets); + std::vector expected_offsets{0, 0, 0}; + EXPECT_EQ(out.size(), sizeof(ResEntryValuePair)); + EXPECT_EQ(offsets, expected_offsets); + } + + { + /* expect a compact entry to only take sizeof(ResTable_entry) */ + BigBuffer out(512); + DeduplicateItemsResEntryWriter writer(&out); + auto offsets = WriteAllEntries(table->GetPartitionedView(), writer); + + std::vector expected_offsets{0, 0, 0}; + EXPECT_EQ(out.size(), sizeof(ResTable_entry)); + EXPECT_EQ(offsets, expected_offsets); + } }; TEST_F(DeduplicateItemsResEntryWriterTest, WriteMapEntriesOneByOne) { @@ -126,13 +166,26 @@ TEST_F(DeduplicateItemsResEntryWriterTest, WriteMapEntriesOneByOne) { .AddValue("com.app.test:array/arr2", std::move(array2)) .Build(); - BigBuffer out(512); - DeduplicateItemsResEntryWriter writer(&out); - auto offsets = WriteAllEntries(table->GetPartitionedView(), writer); + { + BigBuffer out(512); + DeduplicateItemsResEntryWriter writer(&out); + auto offsets = WriteAllEntries(table->GetPartitionedView(), writer); - std::vector expected_offsets{0, sizeof(ResTable_entry_ext) + 2 * sizeof(ResTable_map)}; - EXPECT_EQ(out.size(), 2 * (sizeof(ResTable_entry_ext) + 2 * sizeof(ResTable_map))); - EXPECT_EQ(offsets, expected_offsets); -}; + std::vector expected_offsets{0, sizeof(ResTable_entry_ext) + 2 * sizeof(ResTable_map)}; + EXPECT_EQ(out.size(), 2 * (sizeof(ResTable_entry_ext) + 2 * sizeof(ResTable_map))); + EXPECT_EQ(offsets, expected_offsets); + } + + { + /* compact_entry should have no impact to map items */ + BigBuffer out(512); + DeduplicateItemsResEntryWriter writer(&out); + auto offsets = WriteAllEntries(table->GetPartitionedView(), writer); + + std::vector expected_offsets{0, sizeof(ResTable_entry_ext) + 2 * sizeof(ResTable_map)}; + EXPECT_EQ(out.size(), 2 * (sizeof(ResTable_entry_ext) + 2 * sizeof(ResTable_map))); + EXPECT_EQ(offsets, expected_offsets); + } + }; } // namespace aapt \ No newline at end of file diff --git a/tools/aapt2/format/binary/TableFlattener.cpp b/tools/aapt2/format/binary/TableFlattener.cpp index 318b8b6dcd31..c431730b5410 100644 --- a/tools/aapt2/format/binary/TableFlattener.cpp +++ b/tools/aapt2/format/binary/TableFlattener.cpp @@ -67,7 +67,9 @@ class PackageFlattener { public: PackageFlattener(IAaptContext* context, const ResourceTablePackageView& package, const std::map* shared_libs, - SparseEntriesMode sparse_entries, bool collapse_key_stringpool, + SparseEntriesMode sparse_entries, + bool compact_entries, + bool collapse_key_stringpool, const std::set& name_collapse_exemptions, bool deduplicate_entry_values) : context_(context), @@ -75,6 +77,7 @@ class PackageFlattener { package_(package), shared_libs_(shared_libs), sparse_entries_(sparse_entries), + compact_entries_(compact_entries), collapse_key_stringpool_(collapse_key_stringpool), name_collapse_exemptions_(name_collapse_exemptions), deduplicate_entry_values_(deduplicate_entry_values) { @@ -135,6 +138,33 @@ class PackageFlattener { private: DISALLOW_COPY_AND_ASSIGN(PackageFlattener); + // Use compact entries only if + // 1) it is enabled, and that + // 2) the entries will be accessed on platforms U+, and + // 3) all entry keys can be encoded in 16 bits + bool UseCompactEntries(const ConfigDescription& config, std::vector* entries) const { + return compact_entries_ && + (context_->GetMinSdkVersion() > SDK_TIRAMISU || config.sdkVersion > SDK_TIRAMISU) && + std::none_of(entries->cbegin(), entries->cend(), + [](const auto& e) { return e.entry_key >= std::numeric_limits::max(); }); + } + + std::unique_ptr GetResEntryWriter(bool dedup, bool compact, BigBuffer* buffer) { + if (dedup) { + if (compact) { + return std::make_unique>(buffer); + } else { + return std::make_unique>(buffer); + } + } else { + if (compact) { + return std::make_unique>(buffer); + } else { + return std::make_unique>(buffer); + } + } + } + bool FlattenConfig(const ResourceTableTypeView& type, const ConfigDescription& config, const size_t num_total_entries, std::vector* entries, BigBuffer* buffer) { @@ -150,15 +180,11 @@ class PackageFlattener { std::vector offsets; offsets.resize(num_total_entries, 0xffffffffu); + bool compact_entry = UseCompactEntries(config, entries); + android::BigBuffer values_buffer(512); - std::variant - writer_variant; - ResEntryWriter* res_entry_writer; - if (deduplicate_entry_values_) { - res_entry_writer = &writer_variant.emplace(&values_buffer); - } else { - res_entry_writer = &writer_variant.emplace(&values_buffer); - } + auto res_entry_writer = GetResEntryWriter(deduplicate_entry_values_, + compact_entry, &values_buffer); for (FlatEntry& flat_entry : *entries) { CHECK(static_cast(flat_entry.entry->id.value()) < num_total_entries); @@ -512,6 +538,7 @@ class PackageFlattener { const ResourceTablePackageView package_; const std::map* shared_libs_; SparseEntriesMode sparse_entries_; + bool compact_entries_; android::StringPool type_pool_; android::StringPool key_pool_; bool collapse_key_stringpool_; @@ -568,7 +595,9 @@ bool TableFlattener::Consume(IAaptContext* context, ResourceTable* table) { } PackageFlattener flattener(context, package, &table->included_packages_, - options_.sparse_entries, options_.collapse_key_stringpool, + options_.sparse_entries, + options_.use_compact_entries, + options_.collapse_key_stringpool, options_.name_collapse_exemptions, options_.deduplicate_entry_values); if (!flattener.FlattenPackage(&package_buffer)) { diff --git a/tools/aapt2/format/binary/TableFlattener.h b/tools/aapt2/format/binary/TableFlattener.h index 6151b7e33b3f..35254ba24e53 100644 --- a/tools/aapt2/format/binary/TableFlattener.h +++ b/tools/aapt2/format/binary/TableFlattener.h @@ -44,6 +44,9 @@ struct TableFlattenerOptions { // as a sparse map of entry ID and offset to actual data. SparseEntriesMode sparse_entries = SparseEntriesMode::Disabled; + // When true, use compact entries for simple data + bool use_compact_entries = false; + // When true, the key string pool in the final ResTable // is collapsed to a single entry. All resource entries // have name indices that point to this single value -- cgit v1.2.3-59-g8ed1b From a1f2bce0e56185d8a3ddbbb75cf9daacdcb5a3d2 Mon Sep 17 00:00:00 2001 From: Eric Miao Date: Mon, 12 Sep 2022 11:37:37 -0700 Subject: androidfw: Add support for 16-bit entry offsets Bug: 237583012 Most offsets to the entries can be well encoded in 16-bit, and given entries are 4-byte aligned, this gives us a range of entry offsets from 0x00000 to 0xfffe * 4u, with 0xffffu to represent ResTable_type::NO_ENTRY. For now, 16-bit entry offset will be enabled only when: * all the entry offsets can be represented in 16-bit * --enable-compact-entries switch is turned on Change-Id: I1c815c052aa5fba6eab2529434d31d7714c13694 --- libs/androidfw/LoadedArsc.cpp | 61 ++++++++++++++++-------- libs/androidfw/ResourceTypes.cpp | 19 ++++++-- libs/androidfw/TypeWrappers.cpp | 7 ++- libs/androidfw/include/androidfw/ResourceTypes.h | 9 ++++ tools/aapt2/format/binary/TableFlattener.cpp | 23 +++++++-- 5 files changed, 90 insertions(+), 29 deletions(-) (limited to 'libs/androidfw/LoadedArsc.cpp') diff --git a/libs/androidfw/LoadedArsc.cpp b/libs/androidfw/LoadedArsc.cpp index e78f91ee3f46..386f718208b3 100644 --- a/libs/androidfw/LoadedArsc.cpp +++ b/libs/androidfw/LoadedArsc.cpp @@ -88,7 +88,9 @@ static bool VerifyResTableType(incfs::map_ptr header) { // Make sure that there is enough room for the entry offsets. const size_t offsets_offset = dtohs(header->header.headerSize); const size_t entries_offset = dtohl(header->entriesStart); - const size_t offsets_length = sizeof(uint32_t) * entry_count; + const size_t offsets_length = header->flags & ResTable_type::FLAG_OFFSET16 + ? sizeof(uint16_t) * entry_count + : sizeof(uint32_t) * entry_count; if (offsets_offset > entries_offset || entries_offset - offsets_offset < offsets_length) { LOG(ERROR) << "RES_TABLE_TYPE_TYPE entry offsets overlap actual entry data."; @@ -247,14 +249,13 @@ base::expected LoadedPackage::GetEntryOffset( // The configuration matches and is better than the previous selection. // Find the entry value if it exists for this configuration. const size_t entry_count = dtohl(type_chunk->entryCount); - const size_t offsets_offset = dtohs(type_chunk->header.headerSize); + const auto offsets = type_chunk.offset(dtohs(type_chunk->header.headerSize)); // Check if there is the desired entry in this type. if (type_chunk->flags & ResTable_type::FLAG_SPARSE) { // This is encoded as a sparse map, so perform a binary search. bool error = false; - auto sparse_indices = type_chunk.offset(offsets_offset) - .convert().iterator(); + auto sparse_indices = offsets.convert().iterator(); auto sparse_indices_end = sparse_indices + entry_count; auto result = std::lower_bound(sparse_indices, sparse_indices_end, entry_index, [&error](const incfs::map_ptr& entry, @@ -289,17 +290,26 @@ base::expected LoadedPackage::GetEntryOffset( return base::unexpected(std::nullopt); } - const auto entry_offset_ptr = type_chunk.offset(offsets_offset).convert() + entry_index; - if (UNLIKELY(!entry_offset_ptr)) { - return base::unexpected(IOError::PAGES_MISSING); + uint32_t result; + + if (type_chunk->flags & ResTable_type::FLAG_OFFSET16) { + const auto entry_offset_ptr = offsets.convert() + entry_index; + if (UNLIKELY(!entry_offset_ptr)) { + return base::unexpected(IOError::PAGES_MISSING); + } + result = offset_from16(entry_offset_ptr.value()); + } else { + const auto entry_offset_ptr = offsets.convert() + entry_index; + if (UNLIKELY(!entry_offset_ptr)) { + return base::unexpected(IOError::PAGES_MISSING); + } + result = dtohl(entry_offset_ptr.value()); } - const uint32_t value = dtohl(entry_offset_ptr.value()); - if (value == ResTable_type::NO_ENTRY) { + if (result == ResTable_type::NO_ENTRY) { return base::unexpected(std::nullopt); } - - return value; + return result; } base::expected, NullOrIOError> @@ -382,24 +392,35 @@ base::expected LoadedPackage::FindEntryByName( for (const auto& type_entry : type_spec->type_entries) { const incfs::verified_map_ptr& type = type_entry.type; - size_t entry_count = dtohl(type->entryCount); - for (size_t entry_idx = 0; entry_idx < entry_count; entry_idx++) { - auto entry_offset_ptr = type.offset(dtohs(type->header.headerSize)).convert() + - entry_idx; - if (!entry_offset_ptr) { - return base::unexpected(IOError::PAGES_MISSING); - } + const size_t entry_count = dtohl(type->entryCount); + const auto entry_offsets = type.offset(dtohs(type->header.headerSize)); + for (size_t entry_idx = 0; entry_idx < entry_count; entry_idx++) { uint32_t offset; uint16_t res_idx; if (type->flags & ResTable_type::FLAG_SPARSE) { - auto sparse_entry = entry_offset_ptr.convert(); + auto sparse_entry = entry_offsets.convert() + entry_idx; + if (!sparse_entry) { + return base::unexpected(IOError::PAGES_MISSING); + } offset = dtohs(sparse_entry->offset) * 4u; res_idx = dtohs(sparse_entry->idx); + } else if (type->flags & ResTable_type::FLAG_OFFSET16) { + auto entry = entry_offsets.convert() + entry_idx; + if (!entry) { + return base::unexpected(IOError::PAGES_MISSING); + } + offset = offset_from16(entry.value()); + res_idx = entry_idx; } else { - offset = dtohl(entry_offset_ptr.value()); + auto entry = entry_offsets.convert() + entry_idx; + if (!entry) { + return base::unexpected(IOError::PAGES_MISSING); + } + offset = dtohl(entry.value()); res_idx = entry_idx; } + if (offset != ResTable_type::NO_ENTRY) { auto entry = type.offset(dtohl(type->entriesStart) + offset).convert(); if (!entry) { diff --git a/libs/androidfw/ResourceTypes.cpp b/libs/androidfw/ResourceTypes.cpp index aac52b46f909..ba9354607c8e 100644 --- a/libs/androidfw/ResourceTypes.cpp +++ b/libs/androidfw/ResourceTypes.cpp @@ -6521,8 +6521,12 @@ status_t ResTable::getEntry( // Entry does not exist. continue; } - - thisOffset = dtohl(eindex[realEntryIndex]); + if (thisType->flags & ResTable_type::FLAG_OFFSET16) { + auto eindex16 = reinterpret_cast(eindex); + thisOffset = offset_from16(eindex16[realEntryIndex]); + } else { + thisOffset = dtohl(eindex[realEntryIndex]); + } } if (thisOffset == ResTable_type::NO_ENTRY) { @@ -7574,6 +7578,9 @@ void ResTable::print(bool inclValues) const if (type->flags & ResTable_type::FLAG_SPARSE) { printf(" [sparse]"); } + if (type->flags & ResTable_type::FLAG_OFFSET16) { + printf(" [offset16]"); + } } printf(":\n"); @@ -7605,7 +7612,13 @@ void ResTable::print(bool inclValues) const thisOffset = static_cast(dtohs(entry->offset)) * 4u; } else { entryId = entryIndex; - thisOffset = dtohl(eindex[entryIndex]); + if (type->flags & ResTable_type::FLAG_OFFSET16) { + const auto eindex16 = + reinterpret_cast(eindex); + thisOffset = offset_from16(eindex16[entryIndex]); + } else { + thisOffset = dtohl(eindex[entryIndex]); + } if (thisOffset == ResTable_type::NO_ENTRY) { continue; } diff --git a/libs/androidfw/TypeWrappers.cpp b/libs/androidfw/TypeWrappers.cpp index 2c39a81aca3e..70d14a11830e 100644 --- a/libs/androidfw/TypeWrappers.cpp +++ b/libs/androidfw/TypeWrappers.cpp @@ -59,7 +59,9 @@ const ResTable_entry* TypeVariant::iterator::operator*() const { + dtohl(type->header.size); const uint32_t* const entryIndices = reinterpret_cast( reinterpret_cast(type) + dtohs(type->header.headerSize)); - if (reinterpret_cast(entryIndices) + (sizeof(uint32_t) * entryCount) > containerEnd) { + const size_t indexSize = type->flags & ResTable_type::FLAG_OFFSET16 ? + sizeof(uint16_t) : sizeof(uint32_t); + if (reinterpret_cast(entryIndices) + (indexSize * entryCount) > containerEnd) { ALOGE("Type's entry indices extend beyond its boundaries"); return NULL; } @@ -73,6 +75,9 @@ const ResTable_entry* TypeVariant::iterator::operator*() const { } entryOffset = static_cast(dtohs(ResTable_sparseTypeEntry{*iter}.offset)) * 4u; + } else if (type->flags & ResTable_type::FLAG_OFFSET16) { + auto entryIndices16 = reinterpret_cast(entryIndices); + entryOffset = offset_from16(entryIndices16[mIndex]); } else { entryOffset = dtohl(entryIndices[mIndex]); } diff --git a/libs/androidfw/include/androidfw/ResourceTypes.h b/libs/androidfw/include/androidfw/ResourceTypes.h index 42d8cbeb8dfd..d588b23504f0 100644 --- a/libs/androidfw/include/androidfw/ResourceTypes.h +++ b/libs/androidfw/include/androidfw/ResourceTypes.h @@ -1448,6 +1448,10 @@ struct ResTable_type // Mark any types that use this with a v26 qualifier to prevent runtime issues on older // platforms. FLAG_SPARSE = 0x01, + + // If set, the offsets to the entries are encoded in 16-bit, real_offset = offset * 4u + // An 16-bit offset of 0xffffu means a NO_ENTRY + FLAG_OFFSET16 = 0x02, }; uint8_t flags; @@ -1464,6 +1468,11 @@ struct ResTable_type ResTable_config config; }; +// Convert a 16-bit offset to 32-bit if FLAG_OFFSET16 is set +static inline uint32_t offset_from16(uint16_t off16) { + return dtohs(off16) == 0xffffu ? ResTable_type::NO_ENTRY : dtohs(off16) * 4u; +} + // The minimum size required to read any version of ResTable_type. constexpr size_t kResTableTypeMinSize = sizeof(ResTable_type) - sizeof(ResTable_config) + sizeof(ResTable_config::size); diff --git a/tools/aapt2/format/binary/TableFlattener.cpp b/tools/aapt2/format/binary/TableFlattener.cpp index c431730b5410..f19223411232 100644 --- a/tools/aapt2/format/binary/TableFlattener.cpp +++ b/tools/aapt2/format/binary/TableFlattener.cpp @@ -16,6 +16,7 @@ #include "format/binary/TableFlattener.h" +#include #include #include #include @@ -191,6 +192,9 @@ class PackageFlattener { offsets[flat_entry.entry->id.value()] = res_entry_writer->Write(&flat_entry); } + // whether the offsets can be represented in 2 bytes + bool short_offsets = (values_buffer.size() / 4u) < std::numeric_limits::max(); + bool sparse_encode = sparse_entries_ == SparseEntriesMode::Enabled || sparse_entries_ == SparseEntriesMode::Forced; @@ -203,8 +207,7 @@ class PackageFlattener { } // Only sparse encode if the offsets are representable in 2 bytes. - sparse_encode = - sparse_encode && (values_buffer.size() / 4u) <= std::numeric_limits::max(); + sparse_encode = sparse_encode && short_offsets; // Only sparse encode if the ratio of populated entries to total entries is below some // threshold. @@ -226,12 +229,22 @@ class PackageFlattener { } } else { type_header->entryCount = android::util::HostToDevice32(num_total_entries); - uint32_t* indices = type_writer.NextBlock(num_total_entries); - for (size_t i = 0; i < num_total_entries; i++) { - indices[i] = android::util::HostToDevice32(offsets[i]); + if (compact_entry && short_offsets) { + // use 16-bit offset only when compact_entry is true + type_header->flags |= ResTable_type::FLAG_OFFSET16; + uint16_t* indices = type_writer.NextBlock(num_total_entries); + for (size_t i = 0; i < num_total_entries; i++) { + indices[i] = android::util::HostToDevice16(offsets[i] / 4u); + } + } else { + uint32_t* indices = type_writer.NextBlock(num_total_entries); + for (size_t i = 0; i < num_total_entries; i++) { + indices[i] = android::util::HostToDevice32(offsets[i]); + } } } + type_writer.buffer()->Align4(); type_header->entriesStart = android::util::HostToDevice32(type_writer.size()); type_writer.buffer()->AppendBuffer(std::move(values_buffer)); type_writer.Finish(); -- cgit v1.2.3-59-g8ed1b From 6e8c6039254716dca878e84567d3a4e480d47a22 Mon Sep 17 00:00:00 2001 From: Yurii Zubrytskyi Date: Mon, 14 Nov 2022 23:45:58 -0800 Subject: [res] Change staged alias container to vector Sorted vector is more efficient than a map for all cases but if it gets modified a lot. Here we never modify it after loading Bug: 237583012 Test: build + boot Change-Id: I7d9f1fe38bd8a6d722b9cacaa7034abcdb02d743 --- libs/androidfw/LoadedArsc.cpp | 29 +++++++++++++++++---------- libs/androidfw/include/androidfw/LoadedArsc.h | 6 +++--- 2 files changed, 21 insertions(+), 14 deletions(-) (limited to 'libs/androidfw/LoadedArsc.cpp') diff --git a/libs/androidfw/LoadedArsc.cpp b/libs/androidfw/LoadedArsc.cpp index 386f718208b3..193c8650b2be 100644 --- a/libs/androidfw/LoadedArsc.cpp +++ b/libs/androidfw/LoadedArsc.cpp @@ -645,16 +645,15 @@ std::unique_ptr LoadedPackage::Load(const Chunk& chunk, } std::string name; - util::ReadUtf16StringFromDevice(overlayable->name, arraysize(overlayable->name), &name); + util::ReadUtf16StringFromDevice(overlayable->name, std::size(overlayable->name), &name); std::string actor; - util::ReadUtf16StringFromDevice(overlayable->actor, arraysize(overlayable->actor), &actor); - - if (loaded_package->overlayable_map_.find(name) != - loaded_package->overlayable_map_.end()) { - LOG(ERROR) << "Multiple blocks with the same name '" << name << "'."; + util::ReadUtf16StringFromDevice(overlayable->actor, std::size(overlayable->actor), &actor); + auto [it, inserted] = + loaded_package->overlayable_map_.emplace(name, actor); + if (!inserted) { + LOG(ERROR) << "Multiple blocks with the same name '" << it->first << "'."; return {}; } - loaded_package->overlayable_map_.emplace(name, actor); // Iterate over the overlayable policy chunks contained within the overlayable chunk data ChunkIterator overlayable_iter(child_chunk.data_ptr(), child_chunk.data_size()); @@ -736,6 +735,7 @@ std::unique_ptr LoadedPackage::Load(const Chunk& chunk, const auto entry_end = entry_begin + dtohl(lib_alias->count); std::unordered_set finalized_ids; finalized_ids.reserve(entry_end - entry_begin); + loaded_package->alias_id_map_.reserve(entry_end - entry_begin); for (auto entry_iter = entry_begin; entry_iter != entry_end; ++entry_iter) { if (!entry_iter) { LOG(ERROR) << "NULL ResTable_staged_alias_entry record??"; @@ -749,13 +749,20 @@ std::unique_ptr LoadedPackage::Load(const Chunk& chunk, } auto staged_id = dtohl(entry_iter->stagedResId); - auto [_, success] = loaded_package->alias_id_map_.emplace(staged_id, finalized_id); - if (!success) { + loaded_package->alias_id_map_.emplace_back(staged_id, finalized_id); + } + + std::sort(loaded_package->alias_id_map_.begin(), loaded_package->alias_id_map_.end(), + [](auto&& l, auto&& r) { return l.first < r.first; }); + const auto duplicate_it = + std::adjacent_find(loaded_package->alias_id_map_.begin(), + loaded_package->alias_id_map_.end(), + [](auto&& l, auto&& r) { return l.first == r.first; }); + if (duplicate_it != loaded_package->alias_id_map_.end()) { LOG(ERROR) << StringPrintf("Repeated staged resource id '%08x' in staged aliases.", - staged_id); + duplicate_it->first); return {}; } - } } break; default: diff --git a/libs/androidfw/include/androidfw/LoadedArsc.h b/libs/androidfw/include/androidfw/LoadedArsc.h index 79d962829046..4ab004299500 100644 --- a/libs/androidfw/include/androidfw/LoadedArsc.h +++ b/libs/androidfw/include/androidfw/LoadedArsc.h @@ -275,7 +275,7 @@ class LoadedPackage { return overlayable_map_; } - const std::map& GetAliasResourceIdMap() const { + const std::vector>& GetAliasResourceIdMap() const { return alias_id_map_; } @@ -295,8 +295,8 @@ class LoadedPackage { std::unordered_map type_specs_; ByteBucketArray resource_ids_; std::vector dynamic_package_map_; - std::vector>> overlayable_infos_; - std::map alias_id_map_; + std::vector>> overlayable_infos_; + std::vector> alias_id_map_; // A map of overlayable name to actor std::unordered_map overlayable_map_; -- cgit v1.2.3-59-g8ed1b From 9d22537ee2ef820919e03cc1bf3024ae000194d9 Mon Sep 17 00:00:00 2001 From: Yurii Zubrytskyi Date: Tue, 29 Nov 2022 11:12:18 -0800 Subject: [res] Change OverlayableInfo to hold string views No need to copy the strings as we keep them in separate containers anyway, may just reference them Bug: 237583012 Test: build + boot + UTs Change-Id: I2853cd3c992ca482ed6988e0c52a4037b158d999 --- cmds/idmap2/libidmap2/ResourceMapping.cpp | 2 +- libs/androidfw/AssetManager2.cpp | 2 +- libs/androidfw/LoadedArsc.cpp | 12 ++++++------ libs/androidfw/include/androidfw/LoadedArsc.h | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) (limited to 'libs/androidfw/LoadedArsc.cpp') diff --git a/cmds/idmap2/libidmap2/ResourceMapping.cpp b/cmds/idmap2/libidmap2/ResourceMapping.cpp index bb31c11d629c..b2300cea3a68 100644 --- a/cmds/idmap2/libidmap2/ResourceMapping.cpp +++ b/cmds/idmap2/libidmap2/ResourceMapping.cpp @@ -89,7 +89,7 @@ Result CheckOverlayable(const TargetResourceContainer& target, // If the overlay supplies a target overlayable name, the resource must belong to the // overlayable defined with the specified name to be overlaid. return Error(R"( android:targetName "%s" does not match overlayable name "%s")", - overlay_info.target_name.c_str(), (*overlayable_info)->name.c_str()); + overlay_info.target_name.c_str(), (*overlayable_info)->name.data()); } // Enforce policy restrictions if the resource is declared as overlayable. diff --git a/libs/androidfw/AssetManager2.cpp b/libs/androidfw/AssetManager2.cpp index 1b327018f681..cc7e8714c0ac 100644 --- a/libs/androidfw/AssetManager2.cpp +++ b/libs/androidfw/AssetManager2.cpp @@ -374,7 +374,7 @@ bool AssetManager2::GetOverlayablesToString(android::StringPiece package_name, const std::string name = ToFormattedResourceString(*res_name); output.append(base::StringPrintf( "resource='%s' overlayable='%s' actor='%s' policy='0x%08x'\n", - name.c_str(), info->name.c_str(), info->actor.c_str(), info->policy_flags)); + name.c_str(), info->name.data(), info->actor.data(), info->policy_flags)); } } } diff --git a/libs/androidfw/LoadedArsc.cpp b/libs/androidfw/LoadedArsc.cpp index 193c8650b2be..c0fdfe25da21 100644 --- a/libs/androidfw/LoadedArsc.cpp +++ b/libs/androidfw/LoadedArsc.cpp @@ -648,10 +648,11 @@ std::unique_ptr LoadedPackage::Load(const Chunk& chunk, util::ReadUtf16StringFromDevice(overlayable->name, std::size(overlayable->name), &name); std::string actor; util::ReadUtf16StringFromDevice(overlayable->actor, std::size(overlayable->actor), &actor); - auto [it, inserted] = - loaded_package->overlayable_map_.emplace(name, actor); + auto [name_to_actor_it, inserted] = + loaded_package->overlayable_map_.emplace(std::move(name), std::move(actor)); if (!inserted) { - LOG(ERROR) << "Multiple blocks with the same name '" << it->first << "'."; + LOG(ERROR) << "Multiple blocks with the same name '" + << name_to_actor_it->first << "'."; return {}; } @@ -668,7 +669,6 @@ std::unique_ptr LoadedPackage::Load(const Chunk& chunk, LOG(ERROR) << "RES_TABLE_OVERLAYABLE_POLICY_TYPE too small."; return {}; } - if ((overlayable_child_chunk.data_size() / sizeof(ResTable_ref)) < dtohl(policy_header->entry_count)) { LOG(ERROR) << "RES_TABLE_OVERLAYABLE_POLICY_TYPE too small to hold entries."; @@ -690,8 +690,8 @@ std::unique_ptr LoadedPackage::Load(const Chunk& chunk, // Add the pairing of overlayable properties and resource ids to the package OverlayableInfo overlayable_info { - .name = name, - .actor = actor, + .name = name_to_actor_it->first, + .actor = name_to_actor_it->second, .policy_flags = policy_header->policy_flags }; loaded_package->overlayable_infos_.emplace_back(std::move(overlayable_info), std::move(ids)); diff --git a/libs/androidfw/include/androidfw/LoadedArsc.h b/libs/androidfw/include/androidfw/LoadedArsc.h index 4ab004299500..4d12885ad291 100644 --- a/libs/androidfw/include/androidfw/LoadedArsc.h +++ b/libs/androidfw/include/androidfw/LoadedArsc.h @@ -99,8 +99,8 @@ enum : package_property_t { }; struct OverlayableInfo { - std::string name; - std::string actor; + std::string_view name; + std::string_view actor; uint32_t policy_flags; }; -- cgit v1.2.3-59-g8ed1b