[ELF] - Symbols from object files that override symbols in DSO are added to .dynsym table.

[grimar]

Summary

Main executable did not export symbols that exist both in the main executable and in DSOs before this patch.
Symbols from object files that override symbols in DSO should be added to .dynsym table.

This fixes https://llvm.org/bugs/show_bug.cgi?id=26222

[grimar]

• Updated failing test for mips

[grimar]

Simon, could you please look at mips test part of the patch ? I just not sure is it fine to fix the mips test or it means changes broke something.

[ruiu]

ELF/Symbols.cpp

66

Let's do this here for consistency.

// We want to export all symbols that exist both in the executable and in DSOs
// so that the symbols in the executable can interrupt symbols in the DSO at runtime.
if (isShared() != Other->isShared())
  if (!isa<DefinedRegular<ELFT>>(isShared() ? this : Other))
    IsUsedInDynamicReloc = Other->IsUsedInDynamicReloc = true;

By the way, is this correct to have this inside if (IsUsedInRegularObj || Other->IsUsedInRegularObj) {}?

[grimar]

Addressed review comments.

ELF/Symbols.cpp

66

I think it does not make sence here. SymbolBody::compare() does not accepts lazy symbols and I also call isa<DefinedRegular<ELFT>() check.
So I think this check only works for me like check that at least one symbol is not shared. Since I have direct check for that below, there is no much sence to keep that inside.
I moved logic outside for clarity.

[ruiu]

LGTM

ELF/Symbols.cpp

69

Let's add a comment.

// We want to export all symbols that exist both in the executable
// and in DSOs, so that the symbols in the executable can interrupt
// symbols in the DSO at runtime.

ELF/Symbols.cpp

 static uint8_t getMinVisibility(uint8_t VA, uint8_t VB) {
   if (VA == STV_DEFAULT)
     return VB;
   if (VB == STV_DEFAULT)
     return VA;
   return std::min(VA, VB);
 }
 
+// We want to export all symbols that exist both in the executable and in DSOs.
+template <class ELFT>
+static void checkDynamicSymbolOverride(SymbolBody *L, SymbolBody *R) {
+  SymbolBody *O = L->isShared() ? R : L;
+  if (!isa<DefinedRegular<ELFT>>(O))
+    return;
+  L->setUsedInDynamicReloc();
+  R->setUsedInDynamicReloc();
+}
+
 // Returns 1, 0 or -1 if this symbol should take precedence
 // over the Other, tie or lose, respectively.
 template <class ELFT> int SymbolBody::compare(SymbolBody *Other) {
   typedef typename ELFFile<ELFT>::uintX_t uintX_t;
   assert(!isLazy() && !Other->isLazy());
   std::tuple<bool, bool, bool> L(isDefined(), !isShared(), !isWeak());
   std::tuple<bool, bool, bool> R(Other->isDefined(), !Other->isShared(),
                                  !Other->isWeak());
 
   // Normalize
   if (L > R)
     return -Other->compare<ELFT>(this);
 
   Visibility = Other->Visibility =
       getMinVisibility(Visibility, Other->Visibility);
 
-  if (IsUsedInRegularObj || Other->IsUsedInRegularObj)
+  if (IsUsedInRegularObj || Other->IsUsedInRegularObj) {
     IsUsedInRegularObj = Other->IsUsedInRegularObj = true;
+    if (isShared() != Other->isShared())
+      checkDynamicSymbolOverride<ELFT>(this, Other);
+  }
 
   if (L != R)
     return -1;
   if (!std::get<0>(L) || !std::get<1>(L) || !std::get<2>(L))
     return 1;
   if (isCommon()) {
     if (!Other->isCommon())
       return -1;
     auto *ThisC = cast<DefinedCommon>(this);
     auto *OtherC = cast<DefinedCommon>(Other);

test/ELF/Inputs/symbol-override.s

+.text
+.globl foo
+.type foo,@function
+foo:
+nop
+
+.globl bar
+.type bar,@function
+bar:
+nop
+
+.globl do
+.type do,@function
+do: 
+callq foo@PLT
+callq bar@PLT

test/ELF/mips-dynamic.s

 # EXE:      ]
 # EXE:      DynamicSection [
 # EXE-NEXT:   Tag        Type                 Name/Value
 # EXE-DAG:    0x00000003 PLTGOT               [[GOTADDR]]
 # EXE-DAG:    0x70000001 MIPS_RLD_VERSION     1
 # EXE-DAG:    0x70000005 MIPS_FLAGS           NOTPOT
 # EXE-DAG:    0x70000006 MIPS_BASE_ADDRESS
 # EXE-DAG:    0x7000000A MIPS_LOCAL_GOTNO     2
-# EXE-DAG:    0x70000011 MIPS_SYMTABNO        1
-# EXE-DAG:    0x70000013 MIPS_GOTSYM          0x1
+# EXE-DAG:    0x70000011 MIPS_SYMTABNO        2
+# EXE-DAG:    0x70000013 MIPS_GOTSYM          0x2
 # EXE-DAG:    0x70000016 MIPS_RLD_MAP         [[RLDMAPADDR]]
 # EXE:      ]
 
 # DSO:      Sections [
 # DSO:          Name: .dynamic
 # DSO-NEXT:     Type: SHT_DYNAMIC
 # DSO-NEXT:     Flags [
 # DSO-NEXT:       SHF_ALLOC

test/ELF/symbol-override.s

+// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t1.o
+// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/symbol-override.s -o %t2.o
+// RUN: ld.lld -shared %t2.o -o %t2.so
+// RUN: ld.lld %t1.o %t2.so -o %t
+// RUN: llvm-readobj -dyn-symbols %t | FileCheck %s
+
+// CHECK:      DynamicSymbols [
+// CHECK-NEXT:   Symbol {
+// CHECK-NEXT:     Name:
+// CHECK-NEXT:     Value: 0x0
+// CHECK-NEXT:     Size: 0
+// CHECK-NEXT:     Binding: Local
+// CHECK-NEXT:     Type: None
+// CHECK-NEXT:     Other: 0
+// CHECK-NEXT:     Section: Undefined
+// CHECK-NEXT:   }
+// CHECK-NEXT:   Symbol {
+// CHECK-NEXT:     Name: do
+// CHECK-NEXT:     Value: 0x0
+// CHECK-NEXT:     Size: 0
+// CHECK-NEXT:     Binding: Global
+// CHECK-NEXT:     Type: Function
+// CHECK-NEXT:     Other: 0
+// CHECK-NEXT:     Section: Undefined
+// CHECK-NEXT:   }
+// CHECK-NEXT:   Symbol {
+// CHECK-NEXT:     Name: foo
+// CHECK-NEXT:     Value: 0x11000
+// CHECK-NEXT:     Size: 0
+// CHECK-NEXT:     Binding: Global
+// CHECK-NEXT:     Type: Function
+// CHECK-NEXT:     Other: 0
+// CHECK-NEXT:     Section: .text
+// CHECK-NEXT:   }
+// CHECK-NEXT: ]
+
+.text
+.globl foo
+.type foo,@function
+foo:  
+nop
+
+.text
+.globl _start
+_start:
+callq do