[PATCH] D99822: [lld-macho] Add support for arm64_32

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/MachOStructs.h:40
 
+struct entry_point_command {
+  llvm::support::ulittle32_t cmd;
----------------
gkm wrote:
> Will we be making endian-aware structs here also? E.g., all of the code signing headers are little-endian.
The endianness is secondary; the main motivation here is to avoid misaligned read/write warnings from UBSAN. These wrapper structs should compile down to `memcpy` calls that get elided on architectures supporting unaligned reads (like x86_64).


================
Comment at: lld/test/MachO/arm64-32-stubs.s:3-5
+## FIXME: This test is very similar to arm64-stubs.s, but has been split into a
+## separate file because llvm-objdump doesn't correctly symbolize arm64_32. In
+## particular, the "literal pool symbol address" comments are missing.
----------------
gkm wrote:
> Unifying this one seems within reach:
> * omit the "literal pool symbol address" comments
> * either ...
> ** match `{{[wx]}}16` or
> ** define a FileCheck macro with the proper register name
I think the 'literal pool symbol address' comments are useful though -- they verify that the instructions are actually referring to the right pointers. That (unfortunately) isn't being checked in the arm64-32 test.


================
Comment at: lld/test/MachO/lit.local.cfg:19
+# shortest substitution of "%lld".
 lld = ('ld64.lld -arch x86_64 -platform_version macos 10.0 11.0 -syslibroot ' +
     os.path.join(config.test_source_root, "MachO", "Inputs", "MacOSX.sdk"))
----------------
gkm wrote:
> Perhaps rename this as `%lld-macos` in a separate diff?
eh, I think the `RUN:` lines are too long already as-is...


================
Comment at: lld/test/MachO/segments.s:56-58
+# CHECK-NEXT: Size: {{.*}}
+# CHECK-NEXT: vmaddr: {{.*}}
+# CHECK-NEXT: vmsize: {{.*}}
----------------
gkm wrote:
> Why add ` {{.*}}` here and elsewhere? To emphasize that there is more on the line?
I added `--match-full-lines` to the FileCheck invocation in order to properly distinguish between `LC_SEGMENT` and `LC_SEGMENT_64`. But that also required adding `{{.*}}` here so this line would continue to match. Maybe it'll be cleaner to just add `{{$}}` to the `LC_SEGMENT` lines...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99822