From ad3939a44f11802fc72d6acfe200210c48f6134f Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Fri, 21 Aug 2026 18:21:14 -0700 Subject: [PATCH] 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 | 4 +--- FEXCore/Source/Interface/Core/CodeCache.cpp | 15 ++++++------ FEXCore/Source/Interface/Core/Core.cpp | 4 ++-- FEXCore/Source/Interface/Core/DiskCache.cpp | 26 ++++----------------- FEXCore/Source/Interface/Core/JIT/JIT.cpp | 8 ------- 5 files changed, 15 insertions(+), 42 deletions(-) diff --git a/FEXCore/Source/Interface/Context/Context.h b/FEXCore/Source/Interface/Context/Context.h index 58807e898..166b0ee8c 100644 --- a/FEXCore/Source/Interface/Context/Context.h +++ b/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 Code, std::span Relocations, - uint32_t RelocationOffset, bool ForStorage); + bool ApplyCodeRelocations(uint64_t GuestDelta, std::span Code, std::span Relocations, bool ForStorage); }; class ContextImpl final : public FEXCore::Context::Context, public CPU::SharedCodeBufferManager { diff --git a/FEXCore/Source/Interface/Core/CodeCache.cpp b/FEXCore/Source/Interface/Core/CodeCache.cpp index c19872c6e..e7b46ae82 100644 --- a/FEXCore/Source/Interface/Core/CodeCache.cpp +++ b/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(CodeBuffer->GetBufferBase()), reinterpret_cast(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::setAllocatedSpaceUsed() <= 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 Code, - std::span EntryRelocations, uint32_t RelocationOffset, bool ForStorage) { + std::span 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(StartOffset), false); + (void)ApplyCodeRelocations(Code.GuestBase, StagingSpan, PageRelocations, false); Code.LoadedPages[i] = true; } @@ -896,7 +895,7 @@ void CodeCache::FinalizeCodePages(MappedCodeCacheFile& Code, std::spanHostCode), 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 Relocations; if (DebugData && DebugData->Relocations) { Relocations = *DebugData->Relocations; diff --git a/FEXCore/Source/Interface/Core/DiskCache.cpp b/FEXCore/Source/Interface/Core/DiskCache.cpp index ac3623c95..5177acbf2 100644 --- a/FEXCore/Source/Interface/Core/DiskCache.cpp +++ b/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 \ No newline at end of file +} // namespace FEXCore diff --git a/FEXCore/Source/Interface/Core/JIT/JIT.cpp b/FEXCore/Source/Interface/Core/JIT/JIT.cpp index 23a92406d..46cc17902 100644 --- a/FEXCore/Source/Interface/Core/JIT/JIT.cpp +++ b/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); }