[ELF] Allow R_386_GOTOFF from .debug_info

[MaskRay]

Summary

In GCC emitted .debug_info sections, R_386_GOTOFF may be used to
relocate DW_AT_GNU_call_site_value values
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98946).

R_386_GOTOFF (S + A - GOT) is one of the isStaticLinkTimeConstant relocation
type which is not PC-relative, so it can be used from non-SHF_ALLOC sections. We
current allow new relocation types as needs come. The diagnostic has caught some
bugs in the past.

[manojgupta]

Thanks Fangrui for working on this!

[peter.smith]

Change LGTM. For a future change, would it be worth looking at this from another direction and saying what can we disallow in non SHF_ALLOC sections, for example PC-relative? There can be an allow-list for known relocations and perhaps a warning for unknown ones. This should make us a little more resilient to changes in objects.

[jhenderson]

Not an i386 user myself, but the change seems reasonable to me.

[MaskRay]

Thanks for the review!

In D95994#2541271, @peter.smith wrote:

Change LGTM. For a future change, would it be worth looking at this from another direction and saying what can we disallow in non SHF_ALLOC sections, for example PC-relative? There can be an allow-list for known relocations and perhaps a warning for unknown ones. This should make us a little more resilient to changes in objects.

I stumped upon isStaticLinkTimeConstant - the non PC-relative relocation types can potentially be used in DW_AT_GNU_call_site_value. If such a request raises in the next time, we should try refactoring the isStaticLinkTimeConstant list and make that accessible from InputSection.cpp.

lld/ELF/InputSection.cpp

if (expr == R_NONE)
       continue;
 
     if (expr == R_SIZE) {
       target->relocateNoSym(bufLoc, type,
                             SignExtend64<bits>(sym.getSize() + addend));
       continue;
     }
 
-    if (expr != R_ABS && expr != R_DTPREL && expr != R_RISCV_ADD) {
+    // R_ABS/R_DTPREL and some other relocations can be used from non-SHF_ALLOC
+    // sections.
+    if (expr != R_ABS && expr != R_DTPREL && expr != R_GOTPLTREL &&
+        expr != R_RISCV_ADD) {
       std::string msg = getLocation<ELFT>(offset) +
                         ": has non-ABS relocation " + toString(type) +
                         " against symbol '" + toString(sym) + "'";
       if (expr != R_PC && expr != R_ARM_PCA) {
         error(msg);
         return;
       }
 

lld/test/ELF/non-abs-reloc.s

 // REQUIRES: x86
 // RUN: split-file %s %t
-// RUN: llvm-mc -filetype=obj -triple=x86_64 %t/asm -o %t.o
-// RUN: ld.lld -T %t/lds %t.o -o %t.exe 2>&1 | FileCheck %s
-// CHECK:      warning: {{.*}}.o:(.nonalloc1+0x1): has non-ABS relocation R_X86_64_PC32 against symbol '_start'
-// CHECK-NEXT: warning: {{.*}}.o:(.nonalloc1+0x6): has non-ABS relocation R_X86_64_PC32 against symbol '_start'
+// RUN: llvm-mc -filetype=obj -triple=i386 %t/asm -o %t.o
+// RUN: ld.lld -T %t/lds %t.o -o %t.exe 2>&1 | FileCheck %s --implicit-check-not=warning: --implicit-check-not=error:
+// CHECK:      warning: {{.*}}.o:(.nonalloc1+0x1): has non-ABS relocation R_386_PC32 against symbol '_start'
+// CHECK-NEXT: warning: {{.*}}.o:(.nonalloc1+0x6): has non-ABS relocation R_386_PC32 against symbol '_start'
 
 // RUN: llvm-objdump -D --no-show-raw-insn %t.exe | FileCheck --check-prefix=DISASM %s
 // DISASM:      Disassembly of section .nonalloc:
 // DISASM-EMPTY:
 // DISASM-NEXT: <.nonalloc>:
 // DISASM-NEXT:   0: nop
-// DISASM-NEXT:   1: callq 0x0
-// DISASM-NEXT:   6: callq 0x0
+// DISASM-NEXT:   1: calll 0x0
+// DISASM-NEXT:   6: calll 0x0
 
 //--- lds
 SECTIONS {
   .nonalloc 0 : { *(.nonalloc*) }
 }
 //--- asm
 .globl _start
 _start:
+.L0:
   nop
 
 .section .nonalloc0
   nop
 
 .section .nonalloc1
   .byte 0xe8
   .long _start - . - 4
   .byte 0xe8
   .long _start - . - 4
+
+// GCC may relocate DW_AT_GNU_call_site_value with R_386_GOTOFF.
+// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98946
+.section .debug_info
+  .long .L0@gotoff