[llvm-objcopy][NFC] Propagate errors in removeSymbols/removeSectionReferences

Reviewers: jhenderson, alexshap, jakehehrlich, espindola

Reviewed By: jhenderson

Subscribers: emaste, arichardson, llvm-commits

Tags: #llvm

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

llvm-svn: 352877

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

@@ -185,8 +185,11 @@ static Error linkToBuildIdDir(const CopyConfig &Config, StringRef ToLink,
 static Error splitDWOToFile(const CopyConfig &Config, const Reader &Reader,
                             StringRef File, ElfType OutputElfType) {
   auto DWOFile = Reader.create();
-  DWOFile->removeSections(
-      [&](const SectionBase &Sec) { return onlyKeepDWOPred(*DWOFile, Sec); });
+  auto OnlyKeepDWOPred = [&DWOFile](const SectionBase &Sec) {
+    return onlyKeepDWOPred(*DWOFile, Sec);
+  };
+  if (Error E = DWOFile->removeSections(OnlyKeepDWOPred))
+    return E;
   if (Config.OutputArch)
     DWOFile->Machine = Config.OutputArch.getValue().EMachine;
   FileBuffer FB(File);
@@ -338,7 +341,7 @@ static Error handleArgs(const CopyConfig &Config, Object &Obj,
         Section.markSymbols();
     }
 
-    Obj.removeSymbols([&](const Symbol &Sym) {
+    auto RemoveSymbolsPred = [&](const Symbol &Sym) {
       if (is_contained(Config.SymbolsToKeep, Sym.Name) ||
           (Config.KeepFileSymbols && Sym.Type == STT_FILE))
         return false;
@@ -362,7 +365,9 @@ static Error handleArgs(const CopyConfig &Config, Object &Obj,
         return true;
 
       return false;
-    });
+    };
+    if (Error E = Obj.removeSymbols(RemoveSymbolsPred))
+      return E;
   }
 
   SectionPred RemovePred = [](const SectionBase &) { return false; };
@@ -496,7 +501,8 @@ static Error handleArgs(const CopyConfig &Config, Object &Obj,
           return &Obj.addSection<DecompressedSection>(*CS);
         });
 
-  Obj.removeSections(RemovePred);
+  if (Error E = Obj.removeSections(RemovePred))
+    return E;
 
   if (!Config.SectionsToRename.empty()) {
     for (auto &Sec : Obj.sections()) {

llvm/tools/llvm-objcopy/ELF/Object.cpp

@@ -17,6 +17,7 @@
 #include "llvm/MC/MCTargetOptions.h"
 #include "llvm/Object/ELFObjectFile.h"
 #include "llvm/Support/Compression.h"
+#include "llvm/Support/Errc.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileOutputBuffer.h"
 #include "llvm/Support/Path.h"
@@ -48,8 +49,14 @@ template <class ELFT> void ELFWriter<ELFT>::writePhdr(const Segment &Seg) {
   Phdr.p_align = Seg.Align;
 }
 
-void SectionBase::removeSectionReferences(const SectionBase *Sec) {}
-void SectionBase::removeSymbols(function_ref<bool(const Symbol &)> ToRemove) {}
+Error SectionBase::removeSectionReferences(const SectionBase *Sec) {
+  return Error::success();
+}
+
+Error SectionBase::removeSymbols(function_ref<bool(const Symbol &)> ToRemove) {
+  return Error::success();
+}
+
 void SectionBase::initialize(SectionTableRef SecTable) {}
 void SectionBase::finalize() {}
 void SectionBase::markSymbols() {}
@@ -425,15 +432,17 @@ void SymbolTableSection::addSymbol(Twine Name, uint8_t Bind, uint8_t Type,
   Size += this->EntrySize;
 }
 
-void SymbolTableSection::removeSectionReferences(const SectionBase *Sec) {
+Error SymbolTableSection::removeSectionReferences(const SectionBase *Sec) {
   if (SectionIndexTable == Sec)
     SectionIndexTable = nullptr;
   if (SymbolNames == Sec) {
-    error("String table " + SymbolNames->Name +
-          " cannot be removed because it is referenced by the symbol table " +
-          this->Name);
+    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());
   }
-  removeSymbols([Sec](const Symbol &Sym) { return Sym.DefinedIn == Sec; });
+  return removeSymbols(
+      [Sec](const Symbol &Sym) { return Sym.DefinedIn == Sec; });
 }
 
 void SymbolTableSection::updateSymbols(function_ref<void(Symbol &)> Callable) {
@@ -445,7 +454,7 @@ void SymbolTableSection::updateSymbols(function_ref<void(Symbol &)> Callable) {
   assignIndices();
 }
 
-void SymbolTableSection::removeSymbols(
+Error SymbolTableSection::removeSymbols(
     function_ref<bool(const Symbol &)> ToRemove) {
   Symbols.erase(
       std::remove_if(std::begin(Symbols) + 1, std::end(Symbols),
@@ -453,6 +462,7 @@ void SymbolTableSection::removeSymbols(
       std::end(Symbols));
   Size = Symbols.size() * EntrySize;
   assignIndices();
+  return Error::success();
 }
 
 void SymbolTableSection::initialize(SectionTableRef SecTable) {
@@ -535,15 +545,14 @@ void SymbolTableSection::accept(MutableSectionVisitor &Visitor) {
 }
 
 template <class SymTabType>
-void RelocSectionWithSymtabBase<SymTabType>::removeSectionReferences(
+Error RelocSectionWithSymtabBase<SymTabType>::removeSectionReferences(
     const SectionBase *Sec) {
-  if (Symbols == Sec) {
-    error("Symbol table " + Symbols->Name +
-          " cannot be removed because it is "
-          "referenced by the relocation "
-          "section " +
-          this->Name);
-  }
+  if (Symbols == Sec)
+    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());
+  return Error::success();
 }
 
 template <class SymTabType>
@@ -608,12 +617,15 @@ void RelocationSection::accept(MutableSectionVisitor &Visitor) {
   Visitor.visit(*this);
 }
 
-void RelocationSection::removeSymbols(
+Error RelocationSection::removeSymbols(
     function_ref<bool(const Symbol &)> ToRemove) {
   for (const Relocation &Reloc : Relocations)
     if (ToRemove(*Reloc.RelocSymbol))
-      error("not stripping symbol '" + Reloc.RelocSymbol->Name +
-            "' because it is named in a relocation");
+      return createStringError(
+          llvm::errc::invalid_argument,
+          "not stripping symbol '%s' because it is named in a relocation.",
+          Reloc.RelocSymbol->Name.data());
+  return Error::success();
 }
 
 void RelocationSection::markSymbols() {
@@ -634,13 +646,13 @@ void DynamicRelocationSection::accept(MutableSectionVisitor &Visitor) {
   Visitor.visit(*this);
 }
 
-void Section::removeSectionReferences(const SectionBase *Sec) {
-  if (LinkSection == Sec) {
-    error("Section " + LinkSection->Name +
-          " cannot be removed because it is "
-          "referenced by the section " +
-          this->Name);
-  }
+Error Section::removeSectionReferences(const SectionBase *Sec) {
+  if (LinkSection == Sec)
+    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());
+  return Error::success();
 }
 
 void GroupSection::finalize() {
@@ -648,13 +660,13 @@ void GroupSection::finalize() {
   this->Link = SymTab->Index;
 }
 
-void GroupSection::removeSymbols(function_ref<bool(const Symbol &)> ToRemove) {
-  if (ToRemove(*Sym)) {
-    error("Symbol " + Sym->Name +
-          " cannot be removed because it is "
-          "referenced by the section " +
-          this->Name + "[" + Twine(this->Index) + "]");
-  }
+Error GroupSection::removeSymbols(function_ref<bool(const Symbol &)> ToRemove) {
+  if (ToRemove(*Sym))
+    return createStringError(llvm::errc::invalid_argument,
+                             "Symbol %s cannot be removed because it is "
+                             "referenced by the section %s[%d].",
+                             Sym->Name.data(), this->Name.data(), this->Index);
+  return Error::success();
 }
 
 void GroupSection::markSymbols() {
@@ -1317,7 +1329,8 @@ template <class ELFT> void ELFWriter<ELFT>::writeSectionData() {
     Sec.accept(*SecWriter);
 }
 
-void Object::removeSections(std::function<bool(const SectionBase &)> ToRemove) {
+Error Object::removeSections(
+    std::function<bool(const SectionBase &)> ToRemove) {
 
   auto Iter = std::stable_partition(
       std::begin(Sections), std::end(Sections), [=](const SecPtr &Sec) {
@@ -1342,18 +1355,20 @@ void Object::removeSections(std::function<bool(const SectionBase &)> ToRemove) {
     for (auto &Segment : Segments)
       Segment->removeSection(RemoveSec.get());
     for (auto &KeepSec : make_range(std::begin(Sections), Iter))
-      KeepSec->removeSectionReferences(RemoveSec.get());
+      if (Error E = KeepSec->removeSectionReferences(RemoveSec.get()))
+        return E;
   }
   // Now finally get rid of them all togethor.
   Sections.erase(Iter, std::end(Sections));
+  return Error::success();
 }
 
-void Object::removeSymbols(function_ref<bool(const Symbol &)> ToRemove) {
-  if (!SymbolTable)
-    return;
-
+Error Object::removeSymbols(function_ref<bool(const Symbol &)> ToRemove) {
+  if (SymbolTable)
     for (const SecPtr &Sec : Sections)
-    Sec->removeSymbols(ToRemove);
+      if (Error E = Sec->removeSymbols(ToRemove))
+        return E;
+  return Error::success();
 }
 
 void Object::sortSections() {
@@ -1502,8 +1517,9 @@ template <class ELFT> Error ELFWriter<ELFT>::finalize() {
   // a section header table output. We need to throw an error if a user tries
   // to do that.
   if (Obj.SectionNames == nullptr && WriteSectionHeaders)
-    error("Cannot write section header table because section header string "
-          "table was removed.");
+    return createStringError(llvm::errc::invalid_argument,
+                             "Cannot write section header table because "
+                             "section header string table was removed.");
 
   Obj.sortSections();
 
@@ -1534,9 +1550,10 @@ 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) {
-      Obj.removeSections([this](const SectionBase &Sec) {
+      if (Error E = Obj.removeSections([this](const SectionBase &Sec) {
             return &Sec == Obj.SectionIndexTable;
-      });
+          }))
+        return E;
     }
   }
 

llvm/tools/llvm-objcopy/ELF/Object.h

@@ -273,8 +273,8 @@ public:
 
   virtual void initialize(SectionTableRef SecTable);
   virtual void finalize();
-  virtual void removeSectionReferences(const SectionBase *Sec);
-  virtual void removeSymbols(function_ref<bool(const Symbol &)> ToRemove);
+  virtual Error removeSectionReferences(const SectionBase *Sec);
+  virtual Error removeSymbols(function_ref<bool(const Symbol &)> ToRemove);
   virtual void accept(SectionVisitor &Visitor) const = 0;
   virtual void accept(MutableSectionVisitor &Visitor) = 0;
   virtual void markSymbols();
@@ -334,7 +334,7 @@ public:
 
   void accept(SectionVisitor &Visitor) const override;
   void accept(MutableSectionVisitor &Visitor) override;
-  void removeSectionReferences(const SectionBase *Sec) override;
+  Error removeSectionReferences(const SectionBase *Sec) override;
   void initialize(SectionTableRef SecTable) override;
   void finalize() override;
 };
@@ -521,12 +521,12 @@ public:
   Symbol *getSymbolByIndex(uint32_t Index);
   void updateSymbols(function_ref<void(Symbol &)> Callable);
 
-  void removeSectionReferences(const SectionBase *Sec) override;
+  Error removeSectionReferences(const SectionBase *Sec) override;
   void initialize(SectionTableRef SecTable) override;
   void finalize() override;
   void accept(SectionVisitor &Visitor) const override;
   void accept(MutableSectionVisitor &Visitor) override;
-  void removeSymbols(function_ref<bool(const Symbol &)> ToRemove) override;
+  Error removeSymbols(function_ref<bool(const Symbol &)> ToRemove) override;
 
   static bool classof(const SectionBase *S) {
     return S->Type == ELF::SHT_SYMTAB;
@@ -573,7 +573,7 @@ protected:
   RelocSectionWithSymtabBase() = default;
 
 public:
-  void removeSectionReferences(const SectionBase *Sec) override;
+  Error removeSectionReferences(const SectionBase *Sec) override;
   void initialize(SectionTableRef SecTable) override;
   void finalize() override;
 };
@@ -588,7 +588,7 @@ public:
   void addRelocation(Relocation Rel) { Relocations.push_back(Rel); }
   void accept(SectionVisitor &Visitor) const override;
   void accept(MutableSectionVisitor &Visitor) override;
-  void removeSymbols(function_ref<bool(const Symbol &)> ToRemove) override;
+  Error removeSymbols(function_ref<bool(const Symbol &)> ToRemove) override;
   void markSymbols() override;
 
   static bool classof(const SectionBase *S) {
@@ -623,7 +623,7 @@ public:
   void accept(SectionVisitor &) const override;
   void accept(MutableSectionVisitor &Visitor) override;
   void finalize() override;
-  void removeSymbols(function_ref<bool(const Symbol &)> ToRemove) override;
+  Error removeSymbols(function_ref<bool(const Symbol &)> ToRemove) override;
   void markSymbols() override;
 
   static bool classof(const SectionBase *S) {
@@ -803,8 +803,8 @@ public:
   Range<Segment> segments() { return make_pointee_range(Segments); }
   ConstRange<Segment> segments() const { return make_pointee_range(Segments); }
 
-  void removeSections(std::function<bool(const SectionBase &)> ToRemove);
-  void removeSymbols(function_ref<bool(const Symbol &)> ToRemove);
+  Error removeSections(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)...);
     auto Ptr = Sec.get();