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

[Davide Italiano via Phabricator via llvm-commits]

davide requested changes to this revision.
davide added a comment.
This revision now requires changes to proceed.

This is getting better.
Please rebase the patch because it didn't apply cleanly.



================
Comment at: lld/ELF/SymbolTable.cpp:131
+  // Restore bindings of renamed symbols.
+  for (auto &i : Config->RenamedSymbols) {
+    SymbolBody *Body = find(i.first());
----------------
Convention says all Variable names should start with capital letter. Also, `i` is a little vague.


================
Comment at: llvm/include/llvm/LTO/LTO.h:369-370
   SymbolResolution()
-      : Prevailing(0), FinalDefinitionInLinkageUnit(0), VisibleToRegularObj(0) {
+      : Prevailing(0), FinalDefinitionInLinkageUnit(0), VisibleToRegularObj(0),
+        LinkerRedefined(0) {
   }
----------------
Is this formatted? Doesn't seem so.


================
Comment at: llvm/lib/LTO/LTO.cpp:421
   GlobalRes.VisibleOutsideThinLTO |=
-      (Res.VisibleToRegularObj || Sym.isUsed() ||
+      (Res.LinkerRedefined || Res.VisibleToRegularObj || Sym.isUsed() ||
        Partition == GlobalResolution::RegularLTO);
----------------
If I remove this line, all the tests still pass.


================
Comment at: llvm/lib/LTO/LTO.cpp:441-442
       OS << 'x';
+    if (Res.LinkerRedefined)
+      OS << 'r';
     OS << '\n';
----------------
If I comment out this line, all the tests still pass.


================
Comment at: llvm/lib/LTO/LTO.cpp:547-548
         Keep.push_back(GV);
+        if (Res.LinkerRedefined)
+          GV->setLinkage(GlobalValue::WeakAnyLinkage);
         switch (GV->getLinkage()) {
----------------
This needs a comment.


https://reviews.llvm.org/D33621