[PATCH] D70406: Ignore R_MIPS_JALR relocations against non-function symbols

[Alexander Richardson via Phabricator via llvm-commits]

arichardson created this revision.
arichardson added reviewers: atanasyan, jrtc27.
Herald added subscribers: llvm-commits, MaskRay, krytarowski, sdardis, emaste.
Herald added a reviewer: espindola.
Herald added a project: LLVM.

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). 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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70406

Files:
  lld/ELF/Arch/Mips.cpp
  lld/test/ELF/mips-r-jalr-non-functions.s


Index: lld/test/ELF/mips-r-jalr-non-functions.s
===================================================================
--- /dev/null
+++ lld/test/ELF/mips-r-jalr-non-functions.s
@@ -0,0 +1,60 @@
+# REQUIRES: mips
+## Check that we ignore R_MIPS_JALR relocations agains non-function symbols.
+## Older versions of clang was erroneously generating them for function pointers
+## loaded from any table (not just the GOT), so we need to ignore the relocations
+## to avoid generating binaries that crash when executed.
+
+## This test case was created from the following C source code:
+## int callee(void) { return 0; }
+## typedef int (*fnptr)(void);
+## static fnptr function_table[] = { &callee, &callee };
+## // to prevent the compiler from optimizing away the table:
+## fnptr* get_function_table(void) { return function_table; }
+## int __start(int x) {
+##   return function_table[0]();
+## }
+
+# 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
+# WARNING-MESSAGE: warning: found R_MIPS_JALR relocation against non-function symbol function_table. This is invalid and most likely a compiler bug.
+# RUN: llvm-objdump -d %t.so | FileCheck %s
+
+	.set	noreorder
+	.set	nomacro
+	.set	noat
+	.text
+
+	.globl	__start
+	.p2align	3
+	.type	__start, at function
+	.ent	__start
+__start:
+	ld	$1, %got_page(function_table)($gp)
+	ld	$t9, %got_ofst(function_table)($1)
+## This relocation should not have been generated by the compiler
+## But we should still be able to handle it
+	.reloc .Ltmp0, R_MIPS_JALR, function_table
+.Ltmp0:
+	jalr	$t9
+	nop
+	.end	__start
+.Lfunc_end2:
+	.size	__start, .Lfunc_end2-__start
+
+	.type	function_table, at object
+	.data
+	.p2align	3
+function_table:
+	.8byte	0
+	.8byte	0
+	.size	function_table, 16
+
+
+# CHECK-LABEL: Disassembly of section .text:
+# CHECK-EMPTY:
+# CHECK-NEXT: 0000000000010370 __start:
+# CHECK-NEXT:    10370: df 81 80 20                  	ld	$1, -32736($gp)
+# CHECK-NEXT:    10374: dc 39 03 80                  	ld	$25, 896($1)
+## This previously generated a "bal 65560 <function_table>" which would crash at run time:
+# CHECK-NEXT:    10378: 03 20 f8 09                  	jalr	$25
+# CHECK-NEXT:    1037c: 00 00 00 00                  	nop
Index: lld/ELF/Arch/Mips.cpp
===================================================================
--- lld/ELF/Arch/Mips.cpp
+++ lld/ELF/Arch/Mips.cpp
@@ -83,6 +83,17 @@
 
   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()) {
+      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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70406.229876.patch
Type: text/x-patch
Size: 3232 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191118/38eace9c/attachment.bin>