[llvm-objcopy] Unify format specific options check.

[avl]

Summary

This patch is extracted from D99055 to do functional change separately
from pure refactoring. While handling llvm-objcopy command line options
there is a check for options non-supported on the concrete format.
This check is done differently for various formats. Some formats check
for unsupported options before handling normal options. Other formats
check for unsupported options after handling normal options.
This patch makes processing similar for all formats. It firstly checks
for unsupported options and then handle normal options.

[dblaikie]

Seems OK /if/ we go in the direction of D99055 (so please hold off on committing this until that one's approved/settled). Specifically: If we don't go in that direction, it seems good that each of these checks are in the format-specific parts of the code, rather than all together in the main part of the code.

llvm/tools/llvm-objcopy/COFF/COFFObjcopy.cpp

addSection(
                      Buf->getBufferSize()),
         IMAGE_SCN_CNT_INITIALIZED_DATA | IMAGE_SCN_ALIGN_1BYTES);
   }
 
   if (!Config.AddGnuDebugLink.empty())
     if (Error E = addGnuDebugLink(Obj, Config.AddGnuDebugLink))
       return E;
 
-  if (Config.AllowBrokenLinks || !Config.SplitDWO.empty() ||
-      !Config.SymbolsPrefix.empty() || !Config.AllocSectionsPrefix.empty() ||
-      !Config.DumpSection.empty() || !Config.KeepSection.empty() ||
-      Config.NewSymbolVisibility || !Config.SymbolsToGlobalize.empty() ||
-      !Config.SymbolsToKeep.empty() || !Config.SymbolsToLocalize.empty() ||
-      !Config.SymbolsToWeaken.empty() || !Config.SymbolsToKeepGlobal.empty() ||
-      !Config.SectionsToRename.empty() || !Config.SetSectionAlignment.empty() ||
-      Config.ExtractDWO || Config.LocalizeHidden || Config.PreserveDates ||
-      Config.StripDWO || Config.StripNonAlloc || Config.StripSections ||
-      Config.StripSwiftSymbols || Config.KeepUndefined || Config.Weaken ||
-      Config.DecompressDebugSections ||
-      Config.DiscardMode == DiscardType::Locals ||
-      !Config.SymbolsToAdd.empty() || Config.EntryExpr) {
-    return createStringError(llvm::errc::invalid_argument,
-                             "option not supported by llvm-objcopy for COFF");
-  }
-
   return Error::success();
 }
 
 Error executeObjcopyOnBinary(const CopyConfig &Config, COFFObjectFile &In,
                              raw_ostream &Out) {
   COFFReader Reader(In);
   Expected<std::unique_ptr<Object>> ObjOrErr = Reader.create();
   if (!ObjOrErr)

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

 // This function handles the high level operations of GNU objcopy including
 // handling command line options. It's important to outline certain properties
 // we expect to hold of the command line operations. Any operation that "keeps"
 // should keep regardless of a remove. Additionally any removal should respect
 // 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 Error handleArgs(const CopyConfig &Config, Object &Obj) {
-  if (Config.StripSwiftSymbols || Config.KeepUndefined)
-    return createStringError(llvm::errc::invalid_argument,
-                             "option not supported by llvm-objcopy for ELF");
-
   if (Config.OutputArch) {
     Obj.Machine = Config.OutputArch.getValue().EMachine;
     Obj.OSABI = Config.OutputArch.getValue().OSABI;
   }
 
   if (!Config.SplitDWO.empty() && Config.ExtractDWO) {
     return Obj.removeSections(
         Config.AllowBrokenLinks,

llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp

static Error isValidMachOCannonicalName(StringRef Name) {
   if (Pair.second.size() > 16)
     return createStringError(errc::invalid_argument,
                              "too long section name: '%s'",
                              Pair.second.str().c_str());
   return Error::success();
 }
 
 static Error handleArgs(const CopyConfig &Config, Object &Obj) {
-  if (Config.AllowBrokenLinks || !Config.SplitDWO.empty() ||
-      !Config.SymbolsPrefix.empty() || !Config.AllocSectionsPrefix.empty() ||
-      !Config.KeepSection.empty() || Config.NewSymbolVisibility ||
-      !Config.SymbolsToGlobalize.empty() || !Config.SymbolsToKeep.empty() ||
-      !Config.SymbolsToLocalize.empty() || !Config.SymbolsToWeaken.empty() ||
-      !Config.SymbolsToKeepGlobal.empty() || !Config.SectionsToRename.empty() ||
-      !Config.UnneededSymbolsToRemove.empty() ||
-      !Config.SetSectionAlignment.empty() || !Config.SetSectionFlags.empty() ||
-      Config.ExtractDWO || Config.LocalizeHidden || Config.PreserveDates ||
-      Config.StripAllGNU || Config.StripDWO || Config.StripNonAlloc ||
-      Config.StripSections || Config.Weaken || Config.DecompressDebugSections ||
-      Config.StripUnneeded || Config.DiscardMode == DiscardType::Locals ||
-      !Config.SymbolsToAdd.empty() || Config.EntryExpr) {
-    return createStringError(llvm::errc::invalid_argument,
-                             "option not supported by llvm-objcopy for MachO");
-  }
-
   // Dump sections before add/remove for compatibility with GNU objcopy.
   for (StringRef Flag : Config.DumpSection) {
     StringRef SectionName;
     StringRef FileName;
     std::tie(SectionName, FileName) = Flag.split('=');
     if (Error E = dumpSectionToFile(SectionName, FileName, Obj))
       return E;
   }

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

for (const NewArchiveMember &Member : NewMembers) {
     std::copy(Member.Buf->getBufferStart(), Member.Buf->getBufferEnd(),
               (*FB)->getBufferStart());
     if (Error E = (*FB)->commit())
       return E;
   }
   return Error::success();
 }
 
+static Error checkForNonELFOptions(CopyConfig &Config) {
+  if (Config.StripSwiftSymbols || Config.KeepUndefined)
+    return createStringError(llvm::errc::invalid_argument,
+                             "option not supported by llvm-objcopy for ELF");
+  return Error::success();
+}
+
+static Error checkForNonCOFFOptions(CopyConfig &Config) {
+  if (Config.AllowBrokenLinks || !Config.SplitDWO.empty() ||
+      !Config.SymbolsPrefix.empty() || !Config.AllocSectionsPrefix.empty() ||
+      !Config.DumpSection.empty() || !Config.KeepSection.empty() ||
+      Config.NewSymbolVisibility || !Config.SymbolsToGlobalize.empty() ||
+      !Config.SymbolsToKeep.empty() || !Config.SymbolsToLocalize.empty() ||
+      !Config.SymbolsToWeaken.empty() || !Config.SymbolsToKeepGlobal.empty() ||
+      !Config.SectionsToRename.empty() || !Config.SetSectionAlignment.empty() ||
+      Config.ExtractDWO || Config.LocalizeHidden || Config.PreserveDates ||
+      Config.StripDWO || Config.StripNonAlloc || Config.StripSections ||
+      Config.StripSwiftSymbols || Config.KeepUndefined || Config.Weaken ||
+      Config.DecompressDebugSections ||
+      Config.DiscardMode == DiscardType::Locals ||
+      !Config.SymbolsToAdd.empty() || Config.EntryExpr)
+    return createStringError(llvm::errc::invalid_argument,
+                             "option not supported by llvm-objcopy for COFF");
+  return Error::success();
+}
+
+static Error checkForNonMachOOptions(CopyConfig &Config) {
+  if (Config.AllowBrokenLinks || !Config.SplitDWO.empty() ||
+      !Config.SymbolsPrefix.empty() || !Config.AllocSectionsPrefix.empty() ||
+      !Config.KeepSection.empty() || Config.NewSymbolVisibility ||
+      !Config.SymbolsToGlobalize.empty() || !Config.SymbolsToKeep.empty() ||
+      !Config.SymbolsToLocalize.empty() || !Config.SymbolsToWeaken.empty() ||
+      !Config.SymbolsToKeepGlobal.empty() || !Config.SectionsToRename.empty() ||
+      !Config.UnneededSymbolsToRemove.empty() ||
+      !Config.SetSectionAlignment.empty() || !Config.SetSectionFlags.empty() ||
+      Config.ExtractDWO || Config.LocalizeHidden || Config.PreserveDates ||
+      Config.StripAllGNU || Config.StripDWO || Config.StripNonAlloc ||
+      Config.StripSections || Config.Weaken || Config.DecompressDebugSections ||
+      Config.StripUnneeded || Config.DiscardMode == DiscardType::Locals ||
+      !Config.SymbolsToAdd.empty() || Config.EntryExpr)
+    return createStringError(llvm::errc::invalid_argument,
+                             "option not supported by llvm-objcopy for MachO");
+  return Error::success();
+}
+
+static Error checkForNonWasmOptions(CopyConfig &Config) {
+  if (!Config.AddGnuDebugLink.empty() || Config.ExtractPartition ||
+      !Config.SplitDWO.empty() || !Config.SymbolsPrefix.empty() ||
+      !Config.AllocSectionsPrefix.empty() ||
+      Config.DiscardMode != DiscardType::None || Config.NewSymbolVisibility ||
+      !Config.SymbolsToAdd.empty() || !Config.RPathToAdd.empty() ||
+      !Config.OnlySection.empty() || !Config.SymbolsToGlobalize.empty() ||
+      !Config.SymbolsToKeep.empty() || !Config.SymbolsToLocalize.empty() ||
+      !Config.SymbolsToRemove.empty() ||
+      !Config.UnneededSymbolsToRemove.empty() ||
+      !Config.SymbolsToWeaken.empty() || !Config.SymbolsToKeepGlobal.empty() ||
+      !Config.SectionsToRename.empty() || !Config.SetSectionAlignment.empty() ||
+      !Config.SetSectionFlags.empty() || !Config.SymbolsToRename.empty())
+    return createStringError(
+        llvm::errc::invalid_argument,
+        "only add-section, dump-section, and remove-section are supported");
+  return Error::success();
+}
+
 /// The function executeObjcopyOnIHex does the dispatch based on the format
 /// of the output specified by the command line options.
 static Error executeObjcopyOnIHex(CopyConfig &Config, MemoryBuffer &In,
                                   raw_ostream &Out) {
   // TODO: support output formats other than ELF.
   if (Error E = Config.parseELFConfig())
     return E;
+  if (Error E = checkForNonELFOptions(Config))
+    return E;
   return elf::executeObjcopyOnIHex(Config, In, Out);
 }
 
 /// The function executeObjcopyOnRawBinary does the dispatch based on the format
 /// of the output specified by the command line options.
 static Error executeObjcopyOnRawBinary(CopyConfig &Config, MemoryBuffer &In,
                                        raw_ostream &Out) {
   switch (Config.OutputFormat) {
   case FileFormat::ELF:
   // FIXME: Currently, we call elf::executeObjcopyOnRawBinary even if the
   // output format is binary/ihex or it's not given. This behavior differs from
   // GNU objcopy. See https://bugs.llvm.org/show_bug.cgi?id=42171 for details.
   case FileFormat::Binary:
   case FileFormat::IHex:
   case FileFormat::Unspecified:
     if (Error E = Config.parseELFConfig())
       return E;
+    if (Error E = checkForNonELFOptions(Config))
+      return E;
     return elf::executeObjcopyOnRawBinary(Config, In, Out);
   }
 
   llvm_unreachable("unsupported output format");
 }
 
 /// The function executeObjcopyOnBinary does the dispatch based on the format
 /// of the input binary (ELF, MachO or COFF).
 static Error executeObjcopyOnBinary(CopyConfig &Config, object::Binary &In,
                                     raw_ostream &Out) {
   if (auto *ELFBinary = dyn_cast<object::ELFObjectFileBase>(&In)) {
     if (Error E = Config.parseELFConfig())
       return E;
+    if (Error E = checkForNonELFOptions(Config))
+      return E;
     return elf::executeObjcopyOnBinary(Config, *ELFBinary, Out);
-  } else if (auto *COFFBinary = dyn_cast<object::COFFObjectFile>(&In))
+  } else if (auto *COFFBinary = dyn_cast<object::COFFObjectFile>(&In)) {
+    if (Error E = checkForNonCOFFOptions(Config))
+      return E;
     return coff::executeObjcopyOnBinary(Config, *COFFBinary, Out);
-  else if (auto *MachOBinary = dyn_cast<object::MachOObjectFile>(&In))
+  } else if (auto *MachOBinary = dyn_cast<object::MachOObjectFile>(&In)) {
+    if (Error E = checkForNonMachOOptions(Config))
+      return E;
     return macho::executeObjcopyOnBinary(Config, *MachOBinary, Out);
-  else if (auto *MachOUniversalBinary =
-               dyn_cast<object::MachOUniversalBinary>(&In))
+  } else if (auto *MachOUniversalBinary =
+                 dyn_cast<object::MachOUniversalBinary>(&In)) {
     return macho::executeObjcopyOnMachOUniversalBinary(
         Config, *MachOUniversalBinary, Out);
-  else if (auto *WasmBinary = dyn_cast<object::WasmObjectFile>(&In))
+  } else if (auto *WasmBinary = dyn_cast<object::WasmObjectFile>(&In)) {
+    if (Error E = checkForNonWasmOptions(Config))
+      return E;
     return objcopy::wasm::executeObjcopyOnBinary(Config, *WasmBinary, Out);
-  else
+  } else
     return createStringError(object_error::invalid_file_type,
                              "unsupported object file format");
 }
 
 namespace llvm {
 namespace objcopy {
 
 Expected<std::vector<NewArchiveMember>>

llvm/tools/llvm-objcopy/wasm/WasmObjcopy.cpp

for (StringRef Flag : Config.AddSection) {
     Sec.Name = SecName;
     std::unique_ptr<MemoryBuffer> Buf = std::move(*BufOrErr);
     Sec.Contents = makeArrayRef<uint8_t>(
         reinterpret_cast<const uint8_t *>(Buf->getBufferStart()),
         Buf->getBufferSize());
     Obj.addSectionWithOwnedContents(Sec, std::move(Buf));
   }
 
-  if (!Config.AddGnuDebugLink.empty() || Config.ExtractPartition ||
-      !Config.SplitDWO.empty() || !Config.SymbolsPrefix.empty() ||
-      !Config.AllocSectionsPrefix.empty() ||
-      Config.DiscardMode != DiscardType::None || Config.NewSymbolVisibility ||
-      !Config.SymbolsToAdd.empty() || !Config.RPathToAdd.empty() ||
-      !Config.OnlySection.empty() || !Config.SymbolsToGlobalize.empty() ||
-      !Config.SymbolsToKeep.empty() || !Config.SymbolsToLocalize.empty() ||
-      !Config.SymbolsToRemove.empty() ||
-      !Config.UnneededSymbolsToRemove.empty() ||
-      !Config.SymbolsToWeaken.empty() || !Config.SymbolsToKeepGlobal.empty() ||
-      !Config.SectionsToRename.empty() || !Config.SetSectionAlignment.empty() ||
-      !Config.SetSectionFlags.empty() || !Config.SymbolsToRename.empty()) {
-    return createStringError(
-        llvm::errc::invalid_argument,
-        "only add-section, dump-section, and remove-section are supported");
-  }
   return Error::success();
 }
 
 Error executeObjcopyOnBinary(const CopyConfig &Config,
                              object::WasmObjectFile &In, raw_ostream &Out) {
   Reader TheReader(In);
   Expected<std::unique_ptr<Object>> ObjOrErr = TheReader.create();
   if (!ObjOrErr)