[PATCH] D12226: [LLD] Support for --unresolved-symbols option in llvm lld for ELF file format

[Shridhar Joshi via llvm-commits]

joshishr added inline comments.

================
Comment at: lib/Driver/GnuLdDriver.cpp:439-442
@@ +438,6 @@
+    }
+    else if (val.compare("ignore-all") &&
+             val.compare("report-all") &&
+             val.compare("ignore-in-object-files") &&
+             val.compare("ignore-in-shared-libs")) {
+
----------------
ruiu wrote:
> It's better to define enums for these strings and use them in Resolver.cpp instead of strings.
Yes, enum will be good. Will update the patch.

================
Comment at: lib/Driver/GnuLdDriver.cpp:458
@@ +457,3 @@
+    ctx->setPrintRemainingUndefines(true);
+    ctx->setAllowRemainingUndefines(false);
+  }
----------------
ruiu wrote:
> I'm wondering if this is correct. Does --unresolved-symbols really negate --allow-shlib-undefines?
Not all values of --unresolved-symbols option negate --allow-shlib-undefines.
But --allow-shlib-undefines option is only for shared libraries whereas --unresolved-symbols is for relocatable object files, shared libraries as well as executable.
I have tested behavior of all values of --unresolved-symbols in combination with --allow-shlib-undefines and --no-allow-shlib-undefines with gnu ld and gold.

Implementation of --unresolved-symbols option submitted in lld is in compliance with gnu ld and gold.

================
Comment at: test/elf/unresolved-symbols.test:8
@@ +7,3 @@
+# Test if option is used in correct form
+RUN: clang -c %p/Inputs/main_unresolved-symbols.c -o %p/Inputs/main_unresolved-symbols.o
+RUN: clang -c %p/Inputs/foo_unresolved-symbols.c -o %p/Inputs/foo_unresolved-symbols.o
----------------
atanasyan wrote:
> Is it possible to escape using Clang in the test? Usually build-bots do not build Clang with LLD.
Sure, I will update test case by avoiding clang usage.


http://reviews.llvm.org/D12226