[llvm-objcopy][NFC] More error cleanup

Summary:
This removes calls to `error()`/`reportError()` in the main driver (llvm-objcopy.cpp) as well as the associated argv-parsing (CopyConfig.cpp). `logAllUnhandledErrors()` is now the main way to print errors.

There are still a few uses from within the per-arch drivers, so we can't delete them yet... but almost!

Reviewers: jhenderson, alexshap, espindola

Reviewed By: jhenderson

Subscribers: emaste, arichardson, jakehehrlich, jdoerfert, llvm-commits

Tags: #llvm

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

llvm-svn: 354600

Author: rupprecht

llvm/test/tools/llvm-objcopy/ELF/bad-output-format.test

@@ -10,4 +10,4 @@ FileHeader:
   Type:            ET_EXEC
   Machine:         EM_386
 
-# BAD-OUTPUT-FORMAT: Invalid output format: 'xyz'.
+# BAD-OUTPUT-FORMAT: Invalid output format: 'xyz'

llvm/test/tools/llvm-objcopy/ELF/binary-input-error.test

@@ -6,5 +6,5 @@
 # RUN: not llvm-objcopy -I binary -B xyz %t.txt %t.o 2>&1 \
 # RUN:   | FileCheck %s --check-prefix=BAD-BINARY-ARCH
 
-# MISSING-BINARY-ARCH: Specified binary input without specifiying an architecture.
-# BAD-BINARY-ARCH: Invalid architecture: 'xyz'.
+# MISSING-BINARY-ARCH: Specified binary input without specifiying an architecture
+# BAD-BINARY-ARCH: Invalid architecture: 'xyz'

llvm/test/tools/llvm-objcopy/ELF/compress-and-decompress-debug-sections-error.test

@@ -3,5 +3,5 @@
 # RUN: yaml2obj %p/Inputs/compress-debug-sections.yaml -o %t.o
 # RUN: not llvm-objcopy --compress-debug-sections=zlib --decompress-debug-sections %t.o 2>&1 | FileCheck %s
 
-# CHECK: Cannot specify --compress-debug-sections at the same time as --decompress-debug-sections at the same time.
+# CHECK: Cannot specify --compress-debug-sections at the same time as --decompress-debug-sections at the same time
 

llvm/test/tools/llvm-objcopy/ELF/compress-debug-sections-invalid-format.test

@@ -1,5 +1,5 @@
 # RUN: yaml2obj %p/Inputs/compress-debug-sections.yaml -o %t.o
 # RUN: not llvm-objcopy --compress-debug-sections=zlib-fake %t.o 2>&1 | FileCheck %s
 
-# CHECK: Invalid or unsupported --compress-debug-sections format: zlib-fake.
+# CHECK: Invalid or unsupported --compress-debug-sections format: zlib-fake
 

llvm/test/tools/llvm-objcopy/ELF/input-output-target.test

@@ -19,4 +19,4 @@
 # RUN:     %t.txt %t.4.txt 2>&1 \
 # RUN:     | FileCheck %s --check-prefix=BAD-FLAG
 
-# BAD-FLAG: --target cannot be used with --input-target or --output-target.
+# BAD-FLAG: --target cannot be used with --input-target or --output-target

llvm/test/tools/llvm-objcopy/ELF/rename-section-flag.test

@@ -57,4 +57,4 @@ Sections:
 # WRITE-NEXT:     SHF_WRITE (0x1)
 # CHECK-NEXT:   ]
 
-# BAD-FLAG: Unrecognized section flag 'xyzzy'. Flags supported for GNU compatibility: alloc, load, noload, readonly, debug, code, data, rom, share, contents, merge, strings.
+# BAD-FLAG: Unrecognized section flag 'xyzzy'. Flags supported for GNU compatibility: alloc, load, noload, readonly, debug, code, data, rom, share, contents, merge, strings

llvm/test/tools/llvm-objcopy/ELF/set-section-flags-and-rename.test

@@ -10,5 +10,5 @@ FileHeader:
   Type:            ET_REL
   Machine:         EM_X86_64
 
-# SET-FOO: --set-section-flags=.foo conflicts with --rename-section=.foo=.bar.
-# SET-BAR: --set-section-flags=.bar conflicts with --rename-section=.foo=.bar.
+# SET-FOO: --set-section-flags=.foo conflicts with --rename-section=.foo=.bar
+# SET-BAR: --set-section-flags=.bar conflicts with --rename-section=.foo=.bar

llvm/test/tools/llvm-objcopy/ELF/set-section-flags.test

@@ -65,4 +65,4 @@ Sections:
 # BAD-FORMAT: Bad format for --set-section-flags: missing '='
 # MULTIPLE-SETS: --set-section-flags set multiple times for section .foo
 
-# BAD-FLAG: Unrecognized section flag 'xyzzy'. Flags supported for GNU compatibility: alloc, load, noload, readonly, debug, code, data, rom, share, contents, merge, strings.
+# BAD-FLAG: Unrecognized section flag 'xyzzy'. Flags supported for GNU compatibility: alloc, load, noload, readonly, debug, code, data, rom, share, contents, merge, strings

llvm/test/tools/llvm-objcopy/ELF/strip-multiple-files.test

@@ -72,4 +72,4 @@ Symbols:
 # CHECK-NEXT:   }
 # CHECK-NEXT: ]
 
-# BAD-O-FLAG: Multiple input files cannot be used in combination with -o.
+# BAD-O-FLAG: Multiple input files cannot be used in combination with -o

llvm/tools/llvm-objcopy/CopyConfig.cpp

@@ -15,6 +15,7 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Allocator.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/Regex.h"
 // Necessary for llvm::DebugCompressionType::None
 #include "llvm/Target/TargetOptions.h"
@@ -131,12 +132,12 @@ struct DriverConfig {
 // ParseObjcopyOptions returns the config and sets the input arguments. If a
 // help flag is set then ParseObjcopyOptions will print the help messege and
 // exit.
-DriverConfig parseObjcopyOptions(ArrayRef<const char *> ArgsArr);
+Expected<DriverConfig> parseObjcopyOptions(ArrayRef<const char *> ArgsArr);
 
 // ParseStripOptions returns the config and sets the input arguments. If a
 // help flag is set then ParseStripOptions will print the help messege and
 // exit.
-DriverConfig parseStripOptions(ArrayRef<const char *> ArgsArr);
+Expected<DriverConfig> parseStripOptions(ArrayRef<const char *> ArgsArr);
 
 } // namespace objcopy
 } // namespace llvm

llvm/tools/llvm-objcopy/CopyConfig.h

@@ -183,70 +183,86 @@ static Error executeObjcopyOnArchive(const CopyConfig &Config,
                           Config.DeterministicArchives, Ar.isThin());
 }
 
-static void restoreDateOnFile(StringRef Filename,
-                              const sys::fs::file_status &Stat) {
+static Error restoreDateOnFile(StringRef Filename,
+                               const sys::fs::file_status &Stat) {
   int FD;
 
   if (auto EC =
           sys::fs::openFileForWrite(Filename, FD, sys::fs::CD_OpenExisting))
-    reportError(Filename, EC);
+    return createFileError(Filename, EC);
 
   if (auto EC = sys::fs::setLastAccessAndModificationTime(
           FD, Stat.getLastAccessedTime(), Stat.getLastModificationTime()))
-    reportError(Filename, EC);
+    return createFileError(Filename, EC);
 
   if (auto EC = sys::Process::SafelyCloseFileDescriptor(FD))
-    reportError(Filename, EC);
+    return createFileError(Filename, EC);
+
+  return Error::success();
 }
 
 /// The function executeObjcopy does the higher level dispatch based on the type
 /// of input (raw binary, archive or single object file) and takes care of the
 /// format-agnostic modifications, i.e. preserving dates.
-static void executeObjcopy(const CopyConfig &Config) {
+static Error executeObjcopy(const CopyConfig &Config) {
   sys::fs::file_status Stat;
   if (Config.PreserveDates)
     if (auto EC = sys::fs::status(Config.InputFilename, Stat))
-      reportError(Config.InputFilename, EC);
+      return createFileError(Config.InputFilename, EC);
 
   if (Config.InputFormat == "binary") {
     auto BufOrErr = MemoryBuffer::getFile(Config.InputFilename);
     if (!BufOrErr)
-      reportError(Config.InputFilename, BufOrErr.getError());
+      return createFileError(Config.InputFilename, BufOrErr.getError());
     FileBuffer FB(Config.OutputFilename);
     if (Error E = executeObjcopyOnRawBinary(Config, *BufOrErr->get(), FB))
-      error(std::move(E));
+      return E;
   } else {
     Expected<OwningBinary<llvm::object::Binary>> BinaryOrErr =
         createBinary(Config.InputFilename);
     if (!BinaryOrErr)
-      reportError(Config.InputFilename, BinaryOrErr.takeError());
+      return createFileError(Config.InputFilename, BinaryOrErr.takeError());
 
     if (Archive *Ar = dyn_cast<Archive>(BinaryOrErr.get().getBinary())) {
       if (Error E = executeObjcopyOnArchive(Config, *Ar))
-        error(std::move(E));
+        return E;
     } else {
       FileBuffer FB(Config.OutputFilename);
       if (Error E = executeObjcopyOnBinary(Config,
                                            *BinaryOrErr.get().getBinary(), FB))
-        error(std::move(E));
+        return E;
     }
   }
 
   if (Config.PreserveDates) {
-    restoreDateOnFile(Config.OutputFilename, Stat);
+    if (Error E = restoreDateOnFile(Config.OutputFilename, Stat))
+      return E;
     if (!Config.SplitDWO.empty())
-      restoreDateOnFile(Config.SplitDWO, Stat);
+      if (Error E = restoreDateOnFile(Config.SplitDWO, Stat))
+        return E;
   }
+
+  return Error::success();
 }
 
 int main(int argc, char **argv) {
   InitLLVM X(argc, argv);
   ToolName = argv[0];
-  DriverConfig DriverConfig;
-  if (sys::path::stem(ToolName).contains("strip"))
-    DriverConfig = parseStripOptions(makeArrayRef(argv + 1, argc));
-  else
-    DriverConfig = parseObjcopyOptions(makeArrayRef(argv + 1, argc));
-  for (const CopyConfig &CopyConfig : DriverConfig.CopyConfigs)
-    executeObjcopy(CopyConfig);
+  bool IsStrip = sys::path::stem(ToolName).contains("strip");
+  Expected<DriverConfig> DriverConfig =
+      IsStrip ? parseStripOptions(makeArrayRef(argv + 1, argc))
+              : parseObjcopyOptions(makeArrayRef(argv + 1, argc));
+  if (!DriverConfig) {
+    logAllUnhandledErrors(DriverConfig.takeError(),
+                          WithColor::error(errs(), ToolName));
+    return 1;
+  }
+  for (const CopyConfig &CopyConfig : DriverConfig->CopyConfigs) {
+    if (Error E = executeObjcopy(CopyConfig)) {
+      logAllUnhandledErrors(std::move(E), WithColor::error(errs(), ToolName));
+      return 1;
+    }
+  }
+
+  return 0;
 }