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

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: lld/ELF/Driver.cpp:972
 
+  // Pass wrapped symbols to LTO
+  for (auto *Arg : Args.filtered(OPT_wrap))
----------------
This comment and the one below aren't really accurate any more because this code is used in the non-LTO case as well.


================
Comment at: lld/ELF/LTO.cpp:139
       undefine(Sym);
+    R.LinkerRedefined = Config->RenamedSymbols.find(Sym) !=
+        Config->RenamedSymbols.end();
----------------
I think this can just be `Config->RenamedSymbols.count(Sym)`.


================
Comment at: lld/ELF/SymbolTable.cpp:160
+// Otherwise an undef for it will be created with STB_GLOBAL.
+static uint8_t getSymbolBinding(SymbolBody *SB) {
+  uint8_t Binding = STB_GLOBAL;
----------------
Is this function necessary? All callers pass a non-null SymbolBody, so they can just query the binding directly.


================
Comment at: lld/ELF/SymbolTable.cpp:167-169
-  // We rename symbols by replacing the old symbol's SymbolBody with the new
-  // symbol's SymbolBody. This causes all SymbolBody pointers referring to the
-  // old symbol to instead refer to the new symbol.
----------------
Move this comment to applySymbolRenames.


================
Comment at: lld/ELF/SymbolTable.cpp:172
+template <class ELFT> void SymbolTable<ELFT>::addSymbolWrap(StringRef Name) {
+  SymbolBody *SB = find(Name);
+  if (!SB)
----------------
I'd minimise the diff here and rename this variable back.


https://reviews.llvm.org/D33621