[PATCH] D38790: [ELF] Avoid keeping useless undefined symbols in .dynsym.

Igor Kudrin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 27 06:05:50 PDT 2017


ikudrin added inline comments.


================
Comment at: ELF/Writer.cpp:1281-1291
     if (InX::DynSymTab && S->includeInDynsym()) {
+      if (Body->isUndefined() && !Body->IsUsedInReloc)
+        // This symbol was most probably used in a section, which was collected
+        // in the GC phase. We don't need such symbols in the dynamic symbols
+        // table because they solely introduce useless dependencies.
+        continue;
       InX::DynSymTab->addSymbol(Body);
----------------
ruiu wrote:
> I don't think this is logically correct. If `includeInDynsym` returns true, the symbol should be included in a .dynsym regardless of other conditions. You should modify other place so that `includeInDynsym` works correctly.
`includeInDynsym` is used when computing `IsPreemptible`. This flag is calculated before calling `scanRelocations` because it is used there. And with my approach, we can say that a symbol is useless only after `scanRelocations` is finished and the symbol has not been used anywhere.

`includeInDynsym` is also called in `SymbolTable::handleDynamicList`, even before GC. At that point, we can't say if a symbol is going to become unreferenced after GC or not.

What if we just rename `includeInDynsym` to something like `maybeIncludeInDynsym`?


https://reviews.llvm.org/D38790





More information about the llvm-commits mailing list