[PATCH] D45195: Add --check-library-dependency to maintain compatibility with other linkers

[Rafael Avila de Espindola via Phabricator via llvm-commits]

espindola added inline comments.


================
Comment at: lld/ELF/Options.td:354
+defm check_library_dependency: B<"check-library-dependency",
+    "Do not allow backward symbol references",
+    "Allow backward symbol references">;
----------------
To fetch archive members. Otherwise it sounds like bar.o cannot refer to foo.o  in "foo.o bar.o".



================
Comment at: lld/ELF/SymbolTable.cpp:292
+//
+// --check-library-dependency is an option to detect reverse or cyclic
+// dependencies between static archives, and it can be used to keep your
----------------
This seems to be over selling it. It will not detect cyclic reference if the members that cause it are not used.

I think it is clear to just say that the option prevents an undefined reference from fetching a member written earlier in the command line.



================
Comment at: lld/ELF/SymbolTable.cpp:342
+  if (Config->CheckLibraryDependency && Old && New->GroupId < Old->GroupId)
+    warn("bad backward reference detected: " + Name + " in " + toString(Old) +
+         " depends on " + toString(New));
----------------
I agree that "bad" is out of place. The user asked to find backwards references and we found one.

>From the error it is also not clear that Name is a symbol name. How about something like

Undefined reference Name in Old refers to New.



================
Comment at: lld/ELF/SymbolTable.cpp:378
+
+    checkDependency(Name, File, S->File);
+    fetchLazy<ELFT>(S);
----------------
I think this is the only call to checkDependency that you need. We never find a backward reference when adding a lazy symbol.


https://reviews.llvm.org/D45195