swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-09-08 04:08:32 +09:00
Code Issues Packages Projects Releases Wiki Activity

[Fix, Test] (MG_Impl, MG_Backend/DirectGLES): DSA by-name texture calls corrupted borrowed-slot memo pairings - process-wide glyph death under Iris

Browse Source 
Field report: on Espryt with a BSL Iris pipeline built, every glyph in the
game died - HUD, menu labels, even the vanilla title screen after leaving
the world - while sprites kept rendering. Captured on-device (FCL apitrace
rig), reproduced headlessly on llvmpipe, and pinned with a three-way replay:
the same trace renders full text on raw Mesa desktop GL and on Magma, so
the stream was intact and the execution was Espryt's.

MECHANISM. WithTemporarilyBoundNamedTexture implements the by-name (DSA)
texture entry points by binding the named texture onto the active unit's
real slot, running the bound-texture code, and restoring - without moving
the texture bind generation on either edge. DirectGLES's per-draw texture
sync memo keys on that generation and BORROWS the slot pointer, so a memo
built for texture A kept passing every key while a by-name call had
texture B sitting in the slot: A's backend twin was driven with B's
frontend object, and SyncMipmapsToBackend re-specified A's storage with
B's shape. In the trace, a by-name upload to a BSL 2048x2048 map while
the 16x16 lightmap was bound re-specified the lightmap's GL texture
2048x2048-NULL and back 16x16-NULL. The lightmap exists only as render
output - no glTexSubImage2D ever touches it - so it stayed zero forever,
and rendertype_text (vertexColor = Color * texelFetch(lightmap, ...)),
alpha-discards every glyph. Background quads never sample the lightmap,
which is why only text died.

FIX, class-level, two layers:
- Frontend (shared, closes the same hole for DirectVulkan's generation-
  keyed memos): the temporary bind and the restore each bump the texture
  bind generation (only when the slot actually changed), and the restore
  is an RAII scope guard so a throwing body can no longer leak the
  temporary binding - a second latent bug of the same class. Deliberately
  a generation bump and not a touched-unit note: the high-water mark must
  not chase by-name calls, and a completed bind/restore pair leaves the
  content epoch unchanged, so the cost is an owner-compare re-walk, not a
  memo rebuild.
- DirectGLES defense in depth: both borrowed-pair memos
  (g_unitTextureSyncList, g_fboTextureSyncList) record which frontend
  texture each backend twin was paired with and re-check it before any
  replay (last in the key conjunction, behind the context-id compare). A
  stale pairing now costs a list rebuild instead of silent cross-texture
  storage corruption.

Tests, both red with their own layer reverted:
TextureTest.NamedTextureCallKeepsUnitBindingAccountingCoherent (the
accounting contract) and DirectGLESTextureSync.UnitMemoRefusesToDriveA-
TwinFromAnotherTexture (the corrupting sequence shape against a mock GLES
table, asserting the resident texture's storage is never re-specified).
595/595 unit at default and with the async kill switch. Replay evidence:
the captured BSL ESC-menu trace renders all text through Espryt post-fix,
byte-comparable to the Mesa-direct and Magma replays; the no-shaderpack
control is unchanged. A trace fixture wiring this scene into CI follows
in a separate commit.
This commit is contained in:
BZLZHH
2026-08-09 11:28:24 -04:00
parent 3e0460e472
commit 107669b3db
5 changed files with 261 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp
+27 -6
View File
@@ -1127,10 +1127,29 @@ namespace MobileGL::MG_Backend::DirectGLES {
// slot the key covers still holds a reference to it. Holding either side by shared_ptr
// instead would keep dead frontend textures alive and defeat the registry's
// weak-reference GC.
//
// `texture` records WHICH frontend object `backend` was paired with when the entry was
// built, and PairingsIntact re-checks it before any replay. The keys above are the
// primary guard, but they are all derived state: a slot swap that never reaches the
// bind generation (the DSA by-name emulation used to swap a slot silently) would leave
// every key matching while the borrowed slot pointed at a different texture, and the
// replay would then drive texture A's backend twin from texture B's frontend state -
// re-specifying A's backend storage with B's shape and destroying A's contents. A raw
// pointer compare per entry is far cheaper than the walk it guards, and a stale pairing
// costs only a list rebuild, so this stays as the structural net under the keys.
struct UnitTextureSyncEntry {
const SharedPtr<MG_State::GLState::ITextureObject>* slot = nullptr;
MG_State::GLState::ITextureObject* texture = nullptr;
BackendTextureObject* backend = nullptr;
};
// True while every entry's borrowed slot still holds the texture the entry was paired
// with. Callers put it LAST in the key conjunction so it only runs on a key hit.
static Bool PairingsIntact(const Vector<UnitTextureSyncEntry>& list) {
for (const auto& entry : list) {
if (entry.slot->get() != entry.texture) return false;
}
return true;
}
static Vector<UnitTextureSyncEntry> g_unitTextureSyncList;
static Bool g_unitTextureSyncListValid = false;
static Uint64 g_unitTextureSyncListContextId = 0;
@@ -1197,7 +1216,8 @@ namespace MobileGL::MG_Backend::DirectGLES {
g_unitTextureSyncListMaxUnit == maxTouchedUnit &&
g_unitTextureSyncListContextGeneration == g_textureContextGeneration &&
g_unitTextureSyncListEpoch == unitBindingsEpoch &&
g_unitTextureSyncListSamplingGeneration == samplingGeneration) {
g_unitTextureSyncListSamplingGeneration == samplingGeneration &&
PairingsIntact(g_unitTextureSyncList)) {
for (const auto& entry : g_unitTextureSyncList) {
// Aggregate gate == the conjunction of the three callees' own
// early-outs (see IsDrawSyncClean); skipping on true is
@@ -1219,8 +1239,8 @@ namespace MobileGL::MG_Backend::DirectGLES {
// An image-less default texture (name 0) is the slot's initial / "unbound"
// state; it has nothing to sync, so skip it as cheaply as the old null slot.
if (textureObject && !MG_State::GLState::IsUndefinedDefaultTexture(textureObject.get())) {
g_unitTextureSyncList.push_back(
{&textureObject, SyncTextureObjectToBackend(textureObject).get()});
g_unitTextureSyncList.push_back({&textureObject, textureObject.get(),
SyncTextureObjectToBackend(textureObject).get()});
}
}
}
@@ -1254,7 +1274,8 @@ namespace MobileGL::MG_Backend::DirectGLES {
g_fboTextureSyncListSlotVersion == fboSlotVersion &&
g_fboTextureSyncListObjectVersion == fboObjectVersion &&
g_fboTextureSyncListContextId == keys.contextId &&
g_fboTextureSyncListContextGeneration == g_textureContextGeneration;
g_fboTextureSyncListContextGeneration == g_textureContextGeneration &&
PairingsIntact(g_fboTextureSyncList);
if (fboListValid) {
for (const auto& entry : g_fboTextureSyncList) {
// Same aggregate gate as the unit list above.
@@ -1273,8 +1294,8 @@ namespace MobileGL::MG_Backend::DirectGLES {
if (!attachment.IsTexture()) continue;
auto& textureObject = attachment.GetTexture();
if (textureObject) {
g_fboTextureSyncList.push_back(
{&textureObject, SyncTextureObjectToBackend(textureObject).get()});
g_fboTextureSyncList.push_back({&textureObject, textureObject.get(),
SyncTextureObjectToBackend(textureObject).get()});
}
}
g_fboTextureSyncListFbo = currentFBO.get();
MobileGL/MG_Backend/DirectGLES/Managers.h
+4
View File
@@ -630,6 +630,10 @@ namespace MobileGL::MG_Backend::DirectGLES {
SharedPtr<BackendTextureObject>& SyncTextureObjectToBackend(
const SharedPtr<MG_State::GLState::ITextureObject>& textureObject,
Bool imageBindableStorageRequired = false);
// Brings every texture the next draw reads - the touched units' bindings and the draw
// FBO's texture attachments - onto the backend, through the two borrowed-pair memos
// documented at their definitions. Declared here so tests can drive those memos directly.
void SyncNeccessaryTextures();
extern Array<Array<BackendTextureObject*, (SizeT)TextureTarget::TextureTargetCount>,
MG_State::GLState::TextureState::MAX_TEXTURE_IMAGE_UNITS>
g_boundTexturesCache;
MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp
+51 -3
View File
@@ -86,17 +86,65 @@ namespace MobileGL::MG_Impl::GLImpl {
return false;
}
// DSA emulation: the by-name entry points are implemented by putting the named texture
// on the active unit's slot for their target, running the classic bound-texture code,
// then putting the previous binding back.
//
// Both of those binds are REAL changes to "which texture is bound at this unit" for as
// long as `fn` runs, so both have to move the texture bind generation. Backends memoise
// per-unit work keyed on that generation and BORROW the binding slot (they hold a
// pointer to the slot's shared_ptr, not a copy); a slot swap the generation never saw
// let such a memo replay texture A's backend twin against texture B now sitting in the
// slot - which re-specified A's backend storage with B's shape and silently destroyed
// A's GPU-rendered contents (Minecraft's lightmap, blanked by a by-name upload to an
// Iris shadow map, which then discarded every glyph).
//
// The generation is bumped directly rather than through NoteTextureUnitTouched because
// the touched-unit HIGH-WATER MARK must NOT move: glActiveTexture does not advance it,
// so a DSA-only app would otherwise have every later draw walk up to the highest unit it
// ever aimed a by-name call at. Not advancing it is also sufficient - a unit above the
// mark is outside every memo's coverage and outside the epoch walk, so nothing can
// observe the transient swap there; at or below it, the bump is exactly what makes the
// epoch re-derive. Bumping only on a real change keeps the very common redundant case (a
// by-name call on the texture already bound to the active unit) free.
//
// The restore is a scope guard because `fn` can throw (the unsupported-state paths use
// THROW_EXCEPTION): leaking the temporary binding would leave the wrong texture bound to
// a live unit for the rest of the context's life.
template <typename Fn>
void WithTemporarilyBoundNamedTexture(const SharedPtr<MG_State::GLState::ITextureObject>& textureObject,
Fn&& fn) {
if (!textureObject) return;
auto& activeUnit = MG_State::pGLContext->GetTextureUnitObject(MG_State::pGLContext->GetActiveTextureUnit());
const Int activeUnitIndex = MG_State::pGLContext->GetActiveTextureUnit();
auto& activeUnit = MG_State::pGLContext->GetTextureUnitObject(activeUnitIndex);
auto& bindingSlot = activeUnit.GetBindingSlot(textureObject->GetTarget());
const auto previousBinding = bindingSlot.GetBoundObject();
bindingSlot.Bind(textureObject);
using SlotType = std::remove_reference_t<decltype(bindingSlot)>;
class ScopedSlotRestore {
public:
ScopedSlotRestore(SlotType& slot, SharedPtr<MG_State::GLState::ITextureObject> previous)
: m_slot(slot), m_previous(Move(previous)) {}
~ScopedSlotRestore() {
if (m_slot.Bind(m_previous)) {
MG_State::pGLContext->BumpTextureBindGeneration();
}
}
ScopedSlotRestore(const ScopedSlotRestore&) = delete;
ScopedSlotRestore& operator=(const ScopedSlotRestore&) = delete;
private:
SlotType& m_slot;
SharedPtr<MG_State::GLState::ITextureObject> m_previous;
};
if (bindingSlot.Bind(textureObject)) {
MG_State::pGLContext->BumpTextureBindGeneration();
}
ScopedSlotRestore restore(bindingSlot, previousBinding);
fn(MG_Util::ConvertTextureTargetToGLEnum(textureObject->GetTarget()));
bindingSlot.Bind(previousBinding);
}
SizeT ComputeTextureStorageByteSize(TextureInternalFormat textureInternalFormat, GLsizei width, GLsizei height,
MobileGL/MG_Test/SanityTest.cpp
+129
View File
@@ -1898,3 +1898,132 @@ TEST(FastSTLSanity, ErasingTheOnlyElementReturnsEnd) {
EXPECT_EQ(next, map.end());
EXPECT_TRUE(map.empty());
}
namespace {
// Records what the per-unit texture sync actually pushed at the driver: which backend
// texture id was current when each glTexImage2D landed, and the shape it was given.
struct TexSpecCall {
GLuint texture;
GLsizei width;
GLsizei height;
};
MobileGL::Vector<TexSpecCall>* g_texSpecCalls = nullptr;
GLuint g_texSpecBoundTexture = 0;
void TS_BindTexture(GLenum, GLuint texture) { g_texSpecBoundTexture = texture; }
void TS_ActiveTexture(GLenum) {}
void TS_TexParameteri(GLenum, GLenum, GLint) {}
void TS_TexParameterf(GLenum, GLenum, GLfloat) {}
void TS_TexParameterfv(GLenum, GLenum, const GLfloat*) {}
void TS_PixelStorei(GLenum, GLint) {}
void TS_BindBuffer(GLenum, GLuint) {}
void TS_TexImage2D(GLenum, GLint level, GLint, GLsizei width, GLsizei height, GLint, GLenum, GLenum,
const void*) {
if (g_texSpecCalls && level == 0) {
g_texSpecCalls->push_back({g_texSpecBoundTexture, width, height});
}
}
// Clears the recording hook even when a gtest assertion unwinds the test body.
struct ScopedTexSpecRecording {
explicit ScopedTexSpecRecording(MobileGL::Vector<TexSpecCall>& sink) {
g_texSpecCalls = &sink;
g_texSpecBoundTexture = 0;
}
~ScopedTexSpecRecording() { g_texSpecCalls = nullptr; }
ScopedTexSpecRecording(const ScopedTexSpecRecording&) = delete;
ScopedTexSpecRecording& operator=(const ScopedTexSpecRecording&) = delete;
};
// Gives `name` a complete single-level 2D image of the requested size without going through
// the frontend upload path (the mock table below wires only the state-pushing entry points).
MobileGL::SharedPtr<MobileGL::MG_State::GLState::ITextureObject> MakeComplete2DTexture(GLuint name,
MobileGL::Int size) {
using namespace MobileGL;
MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, name);
auto object = MG_State::pGLContext->GetTextureUnitObject(0)
.GetBindingSlot(TextureTarget::Texture2D)
.GetBoundObject();
object->SetInternalFormat(TextureInternalFormat::RGBA8);
MG_State::GLState::AsMipmapTexture(object.get())
->AllocateStorage(TextureUploadTarget::Texture2D, 0, {{size, size, 1}, 4});
return object;
}
} // namespace
// The per-unit texture sync memo BORROWS the binding slot: an entry holds a pointer to the
// slot's shared_ptr plus the backend twin of whatever was in it when the entry was built. Its
// keys (context id, bind-generation epoch, high-water mark, sampling generation) are the primary
// guard, but they are all derived state - so the memo also has to survive a slot swap that never
// reached them.
//
// It did not. The DSA by-name emulation swapped a slot silently, every key still matched, and
// the replay drove texture A's backend twin from texture B's frontend object: A's backend
// storage was re-specified with B's shape, destroying anything A only ever had on the GPU. On
// Espryt + Iris/BSL that blanked Minecraft's 16x16 lightmap the moment a 2048x2048 shadow map
// was uploaded through a by-name call, and since the text shader multiplies by the lightmap,
// `if (color.a < 0.1) discard` then threw away every glyph in the process - HUD, menus and the
// vanilla title screen alike.
TEST(DirectGLESTextureSync, UnitMemoRefusesToDriveATwinFromAnotherTexture) {
using namespace MobileGL;
ScopedDirectGLESTextureBindings scoped; // fresh GLContext + registry + binding caches
Vector<TexSpecCall> specs;
ScopedTexSpecRecording recording(specs);
auto functions = MG_Backend::DirectGLES::g_GLESFuncs;
functions.glBindTexture = TS_BindTexture;
functions.glActiveTexture = TS_ActiveTexture;
functions.glTexImage2D = TS_TexImage2D;
functions.glTexParameteri = TS_TexParameteri;
functions.glTexParameterf = TS_TexParameterf;
functions.glTexParameterfv = TS_TexParameterfv;
functions.glPixelStorei = TS_PixelStorei;
functions.glBindBuffer = TS_BindBuffer;
MG_Backend::DirectGLES::SetGLESFuncsTable(functions);
GLuint names[2] = {};
MG_Impl::GLImpl::GenTextures(2, names);
// `foreign` stands in for the shadow map, `resident` for the lightmap. Both are fully
// specified BEFORE the first sync so that nothing between the two syncs can move the
// sampling-resolution generation and invalidate the memo for an unrelated reason.
const auto foreign = MakeComplete2DTexture(names[1], 32);
const auto resident = MakeComplete2DTexture(names[0], 16);
ASSERT_NE(foreign, nullptr);
ASSERT_NE(resident, nullptr);
// First sync: builds the memo with unit 0 -> `resident`, and gives `resident`'s twin its
// 16x16 backend storage.
MG_Backend::DirectGLES::TextureImpl::SyncNeccessaryTextures();
auto* residentSlot = MG_Backend::DirectGLES::TextureImpl::g_backendTextureObjects.Find(resident.get());
ASSERT_NE(residentSlot, nullptr);
ASSERT_NE(*residentSlot, nullptr);
const GLuint residentBackendId = (*residentSlot)->GetBackendTextureId();
ASSERT_NE(residentBackendId, 0u);
ASSERT_FALSE(specs.empty());
EXPECT_EQ(specs.back().texture, residentBackendId);
EXPECT_EQ(specs.back().width, 16);
// The hazard, reproduced at the state level: put `foreign` on the slot the memo borrows
// WITHOUT telling the binding accounting, exactly as the by-name emulation used to.
MG_State::pGLContext->GetTextureUnitObject(0).GetBindingSlot(TextureTarget::Texture2D).Bind(foreign);
const SizeT specsBeforeReplay = specs.size();
MG_Backend::DirectGLES::TextureImpl::SyncNeccessaryTextures();
// `foreign` must have been synced through its OWN twin...
auto* foreignSlot = MG_Backend::DirectGLES::TextureImpl::g_backendTextureObjects.Find(foreign.get());
ASSERT_NE(foreignSlot, nullptr);
ASSERT_NE(*foreignSlot, nullptr);
const GLuint foreignBackendId = (*foreignSlot)->GetBackendTextureId();
EXPECT_NE(foreignBackendId, residentBackendId);
// ...and above all, nothing may have re-specified the RESIDENT texture's backend storage.
// That single call is what destroyed the lightmap.
for (SizeT i = specsBeforeReplay; i < specs.size(); ++i) {
EXPECT_NE(specs[i].texture, residentBackendId)
<< "the stale memo entry re-specified the resident texture's backend storage with "
<< specs[i].width << "x" << specs[i].height;
}
MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, 0);
}
MobileGL/MG_Test/Texture/TextureTest.cpp
+50
View File
@@ -2835,3 +2835,53 @@ TEST_F(TextureTest, BindSamplerRejectsUnitsBeyondMaxCombinedTextureImageUnits) {
MG_Impl::GLImpl::DeleteSamplers(1, &sampler);
DrainPendingGlErrors();
}
// The DSA by-name entry points are emulated by temporarily binding the named texture onto the
// active unit's slot for its target, running the classic bound-texture code, then putting the
// previous binding back. For as long as the emulated call runs, that swap is a REAL change to
// which texture is bound at that unit, so both transitions have to move the texture bind
// generation.
//
// They used to move nothing. Backends memoise per-unit work keyed on the bind generation and
// BORROW the binding slot (they hold a pointer to the slot's shared_ptr, not a copy), so a memo
// built while texture A sat in the slot stayed "valid" while B was temporarily in it - and the
// backend then drove A's backend twin from B's frontend state, re-specifying A's backend storage
// with B's shape. Any content A only ever had on the GPU was gone. That is what blanked
// Minecraft's lightmap when Iris uploaded to a BSL shadow map: the text shader multiplies by the
// lightmap, so `if (color.a < 0.1) discard` then threw away every glyph in the process.
TEST_F(TextureTest, NamedTextureCallKeepsUnitBindingAccountingCoherent) {
GLuint names[2] = {};
MG_Impl::GLImpl::GenTextures(2, names);
const GLuint boundName = names[0];
const GLuint namedName = names[1];
MG_Impl::GLImpl::ActiveTexture(GL_TEXTURE0);
// Instantiate both as 2D objects, then leave `boundName` on the unit.
MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, namedName);
MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, boundName);
ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
auto& slot = MG_State::pGLContext->GetTextureUnitObject(0).GetBindingSlot(TextureTarget::Texture2D);
const auto boundObject = slot.GetBoundObject();
ASSERT_NE(boundObject, nullptr);
ASSERT_EQ(boundObject->GetExternalIndex(), boundName);
// TextureParameteriv is one of the by-name calls that is emulated by binding: it reaches
// WithTemporarilyBoundNamedTexture, unlike the scalar TextureParameteri, which edits the
// object directly and never touches a unit.
const Uint64 base = MG_State::pGLContext->GetTextureBindGeneration();
const GLint maxLevel = 0;
MG_Impl::GLImpl::TextureParameteriv(namedName, GL_TEXTURE_MAX_LEVEL, &maxLevel);
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
// The emulation put `namedName` on the unit and took it off again. A generation-keyed memo
// must be able to see that the slot it borrows was not stable across the call.
EXPECT_GT(MG_State::pGLContext->GetTextureBindGeneration(), base)
<< "a by-name texture call swapped a live unit binding without moving the bind generation";
// ...and the application-visible binding is exactly what it was before the call.
EXPECT_EQ(slot.GetBoundObject(), boundObject);
MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, 0);
MG_Impl::GLImpl::DeleteTextures(2, names);
DrainPendingGlErrors();
}