[BinaryFormat] Update Mach-O ARM64E CPU subtype and dumping

[smeenai]

Summary

The new value is taken from <mach/machine.h> in the MacOSX10.14 SDK from
Xcode 10.1. Update llvm-objdump and llvm-readobj accordingly.

[compnerd]

Shouldn't there be an accompanying change to readobj to display the values for the object files?

[mtrent]

I agree, simply updating the enum by itself doesn't accomplish much. Is there something specific driving this change?

EDIT: I mean, is there something specific driving the change other than simply updating to match the macOS 10.14 SDK?

[smeenai]

I was planning on doing the objdump change as a follow-up, but you're right that this change doesn't really accomplish anything as-is :) I'll fold the objdump change into this patch.

[smeenai]

Add objdump and readobj changes

[smeenai]

Of the code additions in this change, I'm only adding a test for one block, because that was the only block I could find related tests for. I'm happy to add tests for the other code blocks for my specific changes, but I don't have the bandwidth to improve the general coverage for them right now.

Note that Apple's objdump (from Xcode 10.1) does print a symbolic subtype for arm64e but not for arm64_v8 (it just prints the numeric subtype there). I don't know if arm64_v8 is actually used for anything right now, and I'm happy to limit this change to just arm64e if Apple prefers.

There's one other block that references CPU_SUBTYPE_ARM64_ALL that I haven't updated, because I don't know what the appropriate values for the new subtypes should be: https://github.com/llvm/llvm-project/blob/95bb3c3cc3f74d2448e6a495c6989f988c24770c/llvm/lib/Object/MachOObjectFile.cpp#L2628

[smeenai]

Limit to arm64e, since that's all Apple tools appear to support

[smeenai]

Ping.

[ab]

Hmm, if I may: what do you want to support? I'd expect Apple to upstream their own patches.

[mtrent]

In D58636#1419023, @smeenai wrote:

Ping.

I'm hoping we can get Ahmed to comment on this review, since this may conflict with the changes he is trying to land.

[smeenai]

In D58636#1420657, @ab wrote:

Hmm, if I may: what do you want to support? I'd expect Apple to upstream their own patches.

I'm playing around with ld64, which requires TAPI for reading tbd files. The existing open source release of TAPI doesn't support arm64e, so it chokes on tbd files from Xcode 10. Adding the CPU_SUBTYPE_ARM64E constant to LLVM allows adding basic arm64e support to TAPI.

From this conversation, it sounds like the arm64e bits might be getting upstreamed soon, which is exciting. I don't know what the timeline on that is like though, and this seems like a small piece that should be pretty similar (if not identical) to what you have internally, so I was hoping it would be okay to land it independently.

[smeenai]

Ping @ab.

[smeenai]

Ping @ab. Any objections to me landing this?

[smeenai]

Ping @ab. Any objections to me landing this?

[smeenai]

@ab, @mtrent – this is a small patch, and I can keep it locally if need be, especially if all the arm64e work will be upstreamed soon. With that said, I'd really appreciate any input on whether this is okay or not from the perspective of conflicting with Apple's planned arm64e upstreaming, and what the timeline on that upstreaming might look like. Also CC'ing @ributzka, since TAPI was my motivation for adding this.

[ab]

Sorry for the delay; go ahead, the patch looks OK.

[smeenai]

Thank you!

llvm/include/llvm/BinaryFormat/MachO.h

enum CPUSubTypeARM {
   //  unused  ARM_V7F     = 10,
   CPU_SUBTYPE_ARM_V7S = 11,
   CPU_SUBTYPE_ARM_V7K = 12,
   CPU_SUBTYPE_ARM_V6M = 14,
   CPU_SUBTYPE_ARM_V7M = 15,
   CPU_SUBTYPE_ARM_V7EM = 16
 };
 
-enum CPUSubTypeARM64 { CPU_SUBTYPE_ARM64_ALL = 0 };
+enum CPUSubTypeARM64 {
+  CPU_SUBTYPE_ARM64_ALL = 0,
+  CPU_SUBTYPE_ARM64_V8 = 1,
+  CPU_SUBTYPE_ARM64E = 2,
+};
 
 enum CPUSubTypeSPARC { CPU_SUBTYPE_SPARC_ALL = 0 };
 
 enum CPUSubTypePowerPC {
   CPU_SUBTYPE_POWERPC_ALL = 0,
   CPU_SUBTYPE_POWERPC_601 = 1,
   CPU_SUBTYPE_POWERPC_602 = 2,
   CPU_SUBTYPE_POWERPC_603 = 3,