[PATCH] D33621: Fix for -wrap linker option and LTO, PR 33145

[Davide Italiano via Phabricator via llvm-commits]

davide added inline comments.


================
Comment at: lld/ELF/Config.h:102
+
+  // Mapping of renamed symbols to their original pre-LTO bindings.
+  llvm::DenseMap<Symbol*, uint8_t> RenamedSymbols;
----------------
This is the only field in `Config` that has a comment. Not sure how I feel about it, I'll defer it to Rui.


================
Comment at: lld/ELF/SymbolTable.cpp:191-194
+  if (!SB) {
+    error("-defsym: undefined symbol: " + Name);
+    return;
+  }
----------------
Is this bit needed? I don't see a test for it.


================
Comment at: lld/ELF/SymbolTable.cpp:195
+  }
+  Symbol *Sym = SB->symbol();
+  Symbol *AliasSym = addUndefined(Alias);
----------------
Can this ever be null? If so, you should check for it.
If not, you may as well inline this in the single use.


================
Comment at: lld/ELF/SymbolTable.h:42-43
   void addCombinedLTOObject();
+  void addLTOSymbolWrap(StringRef Name);
+  void addLTOSymbolAlias(StringRef Alias, StringRef Name);
 
----------------
Unsorted.


================
Comment at: llvm/lib/LTO/LTO.cpp:407-410
   // Set the partition to external if we know it is used elsewhere, e.g.
   // it is visible to a regular object, is referenced from llvm.compiler_used,
   // or was already recorded as being referenced from a different partition.
+  if (Res.LinkerRedefined || Res.VisibleToRegularObj || Sym.isUsed() ||
----------------
This comment is now stale after your change.


https://reviews.llvm.org/D33621