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

Tue Mar 15 11:00:29 PDT 2022

int3 added a comment.

> Curious: are weak externs user annotated? Wondering how weak extern + canBeOmittedFromSymbolTable() can exist.

All the internalized symbols in D121428: [lld-macho] Extend lto-internalize-unnamed-addr.ll <https://reviews.llvm.org/D121428> are weak externs that satisfy `canBeOmittedFromSymbolTable()`. I don't think there's a conflict here...?

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

Oops, I mistyped, I meant that we were setting `FinalDefinitionInLinkageUnit = true`. But I see now that the symbols for which we were setting it to true in `lto-internalize-unnamed-addr.ll` were getting internalized anyway, so that's probably why they weren't marked `dso_local`.

> It seems that !defined->isExternalWeakDef() is not covered by a test.

Added one in `lto-final-definition.ll`.

> I think the condition can be removed, but perhaps there is some Mach-O thing that I haven't thought about.

Weak symbol lookups in Mach-O essentially happen in a flat namespace, so I think they should never be treated as final definitions. Also, this lookup occurs at runtime regardless of whether we are building an executable or dylib, and regardless of whether the symbol is a Defined or a DylibSymbol. Does ELF behave differently?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119506