From 735b70539764fc5e34500af91f852efc132a6597 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Mon, 12 Oct 2026 23:13:18 -0400 Subject: [PATCH] [Fix] (Espryt): invalid image units read zero through a shaped placeholder --- MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 153 ++++++++++++++++++ .../DirectVulkan/Renderer/UniformManager.cpp | 6 +- MobileGL/MG_Impl/Pipe/ImageEmit.h | 11 +- MobileGL/MG_Pipe/MGPipeTypes.h | 6 + MobileGL/MG_Test/Pipe/ImageEmitTest.cpp | 13 +- 5 files changed, 177 insertions(+), 12 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index 4e7aa868..738bb0e4 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -2819,6 +2819,144 @@ namespace MobileGL::MG_Backend::DirectGLES { static_cast(record->Desc.StorageKind) == TextureStorageType::Buffer; } + // GL 4.6 core 8.26: an access through an image unit whose texture does not HAVE the + // (level, layer) the unit names is INVALID - a load returns zero, a store does nothing, + // and neither is an error. The ES storage behind such a unit may well have that level + // (a mutable texture's storage is allocated for its whole chain the first time it is + // image-bound), and the driver would read whatever that undefined level holds. The + // client sends the bindings GL calls incomplete with level kMGPipeImageLevelNone + // (KHR-GL46.shader_image_load_store.incomplete_textures binds level 2 of a texture that + // defines only level 0); a level or layer past the storage is caught here. Decided from + // the applier's records alone, by the same walk the Vulkan backend makes + // (VkTextureManager::ResolveWireTextureStorage): through texture views to the root, + // against each window's levels and layers. + static Bool ImageViewNamesNoTexel(const MG_Pipe::MGPImageView& image) { + using T = MG_Pipe::MGPipeResourceTarget; + const auto& state = MG_Pipe::MGPipeApplier(); + MG_Pipe::MGPipeHandle handle = image.Res; + if (MG_Pipe::MGPipeHandleIsNull(handle) || handle.Slot >= state.TextureResources.size()) return false; + const auto& first = state.TextureResources[handle.Slot]; + if (!first.Live || first.Gen != handle.Gen) return false; + // GL ignores Layer for a target with no layers, and for a layered bind. + const auto firstTarget = static_cast(first.Desc.Target); + const Bool selectsLayer = !image.Layered && + (firstTarget == T::Tex1DArray || firstTarget == T::Tex2DArray || firstTarget == T::Tex2DMSArray || + firstTarget == T::Tex3D || firstTarget == T::TexCube || firstTarget == T::TexCubeArray); + Uint32 level = image.Level; + Uint32 layer = selectsLayer ? image.Layer : 0; + for (SizeT hop = 0; hop <= state.TextureResources.size(); ++hop) { + if (MG_Pipe::MGPipeHandleIsNull(handle) || handle.Slot >= state.TextureResources.size()) return false; + const auto& record = state.TextureResources[handle.Slot]; + if (!record.Live || record.Gen != handle.Gen) return false; + if (MG_Pipe::MGPipeHandleIsNull(record.Desc.ViewOf)) { + if (static_cast(record.Desc.Target) == T::TexBuffer) return false; + if (level >= record.Desc.Levels) return true; + const Uint32 layers = static_cast(record.Desc.Target) == T::Tex3D + ? std::max(record.Desc.Depth >> level, 1u) + : std::max(record.Desc.ArrayLayers, 1u); + return layer >= layers; + } + const auto viewHandle = record.ViewCso; + if (MG_Pipe::MGPipeHandleIsNull(viewHandle) || viewHandle.Slot >= state.SamplerViewCsos.size()) + return false; + const auto& viewRecord = state.SamplerViewCsos[viewHandle.Slot]; + const auto& view = viewRecord.View; + if (!viewRecord.Live || viewRecord.Gen != viewHandle.Gen || view.Texture != handle) return false; + if (level >= view.NumLevels || layer >= view.NumLayers) return true; + level += view.MinLevel; + layer += view.MinLayer; + handle = record.Desc.ViewOf; + } + return false; + } + + // What such a unit is bound to instead. An EMPTY ES unit is not enough: ES 3.2 8.22 leaves + // an invalid load's alpha undefined, and Adreno answers 1 where GL says 0. A texel outside + // a level is invalid too, and that Adreno answers (0, 0, 0, 0) with the store dropped - so + // the unit gets a 1x1 texture of the shape and format the bind names, cleared to zero each + // time it is bound. Only texel (0, 0[, 0]) of it exists; it is private to the unit, so a + // store through one invalid unit cannot reach a load through another. Every context of the + // process shares one group (CreateContextTuple), so one set serves them all. + struct InvalidImagePlaceholder { + GLuint Id = 0; + GLenum Target = 0; + GLenum InternalFormat = 0; + }; + static Array + g_invalidImagePlaceholders{}; + + // Zero when the shape has no 1x1 ES form (multisample, buffer) or the driver refused it; + // the caller then leaves the unit empty, which is right for every channel but alpha. + static GLuint EnsureInvalidImagePlaceholder(Uint unit, TextureTarget textureTarget, GLenum internalFormat) { + GLenum target = 0; + GLsizei depth = 1; + switch (TextureImpl::MapToBackendTextureTarget(textureTarget)) { + case TextureTarget::Texture2D: target = GL_TEXTURE_2D; break; + case TextureTarget::Texture2DArray: target = GL_TEXTURE_2D_ARRAY; break; + case TextureTarget::Texture3D: target = GL_TEXTURE_3D; break; + case TextureTarget::TextureCubeMap: target = GL_TEXTURE_CUBE_MAP; break; + case TextureTarget::TextureCubeMapArray: target = GL_TEXTURE_CUBE_MAP_ARRAY; depth = 6; break; + default: return 0; + } + GLenum format = 0, type = 0; + MG_Util::TextureFormatProcessor::NormalizePixelFormat(internalFormat, Flags{}, + nullptr, &format, &type); + if (format == 0 || type == 0) return 0; + InvalidImagePlaceholder& slot = g_invalidImagePlaceholders[unit]; + if (slot.Id != 0 && (slot.Target != target || slot.InternalFormat != internalFormat)) { + g_GLESFuncs.glDeleteTextures(1, &slot.Id); + slot = {}; + } + GLint previous = 0; + const GLenum bindingQuery = target == GL_TEXTURE_2D ? GL_TEXTURE_BINDING_2D + : target == GL_TEXTURE_2D_ARRAY ? GL_TEXTURE_BINDING_2D_ARRAY + : target == GL_TEXTURE_3D ? GL_TEXTURE_BINDING_3D + : target == GL_TEXTURE_CUBE_MAP ? GL_TEXTURE_BINDING_CUBE_MAP + : GL_TEXTURE_BINDING_CUBE_MAP_ARRAY; + g_GLESFuncs.glGetIntegerv(bindingQuery, &previous); + DebugImpl::ErrorLopper::Clear(); + if (slot.Id == 0) { + g_GLESFuncs.glGenTextures(1, &slot.Id); + g_GLESFuncs.glBindTexture(target, slot.Id); + if (depth > 1 || target == GL_TEXTURE_2D_ARRAY || target == GL_TEXTURE_3D) { + g_GLESFuncs.glTexStorage3D(target, 1, internalFormat, 1, 1, depth); + } else { + g_GLESFuncs.glTexStorage2D(target, 1, internalFormat, 1, 1); + } + slot.Target = target; + slot.InternalFormat = internalFormat; + } else { + g_GLESFuncs.glBindTexture(target, slot.Id); + } + // Six faces of the widest texel (rgba32) is the most any shape here holds. + static constexpr Uint8 kZeros[6 * 16] = {}; + // The backend unpack state rests at the ES default (Managers.cpp + // ScopedDefaultUnpackState), and a single row ignores the alignment. + BufferImpl::BindPixelUnpackBufferId(0); + if (target == GL_TEXTURE_CUBE_MAP) { + for (GLenum face = 0; face < 6; ++face) + g_GLESFuncs.glTexSubImage2D(GL_TEXTURE_CUBE_MAP_POSITIVE_X + face, 0, 0, 0, 1, 1, format, type, + kZeros); + } else if (target == GL_TEXTURE_2D) { + g_GLESFuncs.glTexSubImage2D(target, 0, 0, 0, 1, 1, format, type, kZeros); + } else { + g_GLESFuncs.glTexSubImage3D(target, 0, 0, 0, 0, 1, 1, depth, format, type, kZeros); + } + g_GLESFuncs.glBindTexture(target, static_cast(previous)); + Bool failed = false; + DebugImpl::ErrorLopper::Loop([&failed, internalFormat](GLenum err) { + failed = true; + MGLOG_D("DirectGLES: invalid-image placeholder (%s) refused: %s", + MG_Util::ConvertGLEnumToString(internalFormat).c_str(), + MG_Util::ConvertGLEnumToString(err).c_str()); + }); + if (failed) { + g_GLESFuncs.glDeleteTextures(1, &slot.Id); + slot = {}; + } + return slot.Id; + } + // The record form. `Res` is the texture, the four well-defined fields are the record's, // Access is decoded, and the texture's own TARGET and STORAGE FORMAT - the two // properties the bind rules need and MGPImageView has no room for (24 bytes, no pad) - @@ -2849,6 +2987,21 @@ namespace MobileGL::MG_Backend::DirectGLES { g_GLESFuncs.glBindImageTexture(unit, 0, 0, GL_FALSE, 0, GL_READ_ONLY, GL_RGBA8); return; } + if (ImageViewNamesNoTexel(view)) { + // The format the shader was widened from is the one the application bound, so + // the placeholder takes that format's carrier, as the storage would have. + GLenum format = static_cast(view.InternalFormat); + if (const auto widening = TextureImpl::GetImageBindableStorageWidening( + MG_Util::ConvertGLEnumToTextureInternalFormat(format))) { + format = widening.InternalFormat; + } + const GLuint placeholder = + EnsureInvalidImagePlaceholder(unit, PipeTextureTargetForHandle(view.Res), format); + const GLboolean layered = placeholder != 0 && view.Layered != 0 ? GL_TRUE : GL_FALSE; + g_GLESFuncs.glBindImageTexture(unit, placeholder, 0, layered, 0, GLAccessForImageView(view), + placeholder != 0 ? format : GL_RGBA8); + return; + } const auto textureFormat = static_cast(resource->Desc.InternalFormat); const TextureTarget textureTarget = PipeTextureTargetForHandle(view.Res); const GLenum access = GLAccessForImageView(view); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp index 3cfef5fc..acdb4c93 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp @@ -121,9 +121,9 @@ namespace MobileGL::MG_Backend::DirectVulkan { // to find a texel's storage, and from that walk's "not there" answer ONLY: a null for any // other reason (a dead record, a stale view CSO) stays the descriptor resolve's refusal. // - // NOT DECIDED HERE, deliberately: a level that IS in storage but lies outside - // [BASE_LEVEL, MAX_LEVEL], or of a texture that is not mipmap-complete. The monolith arm - // binds those as it finds them too, and the P7 gate 5 comparison is against that arm. + // A level that IS in storage but lies outside [BASE_LEVEL, MAX_LEVEL], or of a texture that + // is not mipmap-complete, is decided by the client, which sends it as + // kMGPipeImageLevelNone - a level no texture has, so this walk answers it too. static Bool WireShaderImageNamesNoTexel(VkTextureManager& textures, const MG_Pipe::MGPImageView& image) { const auto& state = MG_Pipe::MGPipeApplier(); if (MG_Pipe::MGPipeHandleIsNull(image.Res) || image.Res.Slot >= state.TextureResources.size()) return false; diff --git a/MobileGL/MG_Impl/Pipe/ImageEmit.h b/MobileGL/MG_Impl/Pipe/ImageEmit.h index 7faf7c22..423c6888 100644 --- a/MobileGL/MG_Impl/Pipe/ImageEmit.h +++ b/MobileGL/MG_Impl/Pipe/ImageEmit.h @@ -77,8 +77,10 @@ namespace MobileGL::MG_Pipe { // units ignore the sampler, so "complete" here is the level's own: the base level needs // base-level completeness and any other level a complete mip chain up to it, the reading // the conformance suite applies (a mutable texture with only level 0 defined, bound at - // level 2). Such a binding travels as an EMPTY unit, which both backends already answer - // with "no image" - an ES driver would otherwise happily address the undefined level. + // level 2). Such a binding travels with its texture and level kMGPipeImageLevelNone, which + // both backends answer with a stand-in of the texture's shape that reads zero - an ES driver + // would otherwise happily address the undefined level, and an EMPTY ES unit reads alpha 1 on + // Adreno where GL says 0. inline Bool MGPipeImageBindingIsAccessible(const MG_State::GLState::ImageTextureBinding& binding) { using namespace MG_State::GLState; const ITextureObject* texture = binding.Texture.get(); @@ -158,7 +160,7 @@ namespace MobileGL::MG_Pipe { MGPImageView& entry = m_entries[unit]; entry = MGPImageView{}; entry.Unit = unit; - entry.Res = MGPipeImageBindingIsAccessible(binding) + entry.Res = binding.Texture != nullptr ? MGPipeSlots().Acquire(MGPipeKind::Texture, binding.Texture->GetLifetimeId()) : kMGPipeNullHandle; // D-A4: a texture named in an emitted MGPImageView is SHADER-IMAGE-bound from @@ -176,7 +178,8 @@ namespace MobileGL::MG_Pipe { // that would be answering a driver question from the wrong side. entry.InternalFormat = static_cast(binding.Format); entry.Layer = static_cast(binding.Layer); - entry.Level = static_cast(binding.Level); + entry.Level = MGPipeImageBindingIsAccessible(binding) ? static_cast(binding.Level) + : kMGPipeImageLevelNone; entry.Layered = binding.Layered != GL_FALSE ? 1 : 0; entry.Access = static_cast(MGPipeEncodeImageAccess(binding.Access)); } diff --git a/MobileGL/MG_Pipe/MGPipeTypes.h b/MobileGL/MG_Pipe/MGPipeTypes.h index 6eca4ff0..443a7d77 100644 --- a/MobileGL/MG_Pipe/MGPipeTypes.h +++ b/MobileGL/MG_Pipe/MGPipeTypes.h @@ -993,6 +993,12 @@ namespace MobileGL::MG_Pipe { }; MGP_ASSERT_POD(MGPImageView, 24); + // MGPImageView::Level of a binding GL 4.6 core 8.26 calls INVALID (an incomplete level, one + // outside [BASE_LEVEL, MAX_LEVEL], a level the texture does not have): Res still names the + // texture, so a backend can shape the stand-in it binds after it, and no texture has this + // level, so every "is that texel there" walk answers no. + inline constexpr Uint16 kMGPipeImageLevelNone = 0xFFFF; + struct MGPShaderImages { Uint32 Start, Count; Uint64 ContentHash; diff --git a/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp b/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp index 8eac3859..b1d85349 100644 --- a/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp @@ -609,9 +609,10 @@ void main() { imageStore(img, ivec2(0, 0), vec4(1.0)); } // GL 4.6 core 8.26: a level the texture does not have as a complete image - here a mutable // texture with only level 0 defined and the default MAX_LEVEL, bound at level 2 - is an - // invalid access, so the unit travels EMPTY and the backend binds no image (loads read zero, - // stores are dropped). The same texture bound at its base level stays a real binding. - TEST(ImageEmit, AnIncompleteLevelTravelsAsAnEmptyUnit) { + // invalid access, so the unit travels with level kMGPipeImageLevelNone and the backend binds + // a stand-in (loads read zero, stores are dropped). The same texture bound at its base level + // stays a real binding. + TEST(ImageEmit, AnIncompleteLevelTravelsWithNoLevel) { EmitterScope scope; const GLuint program = MakeComputeProgram(kImageCompute); GL::UseProgram(program); @@ -624,12 +625,14 @@ void main() { imageStore(img, ivec2(0, 0), vec4(1.0)); } ASSERT_TRUE(Ctx().GetImageTextureBinding(1).Texture != nullptr) << "the bind itself is legal"; Emitter().EmitShaderImages(Ctx()); ASSERT_GE(Emitter().Window(), 2u); - EXPECT_TRUE(MGPipeHandleIsNull(Emitter().LastImageViews()[1].Res)) + EXPECT_FALSE(MGPipeHandleIsNull(Emitter().LastImageViews()[1].Res)) + << "the texture still names the stand-in's shape"; + EXPECT_EQ(Emitter().LastImageViews()[1].Level, kMGPipeImageLevelNone) << "an undefined, mip-incomplete level must not reach the backend as an image"; GL::BindImageTexture(1, name, 0, GL_FALSE, 0, GL_READ_WRITE, GL_RGBA8); Emitter().EmitShaderImages(Ctx()); - EXPECT_FALSE(MGPipeHandleIsNull(Emitter().LastImageViews()[1].Res)) + EXPECT_EQ(Emitter().LastImageViews()[1].Level, 0u) << "the base level is base-complete and must stay bound"; GL::UseProgram(0); GL::BindImageTexture(1, 0, 0, GL_FALSE, 0, GL_READ_ONLY, GL_RGBA8);