[llvm-objcopy][llvm-strip] Add switch to allow removing referenced sections

llvm-objcopy currently emits an error if a section to be removed is
referenced by another section. This is a reasonable thing to do, but is
different to GNU objcopy. We should allow users who know what they are
doing to have a way to produce the invalid ELF. This change adds a new
switch --allow-broken-links to both llvm-strip and llvm-objcopy to do
precisely that. The corresponding sh_link field is then set to 0 instead
of an error being emitted.

I cannot use llvm-readelf/readobj to test the link fields because they
emit an error if any sections, like the .dynsym, cannot be properly
loaded.

Reviewed by: rupprecht, grimar

Differential Revision: https://reviews.llvm.org/D60324

llvm-svn: 358649

test/tools/llvm-objcopy/ELF/dynsym-error-remove-strtab.test

@@ -1,3 +1,16 @@
-# RUN: not llvm-objcopy -R .dynstr %p/Inputs/dynsym.so %t 2>&1 >/dev/null | FileCheck %s
+# RUN: not llvm-objcopy -R .dynstr %p/Inputs/dynsym.so %t 2>&1 >/dev/null | FileCheck %s --check-prefix=ERR
+# RUN: cp  %p/Inputs/dynsym.so %t2
+## Use --strip-debug to suppress the default --strip-all behavior of llvm-strip.
+## TODO: Implement a better way to suppress --strip-all behavior.
+# RUN: not llvm-strip --strip-debug -R .dynstr %t2 2>&1 >/dev/null | FileCheck %s --check-prefix=ERR
 
-# CHECK: Section .dynstr cannot be removed because it is referenced by the section .dynsym
+# ERR: Section .dynstr cannot be removed because it is referenced by the section .dynsym
+
+# RUN: llvm-objcopy --allow-broken-links -R .dynstr %p/Inputs/dynsym.so %t3
+# RUN: llvm-objdump --section-headers %t3 | FileCheck %s --check-prefix=SECTIONS --implicit-check-not=.dynstr
+# RUN: cp  %p/Inputs/dynsym.so %t4
+# RUN: llvm-strip --strip-debug --allow-broken-links -R .dynstr %t4
+# RUN: llvm-objdump --section-headers %t4 | FileCheck %s --check-prefix=SECTIONS --implicit-check-not=.dynstr
+
+# SECTIONS: .dynsym
+# SECTIONS: .dynamic

test/tools/llvm-objcopy/ELF/reloc-error-remove-symtab.test

@@ -1,5 +1,9 @@
 # RUN: yaml2obj %s > %t
-# RUN: not llvm-objcopy -R .symtab %t %t2 2>&1 >/dev/null | FileCheck %s
+# RUN: not llvm-objcopy -R .symtab %t %t2 2>&1 >/dev/null | FileCheck %s --check-prefix=ERR
+# RUN: cp %t %t3
+## Use --strip-debug to suppress the default --strip-all behavior of llvm-strip.
+## TODO: Implement a better way to suppress --strip-all behavior.
+# RUN: not llvm-strip --strip-debug -R .symtab %t3 2>&1 >/dev/null | FileCheck %s --check-prefix=ERR
 
 !ELF
 FileHeader:
@@ -29,4 +33,14 @@ Symbols:
     Size:     4
     Binding:  STB_GLOBAL
 
-# CHECK: Symbol table .symtab cannot be removed because it is referenced by the relocation section .rel.text.
+# ERR: Symbol table .symtab cannot be removed because it is referenced by the relocation section .rel.text.
+
+# RUN: llvm-objcopy --allow-broken-links -R .symtab %t %t4
+# RUN: llvm-readobj --sections %t4 | FileCheck %s --check-prefix=SECTIONS --implicit-check-not=.symtab
+# RUN: cp %t %t5
+# RUN: llvm-strip --strip-debug --allow-broken-links -R .symtab %t5
+# RUN: llvm-readobj --sections %t5 | FileCheck %s --check-prefix=SECTIONS --implicit-check-not=.symtab
+
+# SECTIONS:        Name: .rel.text
+# SECTIONS:        Link
+# SECTIONS-SAME: : 0

test/tools/llvm-objcopy/ELF/remove-linked-section.test

@@ -0,0 +1,31 @@
+# RUN: yaml2obj %s -o %t.o
+# RUN: not llvm-objcopy -R .foo %t.o %t1 2>&1 >/dev/null | FileCheck %s --check-prefix=ERR
+# RUN: cp %t.o %t2
+## Use --strip-debug to suppress the default --strip-all behavior of llvm-strip.
+## TODO: Implement a better way to suppress --strip-all behavior.
+# RUN: not llvm-strip --strip-debug -R .foo %t2 2>&1 >/dev/null | FileCheck %s --check-prefix=ERR
+
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .foo
+    Type:            SHT_PROGBITS
+  - Name:            .bar
+    Type:            SHT_PROGBITS
+    Link:            .foo
+
+# ERR: Section .foo cannot be removed because it is referenced by the section .bar
+
+# RUN: llvm-objcopy --allow-broken-links -R .foo %t.o %t3
+# RUN: llvm-readobj --sections %t3 | FileCheck %s --check-prefix=SECTIONS --implicit-check-not=.foo
+# RUN: cp %t.o %t4
+# RUN: llvm-strip --strip-debug --allow-broken-links -R .foo %t4
+# RUN: llvm-readobj --sections %t4 | FileCheck %s --check-prefix=SECTIONS --implicit-check-not=.foo
+
+# SECTIONS:      Name: .bar
+# SECTIONS:      Link
+# SECTIONS-SAME: : 0

test/tools/llvm-objcopy/ELF/symtab-error-on-remove-strtab.test

@@ -1,5 +1,9 @@
 # RUN: yaml2obj %s > %t
-# RUN: not llvm-objcopy -R .strtab %t %t2 2>&1 >/dev/null | FileCheck %s
+# RUN: not llvm-objcopy -R .strtab %t %t2 2>&1 >/dev/null | FileCheck %s --check-prefix=ERR
+# RUN: cp %t %t3
+## Use --strip-debug to suppress the default --strip-all behavior of llvm-strip.
+## TODO: Implement a better way to suppress --strip-all behavior.
+# RUN: not llvm-strip --strip-debug -R .strtab %t3 2>&1 >/dev/null | FileCheck %s --check-prefix=ERR
 
 !ELF
 FileHeader:
@@ -8,4 +12,12 @@ FileHeader:
   Type:            ET_REL
   Machine:         EM_X86_64
 
-# CHECK: String table .strtab cannot be removed because it is referenced by the symbol table .symtab
+# ERR: String table .strtab cannot be removed because it is referenced by the symbol table .symtab
+
+# RUN: llvm-objcopy --allow-broken-links -R .strtab %t %t4
+# RUN: llvm-objdump --section-headers %t4 | FileCheck %s --check-prefix=SECTIONS --implicit-check-not=.strtab
+# RUN: cp %t %t5
+# RUN: llvm-strip --strip-debug --allow-broken-links -R .strtab %t %t5
+# RUN: llvm-objdump --section-headers %t5 | FileCheck %s --check-prefix=SECTIONS --implicit-check-not=.strtab
+
+# SECTIONS: .symtab

tools/llvm-objcopy/COFF/COFFObjcopy.cpp

@@ -174,17 +174,18 @@ static Error handleArgs(const CopyConfig &Config, Object &Obj) {
   if (!Config.AddGnuDebugLink.empty())
     addGnuDebugLink(Obj, Config.AddGnuDebugLink);
 
-  if (!Config.BuildIdLinkDir.empty() || Config.BuildIdLinkInput ||
-      Config.BuildIdLinkOutput || !Config.SplitDWO.empty() ||
-      !Config.SymbolsPrefix.empty() || !Config.AddSection.empty() ||
-      !Config.DumpSection.empty() || !Config.KeepSection.empty() ||
-      !Config.SymbolsToGlobalize.empty() || !Config.SymbolsToKeep.empty() ||
-      !Config.SymbolsToLocalize.empty() || !Config.SymbolsToWeaken.empty() ||
-      !Config.SymbolsToKeepGlobal.empty() || !Config.SectionsToRename.empty() ||
-      !Config.SetSectionFlags.empty() || !Config.SymbolsToRename.empty() ||
-      Config.ExtractDWO || Config.KeepFileSymbols || Config.LocalizeHidden ||
-      Config.PreserveDates || Config.StripDWO || Config.StripNonAlloc ||
-      Config.StripSections || Config.Weaken || Config.DecompressDebugSections ||
+  if (Config.AllowBrokenLinks || !Config.BuildIdLinkDir.empty() ||
+      Config.BuildIdLinkInput || Config.BuildIdLinkOutput ||
+      !Config.SplitDWO.empty() || !Config.SymbolsPrefix.empty() ||
+      !Config.AddSection.empty() || !Config.DumpSection.empty() ||
+      !Config.KeepSection.empty() || !Config.SymbolsToGlobalize.empty() ||
+      !Config.SymbolsToKeep.empty() || !Config.SymbolsToLocalize.empty() ||
+      !Config.SymbolsToWeaken.empty() || !Config.SymbolsToKeepGlobal.empty() ||
+      !Config.SectionsToRename.empty() || !Config.SetSectionFlags.empty() ||
+      !Config.SymbolsToRename.empty() || Config.ExtractDWO ||
+      Config.KeepFileSymbols || Config.LocalizeHidden || Config.PreserveDates ||
+      Config.StripDWO || Config.StripNonAlloc || Config.StripSections ||
+      Config.Weaken || Config.DecompressDebugSections ||
       Config.DiscardMode == DiscardType::Locals ||
       !Config.SymbolsToAdd.empty() || Config.EntryExpr) {
     return createStringError(llvm::errc::invalid_argument,

tools/llvm-objcopy/CopyConfig.cpp

@@ -621,6 +621,8 @@ Expected<DriverConfig> parseObjcopyOptions(ArrayRef<const char *> ArgsArr) {
     Config.SymbolsToAdd.push_back(*NSI);
   }
 
+  Config.AllowBrokenLinks = InputArgs.hasArg(OBJCOPY_allow_broken_links);
+
   Config.DeterministicArchives = InputArgs.hasFlag(
       OBJCOPY_enable_deterministic_archives,
       OBJCOPY_disable_deterministic_archives, /*default=*/true);
@@ -707,6 +709,7 @@ Expected<DriverConfig> parseStripOptions(ArrayRef<const char *> ArgsArr) {
 
   CopyConfig Config;
   bool UseRegexp = InputArgs.hasArg(STRIP_regex);
+  Config.AllowBrokenLinks = InputArgs.hasArg(STRIP_allow_broken_links);
   Config.StripDebug = InputArgs.hasArg(STRIP_strip_debug);
 
   if (InputArgs.hasArg(STRIP_discard_all, STRIP_discard_locals))

tools/llvm-objcopy/CopyConfig.h

@@ -149,6 +149,7 @@ struct CopyConfig {
   std::function<uint64_t(uint64_t)> EntryExpr;
 
   // Boolean options
+  bool AllowBrokenLinks = false;
   bool DeterministicArchives = true;
   bool ExtractDWO = false;
   bool KeepFileSymbols = false;

tools/llvm-objcopy/ELF/ELFObjcopy.cpp

@@ -250,7 +250,8 @@ static Error splitDWOToFile(const CopyConfig &Config, const Reader &Reader,
   auto OnlyKeepDWOPred = [&DWOFile](const SectionBase &Sec) {
     return onlyKeepDWOPred(*DWOFile, Sec);
   };
-  if (Error E = DWOFile->removeSections(OnlyKeepDWOPred))
+  if (Error E = DWOFile->removeSections(Config.AllowBrokenLinks,
+                                        OnlyKeepDWOPred))
     return E;
   if (Config.OutputArch) {
     DWOFile->Machine = Config.OutputArch.getValue().EMachine;
@@ -547,7 +548,7 @@ static Error replaceAndRemoveSections(const CopyConfig &Config, Object &Obj) {
           return &Obj.addSection<DecompressedSection>(*CS);
         });
 
-  return Obj.removeSections(RemovePred);
+  return Obj.removeSections(Config.AllowBrokenLinks, RemovePred);
 }
 
 // This function handles the high level operations of GNU objcopy including

tools/llvm-objcopy/ELF/Object.cpp

@@ -51,6 +51,7 @@ template <class ELFT> void ELFWriter<ELFT>::writePhdr(const Segment &Seg) {
 }
 
 Error SectionBase::removeSectionReferences(
+    bool AllowBrokenLinks,
     function_ref<bool(const SectionBase *)> ToRemove) {
   return Error::success();
 }
@@ -424,14 +425,19 @@ void SymbolTableSection::addSymbol(Twine Name, uint8_t Bind, uint8_t Type,
 }
 
 Error SymbolTableSection::removeSectionReferences(
+    bool AllowBrokenLinks,
     function_ref<bool(const SectionBase *)> ToRemove) {
   if (ToRemove(SectionIndexTable))
     SectionIndexTable = nullptr;
-  if (ToRemove(SymbolNames))
-    return createStringError(llvm::errc::invalid_argument,
-                             "String table %s cannot be removed because it is "
-                             "referenced by the symbol table %s",
-                             SymbolNames->Name.data(), this->Name.data());
+  if (ToRemove(SymbolNames)) {
+    if (!AllowBrokenLinks)
+      return createStringError(
+          llvm::errc::invalid_argument,
+          "String table %s cannot be removed because it is "
+          "referenced by the symbol table %s",
+          SymbolNames->Name.data(), this->Name.data());
+    SymbolNames = nullptr;
+  }
   return removeSymbols(
       [ToRemove](const Symbol &Sym) { return ToRemove(Sym.DefinedIn); });
 }
@@ -476,12 +482,13 @@ void SymbolTableSection::initialize(SectionTableRef SecTable) {
 void SymbolTableSection::finalize() {
   uint32_t MaxLocalIndex = 0;
   for (auto &Sym : Symbols) {
-    Sym->NameIndex = SymbolNames->findIndex(Sym->Name);
+    Sym->NameIndex =
+        SymbolNames == nullptr ? 0 : SymbolNames->findIndex(Sym->Name);
     if (Sym->Binding == STB_LOCAL)
       MaxLocalIndex = std::max(MaxLocalIndex, Sym->Index);
   }
   // Now we need to set the Link and Info fields.
-  Link = SymbolNames->Index;
+  Link = SymbolNames == nullptr ? 0 : SymbolNames->Index;
   Info = MaxLocalIndex + 1;
 }
 
@@ -491,10 +498,14 @@ void SymbolTableSection::prepareForLayout() {
   // indexes later in fillShdnxTable.
   if (SectionIndexTable)  
     SectionIndexTable->reserve(Symbols.size());
+
   // Add all of our strings to SymbolNames so that SymbolNames has the right
   // size before layout is decided.
-  for (auto &Sym : Symbols)
-    SymbolNames->addString(Sym->Name);
+  // If the symbol names section has been removed, don't try to add strings to
+  // the table.
+  if (SymbolNames != nullptr)
+    for (auto &Sym : Symbols)
+      SymbolNames->addString(Sym->Name);
 }
 
 void SymbolTableSection::fillShndxTable() {
@@ -548,12 +559,17 @@ void SymbolTableSection::accept(MutableSectionVisitor &Visitor) {
 }
 
 Error RelocationSection::removeSectionReferences(
+    bool AllowBrokenLinks,
     function_ref<bool(const SectionBase *)> ToRemove) {
-  if (ToRemove(Symbols))
-    return createStringError(llvm::errc::invalid_argument,
-                             "Symbol table %s cannot be removed because it is "
-                             "referenced by the relocation section %s.",
-                             Symbols->Name.data(), this->Name.data());
+  if (ToRemove(Symbols)) {
+    if (!AllowBrokenLinks)
+      return createStringError(
+          llvm::errc::invalid_argument,
+          "Symbol table %s cannot be removed because it is "
+          "referenced by the relocation section %s.",
+          Symbols->Name.data(), this->Name.data());
+    Symbols = nullptr;
+  }
 
   for (const Relocation &R : Relocations) {
     if (!R.RelocSymbol->DefinedIn || !ToRemove(R.RelocSymbol->DefinedIn))
@@ -667,13 +683,16 @@ void DynamicRelocationSection::accept(MutableSectionVisitor &Visitor) {
   Visitor.visit(*this);
 }
 
-Error Section::removeSectionReferences(
+Error Section::removeSectionReferences(bool AllowBrokenDependency,
     function_ref<bool(const SectionBase *)> ToRemove) {
-  if (ToRemove(LinkSection))
-    return createStringError(llvm::errc::invalid_argument,
-                             "Section %s cannot be removed because it is "
-                             "referenced by the section %s",
-                             LinkSection->Name.data(), this->Name.data());
+  if (ToRemove(LinkSection)) {
+    if (!AllowBrokenDependency)
+      return createStringError(llvm::errc::invalid_argument,
+                               "Section %s cannot be removed because it is "
+                               "referenced by the section %s",
+                               LinkSection->Name.data(), this->Name.data());
+    LinkSection = nullptr;
+  }
   return Error::success();
 }
 
@@ -1387,7 +1406,7 @@ template <class ELFT>
 ELFWriter<ELFT>::ELFWriter(Object &Obj, Buffer &Buf, bool WSH)
     : Writer(Obj, Buf), WriteSectionHeaders(WSH && Obj.HadShdrs) {}
 
-Error Object::removeSections(
+Error Object::removeSections(bool AllowBrokenLinks,
     std::function<bool(const SectionBase &)> ToRemove) {
 
   auto Iter = std::stable_partition(
@@ -1423,7 +1442,7 @@ Error Object::removeSections(
   // a live section critically depends on a section being removed somehow
   // (e.g. the removed section is referenced by a relocation).
   for (auto &KeepSec : make_range(std::begin(Sections), Iter)) {
-    if (Error E = KeepSec->removeSectionReferences(
+    if (Error E = KeepSec->removeSectionReferences(AllowBrokenLinks,
             [&RemoveSections](const SectionBase *Sec) {
               return RemoveSections.find(Sec) != RemoveSections.end();
             }))
@@ -1629,9 +1648,11 @@ template <class ELFT> Error ELFWriter<ELFT>::finalize() {
     // Since we don't need SectionIndexTable we should remove it and all
     // references to it.
     if (Obj.SectionIndexTable != nullptr) {
-      if (Error E = Obj.removeSections([this](const SectionBase &Sec) {
-            return &Sec == Obj.SectionIndexTable;
-          }))
+      // We do not support sections referring to the section index table.
+      if (Error E = Obj.removeSections(false /*AllowBrokenLinks*/,
+                                       [this](const SectionBase &Sec) {
+                                         return &Sec == Obj.SectionIndexTable;
+                                       }))
         return E;
     }
   }

tools/llvm-objcopy/ELF/Object.h

@@ -276,7 +276,8 @@ public:
   virtual void finalize();
   // Remove references to these sections. The list of sections must be sorted.
   virtual Error
-  removeSectionReferences(function_ref<bool(const SectionBase *)> ToRemove);
+  removeSectionReferences(bool AllowBrokenLinks,
+                          function_ref<bool(const SectionBase *)> ToRemove);
   virtual Error removeSymbols(function_ref<bool(const Symbol &)> ToRemove);
   virtual void accept(SectionVisitor &Visitor) const = 0;
   virtual void accept(MutableSectionVisitor &Visitor) = 0;
@@ -341,7 +342,7 @@ public:
 
   void accept(SectionVisitor &Visitor) const override;
   void accept(MutableSectionVisitor &Visitor) override;
-  Error removeSectionReferences(
+  Error removeSectionReferences(bool AllowBrokenLinks,
       function_ref<bool(const SectionBase *)> ToRemove) override;
   void initialize(SectionTableRef SecTable) override;
   void finalize() override;
@@ -535,7 +536,7 @@ public:
   Symbol *getSymbolByIndex(uint32_t Index);
   void updateSymbols(function_ref<void(Symbol &)> Callable);
 
-  Error removeSectionReferences(
+  Error removeSectionReferences(bool AllowBrokenLinks,
       function_ref<bool(const SectionBase *)> ToRemove) override;
   void initialize(SectionTableRef SecTable) override;
   void finalize() override;
@@ -605,7 +606,7 @@ public:
   void addRelocation(Relocation Rel) { Relocations.push_back(Rel); }
   void accept(SectionVisitor &Visitor) const override;
   void accept(MutableSectionVisitor &Visitor) override;
-  Error removeSectionReferences(
+  Error removeSectionReferences(bool AllowBrokenLinks,
       function_ref<bool(const SectionBase *)> ToRemove) override;
   Error removeSymbols(function_ref<bool(const Symbol &)> ToRemove) override;
   void markSymbols() override;
@@ -835,7 +836,8 @@ public:
   Range<Segment> segments() { return make_pointee_range(Segments); }
   ConstRange<Segment> segments() const { return make_pointee_range(Segments); }
 
-  Error removeSections(std::function<bool(const SectionBase &)> ToRemove);
+  Error removeSections(bool AllowBrokenLinks,
+                       std::function<bool(const SectionBase &)> ToRemove);
   Error removeSymbols(function_ref<bool(const Symbol &)> ToRemove);
   template <class T, class... Ts> T &addSection(Ts &&... Args) {
     auto Sec = llvm::make_unique<T>(std::forward<Ts>(Args)...);

tools/llvm-objcopy/ObjcopyOpts.td

@@ -9,6 +9,12 @@ multiclass Eq<string name, string help> {
 
 def help : Flag<["-", "--"], "help">;
 
+def allow_broken_links
+    : Flag<["-", "--"], "allow-broken-links">,
+      HelpText<"Allow llvm-objcopy to remove sections even if it would leave "
+               "invalid section references. The appropriate sh_link fields"
+               "will be set to zero.">;
+
 defm binary_architecture
     : Eq<"binary-architecture", "Used when transforming an architecture-less "
                                 "format (such as binary) to another format">;

tools/llvm-objcopy/StripOpts.td

@@ -9,6 +9,12 @@ multiclass Eq<string name, string help> {
 
 def help : Flag<["-", "--"], "help">;
 
+def allow_broken_links
+    : Flag<["-", "--"], "allow-broken-links">,
+      HelpText<"Allow llvm-strip to remove sections even if it would leave "
+               "invalid section references. The appropriate sh_link fields"
+               "will be set to zero.">;
+
 def enable_deterministic_archives
     : Flag<["-", "--"], "enable-deterministic-archives">,
       HelpText<"Enable deterministic mode when stripping archives (use zero "