RuntimeDyldELF: do not create thunk when jump target is in the same object file and is reachable with R_AARCH64_CALL26.

[evgeny777]

Summary

I'm porting LLVM jit to a custom kernel where mmap is disabled and the only way to inject executable code in the running process is to generate and load DSO. This makes it problematic to have absolute jumps with source and target in the same object file, because ASLR is being used and I only know DSO load address after call to dlopen.

To my understanding there is no need to use absolute jumps, when relative can do the job. Also they are faster and require less number of relocs to be resolved.

All LLVM tests pass. I didn't create extra unit tests, because I'd like to know if this is correct way of doing things first.

[davide]

Can you please add a testcase? That would help me understand better the patch.
Also, probably Lang should take a look (cc:ed).

lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp

952–954 ↗

(On Diff #82477)

please add a comment explaining what this function does.

964–966 ↗

(On Diff #82477)

prefer C++-style casts to C-style casts, thanks.

968 ↗

(On Diff #82477)

ditto.

973–975 ↗

(On Diff #82477)

28 is a little bit a magic'ish number, I'm aware where it comes from but it's non-obvious why it's there. can you please elaborate in the previous comment?

[lhames]

As davide mentioned, there should be a test case for this.

Out of interest, where does it currently use absolute jumps (I don't follow RuntimeDyldELF closely)? Can we / should we change it to uniformly use relative jumps?

[evgeny777]

Thanks for looking at it. I've added test case and addressed review comments from Davide.

Currently RuntimeDyldELF emits absolute jump for every relocation in object file. This works for most targets, though being slightly inefficient. Unfortunately this is completely broken for me (see summary for this patch).

[davide]

I have no additional comments but Lang should sign-off before this goes in.

[evgeny777]

Lang, do you have any comments on this?

[lhames]

Apologies for the delayed reply. This looks good to me.

llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h

void resolveX86_64Relocation(const SectionEntry &Section, uint64_t Offset,
                                uint64_t SymOffset);
 
   void resolveX86Relocation(const SectionEntry &Section, uint64_t Offset,
                             uint32_t Value, uint32_t Type, int32_t Addend);
 
   void resolveAArch64Relocation(const SectionEntry &Section, uint64_t Offset,
                                 uint64_t Value, uint32_t Type, int64_t Addend);
 
+  bool resolveAArch64ShortBranch(unsigned SectionID, relocation_iterator RelI,
+                                 const RelocationValueRef &Value);
+
   void resolveARMRelocation(const SectionEntry &Section, uint64_t Offset,
                             uint32_t Value, uint32_t Type, int32_t Addend);
 
   void resolvePPC32Relocation(const SectionEntry &Section, uint64_t Offset,
                               uint64_t Value, uint32_t Type, int64_t Addend);
 
   void resolvePPC64Relocation(const SectionEntry &Section, uint64_t Offset,
                               uint64_t Value, uint32_t Type, int64_t Addend);

llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp

uint32_t RuntimeDyldELF::getMatchingLoRelocation(uint32_t RelType,
   case ELF::R_MIPS_PCHI16:
     return ELF::R_MIPS_PCLO16;
   default:
     break;
   }
   return ELF::R_MIPS_NONE;
 }
 
+// Sometimes we don't need to create thunk for a branch.
+// This typically happens when branch target is located 
+// in the same object file. In such case target is either
+// a weak symbol or symbol in a different executable section.
+// This function checks if branch target is located in the
+// same object file and if distance between source and target
+// fits R_AARCH64_CALL26 relocation. If both conditions are
+// met, it emits direct jump to the target and returns true.
+// Otherwise false is returned and thunk is created.
+bool RuntimeDyldELF::resolveAArch64ShortBranch(
+    unsigned SectionID, relocation_iterator RelI,
+    const RelocationValueRef &Value) {
+  uint64_t Address;
+  if (Value.SymbolName) {
+    auto Loc = GlobalSymbolTable.find(Value.SymbolName);
+
+    // Don't create direct branch for external symbols.
+    if (Loc == GlobalSymbolTable.end())
+      return false;
+
+    const auto &SymInfo = Loc->second;
+    Address = reinterpret_cast<uint64_t>(
+        Sections[SymInfo.getSectionID()].getLoadAddressWithOffset(
+            SymInfo.getOffset()));
+  } else {
+    Address =
+        reinterpret_cast<uint64_t>(Sections[Value.SectionID].getLoadAddress());
+  }
+  uint64_t Offset = RelI->getOffset();
+  uint64_t SourceAddress = Sections[SectionID].getLoadAddressWithOffset(Offset);
+
+  // R_AARCH64_CALL26 requires immediate to be in range -2^27 <= imm < 2^27
+  // If distance between source and target is out of range then we should
+  // create thunk.
+  if (!isInt<28>(Address + Value.Addend - SourceAddress))
+    return false;
+
+  resolveRelocation(Sections[SectionID], Offset, Address, RelI->getType(),
+                    Value.Addend);
+
+  return true;
+}
+
 Expected<relocation_iterator>
 RuntimeDyldELF::processRelocationRef(
     unsigned SectionID, relocation_iterator RelI, const ObjectFile &O,
     ObjSectionToIDMap &ObjSectionToID, StubMap &Stubs) {
   const auto &Obj = cast<ELFObjectFileBase>(O);
   uint64_t RelType = RelI->getType();
   ErrorOr<int64_t> AddendOrErr = ELFRelocationRef(*RelI).getAddend();
   int64_t Addend = AddendOrErr ? *AddendOrErr : 0;
if ((Arch == Triple::aarch64 || Arch == Triple::aarch64_be) &&
 
     // Look for an existing stub.
     StubMap::const_iterator i = Stubs.find(Value);
     if (i != Stubs.end()) {
       resolveRelocation(Section, Offset,
                         (uint64_t)Section.getAddressWithOffset(i->second),
                         RelType, 0);
       DEBUG(dbgs() << " Stub function found\n");
-    } else {
+    } else if (!resolveAArch64ShortBranch(SectionID, RelI, Value)) {
       // Create a new stub function.
       DEBUG(dbgs() << " Create a new stub function\n");
       Stubs[Value] = Section.getStubOffset();
       uint8_t *StubTargetAddr = createStubFunction(
           Section.getAddressWithOffset(Section.getStubOffset()));
 
       RelocationEntry REmovz_g3(SectionID,
                                 StubTargetAddr - Section.getAddress(),

llvm/trunk/test/ExecutionEngine/RuntimeDyld/AArch64/ELF_ARM64_local_branch.s

+# RUN: llvm-mc -triple=arm64-none-linux-gnu -filetype=obj -o %T/branch.o %s
+# RUN: llvm-rtdyld -triple=arm64-none-linux-gnu -verify -check=%s %T/branch.o
+
+.globl _main
+.weak _label1
+
+.section .text.1,"ax"
+_label1:
+  nop
+_main:
+  b _label1
+
+## Branch 1 instruction back from _main
+# rtdyld-check: *{4}(_main) = 0x17ffffff