Store the real binding of shared symbols

[• rafael]

Summary

Currently we mark every shared symbol as STB_WEAK.

That is a hack to make it easy to decide when a .so is needed or not because of a reference to a given symbol.

That hack leaks when we create copy relocations as shown by the update to relocation-copy-alias.s.

This patch stores the original binding when we first read a shared symbol. We still have to update the binding to weak if we see a weak undef, but I find the logic easier to read where it is now.

[ruiu]

LGTM

[• espindola]

319127

ELF/SymbolTable.cpp

Symbol *SymbolTable::addUndefined(StringRef Name, uint8_t Binding,
   std::tie(S, WasInserted) =
       insert(Name, Type, Visibility, CanOmitFromDynSym, File);
   // An undefined symbol with non default visibility must be satisfied
   // in the same DSO.
   if (WasInserted || (isa<SharedSymbol>(S) && Visibility != STV_DEFAULT)) {
     replaceSymbol<Undefined>(S, File, Name, Binding, StOther, Type);
     return S;
   }
-  if (Binding != STB_WEAK) {
-    if (!S->isDefined())
-      S->Binding = Binding;
+  if (S->isShared() || S->isLazy() || (S->isUndefined() && Binding != STB_WEAK))
+  if (Binding != STB_WEAK) {
     if (auto *SS = dyn_cast<SharedSymbol>(S))
       if (!Config->GcSections)
         SS->getFile<ELFT>()->IsNeeded = true;
   }
   if (auto *L = dyn_cast<Lazy>(S)) {
     // An undefined weak will not fetch archive members. See comment on Lazy in
     // Symbols.h for the details.
-    if (Binding == STB_WEAK) {
+    if (Binding == STB_WEAK)
       L->Type = Type;
-      L->Binding = STB_WEAK;
-    } else if (InputFile *F = L->fetch()) {
+    else if (InputFile *F = L->fetch())
       addFile<ELFT>(F);
-    }
-  }
   return S;
 }
 
 // Using .symver foo,foo@@VER unfortunately creates two symbols: foo and
 // foo@@VER. We want to effectively ignore foo, so give precedence to
 // foo@@VER.
 // FIXME: If users can transition to using
 // .symver foo,foo@@@VER
void SymbolTable::addShared(StringRef Name, SharedFile<ELFT> *File,
   if (Sym.getVisibility() == STV_DEFAULT)
     S->ExportDynamic = true;
 
   // An undefined symbol with non default visibility must be satisfied
   // in the same DSO.
   if (WasInserted || ((S->isUndefined() || S->isLazy()) &&
                       S->getVisibility() == STV_DEFAULT)) {
     uint8_t Binding = S->Binding;
-    replaceSymbol<SharedSymbol>(S, File, Name, Sym.st_other, Sym.getType(),
-                                Sym.st_value, Sym.st_size, Alignment, Verdef);
+    replaceSymbol<SharedSymbol>(S, File, Name, Sym.getBinding(), Sym.st_other,
+                                Sym.getType(), Sym.st_value, Sym.st_size,
+                                Alignment, Verdef);
     if (!WasInserted) {
       S->Binding = Binding;
       if (!S->isWeak() && !Config->GcSections)
         File->IsNeeded = true;
     }
   }
 }
 

ELF/Symbols.h

public:
 
   static bool classof(const Symbol *S) { return S->kind() == UndefinedKind; }
 };
 
 class SharedSymbol : public Symbol {
 public:
   static bool classof(const Symbol *S) { return S->kind() == SharedKind; }
 
-  SharedSymbol(StringRef Name, uint8_t StOther, uint8_t Type, uint64_t Value,
-               uint64_t Size, uint32_t Alignment, const void *Verdef)
-      : Symbol(SharedKind, Name, llvm::ELF::STB_WEAK, StOther, Type),
-        Verdef(Verdef), Value(Value), Size(Size), Alignment(Alignment) {
+  SharedSymbol(StringRef Name, uint8_t Binding, uint8_t StOther, uint8_t Type,
+               uint64_t Value, uint64_t Size, uint32_t Alignment,
+               const void *Verdef)
+      : Symbol(SharedKind, Name, Binding, StOther, Type), Verdef(Verdef),
+        Value(Value), Size(Size), Alignment(Alignment) {
     // GNU ifunc is a mechanism to allow user-supplied functions to
     // resolve PLT slot values at load-time. This is contrary to the
     // regualr symbol resolution scheme in which symbols are resolved just
     // by name. Using this hook, you can program how symbols are solved
     // for you program. For example, you can make "memcpy" to be resolved
     // to a SSE-enabled version of memcpy only when a machine running the
     // program supports the SSE instruction set.
     //

test/ELF/relocation-copy-alias.s

 // CHECK-NEXT: Binding: Weak
 // CHECK-NEXT: Type: Object (0x1)
 // CHECK-NEXT: Other: 0
 // CHECK-NEXT: Section: .bss
 
 // CHECK:      Name: b3
 // CHECK-NEXT: Value: [[B]]
 // CHECK-NEXT: Size: 1
-// CHECK-NEXT: Binding: Weak
+// CHECK-NEXT: Binding: Global
 // CHECK-NEXT: Type: Object (0x1)
 // CHECK-NEXT: Other: 0
 // CHECK-NEXT: Section: .bss