[ELF][X86_64] Use R_GOTREL_FROM_END instead of R_GOTREL for R_X86_64_GOTOFF64

[MaskRay]

Summary

R_X86_64_GOTOFF64: S + A - GOT
R_X86_64_GOTPC{32,64}: GOT + A - P (R_GOTONLY_PC_FROM_END)

R_X86_64_GOTOFF64 should use R_GOTREL_FROM_END so that in junction with
R_X86_64_GOTPC{32,64}, the GOT term is neutralized.

[ruiu]

As always, can you add a test?

[ruiu]

Test is not only important to not break the feature in the future, but it helps readers a lot by demonstrating the issue in action, so even for a change that's not ready for submission, adding test is greatly appreciated.

[MaskRay]

test

[ruiu]

Could you create a new test file for the new test case?

[MaskRay]

Simplify test

[MaskRay]

.

[MaskRay]

.

[ruiu]

Well, I still think separating the test is better to keep the test files manageable.

[MaskRay]

Can you take another look? I have removed one function. R_X86_64_GOTPC32 and R_X86_64_GOTPC64 are related and use the same assumption.

[ruiu]

First of all, please do not remove x86-64-reloc-gotoff64.s. I'd write your new test to that file instead. It is easier to maintain two separate test files than having one larger test file.

Second of all, I got a different compilation result for the code that you gave to me: https://godbolt.org/g/qA1K9y. Is this expected?

[MaskRay]

Update test

[MaskRay]

OK moved the test to x86-64-reloc-gotoff64.s, and renamed x86-64-reloc-got.s to x86-64-reloc-gotpc64.s.

Second of all, I got a different compilation result for the code that you gave to me: https://godbolt.org/g/qA1K9y. Is this expected?

Sorry, the code was produced by gcc, not clang

gcc -O2 -S -mcmodel=medium -fPIC a.c emits the expected two instructions whose sum computes the pc-relative address (runtime address of _DYNAMIC)
clang emits a movabsq instruction which compiles to a R_X86_64_RELATIVE relocation after linking, which is not useful to the dynamic loader.

The dynamic loader needs a runtime-address (_DYNAMIC here), subtracting the link-time address (static _DYNAMIC) from it to compute its image base.

[ruiu]

In D48095#1130635, @MaskRay wrote:

OK moved the test to x86-64-reloc-gotoff64.s, and renamed x86-64-reloc-got.s to `x86
gcc -O2 -S -mcmodel=medium -fPIC a.c emits the expected two instructions whose sum computes the pc-relative address (runtime address of _DYNAMIC)
clang emits a movabsq instruction which compiles to a R_X86_64_RELATIVE relocation after linking, which is not useful to the dynamic loader.

Thank you for the explanation. This is perhaps a bit of topic, but does this mean it's a clang's bug? It seems to me that the code clang emits is not position independent.

[MaskRay]

clang emits a movabsq instruction which compiles to a R_X86_64_RELATIVE relocation after linking, which is not useful to the dynamic loader.

The movabsq encodes the link-time address of a symbol, then in runtime the address is added by an offset R_X86_64_RELATIVE.

The clang way: one movabsq plus a R_X86_64_RELATIVE relocation that is supposed to be resolved by the dynamic loader.
The GCC way: two instructions but no relocation to be resolved.

The clang way is still position-independent.

[ruiu]

The clang way: one movabsq plus a R_X86_64_RELATIVE relocation that is supposed to be resolved by the dynamic loader.
The GCC way: two instructions but no relocation to be resolved.

The clang way is still position-independent.

Is that true? Clang's output needs text relocations which in many cases prohibited. As a result, clang's output won't work in many environments.

[ruiu]

LGTM

In any case, this change makes sense to me. Thanks!

[MaskRay]

Yes, clang emits R_X86_64_RELATIVE in .text, which is text relocation and generates DT_TEXTREL and DF_TEXTREL in the linked executable.

When DT_TEXTREL is used in conjunction with R_X86_64_IRELATIVE (ifunc calls, which can even be left in statically linked executables), glibc ld.so will segfault when resolving the relocations.

https://sourceware.org/git/?p=glibc.git;a=blob_plain;f=elf/dl-reloc.c

/* DT_TEXTREL is now in level 2 and might phase out at some time.
   But we rewrite the DT_FLAGS entry to a DT_TEXTREL entry to make
   testing easier and therefore it will be available at all time.  */
if (__builtin_expect (l->l_info[DT_TEXTREL] != NULL, 0))
{
  ...
    /////////// the program text segment is remapped as read+write but not executable ///////
    if (__mprotect (newp->start, newp->len, PROT_READ|PROT_WRITE) < 0)
    ...

  ////// IFUNC (R_X86_64_IRELATIVE) is resolved in this call ///////
  ELF_DYNAMIC_RELOCATE (l, lazy, consider_profiling, skip_ifunc);

...

/////// recover //////////
/* Undo the segment protection changes.  */
while (__builtin_expect (textrels != NULL, 0))
{
  if (__mprotect (textrels->start, textrels->len, textrels->prot) < 0)

ELF/Arch/X86_64.cpp

RelExpr X86_64<ELFT>::getRelExpr(RelType Type, const Symbol &S,
   case R_X86_64_GOT64:
     return R_GOT_FROM_END;
   case R_X86_64_GOTPCREL:
   case R_X86_64_GOTPCRELX:
   case R_X86_64_REX_GOTPCRELX:
   case R_X86_64_GOTTPOFF:
     return R_GOT_PC;
   case R_X86_64_GOTOFF64:
-    return R_GOTREL;
+    return R_GOTREL_FROM_END;
   case R_X86_64_GOTPC32:
   case R_X86_64_GOTPC64:
     return R_GOTONLY_PC_FROM_END;
   case R_X86_64_NONE:
     return R_NONE;
   default:
     return R_INVALID;
   }

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

 // REQUIRES: x86
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 // RUN: ld.lld %t.o -shared -o %t.so
 // RUN: llvm-readelf -s %t.so | FileCheck %s -check-prefix=SECTION
 // RUN: llvm-objdump -d %t.so | FileCheck %s
 
-// SECTION: .got PROGBITS 0000000000003070 003070 000000
+// SECTION: .dynamic DYNAMIC 0000000000003000
+// SECTION: .got PROGBITS 0000000000003070
 
-// 0x3070 (.got end) - 0x1007 = 8297
-// CHECK: gotpc32:
-// CHECK-NEXT: 1000: {{.*}} leaq 8297(%rip), %r15
-.global gotpc32
-gotpc32:
-  leaq _GLOBAL_OFFSET_TABLE_(%rip), %r15
+// FIXME: All the _GLOBAL_OFFSET_TABLE_ occurrences below refer to the address
+// of GOT base, not the address of the symbol _GLOBAL_OFFSET_TABLE_, as these
+// instructions are special and they produce GOT base relative relocations. We
+// currently use .got end as the GOT base, which is not equal to
+// _GLOBAL_OFFSET_TABLE_.
 
+// 0x3070 (.got end) - 0x1000 = 8304
 // CHECK: gotpc64:
-// CHECK-NEXT: 1007: {{.*}} movabsq $8297, %r11
+// CHECK-NEXT: 1000: {{.*}} movabsq $8304, %r11
 .global gotpc64
 gotpc64:
   movabsq $_GLOBAL_OFFSET_TABLE_-., %r11
+
+// The assembly is generated by
+// clang++ -S -mcmodel=medium -fPIC a.c
+// extern long _DYNAMIC[] __attribute__((visibility("hidden")));
+// long* dynamic() { return _DYNAMIC; }
+
+// 0x3070 (.got end) - 0x100a = 8287
+// 0x3000 (_DYNAMIC) - 0x3070 (.got end) = -112
+// CHECK:      100a: {{.*}} leaq 8287(%rip), %rdx
+// CHECK-NEXT: 1011: {{.*}} movabsq $-112, %rax
+.global dynamic
+dynamic:
+  leaq _GLOBAL_OFFSET_TABLE_(%rip), %rdx
+  movabsq $_DYNAMIC@GOTOFF, %rax
+  addq %rdx, %rax
+  ret

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

-// REQUIRES: x86
-
-// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
-// RUN: ld.lld -shared -o %t.so %t.o
-// RUN: llvm-readelf -sections %t.so | FileCheck %s
-// RUN: llvm-objdump -d %t.so | FileCheck -check-prefix=DISASM %s
-
-// CHECK: .dynamic DYNAMIC  0000000000002000 002000
-// CHECK: .got     PROGBITS 0000000000002070 002070
-
-// DISASM: 1000: 48 ba 90 ff ff ff ff ff ff ff   movabsq $-112, %rdx
-
-.global _start
-.weak _DYNAMIC
-.hidden _DYNAMIC
-_start:
-  movabsq $_DYNAMIC@GOTOFF, %rdx