[PATCH] [lld] [ELF] Fix inclusion of weak symbols in the dynamic symbol table

[Rafael Auler]

Hi Shankar,

Thanks for looking this code. I answered your question below.

================
Comment at: lib/ReaderWriter/ELF/DefaultLayout.h:567-586
@@ -564,9 +566,22 @@
+
     // Add runtime relocations to the .rela section.
     for (const auto &reloc : *definedAtom) {
-      if (_context.isDynamicRelocation(*definedAtom, *reloc))
+      bool isLocalReloc = true;
+      if (_context.isDynamicRelocation(*definedAtom, *reloc)) {
         getDynamicRelocationTable()->addRelocation(*definedAtom, *reloc);
-      else if (_context.isPLTRelocation(*definedAtom, *reloc))
+        isLocalReloc = false;
+      } else if (_context.isPLTRelocation(*definedAtom, *reloc)) {
         getPLTRelocationTable()->addRelocation(*definedAtom, *reloc);
-      if (const auto *sla = dyn_cast<SharedLibraryAtom>(reloc->target()))
-        _referencedDynAtoms.insert(sla);
+        isLocalReloc = false;
+      }
+
+      if (!reloc->target())
+        continue;
+
+      //Ignore undefined atoms that are not target of dynamic relocations
+      if (isa<UndefinedAtom>(reloc->target()) && isLocalReloc)
+        continue;
+
+      _referencedDynAtoms.insert(reloc->target());
     }
+
----------------
shankarke wrote:
> If we are creating a dynamic relocation for a symbol, that symbol should go in the dynamic symbol table. Looks like this code is removing undefined symbols that are targets of dynamic relocations. 
If the symbol is the target of a dynamic relocation, that relocation should be reported by the backend by "isDynamicRelocation" or "isPLTRelocation", shouldn't it? This code  should separate regular relocations from dynamic ones, and I think that a good way to figure out which are the dynamic relocations is to ask the backend.

http://reviews.llvm.org/D5571