From 14137bc9a6b80877fc6b2a666628fb6a5aaa1b8e Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 8 Sep 2026 04:08:55 -0400 Subject: [PATCH] [Fix] (Espryt): refuse a backwards generation and an out-of-range slot in the handle-keyed GetOrCreate instead of adopting them --- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 16 +++++++ MobileGL/MG_Backend/DirectGLES/SlotTables.h | 53 +++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index a9ce029c..7e25c867 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -2280,6 +2280,22 @@ namespace MobileGL::MG_Backend::DirectGLES { GLESBufferResource* GetOrCreateBufferResourceForHandle(MG_Pipe::MGPipeHandle res) { if (MG_Pipe::MGPipeHandleIsNull(res)) return nullptr; + // The table refuses both of these itself; this is the release-build VOICE for the + // refusal, because MOBILEGL_ASSERT compiles out at INFO and a resource that silently + // stops being twinned is the failure mode the refusal exists to replace. + if (res.Slot >= BackendBufferResourceTable::kMaxHandleSlot) { + MGLOG_E_ONCE("MGPipe: resource handle slot %u is past the backend table's %u bound - " + "refusing to twin it", + res.Slot, BackendBufferResourceTable::kMaxHandleSlot); + return nullptr; + } + const Uint32 liveGen = g_backendBufferResources.LiveGenAt(res.Slot); + if (liveGen != 0 && liveGen > res.Gen) { + MGLOG_E_ONCE("MGPipe: resource handle {%u, %u} names a generation BEHIND the live twin's " + "%u - refusing rather than dropping the incumbent's driver storage", + res.Slot, res.Gen, liveGen); + return nullptr; + } auto& twin = g_backendBufferResources.GetOrCreate(res); if (!twin) { twin = MakeShared(); diff --git a/MobileGL/MG_Backend/DirectGLES/SlotTables.h b/MobileGL/MG_Backend/DirectGLES/SlotTables.h index 9d4c0137..a84e6e72 100644 --- a/MobileGL/MG_Backend/DirectGLES/SlotTables.h +++ b/MobileGL/MG_Backend/DirectGLES/SlotTables.h @@ -141,6 +141,12 @@ namespace MobileGL::MG_Backend::DirectGLES { using StateWeakPtr = std::weak_ptr; using BackendPtr = SharedPtr; + // The largest slot index this table will grow to for a handle that ARRIVED in a call's + // payload. Slots are dense and allocated per kind, so a million of one kind is already + // far past any application's live object count; the cap is here because the alternative + // is letting a corrupt 32-bit slot decide a vector resize. See GetOrCreate(MGPipeHandle). + static constexpr Uint32 kMaxHandleSlot = 1u << 20; + struct Entry { BackendPtr backend; // LIVENESS ONLY, and only for ForEachLive(), which locks it so the callee holds a @@ -274,17 +280,60 @@ namespace MobileGL::MG_Backend::DirectGLES { "GetOrCreate(handle) named the reserved null handle"); if (MG_Pipe::MGPipeHandleIsNull(handle)) return m_nullTwin; + // A slot index that ARRIVED in a payload indexes a vector this call would RESIZE, + // and nothing between the payload and here bounds it: the applier's blob gates sit + // in front of the vertex-input family, not in front of the resource family, which + // dispatches ops->Create(record.Res, ...) straight through. There is no allocator + // constant to check against on this side - the allocator is the client's - so this + // is a sanity cap and is documented as one: kMaxHandleSlot entries of one kind is + // already orders of magnitude past any real GL object count, while a corrupt 32-bit + // slot asks for a four-billion-entry resize. + if (handle.Slot >= kMaxHandleSlot) { + MOBILEGL_ASSERT(false, "GetOrCreate(handle) named slot %u, past this table's %u bound", + handle.Slot, kMaxHandleSlot); + return m_nullTwin; + } + // Same arming as the minting overload, and for the same reason: twin creation is // the moment a driver-owned id starts needing a guarded destructor. EnsureProcessTeardownSentinel(); + // THE TWO DIRECTIONS ARE NOT SYMMETRIC HERE, where they are on the minting overload. + // There the handle comes straight out of MGPipeSlots().Acquire and can never be + // BEHIND the entry, so a bare `!=` only ever means "the slot was recycled forward". + // Here the handle arrived in a payload, so `handle.Gen < entry.Gen` is a reachable + // input, and adopting it would destroy the INCUMBENT LIVE twin - a driver buffer id, + // a persistent map, a pooled store, released by a defaulted destructor that issues + // no glDeleteBuffers and no pool enrolment - and then stamp the slot back to the + // dead resource's generation, after which the incumbent's own FindByHandle refuses + // it and it is silently handed a fresh, empty twin. That is a leak AND a resource + // that loses its storage with no diagnostic, i.e. the shape commit d7655247 fixed + // and the thing MGPipeHandle::Gen exists to prevent. So: forward is a recycle and + // resets the twin, BACKWARD is refused - which is the same answer FindByHandle + // below already gives the same input. Entry& entry = EntryAt(handle.Slot); + if (entry.Live && entry.Gen > handle.Gen) { + MOBILEGL_ASSERT(false, + "GetOrCreate(handle) named generation %u at slot %u, which is BEHIND " + "the live entry's %u - refusing rather than destroying the incumbent", + handle.Gen, handle.Slot, entry.Gen); + return m_nullTwin; + } if (entry.Live && entry.Gen != handle.Gen) entry.backend.reset(); entry.Gen = handle.Gen; entry.Live = true; return entry.backend; } + // The generation of the LIVE entry at this slot, or 0 when the slot is out of range or + // holds no live entry. It exists so a caller can DIAGNOSE - in a release build, where + // MOBILEGL_ASSERT is inert - the refusal GetOrCreate(handle) above performs silently. + Uint32 LiveGenAt(Uint32 slot) const { + if (slot >= m_slots.size()) return 0; + const Entry& entry = m_slots[slot]; + return entry.Live ? entry.Gen : 0; + } + // P3a: the death half of the overload above, for a kind whose announcement is its own // destroy CALL rather than the shared death notice (D-L). Hands the twin OUT rather // than destroying it in place, because the caller may still have to decide what @@ -448,6 +497,10 @@ namespace MobileGL::MG_Backend::DirectGLES { return true; } + // Grows the table to hold `slot`. Every caller bounds `slot` first - the minting + // overload because the allocator produced it, the handle overload against + // kMaxHandleSlot - because this is the one place a client-supplied number decides an + // allocation size. Entry& EntryAt(Uint32 slot) { if (slot >= m_slots.size()) m_slots.resize(static_cast(slot) + 1); return m_slots[slot];