Do not add weak undefined symbols to .dynsym unless -pic or -shared are given.

[ruiu]

Summary

Do not add weak undefined symbols to .dynsym unless -pic or -shared are given.

[ruiu]

• Fix typos.

[ruiu]

• Rebased

lld/ELF/Relocations.cpp

static RelExpr adjustExpr(Symbol &Sym, RelExpr Expr, RelType Type,
   if (isStaticLinkTimeConstant<ELFT>(Expr, Type, Sym, S, RelOff))
     return Expr;
 
   // If we got here we know that this relocation would require the dynamic
   // linker to write a value to read only memory.
 
   // If the relocation is to a weak undef, give up on it and produce a
   // non preemptible 0.
-  if (Sym.isUndefWeak()) {
-    Sym.IsPreemptible = false;
+  if (Sym.isUndefWeak())
     return Expr;
-  }
 
   // We can hack around it if we are producing an executable and
   // the refered symbol can be preemepted to refer to the executable.
   if (Config->Shared || (Config->Pic && !isRelExpr(Expr))) {
     error(
         "can't create dynamic relocation " + toString(Type) + " against " +
         (Sym.getName().empty() ? "local symbol" : "symbol: " + toString(Sym)) +
         " in readonly segment; recompile object files with -fPIC" +

lld/ELF/Symbols.cpp

if (Visibility != STV_DEFAULT && Visibility != STV_PROTECTED)
     return STB_LOCAL;
   if (VersionId == VER_NDX_LOCAL && isInCurrentOutput())
     return STB_LOCAL;
   if (Config->NoGnuUnique && Binding == STB_GNU_UNIQUE)
     return STB_GLOBAL;
   return Binding;
 }
 
+// Returns true if this symbol should be added to .dynsym.
 bool Symbol::includeInDynsym() const {
   if (!Config->HasDynSymTab)
     return false;
   if (computeBinding() == STB_LOCAL)
     return false;
+
+  // Exported weak symbols are not representable unless they are resolved
+  // through GOT. We simply export weak symbols only when -shared or -pie
+  // were given.
+  //
+  // As a result, if an weak symbol cannot be resolved within the current
+  // output file, and if no -shared nor -pie were given, the symbol is
+  // resolved to zero at link-time. If you want to give it a second chance
+  // to be resolved at load-time, you need to pass -shared or -pie.
+  //
+  // So, why are they not representable? To understand that, assume the
+  // following code:
+  //
+  //   __attribute__((weak)) int foo();
+  //   void bar() { if (foo) foo(); }
+  //
+  // If this code is compiled without -fPIC, function pointer foo in the
+  // "if" is compiled to a direct reference to a function. The linker would
+  // create a PLT entry for foo and use its address as a function pointer
+  // value for foo. It would usually work, because when you jump to foo's
+  // PLT, it in turn jumps to foo's real function definition.
+  //
+  // However, that mechanism can result in a bad combination of pointer
+  // values if the symbol is weak. Symbol foo's function pointer value is
+  // always non-zero because it is resolved to a linker-synthesized foo's
+  // PLT entry (that means the "if" condition is always true). But, if the
+  // loader cannot resolve foo at load-time, it leaves foo's PLT entry zero.
+  // That means when you jump there, it then jumps to an unexpected
+  // address, and the program crashes. As a result, if foo cannot be
+  // resolved at load-time, the program crashes. This is apparently what
+  // users would not expect.
+  //
+  // If all references to symbol foo go through GOT, weak symbols are
+  // representable. If the loader cannot resolve foo, it leaves foo's GOT
+  // entry zero, and the "if" condition simply becomes false.
+  //
+  // So, we want to export weak symbols only when we know they are
+  // referenced through GOT.
+  //
+  // (The decision we are making here is not perfect; even if -shared or
+  // -pie are given to the linker, input object files may have direct
+  // references to weak symbols. We'll report them as errors in
+  // scanRelocations. Likewise, even if -shared nor -pie were not given,
+  // all relocations to weak symbols could go through GOT. Even so, we
+  // won't export them, because it is hard to detect it ahead of time. It
+  // is also generally preferable to let users explicitly control the
+  // linker's behavior via command line options rather than changing
+  // the behavior implicitly depending on existence or absense of some
+  // relocations.)
+  if (!Config->Pic && isUndefWeak())
+    return false;
+
   if (!isInCurrentOutput())
     return true;
   return ExportDynamic;
 }
 
 // Print out a log message for --trace-symbol.
 void elf::printTraceSymbol(Symbol *Sym) {
   std::string S;

lld/test/ELF/weak-undef-export.s

 # REQUIRES: x86
 
-# Test that we don't fail with foo being undefined.
+# Test that we resolve foo to zero at link-time.
 
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 # RUN: ld.lld --export-dynamic %t.o -o %t
-# RUN: llvm-readobj -dyn-symbols %t | FileCheck %s
 
-# CHECK:      DynamicSymbols [
-# CHECK-NEXT:   Symbol {
-# CHECK-NEXT:     Name: @ (0)
-# CHECK-NEXT:     Value: 0x0
-# CHECK-NEXT:     Size: 0
-# CHECK-NEXT:     Binding: Local (0x0)
-# CHECK-NEXT:     Type: None (0x0)
-# CHECK-NEXT:     Other: 0
-# CHECK-NEXT:     Section: Undefined (0x0)
-# CHECK-NEXT:   }
-# CHECK-NEXT:   Symbol {
-# CHECK-NEXT:     Name: foo@ (1)
-# CHECK-NEXT:     Value: 0x0
-# CHECK-NEXT:     Size: 0
-# CHECK-NEXT:     Binding: Weak (0x2)
-# CHECK-NEXT:     Type: None (0x0)
-# CHECK-NEXT:     Other: 0
-# CHECK-NEXT:     Section: Undefined (0x0)
-# CHECK-NEXT:  }
-# CHECK-NEXT: ]
+# RUN: llvm-readobj -symbols %t | FileCheck -check-prefix=SYM %s
+# SYM:      Name: foo
+# SYM-NEXT: Value: 0x0
+
+# RUN: llvm-readobj -dyn-symbols %t | FileCheck -check-prefix=DYNSYM %s
+# DYNSYM-NOT: foo
 
-        .weak foo
-        .quad foo

lld/test/ELF/weak-undef-pic-nopic.s

+# REQUIRES: x86
+# RUN: echo | llvm-mc -filetype=obj -triple=x86_64-pc-linux -o %t1.o -
+# RUN: ld.lld -shared -o %t2.so %t1.o
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux -position-independent %s -o %t2.o
+# RUN: ld.lld -pie -o %t2.exe %t2.o %t2.so
+# RUN: llvm-readobj -dyn-symbols %t2.exe | FileCheck -check-prefix=PIC %s
+
+# RUN: ld.lld -o %t3.exe %t2.o %t2.so
+# RUN: llvm-readobj -dyn-symbols %t3.exe | FileCheck -check-prefix=NOPIC %s
+
+# PIC:      Name: foo
+# PIC-NEXT: Value: 0x0
+# PIC-NEXT: Size: 0
+# PIC-NEXT: Binding: Weak
+# PIC-NEXT: Type: None
+# PIC-NEXT: Other: 0
+# PIC-NEXT: Section: Undefined
+
+# NOPIC-NOT: Name: foo
+
+.weak foo