diff --git a/MobileGL/MG_Test/SanityTest.cpp b/MobileGL/MG_Test/SanityTest.cpp index 80beaa1a..a37edf10 100644 --- a/MobileGL/MG_Test/SanityTest.cpp +++ b/MobileGL/MG_Test/SanityTest.cpp @@ -3141,6 +3141,97 @@ TEST(DirectGLESTextureSync, UnitMemoRefusesToDriveATwinFromAnotherTexture) { MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, 0); } +#if MOBILEGL_PIPE_PUSH +// P5e (tx2), CONTRACT-P5E §4.3: THE HANDLE ARM'S SIBLING OF THE CASE ABOVE, WITH ABA AS THE +// HAZARD INSTEAD OF A SILENT SLOT SWAP. +// +// The case above pins what the BORROWED-SLOT work list needs: a pointer compare per entry, +// because its key is derived state and a slot swap that never reached the key would drive +// texture A's twin from texture B. The by-handle work list carries no such pointer and no +// PairingsIntact, and this is the argument for why it does not have to: +// +// a handle is {slot, gen}. A slot recycled to a NEW object arrives as {s, g+1}, and +// GetOrCreateByHandle RESETS the entry on a forward generation (SlotTables.h) - so the +// successor gets a fresh twin with a fresh driver texture, and the predecessor's handle +// stops resolving. There is no window in which one handle names two objects, which is +// exactly the window the pointer compare above exists to close. +// +// THE RED: drop the forward-Gen reset (make GetOrCreate return the live entry for any +// generation) and `reMinted` below is the SAME twin as `first` - the successor inherits its +// predecessor's driver texture and its predecessor's synced serials, so the first draw that +// samples it reads the dead object's pixels and the clean gate says there is nothing to do. +TEST(DirectGLESTextureSync, ARecycledTextureSlotReMintsTheTwinOnTheHandleArm) { + using namespace MobileGL; + using namespace MobileGL::MG_Backend::DirectGLES; + ScopedDirectGLESTextureBindings scoped; // fresh GLContext + registry + binding caches + if (!TextureResourceSubsystemEnabled()) { + GTEST_SKIP() << "the texture-resource subsystem bit is clear in this configuration"; + } + + auto& registry = TextureImpl::g_backendTextureObjects; + + // Two lifetime ids standing in for two frontend textures, so the client allocator hands + // back the SAME SLOT at a higher generation for the second - which is the whole hazard. + auto firstOwner = MakeShared(0u); + const MG_Pipe::MGPipeHandle first = + MG_Pipe::MGPipeSlots().Acquire(MG_Pipe::MGPipeKind::Texture, firstOwner->GetLifetimeId()); + ASSERT_FALSE(MG_Pipe::MGPipeHandleIsNull(first)); + + const auto describe = [](MG_Pipe::MGPipeHandle res, Uint32 size) { + MG_Pipe::MGPResourceDesc desc{}; + desc.Resource = res; + desc.Target = static_cast(MG_Pipe::MGPipeResourceTarget::Tex2D); + desc.StorageKind = static_cast(TextureStorageType::Mipmap); + desc.InternalFormat = static_cast(TextureInternalFormat::RGBA8); + desc.Width = size; + desc.Height = size; + desc.Depth = 1; + desc.ArrayLayers = 1; + desc.Levels = 1; + EXPECT_TRUE(MG_Pipe::MGPipeApplyResourceCreate(desc)); + }; + describe(first, 8); + + auto* firstTwin = TextureImpl::ResolveTextureTwin(first); + ASSERT_NE(firstTwin, nullptr) << "the record-first resolver declined a handle whose record it " + "had just been given"; + firstTwin->NotePushedSyncHandle(first); + + // The recycle: the client frees the slot and hands it straight back out at {s, g+1}. + MG_Pipe::MGPipeSlots().Free(MG_Pipe::MGPipeKind::Texture, first); + auto secondOwner = MakeShared(0u); + const MG_Pipe::MGPipeHandle second = + MG_Pipe::MGPipeSlots().Acquire(MG_Pipe::MGPipeKind::Texture, secondOwner->GetLifetimeId()); + ASSERT_FALSE(MG_Pipe::MGPipeHandleIsNull(second)); + ASSERT_EQ(second.Slot, first.Slot) << "the allocator did not recycle the slot, so this case " + "is not exercising ABA at all"; + ASSERT_GT(second.Gen, first.Gen); + describe(second, 16); + + auto* reMinted = TextureImpl::ResolveTextureTwin(second); + ASSERT_NE(reMinted, nullptr); + EXPECT_NE(reMinted, firstTwin) + << "the successor at a recycled texture slot inherited its predecessor's twin: it would " + "sample the dead object's driver texture, and the twin's synced serials would tell " + "the clean gate there is nothing to do"; + EXPECT_TRUE(MG_Pipe::MGPipeHandleIsNull(reMinted->PushedSyncHandle())) + << "the re-minted twin kept the dead handle, so its sync prologues would resolve the " + "predecessor's record"; + + // ...and the predecessor's handle stops resolving, which is what makes a stale entry left in + // a work list harmless: its record lookup answers null and the entry is skipped. + EXPECT_EQ(PipeTextureRecordForHandle(first), nullptr) + << "the predecessor's record still answers at a recycled slot"; + + MG_Pipe::MGPipeSlots().Free(MG_Pipe::MGPipeKind::Texture, second); +} +#else +// G2/G14: the same ctest entry exists in the pull build and skips visibly. +TEST(DirectGLESTextureSync, ARecycledTextureSlotReMintsTheTwinOnTheHandleArm) { + GTEST_SKIP() << "the by-handle texture twin is compiled only under MOBILEGL_PIPE_PUSH"; +} +#endif + namespace { // What glTexParameteri actually reached the driver, and which backend texture was bound // when it did. The G9 probe below is a WHITE-BOX assertion (ID-19): the parameter push is diff --git a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp index fc6699ea..83d02e14 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp +++ b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp @@ -1971,10 +1971,23 @@ TEST(RemoteGuards, CapsMirrorFallbackWithNoServerBackendIsFatalByName) { // MarkStorageDirty, MarkStorageDirtyRegion, IsStorageDirty, GetStorageDirtyRegion, // GetStorageDirtyRects // -// Deliberately NOT in the list: the shape reads (GetMipmapTexelSize / GetMipmapByteSize / -// GetMipmapLevelCount / GetUploadTargets / GetTarget / IsComplete), which the pinned -// BARRIER-PULLED object-class rows still answer through the unit-object pointer -// (FieldOwnershipTest's list, P3b/P4b/P7) - the per-draw binding walk reads them every draw. +// P5e (tx2): THE SHAPE READS JOIN THE LIST, and the sentence that stood here is why they could +// not before - "the per-draw binding walk reads them every draw". P5e is the commit that makes +// that untrue: the unit work list is st.BoundSamplerViews[], the clean gate is +// IsDrawSyncCleanByRecord, and the three sync bodies read the resource record and the server's +// staged-texture store. Nothing on the draw path asks a frontend texture for a level count, a +// level extent, a level byte size or a compressed level any more, so the exemption becomes a +// guard and reverting any handle arm aborts BY ACCESSOR NAME: +// +// GetMipmapLevelCount, GetMipmapTexelSize, GetMipmapByteSize, +// GetCompressedFormat, GetCompressedByteSize, MapCompressedMipmapData, +// GetRequestedCompressedFormat +// +// Keyed on the SERVER BACKEND being DirectGLES (ruling 12's shape): Magma is lockstep through +// P7 and its named blit still asks a frontend texture IsComplete(), which is legal for a client +// parked in its own wait. Still NOT in the list: GetUploadTargets / GetTarget / IsComplete, +// which are TextureObject's own and not MipmapStorage's - their rows retire with the object +// class in P7/P9. #define MGL_TEXTURE_GUARD_TEST(Name, Id, Call) \ TEST(RemoteGuards, Name) { \ const auto child = RunInChild([] { \ @@ -2012,6 +2025,21 @@ MGL_TEXTURE_GUARD_TEST(TextureGetStorageDirtyRegionFromTheApplyThreadIsFatalByNa MGL_TEXTURE_GUARD_TEST(TextureGetStorageDirtyRectsFromTheApplyThreadIsFatalByName, 719, (void)texture.GetStorageDirtyRects(TextureUploadTarget::Texture2D, 0, nullptr, 0)) +// ---- P5e (tx2): the seven shape reads the P4a/P5c list exempted ---------------------------- +MGL_TEXTURE_GUARD_TEST(TextureGetMipmapLevelCountFromTheApplyThreadIsFatalByName, 721, + (void)texture.GetMipmapLevelCount()) +MGL_TEXTURE_GUARD_TEST(TextureGetMipmapTexelSizeFromTheApplyThreadIsFatalByName, 722, + (void)texture.GetMipmapTexelSize(TextureUploadTarget::Texture2D, 0)) +MGL_TEXTURE_GUARD_TEST(TextureGetMipmapByteSizeFromTheApplyThreadIsFatalByName, 723, + (void)texture.GetMipmapByteSize(TextureUploadTarget::Texture2D, 0)) +MGL_TEXTURE_GUARD_TEST(TextureGetCompressedFormatFromTheApplyThreadIsFatalByName, 724, + (void)texture.GetMipmapCompressedFormat(TextureUploadTarget::Texture2D, 0)) +MGL_TEXTURE_GUARD_TEST(TextureGetCompressedByteSizeFromTheApplyThreadIsFatalByName, 725, + (void)texture.GetMipmapCompressedByteSize(TextureUploadTarget::Texture2D, 0)) +MGL_TEXTURE_GUARD_TEST(TextureMapCompressedMipmapDataFromTheApplyThreadIsFatalByName, 726, + (void)texture.MapMipmapCompressedImage(TextureUploadTarget::Texture2D, 0)) +MGL_TEXTURE_GUARD_TEST(TextureGetRequestedCompressedFormatFromTheApplyThreadIsFatalByName, 727, + (void)texture.GetMipmapRequestedCompressedFormat(TextureUploadTarget::Texture2D, 0)) #undef MGL_TEXTURE_GUARD_TEST // P5c (gt, CONTRACT-P5C §6 layer 2 / audit A1): the client-side half of the gPipeInputs