Bug 1499915 - Remove support for the elfhack filler segment r=froydnj
authorMike Hommey <mh+mozilla@glandium.org>
Wed, 24 Oct 2018 13:42:31 +0000
changeset 491230 0988f87f6bd2504463cea12fe0f2ba9ef5317b38
parent 491229 418b398baaeeb9e9b5652f0be376aa07bfe3f769
child 491231 550a527e90ee52ce0ec3095b2fd977446ee2f5f2
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersfroydnj
bugs1499915, 822584
milestone65.0a1
Bug 1499915 - Remove support for the elfhack filler segment r=froydnj This effectively backs out bug 822584, which worked around a similar problem to what we are facing with Android xpcshell, being that the crash reporter doesn't handle the address space "fragmentation" induced by elfhack. The work around worked, at the expense of some added complexity. It was used for B2G only, and has effectively been unused since B2G was retired. Differential Revision: https://phabricator.services.mozilla.com/D9622
build/unix/elfhack/elf.cpp
build/unix/elfhack/elfhack.cpp
build/unix/elfhack/elfxx.h
--- a/build/unix/elfhack/elf.cpp
+++ b/build/unix/elfhack/elf.cpp
@@ -621,17 +621,17 @@ void ElfSegment::addSection(ElfSection *
 void ElfSegment::removeSection(ElfSection *section)
 {
     sections.remove(section);
     section->removeFromSegment(this);
 }
 
 unsigned int ElfSegment::getFileSize()
 {
-    if (type == PT_GNU_RELRO || isElfHackFillerSegment())
+    if (type == PT_GNU_RELRO)
         return filesz;
 
     if (sections.empty())
         return 0;
     // Search the last section that is not SHT_NOBITS
     std::list<ElfSection *>::reverse_iterator i;
     for (i = sections.rbegin(); (i != sections.rend()) && ((*i)->getType() == SHT_NOBITS); ++i);
     // All sections are SHT_NOBITS
@@ -640,49 +640,42 @@ unsigned int ElfSegment::getFileSize()
 
     unsigned int end = (*i)->getAddr() + (*i)->getSize();
 
     return end - sections.front()->getAddr();
 }
 
 unsigned int ElfSegment::getMemSize()
 {
-    if (type == PT_GNU_RELRO || isElfHackFillerSegment())
+    if (type == PT_GNU_RELRO)
         return memsz;
 
     if (sections.empty())
         return 0;
 
     unsigned int end = sections.back()->getAddr() + sections.back()->getSize();
 
     return end - sections.front()->getAddr();
 }
 
 unsigned int ElfSegment::getOffset()
 {
     if ((type == PT_GNU_RELRO) && !sections.empty() &&
         (sections.front()->getAddr() != vaddr))
         throw std::runtime_error("PT_GNU_RELRO segment doesn't start on a section start");
 
-    // Neither bionic nor glibc linkers seem to like when the offset of that segment is 0
-    if (isElfHackFillerSegment())
-        return vaddr;
-
     return sections.empty() ? 0 : sections.front()->getOffset();
 }
 
 unsigned int ElfSegment::getAddr()
 {
     if ((type == PT_GNU_RELRO) && !sections.empty() &&
         (sections.front()->getAddr() != vaddr))
         throw std::runtime_error("PT_GNU_RELRO segment doesn't start on a section start");
 
-    if (isElfHackFillerSegment())
-        return vaddr;
-
     return sections.empty() ? 0 : sections.front()->getAddr();
 }
 
 void ElfSegment::clear()
 {
     for (std::list<ElfSection *>::iterator i = sections.begin(); i != sections.end(); ++i)
         (*i)->removeFromSegment(this);
     sections.clear();
--- a/build/unix/elfhack/elfhack.cpp
+++ b/build/unix/elfhack/elfhack.cpp
@@ -459,17 +459,17 @@ void set_relative_reloc(Elf_Rel *rel, El
 
 void set_relative_reloc(Elf_Rela *rel, Elf *elf, unsigned int value) {
     // ld puts the value of relocated relocations both in the addend and
     // at r_offset. For consistency, keep it that way.
     set_relative_reloc((Elf_Rel *)rel, elf, value);
     rel->r_addend = value;
 }
 
-void maybe_split_segment(Elf *elf, ElfSegment *segment, bool fill)
+void maybe_split_segment(Elf *elf, ElfSegment *segment)
 {
     std::list<ElfSection *>::iterator it = segment->begin();
     for (ElfSection *last = *(it++); it != segment->end(); last = *(it++)) {
         // When two consecutive non-SHT_NOBITS sections are apart by more
         // than the alignment of the section, the second can be moved closer
         // to the first, but this requires the segment to be split.
         if (((*it)->getType() != SHT_NOBITS) && (last->getType() != SHT_NOBITS) &&
             ((*it)->getOffset() - last->getOffset() - last->getSize() > segment->getAlign())) {
@@ -479,55 +479,22 @@ void maybe_split_segment(Elf *elf, ElfSe
             phdr.p_vaddr = 0;
             phdr.p_paddr = phdr.p_vaddr + segment->getVPDiff();
             phdr.p_flags = segment->getFlags();
             phdr.p_align = segment->getAlign();
             phdr.p_filesz = (unsigned int)-1;
             phdr.p_memsz = (unsigned int)-1;
             ElfSegment *newSegment = new ElfSegment(&phdr);
             elf->insertSegmentAfter(segment, newSegment);
-            ElfSection *section = *it;
             for (; it != segment->end(); ++it) {
                 newSegment->addSection(*it);
             }
             for (it = newSegment->begin(); it != newSegment->end(); ++it) {
                 segment->removeSection(*it);
             }
-            // Fill the virtual address space gap left between the two PT_LOADs
-            // with a new PT_LOAD with no permissions. This avoids the linker
-            // (especially bionic's) filling the gap with anonymous memory,
-            // which breakpad doesn't like.
-            // /!\ running strip on a elfhacked binary will break this filler
-            // PT_LOAD.
-            if (!fill)
-                break;
-            // Insert dummy segment to normalize the entire Elf with the header
-            // sizes adjusted, before inserting a filler segment.
-            {
-              memset(&phdr, 0, sizeof(phdr));
-              ElfSegment dummySegment(&phdr);
-              elf->insertSegmentAfter(segment, &dummySegment);
-              elf->normalize();
-              elf->removeSegment(&dummySegment);
-            }
-            ElfSection *previous = section->getPrevious();
-            phdr.p_type = PT_LOAD;
-            phdr.p_vaddr = (previous->getAddr() + previous->getSize() + segment->getAlign() - 1) & ~(segment->getAlign() - 1);
-            phdr.p_paddr = phdr.p_vaddr + segment->getVPDiff();
-            phdr.p_flags = 0;
-            phdr.p_align = 0;
-            phdr.p_filesz = (section->getAddr() & ~(newSegment->getAlign() - 1)) - phdr.p_vaddr;
-            phdr.p_memsz = phdr.p_filesz;
-            if (phdr.p_filesz) {
-                newSegment = new ElfSegment(&phdr);
-                assert(newSegment->isElfHackFillerSegment());
-                elf->insertSegmentAfter(segment, newSegment);
-            } else {
-                elf->normalize();
-            }
             break;
         }
     }
 }
 
 // EH_FRAME constants
 static const char DW_EH_PE_absptr = 0x00;
 static const char DW_EH_PE_omit = 0xff;
@@ -762,17 +729,17 @@ static void adjust_eh_frame(ElfSection* 
     }
     return;
 
 malformed:
     throw std::runtime_error("malformed .eh_frame");
 }
 
 template <typename Rel_Type>
-int do_relocation_section(Elf *elf, unsigned int rel_type, unsigned int rel_type2, bool force, bool fill)
+int do_relocation_section(Elf *elf, unsigned int rel_type, unsigned int rel_type2, bool force)
 {
     ElfDynamic_Section *dyn = elf->getDynSection();
     if (dyn == nullptr) {
         fprintf(stderr, "Couldn't find SHT_DYNAMIC section\n");
         return -1;
     }
 
     ElfRel_Section<Rel_Type> *section = (ElfRel_Section<Rel_Type> *)dyn->getSectionForType(Rel_Type::d_tag);
@@ -1131,17 +1098,17 @@ int do_relocation_section(Elf *elf, unsi
         assert(distance == second->getAddr() - first->getAddr());
         first->markDirty();
         adjust_eh_frame(eh_frame, origAddr, elf);
     }
 
     // Adjust PT_LOAD segments
     for (ElfSegment *segment = elf->getSegmentByType(PT_LOAD); segment;
          segment = elf->getSegmentByType(PT_LOAD, segment)) {
-        maybe_split_segment(elf, segment, fill);
+        maybe_split_segment(elf, segment);
     }
 
     // Ensure Elf sections will be at their final location.
     elf->normalize();
     ElfLocation *init = new ElfLocation(relhackcode, relhackcode->getEntryPoint());
     if (init_array) {
         // Adjust the first DT_INIT_ARRAY entry to point at the injected code
         // by transforming its relocation into a relative one pointing to the
@@ -1162,17 +1129,17 @@ int do_relocation_section(Elf *elf, unsi
 
 static inline int backup_file(const char *name)
 {
     std::string fname(name);
     fname += ".bak";
     return rename(name, fname.c_str());
 }
 
-void do_file(const char *name, bool backup = false, bool force = false, bool fill = false)
+void do_file(const char *name, bool backup = false, bool force = false)
 {
     std::ifstream file(name, std::ios::in|std::ios::binary);
     Elf elf(file);
     unsigned int size = elf.getSize();
     fprintf(stderr, "%s: ", name);
     if (elf.getType() != ET_DYN) {
         fprintf(stderr, "Not a shared object. Skipping\n");
         return;
@@ -1185,23 +1152,23 @@ void do_file(const char *name, bool back
             fprintf(stderr, "Already elfhacked. Skipping\n");
             return;
         }
     }
 
     int exit = -1;
     switch (elf.getMachine()) {
     case EM_386:
-        exit = do_relocation_section<Elf_Rel>(&elf, R_386_RELATIVE, R_386_32, force, fill);
+        exit = do_relocation_section<Elf_Rel>(&elf, R_386_RELATIVE, R_386_32, force);
         break;
     case EM_X86_64:
-        exit = do_relocation_section<Elf_Rela>(&elf, R_X86_64_RELATIVE, R_X86_64_64, force, fill);
+        exit = do_relocation_section<Elf_Rela>(&elf, R_X86_64_RELATIVE, R_X86_64_64, force);
         break;
     case EM_ARM:
-        exit = do_relocation_section<Elf_Rel>(&elf, R_ARM_RELATIVE, R_ARM_ABS32, force, fill);
+        exit = do_relocation_section<Elf_Rel>(&elf, R_ARM_RELATIVE, R_ARM_ABS32, force);
         break;
     }
     if (exit == 0) {
         if (!force && (elf.getSize() >= size)) {
             fprintf(stderr, "No gain. Skipping\n");
         } else if (backup && backup_file(name) != 0) {
             fprintf(stderr, "Couln't create backup file\n");
         } else {
@@ -1241,65 +1208,54 @@ void undo_file(const char *name, bool ba
 
     ElfSegment *first = data->getSegmentByType(PT_LOAD);
     ElfSegment *second = text->getSegmentByType(PT_LOAD);
     if (first != second) {
         fprintf(stderr, elfhack_data " and " elfhack_text " not in the same segment. Skipping\n");
         return;
     }
     second = elf.getSegmentByType(PT_LOAD, first);
-    ElfSegment *filler = nullptr;
-    // If the second PT_LOAD is a filler from elfhack --fill, check the third.
-    if (second->isElfHackFillerSegment()) {
-        filler = second;
-        second = elf.getSegmentByType(PT_LOAD, filler);
-    }
     if (second->getFlags() != first->getFlags()) {
         fprintf(stderr, "Couldn't identify elfhacked PT_LOAD segments. Skipping\n");
         return;
     }
     // Move sections from the second PT_LOAD to the first, and remove the
     // second PT_LOAD segment.
     for (std::list<ElfSection *>::iterator section = second->begin();
          section != second->end(); ++section)
         first->addSection(*section);
 
     elf.removeSegment(second);
-    if (filler)
-        elf.removeSegment(filler);
 
     if (backup && backup_file(name) != 0) {
         fprintf(stderr, "Couln't create backup file\n");
     } else {
         std::ofstream ofile(name, std::ios::out|std::ios::binary|std::ios::trunc);
         elf.write(ofile);
         fprintf(stderr, "Grown by %d bytes\n", elf.getSize() - size);
     }
 }
 
 int main(int argc, char *argv[])
 {
     int arg;
     bool backup = false;
     bool force = false;
     bool revert = false;
-    bool fill = false;
     char *lastSlash = rindex(argv[0], '/');
     if (lastSlash != nullptr)
         rundir = strndup(argv[0], lastSlash - argv[0]);
     for (arg = 1; arg < argc; arg++) {
         if (strcmp(argv[arg], "-f") == 0)
             force = true;
         else if (strcmp(argv[arg], "-b") == 0)
             backup = true;
         else if (strcmp(argv[arg], "-r") == 0)
             revert = true;
-        else if (strcmp(argv[arg], "--fill") == 0)
-            fill = true;
         else if (revert) {
             undo_file(argv[arg], backup);
         } else
-            do_file(argv[arg], backup, force, fill);
+            do_file(argv[arg], backup, force);
     }
 
     free(rundir);
     return 0;
 }
--- a/build/unix/elfhack/elfxx.h
+++ b/build/unix/elfhack/elfxx.h
@@ -481,20 +481,16 @@ public:
 
     void addSection(ElfSection *section);
     void removeSection(ElfSection *section);
 
     std::list<ElfSection *>::iterator begin() { return sections.begin(); }
     std::list<ElfSection *>::iterator end() { return sections.end(); }
 
     void clear();
-
-    bool isElfHackFillerSegment() {
-      return type == PT_LOAD && flags == 0;
-    }
 private:
     unsigned int type;
     int v_p_diff; // Difference between physical and virtual address
     unsigned int flags;
     unsigned int align;
     std::list<ElfSection *> sections;
     // The following are only really used for PT_GNU_RELRO until something
     // better is found.