[PATCH] D34073: Relax the overflow checking of R_386_PC16

[Rafael Ávila de Espíndola via Phabricator via llvm-commits]

rafael created this revision.

Currently the freebsd early boot code fail to link. The issue reduces to 16 bit code at position 0x7000 wanting to jump to position 0x9000. That  is represented in the .o file as a relocation with no symbol and an addend of 0x9000 - 2 (The -2 is because i386 uses the ip after the current instruction for jumps).

If the addend is interpreted as signed (it should), it is -28674. In a 32 bit architecture,  that is the address 0xffff8ffe. To get there from 0x7000 we have to add 4294909950 (too big) or subtract 57346 (too small). We then produce an error.

What lld is missing is the fact that at runtime this will actually be a 16 bit architecture and adding 0x1ffe produces 0x8ffe which is the correct result in 16 bits (-28674).

Since we have a 16 bit addend and a 16 bit PC, the relocation can move the PC to any 16 bit address and that is the only thing we really need to check: if the address we are pointing to fits in 16 bits. This is unfortunately hard to do since we have to delay subtracting the PC and if we want to do that outside of Target.cpp, we have to move the overflow check out too.  An incomplete patch that tries to do that is at https://reviews.llvm.org/D34070

This patch instead just relaxes the check. Since the value we have is the destination minus the PC and the PC is 16 bits, it should fit in 17 bits if the destination fits in 16 too.

bfd had a similar issue for some time and got a similar fix: https://sourceware.org/ml/binutils/2005-08/msg00001.html


https://reviews.llvm.org/D34073

Files:
  ELF/Target.cpp
  test/ELF/i386-reloc-large-addend.s
  test/ELF/i386-reloc-range.s


Index: test/ELF/i386-reloc-range.s
===================================================================
--- test/ELF/i386-reloc-range.s
+++ test/ELF/i386-reloc-range.s
@@ -1,17 +1,17 @@
 // RUN: llvm-mc %s -o %t.o -triple i386-pc-linux-code16 -filetype=obj
 
-// RUN: echo ".global foo; foo = 0x8202" > %t1.s
+// RUN: echo ".global foo; foo = 0x10202" > %t1.s
 // RUN: llvm-mc %t1.s -o %t1.o -triple i386-pc-linux -filetype=obj
-// RUN: echo ".global foo; foo = 0x8203" > %t2.s
+// RUN: echo ".global foo; foo = 0x10203" > %t2.s
 // RUN: llvm-mc %t2.s -o %t2.o -triple i386-pc-linux -filetype=obj
 
 // RUN: ld.lld -Ttext 0x200 %t.o %t1.o -o %t1
 // RUN: llvm-objdump -d -triple=i386-pc-linux-code16 %t1 | FileCheck %s
 
 // CHECK:        Disassembly of section .text:
 // CHECK-NEXT: _start:
-// CHECK-NEXT:      200:       e9 ff 7f        jmp     32767
-//                                   0x8202 - 0x203 == 32767
+// CHECK-NEXT:      200: {{.*}} jmp -1
+//              0x10202 - 0x203 == 0xffff
 
 // RUN: not ld.lld -Ttext 0x200 %t.o %t2.o -o %t2 2>&1 | FileCheck --check-prefix=ERR %s
 
Index: test/ELF/i386-reloc-large-addend.s
===================================================================
--- /dev/null
+++ test/ELF/i386-reloc-large-addend.s
@@ -0,0 +1,15 @@
+// RUN: llvm-mc %s -o %t.o -triple i386-pc-linux-code16 -filetype=obj
+
+// RUN: echo ".global foo; foo = 0x1" > %t1.s
+// RUN: llvm-mc %t1.s -o %t1.o -triple i386-pc-linux -filetype=obj
+
+// RUN: ld.lld -Ttext 0x7000 %t.o %t1.o -o %t
+// RUN: llvm-objdump -d -triple=i386-pc-linux-code16 %t | FileCheck %s
+
+// CHECK:        Disassembly of section .text:
+// CHECK-NEXT: _start:
+// CHECK-NEXT:     7000:       e9 fe 1f        jmp     8190
+//                            0x1 + 0x9000 - 0x7003 == 8190
+        .global _start
+_start:
+jmp foo + 0x9000
Index: ELF/Target.cpp
===================================================================
--- ELF/Target.cpp
+++ ELF/Target.cpp
@@ -537,7 +537,17 @@
     write16le(Loc, Val);
     break;
   case R_386_PC16:
-    checkInt<16>(Loc, Val, Type);
+    // R_386_PC16 is normally used with 16 bit code. In that situation
+    // the PC is 16 bits, just like the addend. This means that it can
+    // point from any 16 bit address to any other if the possibility
+    // of wrapping is included.
+    // The only restriction we have to check then is that the destination
+    // address fits in 16 bits. That is impossible to do here. The problem is
+    // that we are passed the final value, which already had the
+    // current location subtracted from it.
+    // We just check that Val fits in 17 bits. This misses some cases, but
+    // should have no false positives.
+    checkInt<17>(Loc, Val, Type);
     write16le(Loc, Val);
     break;
   default:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D34073.102093.patch
Type: text/x-patch
Size: 2784 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170609/b7e5ba3e/attachment.bin>