[PATCH] D119506: [lld-macho] Set FinalDefinitionInLinkageUnit on most LTO externs

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

> I think Mach-O can be stricter here due to the two-level namespace (which is again covered by the initialization of interposable). We should therefore be able to apply dso_local to non-weak externs too. I think we just have to exclude weak externs since they are effectively handled as flat namespace lookups at runtime.

The ELF condition of `(isExec || sym->visibility != STV_DEFAULT) && ...` is because:
in ELF, a default visibility non-local symbol is by default preemptible/interposable.

With macOS two-level namespace (https://maskray.me/blog/2021-05-16-elf-interposition-and-bsymbolic#alternative-symbol-search-models),
macOS can be stricter.

> Surprisingly, however, despite us marking FinalDefinitionInLinkageUnit = false, the LTO backend doesn't seem to add dso_local to these symbols. OTOH the ELF internalize-exportdyn.ll test shows that dso_local gets added for the equivalent inputs. (cc @MaskRay for the dso_local stuff)

Because `FinalDefinitionInLinkageUnit = true`, lib/LTO/LTO.cpp:802 sets dso_local on `_start`.

---

I agree that `!defined->isExternalWeakDef() && !defined->interposable;` is sufficient for Mach-O.
There is no linker script absolute symbols. `defined->interposable` maps to the ELF preemptibility thing.

It seems that `!defined->isExternalWeakDef()` is not covered by a test.
I think the condition can be removed, but perhaps there is some Mach-O thing that I haven't thought about.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119506