From 738b289df8b65eb79bd2d03193580b598acd3258 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 8 Sep 2026 00:57:49 -0400 Subject: [PATCH] [Fix] (Magma, MG_IntegrationTest): drop the fourth ABA-control consumer, which was unreachable, and the scenario comment its own body contradicts - VulkanRenderer::LookupVaoDrawMemo's legacy arm read MG_Config::Features.PipeHandleAbaControl a fourth time, bypassing the one-question accessor MagmaPipeAbaControlDefeatsIdentity that 55d2af9b's own message says all the control's sites go through. It could never be false there: the control's early return fires ahead of BOTH arms, so a run that reaches the legacy compare has the knob off. The compare is unconditional again, with a note saying where the question is answered and to route through the accessor if that early return is ever narrowed - which is the trap the dead line was, since narrowing it would have silently restored D18's retired semantics at this one site. - HandleRecycleScenario's vertex-array case still said "BOTH the VAO and the buffer are recycled here so that a key built out of raw addresses matches", which the body has contradicted since 55d2af9b: both buffers are created before the ABA window and neither is deleted inside it, deliberately, so that no buffer traffic moves VkBufferManager's slice-epoch counter inside the window. Only the VAO is recycled, and only its GL name - the heap block is not handed back, which is why the knob rather than the allocator constructs the AbaControl arms' collision. --- .../DirectVulkan/Renderer/VulkanRenderer.cpp | 21 ++++++++----------- .../Scenarios/HandleRecycleScenario.cpp | 8 +++++-- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index 39589401..7cc2f3e0 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -3728,22 +3728,19 @@ void main() { // its own is recycled, and a slot matched on a recycled address hands the new VAO // the dead one's resolved bindings. const Uint64 lifetimeId = vao->GetLifetimeId(); -#if MOBILEGL_PIPE_PUSH - // Negative control C (P2 brief D18): with MOBILEGL_PIPE_HANDLE_ABA_CONTROL=1 the - // lifetime-id half of the compare is defeated, leaving the recycled address as the - // whole key - exactly the state this table was in before the ABA fix. That is what - // lets HandleRecycleScenario.AbaControl assert the WRONG pixels and so prove that its - // reproducer still reproduces. - const Bool compareLifetimeId = !MG_Config::Features.PipeHandleAbaControl; -#else - constexpr Bool compareLifetimeId = true; -#endif + // Negative control C has NO consumer here. It is answered once, ahead of both arms, by + // the early return above, so a run that reaches this line has the knob off and the + // lifetime-id half of the compare is unconditional. A fourth consumer here would be a + // second site deciding the same question - what MagmaPipeAbaControlDefeatsIdentity + // exists to prevent - and a trap: narrow that early return later and this one would + // silently return to D18's retired semantics. If it is ever narrowed, ask the accessor + // here rather than re-reading MG_Config::Features. VaoDrawMemo& first = m_vaoDrawMemoTable[index]; - if (first.vaoKey == vao && (!compareLifetimeId || first.vaoLifetimeId == lifetimeId)) { + if (first.vaoKey == vao && first.vaoLifetimeId == lifetimeId) { return &first; } VaoDrawMemo& second = m_vaoDrawMemoTable[index ^ 1u]; - if (second.vaoKey == vao && (!compareLifetimeId || second.vaoLifetimeId == lifetimeId)) { + if (second.vaoKey == vao && second.vaoLifetimeId == lifetimeId) { return &second; } // Miss: recycle a slot. Prefer an empty one; otherwise evict the entry whose diff --git a/MobileGL/MG_IntegrationTest/Scenarios/HandleRecycleScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/HandleRecycleScenario.cpp index 3ec0c55e..5ba3dfd1 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/HandleRecycleScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/HandleRecycleScenario.cpp @@ -477,8 +477,12 @@ void main() { oColor = texture(uTex, vUv); } // ------------------------------------------------------------------------------------ // 1. The vertex array. This is the case the AbaControl knob targets: DirectVulkan keys // VertexInputStateFactory's cache on the attribute's buffer identity and VaoDrawMemo - // on the VAO's, and BOTH the VAO and the buffer are recycled here so that a key built - // out of raw addresses matches while the bytes behind it do not. + // on the VAO's. Only the VAO is recycled here: both buffers are created before the + // window and neither is deleted inside it, because buffer traffic in the window moves + // VkBufferManager's slice-epoch counter and that gate is not an identity gate (see + // the two MakeQuadBuffer calls). What is recycled is the GL NAME; the heap block is + // not handed back, which is why the knob - not the allocator - constructs the + // AbaControl arms' collision. // ------------------------------------------------------------------------------------ TEST_F(HandleRecycleScenario, AVertexArrayAtARecycledAddressDoesNotInheritItsPredecessorsVertexInput) { if (!Ready()) return;