Add a host tool to pack R_ARM_RELATIVE relocations in libchrome.<ver>.so.

tools/relocation_packer/src/relocation_packer_elf_file.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// TODO(simonb): Extend for 64-bit target libraries.
+// TODO(simonb): What if we cannot find two empty .dynamic slots?
+
+#include "relocation_packer_elf_file.h"
+
+#include <stdlib.h>
+#include <sys/types.h>
+#include <unistd.h>
+#include <string>
+#include <vector>
+
+#include "libelf.h"
+#include "relocation_packer_debug.h"
+#include "relocation_packer_packer.h"
+
+namespace relocation_packer {
+
+// Stub identifier written to 'null out' packed data.
+const Elf32_Word ElfFile::kStubIdentifier;
+
+// Out-of-band dynamic tags used to indicate the offset and size of the
+// .android.rel.dyn section.
+const Elf32_Sword ElfFile::DT_ANDROID_ARM_REL_OFFSET;
+const Elf32_Sword ElfFile::DT_ANDROID_ARM_REL_SIZE;
+
+namespace {
+
+// Rewrite section data.  Allocates new data and makes it the data element's
+// buffer.  Relies on program exit to free allocated data.

[rmcilroy, 2014/06/02 15:16:35]

I'm not sure I like relying on program exit to free the allocated data.  Can't
we free data->d_buf, or is that potentially allocated a different way?

[simonb (inactive), 2014/06/04 16:40:35]

data->d_buf is allocated by libelf (unless we've been here already), and libelf
will free it on exit.  If we want to gc our allocations we'll need to keep a
track of arrays allocated and written into data->d_buf fields, and free them in
the destructor (called from main only on program exit!) and/or when we find one
while overwriting.  Possible, but semantically the same as relying on program
exit to clean up.  Let me know if you want to add it, though.

+void RewriteSectionData(Elf_Data* data,
+                        const void* section_data,
+                        size_t size) {
+  CHECK(size == data->d_size);
+  uint8_t* area = new uint8_t[size];
+  ::memcpy(area, section_data, size);
+  data->d_buf = area;
+}
+
+// Resize a section.  If the new size is larger than the current size, open
+// up a hole by increasing file offsets that come after the hole.  If smaller
+// than the current size, remove the hole by decreasing those offsets.
+void ResizeSection(Elf* elf,

[rmcilroy, 2014/06/02 15:16:35]

This function is pretty massive and could benefit from splitting into a couple
of sub-functions.  Maybe: AdjustsElfHeadersForHole, AdjustSectionHeadersForHole,
AdjustDynamicSectionForHole, AdjustDynSymSectionForHole,
AdjustRelPltSectionForHole and AdjustSymTabSectionForHole?

[simonb (inactive), 2014/06/04 16:40:35]

Done.

+                   Elf_Scn* section,
+                   size_t new_size) {
+  Elf32_Shdr* section_header = elf32_getshdr(section);
+  if (section_header->sh_size == new_size)
+    return;
+
+  // Note if we are resizing the real .rel.dyn.  If yes, then we have to
+  // massage d_un.d_val in the dynamic section where d_tag is DT_RELSZ and
+  // DT_RELCOUNT.
+  size_t string_index;
+  elf_getshdrstrndx(elf, &string_index);
+  const std::string section_name =
+      elf_strptr(elf, string_index, section_header->sh_name);
+  const bool is_rel_dyn_resize = section_name == ".rel.dyn";
+
+  // Require that the section has exactly one data entry, so that the section
+  // size and the data size are the same.  True in practice for all sections
+  // we resize when packing or unpacking.
+  Elf_Data* data = elf_getdata(section, NULL);

[rmcilroy, 2014/06/02 15:16:35]

nit - section_data

+  CHECK(data && elf_getdata(section, data) == NULL);

[rmcilroy, 2014/06/02 15:16:35]

From the man page it looks like the "&& elf_getdata(dynamic_section, data) ==
NULL" is checking there are no further data buffers - could you add a comment to
that effect since it's note terribly clear just reading the code. 

[simonb (inactive), 2014/06/04 16:40:35]

Done.

+  CHECK(data->d_off == 0 && data->d_size == section_header->sh_size);
+
+  const Elf32_Off hole_start = section_header->sh_offset;
+  const int32_t hole_size = new_size - data->d_size;
+
+  VLOG_IF(hole_size > 0, "expand section size = %lu\n", data->d_size);
+  VLOG_IF(hole_size < 0, "shrink section size = %lu\n", data->d_size);
+
+  // Resize the data and the section header.
+  data->d_size += hole_size;
+  section_header->sh_size += hole_size;
+
+  Elf32_Ehdr* elf_header = elf32_getehdr(elf);
+  Elf32_Phdr* elf_program_header = elf32_getphdr(elf);
+
+  // Add the hole size to all offsets in the ELF file that are after the
+  // start of the hole.  If the hole size is positive we are expanding the
+  // section to create a new hole; if negative, we are closing up a hole.
+
+  // Start with the main ELF header.
+  if (elf_header->e_phoff > hole_start) {
+    elf_header->e_phoff += hole_size;
+    VLOG("e_phoff adjusted to %u\n", elf_header->e_phoff);
+  }
+  if (elf_header->e_shoff > hole_start) {
+    elf_header->e_shoff += hole_size;
+    VLOG("e_shoff adjusted to %u\n", elf_header->e_shoff);
+  }
+
+  // Note of the DYNAMIC entry, picked up while iterating program headers.
+  const Elf32_Phdr* dynamic_program_header = NULL;
+
+  // Adjust all program headers for the hole.
+  for (size_t i = 0; i < elf_header->e_phnum; ++i) {
+    Elf32_Phdr* program_header = &elf_program_header[i];
+    if (program_header->p_offset > hole_start) {
+      // The hole start is past this segment, so adjust offsets and addrs.
+      program_header->p_offset += hole_size;
+      VLOG("phdr %lu p_offset adjusted to %u\n", i, program_header->p_offset);
+
+      // Only adjust vaddr and paddr if this program header has them.
+      if (program_header->p_vaddr != 0) {
+        program_header->p_vaddr += hole_size;
+        VLOG("phdr %lu p_vaddr adjusted to %u\n", i, program_header->p_vaddr);
+      }
+      if (program_header->p_paddr != 0) {
+        program_header->p_paddr += hole_size;
+        VLOG("phdr %lu p_paddr adjusted to %u\n", i, program_header->p_paddr);
+      }
+    } else if (program_header->p_offset +
+               program_header->p_filesz > hole_start) {
+      // The hole start is within this segment, so adjust file and in-memory
+      // sizes, but leave offsets and addrs unchanged.
+      program_header->p_filesz += hole_size;
+      VLOG("phdr %lu p_filesz adjusted to %u\n", i, program_header->p_filesz);
+      program_header->p_memsz += hole_size;
+      VLOG("phdr %lu p_memsz adjusted to %u\n", i, program_header->p_memsz);
+    }
+
+    // If this is the DYNAMIC program header, note it for later.
+    if (program_header->p_type == PT_DYNAMIC) {
+      dynamic_program_header = program_header;
+    }
+  }
+  CHECK(dynamic_program_header);
+
+  // Notes of some sections requiring special attention, picked up during
+  // section iteration.
+  Elf_Scn* dynamic_section = NULL;
+  Elf_Scn* dynsym_section = NULL;
+  Elf_Scn* relplt_section = NULL;
+  Elf_Scn* symtab_section = NULL;

[rmcilroy, 2014/06/02 15:16:35]

Maybe you could just save these sections away as fields (initialized in Load(),
and updated as necessary here) and have this be a private method on ElfFile?

[simonb (inactive), 2014/06/04 16:40:35]

Slightly fiddly.  See if you like the refactored code better.

[rmcilroy, 2014/06/07 11:49:06]

This looks better, thanks.

+  // Note the offset of .android.rel.dyn also.
+  Elf32_Off android_rel_dyn_offset = 0;
+
+  // Adjust all section headers for the hole.
+  section = NULL;
+  while ((section = elf_nextscn(elf, section)) != NULL) {
+    Elf32_Shdr* section_header = elf32_getshdr(section);
+    std::string name = elf_strptr(elf, string_index, section_header->sh_name);
+    if (section_header->sh_offset > hole_start) {
+      section_header->sh_offset += hole_size;
+      VLOG("section %s sh_offset"
+           " adjusted to %u\n", name.c_str(), section_header->sh_offset);
+      // Only adjust section addr if this section has one.
+      if (section_header->sh_addr != 0) {
+        section_header->sh_addr += hole_size;
+        VLOG("section %s sh_addr"
+             " adjusted to %u\n", name.c_str(), section_header->sh_addr);
+      }
+    }
+
+    // Note the special sections that we are looking for as we go along.
+    if (section_header->sh_offset == dynamic_program_header->p_offset) {
+      dynamic_section = section;
+    }
+    if (name == ".dynsym") {
+      dynsym_section = section;
+    }
+    if (name == ".rel.plt") {
+      relplt_section = section;
+    }
+    if (name == ".symtab") {
+      symtab_section = section;
+    }
+
+    // Note .android.rel.dyn offset.
+    if (name == ".android.rel.dyn") {
+      android_rel_dyn_offset = section_header->sh_offset;
+    }
+  }
+  CHECK(dynamic_section != NULL);
+  CHECK(dynsym_section != NULL);
+  CHECK(relplt_section != NULL);
+  CHECK(android_rel_dyn_offset != 0);
+
+  // Load the .dynamic section into a local array.  Because we have to edit
+  // the current contents of .dynamic we disallow resizing it.
+  CHECK(section != dynamic_section);

[rmcilroy, 2014/06/02 15:16:35]

nit - do this check at the top of the function

[simonb (inactive), 2014/06/04 16:40:35]

Can't; dynamic_section is assigned in the loop above and just exited.

+  data = elf_getdata(dynamic_section, NULL);

[rmcilroy, 2014/06/02 15:16:35]

nit - dynamic_section_data

+  CHECK(data && elf_getdata(dynamic_section, data) == NULL);

[rmcilroy, 2014/06/02 15:16:35]

ditto

+
+  const Elf32_Dyn* dynamic_base = reinterpret_cast<Elf32_Dyn*>(data->d_buf);
+  std::vector<Elf32_Dyn> dynamics(
+      dynamic_base,
+      dynamic_base + data->d_size / sizeof(dynamics[0]));
+
+  for (size_t i = 0; i < dynamics.size(); ++i) {
+    Elf32_Dyn* dynamic = &dynamics[i];
+    const Elf32_Sword tag = dynamic->d_tag;
+    // Any tags that hold offsets are adjustment candidates.
+    const bool is_adjustable = (tag == DT_PLTGOT ||
+                                tag == DT_HASH ||
+                                tag == DT_STRTAB ||
+                                tag == DT_SYMTAB ||
+                                tag == DT_RELA ||
+                                tag == DT_INIT ||
+                                tag == DT_FINI ||
+                                tag == DT_REL ||
+                                tag == DT_JMPREL ||
+                                tag == DT_INIT_ARRAY ||
+                                tag == DT_FINI_ARRAY ||
+                                tag == ElfFile::DT_ANDROID_ARM_REL_OFFSET);
+    if (is_adjustable && dynamic->d_un.d_ptr > hole_start) {
+      dynamic->d_un.d_ptr += hole_size;
+      VLOG("dynamic[%lu] %u"
+           " d_ptr adjusted to %u\n", i, dynamic->d_tag, dynamic->d_un.d_ptr);
+    }
+
+    // If we are specifically resizing .rel.dyn, we need to make some added
+    // adjustments to tags that indicate the counts of R_ARM_RELATIVE
+    // relocations in the shared object.
+    if (is_rel_dyn_resize) {
+      // DT_RELSZ is the overall size of relocations.  Adjust by hole size.
+      if (tag == DT_RELSZ) {
+        dynamic->d_un.d_val += hole_size;
+        VLOG("dynamic[%lu] %u"
+             " d_val adjusted to %u\n", i, dynamic->d_tag, dynamic->d_un.d_val);
+      }
+
+      // The crazy linker does not use DT_RELCOUNT, but we keep it updated
+      // anyway.  In practice the section hole is always equal to the size
+      // of R_ARM_RELATIVE relocations, and DT_RELCOUNT is the count of
+      // relative relocations.  So closing a hole on packing reduces
+      // DT_RELCOUNT to zero, and opening a hole on unpacking restores it to
+      // its pre-packed value.
+      if (tag == DT_RELCOUNT) {
+        dynamic->d_un.d_val += hole_size / sizeof(Elf32_Rel);
+        VLOG("dynamic[%lu] %u"
+             " d_val adjusted to %u\n", i, dynamic->d_tag, dynamic->d_un.d_val);
+      }
+
+      // DT_RELENT doesn't change, but make sure it is what we expect.
+      if (tag == DT_RELENT) {
+        CHECK(dynamic->d_un.d_val == sizeof(Elf32_Rel));
+      }
+    }
+  }
+
+  void* section_data = &dynamics[0];

[rmcilroy, 2014/06/02 15:16:35]

nit - new_dynamic_section_data

+  size_t bytes = dynamics.size() * sizeof(dynamics[0]);
+  RewriteSectionData(data, section_data, bytes);
+
+  // Load the .dynsym section into a local array.  We need to adjust the
+  // values for the symbols represented in it.
+  data = elf_getdata(dynsym_section, NULL);

[rmcilroy, 2014/06/02 15:16:35]

nit - dynsym_section_data (and similar for others below)

+  CHECK(data && elf_getdata(dynsym_section, data) == NULL);
+
+  const Elf32_Sym* dynsym_base = reinterpret_cast<Elf32_Sym*>(data->d_buf);
+  std::vector<Elf32_Sym> dynsyms
+      (dynsym_base,
+       dynsym_base + data->d_size / sizeof(dynsyms[0]));
+
+  for (size_t i = 0; i < dynsyms.size(); ++i) {
+    Elf32_Sym* dynsym = &dynsyms[i];
+    const int type = static_cast<int>(ELF32_ST_TYPE(dynsym->st_info));
+    const bool is_adjustable = (type == STT_OBJECT ||
+                                type == STT_FUNC ||
+                                type == STT_SECTION ||
+                                type == STT_FILE ||
+                                type == STT_COMMON ||
+                                type == STT_TLS);
+    if (is_adjustable && dynsym->st_value > hole_start) {
+      dynsym->st_value += hole_size;
+      VLOG("dynsym[%lu] type=%u"
+           " st_value adjusted to %u\n", i, type, dynsym->st_value);
+    }
+  }
+
+  section_data = &dynsyms[0];
+  bytes = dynsyms.size() * sizeof(dynsyms[0]);
+  RewriteSectionData(data, section_data, bytes);
+
+  // Load the .rel.plt section into a local array.  We need to adjust the
+  // offset of every relocation inside it that falls beyond the hole start.
+  data = elf_getdata(relplt_section, NULL);
+  CHECK(data && elf_getdata(relplt_section, data) == NULL);
+
+  const Elf32_Rel* relplt_base = reinterpret_cast<Elf32_Rel*>(data->d_buf);
+  std::vector<Elf32_Rel> relplts(
+      relplt_base,
+      relplt_base + data->d_size / sizeof(relplts[0]));
+
+  for (size_t i = 0; i < relplts.size(); ++i) {
+    Elf32_Rel* relplt = &relplts[i];
+    if (relplt->r_offset > hole_start) {
+      relplt->r_offset += hole_size;
+      VLOG("relplt[%lu] r_offset adjusted to %u\n", i, relplt->r_offset);
+    }
+  }
+
+  section_data = &relplts[0];
+  bytes = relplts.size() * sizeof(relplts[0]);
+  RewriteSectionData(data, section_data, bytes);
+
+  // .symtab may be absent if the shared library was stripped.
+  if (symtab_section) {
+    // Load the .symtab section into a local array.  We need to adjust the
+    // offset of every relocation inside it that falls beyond the hole start.
+    data = elf_getdata(symtab_section, NULL);
+    CHECK(data && elf_getdata(symtab_section, data) == NULL);
+
+    const Elf32_Sym* symtab_base = reinterpret_cast<Elf32_Sym*>(data->d_buf);
+    std::vector<Elf32_Sym> symtab(
+        symtab_base,
+        symtab_base + data->d_size / sizeof(symtab[0]));
+
+    for (size_t i = 0; i < symtab.size(); ++i) {
+      Elf32_Sym* sym = &symtab[i];
+      if (sym->st_value > hole_start) {
+        sym->st_value += hole_size;
+        VLOG("symtab[%lu] value adjusted to %u\n", i, sym->st_value);
+      }
+    }
+
+    section_data = &symtab[0];
+    bytes = symtab.size() * sizeof(symtab[0]);
+    RewriteSectionData(data, section_data, bytes);
+  }
+}
+
+}  // namespace
+
+// Load the complete ELF file into a memory image in libelf, and identify
+// the .rel.dyn, .dynamic, and .android.rel.dyn sections.
+bool ElfFile::Load(int fd) {
+  elf_ = elf_begin(fd, ELF_C_RDWR, NULL);
+  CHECK(elf_);
+
+  if (elf_kind(elf_) != ELF_K_ELF) {
+    LOG("File not in ELF format\n");
+    return false;
+  }
+
+  Elf32_Ehdr* elf_header = elf32_getehdr(elf_);
+  if (!elf_header) {
+    LOG("Failed to load ELF header\n");
+    return false;
+  }
+  if (elf_header->e_machine != EM_ARM) {
+    LOG("File is not an arm32 ELF file\n");
+    return false;
+  }
+
+  // Require that our endianness matches that of the target, and that both
+  // are little-endian.  Safe for all current build/target combinations.
+  const int endian = static_cast<int>(elf_header->e_ident[5]);
+  CHECK(endian == ELFDATA2LSB);
+  CHECK(__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__);
+
+  VLOG("endian = %u\n", endian);
+
+  VLOG("e_phoff = %u\n", elf_header->e_phoff);
+  VLOG("e_shoff = %u\n", elf_header->e_shoff);
+  VLOG("e_ehsize = %u\n", elf_header->e_ehsize);
+  VLOG("e_phentsize = %u\n", elf_header->e_phentsize);
+  VLOG("e_phnum = %u\n", elf_header->e_phnum);
+  VLOG("e_shnum = %u\n", elf_header->e_shnum);
+  VLOG("e_shstrndx = %u\n", elf_header->e_shstrndx);
+
+  const Elf32_Phdr* elf_program_header = elf32_getphdr(elf_);
+  CHECK(elf_program_header);
+
+  const Elf32_Phdr* dynamic_program_header = NULL;
+  for (size_t i = 0; i < elf_header->e_phnum; ++i) {
+    const Elf32_Phdr* program_header = &elf_program_header[i];
+    std::string type;
+    switch (program_header->p_type) {
+      case 0: type = "NULL"; break;

[rmcilroy, 2014/06/02 15:16:35]

nit - you use PT_DYNANIC below, could you use PT_* constant's here instead of
the integer literals please?

[simonb (inactive), 2014/06/04 16:40:35]

Done.

+      case 1: type = "LOAD"; break;
+      case 2: type = "DYNAMIC"; break;
+      case 3: type = "INTERP"; break;
+      case 4: type = "NOTE"; break;
+      case 5: type = "SHLIB"; break;
+      case 6: type = "PHDR"; break;
+      case 7: type = "TLS"; break;
+      default: type = "(OTHER)"; break;
+    }
+    VLOG("phdr %lu : %s\n", i, type.c_str());
+    VLOG("  p_offset = %u\n", program_header->p_offset);
+    VLOG("  p_vaddr = %u\n", program_header->p_vaddr);
+    VLOG("  p_paddr = %u\n", program_header->p_paddr);
+    VLOG("  p_filesz = %u\n", program_header->p_filesz);
+    VLOG("  p_memsz = %u\n", program_header->p_memsz);

[rmcilroy, 2014/06/02 15:16:35]

Please extract the debugging code (from line 373 to line 390) to a separate
function (e.g, VerboseLogProgramHeader())

[simonb (inactive), 2014/06/04 16:40:35]

Done.

+    if (program_header->p_type == PT_DYNAMIC) {
+      CHECK(dynamic_program_header == NULL);
+      dynamic_program_header = program_header;
+    }
+  }
+  CHECK(dynamic_program_header != NULL);
+
+  size_t string_index;
+  elf_getshdrstrndx(elf_, &string_index);
+
+  // Notes of the .rel.dyn, .android.rel.dyn, and .dynamic sections.  Found
+  // while iterating sections, and later stored in class attributes.
+  Elf_Scn* found_rel_dyn_section = NULL;
+  Elf_Scn* found_android_rel_dyn_section = NULL;
+  Elf_Scn* found_dynamic_section = NULL;
+
+  // Flag set if we encounter any .debug* section.  We do not adjust any
+  // offsets or addresses of any debug data, so if we find one of these then
+  // the resulting output shared object should still run, but might not be
+  // usable for debugging, disassembly, and so on.  Provides a warning if
+  // this occurs.
+  bool is_debug = false;

[rmcilroy, 2014/06/02 15:16:35]

nit - has_debug_section 

[simonb (inactive), 2014/06/04 16:40:35]

Done.

+
+  Elf_Scn* section = NULL;
+  while ((section = elf_nextscn(elf_, section)) != NULL) {
+    const Elf32_Shdr* section_header = elf32_getshdr(section);
+    std::string name = elf_strptr(elf_, string_index, section_header->sh_name);
+    VLOG("section %s\n", name.c_str());
+    VLOG("  sh_addr = %u\n", section_header->sh_addr);
+    VLOG("  sh_offset = %u\n", section_header->sh_offset);
+    VLOG("  sh_size = %u\n", section_header->sh_size);
+
+    // Note special sections as we encounter them.
+    if (name == ".rel.dyn") {
+      found_rel_dyn_section = section;
+    }
+    if (name == ".android.rel.dyn") {
+      found_android_rel_dyn_section = section;
+    }
+    if (section_header->sh_offset == dynamic_program_header->p_offset) {
+      found_dynamic_section = section;
+    }
+
+    // If we find a section named .debug*, set the debug warning flag.
+    if (std::string(name).find(".debug") == 0) {
+      is_debug = true;
+    }
+
+    Elf_Data* data = NULL;
+    while ((data = elf_getdata(section, data)) != NULL) {
+      VLOG("  data\n");
+      VLOG("    d_buf = %p\n", data->d_buf);
+      VLOG("    d_off = %lu\n", data->d_off);
+      VLOG("    d_size = %lu\n", data->d_size);
+    }

[rmcilroy, 2014/06/02 15:16:35]

nit - extract verbose debugging code (lines 418-421 and 439-445) as something
like VerboseLogSection()

[simonb (inactive), 2014/06/04 16:40:35]

Done.

+  }
+
+  // Loading failed if we did not find the required special sections.
+  if (!found_rel_dyn_section) {
+    LOG("Missing .rel.dyn section\n");
+    return false;
+  }
+  if (!found_dynamic_section) {
+    LOG("Missing .dynamic section\n");
+    return false;
+  }
+  if (!found_android_rel_dyn_section) {
+    LOG("Missing .android.rel.dyn section (not split/packed?)\n");
+    return false;
+  }
+
+  if (is_debug) {
+    LOG("WARNING: found .debug section(s), and ignored them\n");
+  }
+
+  fd_ = fd;
+  rel_dyn_section_ = found_rel_dyn_section;
+  dynamic_section_ = found_dynamic_section;
+  android_rel_dyn_section_ = found_android_rel_dyn_section;
+  return true;
+}
+
+namespace {

[rmcilroy, 2014/06/02 15:16:35]

nit - I would prefer a single anonymous namespace at the top of the file with
all the helper functions.

[simonb (inactive), 2014/06/04 16:40:35]

I prefer helper functions to be defined close to the items they help (preferably
nested, but not an option in C++).

[rmcilroy, 2014/06/07 11:49:06]

In that case, please move ResizeSection (and all associated helper functions)
down here since they are only used by PackRelocations and UnpackRelocations.

[simonb (inactive), 2014/06/09 14:39:19]

Good point.  Done.  Also moved AddDynamicEntry and RemoveDynamicEntry so that
they are adjacent.

+
+// Replace the first free (unused) slot in a dynamics vector with the given
+// value.  The vector always ends with a free (unused) element, so the slot
+// found cannot be the last one in the vector.

[rmcilroy, 2014/06/02 15:16:35]

As I commented in the Readme, this seems fragile, is there any way we can
guarantee there will be enough free slots?

[simonb (inactive), 2014/06/04 16:40:35]

gold --spare-dynamic-tags.  Code updated.

+void AddDynamicEntry(Elf32_Dyn dyn,
+                     std::vector<Elf32_Dyn>* dynamics) {
+  for (size_t i = 0; i < dynamics->size() - 1; ++i) {

[rmcilroy, 2014/06/02 15:16:35]

nit - add a comment:
// The vector should always end with a free (unused) element, and so 
// we cannot replace the final element.  Therefore, only loop until 
// the penultimate element.

[simonb (inactive), 2014/06/04 16:40:35]

Done.

+    Elf32_Dyn &slot = dynamics->at(i);
+    if (slot.d_tag == DT_NULL) {
+      slot = dyn;
+      VLOG("dynamic[%lu] overwritten with %u\n", i, dyn.d_tag);
+      return;
+    }
+  }
+
+  // TODO(simonb): Are sufficient free slots always available?
+  // No free dynamics vector slot was found.
+  NOTREACHED();
+}
+
+// Apply R_ARM_RELATIVE relocations to the file data to which they refer.
+// This relocates data into the area it will occupy after the hole in
+// .rel.dyn is added or removed.
+void ApplyRelocations(Elf* elf,

[rmcilroy, 2014/06/02 15:16:35]

Should this maybe be called AdjustRelocationTargets?

[simonb (inactive), 2014/06/04 16:40:35]

Done.

+                      Elf32_Off hole_start,
+                      size_t hole_size,
+                      const std::vector<Elf32_Rel>& relative_relocations) {
+  Elf_Scn* section = NULL;
+  while ((section = elf_nextscn(elf, section)) != NULL) {
+    const Elf32_Shdr* section_header = elf32_getshdr(section);
+
+    // Identify this section's start and end addresses.
+    const Elf32_Addr section_start = section_header->sh_addr;
+    const Elf32_Addr section_end = section_start + section_header->sh_size;
+
+    Elf_Data* data = elf_getdata(section, NULL);
+    CHECK(data && elf_getdata(section, data) == NULL);
+
+    // Ignore sections with no effective data.
+    if (data->d_buf == NULL)
+      continue;
+
+    // Create a copy-on-write pointer to the section's data.
+    uint8_t* area = reinterpret_cast<uint8_t*>(data->d_buf);
+
+    for (size_t i = 0; i < relative_relocations.size(); ++i) {
+      const Elf32_Rel* relocation = &relative_relocations[i];
+      // See if this relocation points into the current section.
+      const bool is_encompassed = relocation->r_offset >= section_start &&
+                                  relocation->r_offset < section_end;
+      if (is_encompassed) {

[rmcilroy, 2014/06/02 15:16:35]

nit - just inline is_encompassed caluclation into the if statement

[simonb (inactive), 2014/06/04 16:40:35]

Done.

+        Elf32_Addr byte_offset = relocation->r_offset - section_start;
+        Elf32_Off* target = reinterpret_cast<Elf32_Off*>(area + byte_offset);
+
+        // See if the relocation's target is after the hole's start.
+        if (*target > hole_start) {
+          if (area == data->d_buf) {
+            // We are about to write but we have not yet copied the buffer.
+            // Copy now, and recompute target to point into the newly
+            // allocated copy-on-write buffer.
+            area = new uint8_t[data->d_size];
+            ::memcpy(area, data->d_buf, data->d_size);
+            target = reinterpret_cast<Elf32_Off*>(area + byte_offset);
+          }
+          *target += hole_size;
+          VLOG("relocation[%lu] target adjusted to %u\n", i, *target);
+        }
+      }
+    }
+
+    // If we applied any relocation to this section, write it back.
+    if (area != data->d_buf) {
+      RewriteSectionData(data, area, data->d_size);
+      delete [] area;
+    }
+  }
+}
+
+// Adjust relocations so that the offset that they indicate will be correct
+// after the hole in .rel.dyn is added or removed (in effect, relocate the
+// relocations).
+void AdjustRelocations(Elf32_Off hole_start,
+                       size_t hole_size,
+                       std::vector<Elf32_Rel>* relocations) {
+  for (size_t i = 0; i < relocations->size(); ++i) {
+    Elf32_Rel* relocation = &relocations->at(i);
+    if (relocation->r_offset > hole_start) {
+      relocation->r_offset += hole_size;
+      VLOG("relocation[%lu] offset adjusted to %u\n", i, relocation->r_offset);
+    }
+  }
+}
+
+}  // namespace
+
+// Remove R_ARM_RELATIVE entries from .rel.dyn and write as packed data
+// into .android.rel.dyn.
+bool ElfFile::PackRelocations() {
+  // Retrieve the current .rel.dyn section data.
+  Elf_Data* data = elf_getdata(rel_dyn_section_, NULL);
+  CHECK(data && elf_getdata(rel_dyn_section_, data) == NULL);
+
+  // Convert data to a vector of Elf32 relocations.
+  const Elf32_Rel* relocations_base = reinterpret_cast<Elf32_Rel*>(data->d_buf);
+  std::vector<Elf32_Rel> relocations(
+      relocations_base,
+      relocations_base + data->d_size / sizeof(relocations[0]));
+
+  std::vector<Elf32_Rel> relative_relocations;
+  std::vector<Elf32_Rel> other_relocations;
+
+  // Filter relocations into those that are R_ARM_RELATIVE and others.
+  for (size_t i = 0; i < relocations.size(); ++i) {
+    const Elf32_Rel& relocation = relocations[i];
+    if (ELF32_R_TYPE(relocation.r_info) == R_ARM_RELATIVE) {
+      CHECK(ELF32_R_SYM(relocation.r_info) == 0);
+      relative_relocations.push_back(relocation);
+    } else {
+      other_relocations.push_back(relocation);
+    }
+  }
+  VLOG("R_ARM_RELATIVE: %lu entries\n", relative_relocations.size());
+  VLOG("Other         : %lu entries\n", other_relocations.size());
+  VLOG("Total         : %lu entries\n", relocations.size());
+
+  // If no relative relocations then we have nothing packable.  Perhaps
+  // the shared object has already been packed?
+  if (relative_relocations.empty()) {
+    LOG("No R_ARM_RELATIVE relocations found (already packed?)\n");
+    return false;
+  }
+
+  // Pre-calculate the size of the hole we will close up when we rewrite
+  // .reldyn.  We have to adjust all relocation addresses to account for this.
+  Elf32_Shdr* section_header = elf32_getshdr(rel_dyn_section_);
+  const Elf32_Off hole_start = section_header->sh_offset;
+  const size_t hole_size =
+      relative_relocations.size() * sizeof(relative_relocations[0]);
+
+  // Unless padding, pre-apply R_ARM_RELATIVE relocations to account for the
+  // hole, and pre-adjust all relocation offsets accordingly.
+  if (!is_padding_rel_dyn_) {
+    // Apply relocations to all R_ARM_RELATIVE data to relocate it into the
+    // area it will occupy once the hole in .rel.dyn is removed.
+    ApplyRelocations(elf_, hole_start, -hole_size, relative_relocations);
+    // Relocate the relocations.
+    AdjustRelocations(hole_start, -hole_size, &relative_relocations);
+    AdjustRelocations(hole_start, -hole_size, &other_relocations);
+  }
+
+  // Pack R_ARM_RELATIVE relocations.
+  const size_t initial_bytes =
+      relative_relocations.size() * sizeof(relative_relocations[0]);
+  LOG("Unpacked R_ARM_RELATIVE: %lu bytes\n", initial_bytes);
+  std::vector<uint8_t> packed;
+  RelocationPacker packer;
+  packer.PackRelativeRelocations(relative_relocations, &packed);
+  const void* packed_data = &packed[0];
+  const size_t packed_bytes = packed.size() * sizeof(packed[0]);
+  LOG("Packed   R_ARM_RELATIVE: %lu bytes\n", packed_bytes);
+
+  // If we have insufficient R_ARM_RELATIVE relocations to form a run then
+  // packing fails.
+  if (packed.empty()) {
+    LOG("Too few R_ARM_RELATIVE relocations to pack\n");
+    return false;
+  }
+
+  // Run a loopback self-test as a check that packing is lossless.
+  std::vector<Elf32_Rel> unpacked;
+  packer.UnpackRelativeRelocations(packed, &unpacked);
+  CHECK(unpacked.size() == relative_relocations.size());
+  for (size_t i = 0; i < unpacked.size(); ++i) {
+    CHECK(unpacked[i].r_offset == relative_relocations[i].r_offset);
+    CHECK(unpacked[i].r_info == relative_relocations[i].r_info);
+  }
+
+  // Make sure packing saved some space.
+  if (packed_bytes >= initial_bytes) {
+    LOG("Packing R_ARM_RELATIVE relocations saves no space\n");
+    return false;
+  }
+
+  // If padding, add R_ARM_NONE relocations to other_relocations to make it
+  // the same size as the the original relocations we read in.  This makes
+  // the ResizeSection() below a no-op.
+  if (is_padding_rel_dyn_) {
+    const Elf32_Rel r_arm_none = {R_ARM_NONE, 0};
+    const size_t required = relocations.size() - other_relocations.size();
+    std::vector<Elf32_Rel> padding(required, r_arm_none);
+    other_relocations.insert(
+        other_relocations.end(), padding.begin(), padding.end());
+  }
+
+  // Rewrite the current .rel.dyn section to be only the non-R_ARM_RELATIVE
+  // relocations, then shrink it to size.
+  const void* section_data = &other_relocations[0];

[rmcilroy, 2014/06/02 15:16:35]

nit - new_section_data (or new_data)

+  const size_t bytes = other_relocations.size() * sizeof(other_relocations[0]);
+  ResizeSection(elf_, rel_dyn_section_, bytes);
+  RewriteSectionData(data, section_data, bytes);
+
+  // Rewrite the current .android.rel.dyn section to hold the packed
+  // R_ARM_RELATIVE relocations.
+  data = elf_getdata(android_rel_dyn_section_, NULL);
+  CHECK(data && elf_getdata(android_rel_dyn_section_, data) == NULL);

[rmcilroy, 2014/06/02 15:16:35]

You do this CHECK a lot, maybe have a wrapper function which does the
elf_getdata and the CHECK

[simonb (inactive), 2014/06/04 16:40:35]

Done.

+  // Ensure the current section is not zero-length (that is, has allocated
+  // data that we can validly expand).
+  CHECK(data->d_size > 0 && data->d_buf);

[rmcilroy, 2014/06/02 15:16:35]

nit - do this check in ResizeSection?

[simonb (inactive), 2014/06/04 16:40:35]

Done.

+  ResizeSection(elf_, android_rel_dyn_section_, packed_bytes);
+  RewriteSectionData(data, packed_data, packed_bytes);
+
+  // Rewrite .dynamic to include two new tags describing .android.rel.dyn.
+  data = elf_getdata(dynamic_section_, NULL);
+  CHECK(data && elf_getdata(dynamic_section_, data) == NULL);
+  const Elf32_Dyn* dynamic_base = reinterpret_cast<Elf32_Dyn*>(data->d_buf);
+  std::vector<Elf32_Dyn> dynamics(
+      dynamic_base,
+      dynamic_base + data->d_size / sizeof(dynamics[0]));
+  section_header = elf32_getshdr(android_rel_dyn_section_);
+  // Steal two vacant slots to describe the .android.rel.dyn section.
+  const Elf32_Dyn offset_dyn
+      = {DT_ANDROID_ARM_REL_OFFSET, {section_header->sh_offset}};
+  AddDynamicEntry(offset_dyn, &dynamics);
+  const Elf32_Dyn size_dyn
+      = {DT_ANDROID_ARM_REL_SIZE, {section_header->sh_size}};
+  AddDynamicEntry(size_dyn, &dynamics);
+  const void* dynamics_data = &dynamics[0];
+  const size_t dynamics_bytes = dynamics.size() * sizeof(dynamics[0]);
+  RewriteSectionData(data, dynamics_data, dynamics_bytes);

[rmcilroy, 2014/06/02 15:16:35]

nit - lines 686-702 seem like they could be a separate function.

[simonb (inactive), 2014/06/04 16:40:35]

Left as is for now.

+
+  return true;
+}
+
+namespace {
+
+// Remove elements in the dynamics vector that match the given tag with
+// unused slot data.  The first unused slot effectively terminates the
+// vector, so we can validly only remove items from the end of the vector.
+void RemoveDynamicEntry(Elf32_Sword tag,
+                        std::vector<Elf32_Dyn>* dynamics) {
+  const Elf32_Dyn null_dyn = {DT_NULL, {0}};
+
+  for (size_t i = 0; i < dynamics->size() - 1; ++i) {

[rmcilroy, 2014/06/02 15:16:35]

If you can only remove items from the end of the vector, shouldn't you start the
loop from the end, and assert that the entry is DT_NULL or the expected tag to
ensure you don't early-terminate the vector?

[simonb (inactive), 2014/06/04 16:40:35]

Rewritten to remove the matching entry and shuffle following elements up.

+    Elf32_Dyn &slot = dynamics->at(i);
+    if (slot.d_tag == tag) {
+      slot = null_dyn;
+      VLOG("dynamic[%lu] overwritten with DT_NULL\n", i);
+    }
+  }
+}
+
+}  // namespace
+
+// Find packed R_ARM_RELATIVE relocations in .android.rel.dyn, unpack them,
+// and rewrite the .rel.dyn section in so_file to contain unpacked data.
+bool ElfFile::UnpackRelocations() {
+  // Retrieve the current .android.rel.dyn section data.
+  Elf_Data* data = elf_getdata(android_rel_dyn_section_, NULL);
+  CHECK(data && elf_getdata(android_rel_dyn_section_, data) == NULL);
+
+  // Convert data to a vector of bytes.
+  const uint8_t* packed_base = reinterpret_cast<uint8_t*>(data->d_buf);
+  std::vector<uint8_t> packed(
+      packed_base,
+      packed_base + data->d_size / sizeof(packed[0]));
+
+  // Properly packed data must begin with "APR1".
+  if (packed.empty() ||
+      packed[0] != 'A' || packed[1] != 'P' ||
+      packed[2] != 'R' || packed[3] != '1') {
+    LOG("Packed R_ARM_RELATIVE relocations not found (not packed?)\n");
+    return false;
+  }
+
+  // Unpack the data to re-materialize the R_ARM_RELATIVE relocations.
+  const size_t packed_bytes = packed.size() * sizeof(packed[0]);
+  LOG("Packed   R_ARM_RELATIVE: %lu bytes\n", packed_bytes);
+  std::vector<Elf32_Rel> relative_relocations;
+  RelocationPacker packer;
+  packer.UnpackRelativeRelocations(packed, &relative_relocations);
+  const size_t unpacked_bytes =
+      relative_relocations.size() * sizeof(relative_relocations[0]);
+  LOG("Unpacked R_ARM_RELATIVE: %lu bytes\n", unpacked_bytes);
+
+  // Retrieve the current .rel.dyn section data.
+  data = elf_getdata(rel_dyn_section_, NULL);
+  CHECK(data && elf_getdata(rel_dyn_section_, data) == NULL);
+
+  // Interpret data as Elf32 relocations.
+  const Elf32_Rel* relocations_base = reinterpret_cast<Elf32_Rel*>(data->d_buf);
+  std::vector<Elf32_Rel> relocations(
+      relocations_base,
+      relocations_base + data->d_size / sizeof(relocations[0]));
+
+  std::vector<Elf32_Rel> other_relocations;
+  size_t padding = 0;
+
+  // Filter relocations to locate any that are R_ARM_NONE.  These will occur
+  // if padding was turned on for packing.
+  for (size_t i = 0; i < relocations.size(); ++i) {
+    const Elf32_Rel& relocation = relocations[i];
+    if (ELF32_R_TYPE(relocation.r_info) != R_ARM_NONE) {
+      other_relocations.push_back(relocation);
+    } else {
+      ++padding;
+    }
+  }
+  LOG("R_ARM_RELATIVE: %lu entries\n", relative_relocations.size());
+  LOG("Other         : %lu entries\n", other_relocations.size());
+
+  // If we found the same number of R_ARM_NONE entries in .rel.dyn as we
+  // hold as unpacked relative relocations, then this is a padded file.
+  const bool is_padded = padding == relative_relocations.size();
+
+  // Pre-calculate the size of the hole we will open up when we rewrite
+  // .reldyn.  We have to adjust all relocation addresses to account for this.
+  Elf32_Shdr* section_header = elf32_getshdr(rel_dyn_section_);
+  const Elf32_Off hole_start = section_header->sh_offset;
+  const size_t hole_size =
+      relative_relocations.size() * sizeof(relative_relocations[0]);
+
+  // Unless padded, pre-apply R_ARM_RELATIVE relocations to account for the
+  // hole, and pre-adjust all relocation offsets accordingly.
+  if (!is_padded) {
+    // Apply relocations to all R_ARM_RELATIVE data to relocate it into the
+    // area it will occupy once the hole in .rel.dyn is opened.
+    ApplyRelocations(elf_, hole_start, hole_size, relative_relocations);
+    // Relocate the relocations.
+    AdjustRelocations(hole_start, hole_size, &relative_relocations);
+    AdjustRelocations(hole_start, hole_size, &other_relocations);
+  }
+
+  // Rewrite the current .rel.dyn section to be the R_ARM_RELATIVE relocations
+  // followed by other relocations.  This is the usual order in which we find
+  // them after linking, so this action will normally put the entire .rel.dyn
+  // section back to its pre-split-and-packed state.
+  relocations.assign(relative_relocations.begin(), relative_relocations.end());
+  relocations.insert(relocations.end(),
+                     other_relocations.begin(), other_relocations.end());
+  const void* section_data = &relocations[0];
+  const size_t bytes = relocations.size() * sizeof(relocations[0]);
+  LOG("Total         : %lu entries\n", relocations.size());
+  ResizeSection(elf_, rel_dyn_section_, bytes);
+  RewriteSectionData(data, section_data, bytes);
+
+  // Nearly empty the current .android.rel.dyn section.  Leaves a four-byte
+  // stub so that some data remains allocated to the section.  This is a
+  // convenience which allows us to re-pack this file again without
+  // having to remove the section and then add a new small one with objcopy.
+  // The way we resize sections relies on there being some data in a section.
+  data = elf_getdata(android_rel_dyn_section_, NULL);
+  CHECK(data && elf_getdata(android_rel_dyn_section_, data) == NULL);
+  ResizeSection(elf_, android_rel_dyn_section_, sizeof(kStubIdentifier));
+  RewriteSectionData(data, &kStubIdentifier, sizeof(kStubIdentifier));
+
+  // Rewrite .dynamic to remove two tags describing .android.rel.dyn.
+  data = elf_getdata(dynamic_section_, NULL);
+  CHECK(data && elf_getdata(dynamic_section_, data) == NULL);
+  const Elf32_Dyn* dynamic_base = reinterpret_cast<Elf32_Dyn*>(data->d_buf);
+  std::vector<Elf32_Dyn> dynamics(
+      dynamic_base,
+      dynamic_base + data->d_size / sizeof(dynamics[0]));
+  RemoveDynamicEntry(DT_ANDROID_ARM_REL_SIZE, &dynamics);
+  RemoveDynamicEntry(DT_ANDROID_ARM_REL_OFFSET, &dynamics);
+  const void* dynamics_data = &dynamics[0];
+  const size_t dynamics_bytes = dynamics.size() * sizeof(dynamics[0]);
+  RewriteSectionData(data, dynamics_data, dynamics_bytes);
+
+  return true;
+}
+
+// Flush rewritten shared object file data.
+void ElfFile::Flush() {
+  // Flag all ELF data held in memory as needing to be written back to the
+  // file, and tell libelf that we have controlled the file layout.
+  elf_flagelf(elf_, ELF_C_SET, ELF_F_DIRTY);
+  elf_flagelf(elf_, ELF_C_SET, ELF_F_LAYOUT);
+
+  // Write ELF data back to disk.
+  const off_t file_bytes = elf_update(elf_, ELF_C_WRITE);
+  CHECK(file_bytes > 0);
+  VLOG("elf_update returned: %lu\n", file_bytes);
+
+  // Clean up libelf, and truncate the output file to the number of bytes
+  // written by elf_update().
+  elf_end(elf_);
+  const int truncate = ftruncate(fd_, file_bytes);
+  CHECK(truncate == 0);
+}
+
+}  // namespace relocation_packer