[ELF] Allow relative relocations to absolute symbols in PIC

[phosek]

Summary

While these might cause an overflow at runtime on some targets, lld shouldn't reject such a case as it's clearly meaningful and supported by both BFD ld and gold.

[ruiu]

+pcc who wrote this code.

[phosek]

I ran into this issue while looking a linker script failure described in bug 30406. The failure was caused by the check in ELF/Relocation.cpp. While debugging the issue, I noticed that both BFD ld and gold handle this case and don't throw an error for the relocation-relative-absolute.s test case.

This is really a corner case behavior that's not document very well by the ELF specification, so I got in touch with Ian Lance Taylor and his opinion on this is:

It's an unusual case, but the only possible meaning of this is a call to the absolute address. The linker needs to either generate a dynamic PC-relative relocation to an SHN_ABS symbol (which could of course overflow at runtime on x86_64) or generate a PLT with a JMP_SLOT reference to an SHN_ABS symbol. I don't see how anything else is correct. In particular I don't see how it is correct to reject such a case, it's clearly meaningful and the technique is used on, at least, pre-VDSO GNU/Linux and AIX.

The problem is that neither BFD ld nor gold do this, instead they both generate an absolute jump. This change changes the lld behavior to match the current behavior of ld and gold. If were to implement the behavior Ian suggested, we'd have to also modify the scanRelocs function, in particular the part which handles the constant case and generate a dynamic relocation when the expression is relative relocation to an absolute symbol.

[pcc]

I don't see why we should switch from rejecting to doing the wrong thing. The right thing to do, as Ian suggested, would be to create either a PLT entry or a dynamic relocation.

Independently of that, as I and Rafael mentioned on the review thread for D25560, we need to fix the synthetic symbols we create for linker scripts.

[phosek]

We run into this issue again in our codebase when linking the C library (based on musl) with lld. I've reduced that case into a new test case which I've added to the patch. While the solution that Ian suggested may be the "right thing", it's clearly not what BFD ld nor gold implement and it's unlikely that their implementation is going to change. Since there is code out there relying on this behavior, it'd be useful for lld to support it.

[ruiu]

Sorry for the belated response. I wonder if it is hard to create a dynamic relocation for this case. Even though it is not what GNU linkers do, I don't think doing the same because they do it wrongly is the right thing, unless we really need a bug-to-bug compatibility. Is this a case that we need a bug-compatibility?

[phosek]

This appears to be working, please take a look. The only question is whether we should be also generating the relocation in the weak case or keep it as a special case same as before?

[phosek]

Added tests.

ELF/Relocations.cpp

static bool isStaticLinkTimeConstant(RelExpr E, uint32_t Type,
   if (isPreemptible(Body, Type))
     return false;
 
   if (!Config->Pic)
     return true;
 
   bool AbsVal = isAbsoluteValue<ELFT>(Body);
   bool RelE = isRelExpr(E);
-  if (AbsVal && !RelE)
+  if (AbsVal || RelE)
     return true;
-  if (!AbsVal && RelE)
-    return true;
-
-  // Relative relocation to an absolute value. This is normally unrepresentable,
-  // but if the relocation refers to a weak undefined symbol, we allow it to
-  // resolve to the image base. This is a little strange, but it allows us to
-  // link function calls to such symbols. Normally such a call will be guarded
-  // with a comparison, which will load a zero from the GOT.
-  if (AbsVal && RelE) {
-    if (Body.isUndefined() && !Body.isLocal() && Body.symbol()->isWeak())
-      return true;
-    error(S.getLocation(RelOff) + ": relocation " + toString(Type) +
-          " cannot refer to absolute symbol '" + toString(Body) +
-          "' defined in " + toString(Body.File));
-    return true;
-  }
 
   return Target->usesOnlyLowPageBits(Type);
 }
 
 static RelExpr toPlt(RelExpr Expr) {
   if (Expr == R_PPC_OPD)
     return R_PPC_PLT_OPD;
   if (Expr == R_PC)

test/ELF/linkerscript/symbol-absolute.s

+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+# RUN: echo "PROVIDE_HIDDEN(base = 0);" > %t.script
+# RUN: ld.lld -shared --gc-sections --script %t.script -o %t1 %t
+# RUN: llvm-readobj --elf-output-style=GNU --file-headers --symbols %t1 | FileCheck %s
+# CHECK: 0000000000000000     0 NOTYPE  LOCAL  HIDDEN   ABS base
+
+.text
+.globl _start
+_start:
+  lea base(%rip),%rax

test/ELF/relocation-relative-absolute.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %tinput1.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux \
 # RUN:   %S/Inputs/relocation-relative-absolute.s -o %tinput2.o
-# RUN: not ld.lld %tinput1.o %tinput2.o -o %t -pie 2>&1 | FileCheck %s
+# RUN: ld.lld %tinput1.o %tinput2.o -o %t -pie
+# RUN: llvm-readobj -dyn-relocations %t | FileCheck %s
+
+# CHECK:      Dynamic Relocations {
+# CHECK-NEXT: }
 
 .globl _start
 _start:
 
-# CHECK: {{.*}}input1.o:(.text+0x1): relocation R_X86_64_PLT32 cannot refer to absolute symbol 'answer' defined in {{.*}}input2.o
-
 call answer@PLT