[PATCH] D157020: [lld/ELF] Don't relax R_X86_64_(REX_)GOTPCRELX when offset is too far

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/ELF/Arch/X86_64.cpp:318
+  // overflow.
+  if (maxVA - minVA < (uint64_t(1) << 31) - 1)
+    return false;
----------------
MaskRay wrote:
> aeubanks wrote:
> > @MaskRay is this ok performance wise? this should be very quick and we should skip the expensive stuff below for typical binaries
> `if (maxVA - minVA < (uint64_t(1) << 31))` or `isUInt<31>(...)`
`isUInt<31>(...)` is better


================
Comment at: lld/ELF/Relocations.cpp:1438
 
+  if (expr == R_GOT_PC && !sym.isPreemptible && !isAbsoluteValue(sym) &&
+      (!sym.isGnuIFunc() || config->zIfuncNoplt)) {
----------------
Repeating the complex `!sym.isPreemptible && !isAbsoluteValue(sym) && ...` condition is error-prone and not nice.

If the intent is to execute `in.got->hasGotOffRel.store(true, std::memory_order_relaxed);`, just duplicate this statement when expr becomes `R_RELAX_GOT_PC`


================
Comment at: lld/test/ELF/x86-64-gotpc-err.s:10
-
-# CHECK:      error: {{.*}}:(.text+0x2): relocation R_X86_64_GOTPCRELX out of range: 2147483655 is not in [-2147483648, 2147483647]; references '__stop_data'
-# CHECK-NEXT: error: {{.*}}:(.text+0x9): relocation R_X86_64_REX_GOTPCRELX out of range: 2147483648 is not in [-2147483648, 2147483647]; references '__stop_data'
----------------
You move this into `x86-64-gotpc-relax-too-far.s` but lose this error.

We need another linker script to place `.got` far away from `.text` to retain coverage for this error.




================
Comment at: lld/test/ELF/x86-64-gotpc-relax.s:16
 
-## .got is removed as all GOT-generating relocations are optimized.
+##  In our implementation, .got is retained even if all GOT-generating relocations are optimized.
 # CHECK:      Name              Type            Address          Off    Size   ES Flg Lk Inf Al
----------------



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157020/new/

https://reviews.llvm.org/D157020