[llvm-objcopy] Ignore -B --binary-architecture=

[MaskRay]

Summary

GNU objcopy documents that -B is only useful with architecture-less
input (i.e. "binary" or "ihex"). After D67144, -O defaults to -I, and
-B is essentially a NOP.

• If -O is binary/ihex, GNU objcopy ignores -B.
• If -O is elf*, -B provides the e_machine field in GNU objcopy.

So to convert a blob to an ELF, -I binary -B i386:x86-64 -O elf64-x86-64 has to be specified.

-I binary -B i386:x86-64 -O elf64-x86-64 creates an ELF with its
e_machine field set to EM_NONE in GNU objcopy, but a regular x86_64 ELF
in elftoolchain elfcopy. Follow the elftoolchain approach (ignoring -B)
to simplify code. Users that expect their command line portable should
specify -B.

[jhenderson]

Please could you update the docs with this change. I completely agree with following the elf toolchain approach here.

[MaskRay]

Update docs/CommandGuide/llvm-objcopy.rst

[jhenderson]

LGTM, aside from one comment. Perhaps worth confirming with @jakehehrlich about it?

docs/CommandGuide/llvm-objcopy.rst

180–182 ↗

(On Diff #218879)

As this is now ignored, perhaps this can be moved to the generic options section, rather than ELF-specific?

[MaskRay]

Move -B from "ELF-specific" section to generic options section

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

 # RUN: echo abcd > %t.txt
 
-# RUN: not llvm-objcopy -I binary -B xyz %t.txt %t.o 2>&1 \
-# RUN:   | FileCheck %s --check-prefix=BAD-BINARY-ARCH
-
-# BAD-BINARY-ARCH: invalid architecture: 'xyz'
+## -B is ignored.
+# RUN: llvm-objcopy -I binary -B xyz %t.txt %t.o

tools/llvm-objcopy/CopyConfig.h

 // Configuration for copying/stripping a single file.
 struct CopyConfig {
   // Main input/output options
   StringRef InputFilename;
   FileFormat InputFormat;
   StringRef OutputFilename;
   FileFormat OutputFormat;
 
-  // Only applicable for --input-format=binary
-  MachineInfo BinaryArch;
   // Only applicable when --output-format!=binary (e.g. elf64-x86-64).
   Optional<MachineInfo> OutputArch;
 
   // Advanced options
   StringRef AddGnuDebugLink;
   // Cached gnu_debuglink's target CRC
   uint32_t GnuDebugLinkCRC32;
   StringRef BuildIdLinkDir;

tools/llvm-objcopy/CopyConfig.cpp

static Expected<NewSymbolInfo> parseNewSymbolInfo(StringRef FlagValue,
   if (!UnsupportedFlags.empty())
     return createStringError(errc::invalid_argument,
                              "unsupported flag%s for --add-symbol: '%s'",
                              UnsupportedFlags.size() > 1 ? "s" : "",
                              join(UnsupportedFlags, "', '").c_str());
   return SI;
 }
 
-static const StringMap<MachineInfo> ArchMap{
-    // Name, {EMachine, 64bit, LittleEndian}
-    {"aarch64", {ELF::EM_AARCH64, true, true}},
-    {"arm", {ELF::EM_ARM, false, true}},
-    {"i386", {ELF::EM_386, false, true}},
-    {"i386:x86-64", {ELF::EM_X86_64, true, true}},
-    {"mips", {ELF::EM_MIPS, false, false}},
-    {"powerpc:common64", {ELF::EM_PPC64, true, true}},
-    {"riscv:rv32", {ELF::EM_RISCV, false, true}},
-    {"riscv:rv64", {ELF::EM_RISCV, true, true}},
-    {"sparc", {ELF::EM_SPARC, false, false}},
-    {"sparcel", {ELF::EM_SPARC, false, true}},
-    {"x86-64", {ELF::EM_X86_64, true, true}},
-};
-
-static Expected<const MachineInfo &> getMachineInfo(StringRef Arch) {
-  auto Iter = ArchMap.find(Arch);
-  if (Iter == std::end(ArchMap))
-    return createStringError(errc::invalid_argument,
-                             "invalid architecture: '%s'", Arch.str().c_str());
-  return Iter->getValue();
-}
-
 struct TargetInfo {
   FileFormat Format;
   MachineInfo Machine;
 };
 
 // FIXME: consolidate with the bfd parsing used by lld.
 static const StringMap<MachineInfo> TargetMap{
     // Name, {EMachine, 64bit, LittleEndian}
Expected<DriverConfig> parseObjcopyOptions(ArrayRef<const char *> ArgsArr) {
 
   // 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()) {
-      Expected<const MachineInfo &> MI = getMachineInfo(BinaryArch);
-      if (!MI)
-        return MI.takeError();
-      Config.BinaryArch = *MI;
-    }
-  }
 
   if (opt::Arg *A = InputArgs.getLastArg(OBJCOPY_new_symbol_visibility)) {
     const uint8_t Invalid = 0xff;
     Config.NewSymbolVisibility = StringSwitch<uint8_t>(A->getValue())
                                      .Case("default", ELF::STV_DEFAULT)
                                      .Case("hidden", ELF::STV_HIDDEN)
                                      .Case("internal", ELF::STV_INTERNAL)
                                      .Case("protected", ELF::STV_PROTECTED)

tools/llvm-objcopy/ELF/ELFObjcopy.cpp

static Error writeOutput(const CopyConfig &Config, Object &Obj, Buffer &Out,
   return Writer->write();
 }
 
 Error executeObjcopyOnIHex(const CopyConfig &Config, MemoryBuffer &In,
                            Buffer &Out) {
   IHexReader Reader(&In);
   std::unique_ptr<Object> Obj = Reader.create();
   const ElfType OutputElfType =
-      getOutputElfType(Config.OutputArch.getValueOr(Config.BinaryArch));
+    getOutputElfType(Config.OutputArch.getValueOr(MachineInfo()));
   if (Error E = handleArgs(Config, *Obj, Reader, OutputElfType))
     return E;
   return writeOutput(Config, *Obj, Out, OutputElfType);
 }
 
 Error executeObjcopyOnRawBinary(const CopyConfig &Config, MemoryBuffer &In,
                                 Buffer &Out) {
   uint8_t NewSymbolVisibility =
       Config.NewSymbolVisibility.getValueOr((uint8_t)ELF::STV_DEFAULT);
-  BinaryReader Reader(Config.BinaryArch, &In, NewSymbolVisibility);
+  BinaryReader Reader(&In, NewSymbolVisibility);
   std::unique_ptr<Object> Obj = Reader.create();
 
   // Prefer OutputArch (-O<format>) if set, otherwise fallback to BinaryArch
   // (-B<arch>).
   const ElfType OutputElfType =
-      getOutputElfType(Config.OutputArch.getValueOr(Config.BinaryArch));
+      getOutputElfType(Config.OutputArch.getValueOr(MachineInfo()));
   if (Error E = handleArgs(Config, *Obj, Reader, OutputElfType))
     return E;
   return writeOutput(Config, *Obj, Out, OutputElfType);
 }
 
 Error executeObjcopyOnBinary(const CopyConfig &Config,
                              object::ELFObjectFileBase &In, Buffer &Out) {
   ELFReader Reader(&In, Config.ExtractPartition);

tools/llvm-objcopy/ELF/Object.h

 
 using object::Binary;
 using object::ELFFile;
 using object::ELFObjectFile;
 using object::OwningBinary;
 
 class BasicELFBuilder {
 protected:
-  uint16_t EMachine;
   std::unique_ptr<Object> Obj;
 
   void initFileHeader();
   void initHeaderSegment();
   StringTableSection *addStrTab();
   SymbolTableSection *addSymTab(StringTableSection *StrTab);
   void initSections();
 
 public:
-  BasicELFBuilder(uint16_t EM)
-      : EMachine(EM), Obj(std::make_unique<Object>()) {}
+  BasicELFBuilder() : Obj(std::make_unique<Object>()) {}
 };
 
 class BinaryELFBuilder : public BasicELFBuilder {
   MemoryBuffer *MemBuf;
   uint8_t NewSymbolVisibility;
   void addData(SymbolTableSection *SymTab);
 
 public:
-  BinaryELFBuilder(uint16_t EM, MemoryBuffer *MB, uint8_t NewSymbolVisibility)
-      : BasicELFBuilder(EM), MemBuf(MB),
+  BinaryELFBuilder(MemoryBuffer *MB, uint8_t NewSymbolVisibility)
+      : BasicELFBuilder(), MemBuf(MB),
         NewSymbolVisibility(NewSymbolVisibility) {}
 
   std::unique_ptr<Object> build();
 };
 
 class IHexELFBuilder : public BasicELFBuilder {
   const std::vector<IHexRecord> &Records;
 
   void addDataSections();
 
 public:
   IHexELFBuilder(const std::vector<IHexRecord> &Records)
-      : BasicELFBuilder(ELF::EM_386), Records(Records) {}
+      : BasicELFBuilder(), Records(Records) {}
 
   std::unique_ptr<Object> build();
 };
 
 template <class ELFT> class ELFBuilder {
 private:
   using Elf_Addr = typename ELFT::Addr;
   using Elf_Shdr = typename ELFT::Shdr;
ELFBuilder(const ELFObjectFile<ELFT> &ElfObj, Object &Obj,
              Optional<StringRef> ExtractPartition)
       : ElfFile(*ElfObj.getELFFile()), Obj(Obj),
         ExtractPartition(ExtractPartition) {}
 
   void build();
 };
 
 class BinaryReader : public Reader {
-  const MachineInfo &MInfo;
   MemoryBuffer *MemBuf;
   uint8_t NewSymbolVisibility;
 
 public:
-  BinaryReader(const MachineInfo &MI, MemoryBuffer *MB,
-               const uint8_t NewSymbolVisibility)
-      : MInfo(MI), MemBuf(MB), NewSymbolVisibility(NewSymbolVisibility) {}
+  BinaryReader(MemoryBuffer *MB, const uint8_t NewSymbolVisibility)
+      : MemBuf(MB), NewSymbolVisibility(NewSymbolVisibility) {}
   std::unique_ptr<Object> create() const override;
 };
 
 class IHexReader : public Reader {
   MemoryBuffer *MemBuf;
 
   Expected<std::vector<IHexRecord>> parse() const;
   Error parseError(size_t LineNo, Error E) const {

tools/llvm-objcopy/ELF/Object.cpp

 }
 
 void BasicELFBuilder::initFileHeader() {
   Obj->Flags = 0x0;
   Obj->Type = ET_REL;
   Obj->OSABI = ELFOSABI_NONE;
   Obj->ABIVersion = 0;
   Obj->Entry = 0x0;
-  Obj->Machine = EMachine;
+  Obj->Machine = EM_NONE;
   Obj->Version = 1;
 }
 
 void BasicELFBuilder::initHeaderSegment() { Obj->ElfHdrSegment.Index = 0; }
 
 StringTableSection *BasicELFBuilder::addStrTab() {
   auto &StrTab = Obj->addSection<StringTableSection>();
   StrTab.Name = ".strtab";
template <class ELFT> void ELFBuilder<ELFT>::build() {
   readProgramHeaders(HeadersFile);
 }
 
 Writer::~Writer() {}
 
 Reader::~Reader() {}
 
 std::unique_ptr<Object> BinaryReader::create() const {
-  return BinaryELFBuilder(MInfo.EMachine, MemBuf, NewSymbolVisibility).build();
+  return BinaryELFBuilder(MemBuf, NewSymbolVisibility).build();
 }
 
 Expected<std::vector<IHexRecord>> IHexReader::parse() const {
   SmallVector<StringRef, 16> Lines;
   std::vector<IHexRecord> Records;
   bool HasSections = false;
 
   MemBuf->getBuffer().split(Lines, '\n');

tools/llvm-objcopy/ObjcopyOpts.td

 include "CommonOpts.td"
 
 defm binary_architecture
-    : Eq<"binary-architecture", "Used when transforming an architecture-less "
-                                "format (such as binary) to another format">;
+    : Eq<"binary-architecture", "Ignored for compatibility">;
 def B : JoinedOrSeparate<["-"], "B">,
         Alias<binary_architecture>,
         HelpText<"Alias for --binary-architecture">;
 
 defm target : Eq<"target", "Format of the input and output file">,
               Values<"binary">;
 def F : JoinedOrSeparate<["-"], "F">,
         Alias<target>,