[ELF][MIPS] Handle R_MIPS_HI16/LO16 relocations against _gp_disp symbol

[atanasyan]

Summary

The _gp_disp is a magic symbol designates offset between start of function and gp pointer into GOT. Only R_MIPS_HI16 and R_MIPS_LO16 relocations are permitted with _gp_disp. The patch adds the _gp_disp as an ignored symbol and adjusts symbol value before call the relocateOne for R_MIPS_HI16/LO16 relocations.

I put code which adjust symbol value in case of R_MIPS_HI16/LO16 relocations into the InputSectionBase<ELFT>::relocate routine to escape passing SymboBody to each relocateOne methods while it is necessary for MIPS target only.

It is possible to optimize the current solution a bit. We can save a reference to the SymbolBody instance created by the addIgnoredSym call and replace "_gp_disp" string comparison by comparison of SymbolBody references. But I cannot find a good place to keep the reference to the "_gp_disp" SymbolBody.

[ruiu]

ELF/InputSection.cpp

209–212

How about creating a new field, MipsGpDisp of type SymbolBody*, to InputSection class as a public static member to avoid string comparison?

Or, you can modify SymbolTable::addIgnoredSym to return a symbol body which is just added, and assign it to Config or somewhere. And then compare that value's repl() with Body here.

I don't know which is better, or no strong preference.

[atanasyan]

Save SymbolBody for the _gp_disp symbol into the Config::MipsGpDisp field and use the saved pointer instead of string "_gp_disp" comparison.

[ruiu]

LGTM

[grimar]

lld/trunk/ELF/InputSection.cpp

213 ↗

(On Diff #43005)

I thinking about that place last time.
May be we should add A argument to Target->relocateOne() ?
That can help to move that calculation to mips target, since it looks like absence of A inside relocateOne() is the only stoppable for doing that.

[llvm-commits]

I thinking about that place last time.
May be we should add A argument to Target->relocateOne() ?
That can help to move that calculation to mips target, since it looks like absence of A inside relocateOne() is the only stoppable for doing that.

Currently S+A is a single parameter SA. Are you suggesting split that as two parameters, S and A?

I don't know if that makes code simpler, but it may worth trying.

Not sure about splitting. That seems to be more complicated. Just adding A can save from complicating logic outside allowing relocateOne to use it if it is needed for concrete relocation`s code like here for mips.

---

llvm-commits mailing list
[e-mail]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

ELF/Driver.cpp

if (!Config->Shared) {
     // The situation is even stranger on x86_64 where the assembly doesn't
     // need the magical symbol, but gas still puts _GLOBAL_OFFSET_TABLE_ as
     // an undefined symbol in the .o files.
     // Given that the symbol is effectively unused, we just create a dummy
     // hidden one to avoid the undefined symbol error.
     Symtab.addIgnoredSym("_GLOBAL_OFFSET_TABLE_");
   }
 
+  // On MIPS O32 ABI _gp_disp is a magic symbol designates offset between
+  // start of function and gp pointer into GOT.
+  if (Config->EMachine == EM_MIPS)
+    Symtab.addIgnoredSym("_gp_disp");
+
   if (!Config->Entry.empty()) {
     // Set either EntryAddr (if S is a number) or EntrySym (otherwise).
     StringRef S = Config->Entry;
     if (S.getAsInteger(0, Config->EntryAddr))
       Config->EntrySym = Symtab.addUndefined(S);
   }
 
   // Define _gp for MIPS. st_value of _gp symbol will be updated by Writer

ELF/InputSection.cpp

if (Target->relocNeedsPlt(Type, Body)) {
       if (Body.isTLS())
         Type = Target->getTlsGotReloc();
     } else if (!Target->relocNeedsCopy(Type, Body) &&
                isa<SharedSymbol<ELFT>>(Body)) {
       continue;
     } else if (Target->isTlsDynReloc(Type) ||
                Target->isSizeDynReloc(Type, Body)) {
       continue;
+    } else if (Config->EMachine == EM_MIPS) {
+      if (Type == R_MIPS_HI16 && Body.getName() == "_gp_disp")
+        SymVA = getMipsGpAddr<ELFT>() - AddrLoc;
+      else if (Type == R_MIPS_LO16 && Body.getName() == "_gp_disp")
+        SymVA = getMipsGpAddr<ELFT>() - AddrLoc + 4;
     }
     uintX_t A = getAddend<ELFT>(RI);
     uintX_t Size = getSymSize<ELFT>(Body);
     Target->relocateOne(BufLoc, BufEnd, Type, AddrLoc, SymVA + A, Size + A,
                         findMipsPairedReloc(Buf, SymIndex, Type, NextRelocs));
   }
 }
 

test/ELF/mips-hilo-gp-disp.s

+# Check R_MIPS_HI16 / LO16 relocations calculation against _gp_disp.
+
+# RUN: llvm-mc -filetype=obj -triple=mips-unknown-linux %s -o %t1.o
+# RUN: llvm-mc -filetype=obj -triple=mips-unknown-linux \
+# RUN:         %S/Inputs/mips-dynamic.s -o %t2.o
+# RUN: ld.lld %t1.o %t2.o -o %t.exe
+# RUN: llvm-objdump -d -t %t.exe | FileCheck -check-prefix=EXE %s
+# RUN: ld.lld %t1.o %t2.o -shared -o %t.so
+# RUN: llvm-objdump -d -t %t.so | FileCheck -check-prefix=SO %s
+
+# REQUIRES: mips
+
+  .text
+  .globl  __start
+__start:
+  lui    $t0,%hi(_gp_disp)
+  addi   $t0,$t0,%lo(_gp_disp)
+  lw     $v0,%call16(_foo)($gp)
+
+# EXE:      Disassembly of section .text:
+# EXE-NEXT: __start:
+# EXE-NEXT:  20000:   3c 08 00 01   lui    $8, 1
+#                                              ^-- %hi(0x37ff0-0x20000)
+# EXE-NEXT:  20004:   21 08 7f f0   addi   $8, $8, 32752
+#                                                  ^-- %lo(0x37ff0-0x20004+4)
+
+# EXE: SYMBOL TABLE:
+# EXE: 00037ff0     *ABS*   00000000 _gp
+# EXE: 00020000     .text   00000000 __start
+# EXE: 00020010     .text   00000000 _foo
+
+# SO:      Disassembly of section .text:
+# SO-NEXT: __start:
+# SO-NEXT:  10000:   3c 08 00 01   lui    $8, 1
+#                                             ^-- %hi(0x27ff0-0x10000)
+# SO-NEXT:  10004:   21 08 7f f0   addi   $8, $8, 32752
+#                                                 ^-- %lo(0x27ff0-0x10004+4)
+
+# SO: SYMBOL TABLE:
+# SO: 00027ff0     *ABS*   00000000 _gp
+# SO: 00010000     .text   00000000 __start
+# SO: 00010010     .text   00000000 _foo