[PATCH] D63003: [ELF] Don't emit dynamic relocations with weak undef in writable sections

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: grimar, pcc, ruiu.
Herald added subscribers: llvm-commits, arichardson, emaste.
Herald added a reviewer: espindola.
Herald added a project: LLVM.

In processRelocAux(), our handling of 1) link-time constant and 2) weak
undef is the same, so put them together to simplify the logic.

This moves the weak undef code around. The result is that we will not
emit dynamic relocations for initial relocations to weak undefined
symbols. The condition `!Config->Shared` was there probably because it
is common for a -shared link not to specify full dependencies. Keep it
now but we may revisit the decision in the future.

gABI says:

> The behavior of weak symbols in areas not specified by this document is
>  implementation defined. Weak symbols are intended primarily for use in
>  system software. Applications using weak symbols are unreliable since
>  changes in the runtime environment might cause the execution to fail.

With this change, we can simplify another place
`Symbol::includeInDynsym()` (see D59549 <https://reviews.llvm.org/D59549>) a bit.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D63003

Files:
  ELF/Relocations.cpp
  test/ELF/weak-undef-rw.s


Index: test/ELF/weak-undef-rw.s
===================================================================
--- test/ELF/weak-undef-rw.s
+++ test/ELF/weak-undef-rw.s
@@ -1,9 +1,13 @@
 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 # RUN: ld.lld %t.o -o %t --export-dynamic
-# RUN: llvm-readobj -r %t | FileCheck %s
+# RUN: llvm-readelf -r %t | FileCheck %s
 
-# CHECK: R_X86_64_64 foobar 0x0
+## gABI leaves the behavior of weak undefined references implementation defined.
+## We choose to resolve it statically and not create a dynamic relocation for
+## implementation simplicity.
+
+# CHECK: no relocations
 
         .global _start
 _start:
Index: ELF/Relocations.cpp
===================================================================
--- ELF/Relocations.cpp
+++ ELF/Relocations.cpp
@@ -911,10 +911,19 @@
 static void processRelocAux(InputSectionBase &Sec, RelExpr Expr, RelType Type,
                             uint64_t Offset, Symbol &Sym, const RelTy &Rel,
                             int64_t Addend) {
-  if (isStaticLinkTimeConstant(Expr, Type, Sym, Sec, Offset)) {
+  // If the relocation is known to be a link-time constant, we know no dynamic
+  // relocation will be created, pass the control to relocateAlloc or
+  // relocateNonAlloc to resolve it.
+  //
+  // The behavior of an undefined weak reference is implementation defined. If
+  // the relocation is to a weak undef, and we are producing executable, give up
+  // on it and let relocate{,Non}Alloc resolve it.
+  if (isStaticLinkTimeConstant(Expr, Type, Sym, Sec, Offset) ||
+      (!Config->Shared && Sym.isUndefWeak())) {
     Sec.Relocations.push_back({Expr, Type, Offset, Addend, &Sym});
     return;
   }
+
   bool CanWrite = (Sec.Flags & SHF_WRITE) || !Config->ZText;
   if (CanWrite) {
     // R_GOT refers to a position in the got, even if the symbol is preemptible.
@@ -947,13 +956,6 @@
     }
   }
 
-  // If the relocation is to a weak undef, and we are producing
-  // executable, give up on it and produce a non preemptible 0.
-  if (!Config->Shared && Sym.isUndefWeak()) {
-    Sec.Relocations.push_back({Expr, Type, Offset, Addend, &Sym});
-    return;
-  }
-
   if (!CanWrite && (Config->Pic && !isRelExpr(Expr))) {
     error(
         "can't create dynamic relocation " + toString(Type) + " against " +


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D63003.203528.patch
Type: text/x-patch
Size: 2327 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190607/37ff286a/attachment.bin>