[ELF] - Do not forget to include to .dymsym symbols that were converted to Defined.

[grimar]

Summary

This is the fix for
"Bug 39104 - LLD links incorrect ELF executable if version script contains "local: *;" (https://bugs.llvm.org/show_bug.cgi?id=39104).

The issue happens when we have non-PIC program call to function in a shared library.
(for example, the PR above has R_X86_64_PC32 relocation against __libc_start_main)

LLD converts symbol to Defined in that case with the use of replaceWithDefined()

The issue is that after above we create a broken relocation because do not
include the symbol into .dynsym:

location section '.rela.plt' at offset 0x2a0 contains 1 entries:
   Offset          Info           Type           Sym. Value    Sym. Name + Addend
000000202018  000000000007 R_X86_64_JUMP_SLO                    0

Symbol table '.dynsym' contains 2 entries:
  Num:    Value          Size Type    Bind   Vis      Ndx Name
    0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND 
    1: 0000000000201000     0 NOTYPE  GLOBAL DEFAULT    7 main

That happens when the version script is used because we treat the symbol as
STB_LOCAL if the following condition match:
VersionId == VER_NDX_LOCAL && isDefined() and do not include it to
.dynsym because of that.

The patch shows the change I am suggesting.

[ruiu]

test/ELF/local-ver-preemptible.s

1 ↗

(On Diff #167732)

Can you write this in assembly?

[smeenai]

test/ELF/local-ver-preemptible.s

1 ↗

(On Diff #167732)

I think that's addressed in the summary?

Note that I had to use YAML input, because llvm-mc does not produce R_X86_64_PC32 for call foo and jmp foo since rL325569 (https://reviews.llvm.org/D43383).

[ruiu]

test/ELF/local-ver-preemptible.s

1 ↗

(On Diff #167732)

Yeah, but since I could reproduce https://bugs.llvm.org/show_bug.cgi?id=39104 without a YAML file, I believe you can do it without YAML.

[grimar]

• Stop using YAML for the test.

[ruiu]

LGTM

[orivej]

Thank you! This patch works as intended.

It seems a bit undesirable that the test generates a warning cannot find entry symbol _start; defaulting to 0x201000, and that it expects a possibly unstable number 0x288, but maybe it's OK.

[grimar]

In D52724#1253355, @orivej wrote:

Thank you! This patch works as intended.

It seems a bit undesirable that the test generates a warning cannot find entry symbol _start; defaulting to 0x201000, and that it expects a possibly unstable number 0x288, but maybe it's OK.

I'll add the _start symbol before commit. Generally, we have test cases which adds it to avoid the warning, but also have tests without. Personally, I have no preference here.

lld/trunk/ELF/Symbols.cpp

return CHECK(C.getMemoryBufferRef(),
                    Sym.getName());
 }
 
 uint8_t Symbol::computeBinding() const {
   if (Config->Relocatable)
     return Binding;
   if (Visibility != STV_DEFAULT && Visibility != STV_PROTECTED)
     return STB_LOCAL;
-  if (VersionId == VER_NDX_LOCAL && isDefined())
+  if (VersionId == VER_NDX_LOCAL && isDefined() && !IsPreemptible)
     return STB_LOCAL;
   if (!Config->GnuUnique && Binding == STB_GNU_UNIQUE)
     return STB_GLOBAL;
   return Binding;
 }
 
 bool Symbol::includeInDynsym() const {
   if (!Config->HasDynSymTab)

lld/trunk/test/ELF/local-ver-preemptible.s

+# REQUIRES: x86
+# RUN: echo '.global foo; .type foo, @function; foo:' | \
+# RUN:   llvm-mc -filetype=obj -triple=x86_64-unknown-linux - -o %t.so.o
+# RUN: ld.lld %t.so.o -o %t.so -shared
+
+# RUN: echo "{ global: main; local: *; };" > %t.script
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: ld.lld %t.o %t.so -o %t -version-script %t.script
+# RUN: llvm-readelf -r --symbols %t | FileCheck %s
+
+# CHECK:      Relocation section '.rela.plt' at offset 0x288 contains 1 entries:
+# CHECK:        R_X86_64_JUMP_SLOT 0000000000201020 foo + 0
+
+# CHECK:      Symbol table '.dynsym' contains 2 entries:
+# CHECK-NEXT:   Num:    Value          Size Type    Bind   Vis      Ndx Name
+# CHECK-NEXT:     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND @
+# CHECK-NEXT:     1: 0000000000201020     0 FUNC    GLOBAL DEFAULT  UND foo@
+
+_start:
+  movl $foo - ., %eax