[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;
   }