[ELF] Don't set versionId on undefined weak lazy symbols

[MaskRay]

Summary

An unfetched lazy symbol (undefined weak) should be considered to have its
original versionId which is VER_NDX_GLOBAL, instead of the lazy symbol's
versionId. (The original versionId cannot be non-VER_NDX_GLOBAL because a
undefined versioned symbol is an error.)

The regression was introduced in D77280 when making version scripts work
with lazy symbols fetched by LTO calls.

Fix PR49915

[MaskRay]

use --dyn-syms instead of -V

[peter.smith]

LGTM.

If I've got this right:

• We assign versionid before LTO code generation
• We assign versionid to lazy symbols from script in case LTO fetches the symbol
• If the lazy symbol is not referenced we want to give it VER_NDX_GLOBAL so we have to do this at write time.

I'm trying to think if there is any reason we can't delay scanVersionScript() till after LTO code generation. If we could do that then we'd know which symbols were lazy and we wouldn't need to assign them a versionid. I don't know off the top of my head how much work it would be to check that though.

[MaskRay]

In D100624#2701483, @peter.smith wrote:

LGTM.

Thanks for the review.

If I've got this right:

• We assign versionid before LTO code generation
• We assign versionid to lazy symbols from script in case LTO fetches the symbol
• If the lazy symbol is not referenced we want to give it VER_NDX_GLOBAL so we have to do this at write time.

I'm trying to think if there is any reason we can't delay scanVersionScript() till after LTO code generation. If we could do that then we'd know which symbols were lazy and we wouldn't need to assign them a versionid. I don't know off the top of my head how much work it would be to check that though.

If scanVersionScript() is before LTO code generation, these tests will fail.

Failed Tests (5):
  lld :: ELF/lto/devirt_vcall_vis_export_dynamic.ll
  lld :: ELF/lto/devirt_vcall_vis_localize.ll
  lld :: ELF/lto/dynamic-list.ll
  lld :: ELF/lto/internalize-version-script.ll
  lld :: ELF/lto/version-libcall.ll

We handle dynamic lists in scanVersionScript, so the includeInDynsym and inDynamicList properties can be incorrect in LTO.cpp BitcodeCompiler::add.
A bit recently, I added sym->computeBinding() != STB_LOCAL in D98220 so the whole program devirtualization pass will lose information as well.

So we need a scanVersionScript before compileBitcodeFiles. If we simply call scanVersionScript another time after compileBitcodeFiles, we will get error: attempt to reassign symbol 'foo' of version 'VER2' to version 'VER1' from version-script-symver2.s

[MaskRay]

Add an inline comment

lld/ELF/SyntheticSections.cpp

 
 size_t VersionTableSection::getSize() const {
   return (getPartition().dynSymTab->getSymbols().size() + 1) * 2;
 }
 
 void VersionTableSection::writeTo(uint8_t *buf) {
   buf += 2;
   for (const SymbolTableEntry &s : getPartition().dynSymTab->getSymbols()) {
-    write16(buf, s.sym->versionId);
+    // Use the original versionId for an unfetched lazy symbol (undefined weak),
+    // which must be VER_NDX_GLOBAL (an undefined versioned symbol is an error).
+    write16(buf, s.sym->isLazy() ? VER_NDX_GLOBAL : s.sym->versionId);
     buf += 2;
   }
 }
 
 bool VersionTableSection::isNeeded() const {
   return isLive() &&
          (getPartition().verDef || getPartition().verNeed->isNeeded());
 }

lld/test/ELF/version-script-weak.s

 # CHECK-NEXT:   Value: 0x0
 # CHECK-NEXT:   Size: 0
 # CHECK-NEXT:   Binding: Weak
 # CHECK-NEXT:   Type: None
 # CHECK-NEXT:   Other: 0
 # CHECK-NEXT:   Section: Undefined
 # CHECK-NEXT: }
 
+## The version of an unfetched lazy symbol is VER_NDX_GLOBAL. It is not affected by version scripts.
+# RUN: echo "v1 { *; };" > %t2.script
+# RUN: ld.lld -shared --version-script %t2.script %t.o --start-lib %t1.o --end-lib -o %t2.so
+# RUN: llvm-readelf --dyn-syms %t2.so | FileCheck %s --check-prefix=CHECK2
+
+# CHECK2: NOTYPE WEAK DEFAULT UND foo{{$}}
+
+# RUN: ld.lld -shared --soname=tshared --version-script %t2.script %t1.o -o %tshared.so
+# RUN: ld.lld -shared --version-script %t2.script %t.o --start-lib %t1.o --end-lib %tshared.so -o %t3.so
+# RUN: llvm-readelf --dyn-syms %t3.so | FileCheck %s --check-prefix=CHECK3
+
+# CHECK3: NOTYPE WEAK DEFAULT UND foo@v1
+
 .text
  callq foo@PLT
 .weak foo