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

[George Rimar via Phabricator via llvm-commits]

grimar created this revision.
grimar added a reviewer: ruiu.
Herald added subscribers: arichardson, emaste.
Herald added a reviewer: espindola.

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.

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 https://reviews.llvm.org/rL325569 (https://reviews.llvm.org/D43383).


https://reviews.llvm.org/D52724

Files:
  ELF/Symbols.cpp
  test/ELF/local-ver-preemptible.s


Index: test/ELF/local-ver-preemptible.s
===================================================================
--- test/ELF/local-ver-preemptible.s
+++ test/ELF/local-ver-preemptible.s
@@ -0,0 +1,67 @@
+# 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: yaml2obj < %s > %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@
+
+--- !ELF
+FileHeader:      
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_X86_64
+Sections:        
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x0000000000000001
+    Content:         E800000000
+  - Name:            .rela.text
+    Type:            SHT_RELA
+    Flags:           [ SHF_INFO_LINK ]
+    Link:            .symtab
+    AddressAlign:    0x0000000000000008
+    Info:            .text
+    Relocations:     
+      - Offset:          0x0000000000000001
+        Symbol:          foo
+        Type:            R_X86_64_PC32
+        Addend:          -4
+  - Name:            .data
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_WRITE, SHF_ALLOC ]
+    AddressAlign:    0x0000000000000001
+    Content:         ''
+  - Name:            .bss
+    Type:            SHT_NOBITS
+    Flags:           [ SHF_WRITE, SHF_ALLOC ]
+    AddressAlign:    0x0000000000000001
+Symbols:         
+  Local:           
+    - Name:            .text
+      Type:            STT_SECTION
+      Section:         .text
+    - Name:            .data
+      Type:            STT_SECTION
+      Section:         .data
+    - Name:            .bss
+      Type:            STT_SECTION
+      Section:         .bss
+    - Name:            _start
+      Section:         .text
+  Global:          
+    - Name:            foo
+DynamicSymbols:  
+...
Index: ELF/Symbols.cpp
===================================================================
--- ELF/Symbols.cpp
+++ ELF/Symbols.cpp
@@ -225,7 +225,7 @@
     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;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D52724.167732.patch
Type: text/x-patch
Size: 3015 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181001/c24fa04d/attachment.bin>