[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 ↗

(On Diff #58571)

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.

lld/trunk/ELF/Relocations.cpp

 
 static bool refersToGotEntry(RelExpr Expr) {
   return Expr == R_GOT || Expr == R_GOT_OFF || Expr == R_MIPS_GOT_LOCAL ||
          Expr == R_MIPS_GOT_LOCAL_PAGE || Expr == R_GOT_PAGE_PC ||
          Expr == R_GOT_PC || Expr == R_GOT_FROM_END || Expr == R_TLSGD ||
          Expr == R_TLSGD_PC;
 }
 
+static bool isPreemptible(const SymbolBody &Body, uint32_t Type) {
+  // 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. There are two two MIPS GP-relative
+  // relocations R_MIPS_GPREL16 and R_MIPS_GPREL32. But only R_MIPS_GPREL16
+  // can be against a preemptible symbol.
+  // To get MIPS relocation type we apply 0xf mask. In case of O32 ABI all
+  // relocation types occupy eight bit. In case of N64 ABI we extract first
+  // relocation from 3-in-1 packet because only the first relocation can
+  // be against a real symbol.
+  if (Config->EMachine == EM_MIPS && (Type & 0xf) == R_MIPS_GPREL16)
+    return false;
+  return Body.isPreemptible();
+}
+
 // Returns the number of relocations processed.
 template <class ELFT>
 static unsigned handleTlsRelocation(uint32_t Type, SymbolBody &Body,
                                     InputSectionBase<ELFT> &C,
                                     typename ELFT::uint Offset,
                                     typename ELFT::uint Addend, RelExpr Expr) {
   if (!(C.getSectionHdr()->sh_flags & SHF_ALLOC))
     return 0;
if (Config->Shared) {
                                       &Body, 0});
       }
       C.Relocations.push_back({Expr, Type, Offset, Addend, &Body});
       return 1;
     }
 
     // Global-Dynamic relocs can be relaxed to Initial-Exec or Local-Exec
     // depending on the symbol being locally defined or not.
-    if (Body.isPreemptible()) {
+    if (isPreemptible(Body, Type)) {
       C.Relocations.push_back(
           {R_RELAX_TLS_GD_TO_IE, Type, Offset, Addend, &Body});
       if (!Body.isInGot()) {
         Out<ELFT>::Got->addEntry(Body);
         Out<ELFT>::RelaDyn->addReloc({Target->TlsGotRel, Out<ELFT>::Got,
                                       Body.getGotOffset<ELFT>(), false, &Body,
                                       0});
       }
       return 2;
     }
     C.Relocations.push_back(
         {R_RELAX_TLS_GD_TO_LE, Type, Offset, Addend, &Body});
     return Target->TlsGdToLeSkip;
   }
 
   // Initial-Exec relocs can be relaxed to Local-Exec if the symbol is locally
   // defined.
   if (Target->isTlsInitialExecRel(Type) && !Config->Shared &&
-      !Body.isPreemptible()) {
+      !isPreemptible(Body, Type)) {
     C.Relocations.push_back(
         {R_RELAX_TLS_IE_TO_LE, Type, Offset, Addend, &Body});
     return 1;
   }
   return 0;
 }
 
 // Some targets might require creation of thunks for relocations. Now we
if (E == R_SIZE || E == R_GOT_FROM_END || E == R_GOT_OFF ||
       E == R_TLSGD || E == R_PPC_PLT_OPD)
     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())
+  if (isPreemptible(Body, Type))
     return false;
 
   if (!Config->Pic)
     return true;
 
   bool AbsVal = isAbsolute<ELFT>(Body) || Body.isTls();
   bool RelE = isRelExpr(E);
   if (AbsVal && !RelE)

 }
 
 template <class ELFT>
 static RelExpr adjustExpr(const elf::ObjectFile<ELFT> &File, SymbolBody &Body,
                           bool IsWrite, RelExpr Expr, uint32_t Type,
                           const uint8_t *Data, typename ELFT::uint Offset) {
   if (Target->needsThunk(Type, File, Body))
     return R_THUNK;
-  bool Preemptible = Body.isPreemptible();
+  bool Preemptible = isPreemptible(Body, Type);
   if (Body.isGnuIFunc()) {
     Expr = toPlt(Expr);
   } else if (!Preemptible) {
     if (needsPlt(Expr))
       Expr = fromPlt(Expr);
     if (Expr == R_GOT_PC && Target->canRelaxGot(Type, Data + Offset))
       Expr = R_RELAX_GOT_PC;
   }
for (auto I = Rels.begin(), E = Rels.end(); I != E; ++I) {
     // Ignore "hint" relocation because it is for optional code optimization.
     if (Expr == R_HINT)
       continue;
 
     uintX_t Offset = C.getOffset(RI.r_offset);
     if (Offset == (uintX_t)-1)
       continue;
 
-    bool Preemptible = Body.isPreemptible();
+    bool Preemptible = isPreemptible(Body, Type);
     Expr = adjustExpr(File, Body, IsWrite, Expr, Type, Buf, Offset);
     if (HasError)
       continue;
 
     // This relocation does not require got entry, but it is relative to got and
     // needs it to be created. Here we request for that.
     if (Expr == R_GOTONLY_PC || Expr == R_GOTREL || Expr == R_PPC_TOC)
       Out<ELFT>::Got->HasGotOffRel = true;
 
     uintX_t Addend = computeAddend(File, Buf, E, RI, Expr, Body);
 
     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()) {
+        !isPreemptible(Body, Type)) {
       // 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.

lld/trunk/test/ELF/mips-64-gprel-so.s

+# Check setup of GP relative offsets in a function's prologue.
+
+# RUN: llvm-mc -filetype=obj -triple=mips64-unknown-linux %s -o %t.o
+# RUN: ld.lld %t.o -shared -o %t.so
+# RUN: llvm-objdump -d -t %t.so | FileCheck %s
+
+# REQUIRES: mips
+
+# CHECK:      Disassembly of section .text:
+# CHECK-NEXT: foo:
+# CHECK-NEXT:    10000:    3c 1c 00 01    lui     $gp, 1
+# CHECK-NEXT:    10004:    03 99 e0 2d    daddu   $gp, $gp, $25
+# CHECK-NEXT:    10008:    67 9c 7f f0    daddiu  $gp, $gp, 32752
+
+# CHECK: 0000000000027ff0   .got    00000000 .hidden _gp
+# CHECK: 0000000000010000   .text   00000000 foo
+
+  .text
+  .global foo
+foo:
+  lui     $gp,%hi(%neg(%gp_rel(foo)))
+  daddu   $gp,$gp,$t9
+  daddiu  $gp,$gp,%lo(%neg(%gp_rel(foo)))