[llvm] r352625 - [llvm-objcopy][NFC] More error propagation

Jordan Rupprecht via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 30 06:36:54 PST 2019


Author: rupprecht
Date: Wed Jan 30 06:36:53 2019
New Revision: 352625

URL: http://llvm.org/viewvc/llvm-project?rev=352625&view=rev
Log:
[llvm-objcopy][NFC] More error propagation

Summary: Do some more error cleanup, removing some dependencies from llvm-objcopy's error/reportError in [ELF/COFF]Objcopy methods.

Reviewers: jhenderson, alexshap, jakehehrlich, mstorsjo, espindola

Subscribers: emaste, arichardson

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

Modified:
    llvm/trunk/test/tools/llvm-objcopy/ELF/bad-build-id.test
    llvm/trunk/tools/llvm-objcopy/COFF/COFFObjcopy.cpp
    llvm/trunk/tools/llvm-objcopy/COFF/COFFObjcopy.h
    llvm/trunk/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
    llvm/trunk/tools/llvm-objcopy/ELF/ELFObjcopy.h
    llvm/trunk/tools/llvm-objcopy/llvm-objcopy.cpp

Modified: llvm/trunk/test/tools/llvm-objcopy/ELF/bad-build-id.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objcopy/ELF/bad-build-id.test?rev=352625&r1=352624&r2=352625&view=diff
==============================================================================
--- llvm/trunk/test/tools/llvm-objcopy/ELF/bad-build-id.test (original)
+++ llvm/trunk/test/tools/llvm-objcopy/ELF/bad-build-id.test Wed Jan 30 06:36:53 2019
@@ -1,7 +1,7 @@
 # RUN: yaml2obj %s > %t
 # RUN: not llvm-objcopy --build-id-link-dir=%t-dir --build-id-link-input=.debug %t 2>&1 >/dev/null | FileCheck %s
 
-# CHECK: build ID in file {{.*}} is smaller than two bytes.
+# CHECK: build ID is smaller than two bytes.
 
 --- !ELF
 FileHeader:

Modified: llvm/trunk/tools/llvm-objcopy/COFF/COFFObjcopy.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objcopy/COFF/COFFObjcopy.cpp?rev=352625&r1=352624&r2=352625&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-objcopy/COFF/COFFObjcopy.cpp (original)
+++ llvm/trunk/tools/llvm-objcopy/COFF/COFFObjcopy.cpp Wed Jan 30 06:36:53 2019
@@ -188,19 +188,20 @@ static Error handleArgs(const CopyConfig
   return Error::success();
 }
 
-void executeObjcopyOnBinary(const CopyConfig &Config,
-                            COFFObjectFile &In, Buffer &Out) {
+Error executeObjcopyOnBinary(const CopyConfig &Config, COFFObjectFile &In,
+                             Buffer &Out) {
   COFFReader Reader(In);
   Expected<std::unique_ptr<Object>> ObjOrErr = Reader.create();
   if (!ObjOrErr)
-    reportError(Config.InputFilename, ObjOrErr.takeError());
+    return createFileError(Config.InputFilename, ObjOrErr.takeError());
   Object *Obj = ObjOrErr->get();
   assert(Obj && "Unable to deserialize COFF object");
   if (Error E = handleArgs(Config, *Obj))
-    reportError(Config.InputFilename, std::move(E));
+    return createFileError(Config.InputFilename, std::move(E));
   COFFWriter Writer(*Obj, Out);
   if (Error E = Writer.write())
-    reportError(Config.OutputFilename, std::move(E));
+    return createFileError(Config.OutputFilename, std::move(E));
+  return Error::success();
 }
 
 } // end namespace coff

Modified: llvm/trunk/tools/llvm-objcopy/COFF/COFFObjcopy.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objcopy/COFF/COFFObjcopy.h?rev=352625&r1=352624&r2=352625&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-objcopy/COFF/COFFObjcopy.h (original)
+++ llvm/trunk/tools/llvm-objcopy/COFF/COFFObjcopy.h Wed Jan 30 06:36:53 2019
@@ -10,6 +10,7 @@
 #define LLVM_TOOLS_OBJCOPY_COFFOBJCOPY_H
 
 namespace llvm {
+class Error;
 
 namespace object {
 class COFFObjectFile;
@@ -20,8 +21,8 @@ struct CopyConfig;
 class Buffer;
 
 namespace coff {
-void executeObjcopyOnBinary(const CopyConfig &Config,
-                            object::COFFObjectFile &In, Buffer &Out);
+Error executeObjcopyOnBinary(const CopyConfig &Config,
+                             object::COFFObjectFile &In, Buffer &Out);
 
 } // end namespace coff
 } // end namespace objcopy

Modified: llvm/trunk/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objcopy/ELF/ELFObjcopy.cpp?rev=352625&r1=352624&r2=352625&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-objcopy/ELF/ELFObjcopy.cpp (original)
+++ llvm/trunk/tools/llvm-objcopy/ELF/ELFObjcopy.cpp Wed Jan 30 06:36:53 2019
@@ -178,8 +178,8 @@ static void linkToBuildIdDir(const CopyC
   }
 }
 
-static void splitDWOToFile(const CopyConfig &Config, const Reader &Reader,
-                           StringRef File, ElfType OutputElfType) {
+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); });
@@ -188,9 +188,8 @@ static void splitDWOToFile(const CopyCon
   FileBuffer FB(File);
   auto Writer = createWriter(Config, *DWOFile, FB, OutputElfType);
   if (Error E = Writer->finalize())
-    error(std::move(E));
-  if (Error E = Writer->write())
-    error(std::move(E));
+    return E;
+  return Writer->write();
 }
 
 static Error dumpSectionToFile(StringRef SecName, StringRef Filename,
@@ -269,12 +268,14 @@ static void replaceDebugSections(
 // any previous removals. Lastly whether or not something is removed shouldn't
 // depend a) on the order the options occur in or b) on some opaque priority
 // system. The only priority is that keeps/copies overrule removes.
-static void handleArgs(const CopyConfig &Config, Object &Obj,
-                       const Reader &Reader, ElfType OutputElfType) {
+static Error handleArgs(const CopyConfig &Config, Object &Obj,
+                        const Reader &Reader, ElfType OutputElfType) {
+
+  if (!Config.SplitDWO.empty())
+    if (Error E =
+            splitDWOToFile(Config, Reader, Config.SplitDWO, OutputElfType))
+      return E;
 
-  if (!Config.SplitDWO.empty()) {
-    splitDWOToFile(Config, Reader, Config.SplitDWO, OutputElfType);
-  }
   if (Config.OutputArch)
     Obj.Machine = Config.OutputArch.getValue().EMachine;
 
@@ -520,7 +521,7 @@ static void handleArgs(const CopyConfig
       ErrorOr<std::unique_ptr<MemoryBuffer>> BufOrErr =
           MemoryBuffer::getFile(File);
       if (!BufOrErr)
-        reportError(File, BufOrErr.getError());
+        return createFileError(File, errorCodeToError(BufOrErr.getError()));
       std::unique_ptr<MemoryBuffer> Buf = std::move(*BufOrErr);
       ArrayRef<uint8_t> Data(
           reinterpret_cast<const uint8_t *>(Buf->getBufferStart()),
@@ -538,16 +539,18 @@ static void handleArgs(const CopyConfig
       StringRef SecName = SecPair.first;
       StringRef File = SecPair.second;
       if (Error E = dumpSectionToFile(SecName, File, Obj))
-        reportError(Config.InputFilename, std::move(E));
+        return createFileError(Config.InputFilename, std::move(E));
     }
   }
 
   if (!Config.AddGnuDebugLink.empty())
     Obj.addSection<GnuDebugLinkSection>(Config.AddGnuDebugLink);
+
+  return Error::success();
 }
 
-void executeObjcopyOnRawBinary(const CopyConfig &Config, MemoryBuffer &In,
-                               Buffer &Out) {
+Error executeObjcopyOnRawBinary(const CopyConfig &Config, MemoryBuffer &In,
+                                Buffer &Out) {
   BinaryReader Reader(Config.BinaryArch, &In);
   std::unique_ptr<Object> Obj = Reader.create();
 
@@ -555,17 +558,17 @@ void executeObjcopyOnRawBinary(const Cop
   // (-B<arch>).
   const ElfType OutputElfType = getOutputElfType(
       Config.OutputArch ? Config.OutputArch.getValue() : Config.BinaryArch);
-  handleArgs(Config, *Obj, Reader, OutputElfType);
+  if (Error E = handleArgs(Config, *Obj, Reader, OutputElfType))
+    return E;
   std::unique_ptr<Writer> Writer =
       createWriter(Config, *Obj, Out, OutputElfType);
   if (Error E = Writer->finalize())
-    error(std::move(E));
-  if (Error E = Writer->write())
-    error(std::move(E));
+    return E;
+  return Writer->write();
 }
 
-void executeObjcopyOnBinary(const CopyConfig &Config,
-                            object::ELFObjectFileBase &In, Buffer &Out) {
+Error executeObjcopyOnBinary(const CopyConfig &Config,
+                             object::ELFObjectFileBase &In, Buffer &Out) {
   ELFReader Reader(&In);
   std::unique_ptr<Object> Obj = Reader.create();
   // Prefer OutputArch (-O<format>) if set, otherwise infer it from the input.
@@ -577,25 +580,29 @@ void executeObjcopyOnBinary(const CopyCo
   if (!Config.BuildIdLinkDir.empty()) {
     BuildIdBytes = unwrapOrError(findBuildID(In));
     if (BuildIdBytes.size() < 2)
-      error("build ID in file '" + Config.InputFilename +
-            "' is smaller than two bytes");
+      return createFileError(
+          Config.InputFilename,
+          createStringError(object_error::parse_failed,
+                            "build ID is smaller than two bytes."));
   }
 
   if (!Config.BuildIdLinkDir.empty() && Config.BuildIdLinkInput) {
     linkToBuildIdDir(Config, Config.InputFilename,
                      Config.BuildIdLinkInput.getValue(), BuildIdBytes);
   }
-  handleArgs(Config, *Obj, Reader, OutputElfType);
+  if (Error E = handleArgs(Config, *Obj, Reader, OutputElfType))
+    return E;
   std::unique_ptr<Writer> Writer =
       createWriter(Config, *Obj, Out, OutputElfType);
   if (Error E = Writer->finalize())
-    error(std::move(E));
+    return E;
   if (Error E = Writer->write())
-    error(std::move(E));
+    return E;
   if (!Config.BuildIdLinkDir.empty() && Config.BuildIdLinkOutput) {
     linkToBuildIdDir(Config, Config.OutputFilename,
                      Config.BuildIdLinkOutput.getValue(), BuildIdBytes);
   }
+  return Error::success();
 }
 
 } // end namespace elf

Modified: llvm/trunk/tools/llvm-objcopy/ELF/ELFObjcopy.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objcopy/ELF/ELFObjcopy.h?rev=352625&r1=352624&r2=352625&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-objcopy/ELF/ELFObjcopy.h (original)
+++ llvm/trunk/tools/llvm-objcopy/ELF/ELFObjcopy.h Wed Jan 30 06:36:53 2019
@@ -10,6 +10,7 @@
 #define LLVM_TOOLS_OBJCOPY_ELFOBJCOPY_H
 
 namespace llvm {
+class Error;
 class MemoryBuffer;
 
 namespace object {
@@ -21,10 +22,10 @@ struct CopyConfig;
 class Buffer;
 
 namespace elf {
-void executeObjcopyOnRawBinary(const CopyConfig &Config, MemoryBuffer &In,
-                               Buffer &Out);
-void executeObjcopyOnBinary(const CopyConfig &Config,
-                            object::ELFObjectFileBase &In, Buffer &Out);
+Error executeObjcopyOnRawBinary(const CopyConfig &Config, MemoryBuffer &In,
+                                Buffer &Out);
+Error executeObjcopyOnBinary(const CopyConfig &Config,
+                             object::ELFObjectFileBase &In, Buffer &Out);
 
 } // end namespace elf
 } // end namespace objcopy

Modified: llvm/trunk/tools/llvm-objcopy/llvm-objcopy.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objcopy/llvm-objcopy.cpp?rev=352625&r1=352624&r2=352625&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-objcopy/llvm-objcopy.cpp (original)
+++ llvm/trunk/tools/llvm-objcopy/llvm-objcopy.cpp Wed Jan 30 06:36:53 2019
@@ -122,8 +122,8 @@ static Error deepWriteArchive(StringRef
 
 /// The function executeObjcopyOnRawBinary does the dispatch based on the format
 /// of the output specified by the command line options.
-static void executeObjcopyOnRawBinary(const CopyConfig &Config,
-                                      MemoryBuffer &In, Buffer &Out) {
+static Error executeObjcopyOnRawBinary(const CopyConfig &Config,
+                                       MemoryBuffer &In, Buffer &Out) {
   // TODO: llvm-objcopy should parse CopyConfig.OutputFormat to recognize
   // formats other than ELF / "binary" and invoke
   // elf::executeObjcopyOnRawBinary, macho::executeObjcopyOnRawBinary or
@@ -133,18 +133,19 @@ static void executeObjcopyOnRawBinary(co
 
 /// The function executeObjcopyOnBinary does the dispatch based on the format
 /// of the input binary (ELF, MachO or COFF).
-static void executeObjcopyOnBinary(const CopyConfig &Config, object::Binary &In,
-                                   Buffer &Out) {
+static Error executeObjcopyOnBinary(const CopyConfig &Config,
+                                    object::Binary &In, Buffer &Out) {
   if (auto *ELFBinary = dyn_cast<object::ELFObjectFileBase>(&In))
     return elf::executeObjcopyOnBinary(Config, *ELFBinary, Out);
   else if (auto *COFFBinary = dyn_cast<object::COFFObjectFile>(&In))
     return coff::executeObjcopyOnBinary(Config, *COFFBinary, Out);
   else
-    error("Unsupported object file format");
+    return createStringError(object_error::invalid_file_type,
+                             "Unsupported object file format");
 }
 
-static void executeObjcopyOnArchive(const CopyConfig &Config,
-                                    const Archive &Ar) {
+static Error executeObjcopyOnArchive(const CopyConfig &Config,
+                                     const Archive &Ar) {
   std::vector<NewArchiveMember> NewArchiveMembers;
   Error Err = Error::success();
   for (const Archive::Child &Child : Ar.children(Err)) {
@@ -158,7 +159,8 @@ static void executeObjcopyOnArchive(cons
       reportError(Ar.getFileName(), ChildNameOrErr.takeError());
 
     MemBuffer MB(ChildNameOrErr.get());
-    executeObjcopyOnBinary(Config, *Bin, MB);
+    if (Error E = executeObjcopyOnBinary(Config, *Bin, MB))
+      return E;
 
     Expected<NewArchiveMember> Member =
         NewArchiveMember::getOldMember(Child, Config.DeterministicArchives);
@@ -175,6 +177,7 @@ static void executeObjcopyOnArchive(cons
                                  Ar.hasSymbolTable(), Ar.kind(),
                                  Config.DeterministicArchives, Ar.isThin()))
     reportError(Config.OutputFilename, std::move(E));
+  return Error::success();
 }
 
 static void restoreDateOnFile(StringRef Filename,
@@ -207,7 +210,8 @@ static void executeObjcopy(const CopyCon
     if (!BufOrErr)
       reportError(Config.InputFilename, BufOrErr.getError());
     FileBuffer FB(Config.OutputFilename);
-    executeObjcopyOnRawBinary(Config, *BufOrErr->get(), FB);
+    if (Error E = executeObjcopyOnRawBinary(Config, *BufOrErr->get(), FB))
+      error(std::move(E));
   } else {
     Expected<OwningBinary<llvm::object::Binary>> BinaryOrErr =
         createBinary(Config.InputFilename);
@@ -215,10 +219,13 @@ static void executeObjcopy(const CopyCon
       reportError(Config.InputFilename, BinaryOrErr.takeError());
 
     if (Archive *Ar = dyn_cast<Archive>(BinaryOrErr.get().getBinary())) {
-      executeObjcopyOnArchive(Config, *Ar);
+      if (Error E = executeObjcopyOnArchive(Config, *Ar))
+        error(std::move(E));
     } else {
       FileBuffer FB(Config.OutputFilename);
-      executeObjcopyOnBinary(Config, *BinaryOrErr.get().getBinary(), FB);
+      if (Error E = executeObjcopyOnBinary(Config,
+                                           *BinaryOrErr.get().getBinary(), FB))
+        error(std::move(E));
     }
   }
 




More information about the llvm-commits mailing list