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

Jez Ng via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 17 22:53:16 PDT 2021


int3 added a comment.

Will do a proper review tomorrow... just some quick comments before I go to sleep.

> No that's not weird - they said the _execute_ is an absolute symbol.

I said it was weird because in my first `test.cpp` example it isn't an absolute symbol, but in the load-command.s test it was. I realized that was because our lit.local.cfg file specifies an early macOS platform version, whereas by default ld64 will target a modern macOS (>= 10.6) where PIEs are the default (see our `isPie()` function). We should probably mimic this behavior.



================
Comment at: lld/MachO/Symbols.h:86-88
+  // Whether this symbol should be forced to appear in the output binary's
+  // symbol table.
+  SymbolTablePresence presence : 2;
----------------
oontvoo wrote:
> int3 wrote:
> > hm why the need to replace `linkerInternal` with this? (Do we still need it if the `__mh_*` symbols are Defineds?)
> How about calling it `presentInSymbolTable`?
> 
> Calling it `linkerInternal` is weird because all of these are linker internal ... it's just some of them are not to be included in the symbol table
> 
Let me sleep on this... but naming aside, I'm also trying to understand if we need a tristate enum rather than a boolean


================
Comment at: lld/MachO/SyntheticSections.cpp:70-71
 
+PreloadHeaderSection::PreloadHeaderSection()
+    : MachHeaderSection(segment_names::header, section_names::header) {}
+
----------------
```
     -preload    Produces a mach-o file in which the mach_header, load commands, and sym-
                 bol table are not in any segment.  This output type is used for firmware
                 or embedded development where the segments are copied out of the mach-o
                 into ROM/Flash.
```

I think it's unlikely that we'll add real support for `-preload` any time soon (due to lack of use cases), so I wouldn't bother even partially supporting the MH_PRELOAD case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97007



More information about the llvm-commits mailing list