[ELF][MIPS] Always resolve MIPS GP-relative relocations to 'local' definitions

[atanasyan]

Summary

In case of MIPS, GP-relative relocations always resolve to a definition in a regular input file, ignoring the one-definition rule. Such relocations are used to setup GP relative offsets in a function's prologue. So we, for example, should not attempt to create a dynamic relocation even if the target symbol is preemptible.

Fixes bug 27880.

[• rafael]

ELF/Relocations.cpp

234

From the description it looks like what you want is

static bool isPreemptible(const SymbolBody &B, RelExpr E) {

if (Config->EMachine == EM_MIPS &&  E == R_GOTREL)
  return false;
return Body.isPreemptible();

}

and use that in here instead of a plain Body.isPreemptible.

[atanasyan]

Replace isForcedLinkTimeConstant by isPreemptible. It is better reflect the fact that we consider symbol as non-preemptible under some conditions.

[• rafael]

So, while this should work, it fairly undesirable that preemption depends on the relocation.

Any idea why gpword is used with a global? The comment in the MC parser implies it should be used with a local:

/ parseDirectiveGpWord
/ ::= .gpword local_sym

Also, llvm doesn't seem to print it for anything but jump tables.

So, long way of saying LGTM, but please add a fixme about dropping this once freebsd moves out of the prehistorical gcc.

[atanasyan]

In D20664#442189, @rafael wrote:

Any idea why gpword is used with a global?

It's my mistake. I select a wrong test case to show the problem (it was easier to copy-paste existing test code). A real case is R_MIPS_GPREL16. This relocation is used to setup GP offset in a function's prologue. That is why it should be always resolved locally. Here is an example. I am going to update the test case before commit.

  .text                                                                                                                                                                                                                                                     
  .global  entry                                                                                                                                                                                                                                            
entry:                                                                                                                                                                                                                                                      
  lui     $gp,%hi(%neg(%gp_rel(foo)))                                                                                                                                                                                                                       
  daddu   $gp,$gp,$t9                                                                                                                                                                                                                                       
  daddiu  $gp,$gp,%lo(%neg(%gp_rel(foo)))

R_MIPS_GPREL32 is for local symbols. But GNU linker does not show any error if R_MIPS_GPREL32's target is preemptible symbol. So I still think it is better to handle them uniformly too.

And thanks for review.

[atanasyan]

Use stricter condition in the isPreemptible. Now we check on R_MIPS_GPREL16 relocation only because only this relocation can be against preemptible symbol and should be resolved locally. The side effect - isPreemptible routine becomes more complicated with a long comment describes handling 3-in-1 MIPS relocation packs.

Still LGTM?

[• rafael]

This LGTM, but we should make sure that llvm instead produces something like

.global foo

foo:
.Lbar:

lui     $gp,%hi(%neg(%gp_rel(.Lbar)))
daddu   $gp,$gp,$t9
daddiu  $gp,$gp,%lo(%neg(%gp_rel(.Lbar))

so that we can on day remove this special case.

ELF/Relocations.cpp

 
 // True if this expression is of the form Sym - X, where X is a position in the
 // file (PC, or GOT for example).
 static bool isRelExpr(RelExpr Expr) {
   return Expr == R_PC || Expr == R_GOTREL || Expr == R_PAGE_PC ||
          Expr == R_RELAX_GOT_PC;
 }
 
+static bool isForcedLinkTimeConstant(RelExpr E) {
+  // In case of MIPS GP-relative relocations always resolve to a definition
+  // in a regular input file, ignoring the one-definition rule. So we,
+  // for example, should not attempt to create a dynamic relocation even
+  // if the target symbol is preemptible.
+  return Config->EMachine == EM_MIPS && E == R_GOTREL;
+}
+
 template <class ELFT>
 static bool isStaticLinkTimeConstant(RelExpr E, uint32_t Type,
                                      const SymbolBody &Body) {
   // These expressions always compute a constant
   if (E == R_SIZE || E == R_GOT_FROM_END || E == R_GOT_OFF ||
       E == R_MIPS_GOT_LOCAL || E == R_MIPS_GOT_LOCAL_PAGE ||
       E == R_GOT_PAGE_PC || E == R_GOT_PC || E == R_PLT_PC || E == R_TLSGD_PC ||
       E == R_TLSGD || E == R_PPC_PLT_OPD)
     return true;
 
+  if (isForcedLinkTimeConstant(E))
+    return true;
+
   // These never do, except if the entire file is position dependent or if
   // only the low bits are used.
   if (E == R_GOT || E == R_PLT)
     return Target->usesOnlyLowPageBits(Type) || !Config->Pic;
 
   if (Body.isPreemptible())
     return false;
 
for (auto I = Rels.begin(), E = Rels.end(); I != E; ++I) {
 
     if (unsigned Processed =
             handleTlsRelocation<ELFT>(Type, Body, C, Offset, Addend, Expr)) {
       I += (Processed - 1);
       continue;
     }
 
     if (needsPlt(Expr) || Expr == R_THUNK || refersToGotEntry(Expr) ||
-        !Body.isPreemptible()) {
+        !Body.isPreemptible() || isForcedLinkTimeConstant(Expr)) {
       // If the relocation points to something in the file, we can process it.
       bool Constant = isStaticLinkTimeConstant<ELFT>(Expr, Type, Body);
 
       // If the output being produced is position independent, the final value
       // is still not known. In that case we still need some help from the
       // dynamic linker. We can however do better than just copying the incoming
       // relocation. We can process some of it and and just ask the dynamic
       // linker to add the load address.

test/ELF/mips-gprel32-relocs-so.s

+# Check R_MIPS_GPREL32 relocation calculation in case of DSO output.
+# GP-relative relocation always resolve to a definition in a regular input file.
+
+# RUN: llvm-mc -filetype=obj -triple=mips-unknown-linux %s -o %t.o
+# RUN: ld.lld -shared -o %t.so %t.o
+# RUN: llvm-objdump -s -t %t.so | FileCheck %s
+
+# REQUIRES: mips
+
+  .text
+  .global foo
+foo:
+  nop
+
+  .section .rodata, "a"
+  .gpword foo
+
+# CHECK: Contents of section .rodata:
+# CHECK:  0114 fffe8010
+#              ^ 0x10000 - 0x27ff0
+
+# CHECK: SYMBOL TABLE:
+# CHECK: 00027ff0   .got    00000000 .hidden _gp
+# CHECK: 00010000   .text   00000000 foo