[PATCH] D97521: [lld/mac] Add some support for dynamic lookup symbols, and implement -U

[Nico Weber via Phabricator via llvm-commits]

thakis added inline comments.


================
Comment at: lld/MachO/SyntheticSections.cpp:537
     os << static_cast<uint8_t>(MachO::BIND_OPCODE_SET_DYLIB_ORDINAL_ULEB);
-    encodeULEB128(sym.getFile()->ordinal, os);
   }
----------------
I'll extract the ordinal encoding code here and further up into a shared helper function before landing this change if that sounds ok. It's the same code down here and up there, and it needs the same change now.


================
Comment at: lld/MachO/Writer.cpp:456
       if (defined->overridesWeakDef)
-        in.weakBinding->addNonWeakDefinition(defined);
+        in.weakBinding->addNonWeakDefinition(defined); // XXX do i want this for dynamics?
     } else if (const auto *dysym = dyn_cast<DylibSymbol>(sym)) {
----------------
This is a remainder from an old version of the patch, where I tried to make dynamic lookup symbols weak. But that didn't work due to this getting set, so I changed SymbolTable to make non-dynamic-lookup symbols always win over them. So this XXX is already done (and covered by tests).


================
Comment at: lld/MachO/Writer.cpp:458
     } else if (const auto *dysym = dyn_cast<DylibSymbol>(sym)) {
+      if (dysym->isDynamicLookup()) // XXX votes for separate class
+        continue;
----------------
I was debating if I should make a new Symbol subclass for DylibSymbols that don't come from a file, but in most places both dynamic lookup symbols and dylib-bound symbols will be handled the same way, so I decided against the new class.


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

https://reviews.llvm.org/D97521