Remove malloc/free for inline overlay values

Remove malloc/free of android::ResTable_entry for inline overlay
values.

Add `target_entry_inline` to the idmap format to encode inline overlay
values separate from direct mapping of target resource to overlay
resource. This reduces the number of bytes needed to represent a direct
mapping of target resource to overlay resource from 9 bytes to 8 bytes
per entry.

Fixed all idmap alignment issues that required the framework to use
"#pragma pack(push, 1)" when loading idmaps.

Bug: 170341022
Test: idmap2_tests and libandroidfw_tests
Change-Id: Iab4d3902508f02773464724913e0ee966e3689e4
diff --git a/cmds/idmap2/libidmap2/BinaryStreamVisitor.cpp b/cmds/idmap2/libidmap2/BinaryStreamVisitor.cpp
index 255212a..726f6c5 100644
--- a/cmds/idmap2/libidmap2/BinaryStreamVisitor.cpp
+++ b/cmds/idmap2/libidmap2/BinaryStreamVisitor.cpp
@@ -24,10 +24,6 @@
 
 namespace android::idmap2 {
 
-void BinaryStreamVisitor::Write(const void* value, size_t length) {
-  stream_.write(reinterpret_cast<const char*>(value), length);
-}
-
 void BinaryStreamVisitor::Write8(uint8_t value) {
   stream_.write(reinterpret_cast<char*>(&value), sizeof(uint8_t));
 }
@@ -49,11 +45,11 @@
   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());
+void BinaryStreamVisitor::WriteString(const StringPiece& value) {
+  // pad with null to nearest word boundary;
+  size_t padding_size = CalculatePadding(value.size());
+  Write32(value.size());
+  stream_.write(value.data(), value.size());
   stream_.write("\0\0\0\0", padding_size);
 }
 
@@ -67,7 +63,7 @@
   Write32(header.GetTargetCrc());
   Write32(header.GetOverlayCrc());
   Write32(header.GetFulfilledPolicies());
-  Write8(static_cast<uint8_t>(header.GetEnforceOverlayable()));
+  Write32(static_cast<uint8_t>(header.GetEnforceOverlayable()));
   WriteString256(header.GetTargetPath());
   WriteString256(header.GetOverlayPath());
   WriteString(header.GetDebugInfo());
@@ -76,8 +72,16 @@
 void BinaryStreamVisitor::visit(const IdmapData& data) {
   for (const auto& target_entry : data.GetTargetEntries()) {
     Write32(target_entry.target_id);
-    Write8(target_entry.data_type);
-    Write32(target_entry.data_value);
+    Write32(target_entry.overlay_id);
+  }
+
+  static constexpr uint16_t kValueSize = 8U;
+  for (const auto& target_entry : data.GetTargetInlineEntries()) {
+    Write32(target_entry.target_id);
+    Write16(kValueSize);
+    Write8(0U);  // padding
+    Write8(target_entry.value.data_type);
+    Write32(target_entry.value.data_value);
   }
 
   for (const auto& overlay_entry : data.GetOverlayEntries()) {
@@ -85,16 +89,18 @@
     Write32(overlay_entry.target_id);
   }
 
-  Write(data.GetStringPoolData(), data.GetHeader()->GetStringPoolLength());
+  WriteString(data.GetStringPoolData());
 }
 
 void BinaryStreamVisitor::visit(const IdmapData::Header& header) {
   Write8(header.GetTargetPackageId());
   Write8(header.GetOverlayPackageId());
+  Write8(0U);  // padding
+  Write8(0U);  // padding
   Write32(header.GetTargetEntryCount());
+  Write32(header.GetTargetInlineEntryCount());
   Write32(header.GetOverlayEntryCount());
   Write32(header.GetStringPoolIndexOffset());
-  Write32(header.GetStringPoolLength());
 }
 
 }  // namespace android::idmap2
diff --git a/cmds/idmap2/libidmap2/Idmap.cpp b/cmds/idmap2/libidmap2/Idmap.cpp
index 23c25a7..1129413 100644
--- a/cmds/idmap2/libidmap2/Idmap.cpp
+++ b/cmds/idmap2/libidmap2/Idmap.cpp
@@ -51,19 +51,19 @@
   return false;
 }
 
-bool WARN_UNUSED Read32(std::istream& stream, uint32_t* out) {
-  uint32_t value;
-  if (stream.read(reinterpret_cast<char*>(&value), sizeof(uint32_t))) {
-    *out = dtohl(value);
+bool WARN_UNUSED Read16(std::istream& stream, uint16_t* out) {
+  uint16_t value;
+  if (stream.read(reinterpret_cast<char*>(&value), sizeof(uint16_t))) {
+    *out = dtohs(value);
     return true;
   }
   return false;
 }
 
-bool WARN_UNUSED ReadBuffer(std::istream& stream, std::unique_ptr<uint8_t[]>* out, size_t length) {
-  auto buffer = std::unique_ptr<uint8_t[]>(new uint8_t[length]);
-  if (stream.read(reinterpret_cast<char*>(buffer.get()), length)) {
-    *out = std::move(buffer);
+bool WARN_UNUSED Read32(std::istream& stream, uint32_t* out) {
+  uint32_t value;
+  if (stream.read(reinterpret_cast<char*>(&value), sizeof(uint32_t))) {
+    *out = dtohl(value);
     return true;
   }
   return false;
@@ -95,8 +95,11 @@
   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()));
+  uint32_t padding_size = CalculatePadding(size);
+  std::string padding(padding_size, '\0');
+  if (!stream.read(padding.data(), padding_size)) {
+    return Error("failed to read string padding of size %u", padding_size);
+  }
   return buf;
 }
 
@@ -112,16 +115,16 @@
 
 std::unique_ptr<const IdmapHeader> IdmapHeader::FromBinaryStream(std::istream& stream) {
   std::unique_ptr<IdmapHeader> idmap_header(new IdmapHeader());
-  uint8_t enforce_overlayable;
+  uint32_t enforce_overlayable;
   if (!Read32(stream, &idmap_header->magic_) || !Read32(stream, &idmap_header->version_) ||
       !Read32(stream, &idmap_header->target_crc_) || !Read32(stream, &idmap_header->overlay_crc_) ||
-      !Read32(stream, &idmap_header->fulfilled_policies_) || !Read8(stream, &enforce_overlayable) ||
-      !ReadString256(stream, idmap_header->target_path_) ||
+      !Read32(stream, &idmap_header->fulfilled_policies_) ||
+      !Read32(stream, &enforce_overlayable) || !ReadString256(stream, idmap_header->target_path_) ||
       !ReadString256(stream, idmap_header->overlay_path_)) {
     return nullptr;
   }
 
-  idmap_header->enforce_overlayable_ = static_cast<bool>(enforce_overlayable);
+  idmap_header->enforce_overlayable_ = enforce_overlayable != 0U;
 
   auto debug_str = ReadString(stream);
   if (!debug_str) {
@@ -207,12 +210,13 @@
 std::unique_ptr<const IdmapData::Header> IdmapData::Header::FromBinaryStream(std::istream& stream) {
   std::unique_ptr<IdmapData::Header> idmap_data_header(new IdmapData::Header());
 
+  uint8_t padding;
   if (!Read8(stream, &idmap_data_header->target_package_id_) ||
-      !Read8(stream, &idmap_data_header->overlay_package_id_) ||
-      !Read32(stream, &idmap_data_header->target_entry_count) ||
+      !Read8(stream, &idmap_data_header->overlay_package_id_) || !Read8(stream, &padding) ||
+      !Read8(stream, &padding) || !Read32(stream, &idmap_data_header->target_entry_count) ||
+      !Read32(stream, &idmap_data_header->target_entry_inline_count) ||
       !Read32(stream, &idmap_data_header->overlay_entry_count) ||
-      !Read32(stream, &idmap_data_header->string_pool_index_offset) ||
-      !Read32(stream, &idmap_data_header->string_pool_len)) {
+      !Read32(stream, &idmap_data_header->string_pool_index_offset)) {
     return nullptr;
   }
 
@@ -225,14 +229,27 @@
   if (!data->header_) {
     return nullptr;
   }
+
   // Read the mapping of target resource id to overlay resource value.
   for (size_t i = 0; i < data->header_->GetTargetEntryCount(); i++) {
     TargetEntry target_entry{};
-    if (!Read32(stream, &target_entry.target_id) || !Read8(stream, &target_entry.data_type) ||
-        !Read32(stream, &target_entry.data_value)) {
+    if (!Read32(stream, &target_entry.target_id) || !Read32(stream, &target_entry.overlay_id)) {
       return nullptr;
     }
-    data->target_entries_.emplace_back(target_entry);
+    data->target_entries_.push_back(target_entry);
+  }
+
+  // Read the mapping of target resource id to inline overlay values.
+  uint8_t unused1;
+  uint16_t unused2;
+  for (size_t i = 0; i < data->header_->GetTargetInlineEntryCount(); i++) {
+    TargetInlineEntry target_entry{};
+    if (!Read32(stream, &target_entry.target_id) || !Read16(stream, &unused2) ||
+        !Read8(stream, &unused1) || !Read8(stream, &target_entry.value.data_type) ||
+        !Read32(stream, &target_entry.value.data_value)) {
+      return nullptr;
+    }
+    data->target_inline_entries_.push_back(target_entry);
   }
 
   // Read the mapping of overlay resource id to target resource id.
@@ -245,9 +262,11 @@
   }
 
   // Read raw string pool bytes.
-  if (!ReadBuffer(stream, &data->string_pool_, data->header_->string_pool_len)) {
+  auto string_pool_data = ReadString(stream);
+  if (!string_pool_data) {
     return nullptr;
   }
+  data->string_pool_data_ = std::move(*string_pool_data);
 
   return std::move(data);
 }
@@ -290,27 +309,28 @@
   }
 
   std::unique_ptr<IdmapData> data(new IdmapData());
-  for (const auto& mappings : resource_mapping.GetTargetToOverlayMap()) {
-    data->target_entries_.emplace_back(IdmapData::TargetEntry{
-        mappings.first, mappings.second.data_type, mappings.second.data_value});
+  data->string_pool_data_ = resource_mapping.GetStringPoolData().to_string();
+  for (const auto& mapping : resource_mapping.GetTargetToOverlayMap()) {
+    if (auto overlay_resource = std::get_if<ResourceId>(&mapping.second)) {
+      data->target_entries_.push_back({mapping.first, *overlay_resource});
+    } else {
+      data->target_inline_entries_.push_back(
+          {mapping.first, std::get<TargetValue>(mapping.second)});
+    }
   }
 
-  for (const auto& mappings : resource_mapping.GetOverlayToTargetMap()) {
-    data->overlay_entries_.emplace_back(IdmapData::OverlayEntry{mappings.first, mappings.second});
+  for (const auto& mapping : resource_mapping.GetOverlayToTargetMap()) {
+    data->overlay_entries_.emplace_back(IdmapData::OverlayEntry{mapping.first, mapping.second});
   }
 
   std::unique_ptr<IdmapData::Header> data_header(new IdmapData::Header());
   data_header->target_package_id_ = resource_mapping.GetTargetPackageId();
   data_header->overlay_package_id_ = resource_mapping.GetOverlayPackageId();
   data_header->target_entry_count = static_cast<uint32_t>(data->target_entries_.size());
+  data_header->target_entry_inline_count =
+      static_cast<uint32_t>(data->target_inline_entries_.size());
   data_header->overlay_entry_count = static_cast<uint32_t>(data->overlay_entries_.size());
   data_header->string_pool_index_offset = resource_mapping.GetStringPoolOffset();
-
-  const auto string_pool_data = resource_mapping.GetStringPoolData();
-  data_header->string_pool_len = string_pool_data.second;
-  data->string_pool_ = std::unique_ptr<uint8_t[]>(new uint8_t[data_header->string_pool_len]);
-  memcpy(data->string_pool_.get(), string_pool_data.first, data_header->string_pool_len);
-
   data->header_ = std::move(data_header);
   return {std::move(data)};
 }
diff --git a/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp b/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp
index 63ee8a6..a93202a 100644
--- a/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp
+++ b/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp
@@ -38,6 +38,7 @@
   stream_ << "Paths:" << std::endl
           << TAB "target apk path  : " << header.GetTargetPath() << std::endl
           << TAB "overlay apk path : " << header.GetOverlayPath() << std::endl;
+
   const std::string& debug = header.GetDebugInfo();
   if (!debug.empty()) {
     std::istringstream debug_stream(debug);
@@ -48,10 +49,16 @@
     }
   }
 
-  target_apk_ = ApkAssets::Load(header.GetTargetPath().to_string());
-  if (target_apk_) {
+  if (auto target_apk_ = ApkAssets::Load(header.GetTargetPath().to_string())) {
     target_am_.SetApkAssets({target_apk_.get()});
+    apk_assets_.push_back(std::move(target_apk_));
   }
+
+  if (auto overlay_apk = ApkAssets::Load(header.GetOverlayPath().to_string())) {
+    overlay_am_.SetApkAssets({overlay_apk.get()});
+    apk_assets_.push_back(std::move(overlay_apk));
+  }
+
   stream_ << "Mapping:" << std::endl;
 }
 
@@ -59,34 +66,56 @@
 }
 
 void PrettyPrintVisitor::visit(const IdmapData& data) {
+  static constexpr const char* kUnknownResourceName = "???";
+
   const bool target_package_loaded = !target_am_.GetApkAssets().empty();
-  const ResStringPool string_pool(data.GetStringPoolData(),
-                                  data.GetHeader()->GetStringPoolLength());
+  const bool overlay_package_loaded = !overlay_am_.GetApkAssets().empty();
+
+  const ResStringPool string_pool(data.GetStringPoolData().data(), data.GetStringPoolData().size());
   const size_t string_pool_offset = data.GetHeader()->GetStringPoolIndexOffset();
 
-  for (auto& target_entry : data.GetTargetEntries()) {
-    stream_ << TAB << base::StringPrintf("0x%08x ->", target_entry.target_id);
-
-    if (target_entry.data_type != Res_value::TYPE_REFERENCE &&
-        target_entry.data_type != Res_value::TYPE_DYNAMIC_REFERENCE) {
-      stream_ << " " << utils::DataTypeToString(target_entry.data_type);
-    }
-
-    if (target_entry.data_type == Res_value::TYPE_STRING) {
-      stream_ << " \""
-              << string_pool.string8ObjectAt(target_entry.data_value - string_pool_offset).c_str()
-              << "\"";
-    } else {
-      stream_ << " " << base::StringPrintf("0x%08x", target_entry.data_value);
-    }
-
+  for (const auto& target_entry : data.GetTargetEntries()) {
+    std::string target_name = kUnknownResourceName;
     if (target_package_loaded) {
-      Result<std::string> name = utils::ResToTypeEntryName(target_am_, target_entry.target_id);
-      if (name) {
-        stream_ << " " << *name;
+      if (auto name = utils::ResToTypeEntryName(target_am_, target_entry.target_id)) {
+        target_name = *name;
       }
     }
-    stream_ << std::endl;
+
+    std::string overlay_name = kUnknownResourceName;
+    if (overlay_package_loaded) {
+      if (auto name = utils::ResToTypeEntryName(overlay_am_, target_entry.overlay_id)) {
+        overlay_name = *name;
+      }
+    }
+
+    stream_ << TAB
+            << base::StringPrintf("0x%08x -> 0x%08x (%s -> %s)", target_entry.target_id,
+                                  target_entry.overlay_id, target_name.c_str(),
+                                  overlay_name.c_str())
+            << std::endl;
+  }
+
+  for (auto& target_entry : data.GetTargetInlineEntries()) {
+    stream_ << TAB << base::StringPrintf("0x%08x -> ", target_entry.target_id)
+            << utils::DataTypeToString(target_entry.value.data_type);
+
+    size_t unused;
+    if (target_entry.value.data_type == Res_value::TYPE_STRING) {
+      auto str = string_pool.stringAt(target_entry.value.data_value - string_pool_offset, &unused);
+      stream_ << " \"" << StringPiece16(str) << "\"";
+    } else {
+      stream_ << " " << base::StringPrintf("0x%08x", target_entry.value.data_value);
+    }
+
+    std::string target_name = kUnknownResourceName;
+    if (target_package_loaded) {
+      if (auto name = utils::ResToTypeEntryName(target_am_, target_entry.target_id)) {
+        target_name = *name;
+      }
+    }
+
+    stream_ << " (" << target_name << ")" << std::endl;
   }
 }
 
diff --git a/cmds/idmap2/libidmap2/RawPrintVisitor.cpp b/cmds/idmap2/libidmap2/RawPrintVisitor.cpp
index 3f62a2a..82f5d26 100644
--- a/cmds/idmap2/libidmap2/RawPrintVisitor.cpp
+++ b/cmds/idmap2/libidmap2/RawPrintVisitor.cpp
@@ -30,15 +30,6 @@
 using android::ApkAssets;
 using android::idmap2::policy::PoliciesToDebugString;
 
-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 {
 
 void RawPrintVisitor::visit(const Idmap& idmap ATTRIBUTE_UNUSED) {
@@ -51,19 +42,24 @@
   print(header.GetOverlayCrc(), "overlay crc");
   print(header.GetFulfilledPolicies(), "fulfilled policies: %s",
         PoliciesToDebugString(header.GetFulfilledPolicies()).c_str());
-  print(static_cast<uint8_t>(header.GetEnforceOverlayable()), "enforce overlayable");
+  print(static_cast<uint32_t>(header.GetEnforceOverlayable()), "enforce overlayable");
   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());
+  uint32_t debug_info_size = header.GetDebugInfo().size();
+  print(debug_info_size, "debug info size");
+  print("...", debug_info_size + CalculatePadding(debug_info_size), "debug info");
+
+  auto target_apk_ = ApkAssets::Load(header.GetTargetPath().to_string());
   if (target_apk_) {
     target_am_.SetApkAssets({target_apk_.get()});
+    apk_assets_.push_back(std::move(target_apk_));
   }
 
-  overlay_apk_ = ApkAssets::Load(header.GetOverlayPath().to_string());
+  auto overlay_apk_ = ApkAssets::Load(header.GetOverlayPath().to_string());
   if (overlay_apk_) {
     overlay_am_.SetApkAssets({overlay_apk_.get()});
+    apk_assets_.push_back(std::move(overlay_apk_));
   }
 }
 
@@ -82,18 +78,44 @@
       print(target_entry.target_id, "target id");
     }
 
-    print(target_entry.data_type, "type: %s",
-          utils::DataTypeToString(target_entry.data_type).data());
-
     Result<std::string> overlay_name(Error(""));
-    if (overlay_package_loaded && (target_entry.data_type == Res_value::TYPE_REFERENCE ||
-                                   target_entry.data_type == Res_value::TYPE_DYNAMIC_REFERENCE)) {
-      overlay_name = utils::ResToTypeEntryName(overlay_am_, target_entry.data_value);
+    if (overlay_package_loaded) {
+      overlay_name = utils::ResToTypeEntryName(overlay_am_, target_entry.overlay_id);
     }
     if (overlay_name) {
-      print(target_entry.data_value, "value: %s", overlay_name->c_str());
+      print(target_entry.overlay_id, "overlay id: %s", overlay_name->c_str());
     } else {
-      print(target_entry.data_value, "value");
+      print(target_entry.overlay_id, "overlay id");
+    }
+  }
+
+  for (auto& target_entry : data.GetTargetInlineEntries()) {
+    Result<std::string> target_name(Error(""));
+    if (target_package_loaded) {
+      target_name = utils::ResToTypeEntryName(target_am_, target_entry.target_id);
+    }
+    if (target_name) {
+      print(target_entry.target_id, "target id: %s", target_name->c_str());
+    } else {
+      print(target_entry.target_id, "target id");
+    }
+
+    print("...", sizeof(Res_value::size) + sizeof(Res_value::res0), "padding");
+
+    print(target_entry.value.data_type, "type: %s",
+          utils::DataTypeToString(target_entry.value.data_type).data());
+
+    Result<std::string> overlay_name(Error(""));
+    if (overlay_package_loaded &&
+        (target_entry.value.data_value == Res_value::TYPE_REFERENCE ||
+         target_entry.value.data_value == Res_value::TYPE_DYNAMIC_REFERENCE)) {
+      overlay_name = utils::ResToTypeEntryName(overlay_am_, target_entry.value.data_value);
+    }
+
+    if (overlay_name) {
+      print(target_entry.value.data_value, "data: %s", overlay_name->c_str());
+    } else {
+      print(target_entry.value.data_value, "data");
     }
   }
 
@@ -121,19 +143,19 @@
     }
   }
 
-  const size_t string_pool_length = data.GetHeader()->GetStringPoolLength();
-  if (string_pool_length > 0) {
-    print_raw(string_pool_length, "%zu raw string pool bytes", string_pool_length);
-  }
+  uint32_t string_pool_size = data.GetStringPoolData().size();
+  print(string_pool_size, "string pool size");
+  print("...", string_pool_size + CalculatePadding(string_pool_size), "string pool");
 }
 
 void RawPrintVisitor::visit(const IdmapData::Header& header) {
   print(header.GetTargetPackageId(), "target package id");
   print(header.GetOverlayPackageId(), "overlay package id");
+  print("...", sizeof(Idmap_data_header::p0), "padding");
   print(header.GetTargetEntryCount(), "target entry count");
+  print(header.GetTargetInlineEntryCount(), "target inline entry count");
   print(header.GetOverlayEntryCount(), "overlay entry count");
   print(header.GetStringPoolIndexOffset(), "string pool index offset");
-  print(header.GetStringPoolLength(), "string pool byte length");
 }
 
 // NOLINTNEXTLINE(cert-dcl50-cpp)
@@ -190,17 +212,4 @@
   offset_ += encoded_size;
 }
 
-// NOLINTNEXTLINE(cert-dcl50-cpp)
-void RawPrintVisitor::print_raw(uint32_t length, const char* fmt, ...) {
-  va_list ap;
-  va_start(ap, fmt);
-  std::string comment;
-  base::StringAppendV(&comment, fmt, ap);
-  va_end(ap);
-
-  stream_ << base::StringPrintf("%08zx: ", offset_) << "........  " << comment << std::endl;
-
-  offset_ += length;
-}
-
 }  // namespace android::idmap2
diff --git a/cmds/idmap2/libidmap2/ResourceMapping.cpp b/cmds/idmap2/libidmap2/ResourceMapping.cpp
index fd8b4eb..122f068 100644
--- a/cmds/idmap2/libidmap2/ResourceMapping.cpp
+++ b/cmds/idmap2/libidmap2/ResourceMapping.cpp
@@ -205,19 +205,14 @@
       overlay_resource->data += string_pool_offset;
     }
 
-    // Only rewrite resources defined within the overlay package to their corresponding target
-    // resource ids at runtime.
-    bool rewrite_overlay_reference =
-        IsReference(overlay_resource->dataType)
-            ? overlay_package_id == EXTRACT_PACKAGE(overlay_resource->data)
-            : false;
-
-    if (rewrite_overlay_reference) {
-      overlay_resource->dataType = Res_value::TYPE_DYNAMIC_REFERENCE;
+    if (IsReference(overlay_resource->dataType)) {
+      // Only rewrite resources defined within the overlay package to their corresponding target
+      // resource ids at runtime.
+      bool rewrite_reference = overlay_package_id == EXTRACT_PACKAGE(overlay_resource->data);
+      resource_mapping.AddMapping(target_id, overlay_resource->data, rewrite_reference);
+    } else {
+      resource_mapping.AddMapping(target_id, overlay_resource->dataType, overlay_resource->data);
     }
-
-    resource_mapping.AddMapping(target_id, overlay_resource->dataType, overlay_resource->data,
-                                rewrite_overlay_reference);
   }
 
   return resource_mapping;
@@ -246,9 +241,8 @@
 
     // Retrieve the compile-time resource id of the target resource.
     target_resource = REWRITE_PACKAGE(target_resource, target_package_id);
-
-    resource_mapping.AddMapping(target_resource, Res_value::TYPE_REFERENCE, overlay_resid,
-                                /* rewrite_overlay_reference */ false);
+    resource_mapping.AddMapping(target_resource, overlay_resid,
+                                false /* rewrite_overlay_reference */);
   }
 
   return resource_mapping;
@@ -396,9 +390,7 @@
   return map;
 }
 
-Result<Unit> ResourceMapping::AddMapping(ResourceId target_resource,
-                                         TargetValue::DataType data_type,
-                                         TargetValue::DataValue data_value,
+Result<Unit> ResourceMapping::AddMapping(ResourceId target_resource, ResourceId overlay_resource,
                                          bool rewrite_overlay_reference) {
   if (target_map_.find(target_resource) != target_map_.end()) {
     return Error(R"(target resource id "0x%08x" mapped to multiple values)", target_resource);
@@ -407,13 +399,26 @@
   // TODO(141485591): Ensure that the overlay type is compatible with the target type. If the
   // runtime types are not compatible, it could cause runtime crashes when the resource is resolved.
 
-  target_map_.insert(std::make_pair(target_resource, TargetValue{data_type, data_value}));
+  target_map_.insert(std::make_pair(target_resource, overlay_resource));
 
-  if (rewrite_overlay_reference && IsReference(data_type)) {
-    overlay_map_.insert(std::make_pair(data_value, target_resource));
+  if (rewrite_overlay_reference) {
+    overlay_map_.insert(std::make_pair(overlay_resource, target_resource));
+  }
+  return Unit{};
+}
+
+Result<Unit> ResourceMapping::AddMapping(ResourceId target_resource,
+                                         TargetValue::DataType data_type,
+                                         TargetValue::DataValue data_value) {
+  if (target_map_.find(target_resource) != target_map_.end()) {
+    return Error(R"(target resource id "0x%08x" mapped to multiple values)", target_resource);
   }
 
-  return Result<Unit>({});
+  // TODO(141485591): Ensure that the overlay type is compatible with the target type. If the
+  // runtime types are not compatible, it could cause runtime crashes when the resource is resolved.
+
+  target_map_.insert(std::make_pair(target_resource, TargetValue{data_type, data_value}));
+  return Unit{};
 }
 
 void ResourceMapping::RemoveMapping(ResourceId target_resource) {
@@ -422,14 +427,15 @@
     return;
   }
 
-  const TargetValue value = target_iter->second;
+  const auto value = target_iter->second;
   target_map_.erase(target_iter);
 
-  if (!IsReference(value.data_type)) {
+  const ResourceId* overlay_resource = std::get_if<ResourceId>(&value);
+  if (overlay_resource == nullptr) {
     return;
   }
 
-  auto overlay_iter = overlay_map_.equal_range(value.data_value);
+  auto overlay_iter = overlay_map_.equal_range(*overlay_resource);
   for (auto i = overlay_iter.first; i != overlay_iter.second; ++i) {
     if (i->second == target_resource) {
       overlay_map_.erase(i);