[PATCH] D66449: [llvm-objcopy] Accept MachO formats in commad-line parsing

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.cpp:296-297
+    {"elf64-x86-64", {ELF::EM_X86_64, 0, 0, true, true}},
+    {"mach-o-x86-64",
+     {0, MachO::CPU_TYPE_X86_64, MachO::CPU_SUBTYPE_X86_64_ALL, true, true}},
     // Intel MCU
----------------
The others appear to be:

```
./objcopy --info | grep ^mach-o | sort -u
mach-o-arm
mach-o-arm64
mach-o-be
mach-o-fat
mach-o-i386
mach-o-le
mach-o-x86-64
```

I don't think we need to add them all in this patch, but it'd be nice to see at least one more to make sure the test isn't too rigid. The rest can then be added in a trivial followup patch. (Or you can add them all now).


================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.cpp:329
   StringRef OriginalTargetName = TargetName;
   bool IsFreeBSD = TargetName.consume_back("-freebsd");
   auto Iter = TargetMap.find(TargetName);
----------------
`-freebsd` seems to be an ELF-specific thing -- this should not be consumed if the target does not start with "elf"


================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.h:45-49
   // Alternative constructor that defaults to NONE for OSABI.
-  MachineInfo(uint16_t EM, bool Is64, bool IsLittle)
-      : MachineInfo(EM, ELF::ELFOSABI_NONE, Is64, IsLittle) {}
+  MachineInfo(uint16_t EM, uint32_t MachOCPUType, uint32_t MachOSubCPUType,
+              bool Is64, bool IsLittle)
+      : MachineInfo(EM, ELF::ELFOSABI_NONE, MachOCPUType, MachOSubCPUType, Is64,
+                    IsLittle) {}
----------------
I don't think this constructor is actually used anymore


================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.h:53-58
+  // ELF values.
   uint16_t EMachine;
   uint8_t OSABI;
+  // MachO values.
+  uint32_t MachOCPUType;
+  uint32_t MachOCPUSubType;
----------------
Might be good to extract these to separate types, then initialized like:

```
{"x86-64",
     {{ELF::EM_X86_64}, {MachO::CPU_TYPE_X86_64, MachO::CPU_SUBTYPE_X86_64_ALL},
     true, true}
```

to more clearly separate elf vs mach-o fields (and I imagine we could have COFF fields too)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66449/new/

https://reviews.llvm.org/D66449