From 02f8ab5f87e3912a1a115875c0223fbe37a8f388 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Fri, 21 Aug 2026 14:06:12 -0700 Subject: [PATCH] SharedCodeBufferManager: Leak less internal details about implementation The various places that were using the CodeBuffer object were using internal implementation details that are changing as we move over to a bitmap allocator. Preempt this by hiding some of the implementation details early without changing behaviour. `GetBufferBase` is still technically leaking some of the internal details, but it needs changes around how relocations are being handled and how the disk cache validation works in order to handle that right now. Should be no functional change. --- FEXCore/Source/Interface/Core/CPUBackend.cpp | 9 ++--- FEXCore/Source/Interface/Core/CPUBackend.h | 3 +- FEXCore/Source/Interface/Core/CodeCache.cpp | 13 +++---- FEXCore/Source/Interface/Core/Core.cpp | 2 +- FEXCore/Source/Interface/Core/JIT/JIT.cpp | 10 +++--- .../Core/SharedCodeBufferManager.cpp | 2 +- .../Interface/Core/SharedCodeBufferManager.h | 36 ++++++++++++------- 7 files changed, 42 insertions(+), 33 deletions(-) diff --git a/FEXCore/Source/Interface/Core/CPUBackend.cpp b/FEXCore/Source/Interface/Core/CPUBackend.cpp index e7eac0b03..ed77028c6 100644 --- a/FEXCore/Source/Interface/Core/CPUBackend.cpp +++ b/FEXCore/Source/Interface/Core/CPUBackend.cpp @@ -307,7 +307,7 @@ namespace CPU { CPUBackend::~CPUBackend() = default; - auto CPUBackend::GetEmptySharedCodeBuffer() -> CodeBuffer* { + auto CPUBackend::AcquireNewSharedCodeBuffer() -> CodeBuffer* { auto PrevCodeBuffer = CurrentCodeBuffer; // Resize the code buffer and reallocate our code size @@ -339,11 +339,8 @@ namespace CPU { bool CPUBackend::IsAddressInCodeBuffer(uintptr_t Address) const { const auto CheckCodeBuffer = [](const CodeBuffer& Buffer, uintptr_t Address) { - const auto BufferPtr = reinterpret_cast(Buffer.Ptr); - - // The last page of the code buffer is protected, so we need to exclude it from the valid range - // when checking if the address is in the code buffer. - const uintptr_t LastPageAddr = AlignDown(BufferPtr + Buffer.AllocatedSize - 1, FEXCore::Utils::FEX_PAGE_SIZE); + const auto BufferPtr = reinterpret_cast(Buffer.GetBufferBase()); + const uintptr_t LastPageAddr = BufferPtr + Buffer.UsableSize(); return (Address >= BufferPtr && Address < LastPageAddr); }; diff --git a/FEXCore/Source/Interface/Core/CPUBackend.h b/FEXCore/Source/Interface/Core/CPUBackend.h index c97f86917..19d82eac0 100644 --- a/FEXCore/Source/Interface/Core/CPUBackend.h +++ b/FEXCore/Source/Interface/Core/CPUBackend.h @@ -149,8 +149,9 @@ namespace CPU { FEXCore::Core::InternalThreadState* ThreadState; + // Acquires a new shared code buffer, setting `CurrentCodeBuffer` and returning a pointer to it. [[nodiscard]] - CodeBuffer* GetEmptySharedCodeBuffer(); + CodeBuffer* AcquireNewSharedCodeBuffer(); // This is the code buffer containing the main code under execution by this thread. // CheckCodeBufferUpdate must be used before compiling new code. diff --git a/FEXCore/Source/Interface/Core/CodeCache.cpp b/FEXCore/Source/Interface/Core/CodeCache.cpp index 70d67ea4c..f43b9b82c 100644 --- a/FEXCore/Source/Interface/Core/CodeCache.cpp +++ b/FEXCore/Source/Interface/Core/CodeCache.cpp @@ -337,7 +337,7 @@ bool CodeCache::SaveData(Core::InternalThreadState& Thread, int fd, const Execut Guest -= SourceBinary.FileStartVA; ::write(fd, &Guest, sizeof(Guest)); - uint64_t HostCode = Host->HostCode - reinterpret_cast(CodeBuffer->Ptr); + uint64_t HostCode = Host->HostCode - reinterpret_cast(CodeBuffer->GetBufferBase()); ::write(fd, &HostCode, sizeof(HostCode)); uint64_t NumCodePages = Host->CodePages.size(); ::write(fd, &NumCodePages, sizeof(NumCodePages)); @@ -361,8 +361,8 @@ 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->Ptr), - reinterpret_cast(CodeBuffer->Ptr) + CodeBuffer->GetAllocatedSize()); + std::span CodeBufferData(reinterpret_cast(CodeBuffer->GetBufferBase()), + reinterpret_cast(CodeBuffer->GetBufferBase()) + CodeBuffer->GetAllocatedSize()); if (!ApplyCodeRelocations(SerializedBaseAddress, CodeBufferData, Relocations, 0, true)) { LOGMAN_THROW_A_FMT(false, "Failed to apply code relocations"); return false; @@ -427,11 +427,12 @@ void CodeCache::Validate(const ExecutableFileSectionInfo& Section, fextl::set NewCodeBuffer->UsableSize()) { ValidationCTX->ClearCodeCache(ValidationThread.get()); NewCodeBuffer = ValidationCTX->GetLatest(); - LogMan::Msg::IFmt("Increased cache validation code buffer size to {} MiB", NewCodeBuffer->AllocatedSize / 1024 / 1024); + LogMan::Msg::IFmt("Increased cache validation code buffer size to {} MiB", NewCodeBuffer->GetAllocatedSize() / 1024 / 1024); } std::span CodeBufferRangeRef = - std::as_writable_bytes(std::span {NewCodeBuffer->Ptr, NewCodeBuffer->Ptr + NewCodeBuffer->UsableSize()}).subspan(0, CachedCode.size_bytes()); + std::as_writable_bytes(std::span {NewCodeBuffer->GetBufferBase(), NewCodeBuffer->GetBufferBase() + NewCodeBuffer->UsableSize()}) + .subspan(0, CachedCode.size_bytes()); while (!GuestBlocks.empty()) { auto [CompiledBlocks, _, _2, _3, _4] = ValidationCTX->CompileCode(ValidationThread.get(), *GuestBlocks.begin(), 0 /* TODO: Set MaxInst? */); @@ -501,7 +502,7 @@ void CodeCache::Validate(const ExecutableFileSectionInfo& Section, fextl::setLookupCache->ClearCache(ValidationThread->LookupCache->AcquireWriteLock()); - NewCodeBuffer->CodeBufferOffset = NewCodeBuffer->Ptr; + NewCodeBuffer->Reset(); LogMan::Msg::IFmt(" successfully validated cache"); } diff --git a/FEXCore/Source/Interface/Core/Core.cpp b/FEXCore/Source/Interface/Core/Core.cpp index 32ee7e9eb..5e258ff0f 100644 --- a/FEXCore/Source/Interface/Core/Core.cpp +++ b/FEXCore/Source/Interface/Core/Core.cpp @@ -483,7 +483,7 @@ void ContextImpl::LockBeforeFork(FEXCore::Core::InternalThreadState* Thread) { void ContextImpl::OnCodeBufferAllocated(const fextl::shared_ptr& Buffer) { if (Config.GlobalJITNaming()) { - Symbols.RegisterJITSpace(Buffer->Ptr, Buffer->AllocatedSize); + Symbols.RegisterJITSpace(Buffer->GetBufferBase(), Buffer->GetAllocatedSize()); } { diff --git a/FEXCore/Source/Interface/Core/JIT/JIT.cpp b/FEXCore/Source/Interface/Core/JIT/JIT.cpp index 8f509ec9f..82317d9a0 100644 --- a/FEXCore/Source/Interface/Core/JIT/JIT.cpp +++ b/FEXCore/Source/Interface/Core/JIT/JIT.cpp @@ -675,10 +675,8 @@ void Arm64JITCore::ClearCache() { auto PrevCodeBuffer = CurrentCodeBuffer; auto lk = PrevCodeBuffer->LookupCache->AcquireWriteLock(); - auto CodeBuffer = GetEmptySharedCodeBuffer(); - SetBuffer(CodeBuffer->Ptr, CodeBuffer->AllocatedSize); - - ThreadState->LookupCache->ChangeGuestToHostMapping(*PrevCodeBuffer, *CurrentCodeBuffer->LookupCache, lk); + auto CodeBuffer = AcquireNewSharedCodeBuffer(); + ThreadState->LookupCache->ChangeGuestToHostMapping(*PrevCodeBuffer, *CodeBuffer->LookupCache, lk); } Arm64JITCore::~Arm64JITCore() {} @@ -1117,7 +1115,7 @@ CPUBackend::CompiledCode Arm64JITCore::CompileCode(uint64_t Entry, uint64_t Size EntryPoint.second += Delta; } CodeBegin += Delta; - CodeData.HostCodeOffset = CodeData.BlockBegin - CurrentCodeBuffer->Ptr; + 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. @@ -1179,7 +1177,7 @@ CPUBackend::CompiledCode Arm64JITCore::LoadCachedCode(std::span H CPUBackend::CompiledCode Result; Result.BlockBegin = Dest; Result.Size = HostBytes.size(); - Result.HostCodeOffset = Dest - CurrentCodeBuffer->Ptr; + Result.HostCodeOffset = Dest - CurrentCodeBuffer->GetBufferBase(); for (const auto& Ep : EntryPoints) { Result.EntryPoints[Ep.GuestRIP] = Dest + Ep.HostOffset; } diff --git a/FEXCore/Source/Interface/Core/SharedCodeBufferManager.cpp b/FEXCore/Source/Interface/Core/SharedCodeBufferManager.cpp index 78d6108d6..5f8552c90 100644 --- a/FEXCore/Source/Interface/Core/SharedCodeBufferManager.cpp +++ b/FEXCore/Source/Interface/Core/SharedCodeBufferManager.cpp @@ -88,7 +88,7 @@ fextl::shared_ptr SharedCodeBufferManager::StartLargerCodeBuffer() { return GetLatest(); } - auto NewCodeBufferSize = GetLatest()->AllocatedSize; + auto NewCodeBufferSize = GetLatest()->GetAllocatedSize(); NewCodeBufferSize = std::min(NewCodeBufferSize * 2, MAX_CODE_SIZE); return AllocateNew(NewCodeBufferSize); } diff --git a/FEXCore/Source/Interface/Core/SharedCodeBufferManager.h b/FEXCore/Source/Interface/Core/SharedCodeBufferManager.h index f81196cfb..ebfaa1a73 100644 --- a/FEXCore/Source/Interface/Core/SharedCodeBufferManager.h +++ b/FEXCore/Source/Interface/Core/SharedCodeBufferManager.h @@ -21,13 +21,6 @@ struct GuestToHostMap; namespace FEXCore::CPU { struct CodeBuffer { - uint8_t* Ptr; - uint8_t* CodeBufferEnd; - size_t AllocatedSize; // including guard page; see UsableSize() - - // Code buffer allocation information. - std::atomic CodeBufferOffset {}; - fextl::unique_ptr LookupCache; CodeBuffer(size_t Size); @@ -38,11 +31,6 @@ struct CodeBuffer { ~CodeBuffer(); - /// Returns the number of bytes available for storing code - size_t UsableSize() const { - return AllocatedSize - FEXCore::Utils::FEX_PAGE_SIZE; - } - // Atomically allocate a fixed size buffer out of the current allocated codebuffer. // Lockless because it's just a linear allocator. struct CodeBufferAllocation { @@ -78,9 +66,33 @@ struct CodeBuffer { }; } + // Returns the total number of bytes available for storing code + size_t UsableSize() const { + return AllocatedSize - FEXCore::Utils::FEX_PAGE_SIZE; + } + + // Returns the num of bytes currently allocated from the allocator. size_t GetAllocatedSize() const { return CodeBufferOffset - Ptr; } + + // Trivially reset the allocator. + void Reset() { + CodeBufferOffset = Ptr; + } + + // Returns the base of the buffer. + uint8_t* GetBufferBase() const { + return Ptr; + } + +private: + uint8_t* Ptr; + uint8_t* CodeBufferEnd; + size_t AllocatedSize; // including guard page; see UsableSize() + + // Code buffer allocation information. + std::atomic CodeBufferOffset {}; }; /**