[PATCH] D21794: [ELF] - Added support for --unresolved-symbols option.

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/Config.h:40
@@ +39,3 @@
+  IgnoreInObjectFiles,
+  IgnoreInSharedLibs
+};
----------------
ruiu wrote:
> We always ignore undefined symbols in DSOs, so this enum shouldn't have IgnoreInSharedLibs. This also overlaps with NoinhibitExec, so it needs reorganization.
> 
> I think you want to define this in Config.
> 
>   enum { Error, Warn, Ignore };
> 
> and
> 
>  - set to Error by default or if --unresolved-symbols=IgnoreAll
>  - set to Warn if --noinhibit-exec
>  - set to Ignore if --unresolved-symbols={IgnoreAll,IgnoreInObjectFiles,IgnoreInSharedLibs}
This enum was for describing possible options for unresolved-symbols initially,
but if you want to remove reportUndefinedFromSharedLibs() that of cource
needs reorganization.

I would also add --no-undefined here then.

So can be:

Error by default or if --no-undefined or --unresolved-symbols={ReportAll, IgnoreInSharedLibs}
Warn if --noinhibit-exec
Ignore if --unresolved-symbols={IgnoreAll,IgnoreInObjectFiles}

================
Comment at: ELF/Writer.cpp:734
@@ +733,3 @@
+template <class ELFT>
+static void reportUndefinedFromSharedLibs(SymbolTable<ELFT> &Symtab) {
+  // In lld we do not report about undefines in DSO, so
----------------
ruiu wrote:
> We don't want to do this -- we don't care whether undefined symbols in DSOs are resolved or not unlike GNU linkers.
Generally yes, but I was thinking about this option as about the only option that can show this information in lld.
That was key feature of this option for me.

Shouldn't we show warning then ? For example when IgnoreInObjectFiles is used explicitly we probably want to inform user that
it does not do what is expected from documentation ? (does not show eerors for symbols from DSO).


http://reviews.llvm.org/D21794