[ELF] When a relocation is out of range print the value and the range

[ruiu]

ELF/Target.h

147

In general a function name should be a verb, so how about reportRangeError?

151–152

I think the range is half-open, so you should use [) instead of [].

Please add a space character after the comma.

[arichardson]

Addressed review comments

[arichardson]

ELF/Target.h

147

Good suggestion, done.

151–152

Actually maxInt() returns the highest possible value so the range is inclusive.

[grimar]

Patch LGTM, please wait for Rui approval.

ELF/Target.h

150

nit: I would shorten this:
relocation R_386_PC16 out of range: 65536 is not in [-65536, 65535]
It is the matter of taste though.

[arichardson]

ELF/Target.h

150

Sounds good, I'll change if it to that if Rui also agrees.

[ruiu]

ELF/Target.h

150

Sounds good.

171

Why don't you just pass Twine((int64_t)V)?

[arichardson]

ELF/Target.h

171

Since we only call this with values <= INT64_MAX that is much better, thanks.

[ruiu]

ELF/Target.h

171

But why INT64_MAX? It seems just inconsistent. If you sign-extend, you want to always do that, no?

[arichardson]

ELF/Target.h

171

I meant that this function is either called with a negative signed number or an unsigned value that doesn't have the int64 sign bit set so that using Twine((int64_t)V) is exactly the same as the ternary expression that I had here.

[ruiu]

ELF/Target.h

171

If Twine((int64_t)V) is exactly the same as the ternary expression, please use the simpler form which is Twine((int64_t)V).

[arichardson]

ELF/Target.h

171

Yes that is exactly what I meant. Sorry about the miscommunication.

[arichardson]

Updated error message
Simplified ternary expression

[ruiu]

LGTM

test/ELF/i386-reloc-16.s

12

Please remove an extra space after "," (you have two spaces after ",").

test/ELF/i386-reloc-8.s

12

Ditto

test/ELF/x86-64-reloc-16.s

12

Ditto

test/ELF/x86-64-reloc-8.s

12–13

Ditto

test/ELF/x86-64-reloc-error.s

9–10

Ditto

ELF/Target.h

 uint64_t getPPC64TocBase();
 uint64_t getAArch64Page(uint64_t Expr);
 
 extern TargetInfo *Target;
 TargetInfo *getTarget();
 
 template <class ELFT> bool isMipsPIC(const Defined *Sym);
 
+static inline void reportRangeError(uint8_t *Loc, RelType Type, const Twine &V,
+                                    int64_t Min, uint64_t Max) {
+  error(getErrorLocation(Loc) + "relocation " + lld::toString(Type) +
+        " out of range: " + V + " is not in [" + Twine(Min) + ", " +
+        Twine(Max) + "]");
+}
+
 template <unsigned N>
 static void checkInt(uint8_t *Loc, int64_t V, RelType Type) {
   if (!llvm::isInt<N>(V))
-    error(getErrorLocation(Loc) + "relocation " + lld::toString(Type) +
-          " out of range");
+    reportRangeError(Loc, Type, Twine(V), llvm::minIntN(N), llvm::maxIntN(N));
 }
 
 template <unsigned N>
 static void checkUInt(uint8_t *Loc, uint64_t V, RelType Type) {
   if (!llvm::isUInt<N>(V))
-    error(getErrorLocation(Loc) + "relocation " + lld::toString(Type) +
-          " out of range");
+    reportRangeError(Loc, Type, Twine(V), 0, llvm::maxUIntN(N));
 }
 
 template <unsigned N>
 static void checkIntUInt(uint8_t *Loc, uint64_t V, RelType Type) {
   if (!llvm::isInt<N>(V) && !llvm::isUInt<N>(V))
-    error(getErrorLocation(Loc) + "relocation " + lld::toString(Type) +
-          " out of range");
+    // For the error message we should cast V to a signed integer so that error
+    // messages show a small negative value rather than an extremely large one
+    reportRangeError(Loc, Type, Twine((int64_t)V), llvm::minIntN(N),
+                     llvm::maxUIntN(N));
 }
 
 template <unsigned N>
 static void checkAlignment(uint8_t *Loc, uint64_t V, RelType Type) {
   if ((V & (N - 1)) != 0)
     error(getErrorLocation(Loc) + "improper alignment for relocation " +
           lld::toString(Type) + ": 0x" + llvm::utohexstr(V) +
           " is not aligned to " + Twine(N) + " bytes");
 }
 } // namespace elf
 } // namespace lld
 
 #endif

test/ELF/aarch64-abs16.s

 // 11002: S = 0x100, A = -0x8100
 //        S + A = 0x8000
 // CHECK-NEXT: 20000 ffff0080
 
 // RUN: not ld.lld %t.o %t255.o -o %t2
 //   | FileCheck %s --check-prefix=OVERFLOW
 // RUN: not ld.lld %t.o %t257.o -o %t2
 //   | FileCheck %s --check-prefix=OVERFLOW
-// OVERFLOW: Relocation R_AARCH64_ABS16 out of range
+// OVERFLOW: Relocation R_AARCH64_ABS16 out of range: 65536 is not in [-32768, 65535]

test/ELF/aarch64-abs32.s

 // 20004: S = 0x100, A = -0x80000100
 //        S + A = 0x80000000
 // CHECK-NEXT: 20000 ffffffff 00000080
 
 // RUN: not ld.lld %t.o %t255.o -o %t2
 //   | FileCheck %s --check-prefix=OVERFLOW
 // RUN: not ld.lld %t.o %t257.o -o %t2
 //   | FileCheck %s --check-prefix=OVERFLOW
-// OVERFLOW: Relocation R_AARCH64_ABS32 out of range
+// OVERFLOW: Relocation R_AARCH64_ABS32 out of range: 4294967296 is not in [-2147483648, 4294967295]

test/ELF/aarch64-ldprel-lo19-invalid.s

 # REQUIRES: aarch64
 
 # RUN: llvm-mc -filetype=obj -triple=aarch64-linux-none %s -o %t.o
 # RUN: not ld.lld -shared %t.o -o %t 2>&1 | FileCheck %s
 
-# CHECK: relocation R_AARCH64_LD_PREL_LO19 out of range
+# CHECK: relocation R_AARCH64_LD_PREL_LO19 out of range: 2065536 is not in [-1048576, 1048575]
 
   ldr x8, patatino
   .data
   .zero 2000000
 patatino:

test/ELF/aarch64-prel16.s

 // 11002: S = 0x100, A = 0x8f02, P = 0x11002
 //        S + A - P = 0x8000
 // CHECK-NEXT: 11000 ffff0080
 
 // RUN: not ld.lld %t.o %t255.o -o %t2
 //   | FileCheck %s --check-prefix=OVERFLOW
 // RUN: not ld.lld %t.o %t257.o -o %t2
 //   | FileCheck %s --check-prefix=OVERFLOW
-// OVERFLOW: Relocation R_AARCH64_PREL16 out of range
+// OVERFLOW: Relocation R_AARCH64_PREL16 out of range: -94209 is not in [-32768, 65535]

test/ELF/aarch64-prel32.s

 // 11004: S = 0x100, A = -0x7ffef0fc, P = 0x11004
 //        S + A - P = 0x80000000
 // CHECK-NEXT: 11000 ffffffff 00000080
 
 // RUN: not ld.lld %t.o %t255.o -o %t2
 //   | FileCheck %s --check-prefix=OVERFLOW
 // RUN: not ld.lld %t.o %t257.o -o %t2
 //   | FileCheck %s --check-prefix=OVERFLOW
-// OVERFLOW: Relocation R_AARCH64_PREL32 out of range
+// OVERFLOW: Relocation R_AARCH64_PREL32 out of range: 18446744071562006527 is not in [-2147483648, 4294967295]

test/ELF/i386-reloc-16.s

 // REQUIRES: x86
 
 // RUN: llvm-mc -filetype=obj -triple=i386-pc-linux %S/Inputs/x86-64-reloc-16.s -o %t1
 // RUN: llvm-mc -filetype=obj -triple=i386-pc-linux %S/Inputs/x86-64-reloc-16-error.s -o %t2
 // RUN: llvm-mc -filetype=obj -triple=i386-pc-linux %s -o %t
 // RUN: ld.lld -shared %t %t1 -o %t3
 
 // CHECK:      Contents of section .text:
 // CHECK-NEXT:   200000 42
 
 // RUN: not ld.lld -shared %t %t2 -o %t4 2>&1 | FileCheck --check-prefix=ERROR %s
-// ERROR: relocation R_386_16 out of range
+// ERROR: relocation R_386_16 out of range: 65536 is not in [0, 65535]
 
 .short foo

test/ELF/i386-reloc-8.s

 // REQUIRES: x86
 
 // RUN: llvm-mc -filetype=obj -triple=i386-pc-linux %S/Inputs/i386-reloc-8.s -o %t1
 // RUN: llvm-mc -filetype=obj -triple=i386-pc-linux %S/Inputs/i386-reloc-8-error.s -o %t2
 // RUN: llvm-mc -filetype=obj -triple=i386-pc-linux %s -o %t
 // RUN: ld.lld -shared %t %t1 -o %t3
 
 // CHECK:      Contents of section .text:
 // CHECK-NEXT:   200000 42
 
 // RUN: not ld.lld -shared %t %t2 -o %t4 2>&1 | FileCheck --check-prefix=ERROR %s
-// ERROR: relocation R_386_8 out of range
+// ERROR: relocation R_386_8 out of range: 256 is not in [0, 255]
 
 .byte foo

test/ELF/i386-reloc-range.s

 
 // CHECK:        Disassembly of section .text:
 // CHECK-NEXT: _start:
 // 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
 
-// ERR: {{.*}}:(.text+0x1): relocation R_386_PC16 out of range
+// ERR: {{.*}}:(.text+0x1): relocation R_386_PC16 out of range: 65536 is not in [-65536, 65535]
 
         .global _start
 _start:
         jmp foo

test/ELF/linkerscript/eh-frame-reloc-out-of-range.s

 ## Check that error is correctly reported when .eh_frame reloc
 ## is out of range
 
 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
 # RUN: echo "PHDRS { eh PT_LOAD; text PT_LOAD; }  \
 # RUN:       SECTIONS { . = 0x10000; \
 # RUN:         .eh_frame_hdr : { *(.eh_frame_hdr*) } : eh \
 # RUN:         .eh_frame : { *(.eh_frame) } : eh \
 # RUN:         . = 0xF00000000; \
 # RUN:         .text : { *(.text*) } : text \
 # RUN:       }" > %t.script
 # RUN: not ld.lld %t.o -T %t.script -o %t 2>&1 | FileCheck %s
 
-# CHECK: error: {{.*}}:(.eh_frame+0x20): relocation R_X86_64_PC32 out of range
+# CHECK: error: {{.*}}:(.eh_frame+0x20): relocation R_X86_64_PC32 out of range: 64424443872 is not in [-2147483648, 2147483647]
 
   .text
   .globl _start
 _start:
   .cfi_startproc
   .cfi_lsda 0, _ex
   nop
   .cfi_endproc
 
   .data
 _ex:
   .word 0

test/ELF/mips-out-of-bounds-call16-reloc.s

 # Check that we create an error on an out-of-bounds R_MIPS_CALL_16
 
 # REQUIRES: mips
 # RUN: llvm-mc -filetype=obj -triple=mips64-unknown-linux %s -o %t1.o
 # RUN: not ld.lld %t1.o -o %t.exe 2>&1 | FileCheck %s
 
-# CHECK: relocation R_MIPS_CALL16 out of range
+# CHECK: relocation R_MIPS_CALL16 out of range: 32768 is not in [-32768, 32767]
 
 .macro generate_values
   .irp i, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9,
     .irp j, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9,
       .irp k, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9,
         .irp l, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9,
           .text
           .globl sym_\i\j\k\l

test/ELF/ppc64-addr16-error.s

 // RUN: llvm-mc -filetype=obj -triple=powerpc64-unknown-linux %s -o %t
 // RUN: llvm-mc -filetype=obj -triple=powerpc64-unknown-linux %p/Inputs/ppc64-addr16-error.s -o %t2
 // RUN: not ld.lld -shared %t %t2 -o %t3 2>&1 | FileCheck %s
 // REQUIRES: ppc
 
 .short sym+65539
 
-// CHECK: relocation R_PPC64_ADDR16 out of range
+// CHECK: relocation R_PPC64_ADDR16 out of range: 65539 is not in [-32768, 32767]

test/ELF/x86-64-reloc-16.s

 // REQUIRES: x86
 
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %S/Inputs/x86-64-reloc-16.s -o %t1
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %S/Inputs/x86-64-reloc-16-error.s -o %t2
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t
 // RUN: ld.lld -shared %t %t1 -o %t3
 
 // CHECK:      Contents of section .text:
 // CHECK-NEXT:   200000 42
 
 // RUN: not ld.lld -shared %t %t2 -o %t4 2>&1 | FileCheck --check-prefix=ERROR %s
-// ERROR: relocation R_X86_64_16 out of range
+// ERROR: relocation R_X86_64_16 out of range: 65536 is not in [0, 65535]
 
 .short foo

test/ELF/x86-64-reloc-8.s

 // REQUIRES: x86
 
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %S/Inputs/x86-64-reloc-8.s -o %t1
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %S/Inputs/x86-64-reloc-8-error.s -o %t2
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t
 // RUN: ld.lld -shared %t %t1 -o %t3
 
 // CHECK:      Contents of section .text:
 // CHECK-NEXT:   200000 42
 
 // RUN: not ld.lld -shared %t %t2 -o %t4 2>&1 | FileCheck --check-prefix=ERROR %s
-// ERROR: relocation R_X86_64_8 out of range
+// ERROR: relocation R_X86_64_8 out of range: 256 is not in [0, 255]
+// ERROR: relocation R_X86_64_8 out of range: 256 is not in [0, 255]
 
 .byte foo

test/ELF/x86-64-reloc-error.s

 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %S/Inputs/x86-64-reloc-error.s -o %tabs
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t
 // RUN: not ld.lld -shared %tabs %t -o %t2 2>&1 | FileCheck %s
 // REQUIRES: x86
 
   movl $big, %edx
   movq $foo - 0x1000000000000, %rdx
 
-# CHECK: {{.*}}:(.text+0x1): relocation R_X86_64_32 out of range
-# CHECK: {{.*}}:(.text+0x8): relocation R_X86_64_32S out of range
+# CHECK: {{.*}}:(.text+0x1): relocation R_X86_64_32 out of range: 68719476736 is not in [0, 4294967295]
+# CHECK: {{.*}}:(.text+0x8): relocation R_X86_64_32S out of range: -281474976710656 is not in [-2147483648, 2147483647]

test/ELF/x86-64-reloc-range.s

 // RUN: llvm-mc %s -o %t.o -triple x86_64-pc-linux -filetype=obj
 // RUN: not ld.lld %t.o -o %t.so -shared 2>&1 | FileCheck %s
 
-// CHECK: {{.*}}:(.text+0x3): relocation R_X86_64_PC32 out of range
+// CHECK: {{.*}}:(.text+0x3): relocation R_X86_64_PC32 out of range: 2147483648 is not in [-2147483648, 2147483647]
 // CHECK-NOT: relocation
 
         lea     foo(%rip), %rax
         lea     foo(%rip), %rax
 
         .hidden foo
         .bss
         .zero 0x7fffe007
 foo: