[llvm-objcopy] [COFF] Error out on use of unhandled options

[mstorsjo]

Summary

Prefer erroring out than silently not doing what was requested.

The current set of implemented options are the only ones used in my test corpus.

[rupprecht]

This doesn't seem like the best solution long-term, but probably makes testing llvm-objcopy on COFF much easier (expose missing functionality more loudly).

Maybe we could add something to the tablegen saying which object type each flag is supported for, and if that flag is set & we detect the input is not that object type, error?

[mstorsjo]

In D57045#1367024, @rupprecht wrote:

This doesn't seem like the best solution long-term, but probably makes testing llvm-objcopy on COFF much easier (expose missing functionality more loudly).

Maybe we could add something to the tablegen saying which object type each flag is supported for, and if that flag is set & we detect the input is not that object type, error?

That's probably a better long term solution, but also not quite trivial to do. The detection of which format is being handled happens long after the options are parsed:

static void 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");
}

At this point, the options have already been parsed out from the arguments and set in CopyConfig.

Also in a pathological case, with an archive file you could have different archive members of different formats...

tools/llvm-objcopy/COFF/COFFObjcopy.cpp

Obj.removeSymbols([&](const Symbol &Sym) {
     }
 
     return false;
   });
 
   if (!Config.AddGnuDebugLink.empty())
     addGnuDebugLink(Obj, Config.AddGnuDebugLink);
 
+  if (!Config.BuildIdLinkDir.empty() || Config.BuildIdLinkInput ||
+      Config.BuildIdLinkOutput || !Config.SplitDWO.empty() ||
+      !Config.SymbolsPrefix.empty() || !Config.AddSection.empty() ||
+      !Config.DumpSection.empty() || !Config.KeepSection.empty() ||
+      !Config.SymbolsToGlobalize.empty() || !Config.SymbolsToKeep.empty() ||
+      !Config.SymbolsToLocalize.empty() || !Config.SymbolsToWeaken.empty() ||
+      !Config.SymbolsToKeepGlobal.empty() || !Config.SectionsToRename.empty() ||
+      !Config.SymbolsToRename.empty() || Config.ExtractDWO ||
+      Config.KeepFileSymbols || Config.LocalizeHidden || Config.PreserveDates ||
+      Config.StripDWO || Config.StripNonAlloc || Config.StripSections ||
+      Config.Weaken || Config.DecompressDebugSections) {
+    return createStringError(llvm::errc::invalid_argument,
+                             "Option not supported by llvm-objcopy for COFF");
+  }
+
   return Error::success();
 }
 
 void executeObjcopyOnBinary(const CopyConfig &Config,
                             COFFObjectFile &In, Buffer &Out) {
   COFFReader Reader(In);
   Expected<std::unique_ptr<Object>> ObjOrErr = Reader.create();
   if (!ObjOrErr)