[ELF] --exclude-libs: don't assign VER_NDX_LOCAL to undefined symbols

[MaskRay]

Summary

Suggested by Peter Collingbourne.

Non-VER_NDX_GLOBAL versions should not be assigned to defined symbols. --exclude-libs violates this and can cause a spurious error "cannot refer to absolute symbol" after D71795.

excludeLibs incorrectly assigns VER_NDX_LOCAL to an undefined weak symbol =>
isPreemptible is false =>
R_PLT_PC is optimized to R_PC =>
in isStaticLinkTimeConstant, an error is emitted.

[merge_guards_bot]

Unit tests: pass. 61800 tests passed, 0 failed and 781 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[grimar]

I think this is fine.
I'd suggest to add a comment to the test case (its generally helpfull in a long term, though I know we often do not add them in LLDs tests.)

lld/test/ELF/exclude-libs-undef.s

7

Do you need both t.a and %t.dir/t.a?

[MaskRay]

Add comment

[grimar]

LGTM, but please wait for another possible comments too.

lld/test/ELF/exclude-libs-undef.s

4

"we do" I think.

[merge_guards_bot]

Unit tests: fail. 61801 tests passed, 1 failed and 781 were skipped.

failed: libc++.std/thread/thread_mutex/thread_mutex_requirements/thread_sharedtimedmutex_requirements/thread_sharedtimedmutex_class/try_lock.pass.cpp

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[pcc]

LGTM

[MaskRay]

Fix comment

[merge_guards_bot]

Unit tests: pass. 61848 tests passed, 0 failed and 781 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

lld/ELF/Driver.cpp

 static void excludeLibs(opt::InputArgList &args) {
   DenseSet<StringRef> libs = getExcludeLibs(args);
   bool all = libs.count("ALL");
 
   auto visit = [&](InputFile *file) {
     if (!file->archiveName.empty())
       if (all || libs.count(path::filename(file->archiveName)))
         for (Symbol *sym : file->getSymbols())
-          if (!sym->isLocal() && sym->file == file)
+          if (!sym->isUndefined() && !sym->isLocal() && sym->file == file)
             sym->versionId = VER_NDX_LOCAL;
   };
 
   for (InputFile *file : objectFiles)
     visit(file);
 
   for (BitcodeFile *file : bitcodeFiles)
     visit(file);

lld/test/ELF/exclude-libs-undef.s

+# REQUIRES: x86
+## Test we don't assign VER_NDX_LOCAL to an undefined symbol.
+## If we do, an undefined weak will become non-preemptible,
+## and we will report an error when an R_PLT_PC (optimized to R_PC)
+## references the undefined weak (considered absolute).
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
+# RUN: rm -f %t.a
+# RUN: llvm-ar rc %t.a %t.o
+# RUN: ld.lld -shared --whole-archive --exclude-libs=ALL %t.a -o %t.so
+# RUN: llvm-readelf --dyn-syms %t.so | FileCheck %s
+
+# CHECK:     1: {{.*}} WEAK DEFAULT UND bar
+# CHECK-NOT: 2:
+
+.globl foo
+.weak bar
+foo:
+  call bar