[PATCH] D133825: [lld-macho] Add support for N_INDR symbols

[Vincent Lee via Phabricator via llvm-commits]

thevinster accepted this revision.
thevinster added a comment.

Overall, lgtm. Thanks!



================
Comment at: lld/MachO/Driver.cpp:1216
+    if (auto *objFile = dyn_cast<ObjFile>(file)) {
+      for (const AliasSymbol *alias : objFile->aliases) {
+        if (const auto &aliased = symtab->find(alias->getAliasedName())) {
----------------
thakis wrote:
> This feels a bit inelegant. Conceptually, an alias and its target could be two map entries pointing at the same symbol – if it weren't for the fact that the alias can have more restrictive visibility :/
If we have an object file that uses aliases and we also specify `-alias` from the command line, the object file alias will replace the one from the command line. Conceptually speaking, should it be the other way around? I haven't tested this explicitly but am interested to know this result. 


================
Comment at: lld/MachO/SymbolTable.cpp:120
+                                   InputFile *newFile, bool makePrivateExtern) {
+  bool isPrivateExtern = makePrivateExtern || src->privateExtern;
+  return addDefined(target, newFile, src->isec, src->value, src->size,
----------------
Does this mean that both the alias and aliased symbol can differ in symbol visibility? If so, might be worth adding a test for it if it's expected behavior. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133825