[PATCH] D86910: [lld-macho] Implement and test resolution of common symbols

[Greg McGary via Phabricator via llvm-commits]

gkm accepted this revision.
gkm added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lld/MachO/SymbolTable.cpp:85
     if (auto *common = dyn_cast<CommonSymbol>(s)) {
       if (size < common->size)
         return s;
----------------
int3 wrote:
> gkm wrote:
> > s/`<`/`<=`/. I assume that `replaceSymbol` implies extra work, so why do that for equal-sized data?
> it's to change the alignment. (See also discussion in D86909)
Let me be sure I understand:
  - You coded it this way for sake of consistency with ld64.
  - A later equal-sized common with smaller alignment will supersede the earlier with greater alignment.
  - This seems wrong, but whatever. Call it bug-for-bug compatible.
  - It must be a bug of little consequence, otherwise it would be fixed. Maybe we should fix it anyway and see if someone complains!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86910