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

[Greg McGary via Phabricator via llvm-commits]

gkm added inline comments.


================
Comment at: lld/MachO/Arch/ARM64.cpp:78
 
-void ARM64::writeStub(uint8_t *buf8, const Symbol &sym) const {
-  auto *buf32 = reinterpret_cast<uint32_t *>(buf8);
-  uint64_t pcPageBits =
-      pageBits(in.stubs->addr + sym.stubsIndex * sizeof(stubCode));
-  uint64_t lazyPointerVA =
-      in.lazyPointers->addr + sym.stubsIndex * LP64::wordSize;
-  buf32[0] = encodePage21({&sym, "stub"}, stubCode[0],
-                          pageBits(lazyPointerVA) - pcPageBits);
-  buf32[1] = encodePageOff12(stubCode[1], lazyPointerVA);
-  buf32[2] = stubCode[2];
+void ARM64::writeStub(uint8_t *buf8, const macho::Symbol &sym) const {
+  ::writeStub<LP64, stubCode>(buf8, sym);
----------------



================
Comment at: lld/MachO/Arch/ARM64_32.cpp:32
+  ARM64_32();
+  void writeStub(uint8_t *buf, const macho::Symbol &) const override;
+  void writeStubHelperHeader(uint8_t *buf) const override;
----------------



================
Comment at: lld/MachO/Arch/ARM64_32.cpp:47
+      {"UNSIGNED",
+       B(UNSIGNED) | B(ABSOLUTE) | B(EXTERN) | B(LOCAL) | B(DYSYM8) | B(BYTE4)},
+      {"SUBTRACTOR", B(SUBTRAHEND) | B(BYTE4)},
----------------



================
Comment at: lld/MachO/Arch/ARM64_32.cpp:77
+
+void ARM64_32::writeStub(uint8_t *buf8, const macho::Symbol &sym) const {
+  ::writeStub<ILP32, stubCode>(buf8, sym);
----------------



================
Comment at: lld/MachO/MachOStructs.h:40
 
+struct entry_point_command {
+  llvm::support::ulittle32_t cmd;
----------------
Will we be making endian-aware structs here also? E.g., all of the code signing headers are little-endian.


================
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.
----------------
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


================
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"))
----------------
Perhaps rename this as `%lld-macos` in a separate diff?


================
Comment at: lld/test/MachO/segments.s:56-58
+# CHECK-NEXT: Size: {{.*}}
+# CHECK-NEXT: vmaddr: {{.*}}
+# CHECK-NEXT: vmsize: {{.*}}
----------------
Why add ` {{.*}}` here and elsewhere? To emphasize that there is more on the line?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99822