idmap2: add debug information to idmap file format
Add a new variable length string to the idmap file format. This string will
hold debug information like fulfilled policies and any warnings triggered while
generating the file.
Bump the idmap version to 3.
Adjust the idmap header definition in ResourceType.h to take the new string
into account.
Example debug info:
$ idmap2 create \
--target-apk-path frameworks/base/cmds/idmap2/tests/data/target/target.apk \
--overlay-apk-path frameworks/base/cmds/idmap2/tests/data/overlay/overlay.apk \
--idmap-path /tmp/a.idmap \
--policy public \
--policy oem
$ idmap2 dump --idmap-path /tmp/a.idmap
target apk path : frameworks/base/cmds/idmap2/tests/data/target/target.apk
overlay apk path : frameworks/base/cmds/idmap2/tests/data/overlay/overlay.apk
I fulfilled_policies=oem|public enforce_overlayable=true
W failed to find resource "integer/not_in_target" in target resources
0x7f010000 -> 0x7f010000 integer/int1
0x7f02000c -> 0x7f020000 string/str1
[...]
$ idmap2 dump --idmap-path /tmp/a.idmap --verbose
00000000: 504d4449 magic
00000004: 00000003 version
00000008: 76a20829 target crc
0000000c: c054fb26 overlay crc
00000010: ........ target path: frameworks/base/cmds/idmap2/tests/data/target/target.apk
00000110: ........ overlay path: frameworks/base/cmds/idmap2/tests/data/overlay/overlay.apk
00000210: ........ debug info: ...
00000294: 7f target package id
00000295: 7f overlay package id
[...]
Also, tell cpplint to accept non-const references as function parameters:
they make more sense as out-parameters than pointers that are assumed to
be non-null.
Also, switch to regular expressions in the RawPrintVisitorTests: no more
manual fixups of the stream offsets! Tell cpplint that the <regex>
header is OK to use.
Bug: 140790707
Test: idmap2_tests
Change-Id: Ib94684a3b4001240321801e21af8e132fbcf6609
diff --git a/cmds/idmap2/libidmap2/BinaryStreamVisitor.cpp b/cmds/idmap2/libidmap2/BinaryStreamVisitor.cpp
index 3b0940a..362dcb3 100644
--- a/cmds/idmap2/libidmap2/BinaryStreamVisitor.cpp
+++ b/cmds/idmap2/libidmap2/BinaryStreamVisitor.cpp
@@ -42,13 +42,21 @@
stream_.write(reinterpret_cast<char*>(&x), sizeof(uint32_t));
}
-void BinaryStreamVisitor::WriteString(const StringPiece& value) {
+void BinaryStreamVisitor::WriteString256(const StringPiece& value) {
char buf[kIdmapStringLength];
memset(buf, 0, sizeof(buf));
memcpy(buf, value.data(), std::min(value.size(), sizeof(buf)));
stream_.write(buf, sizeof(buf));
}
+void BinaryStreamVisitor::WriteString(const std::string& value) {
+ // pad with null to nearest word boundary; include at least one terminating null
+ size_t padding_size = 4 - (value.size() % 4);
+ Write32(value.size() + padding_size);
+ stream_.write(value.c_str(), value.size());
+ stream_.write("\0\0\0\0", padding_size);
+}
+
void BinaryStreamVisitor::visit(const Idmap& idmap ATTRIBUTE_UNUSED) {
// nothing to do
}
@@ -58,8 +66,9 @@
Write32(header.GetVersion());
Write32(header.GetTargetCrc());
Write32(header.GetOverlayCrc());
- WriteString(header.GetTargetPath());
- WriteString(header.GetOverlayPath());
+ WriteString256(header.GetTargetPath());
+ WriteString256(header.GetOverlayPath());
+ WriteString(header.GetDebugInfo());
}
void BinaryStreamVisitor::visit(const IdmapData& data) {
diff --git a/cmds/idmap2/libidmap2/Idmap.cpp b/cmds/idmap2/libidmap2/Idmap.cpp
index 5cb91d7..7f2cd959 100644
--- a/cmds/idmap2/libidmap2/Idmap.cpp
+++ b/cmds/idmap2/libidmap2/Idmap.cpp
@@ -70,7 +70,7 @@
}
// a string is encoded as a kIdmapStringLength char array; the array is always null-terminated
-bool WARN_UNUSED ReadString(std::istream& stream, char out[kIdmapStringLength]) {
+bool WARN_UNUSED ReadString256(std::istream& stream, char out[kIdmapStringLength]) {
char buf[kIdmapStringLength];
memset(buf, 0, sizeof(buf));
if (!stream.read(buf, sizeof(buf))) {
@@ -83,6 +83,23 @@
return true;
}
+Result<std::string> ReadString(std::istream& stream) {
+ uint32_t size;
+ if (!Read32(stream, &size)) {
+ return Error("failed to read string size");
+ }
+ if (size == 0) {
+ return std::string("");
+ }
+ std::string buf(size, '\0');
+ if (!stream.read(buf.data(), size)) {
+ return Error("failed to read string of size %u", size);
+ }
+ // buf is guaranteed to be null terminated (with enough nulls to end on a word boundary)
+ buf.resize(strlen(buf.c_str()));
+ return buf;
+}
+
Result<uint32_t> GetCrc(const ZipFile& zip) {
const Result<uint32_t> a = zip.Crc("resources.arsc");
const Result<uint32_t> b = zip.Crc("AndroidManifest.xml");
@@ -98,11 +115,17 @@
if (!Read32(stream, &idmap_header->magic_) || !Read32(stream, &idmap_header->version_) ||
!Read32(stream, &idmap_header->target_crc_) || !Read32(stream, &idmap_header->overlay_crc_) ||
- !ReadString(stream, idmap_header->target_path_) ||
- !ReadString(stream, idmap_header->overlay_path_)) {
+ !ReadString256(stream, idmap_header->target_path_) ||
+ !ReadString256(stream, idmap_header->overlay_path_)) {
return nullptr;
}
+ auto debug_str = ReadString(stream);
+ if (!debug_str) {
+ return nullptr;
+ }
+ idmap_header->debug_info_ = std::move(*debug_str);
+
return std::move(idmap_header);
}
@@ -307,17 +330,15 @@
memset(header->overlay_path_, 0, sizeof(header->overlay_path_));
memcpy(header->overlay_path_, overlay_apk_path.data(), overlay_apk_path.size());
- std::unique_ptr<Idmap> idmap(new Idmap());
- idmap->header_ = std::move(header);
-
auto overlay_info = utils::ExtractOverlayManifestInfo(overlay_apk_path);
if (!overlay_info) {
return overlay_info.GetError();
}
+ LogInfo log_info;
auto resource_mapping =
ResourceMapping::FromApkAssets(target_apk_assets, overlay_apk_assets, *overlay_info,
- fulfilled_policies, enforce_overlayable);
+ fulfilled_policies, enforce_overlayable, log_info);
if (!resource_mapping) {
return resource_mapping.GetError();
}
@@ -327,7 +348,11 @@
return idmap_data.GetError();
}
+ std::unique_ptr<Idmap> idmap(new Idmap());
+ header->debug_info_ = log_info.GetString();
+ idmap->header_ = std::move(header);
idmap->data_.push_back(std::move(*idmap_data));
+
return {std::move(idmap)};
}
diff --git a/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp b/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp
index a662aa5..f6e1345 100644
--- a/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp
+++ b/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp
@@ -34,6 +34,10 @@
void PrettyPrintVisitor::visit(const IdmapHeader& header) {
stream_ << "target apk path : " << header.GetTargetPath() << std::endl
<< "overlay apk path : " << header.GetOverlayPath() << std::endl;
+ const std::string& debug = header.GetDebugInfo();
+ if (!debug.empty()) {
+ stream_ << debug; // assume newline terminated
+ }
target_apk_ = ApkAssets::Load(header.GetTargetPath().to_string());
if (target_apk_) {
diff --git a/cmds/idmap2/libidmap2/RawPrintVisitor.cpp b/cmds/idmap2/libidmap2/RawPrintVisitor.cpp
index 13973d6..751c60c 100644
--- a/cmds/idmap2/libidmap2/RawPrintVisitor.cpp
+++ b/cmds/idmap2/libidmap2/RawPrintVisitor.cpp
@@ -16,6 +16,7 @@
#include "idmap2/RawPrintVisitor.h"
+#include <algorithm>
#include <cstdarg>
#include <string>
@@ -27,6 +28,15 @@
using android::ApkAssets;
+namespace {
+
+size_t StringSizeWhenEncoded(const std::string& s) {
+ size_t null_bytes = 4 - (s.size() % 4);
+ return sizeof(uint32_t) + s.size() + null_bytes;
+}
+
+} // namespace
+
namespace android::idmap2 {
// verbatim copy fomr PrettyPrintVisitor.cpp, move to common utils
@@ -40,8 +50,9 @@
print(header.GetVersion(), "version");
print(header.GetTargetCrc(), "target crc");
print(header.GetOverlayCrc(), "overlay crc");
- print(header.GetTargetPath().to_string(), "target path");
- print(header.GetOverlayPath().to_string(), "overlay path");
+ print(header.GetTargetPath().to_string(), kIdmapStringLength, "target path");
+ print(header.GetOverlayPath().to_string(), kIdmapStringLength, "overlay path");
+ print("...", StringSizeWhenEncoded(header.GetDebugInfo()), "debug info");
target_apk_ = ApkAssets::Load(header.GetTargetPath().to_string());
if (target_apk_) {
@@ -164,7 +175,7 @@
}
// NOLINTNEXTLINE(cert-dcl50-cpp)
-void RawPrintVisitor::print(const std::string& value, const char* fmt, ...) {
+void RawPrintVisitor::print(const std::string& value, size_t encoded_size, const char* fmt, ...) {
va_list ap;
va_start(ap, fmt);
std::string comment;
@@ -174,7 +185,7 @@
stream_ << base::StringPrintf("%08zx: ", offset_) << "........ " << comment << ": " << value
<< std::endl;
- offset_ += kIdmapStringLength;
+ offset_ += encoded_size;
}
// NOLINTNEXTLINE(cert-dcl50-cpp)
diff --git a/cmds/idmap2/libidmap2/ResourceMapping.cpp b/cmds/idmap2/libidmap2/ResourceMapping.cpp
index 651d20f..229628c 100644
--- a/cmds/idmap2/libidmap2/ResourceMapping.cpp
+++ b/cmds/idmap2/libidmap2/ResourceMapping.cpp
@@ -146,7 +146,8 @@
const LoadedPackage* target_package,
const LoadedPackage* overlay_package,
size_t string_pool_offset,
- const XmlParser& overlay_parser) {
+ const XmlParser& overlay_parser,
+ LogInfo& log_info) {
ResourceMapping resource_mapping;
auto root_it = overlay_parser.tree_iterator();
if (root_it->event() != XmlParser::Event::START_TAG || root_it->name() != "overlay") {
@@ -181,7 +182,8 @@
ResourceId target_id =
target_am->GetResourceId(*target_resource, "", target_package->GetPackageName());
if (target_id == 0U) {
- LOG(WARNING) << "failed to find resource \"" << *target_resource << "\" in target resources";
+ log_info.Warning(LogMessage() << "failed to find resource \"" << *target_resource
+ << "\" in target resources");
continue;
}
@@ -196,7 +198,7 @@
overlay_resource->dataType == Res_value::TYPE_DYNAMIC_REFERENCE)
? overlay_package_id == EXTRACT_PACKAGE(overlay_resource->data)
: false;
-
+
if (rewrite_overlay_reference) {
overlay_resource->dataType = Res_value::TYPE_DYNAMIC_REFERENCE;
}
@@ -239,7 +241,8 @@
const LoadedPackage* target_package,
const LoadedPackage* overlay_package,
const OverlayManifestInfo& overlay_info,
- const PolicyBitmask& fulfilled_policies) {
+ const PolicyBitmask& fulfilled_policies,
+ LogInfo& log_info) {
std::set<ResourceId> remove_ids;
for (const auto& target_map : target_map_) {
const ResourceId target_resid = target_map.first;
@@ -256,9 +259,9 @@
name = StringPrintf("0x%08x", target_resid);
}
- LOG(WARNING) << "overlay \"" << overlay_package->GetPackageName()
- << "\" is not allowed to overlay resource \"" << *name
- << "\" in target: " << success.GetErrorMessage();
+ log_info.Warning(LogMessage() << "overlay \"" << overlay_package->GetPackageName()
+ << "\" is not allowed to overlay resource \"" << *name
+ << "\" in target: " << success.GetErrorMessage());
remove_ids.insert(target_resid);
}
@@ -272,7 +275,15 @@
const ApkAssets& overlay_apk_assets,
const OverlayManifestInfo& overlay_info,
const PolicyBitmask& fulfilled_policies,
- bool enforce_overlayable) {
+ bool enforce_overlayable,
+ LogInfo& log_info) {
+ if (enforce_overlayable) {
+ log_info.Info(LogMessage() << "fulfilled_policies="
+ << ConcatPolicies(BitmaskToPolicies(fulfilled_policies))
+ << " enforce_overlayable="
+ << (enforce_overlayable ? "true" : "false"));
+ }
+
AssetManager2 target_asset_manager;
if (!target_asset_manager.SetApkAssets({&target_apk_assets}, true /* invalidate_caches */,
false /* filter_incompatible_configs*/)) {
@@ -333,7 +344,7 @@
string_pool_offset = overlay_arsc->GetStringPool()->size();
resource_mapping = CreateResourceMapping(&target_asset_manager, target_pkg, overlay_pkg,
- string_pool_offset, *(*parser));
+ string_pool_offset, *(*parser), log_info);
} else {
// If no file is specified using android:resourcesMap, it is assumed that the overlay only
// defines resources intended to override target resources of the same type and name.
@@ -349,7 +360,7 @@
// Filter out resources the overlay is not allowed to override.
(*resource_mapping)
.FilterOverlayableResources(&target_asset_manager, target_pkg, overlay_pkg, overlay_info,
- fulfilled_policies);
+ fulfilled_policies, log_info);
}
resource_mapping->target_package_id_ = target_pkg->GetPackageId();