[PATCH] D97007: [lld-macho] Define __mh_*_header synthetic symbols.

[Jez Ng via Phabricator via llvm-commits]

int3 added a comment.

So if I understand correctly, we need to make `__mh_execute_header` look like it's part of `__text` when we emit the symbol table. I have two ideas:

1. This is kinda sneaky but ... if all we care about is the symbol table entry matching the right section, that means we just need to get the `n_sect` value to match, so we could have MachHeaderSection pretend to be the text section by giving it a section index of 1.
2. We could make MachHeaderSection inherit from an InputSection rather than a OutputSection (SyntheticSection), and then have it be part of the `__TEXT` MergedOutputSection. This is more architecturally consistent, but would probably require a good deal of refactoring.

My inclination is to go with #1; I think it's a simple enough hack, and unlikely to bite us in the ass down the road (fingers crossed)



================
Comment at: lld/MachO/SyntheticSections.cpp:1018
+void macho::createSyntheticSymbols(bool isPie) {
+  auto addSynSymMachHeader = [](const char *name, bool privateExtern = true) {
+    symtab->addSynthetic(name, in.header->isec, 0,
----------------
`addSynSymMachHeader` is kind of a mouthful... how about `addHeaderSymbol`?


================
Comment at: lld/MachO/SyntheticSections.cpp:1025-1026
+  switch (config->outputType) {
+  // From the Apple documentation, this symbol is an absolute
+  // symbol if not linking a PIE.
+  case MH_EXECUTE:
----------------
I would drop this comment... the default assumption is that we're emulating ld64's behavior.


================
Comment at: lld/MachO/SyntheticSections.cpp:1028
+  case MH_EXECUTE:
+    if (isPie) {
+      // FIXME: Try to find the (__TEXT, __text) section
----------------
instead of passing isPie, we could just check if `config->isPic` here. MH_EXECUTE + isPic => PIE.


================
Comment at: lld/MachO/SyntheticSections.cpp:1033
+      symtab->addSynthetic("__mh_execute_header",
+                           /*isec*/ nullptr, 0,
+                           /*privateExtern*/ false,
----------------
>From ld64's output, it looks like the absolute symbol will still have the address of the header. I guess we could support it by putting `in.header->addr` here, but we would need to move `createSyntheticSymbols` after `assignAddresses`.

(If it turns out to be uglier than that, I think it's fine that we don't have 100% parity; modern macOS binaries are all PIE, so this will be a very rare use case.)


================
Comment at: lld/MachO/SyntheticSections.cpp:1056
+  }
+#undef addSynHelper
+  // The Itanium C++ ABI requires dylibs to pass a pointer to __cxa_atexit
----------------
can rm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97007