[ELF] - Do not change binding of symbols when creating relocatable output.

[grimar]

Summary

Spec says "A hidden symbol contained in a relocatable object must be either
removed or converted to STB_LOCAL binding by the link-editor when the
relocatable object is included in an executable file or shared object".
But we previously converted symbols to STB_LOCAL even when -r was specified.

Broken binary was produced, this is PR28967, patch fixes the issue.

[ruiu]

ELF/OutputSections.cpp

1321–1322

I'm not sure if this would fix the problem entirely. My gut is that we want to keep the original symbol type if -r is specified. If that's the case, we want to add

if (Config->Relocatable)
  return S->Binding;

here. But I don't know if that's correct.

[grimar]

ELF/OutputSections.cpp

1321–1322

gold allows GNU_UNIQUE->STB_GLOBAL transformation for relocatable output,
so I tried to be consistent with it here.
That is the only difference with your suggestion I think. And looks gnu ld does not support
--no-gnu-unique at all, so probably we can go with any solution here.

[ruiu]

I don't think that the current gold behavior you described was intended one. I cannot describe why they are doing it. On the other hand, the semantics that "all symbols are copied as-is to the output if -r is given" is very easy to understand and justify. I believe we should do that way.

[grimar]

• Do not change binding of symbols when creating relocatable output.

[ruiu]

LGTM!

ELF/OutputSections.cpp

static bool sortMipsSymbols(const std::pair<SymbolBody *, unsigned> &L,
   bool RIsInLocalGot = !R.first->IsInGlobalMipsGot;
   if (LIsInLocalGot || RIsInLocalGot)
     return !RIsInLocalGot;
   return L.first->GotIndex < R.first->GotIndex;
 }
 
 static uint8_t getSymbolBinding(SymbolBody *Body) {
   Symbol *S = Body->symbol();
+  if (Config->Relocatable)
+    return S->Binding;
   uint8_t Visibility = S->Visibility;
   if (Visibility != STV_DEFAULT && Visibility != STV_PROTECTED)
     return STB_LOCAL;
   if (Config->NoGnuUnique && S->Binding == STB_GNU_UNIQUE)
     return STB_GLOBAL;
   return S->Binding;
 }
 
 template <class ELFT> void SymbolTableSection<ELFT>::finalize() {
   if (this->Header.sh_size)

test/ELF/relocatable-visibility.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: ld.lld -r %t.o -o %t1
+# RUN: llvm-readobj -t %t1 | FileCheck --check-prefix=RELOCATABLE %s
+
+# RELOCATABLE:      Name: foo
+# RELOCATABLE-NEXT: Value: 0x0
+# RELOCATABLE-NEXT: Size: 0
+# RELOCATABLE-NEXT: Binding: Global
+# RELOCATABLE-NEXT: Type: None
+# RELOCATABLE-NEXT: Other [
+# RELOCATABLE-NEXT:   STV_HIDDEN
+# RELOCATABLE-NEXT: ]
+# RELOCATABLE-NEXT: Section: Undefined
+
+.global _start
+_start:
+ callq foo
+ .hidden foo