[llvm-objcopy] Add support for --strip-sections to remove all section headers leaving only program headers and loadable segment data

[jakehehrlich]

Summary

elf utils implements a particularly extreme form of stripping that I'd like to support. eu-strip has an option called "strip-sections" that removes all section headers and leaves only program headers and the segment data. I have implemented this option partly as a test but mainly because in Fuchsia we would like to use this option to minimize the size of our executables. The other strip options that are on my list include --strip-all and --strip-debug. This is a preliminary implementation that I'd like to start using in Fuchsia builds if possible. This change implements such a stripping option for llvm-objcopy

[jhenderson]

I definitely support the new switch, but there are a few issues with this change as it stands:

As mentioned inline, I think quite a few of these changes are not specific to removing all section headers, but rather should be in D38260.

Also, I'm not convinced that removing all sections is the same as removing all section headers, i.e. -R on every section is subtly different from --strip-sections. This is because removing sections one-by-one should still leave the null section header, in my opinion, whereas --strip-sections shouldn't. Essentially there's a slight semantic difference in an ELF without a section header table, and one with no sections.

Finally, there's currently no test for trying to strip the section header string table directly, if there are any remaining sections. I'm not sure it should be an error anyway - it looks like GNU binutils at the very least can handle an ELF with no section header string table, but with section headers, without too much problem, although I'd expect the e_shstrndx field to be set to 0 in this case.

test/tools/llvm-objcopy/strip-sections.test

1

This test needs to test the ELF header fields as well.

tools/llvm-objcopy/Object.cpp

630

Full stop.

Why do we need the first loop here? What was wrong with the previous behaviour? If the answer is because the section data should be preserved in the PT_LOAD segment regardless of whether a section is stripped or not, then shouldn't this change also be present in D38260?

660–661

This sounds like it's not limited to this change? What's stopping us using -R to strip the .symtab (as long as we also strip any other sections referring to it)?

662–670

Ditto - should this not be part of D38260?

[jakehehrlich]

After some thought I agree. Here's the plan I propose

1. Update D38260 to add the supported features (including targeted tests that don't just remove everything)
2. rebase those changes into this change
3. Add a Boolean for weather or not to output a section table
4. Have --strip-sections remove all sections *and* set that boolean.

An interesting option from there might be to implement a flag to toggle weather or not the section table is output from there. "llvm-objcopy --strip-sections" would then become equivalent to "llvm-objcopy --no-section-headers -R *" if we were implement globs for section names in section removal.

[jakehehrlich]

1. Added boolean flag so that removing sections isn't tied to not having a section header.
2. Moved some of these changes into the -R change where they were more appropriate
3. This change should be the official change where writing out files without section headers is added.

[jhenderson]

In D38335#887684, @jakehehrlich wrote:

An interesting option from there might be to implement a flag to toggle weather or not the section table is output from there. "llvm-objcopy --strip-sections" would then become equivalent to "llvm-objcopy --no-section-headers -R *" if we were implement globs for section names in section removal.

I would have no problem with this.

test/tools/llvm-objcopy/strip-sections.test

1

For clarity, I mean the e_shnum, e_shoff, and e_shstrndx fields.

tools/llvm-objcopy/Object.cpp

763

We don't need to do this if there is no section header table.

tools/llvm-objcopy/llvm-objcopy.cpp

78–85

It feels like there's a lot of overlap here. Would it be possible to fold the two cases together for this bit? I.e. the predicate becomes "if StripSections or in ToRemove list".

[jakehehrlich]

1. rebased latest remove section changes
2. Made sure SHOffset wasn't bumped too far if no section header was going to be output

I realize I haven't added the ELF header stuff to the test. I need to go now but I'll do that tomorrow.

[jhenderson]

This all looks good to me, ignoring the pending test change.

[jakehehrlich]

Fixed missing "RUN" in test that I accidentally left in there.

[jhenderson]

LGTM.

test/tools/llvm-objcopy/strip-sections.test

29

FWIW, I don't think you need to check all the fields, just the section header ones, but if you think it's useful, I'm not opposed to it.

[jakehehrlich]

test/tools/llvm-objcopy/strip-sections.test

29

To be perfectly honest I just copy and paste the headers for %t and then make the changes that should be made so that they match what %t2 should be. So this is me just being lazy not me covering bases. I don't think it's harming anything so I'll just leave it.

test/tools/llvm-objcopy/strip-sections.test

+# RUN: yaml2obj %s > %t
+# RUN: llvm-objcopy --strip-sections %t %t2
+# RUN: llvm-readobj -program-headers %t2 | FileCheck %s
+# RUN: od -t x1 -j 4096 %t2 | FileCheck %s --check-prefix=DATA
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x0000000000001000
+    Content:         "DEADBEEF"
+ProgramHeaders:
+- Type: PT_LOAD
+  Flags: [ PF_X, PF_R ]
+  Sections:
+    - Section: .text
+
+#DATA: 0010000 de ad be ef
+
+#CHECK:     ProgramHeaders [
+#CHECK-NEXT:  ProgramHeader {
+#CHECK-NEXT:    Type: PT_LOAD (0x1)
+#CHECK-NEXT:    Offset: 0x1000
+#CHECK-NEXT:    VirtualAddress: 0x0
+#CHECK-NEXT:    PhysicalAddress: 0x0
+#CHECK-NEXT:    FileSize: 4
+#CHECK-NEXT:    MemSize: 4
+#CHECK-NEXT:    Flags [ (0x5)
+#CHECK-NEXT:      PF_R (0x4)
+#CHECK-NEXT:      PF_X (0x1)
+#CHECK-NEXT:    ]
+#CHECK-NEXT:    Alignment: 4096
+#CHECK-NEXT:  }
+#CHECK-NEXT:]

tools/llvm-objcopy/Object.cpp

void Object<ELFT>::writeHeader(FileOutputBuffer &Out) const {
   uint8_t *Buf = Out.getBufferStart();
   Elf_Ehdr &Ehdr = *reinterpret_cast<Elf_Ehdr *>(Buf);
   std::copy(Ident, Ident + 16, Ehdr.e_ident);
   Ehdr.e_type = Type;
   Ehdr.e_machine = Machine;
   Ehdr.e_version = Version;
   Ehdr.e_entry = Entry;
   Ehdr.e_phoff = sizeof(Elf_Ehdr);
-  Ehdr.e_shoff = SHOffset;
   Ehdr.e_flags = Flags;
   Ehdr.e_ehsize = sizeof(Elf_Ehdr);
   Ehdr.e_phentsize = sizeof(Elf_Phdr);
   Ehdr.e_phnum = Segments.size();
   Ehdr.e_shentsize = sizeof(Elf_Shdr);
+  if (SectionNames != nullptr) {
+    Ehdr.e_shoff = SHOffset;
     Ehdr.e_shnum = Sections.size() + 1;
     Ehdr.e_shstrndx = SectionNames->Index;
+  } else {
+    Ehdr.e_shoff = 0;
+    Ehdr.e_shnum = 0;
+    Ehdr.e_shstrndx = 0;
+  }
 }
 
 template <class ELFT>
 void Object<ELFT>::writeProgramHeaders(FileOutputBuffer &Out) const {
   for (auto &Phdr : Segments)
     Phdr->template writeHeader<ELFT>(Out);
 }
 
void Object<ELFT>::writeSectionHeaders(FileOutputBuffer &Out) const {
   Shdr.sh_entsize = 0;
 
   for (auto &Section : Sections)
     Section->template writeHeader<ELFT>(Out);
 }
 
 template <class ELFT>
 void Object<ELFT>::writeSectionData(FileOutputBuffer &Out) const {
-  for (auto &Section : Sections)
+  // First write segment data that needs to be loaded
+  for (auto &Segment : this->Segments) {
+    if (Segment->Type == llvm::ELF::PT_LOAD) {
+      Segment->writeSegment(Out);
+    }
+  }
+  for (auto &Section : Sections) {
+    // Since we already write PT_LOAD segment data before this we don't need to
+    // write the data a second time.
+    if (Section->ParentSegment == nullptr ||
+        Section->ParentSegment->Type != PT_LOAD)
       Section->writeSection(Out);
   }
+}
 
 template <class ELFT>
 void Object<ELFT>::removeSections(
     std::function<bool(const SectionBase &)> ToRemove) {
 
   auto Iter =
       std::stable_partition(std::begin(Sections), std::end(Sections),
                             [=](const SecPtr &Sec) {
         if (ToRemove(*Sec))
           return false;
         if (auto RelSec = dyn_cast<RelocationSectionBase>(Sec.get()))
           return !ToRemove(*RelSec->getSection());
         return true;
       });
+  // The Object type maintains some references to certian special sections. We
+  // can remove those as well though and the output should still be sensible.
+  if (ToRemove(*SymbolTable))
+    SymbolTable = nullptr;
+  if (ToRemove(*SectionNames)) {
+    // It only makes sense to remove the section header names if we also remove
+    // all sections.
+    if (Iter != Sections.begin())
+      error("Cannot remove " + SectionNames->Name +
+            " because it is the section header string table and some sections"
+            " are being left in the section header table");
+    SectionNames = nullptr;
+  }
   // Now make sure there are no remaining references to the sections that will
   // be removed. Sometimes it is impossible to remove a reference so we emit
   // an error here instead.
   for (auto &RemoveSec : make_range(Iter, std::end(Sections))) {
     for (auto &Segment : Segments)
       Segment->removeSection(RemoveSec.get());
     for (auto &KeepSec : make_range(std::begin(Sections), Iter))
       KeepSec->removeSectionReferences(RemoveSec.get());
for (auto &Section : this->Sections) {
     } else {
       Offset = alignTo(Offset, Section->Offset);
       Section->Offset = Offset;
       if (Section->Type != SHT_NOBITS)
         Offset += Section->Size;
     }
   }
 
   Offset = alignTo(Offset, sizeof(typename ELFT::Addr));
   this->SHOffset = Offset;
 }
 
 template <class ELFT> size_t ELFObject<ELFT>::totalSize() const {
   // We already have the section header offset so we can calculate the total
   // size by just adding up the size of each section header.
+  auto NullSectionSize = this->SectionNames != nullptr ? sizeof(Elf_Shdr) : 0;
   return this->SHOffset + this->Sections.size() * sizeof(Elf_Shdr) +
-         sizeof(Elf_Shdr);
+         NullSectionSize;
 }
 
 template <class ELFT> void ELFObject<ELFT>::write(FileOutputBuffer &Out) const {
   this->writeHeader(Out);
   this->writeProgramHeaders(Out);
   this->writeSectionData(Out);
+  if (this->SectionNames != nullptr)
     this->writeSectionHeaders(Out);
 }
 
 template <class ELFT> void ELFObject<ELFT>::finalize() {
   // Make sure we add the names of all the sections.
+  if (this->SectionNames != nullptr)
     for (const auto &Section : this->Sections) {
       this->SectionNames->addString(Section->Name);
     }
   // Make sure we add the names of all the symbols.
+  if (this->SymbolTable != nullptr)
     this->SymbolTable->addSymbolNames();
 
   sortSections();
   assignOffsets();
 
   // Finalize SectionNames first so that we can assign name indexes.
+  if (this->SectionNames != nullptr)
     this->SectionNames->finalize();
   // Finally now that all offsets and indexes have been set we can finalize any
   // remaining issues.
   uint64_t Offset = this->SHOffset + sizeof(Elf_Shdr);
   for (auto &Section : this->Sections) {
     Section->HeaderOffset = Offset;
     Offset += sizeof(Elf_Shdr);
     Section->NameIndex = this->SectionNames->findIndex(Section->Name);
     Section->finalize();

tools/llvm-objcopy/llvm-objcopy.cpp

 }
 
 cl::opt<std::string> InputFilename(cl::Positional, cl::desc("<input>"));
 cl::opt<std::string> OutputFilename(cl::Positional, cl::desc("<output>"),
                                     cl::init("-"));
 cl::opt<std::string>
     OutputFormat("O", cl::desc("set output format to one of the following:"
                                "\n\tbinary"));
-// TODO: make this a cl::list to support removing multiple sections
 cl::list<std::string> ToRemove("remove-section",
                                cl::desc("Remove a specific section"));
-cl::alias ToRemoveA("R", cl::desc("Alias for -quiet"), cl::aliasopt(ToRemove));
+cl::alias ToRemoveA("R", cl::desc("Alias for remove-section"),
+                    cl::aliasopt(ToRemove));
+cl::opt<bool> StripSections("strip-sections",
+                            cl::desc("Remove all section headers"));
+
+typedef std::function<bool(const SectionBase& Sec)> SectionPred;
 
 void CopyBinary(const ELFObjectFile<ELF64LE> &ObjFile) {
   std::unique_ptr<FileOutputBuffer> Buffer;
   std::unique_ptr<Object<ELF64LE>> Obj;
   if (!OutputFormat.empty() && OutputFormat != "binary")
     error("invalid output format '" + OutputFormat + "'");
   if (!OutputFormat.empty() && OutputFormat == "binary")
     Obj = llvm::make_unique<BinaryObject<ELF64LE>>(ObjFile);
   else
     Obj = llvm::make_unique<ELFObject<ELF64LE>>(ObjFile);
+
+  SectionPred RemovePred = [](const SectionBase &) { return false; };
+
   if (!ToRemove.empty()) {
-    Obj->removeSections([&](const SectionBase &Sec) {
+    RemovePred = [&](const SectionBase &Sec) {
       return std::find(std::begin(ToRemove), std::end(ToRemove), Sec.Name) !=
              std::end(ToRemove);
-    });
+    };
   }
+
+  if (StripSections) {
+    RemovePred = [](const SectionBase &) { return true; };
+  }
+
+  Obj->removeSections(RemovePred);
+
   Obj->finalize();
   ErrorOr<std::unique_ptr<FileOutputBuffer>> BufferOrErr =
       FileOutputBuffer::create(OutputFilename, Obj->totalSize(),
                                FileOutputBuffer::F_executable);
   if (BufferOrErr.getError())
     error("failed to open " + OutputFilename);
   else
     Buffer = std::move(*BufferOrErr);