mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-11-16 06:23:23 +09:00
[Fix, Test] (Magma, MG_IntegrationTest): make the handle-ABA negative control construct its own collision and defeat the {slot, gen} generation
- MOBILEGL_PIPE_HANDLE_ABA_CONTROL asserted the corruption and saw correct pixels, so
DirectVulkan.HandleRecycle.AbaControl.*AVertexArray* was RED in an always-on
integration-gpu lane while every guard it was supposed to be defeating stood. Two
measured reasons, neither of them the {slot, gen} re-key: (1) D18 spelled the control as
"hash the raw BufferObject* instead of its lifetime id, and skip the vaoLifetimeId
compare", which only collides if the allocator hands the freed block back - it does not.
glGen* recycles the NAME, but a VertexArrayObject is 3920 bytes, past glibc's tcache, so
its chunk goes to the unsorted bin and is split by the next allocation the replacement
path makes; four create/delete cycles in one run gave four addresses ~1 MiB apart, and
the BufferObject behaves the same. (2) The reproducer put a frame boundary between the
arming draw and the recycled draw, and the only memo that carries a GPU slice rather
than a layout - ResolvedVertexBindings - declines across frames by design, so no key
collision whatsoever could have shown up in pixels.
- The control no longer asks the allocator for the collision: on both arms it replaces the
object identity in DirectVulkan's vertex-input keys with a constant, which is the
strongest form of "the block came back" and is deterministic. Three sites, all behind
one question (MagmaPipeAbaControlDefeatsIdentity): the buffer identity leaves
VertexInputStateFactory::ComputeHash, VertexInputStateFactory::MemosFor claims one entry
without its Owner compare, and VulkanRenderer::LookupVaoDrawMemo hands one entry back
uncleared ahead of both arms.
- That is what makes the control cover the key P2 SHIPS. Under MOBILEGL_PIPE_PUSH=0 the
handle arm is not executed at all, so the old control said nothing about the generation
in {slot, gen} - the whole of what makes the re-keyed memos ABA-safe. A second lane,
DirectVulkan.HandleRecycle.AbaControlHandles., runs the handle arm with the knob and
asserts the same corruption; D18's lane is kept verbatim beside it for the pre-handle arm.
- The reproducer's two draws now share a frame, and both buffers are realised before the
window, so a moved slice epoch cannot mask the ABA behind a gate that is not about
identity. Nothing else is relaxed: the frame serial, the slice epochs and the host-map
check stay in force, so a green arm still means "a replacement object was handed its
predecessor's resolved vertex bindings because the identity halves of the keys were
defeated".
- ExpectPixelsFor now prints what it OBSERVED (STALE/FRESH/NEITHER) next to what the arm
expected, on every arm and whether or not the case passes.
- Knob-off is unchanged and the pull build is untouched: every new branch is
#if MOBILEGL_PIPE_PUSH, and symbol_report.py --threshold 0 against the pre-P2 baseline
still reports 0 added / 0 removed / 0 renamed and the same four resized symbols
(RenderState::RenderState, SetCapability, IsCapabilityEnabled, _GLOBAL__sub_I_DirectGLES.cpp).
This commit is contained in:
@@ -63,14 +63,23 @@ namespace MobileGL::MG_Backend::DirectVulkan {
|
||||
const MG_Pipe::MGPipeHandle handle =
|
||||
m_identity->HandleOf(MG_Pipe::MGPipeKind::Buffer, attr.Buffer->GetLifetimeId());
|
||||
bufferKey = static_cast<Uint64>(handle.Slot) | (static_cast<Uint64>(handle.Gen) << 32);
|
||||
} else if (MG_Config::Features.PipeHandleAbaControl) {
|
||||
// Negative control C (P2 brief D18), and it applies to the PRE-HANDLE arm
|
||||
// on purpose: hash the raw BufferObject* the way this did before the
|
||||
// lifetime-id fix, so HandleRecycleScenario.AbaControl can reproduce the
|
||||
// ABA and assert the WRONG pixels. That arm is what proves the reproducer
|
||||
// still reproduces; if the allocator stops handing the address back, it
|
||||
// fails instead of passing for the wrong reason.
|
||||
bufferKey = static_cast<Uint64>(reinterpret_cast<SizeT>(attr.Buffer.get()));
|
||||
}
|
||||
if (MagmaPipeAbaControlDefeatsIdentity()) {
|
||||
// Negative control C (P2 brief D18), on WHICHEVER arm this run is on - the
|
||||
// pre-handle lifetime id and the handle's {slot, gen} are the same guard
|
||||
// wearing two hats, and a control that defeated only the retired one would
|
||||
// say nothing about the key P2 ships.
|
||||
//
|
||||
// The identity is replaced by a constant rather than by the raw
|
||||
// BufferObject*, because the address is not recycled in practice and so
|
||||
// never collides (see MagmaPipeAbaControlDefeatsIdentity). Zero is what a
|
||||
// key with NO buffer identity in it looks like - the exact defect this
|
||||
// hash was fixed for: "the hash is what TryBindResolvedVertexBindings
|
||||
// accepts as proof that a memoised binding still reads the buffer it was
|
||||
// resolved from", and with the identity gone it accepts a binding resolved
|
||||
// from a different buffer. HandleRecycleScenario.AbaControl then draws a
|
||||
// replacement VAO and gets its dead predecessor's vertex data.
|
||||
bufferKey = 0;
|
||||
}
|
||||
}
|
||||
#endif
|
||||
@@ -89,6 +98,13 @@ namespace MobileGL::MG_Backend::DirectVulkan {
|
||||
// does this, and no two live VAOs can share an entry however large the working set is.
|
||||
// There is no probe in front of it because the mint itself is one - a one-entry memo
|
||||
// hit for every acquisition after this draw's first, and a hash probe otherwise.
|
||||
if (MagmaPipeAbaControlDefeatsIdentity()) {
|
||||
// Negative control C: one entry for every VAO, claimed without the Owner compare,
|
||||
// which is precisely "the slot was recycled and Gen did not move". The replacement
|
||||
// therefore inherits the dead VAO's content hash and its resolved-entry pointer -
|
||||
// the two facts the generation is the only thing protecting.
|
||||
return m_vaoMemos[kMagmaPipeAbaControlSlotIndex];
|
||||
}
|
||||
VaoBackendMemos& memos = m_vaoMemos[MagmaPipeSlotIndex(handle)];
|
||||
if (!(memos.Owner == handle)) {
|
||||
// A slot whose Gen moved because the identity table recycled it for a different
|
||||
|
||||
Reference in New Issue
Block a user