[lld] 5bab291 - Ignore R_MIPS_JALR relocations against non-function symbols

[Alex Richardson via llvm-commits]

Author: Alex Richardson
Date: 2019-11-20T13:23:26Z
New Revision: 5bab291b7bd043104abf1ca7977e8248684cae95

URL: https://github.com/llvm/llvm-project/commit/5bab291b7bd043104abf1ca7977e8248684cae95
DIFF: https://github.com/llvm/llvm-project/commit/5bab291b7bd043104abf1ca7977e8248684cae95.diff

LOG: Ignore R_MIPS_JALR relocations against non-function symbols

Summary:
Current versions of clang would erroneously emit this relocation not only
against functions (loaded from the GOT) but also against data symbols
(e.g. a table of function pointers). LLD was then changing this into a
branch-and-link instruction, causing the program to jump to the data
symbol at run time. I discovered this problem when attempting to boot
MIPS64 FreeBSD after updating the to the latest upstream master.

Reviewers: atanasyan, jrtc27, espindola

Reviewed By: atanasyan

Subscribers: emaste, sdardis, krytarowski, MaskRay, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D70406

Added: 
    lld/test/ELF/mips-jalr-non-functions.s

Modified: 
    lld/ELF/Arch/Mips.cpp

Removed: 
    


################################################################################
diff  --git a/lld/ELF/Arch/Mips.cpp b/lld/ELF/Arch/Mips.cpp
index d8fa306a6205..74c0b59ecd5b 100644
--- a/lld/ELF/Arch/Mips.cpp
+++ b/lld/ELF/Arch/Mips.cpp
@@ -83,6 +83,17 @@ RelExpr MIPS<ELFT>::getRelExpr(RelType type, const Symbol &s,
 
   switch (type) {
   case R_MIPS_JALR:
+    // Older versions of clang would erroneously emit this relocation not only
+    // against functions (loaded from the GOT) but also against data symbols
+    // (e.g. a table of function pointers). When we encounter this, ignore the
+    // relocation and emit a warning instead.
+    if (!s.isFunc() && s.type != STT_NOTYPE) {
+      warn(getErrorLocation(loc) +
+           "found R_MIPS_JALR relocation against non-function symbol " +
+           toString(s) + ". This is invalid and most likely a compiler bug.");
+      return R_NONE;
+    }
+
     // If the target symbol is not preemptible and is not microMIPS,
     // it might be possible to replace jalr/jr instruction by bal/b.
     // It depends on the target symbol's offset.

diff  --git a/lld/test/ELF/mips-jalr-non-functions.s b/lld/test/ELF/mips-jalr-non-functions.s
new file mode 100644
index 000000000000..410b742c3e6f
--- /dev/null
+++ b/lld/test/ELF/mips-jalr-non-functions.s
@@ -0,0 +1,53 @@
+# REQUIRES: mips
+## Check that we ignore R_MIPS_JALR relocations agains non-function symbols.
+## Older versions of clang were erroneously generating them for function pointers
+## loaded from any table (not just the GOT) as well as against TLS function
+## pointers (when using the local-dynamic model), so we need to ignore these
+## relocations to avoid generating binaries that crash when executed.
+
+# RUN: llvm-mc -filetype=obj -triple=mips64-unknown-linux %s -o %t.o
+# RUN: ld.lld -shared %t.o -o %t.so 2>&1 | FileCheck %s -check-prefix WARNING-MESSAGE
+# RUN: llvm-objdump --no-show-raw-insn --no-leading-addr -d %t.so | FileCheck %s
+
+.set	noreorder
+test:
+  .reloc .Ltmp1, R_MIPS_JALR, tls_obj
+.Ltmp1:
+  jr  $t9
+  nop
+# WARNING-MESSAGE: warning: found R_MIPS_JALR relocation against non-function symbol tls_obj. This is invalid and most likely a compiler bug.
+
+  .reloc .Ltmp2, R_MIPS_JALR, reg_obj
+.Ltmp2:
+  jr  $t9
+  nop
+# WARNING-MESSAGE: warning: found R_MIPS_JALR relocation against non-function symbol reg_obj. This is invalid and most likely a compiler bug.
+
+  .reloc .Ltmp3, R_MIPS_JALR, untyped
+.Ltmp3:
+  jr  $t9
+  nop
+
+## However, we do perform the optimization for untyped symbols:
+untyped:
+  nop
+
+  .type  tls_obj, at object
+  .section  .tbss,"awT", at nobits
+tls_obj:
+  .word 0
+
+  .type  reg_obj, at object
+  .data
+reg_obj:
+  .word 0
+
+# CHECK-LABEL: Disassembly of section .text:
+# CHECK-EMPTY:
+# CHECK-NEXT: test:
+# CHECK-NEXT: jr	$25
+# CHECK-NEXT: nop
+# CHECK-NEXT: jr	$25
+# CHECK-NEXT: nop
+# CHECK-NEXT: b	8 <untyped>
+# CHECK-NEXT: nop