[PATCH] D21550: [lld] support --trace-symbol (alias -y) option

[Shridhar Joshi via llvm-commits]

joshishr marked 14 inline comments as done.
joshishr added a comment.

In http://reviews.llvm.org/D21550#463636, @rafael wrote:

> Please also add a test with --start-lib/--end-lib.


Added.


================
Comment at: ELF/InputFiles.cpp:405
@@ -374,3 +404,3 @@
           ->body();
     return elf::Symtab<ELFT>::X->addRegular(Name, *Sym, Sec)->body();
   }
----------------
rafael wrote:
> Don't you want to trace this?
It was traced inside addRegular to distinguish between weak and strong symbol. In updated patch it is handled in SymbolTable.

================
Comment at: ELF/InputFiles.cpp:558
@@ -527,2 +557,3 @@
     if (Sym.isUndefined()) {
+      InputFile::traceUndefined(Name);
       Undefs.push_back(Name);
----------------
ruiu wrote:
> rafael wrote:
> > Do you really want to trace this? It is not a "real" undefined symbol. We will not fetch archive members or report an error for it.
> Agreed. That's why I asked to define `traceUndefined` to `ObjectFile` instead of `InputFile` since the class is the only class that needs the method.
This was required to print undefined symbols from shared library and not from archive members.
For e.g. main calls foo() defined in libfoo.so and this foo() calls bar() defined in libbar.so. this call to traceUndefined() was required to print reference to bar(). Anyway i have removed it in updated patch and so updated patch does not report references from shared library.

Anyway if you think this is required and we should also move traceUndefined() to ObjectFile then also we can report this using Undefs inside SymbolTable. But then we have to rename traceDefined() as it will handle undefined symbols from shared library as well as defined symbols from object files, shared and archive libraries.

Please confirm.


================
Comment at: ELF/SymbolTable.cpp:348-349
@@ -347,2 +347,4 @@
   int Cmp = compareDefinedNonCommon(S, WasInserted, Sym.getBinding());
+  if (Cmp >= 0)
+    cast<InputFile>(Section->getFile())->traceDefined(Name);
   if (Cmp > 0)
----------------
ruiu wrote:
> I don't think we need to print out weak symbols that are not linked to the resulting executable. We are trying hard to minimize the amount of code that's executed for each symbol, so I don't want to add new code here. Please iterate only over symbols passed as --trace-symbol arguments instead of iterating over all symbols in all input files.
With traceDefined() moved to SymbolTable class, there is change in behaviour:
1. Definition to weak function will not be printed even if the object file or shared shared library defining it comes before in the link order than object file or shared library defining the same symbol strong.
2. Definition of common symbols from object files and archive library will not be printed but common symbols from shared library will be printed. This looks like because DefinedCommon class does not contain source file information.



http://reviews.llvm.org/D21550