Core: Adds more validation around pool buffer ownership

All buffers should be disowned leaving their respective compilation
sites, and reowning a buffer should never have the flag already be
owned.

Throw an assert in both cases because that would be a programming error
and result in some squirrely buffer handling
This commit is contained in:
Ryan Houdek committed 2026-09-03 14:39:01 -07:00
1 parent e010e8adf0
commit a0cb91daa4
5 files changed
+41

No files matched your search

+12
View File
@@ -816,6 +816,8 @@ ContextImpl::CompileCodeResult ContextImpl::CompileCode(FEXCore::Core::InternalT
auto [IRView, TotalInstructions, TotalInstructionsLength, StartAddr, Length, NeedsAddGuestCodeRanges] =
GenerateIR(Thread, GuestRIP, Config.GDBSymbols(), MaxInst);
if (!IRView) {
Thread->FrontendDecoder->ValidateDisownedOrFree();
Thread->OpDispatcher->ValidateDisownedOrFree();
// OpDispatcher IR already released in this case.
return {{}, nullptr, 0, 0, false};
}
@@ -829,6 +831,8 @@ ContextImpl::CompileCodeResult ContextImpl::CompileCode(FEXCore::Core::InternalT
if (auto Block = Thread->LookupCache->FindBlock(Thread, GuestRIP)) {
// Raced to compile, release the OpDispatcher IR.
Thread->OpDispatcher->DelayedDisownBuffer();
Thread->FrontendDecoder->ValidateDisownedOrFree();
Thread->OpDispatcher->ValidateDisownedOrFree();
return {.CompiledCode = {.BlockBegin = reinterpret_cast<uint8_t*>(Block), .EntryPoints = {{GuestRIP, reinterpret_cast<uint8_t*>(Block)}}},
.DebugData = nullptr,
.StartAddr = 0,
@@ -847,6 +851,8 @@ ContextImpl::CompileCodeResult ContextImpl::CompileCode(FEXCore::Core::InternalT
// Release the IR
Thread->OpDispatcher->DelayedDisownBuffer();
Thread->FrontendDecoder->ValidateDisownedOrFree();
Thread->OpDispatcher->ValidateDisownedOrFree();
return {
.CompiledCode = std::move(CompiledCode),
.DebugData = std::move(DebugData),
@@ -921,6 +927,9 @@ uintptr_t ContextImpl::CompileBlock(FEXCore::Core::CpuStateFrame* Frame, uint64_
FEXCORE_PROFILE_INSTANT_INCREMENT(Thread, AccumulatedDiskCacheHitCount, 1);
Thread->FrontendDecoder->DelayedDisownBuffer();
Thread->FrontendDecoder->ValidateDisownedOrFree();
Thread->OpDispatcher->ValidateDisownedOrFree();
return CachedHostCode;
}
}
@@ -1025,6 +1034,9 @@ uintptr_t ContextImpl::CompileBlock(FEXCore::Core::CpuStateFrame* Frame, uint64_
Thread->CPUBackend->ClearRelocations();
}
Thread->FrontendDecoder->ValidateDisownedOrFree();
Thread->OpDispatcher->ValidateDisownedOrFree();
return (uintptr_t)CodePtr;
}
+4
View File
@@ -72,6 +72,10 @@ public:
PoolObject.DelayedDisownBuffer();
}
void ValidateDisownedOrFree() const {
PoolObject.ValidateDisownedOrFree();
}
void ResetExecutableRangeCache() {
ExecutableRangeBase = ExecutableRangeEnd = 0;
}
+4
View File
@@ -36,6 +36,10 @@ public:
DualListData.DelayedDisownBuffer();
}
void ValidateDisownedOrFree() const {
DualListData.ValidateDisownedOrFree();
}
IRListView ViewIR() {
return IRListView(&DualListData);
}
@@ -129,6 +129,10 @@ public:
PoolObject.DelayedDisownBuffer();
}
void ValidateDisownedOrFree() const {
PoolObject.ValidateDisownedOrFree();
}
private:
Utils::PoolBufferWithTimedRetirement<uintptr_t, 5000, 500> PoolObject;
};
@@ -162,6 +162,9 @@ public:
std::optional<ContainerType::iterator> TryToReownBuffer(const ContainerType::iterator& Buffer, size_t Size, BufferOwnedFlag* CurrentClientFlag) {
ClientFlags Expected = ClientFlags::FLAG_DISOWNED;
if (!CurrentClientFlag->compare_exchange_strong(Expected, ClientFlags::FLAG_OWNED)) {
#if defined(ASSERTIONS_ENABLED) && ASSERTIONS_ENABLED
LOGMAN_THROW_A_FMT(Expected != IntrusivePooledAllocator::ClientFlags::FLAG_OWNED, "Tried to reown buffer but it was already owned!");
#endif
return std::nullopt;
}
@@ -540,6 +543,20 @@ public:
return ReownOrClaimBufferWithSize(NewSize).Ptr;
}
/**
* @brief Assertion feature to check if current buffer is in a disowned or free state.
*
* Useful to validate that buffers are disowned at the correct places in code.
*/
void ValidateDisownedOrFree() const {
#if defined(ASSERTIONS_ENABLED) && ASSERTIONS_ENABLED
auto OwnedState = ClientOwnedFlag.load();
LOGMAN_THROW_A_FMT(
OwnedState == IntrusivePooledAllocator::ClientFlags::FLAG_DISOWNED || OwnedState == IntrusivePooledAllocator::ClientFlags::FLAG_FREE,
"ThreadPoolAllocator should have been disowned!");
#endif
}
/**
* @brief Disown or unclaim the buffer, letting the `Allocator` know it can reclaim the buffer
*