Revert [llvm-objcopy][NFC] Refactor output target parsing

This reverts r364254 (git commit 545f001d1b)

This change causes some llvm-obcopy tests to fail with valgrind.

Following is the output for basic-keep.test
Command Output (stderr):
--

==107406== Conditional jump or move depends on uninitialised value(s)
==107406==    at 0x1A30DD: executeObjcopy(llvm::objcopy::CopyConfig const&) (llvm-objcopy.cpp:235)
==107406==    by 0x1A3935: main (llvm-objcopy.cpp:294)

llvm-svn: 364379

llvm/tools/llvm-objcopy/CopyConfig.cpp

@@ -277,13 +277,8 @@ static Expected<const MachineInfo &> getMachineInfo(StringRef Arch) {
   return Iter->getValue();
 }
 
-struct TargetInfo {
-  FileFormat Format;
-  MachineInfo Machine;
-};
-
 // FIXME: consolidate with the bfd parsing used by lld.
-static const StringMap<MachineInfo> TargetMap{
+static const StringMap<MachineInfo> OutputFormatMap{
     // Name, {EMachine, 64bit, LittleEndian}
     // x86
     {"elf32-i386", {ELF::EM_386, false, true}},
@@ -317,28 +312,18 @@ static const StringMap<MachineInfo> TargetMap{
     {"elf32-sparcel", {ELF::EM_SPARC, false, true}},
 };
 
-static Expected<TargetInfo>
+static Expected<MachineInfo> getOutputFormatMachineInfo(StringRef Format) {
-getOutputTargetInfoByTargetName(StringRef TargetName) {
+  StringRef OriginalFormat = Format;
-  StringRef OriginalTargetName = TargetName;
+  bool IsFreeBSD = Format.consume_back("-freebsd");
-  bool IsFreeBSD = TargetName.consume_back("-freebsd");
+  auto Iter = OutputFormatMap.find(Format);
-  auto Iter = TargetMap.find(TargetName);
+  if (Iter == std::end(OutputFormatMap))
-  if (Iter == std::end(TargetMap))
     return createStringError(errc::invalid_argument,
                              "invalid output format: '%s'",
-                             OriginalTargetName.str().c_str());
+                             OriginalFormat.str().c_str());
   MachineInfo MI = Iter->getValue();
   if (IsFreeBSD)
     MI.OSABI = ELF::ELFOSABI_FREEBSD;
-
+  return {MI};
-  FileFormat Format;
-  if (TargetName.startswith("elf"))
-    Format = FileFormat::ELF;
-  else
-    // This should never happen because `TargetName` is valid (it certainly
-    // exists in the TargetMap).
-    llvm_unreachable("unknown target prefix");
-
-  return {TargetInfo{Format, MI}};
 }
 
 static Error addSymbolsFromFile(std::vector<NameOrRegex> &Symbols,
@@ -460,23 +445,14 @@ Expected<DriverConfig> parseObjcopyOptions(ArrayRef<const char *> ArgsArr) {
         "--target cannot be used with --input-target or --output-target");
 
   bool UseRegex = InputArgs.hasArg(OBJCOPY_regex);
-  StringRef InputFormat, OutputFormat;
   if (InputArgs.hasArg(OBJCOPY_target)) {
-    InputFormat = InputArgs.getLastArgValue(OBJCOPY_target);
+    Config.InputFormat = InputArgs.getLastArgValue(OBJCOPY_target);
-    OutputFormat = InputArgs.getLastArgValue(OBJCOPY_target);
+    Config.OutputFormat = InputArgs.getLastArgValue(OBJCOPY_target);
   } else {
-    InputFormat = InputArgs.getLastArgValue(OBJCOPY_input_target);
+    Config.InputFormat = InputArgs.getLastArgValue(OBJCOPY_input_target);
-    OutputFormat = InputArgs.getLastArgValue(OBJCOPY_output_target);
+    Config.OutputFormat = InputArgs.getLastArgValue(OBJCOPY_output_target);
   }
-
+  if (Config.InputFormat == "binary") {
-  // FIXME:  Currently, we ignore the target for non-binary/ihex formats
-  // explicitly specified by -I option (e.g. -Ielf32-x86-64) and guess the
-  // format by llvm::object::createBinary regardless of the option value.
-  Config.InputFormat = StringSwitch<FileFormat>(InputFormat)
-                           .Case("binary", FileFormat::Binary)
-                           .Case("ihex", FileFormat::IHex)
-                           .Default(FileFormat::Unspecified);
-  if (Config.InputFormat == FileFormat::Binary) {
     auto BinaryArch = InputArgs.getLastArgValue(OBJCOPY_binary_architecture);
     if (BinaryArch.empty())
       return createStringError(
@@ -487,17 +463,12 @@ Expected<DriverConfig> parseObjcopyOptions(ArrayRef<const char *> ArgsArr) {
       return MI.takeError();
     Config.BinaryArch = *MI;
   }
-
+  if (!Config.OutputFormat.empty() && Config.OutputFormat != "binary" &&
-  Config.OutputFormat = StringSwitch<FileFormat>(OutputFormat)
+      Config.OutputFormat != "ihex") {
-                            .Case("binary", FileFormat::Binary)
+    Expected<MachineInfo> MI = getOutputFormatMachineInfo(Config.OutputFormat);
-                            .Case("ihex", FileFormat::IHex)
+    if (!MI)
-                            .Default(FileFormat::Unspecified);
+      return MI.takeError();
-  if (Config.OutputFormat == FileFormat::Unspecified && !OutputFormat.empty()) {
+    Config.OutputArch = *MI;
-    Expected<TargetInfo> Target = getOutputTargetInfoByTargetName(OutputFormat);
-    if (!Target)
-      return Target.takeError();
-    Config.OutputFormat = Target->Format;
-    Config.OutputArch = Target->Machine;
   }
 
   if (auto Arg = InputArgs.getLastArg(OBJCOPY_compress_debug_sections,

llvm/tools/llvm-objcopy/CopyConfig.h

@@ -26,13 +26,6 @@
 namespace llvm {
 namespace objcopy {
 
-enum class FileFormat {
-  Unspecified,
-  ELF,
-  Binary,
-  IHex,
-};
-
 // This type keeps track of the machine info for various architectures. This
 // lets us map architecture names to ELF types and the e_machine value of the
 // ELF file.
@@ -111,9 +104,9 @@ struct NewSymbolInfo {
 struct CopyConfig {
   // Main input/output options
   StringRef InputFilename;
-  FileFormat InputFormat;
+  StringRef InputFormat;
   StringRef OutputFilename;
-  FileFormat OutputFormat;
+  StringRef OutputFormat;
 
   // Only applicable for --input-format=binary
   MachineInfo BinaryArch;

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

@@ -154,14 +154,12 @@ static std::unique_ptr<Writer> createELFWriter(const CopyConfig &Config,
 static std::unique_ptr<Writer> createWriter(const CopyConfig &Config,
                                             Object &Obj, Buffer &Buf,
                                             ElfType OutputElfType) {
-  switch (Config.OutputFormat) {
+  using Functor = std::function<std::unique_ptr<Writer>()>;
-  case FileFormat::Binary:
+  return StringSwitch<Functor>(Config.OutputFormat)
-    return llvm::make_unique<BinaryWriter>(Obj, Buf);
+      .Case("binary", [&] { return llvm::make_unique<BinaryWriter>(Obj, Buf); })
-  case FileFormat::IHex:
+      .Case("ihex", [&] { return llvm::make_unique<IHexWriter>(Obj, Buf); })
-    return llvm::make_unique<IHexWriter>(Obj, Buf);
+      .Default(
-  default:
+          [&] { return createELFWriter(Config, Obj, Buf, OutputElfType); })();
-    return createELFWriter(Config, Obj, Buf, OutputElfType);
-  }
 }
 
 template <class ELFT>

llvm/tools/llvm-objcopy/llvm-objcopy.cpp

@@ -140,16 +140,11 @@ static Error executeObjcopyOnIHex(const CopyConfig &Config, MemoryBuffer &In,
 /// of the output specified by the command line options.
 static Error executeObjcopyOnRawBinary(const CopyConfig &Config,
                                        MemoryBuffer &In, Buffer &Out) {
-  switch (Config.OutputFormat) {
+  // TODO: llvm-objcopy should parse CopyConfig.OutputFormat to recognize
-  case FileFormat::ELF:
+  // formats other than ELF / "binary" and invoke
-  // FIXME: Currently, we call elf::executeObjcopyOnRawBinary even if the
+  // elf::executeObjcopyOnRawBinary, macho::executeObjcopyOnRawBinary or
-  // output format is binary/ihex or it's not given. This behavior differs from
+  // coff::executeObjcopyOnRawBinary accordingly.
-  // GNU objcopy. See https://bugs.llvm.org/show_bug.cgi?id=42171 for details.
+  return elf::executeObjcopyOnRawBinary(Config, In, Out);
-  case FileFormat::Binary:
-  case FileFormat::IHex:
-  case FileFormat::Unspecified:
-    return elf::executeObjcopyOnRawBinary(Config, In, Out);
-  }
 }
 
 /// The function executeObjcopyOnBinary does the dispatch based on the format
@@ -229,17 +224,10 @@ static Error executeObjcopy(const CopyConfig &Config) {
       return createFileError(Config.InputFilename, EC);
 
   typedef Error (*ProcessRawFn)(const CopyConfig &, MemoryBuffer &, Buffer &);
-  ProcessRawFn ProcessRaw;
+  auto ProcessRaw = StringSwitch<ProcessRawFn>(Config.InputFormat)
-  switch (Config.InputFormat) {
+                        .Case("binary", executeObjcopyOnRawBinary)
-  case FileFormat::Binary:
+                        .Case("ihex", executeObjcopyOnIHex)
-    ProcessRaw = executeObjcopyOnRawBinary;
+                        .Default(nullptr);
-    break;
-  case FileFormat::IHex:
-    ProcessRaw = executeObjcopyOnIHex;
-    break;
-  default:
-    ProcessRaw = nullptr;
-  }
 
   if (ProcessRaw) {
     auto BufOrErr = MemoryBuffer::getFileOrSTDIN(Config.InputFilename);