Merge pull request #5359 from Sonicadvance1/107

OpcodeDispatcher: Fixes memory leak in IREmitter pool allocator

FEXCore/Source/Interface/Core/Core.cpp

@@ -506,7 +506,6 @@ ContextImpl::GenerateIRResult
 ContextImpl::GenerateIR(FEXCore::Core::InternalThreadState* Thread, uint64_t GuestRIP, bool ExtendedDebugInfo, uint64_t MaxInst) {
   FEXCORE_PROFILE_SCOPED("GenerateIR");
 
-  Thread->OpDispatcher->ReownOrClaimBuffer();
   Thread->OpDispatcher->ResetWorkingList();
 
   uint64_t TotalInstructions {0};
@@ -706,8 +705,8 @@ ContextImpl::GenerateIR(FEXCore::Core::InternalThreadState* Thread, uint64_t Gue
         // If we had a dispatch error then leave early
         if (HadDispatchError && TotalInstructions == 0) {
           // Couldn't handle any instruction in op dispatcher
-          Thread->OpDispatcher->ResetWorkingList();
-          return {{}, 0, 0, 0, 0};
+          Thread->OpDispatcher->DelayedDisownBuffer();
+          return {std::nullopt, 0, 0, 0, 0};
         }
 
         if (NeedsBlockEnd) {
@@ -774,6 +773,7 @@ ContextImpl::CompileCodeResult ContextImpl::CompileCode(FEXCore::Core::InternalT
   auto [IRView, TotalInstructions, TotalInstructionsLength, StartAddr, Length, NeedsAddGuestCodeRanges] =
     GenerateIR(Thread, GuestRIP, Config.GDBSymbols(), MaxInst);
   if (!IRView) {
+    // OpDispatcher IR already released in this case.
     return {{}, nullptr, 0, 0, false};
   }
 
@@ -784,6 +784,7 @@ ContextImpl::CompileCodeResult ContextImpl::CompileCode(FEXCore::Core::InternalT
   // as expensive and are easily reverted.
   if (MaxInst != 1) {
     if (auto Block = Thread->LookupCache->FindBlock(Thread, GuestRIP)) {
+      // Raced to compile, release the OpDispatcher IR.
       Thread->OpDispatcher->DelayedDisownBuffer();
       return {.CompiledCode = {.BlockBegin = reinterpret_cast<uint8_t*>(Block), .EntryPoints = {{GuestRIP, reinterpret_cast<uint8_t*>(Block)}}},
               .DebugData = nullptr,

FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp

@@ -4487,8 +4487,6 @@ void OpDispatchBuilder::StoreResult(RegClass Class, X86Tables::DecodedOp Op, Ref
 OpDispatchBuilder::OpDispatchBuilder(FEXCore::Context::ContextImpl* ctx)
   : IREmitter {ctx->OpDispatcherAllocator, ctx->HostFeatures.SupportsTSOImm9}
   , CTX {ctx} {
-  ResetWorkingList();
-
   if (CTX->HostFeatures.SupportsAVX && CTX->HostFeatures.SupportsSVE256) {
     SaveAVXStateFunc = &OpDispatchBuilder::SaveAVXState;
     RestoreAVXStateFunc = &OpDispatchBuilder::RestoreAVXState;
@@ -4501,7 +4499,8 @@ OpDispatchBuilder::OpDispatchBuilder(FEXCore::Context::ContextImpl* ctx)
 }
 
 void OpDispatchBuilder::ResetWorkingList() {
-  IREmitter::ResetWorkingList();
+  IREmitter::ReownOrClaimBuffer();
+
   JumpTargets.clear();
   BlockSetRIP = false;
   DecodeFailure = false;

FEXCore/Source/Interface/Core/OpcodeDispatcher.h

@@ -305,7 +305,9 @@ public:
 
   OpDispatchBuilder(FEXCore::Context::ContextImpl* ctx);
 
+  // Should only be called at the start of IR Emission.
   void ResetWorkingList();
+
   void ResetDecodeFailure() {
     NeedsBlockEnd = DecodeFailure = false;
   }

FEXCore/Source/Interface/IR/IREmitter.h

@@ -21,15 +21,15 @@ class IREmitter {
 public:
   IREmitter(FEXCore::Utils::IntrusivePooledAllocator& ThreadAllocator, bool SupportsTSOImm9)
     : DualListData {ThreadAllocator, 8 * 1024 * 1024}
-    , SupportsTSOImm9(SupportsTSOImm9) {
-    ReownOrClaimBuffer();
-    ResetWorkingList();
-  }
+    , SupportsTSOImm9(SupportsTSOImm9) {}
 
   virtual ~IREmitter() = default;
 
   void ReownOrClaimBuffer() {
     DualListData.ReownOrClaimBuffer();
+
+    // Reset the working list on new buffer.
+    ResetWorkingList();
   }
 
   void DelayedDisownBuffer() {
@@ -39,7 +39,6 @@ public:
   IRListView ViewIR() {
     return IRListView(&DualListData);
   }
-  void ResetWorkingList();
 
   /**
    * @name IR allocation routines
@@ -512,6 +511,9 @@ protected:
   fextl::vector<Ref> CodeBlocks;
   uint64_t Entry {};
   bool SupportsTSOImm9 {};
+
+private:
+  void ResetWorkingList();
 };
 
 } // namespace FEXCore::IR

FEXCore/Source/Interface/IR/IntrusiveIRList.h

@@ -119,11 +119,7 @@ class DualIntrusiveAllocatorThreadPool final : public DualIntrusiveAllocator {
 public:
   DualIntrusiveAllocatorThreadPool(FEXCore::Utils::IntrusivePooledAllocator& ThreadAllocator, size_t Size)
     : DualIntrusiveAllocator {Size}
-    , PoolObject {ThreadAllocator, Size * 2} {
-    // Claim a buffer on allocation
-    PoolObject.ReownOrClaimBuffer();
-  }
-
+    , PoolObject {ThreadAllocator, Size * 2} {}
   void ReownOrClaimBuffer() {
     Data = PoolObject.ReownOrClaimBuffer();
     List = Data + MemorySize;