From e017894bb415315c4b130cdb3bb920f5f2c52009 Mon Sep 17 00:00:00 2001 From: Chad Date: Sun, 4 Oct 2026 19:08:54 -0500 Subject: [PATCH] fix(agc): free per-thread driver state at thread exit without emutls On MinGW, thread_local storage is emutls: libgcc frees a thread's blocks from a winpthread key destructor when the thread function returns, and the CRT runs the thread_local destructors later, from the module's TLS callback at thread detach. The destructors of the driver's non-trivial thread_locals (the deferred label queue, packet profile, capture and revalidation scratch vectors, build profile, memory counters, disk cache buffers) therefore ran on freed storage. Under the debug heap the flip test failed on every run at the queue worker's exit after the injected flip failure; without it the use-after-free showed up as intermittent heap corruption or a stack overflow on the Windows CI job. ShaderRecompiler::ThreadOwned keeps such state behind a trivially destructible thread_local pointer and deletes it from its own pthread key destructor, which receives the objects directly instead of reading emutls. The driver no longer registers any thread_local destructor. Co-Authored-By: Claude Opus 5.5 --- .../src/Driver/Memory/BufferFill.cpp | 4 +- .../src/Driver/Memory/CaptureValidation.cpp | 19 ++++-- .../src/Driver/Packets/PacketExecution.cpp | 4 +- .../Driver/Synchronization/DeferredLabels.cpp | 8 ++- .../Execution/src/GuestMemory.cpp | 5 +- .../Graphics/src/GuestBufferMemory.cpp | 4 +- .../Graphics/src/ShaderResources.cpp | 49 ++++++++++----- core/shader/recompiler/CMakeLists.txt | 1 + core/shader/recompiler/ShaderDiskCache.cpp | 7 ++- core/shader/recompiler/ThreadOwned.hpp | 59 +++++++++++++++++++ 10 files changed, 128 insertions(+), 32 deletions(-) create mode 100644 core/shader/recompiler/ThreadOwned.hpp diff --git a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Memory/BufferFill.cpp b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Memory/BufferFill.cpp index baa341ee2..2d614b5b4 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Memory/BufferFill.cpp +++ b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Memory/BufferFill.cpp @@ -1,4 +1,5 @@ #include "prx/libSceAgcDriver/Execution/include/Driver/Driver.hpp" +#include "ThreadOwned.hpp" #include "prx/libSceAgcDriver/Execution/include/Driver/Diagnostics.hpp" #include "prx/libSceAgcDriver/Execution/include/GuestMemory.hpp" #include @@ -192,7 +193,8 @@ bool Driver::fillBuffer(QueueState& queue, std::uint32_t queueId, std::spanWaitIdle(); - static thread_local std::vector block; + static thread_local std::vector* blockSlot = nullptr; + auto& block = ShaderRecompiler::ThreadOwned(blockSlot); const auto chunk = std::min(bytes, 1u << 20u); block.resize(chunk); for (std::size_t at = 0; at < chunk; at += 16) std::memcpy(block.data() + at, pattern.data(), 16); diff --git a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Memory/CaptureValidation.cpp b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Memory/CaptureValidation.cpp index 852c8d12c..2eb194129 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Memory/CaptureValidation.cpp +++ b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Memory/CaptureValidation.cpp @@ -1,4 +1,5 @@ #include "prx/libSceAgcDriver/Execution/include/Driver/Driver.hpp" +#include "ThreadOwned.hpp" #include "prx/libSceAgcDriver/Execution/include/GuestMemory.hpp" #include #include @@ -14,7 +15,8 @@ bool Driver::captureStable(std::span captu double waited = 0; PendingView pending; pending.Load(); - thread_local std::vector known; + thread_local std::vector* knownSlot = nullptr; + auto& known = ShaderRecompiler::ThreadOwned(knownSlot); for (const auto& region : captured) { if (!stable) break; known.resize(region.bytes.size()); @@ -58,7 +60,8 @@ bool Driver::captureStable(std::span captu bool Driver::validateCaptured(std::uint64_t program, std::uint32_t queue, std::span captured, const ShaderRecompiler::RecompileResult& compiled, bool inPlace, const PendingView& view, bool* unmapped, std::optional* sampling, const DataMask* data) { using Policy = ShaderMemory::PendingWrite; - thread_local std::vector pending; + thread_local std::vector* pendingSlot = nullptr; + auto& pending = ShaderRecompiler::ThreadOwned(pendingSlot); pending.clear(); for (std::size_t i = 0; i < captured.size(); ++i) { const auto& region = captured[i]; @@ -66,7 +69,8 @@ bool Driver::validateCaptured(std::uint64_t program, std::uint32_t queue, std::s } const auto compareMasked = [&](const ShaderRecompiler::MemoryRegion& region, std::size_t first) { - thread_local std::vector live; + thread_local std::vector* liveSlot = nullptr; + auto& live = ShaderRecompiler::ThreadOwned(liveSlot); live.resize(region.bytes.size()); const auto copied = GuestMemory::CopyMapped(region.guestAddress, live); if (copied != GuestMemory::Compare::Equal) return copied; @@ -84,9 +88,12 @@ bool Driver::validateCaptured(std::uint64_t program, std::uint32_t queue, std::s }; static constexpr std::size_t NoKnownValue = std::numeric_limits::max(); - thread_local std::vector policies; - thread_local std::vector knownOffsets; - thread_local std::vector knownBytes; + thread_local std::vector* policiesSlot = nullptr; + thread_local std::vector* knownOffsetsSlot = nullptr; + thread_local std::vector* knownBytesSlot = nullptr; + auto& policies = ShaderRecompiler::ThreadOwned(policiesSlot); + auto& knownOffsets = ShaderRecompiler::ThreadOwned(knownOffsetsSlot); + auto& knownBytes = ShaderRecompiler::ThreadOwned(knownBytesSlot); policies.assign(pending.size(), Policy::None); knownOffsets.assign(pending.size(), NoKnownValue); knownBytes.clear(); diff --git a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketExecution.cpp b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketExecution.cpp index 3ebdecd90..80355fc7e 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketExecution.cpp +++ b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketExecution.cpp @@ -1,4 +1,5 @@ #include "prx/libSceAgcDriver/Execution/include/Driver/Driver.hpp" +#include "ThreadOwned.hpp" #include "prx/libSceAgcDriver/Execution/include/Driver/Diagnostics.hpp" #include "prx/libSceAgcDriver/Execution/include/Driver/Synchronization/SynchronizationStatistics.hpp" #include "prx/libSceAgcDriver/Execution/include/Driver/Synchronization/DeferredLabels.hpp" @@ -97,7 +98,8 @@ void Driver::execute(const Submission& submission) { static const bool profilePackets = std::getenv("APS5_PROFILE_DRAW") != nullptr; - thread_local PacketProfile packetProfile; + thread_local PacketProfile* packetProfileSlot = nullptr; + auto& packetProfile = ShaderRecompiler::ThreadOwned(packetProfileSlot); ++packetProfile.submissions; bumpEpoch(&EpochBumps::submissions); diff --git a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Synchronization/DeferredLabels.cpp b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Synchronization/DeferredLabels.cpp index e8be75566..395c2ef2e 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Synchronization/DeferredLabels.cpp +++ b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Synchronization/DeferredLabels.cpp @@ -2,13 +2,14 @@ #include "prx/libSceAgcDriver/Execution/include/Driver/Synchronization/DeferredLabels.hpp" #include "prx/libSceAgcDriver/Execution/include/Driver/Synchronization/SynchronizationStatistics.hpp" #include "prx/libSceAgcDriver/Execution/include/GuestMemory.hpp" +#include "ThreadOwned.hpp" #include namespace AgcDriver::DriverDetail { DeferredLabels& deferredLabels() { - static thread_local DeferredLabels deferred; - return deferred; + static thread_local DeferredLabels* deferred = nullptr; + return ShaderRecompiler::ThreadOwned(deferred); } bool DeferLabels() { @@ -91,7 +92,8 @@ void Driver::recordDeferredLabels(VulkanDevice* localDevice, std::uint32_t queue auto& deferred = deferredLabels(); if (deferred.labels.empty()) return; - static thread_local std::vector labels; + static thread_local std::vector* recording = nullptr; + auto& labels = ShaderRecompiler::ThreadOwned(recording); labels.clear(); labels.swap(deferred.labels); struct Clear { diff --git a/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp b/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp index 0af8293e2..291369842 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp +++ b/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp @@ -1,4 +1,5 @@ #include "prx/libSceAgcDriver/Execution/include/GuestMemory.hpp" +#include "ThreadOwned.hpp" #include "prx/libc/include/GuestAllocations.hpp" #include "prx/libc/include/GuestArena.hpp" #include "prx/libc/include/GuestWriteWatch.hpp" @@ -169,8 +170,8 @@ struct ThreadMemoryCounters { MemoryCounter& CounterFor(MemoryCounterKind kind) { if (SharedMemoryCounters()) return Profile().shared.counters[kind]; - thread_local ThreadMemoryCounters thread; - return thread.counters.counters[kind]; + thread_local ThreadMemoryCounters* thread = nullptr; + return ShaderRecompiler::ThreadOwned(thread).counters.counters[kind]; } // Every thread's counters, the retired and the shared ones summed: calls, bytes, nanoseconds per kind. diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp index da3e6332a..9c08d1a48 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp @@ -1,4 +1,5 @@ #include "prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp" +#include "ThreadOwned.hpp" #include "prx/libSceAgcDriver/Graphics/include/BdaResources.hpp" #include "prx/libSceAgcDriver/Graphics/include/Texture.hpp" #include "prx/libSceAgcDriver/Graphics/include/Recorder.hpp" @@ -671,7 +672,8 @@ void refreshMirror(ImageMirror& mirror, std::uint64_t address, std::uint64_t byt void refreshHeapMirrors(std::vector& mirrors, std::vector& blocks) { constexpr std::uint64_t block = 65536; std::sort(mirrors.begin(), mirrors.end(), [](const ImageMirror* left, const ImageMirror* right) { return left->base < right->base; }); - thread_local std::vector changed; + thread_local std::vector* changedSlot = nullptr; + auto& changed = ShaderRecompiler::ThreadOwned(changedSlot); for (std::size_t first = 0; first < mirrors.size();) { auto last = first + 1; while (last < mirrors.size() && mirrors[last]->base == mirrors[last - 1]->base + mirrors[last - 1]->bytes) ++last; diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp index 5169e3ef8..847cd80a5 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp @@ -1,4 +1,5 @@ #include "prx/libSceAgcDriver/Execution/include/CaptureTrace.hpp" +#include "ThreadOwned.hpp" #include "prx/libSceAgcDriver/Graphics/include/DepthSurface.hpp" #include "prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp" #include "prx/libSceAgcDriver/Graphics/include/Recorder.hpp" @@ -186,14 +187,19 @@ struct LookupRecord { const StorageTexture* source; }; -thread_local std::vector lookupLog; +thread_local std::vector* lookupLogSlot = nullptr; + +std::vector& lookupLog() { + return ShaderRecompiler::ThreadOwned(lookupLogSlot); +} void logLookup(const LookupRecord& record) { // The bound drops the oldest half, never everything: one build's lookups (a few dozen at most) // are the newest records, and a capture must find them even when the bound trips mid-build. constexpr std::size_t bound = 1024; - if (lookupLog.size() >= bound) lookupLog.erase(lookupLog.begin(), lookupLog.begin() + bound / 2); - lookupLog.push_back(record); + auto& log = lookupLog(); + if (log.size() >= bound) log.erase(log.begin(), log.begin() + bound / 2); + log.push_back(record); } std::shared_ptr cachedStorageTexture(const Context& context, std::span words, const GuestTextureResource& resource, std::uint32_t mip, std::uint64_t guestBytes = 0); @@ -793,7 +799,8 @@ struct ThreadBuildProfile { }; ThreadBuildProfile& ThreadBuilds() { - thread_local ThreadBuildProfile profile; + thread_local ThreadBuildProfile* profileSlot = nullptr; + auto& profile = ShaderRecompiler::ThreadOwned(profileSlot); return profile; } @@ -1310,7 +1317,8 @@ bool EpochRevalidate() { // ValidatedSurface); the last record of an object is the freshest. void ShaderResources::captureValidation() { const auto record = [](const void* object) -> const LookupRecord* { - for (auto it = lookupLog.rbegin(); it != lookupLog.rend(); ++it) { + const auto& log = lookupLog(); + for (auto it = log.rbegin(); it != log.rend(); ++it) { if (it->object == object) return &*it; } return nullptr; @@ -1319,7 +1327,7 @@ void ShaderResources::captureValidation() { for (std::size_t i = 0; i < textures.size(); ++i) { if (const auto* found = record(textures[i].get())) validatedTextures[i] = {found->resource, found->bytes, found->keys, found->generation, 0, found->source, true}; } - lookupLog.clear(); + lookupLog().clear(); } // Proves every texture and storage image still current without repeating its lookup: exactly the @@ -1350,12 +1358,17 @@ bool ShaderResources::fastRevalidate(std::uint64_t serialBefore, std::span queries; - thread_local std::vector pending; + thread_local std::vector* queriesSlot = nullptr; + auto& queries = ShaderRecompiler::ThreadOwned(queriesSlot); + thread_local std::vector* pendingSlot = nullptr; + auto& pending = ShaderRecompiler::ThreadOwned(pendingSlot); // The element each pending query stands for, in query order. - thread_local std::vector owners; - thread_local std::vector images; - thread_local std::vector scannedKeys; + thread_local std::vector* ownersSlot = nullptr; + auto& owners = ShaderRecompiler::ThreadOwned(ownersSlot); + thread_local std::vector* imagesSlot = nullptr; + auto& images = ShaderRecompiler::ThreadOwned(imagesSlot); + thread_local std::vector* scannedKeysSlot = nullptr; + auto& scannedKeys = ShaderRecompiler::ThreadOwned(scannedKeysSlot); queries.clear(); pending.clear(); owners.clear(); @@ -1628,7 +1641,7 @@ bool ShaderResources::Revalidate(std::span shaders, ProofR // they must hand back the very objects the set's views belong to. The build appended them stage // by stage, binding by binding, so the walk repeats that order. const auto fullWalk = [&] { - lookupLog.clear(); + lookupLog().clear(); std::size_t textureIndex = 0; std::size_t storageIndex = 0; for (const auto& shader : shaders) { @@ -1664,8 +1677,10 @@ bool ShaderResources::Revalidate(std::span shaders, ProofR captureValidation(); return true; }; - thread_local std::vector overlapping; - thread_local std::vector refreshed; + thread_local std::vector* overlappingSlot = nullptr; + auto& overlapping = ShaderRecompiler::ThreadOwned(overlappingSlot); + thread_local std::vector* refreshedSlot = nullptr; + auto& refreshed = ShaderRecompiler::ThreadOwned(refreshedSlot); refreshed.clear(); FastFail reason = FastFail::Count; bool accepted = false; @@ -1711,7 +1726,8 @@ bool ShaderResources::Revalidate(std::span shaders, ProofR // The walk after the refresh, or beside a proof that accepted a foreign overlap, must // find every object in place and current: another object, or an upload (a moved // content version), is a decision the proof got wrong. - thread_local std::vector> versions; + thread_local std::vector>* versionsSlot = nullptr; + auto& versions = ShaderRecompiler::ThreadOwned(versionsSlot); versions.clear(); for (const auto& surface : validatedTextures) { if (surface.source != nullptr) versions.emplace_back(surface.source, surface.source->Version()); @@ -1744,7 +1760,8 @@ bool ShaderResources::Revalidate(std::span shaders, ProofR }; if (EpochRevalidate()) { if (!directRegions.empty() && !(pendingSerialSeen != 0 && pendingSerialSeen == StorageTexture::PendingSerial())) { - thread_local std::vector regions; + thread_local std::vector* regionsSlot = nullptr; + auto& regions = ShaderRecompiler::ThreadOwned(regionsSlot); regions.clear(); for (const auto& region : directRegions) regions.push_back({region.begin, region.end, nullptr, nullptr, false}); StorageTexture::ScanPending(regions); diff --git a/core/shader/recompiler/CMakeLists.txt b/core/shader/recompiler/CMakeLists.txt index 0082a7142..29d4bf176 100644 --- a/core/shader/recompiler/CMakeLists.txt +++ b/core/shader/recompiler/CMakeLists.txt @@ -218,6 +218,7 @@ function(add_shader_recompiler target runtimeTarget) ResourceMaterializer::SpecializationNanoseconds SetDebugProbeActive ShaderCacheDirectory + ThreadOwned WriteFileAtomically ) set(shaderCacheManifest) diff --git a/core/shader/recompiler/ShaderDiskCache.cpp b/core/shader/recompiler/ShaderDiskCache.cpp index e7ed428b8..fd57be587 100644 --- a/core/shader/recompiler/ShaderDiskCache.cpp +++ b/core/shader/recompiler/ShaderDiskCache.cpp @@ -1,6 +1,7 @@ #include "ShaderDiskCache.hpp" #include "CacheKey.hpp" #include "ShaderCacheDirectory.hpp" +#include "ThreadOwned.hpp" #ifdef _WIN32 #ifndef NOMINMAX #define NOMINMAX @@ -603,7 +604,8 @@ public: bool Load(std::span key, CompiledVariant& variant) { const auto started = std::chrono::steady_clock::now(); const auto name = EntryName(key); - thread_local std::vector file; + thread_local std::vector* fileSlot = nullptr; + auto& file = ThreadOwned(fileSlot); bool loaded = false; if (!ReadWholeFile(directory / name, file)) { misses.fetch_add(1, std::memory_order_relaxed); @@ -778,7 +780,8 @@ void BuildKey(const RecompileRequest& request, std::uint32_t hostSubgroupSize, c writer.Value(FileMagic); writer.Value(FormatVersion); writer.Value(SourceVersion()); - thread_local std::vector memoryKey; + thread_local std::vector* memoryKeySlot = nullptr; + auto& memoryKey = ThreadOwned(memoryKeySlot); RecompileCacheKey::Build(request, memoryKey); writer.Values(std::span(memoryKey)); writer.Values(request.shader.code); diff --git a/core/shader/recompiler/ThreadOwned.hpp b/core/shader/recompiler/ThreadOwned.hpp new file mode 100644 index 000000000..94a0d084b --- /dev/null +++ b/core/shader/recompiler/ThreadOwned.hpp @@ -0,0 +1,59 @@ +#ifndef CORE_SHADER_RECOMPILER_THREADOWNED_HPP +#define CORE_SHADER_RECOMPILER_THREADOWNED_HPP + +#include +#include +#include +#include + +namespace ShaderRecompiler { + +namespace ThreadOwnedDetail { + +struct Owned { + void* object; + void (*destroy)(void*); +}; + +inline void DestroyAll(void* value) { + auto* owned = static_cast*>(value); + for (auto it = owned->rbegin(); it != owned->rend(); ++it) it->destroy(it->object); + delete owned; +} + +inline pthread_key_t Key() { + static const pthread_key_t key = [] { + pthread_key_t created{}; + if (pthread_key_create(&created, DestroyAll) != 0) throw std::runtime_error("ThreadOwned: pthread_key_create failed"); + return created; + }(); + return key; +} + +inline void DestroyAtThreadExit(void* object, void (*destroy)(void*)) { + auto* owned = static_cast*>(pthread_getspecific(Key())); + if (owned == nullptr) { + owned = new std::vector(); + if (pthread_setspecific(Key(), owned) != 0) { + delete owned; + throw std::runtime_error("ThreadOwned: pthread_setspecific failed"); + } + } + owned->push_back({object, destroy}); +} + +} + +template +T& ThreadOwned(T*& slot) { + if (slot == nullptr) { + auto object = std::make_unique(); + ThreadOwnedDetail::DestroyAtThreadExit(object.get(), [](void* value) { delete static_cast(value); }); + slot = object.release(); + } + return *slot; +} + +} + +#endif