[PATCH] D78270: [lld-macho] Support calls to functions in dylibs
Jez Ng via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 29 00:29:49 PDT 2020
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
More information about the llvm-commits
mailing list