[PATCH] D13135: [ELF2] Add support for -L and -l command line switches

Denis Protivensky via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 25 02:34:15 PDT 2015


denis-protivensky added a subscriber: denis-protivensky.

================
Comment at: ELF/Driver.cpp:73
@@ +72,3 @@
+  bool HasColonPrefix = Path[0] == ':';
+  SmallString<64> StaticName, DynamicName;
+  if (HasColonPrefix) {
----------------
Maybe better to have `SmallVector<SmallString<64>, 2> Libs` instead?
This solves couple of problems:
1. Removes ambiguity when `HasColonPrefix == true`, because you'll put just one name into the vector instead of having two equal names like now.
2. Removes code duplication in the search dir loop, because you'll have both dynamic and static names in the vector and will iterate over them in the unified way.
3. When `-static` is added, you won't need to make complex checks or code changes. Just check for static flag and don't add dynamic name to the vector.

================
Comment at: ELF/Driver.cpp:96
@@ +95,3 @@
+  }
+  if (HasColonPrefix && sys::fs::exists(StaticName))
+    return StaticName.str().str();
----------------
This check is not correct. Colon prefix affects only name being searched, but it doesn't allow to specify absolute name which should be used.

================
Comment at: ELF/Driver.cpp:194
@@ +193,3 @@
+  const ErrorOr<std::string> PathOrErr = searchLibrary(Path);
+  error(PathOrErr, Twine("Unable to find library -l", Path));
+  assert(sys::fs::exists(PathOrErr.get()));
----------------
You need a negative test for this error case.

================
Comment at: test/elf2/libsearch.s:8
@@ +7,3 @@
+// Should not link because of undefined symbol _bar
+// RUN: not lld -flavor gnu2 -o %t3 %t.o 2>&1 | FileCheck --check-prefix=UNDEFINED %s
+// UNDEFINED: undefined symbol: _bar
----------------
Need to check that `lld -flavor gnu2 -o %t3 -L%T -lls %t.o` also fails with undefined symbol.

================
Comment at: test/elf2/libsearch.s:11
@@ +10,3 @@
+
+// RUN: lld -flavor gnu2 -o %t3 %t.o -L%T -lls
+// RUN: lld -flavor gnu2 -o %t3 %t.o -L%T -l:libls.a
----------------
Check also this combination as correct: `lld -flavor gnu2 -o %t3 %t.o -lls -L%T`


http://reviews.llvm.org/D13135





More information about the llvm-commits mailing list