Do not patch FDE symbols in RuntimeDyld, on targets that use non-absolute symbol relocations in `.eh_frame`

[topolarity]

Summary

Since processFDE adds a delta to the values in the FDE, it assumes that the relocations for the .eh_frame section have not been applied by RuntimeDyld. It expects instead that only the relocation addend has been written to the symbol locations, and that the section-to-section offset needs to be added.

However, there are platform differences that interfere with this:

1. X86-64 has DwarfFDESymbolsUseAbsDiff enabled in its AsmInfo, causing an absolute symbol to be emitted for the FDE pcStart. Absolute symbols are skipped as a relocation by RuntimeDyld, so the processFDE function in RuntimeDyldMachO.cpp calculates the relocation correctly.
2. AArch64 has DwarfFDESymbolsUseAbsDiff disabled, so a relocation is emitted in the eh_frame section. Since this isn't absolute, the relocation is applied by RuntimeDyld. This means that processFDE ends up adding an additional section-to-section offset to the pcStart field, generating an incorrect FDE

[topolarity]

With linting this time

[topolarity]

I also wanted to mention that there's one more barrier to getting stack unwinding working on AArch64 for JIT'd functions:

We need to force the emission of the EH Frame section (currently done via SupportsCompactUnwindWithoutEHFrame in the MCObjectFileInfo for the target), since libunwind doesn't yet support dynamically registering compact unwind information at run-time. For now I've been doing this with a simple patch to MCObjectFileInfo.cpp, commenting out the logic that enables the flag on AArch64. However, for typical JIT users, it'd probably be preferable to configure this correctly at run-time, without the compile-time patching.

Is there precedent for updating these kinds of flags/settings during the JIT setup? If so I'm happy to include that in this change, as well.

[lhames]

The logic looks good to me. Normally I'd ask for a test case but we don't have a good way to test this at the moment, and I don't think we want to invest in RuntimeDyld testing infrastructure for MachO -- we just want to make sure this works under JITLink instead.

Speaking of which -- did you happen to check whether your test cases worked under JITLink?

• Lang.

[topolarity]

In D103052#2846623, @lhames wrote:

did you happen to check whether your test cases worked under JITLink?

I checked this out briefly, but I hit a different issue for JITLink.

For some reason the .eh_frame section is empty when the EHFrameRegistrar is notified of the emitted object (EHFrameSupport.cpp:796-800), so no frames end up getting registered. When I did some debugging, it looked like the compact_unwind section is empty at that point too, so something may be preventing debug frames from being generated but I'm not sure what.

I'll try to dig further and see if I can find the root cause, but it sounds like that would be a separate fix, along with the open mentioned above about how to support forcing EH Frame generation in the JIT, at least until libunwind supports dynamic registration of compact_unwind frames.

[Hahnfeld]

Any updates on this, do you plan to eventually land this change? FWIW this solves one third of https://github.com/llvm/llvm-project/issues/49036

[topolarity]

Rebasing onto trunk.

llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldMachO.h

struct EHFrameRelatedSections {
 
     EHFrameRelatedSections(SID EH, SID T, SID Ex)
         : EHFrameSID(EH), TextSID(T), ExceptTabSID(Ex) {}
     SID EHFrameSID;
     SID TextSID;
     SID ExceptTabSID;
   };
 
+  // Returns true if the FDE section includes absolute symbol relocations
+  // on this platform.
+  virtual bool doDwarfFDESymbolsUseAbsDiff() = 0;
+
   // When a module is loaded we save the SectionID of the EH frame section
   // in a table until we receive a request to register all unregistered
   // EH frame sections with the memory manager.
   SmallVector<EHFrameRelatedSections, 2> UnregisteredEHFrameSections;
 
   RuntimeDyldMachO(RuntimeDyld::MemoryManager &MemMgr,
                    JITSymbolResolver &Resolver)
       : RuntimeDyldImpl(MemMgr, Resolver) {}

 /// Recurring Template Idiom). Concrete subclasses for each target
 /// can be found in ./Targets.
 template <typename Impl>
 class RuntimeDyldMachOCRTPBase : public RuntimeDyldMachO {
 private:
   Impl &impl() { return static_cast<Impl &>(*this); }
   const Impl &impl() const { return static_cast<const Impl &>(*this); }
 
-  unsigned char *processFDE(uint8_t *P, int64_t DeltaForText,
+  unsigned char *patchFDERelocations(uint8_t *P, int64_t DeltaForText,
                                      int64_t DeltaForEH);
 
 public:
   RuntimeDyldMachOCRTPBase(RuntimeDyld::MemoryManager &MemMgr,
                            JITSymbolResolver &Resolver)
     : RuntimeDyldMachO(MemMgr, Resolver) {}
 
   Error finalizeLoad(const ObjectFile &Obj,
                      ObjSectionToIDMap &SectionMap) override;
   void registerEHFrames() override;
 };
 
 } // end namespace llvm
 
 #undef DEBUG_TYPE
 
 #endif

llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldMachO.cpp

RuntimeDyldMachOCRTPBase<Impl>::finalizeLoad(const ObjectFile &Obj,
   }
   UnregisteredEHFrameSections.push_back(
     EHFrameRelatedSections(EHFrameSID, TextSID, ExceptTabSID));
 
   return Error::success();
 }
 
 template <typename Impl>
-unsigned char *RuntimeDyldMachOCRTPBase<Impl>::processFDE(uint8_t *P,
+unsigned char *RuntimeDyldMachOCRTPBase<Impl>::patchFDERelocations(uint8_t *P,
                                                                    int64_t DeltaForText,
                                                                    int64_t DeltaForEH) {
   typedef typename Impl::TargetPtrT TargetPtrT;
 
   LLVM_DEBUG(dbgs() << "Processing FDE: Delta for text: " << DeltaForText
                     << ", Delta for EH: " << DeltaForEH << "\n");
   uint32_t Length = readBytesUnaligned(P, 4);
   P += 4;
   uint8_t *Ret = P + Length;
   uint32_t Offset = readBytesUnaligned(P, 4);
void RuntimeDyldMachOCRTPBase<Impl>::registerEHFrames() {
 
   for (int i = 0, e = UnregisteredEHFrameSections.size(); i != e; ++i) {
     EHFrameRelatedSections &SectionInfo = UnregisteredEHFrameSections[i];
     if (SectionInfo.EHFrameSID == RTDYLD_INVALID_SECTION_ID ||
         SectionInfo.TextSID == RTDYLD_INVALID_SECTION_ID)
       continue;
     SectionEntry *Text = &Sections[SectionInfo.TextSID];
     SectionEntry *EHFrame = &Sections[SectionInfo.EHFrameSID];
+
+    // If the FDE includes absolute symbol relocations (not supported
+    // by RuntimeDyld), we need to manually patch-up the values
+    if (doDwarfFDESymbolsUseAbsDiff()) {
       SectionEntry *ExceptTab = nullptr;
       if (SectionInfo.ExceptTabSID != RTDYLD_INVALID_SECTION_ID)
         ExceptTab = &Sections[SectionInfo.ExceptTabSID];
 
       int64_t DeltaForText = computeDelta(Text, EHFrame);
       int64_t DeltaForEH = 0;
       if (ExceptTab)
         DeltaForEH = computeDelta(ExceptTab, EHFrame);
 
       uint8_t *P = EHFrame->getAddress();
       uint8_t *End = P + EHFrame->getSize();
       while (P != End) {
-      P = processFDE(P, DeltaForText, DeltaForEH);
+        P = patchFDERelocations(P, DeltaForText, DeltaForEH);
+      }
     }
 
     MemMgr.registerEHFrames(EHFrame->getAddress(), EHFrame->getLoadAddress(),
                             EHFrame->getSize());
   }
   UnregisteredEHFrameSections.clear();
 }
 

llvm/lib/ExecutionEngine/RuntimeDyld/Targets/RuntimeDyldMachOAArch64.h

public:
   RuntimeDyldMachOAArch64(RuntimeDyld::MemoryManager &MM,
                           JITSymbolResolver &Resolver)
       : RuntimeDyldMachOCRTPBase(MM, Resolver) {}
 
   unsigned getMaxStubSize() const override { return 8; }
 
   unsigned getStubAlignment() override { return 8; }
 
+  bool doDwarfFDESymbolsUseAbsDiff() override { return false; }
+  
   /// Extract the addend encoded in the instruction / memory location.
   Expected<int64_t> decodeAddend(const RelocationEntry &RE) const {
     const SectionEntry &Section = Sections[RE.SectionID];
     uint8_t *LocalAddress = Section.getAddressWithOffset(RE.Offset);
     unsigned NumBytes = 1 << RE.Size;
     int64_t Addend = 0;
     // Verify that the relocation has the correct size and alignment.
     switch (RE.RelType) {

llvm/lib/ExecutionEngine/RuntimeDyld/Targets/RuntimeDyldMachOARM.h

public:
   RuntimeDyldMachOARM(RuntimeDyld::MemoryManager &MM,
                       JITSymbolResolver &Resolver)
     : RuntimeDyldMachOCRTPBase(MM, Resolver) {}
 
   unsigned getMaxStubSize() const override { return 8; }
 
   unsigned getStubAlignment() override { return 4; }
 
+  bool doDwarfFDESymbolsUseAbsDiff() override { return false; }
+
   Expected<JITSymbolFlags> getJITSymbolFlags(const SymbolRef &SR) override {
     auto Flags = RuntimeDyldImpl::getJITSymbolFlags(SR);
     if (!Flags)
       return Flags.takeError();
     Flags->getTargetFlags() = ARMJITSymbolFlags::fromObjectSymbol(SR);
     return Flags;
   }
 

llvm/lib/ExecutionEngine/RuntimeDyld/Targets/RuntimeDyldMachOI386.h

public:
   RuntimeDyldMachOI386(RuntimeDyld::MemoryManager &MM,
                        JITSymbolResolver &Resolver)
       : RuntimeDyldMachOCRTPBase(MM, Resolver) {}
 
   unsigned getMaxStubSize() const override { return 0; }
 
   unsigned getStubAlignment() override { return 1; }
 
+  bool doDwarfFDESymbolsUseAbsDiff() override { return true; }
+
   Expected<relocation_iterator>
   processRelocationRef(unsigned SectionID, relocation_iterator RelI,
                        const ObjectFile &BaseObjT,
                        ObjSectionToIDMap &ObjSectionToID,
                        StubMap &Stubs) override {
     const MachOObjectFile &Obj =
         static_cast<const MachOObjectFile &>(BaseObjT);
     MachO::any_relocation_info RelInfo =

llvm/lib/ExecutionEngine/RuntimeDyld/Targets/RuntimeDyldMachOX86_64.h

public:
   RuntimeDyldMachOX86_64(RuntimeDyld::MemoryManager &MM,
                          JITSymbolResolver &Resolver)
       : RuntimeDyldMachOCRTPBase(MM, Resolver) {}
 
   unsigned getMaxStubSize() const override { return 8; }
 
   unsigned getStubAlignment() override { return 8; }
 
+  bool doDwarfFDESymbolsUseAbsDiff() override { return true; }
+
   Expected<relocation_iterator>
   processRelocationRef(unsigned SectionID, relocation_iterator RelI,
                        const ObjectFile &BaseObjT,
                        ObjSectionToIDMap &ObjSectionToID,
                        StubMap &Stubs) override {
     const MachOObjectFile &Obj =
       static_cast<const MachOObjectFile &>(BaseObjT);
     MachO::any_relocation_info RelInfo =