Merge pull request #5837 from Sonicadvance1/206

SharedCodeBufferManager: Leak less internal details about implementation

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<uintptr_t>(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<uintptr_t>(Buffer.GetBufferBase());
+      const uintptr_t LastPageAddr = BufferPtr + Buffer.UsableSize();
       return (Address >= BufferPtr && Address < LastPageAddr);
     };
 

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.

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<uintptr_t>(CodeBuffer->Ptr);
+      uint64_t HostCode = Host->HostCode - reinterpret_cast<uintptr_t>(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<std::byte*>(CodeBuffer->Ptr),
-                           reinterpret_cast<std::byte*>(CodeBuffer->Ptr) + CodeBuffer->GetAllocatedSize());
+  std::span CodeBufferData(reinterpret_cast<std::byte*>(CodeBuffer->GetBufferBase()),
+                           reinterpret_cast<std::byte*>(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<ui
   while (CachedCode.size_bytes() > 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<std::byte> 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::set<ui
 
   // Reset Context state for next validation
   ValidationThread->LookupCache->ClearCache(ValidationThread->LookupCache->AcquireWriteLock());
-  NewCodeBuffer->CodeBufferOffset = NewCodeBuffer->Ptr;
+  NewCodeBuffer->Reset();
 
   LogMan::Msg::IFmt("            successfully validated cache");
 }

FEXCore/Source/Interface/Core/Core.cpp

@@ -483,7 +483,7 @@ void ContextImpl::LockBeforeFork(FEXCore::Core::InternalThreadState* Thread) {
 
 void ContextImpl::OnCodeBufferAllocated(const fextl::shared_ptr<CPU::CodeBuffer>& Buffer) {
   if (Config.GlobalJITNaming()) {
-    Symbols.RegisterJITSpace(Buffer->Ptr, Buffer->AllocatedSize);
+    Symbols.RegisterJITSpace(Buffer->GetBufferBase(), Buffer->GetAllocatedSize());
   }
 
   {

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<const uint8_t> 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;
   }

FEXCore/Source/Interface/Core/SharedCodeBufferManager.cpp

@@ -88,7 +88,7 @@ fextl::shared_ptr<CodeBuffer> SharedCodeBufferManager::StartLargerCodeBuffer() {
     return GetLatest();
   }
 
-  auto NewCodeBufferSize = GetLatest()->AllocatedSize;
+  auto NewCodeBufferSize = GetLatest()->GetAllocatedSize();
   NewCodeBufferSize = std::min<size_t>(NewCodeBufferSize * 2, MAX_CODE_SIZE);
   return AllocateNew(NewCodeBufferSize);
 }

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<uint8_t*> CodeBufferOffset {};
-
   fextl::unique_ptr<GuestToHostMap> 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<uint8_t*> CodeBufferOffset {};
 };
 
 /**