[PATCH] D78270: [lld-macho] Support calls to functions in dylibs

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/InputSection.cpp:11
 #include "OutputSegment.h"
 #include "Symbols.h"
 #include "Target.h"
----------------
MaskRay wrote:
> Why is #include "SyntheticSections.h" dropped?
It was originally included so we could access `in.got`. But now we're just using `target->getDylibSymbolVA()` from `Target.h`.


================
Comment at: lld/test/MachO/dylink-lazy.s:20
+# RUN: (llvm-objdump -d --no-show-raw-insn --syms --bind --lazy-bind %t/dylink-lazy; \
+# RUN:  llvm-objdump --disassemble-all --no-show-raw-insn %t/dylink-lazy) | FileCheck %s
+
----------------
MaskRay wrote:
> `--disassemble-all` can be shorten as `-D`. --no-show-raw-insn may not be needed for -D
Will change. Though it looks like `--no-show-raw-insn` is still needed


================
Comment at: lld/test/MachO/dylink-lazy.s:41
+# CHECK-LABEL: Disassembly of section __TEXT,__stub_helper:
+# CHECK:       {{0*}}[[#%x, STUB_HELPER_ENTRY:]] <__stub_helper>:
+# CHECK-NEXT:  leaq [[#%u, IMGLOADER - STUB_HELPER_ENTRY - 7]](%rip), %r11
----------------
MaskRay wrote:
> Consider increasing indentation (i.e. add one space) for instructions covered by the section `Disassembly of section __TEXT,__stub_helper:`
Looking at a smattering of lld-ELF's tests, I found files that used zero, two, four, and eight spaces :|

An odd number of spaces feels a bit, uh, odd :P I think I'll go with two


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78270