Avoid double offset relocations

FEX Relocations now live at an offset from the `CodeData.BlockBegin` of the
code. Regardless of where the relocation moves to, it should always be
relative to that address. This is what makes it PIC compatible.

We were preemptively offsetting the relocation location to be relative
to the memory base in the buffer, which is unnecessary and causes code
caching to basically relocate twice to get the real location.

So in JIT.cpp, stop relocating the offsets, they're already relative to
`BlockBegin`, which is offset 0.

Then when storing the relocation, stop relocating offsets AGAIN because it's
already relative to the code being serialized.

Then when loading the relocations in `CodeCache::ApplyCodeRelocations`
stop relocating offsets YET ANOTHER TIME.

All this is to say that relocation offsets are already PIC and relative
to offset 0, so we don't need to do it three times.

FEXCore/Source/Interface/Context/Context.h

@@ -122,14 +122,12 @@ public:
    * Note that FEX relocations are unrelated to ELF/PE relocations.
    *
    * @param GuestDelta Guest address offset to apply to RIP-relative data
-   * @param RelocationOffset Offset to subtract from relocation target offsets
    * @param ForStorage True for serializing data (producing deterministic output); false for de-serializing it (resolving dynamic symbols)
    *
    * @return Returns true on success
    */
   [[nodiscard]]
-  bool ApplyCodeRelocations(uint64_t GuestDelta, std::span<std::byte> Code, std::span<const CPU::Relocation> Relocations,
-                            uint32_t RelocationOffset, bool ForStorage);
+  bool ApplyCodeRelocations(uint64_t GuestDelta, std::span<std::byte> Code, std::span<const CPU::Relocation> Relocations, bool ForStorage);
 };
 
 class ContextImpl final : public FEXCore::Context::Context, public CPU::SharedCodeBufferManager {

FEXCore/Source/Interface/Core/CodeCache.cpp

@@ -363,7 +363,7 @@ bool CodeCache::SaveData(Core::InternalThreadState& Thread, int fd, const Execut
   // Dump the host code (relocated for position-independent serialization)
   std::span CodeBufferData(reinterpret_cast<std::byte*>(CodeBuffer->GetBufferBase()),
                            reinterpret_cast<std::byte*>(CodeBuffer->GetBufferBase()) + CodeBuffer->AllocatedSpaceUsed());
-  if (!ApplyCodeRelocations(SerializedBaseAddress, CodeBufferData, Relocations, 0, true)) {
+  if (!ApplyCodeRelocations(SerializedBaseAddress, CodeBufferData, Relocations, true)) {
     LOGMAN_THROW_A_FMT(false, "Failed to apply code relocations");
     return false;
   }
@@ -446,7 +446,7 @@ void CodeCache::Validate(const ExecutableFileSectionInfo& Section, fextl::set<ui
   NewRelocations.erase(std::remove_if(NewRelocations.begin(), NewRelocations.end(), [](const CPU::Relocation& Reloc) {
     return Reloc.Header.Type != CPU::RelocationTypes::RELOC_NAMED_SYMBOL_LITERAL && Reloc.Header.Type != CPU::RelocationTypes::RELOC_NAMED_THUNK_MOVE;
   }));
-  (void)ApplyCodeRelocations(Section.FileStartVA, CodeBufferRangeRef, NewRelocations, 0, false);
+  (void)ApplyCodeRelocations(Section.FileStartVA, CodeBufferRangeRef, NewRelocations, false);
 
   if (NewCodeBuffer->AllocatedSpaceUsed() <= CodeBufferRangeRef.size()) {
     // Reference compilation produced fewer bytes than our cache, so validation is going to fail.
@@ -508,13 +508,12 @@ void CodeCache::Validate(const ExecutableFileSectionInfo& Section, fextl::set<ui
 }
 
 bool CodeCache::ApplyCodeRelocations(uint64_t GuestEntry, std::span<std::byte> Code,
-                                     std::span<const FEXCore::CPU::Relocation> EntryRelocations, uint32_t RelocationOffset, bool ForStorage) {
+                                     std::span<const FEXCore::CPU::Relocation> EntryRelocations, bool ForStorage) {
   CPU::Arm64Emitter Emitter(&CTX, Code.data(), Code.size_bytes());
   for (size_t j = 0; j < EntryRelocations.size(); ++j) {
     const FEXCore::CPU::Relocation& Reloc = EntryRelocations[j];
-    LOGMAN_THROW_A_FMT(Reloc.Header.Offset >= RelocationOffset, "Invalid relocation offset");
-    LOGMAN_THROW_A_FMT(Reloc.Header.Offset - RelocationOffset < Code.size_bytes(), "Invalid relocation offset");
-    Emitter.SetCursorOffset(Reloc.Header.Offset - RelocationOffset);
+    LOGMAN_THROW_A_FMT(Reloc.Header.Offset < Code.size_bytes(), "Invalid relocation offset");
+    Emitter.SetCursorOffset(Reloc.Header.Offset);
 
     switch (Reloc.Header.Type) {
     case FEXCore::CPU::RelocationTypes::RELOC_NAMED_SYMBOL_LITERAL: {
@@ -873,7 +872,7 @@ void CodeCache::FinalizeCodePages(MappedCodeCacheFile& Code, std::span<std::byte
   auto StagingSpan = std::span {Staging, Size};
   for (size_t i = StartPage; i < EndPage; ++i) {
     auto PageRelocations = SpanPageRelocations(Code, i);
-    (void)ApplyCodeRelocations(Code.GuestBase, StagingSpan, PageRelocations, static_cast<uint32_t>(StartOffset), false);
+    (void)ApplyCodeRelocations(Code.GuestBase, StagingSpan, PageRelocations, false);
     Code.LoadedPages[i] = true;
   }
 
@@ -896,7 +895,7 @@ void CodeCache::FinalizeCodePages(MappedCodeCacheFile& Code, std::span<std::byte
 #endif
   for (size_t i = StartPage; i < EndPage; ++i) {
     auto PageRelocations = SpanPageRelocations(Code, i);
-    (void)ApplyCodeRelocations(Code.GuestBase, Code.CodeBuffer, PageRelocations, 0, false);
+    (void)ApplyCodeRelocations(Code.GuestBase, Code.CodeBuffer, PageRelocations, false);
     Code.LoadedPages[i] = true;
   }
 #endif

FEXCore/Source/Interface/Core/Core.cpp

@@ -868,7 +868,7 @@ uintptr_t ContextImpl::CompileBlock(FEXCore::Core::CpuStateFrame* Frame, uint64_
     FEXCORE_PROFILE_ACCUMULATION(Thread, AccumulatedDiskCacheLookupTime);
     Hit = DiskCache.Lookup(Thread, *Region, GuestRIP);
     if (Hit) {
-      DiskCacheHitRelocationsApplied = CodeCache.ApplyCodeRelocations(GuestRIP, std::as_writable_bytes(Hit->HostCode), Hit->Relocations, 0, false);
+      DiskCacheHitRelocationsApplied = CodeCache.ApplyCodeRelocations(GuestRIP, std::as_writable_bytes(Hit->HostCode), Hit->Relocations, false);
 
       if (DiskCacheHitRelocationsApplied && LoadDiskCacheCode) {
         auto LoadedCode = Thread->CPUBackend->LoadCachedCode(Hit->HostCode, Hit->EntryPoints);
@@ -967,7 +967,7 @@ uintptr_t ContextImpl::CompileBlock(FEXCore::Core::CpuStateFrame* Frame, uint64_
   }
 
   // Disk Cache
-  if (Region && Region->FileStartVA != 0) {
+  if (Region && Region->FileStartVA != 0 && !CodeCache.IsGeneratingCache) {
     std::span<const FEXCore::CPU::Relocation> Relocations;
     if (DebugData && DebugData->Relocations) {
       Relocations = *DebugData->Relocations;

FEXCore/Source/Interface/Core/DiskCache.cpp

@@ -428,10 +428,6 @@ namespace DiskCache {
     return HitData;
   }
 
-  static inline bool IsRelocationInBlock(const FEXCore::CPU::Relocation& Reloc, const CPU::CPUBackend::CompiledCode& CompiledCode) {
-    return Reloc.Header.Offset >= CompiledCode.HostCodeOffset && Reloc.Header.Offset < CompiledCode.HostCodeOffset + CompiledCode.Size;
-  }
-
   struct DiskCache::CacheStoreWorkItem final : WorkQueueThread::WorkItem {
     DiskCache* Self;
     IndexedDB* DB;
@@ -461,9 +457,6 @@ namespace DiskCache {
     // todo what are they exactly? caching those blocks is great when it works, so need to figure this out and make finer-grained if we can
     if (RelocationFilter) {
       for (const auto& Reloc : Relocations) {
-        if (!IsRelocationInBlock(Reloc, CompiledCode)) {
-          continue;
-        }
         if (Reloc.Header.Type != CPU::RelocationTypes::RELOC_GUEST_RIP_LITERAL && Reloc.Header.Type != CPU::RelocationTypes::RELOC_GUEST_RIP_MOVE) {
           continue;
         }
@@ -483,9 +476,6 @@ namespace DiskCache {
     uint32_t ThunkRelocCount = 0;
     for (const auto& Reloc : Relocations) {
       // relocs aren't cleared every time if IsGeneratingCache, so filter just in case
-      if (!IsRelocationInBlock(Reloc, CompiledCode)) {
-        continue;
-      }
       if (Reloc.Header.Type == CPU::RelocationTypes::RELOC_NAMED_THUNK_MOVE) {
         ThunkRelocCount++;
       } else {
@@ -542,18 +532,12 @@ namespace DiskCache {
     uint32_t SmallIdx = 0;
     uint32_t ThunkIdx = 0;
     for (const auto& Reloc : Relocations) {
-      if (!IsRelocationInBlock(Reloc, CompiledCode)) {
-        continue;
-      }
-      // re-relocate :harold:
-      uint32_t LocalOffset = uint32_t(Reloc.Header.Offset - CompiledCode.HostCodeOffset);
-
       switch (Reloc.Header.Type) {
       // it's important to zero-init the element completely so we don't have garbage in unused fields
       // this way, the caches stay deterministic across machines
       case CPU::RelocationTypes::RELOC_NAMED_SYMBOL_LITERAL: {
         BlobSmallRelocation SmallReloc = {};
-        SmallReloc.Offset = LocalOffset;
+        SmallReloc.Offset = Reloc.Header.Offset;
         SmallReloc.Type = uint8_t(Reloc.Header.Type);
         SmallReloc.Named.Symbol = uint32_t(Reloc.NamedSymbolLiteral.Symbol);
         SmallRelocs[SmallIdx++] = SmallReloc;
@@ -561,7 +545,7 @@ namespace DiskCache {
       }
       case CPU::RelocationTypes::RELOC_GUEST_RIP_LITERAL: {
         BlobSmallRelocation SmallReloc = {};
-        SmallReloc.Offset = LocalOffset;
+        SmallReloc.Offset = Reloc.Header.Offset;
         SmallReloc.Type = uint8_t(Reloc.Header.Type);
         SmallReloc.RIPLiteral.GuestRIP = Reloc.GuestRIP.GuestRIP - GuestRIP;
         SmallRelocs[SmallIdx++] = SmallReloc;
@@ -569,7 +553,7 @@ namespace DiskCache {
       }
       case CPU::RelocationTypes::RELOC_GUEST_RIP_MOVE: {
         BlobSmallRelocation SmallReloc = {};
-        SmallReloc.Offset = LocalOffset;
+        SmallReloc.Offset = Reloc.Header.Offset;
         SmallReloc.Type = uint8_t(Reloc.Header.Type);
         SmallReloc.RIPMove.RegisterIndex = Reloc.GuestRIP.RegisterIndex;
         SmallReloc.RIPMove.GuestRIP = Reloc.GuestRIP.GuestRIP - GuestRIP;
@@ -578,7 +562,7 @@ namespace DiskCache {
       }
       case CPU::RelocationTypes::RELOC_NAMED_THUNK_MOVE: {
         BlobThunkRelocation BigReloc = {};
-        BigReloc.Offset = LocalOffset;
+        BigReloc.Offset = Reloc.Header.Offset;
         BigReloc.RegisterIndex = Reloc.NamedThunkMove.RegisterIndex;
         memcpy(BigReloc.SymbolHash, &Reloc.NamedThunkMove.Symbol, sizeof(BigReloc.SymbolHash));
         ThunkRelocs[ThunkIdx++] = BigReloc;
@@ -604,4 +588,4 @@ namespace DiskCache {
 
 } // namespace DiskCache
 
-} // namespace FEXCore
+} // namespace FEXCore

FEXCore/Source/Interface/Core/JIT/JIT.cpp

@@ -1117,14 +1117,6 @@ CPUBackend::CompiledCode Arm64JITCore::CompileCode(uint64_t Entry, uint64_t Size
     CodeBegin += Delta;
     CodeData.HostCodeOffset = CodeData.BlockBegin - CurrentCodeBuffer->GetBufferBase();
 
-    // Offset the relocations based on how far forward they moved from the temp buffer to the new buffer.
-    // TODO: Relocations should instead be relocated based on the block entrypoint instead of the codebuffer base.
-    // This would make this relocation movement here get deleted and then `CodeCache::HandleRelocations` can just handle it.
-    const size_t AllocationOffset = AllocatedInfo.BufferAllocationOffset - AllocatedInfo.BufferBase;
-    for (std::size_t Idx = PrevNumAllocations; Idx != Relocations.size(); ++Idx) {
-      Relocations[Idx].Header.Offset += AllocationOffset;
-    }
-
     // Copy over CodeBuffer contents
     memcpy(AllocatedInfo.BufferAllocationOffset, TempCodeBuffer, CodeData.Size);
   }