[PATCH] D119294: [lld-macho] -flat_namespace for dylibs should make all externs interposable

Di Mo via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 9 13:13:29 PST 2022


modimo added inline comments.
Herald added a project: All.


================
Comment at: lld/MachO/SymbolTable.cpp:99
 
+  // With -flat_namespace, all extern symbols in dylibs are interposable.
+  bool interposable = config->namespaceKind == NamespaceKind::flat &&
----------------
Could this be a `config` property rather than a per-symbol? It looks like a link invocation will always have flat_namespace or never have it.


================
Comment at: lld/MachO/SyntheticSections.cpp:422
 
+static int16_t ordinalForSymbol(const Symbol &sym) {
+  if (const auto *dysym = dyn_cast<DylibSymbol>(&sym))
----------------
>Note that the non-lazy binding and lazy binding sections now have to accommodate both DylibSymbols and (interposable) Defined symbols.

Aside from s/`DylibSymbol`/`Symbol`/g is the only other change to support this here?

Also, would recommended factoring out the s/`DylibSymbol`/`Symbol`/g change to a separate diff as a parent to make it easier to see what files have logic changes in them 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119294



More information about the llvm-commits mailing list