diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.cpp index e70cbc6b..43e21947 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.cpp @@ -252,6 +252,19 @@ namespace MobileGL::MG_Backend::DirectVulkan { } VkPipeline pipeline = CreatePipeline(payload); + // A failed creation must never be memoized. Caching VK_NULL_HANDLE served the null back for + // the rest of the process, so one transient driver rejection turned every later draw with + // the same state into a vkCmdBindPipeline(VK_NULL_HANDLE) - the SIGSEGV behind 9 of the 15 + // CTS process deaths. Retrying costs one failed vkCreateGraphicsPipelines per draw, which + // is the correct price for a broken pipeline and is bounded by the draw itself being + // skipped. + if (pipeline == VK_NULL_HANDLE) { + MGLOG_I("PipelineFactory::GetOrCreatePipeline: creation failed for hash=0x%llx " + "programHash=0x%llx; not caching the failure", + static_cast(hash), + static_cast(payload.programHash)); + return VK_NULL_HANDLE; + } m_cache.emplace(hash, PipelineCacheEntry{pipeline, payload.programHash, payload.renderPass, m_frameCounter}); return pipeline; @@ -507,6 +520,31 @@ namespace MobileGL::MG_Backend::DirectVulkan { MGLOG_F("PipelineFactory::CreatePipeline vertex input: bindingCount=%u attributeCount=%u", payload.vertexInputState->vertexBindingDescriptionCount, payload.vertexInputState->vertexAttributeDescriptionCount); + // The driver's own answer is VK_ERROR_UNKNOWN, i.e. no information at all, so the only + // way to work out WHICH shader it choked on (the open sampler-array-in-struct + // investigation) is to name the modules. MGLOG_I, not _D/_E: this must survive in the + // INFO-level builds that CTS actually runs against. + if (payload.stageSpirvDigests) { + for (SizeT i = 0; i < payload.stageSpirvDigests->size(); ++i) { + const auto& digest = (*payload.stageSpirvDigests)[i]; + MGLOG_I("PipelineFactory::CreatePipeline spirv[%zu]: stage=0x%x words=%u bytes=%zu " + "hash=0x%llx", + i, digest.stage, digest.wordCount, + static_cast(digest.wordCount) * sizeof(Uint32), + static_cast(digest.hash)); + } + } else { + MGLOG_I("PipelineFactory::CreatePipeline: no SPIR-V digests attached to the payload"); + } + if (payload.stages) { + for (SizeT i = 0; i < payload.stages->size(); ++i) { + const auto& stage = (*payload.stages)[i]; + MGLOG_I("PipelineFactory::CreatePipeline stage[%zu]: stage=0x%x module=%p entry=%s " + "specialization=%d", + i, static_cast(stage.stage), static_cast(stage.module), + stage.pName ? stage.pName : "(null)", stage.pSpecializationInfo ? 1 : 0); + } + } for (Uint32 i = 0; i < payload.colorAttachmentCount; ++i) { const auto& attachment = payload.colorBlendAttachments[i]; MGLOG_F("PipelineFactory::CreatePipeline colorAttachment[%u]: blend=%d colorWriteMask=0x%x srcColor=%d dstColor=%d colorOp=%d srcAlpha=%d dstAlpha=%d alphaOp=%d", diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.h index 5bdbfdb1..77948c4c 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.h @@ -14,6 +14,16 @@ #include namespace MobileGL::MG_Backend::DirectVulkan { + // Enough of a fingerprint to identify the exact module the driver rejected without keeping the + // SPIR-V alive for every program in the cache: a driver that answers VK_ERROR_UNKNOWN tells us + // nothing, so the log has to carry the shader's identity itself. Diagnostic only - never part + // of any pipeline or program hash. + struct ShaderStageSpirvDigest { + Uint32 stage = 0; // VkShaderStageFlagBits + Uint32 wordCount = 0; + Uint64 hash = 0; + }; + class PipelineFactory { public: using HashType = Uint64; @@ -62,6 +72,8 @@ namespace MobileGL::MG_Backend::DirectVulkan { Array colorBlendAttachments{}; const Vector* stages = nullptr; const VkPipelineVertexInputStateCreateInfo* vertexInputState = nullptr; + // Diagnostic only; may be null. Read solely from the pipeline-creation failure path. + const Vector* stageSpirvDigests = nullptr; }; explicit PipelineFactory(VkDevice device, const VulkanRendererConfig& config); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp index bfb89d13..c121f133 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp @@ -2568,6 +2568,9 @@ namespace MobileGL::MG_Backend::DirectVulkan { entry.modules.push_back(module); entry.stages.push_back(stage); + entry.stageSpirvDigests.push_back(ShaderStageSpirvDigest{ + static_cast(stage.stage), static_cast(moduleSpv.size()), + XXH64(moduleSpv.data(), moduleSpv.size() * sizeof(Uint), 0)}); } // Reflect and create layout as part of the program object diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h index 0f8570f3..942dd698 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h @@ -9,6 +9,7 @@ #pragma once #include "../VkIncludes.h" +#include "PipelineFactory.h" #include "MG_State/GLState/ProgramState/ProgramObject.h" #include "MG_State/GLState/ProgramState/ShaderObject.h" #include "MG_State/GLState/TextureState/TextureEnum.h" @@ -62,6 +63,9 @@ namespace MobileGL::MG_Backend::DirectVulkan { HashType hash = 0; Vector stages; Vector modules; + // Parallel to stages; identifies the exact module bytes handed to the driver when a + // pipeline creation fails. Sixteen bytes per stage instead of keeping the SPIR-V. + Vector stageSpirvDigests; // Layout data (previously in separate VkProgramLayout) VkDescriptorSetLayout descriptorSetLayout = VK_NULL_HANDLE; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index 027fd5e3..6bbbd1ed 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -4049,7 +4049,8 @@ void main() { .depthWriteEnable = false, .depthCompareOp = VK_COMPARE_OP_ALWAYS, .stages = &programObj.stages, - .vertexInputState = &kEmptyVertexInputState + .vertexInputState = &kEmptyVertexInputState, + .stageSpirvDigests = &programObj.stageSpirvDigests }; static constexpr VkColorComponentFlags kColorWriteMask = VK_COLOR_COMPONENT_R_BIT | VK_COLOR_COMPONENT_G_BIT | @@ -4727,7 +4728,8 @@ void main() { .backStencilCompareOp = MG_Util::ConvertDepthTestFuncToVkEnum(backStencil.Func), .fragmentReplacesDepth = programObj.fragmentReplacesDepth, .stages = &programObj.stages, - .vertexInputState = pipelineVertexInputState + .vertexInputState = pipelineVertexInputState, + .stageSpirvDigests = &programObj.stageSpirvDigests }; if (!payload.stencilTestEnable) { payload.frontStencilFailOp = VK_STENCIL_OP_KEEP; @@ -5911,6 +5913,17 @@ void main() { } auto pipeline = GetOrCreatePipeline(mode, program, programObj, transformFlags, vao, *renderPassEntry); + // GetOrCreatePipeline documents a VK_NULL_HANDLE return (empty stages, or a driver that + // rejected vkCreateGraphicsPipelines). Binding it dereferences null inside the driver - + // 9 of the 15 CTS process deaths were exactly this vkCmdBindPipeline. A draw that has no + // pipeline is a skipped draw, which is what every other failure below already does. + // MGLOG_I so the skip is visible in the INFO builds CTS runs against. + if (pipeline == VK_NULL_HANDLE) { + MGLOG_I("SetupDraw skipped: no graphics pipeline for program=%u (creation failed or the " + "program has no shader stages)", + program.GetExternalIndex()); + return false; + } activeRenderPass = VkRenderPassManager::GetActiveRenderPass(); // Begin render pass, and handle clear diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index e17f8417..2112a071 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -53,6 +53,7 @@ add_executable(MobileGLIntegrationTest Scenarios/AsyncCompileScenario.cpp Scenarios/XfbAfterClipDistanceScenario.cpp Scenarios/ThreeChannelAttachmentScenario.cpp + Scenarios/PipelineFailureScenario.cpp ) target_include_directories(MobileGLIntegrationTest PRIVATE diff --git a/MobileGL/MG_IntegrationTest/Scenarios/PipelineFailureScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/PipelineFailureScenario.cpp new file mode 100644 index 00000000..ae58b7ad --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Scenarios/PipelineFailureScenario.cpp @@ -0,0 +1,197 @@ +// MobileGL - MobileGL/MG_IntegrationTest/Scenarios/PipelineFailureScenario.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header +// +// "The draw had no pipeline, so we bound null." +// +// DirectVulkan's SetupDraw called GetOrCreatePipeline - a function that DOCUMENTS a +// VK_NULL_HANDLE return - and passed the result straight to vkCmdBindPipeline. When the +// Adreno driver answered vkCreateGraphicsPipelines with VK_ERROR_UNKNOWN, the next +// instruction dereferenced null inside the driver: SIGSEGV at fault addr 0x8, and that one +// shape accounted for 9 of the 15 process deaths in the 2026-08-10 GL-CTS run +// (KHR-GL33/GL40.shaders.struct.uniform.sampler_array_vertex, six +// KHR-GL42.shader_image_load_store cases, one shader_storage_buffer_object case). +// +// It was made permanent by a second defect: PipelineFactory memoized the failure, so the +// null was served for the rest of the process. Every later draw with the same state died +// too, which is why a single bad program took whole CTS groups down with it. +// +// What this scenario pins, on both backends: +// 1. The GL program shape the CTS crashed on (an array of structs each containing a +// sampler, sampled from the VERTEX stage) draws without killing the process. +// 2. It draws AGAIN and produces the identical image. A second draw is the only thing +// that can tell a working pipeline apart from a poisoned cache entry: if the first +// creation had failed and been memoized, the second draw is where the null would be +// served back. +// +// A deterministic driver-side pipeline-creation FAILURE is not reachable from the GL API on +// the llvmpipe/lavapipe lanes - both accept every pipeline these scenarios can describe - so +// the guard itself is proven structurally (PipelineFactory returns before it can emplace a +// VK_NULL_HANDLE, SetupDraw returns false before it can bind one) and this scenario holds +// the surrounding path honest. + +#include +#include + +#include "../Harness/HeadlessGL.h" +#include "../Harness/ScenarioFixture.h" + +#ifdef GLAPI +#undef GLAPI +#endif +#define GL_GLEXT_PROTOTYPES +#include +#include +#undef GL_GLEXT_PROTOTYPES + +namespace MGITest { + namespace { + + // Lifted from KHR-GL33.shaders.struct.uniform.sampler_array_vertex (the QPA records the + // source verbatim): an array of structs, each carrying an opaque sampler, sampled in the + // vertex stage. The fragment sibling of this case only FAILS on Magma; only the vertex one + // takes the process down, so the stage matters and is kept. + constexpr const char* kSamplerArrayVertexSource = R"(#version 330 core +struct S { + float a; + vec3 b; + sampler2D c; +}; +uniform S s[2]; +in vec2 aPos; +out vec4 vColor; +void main() { + vec2 coords = aPos * 0.5 + 0.5; + vColor = vec4(texture(s[1].c, coords * s[0].b.xy + s[1].b.z).rgb, s[0].a); + gl_Position = vec4(aPos, 0.0, 1.0); +} +)"; + + constexpr const char* kPassthroughFragmentSource = R"(#version 330 core +in vec4 vColor; +out vec4 oColor; +void main() { + oColor = vColor; +} +)"; + + struct Vertex { + float x, y; + }; + + std::vector FullscreenTriangleStrip() { + return {{-1.0f, -1.0f}, {1.0f, -1.0f}, {-1.0f, 1.0f}, {1.0f, 1.0f}}; + } + + class PipelineFailureScenario : public ScenarioTest { + protected: + void SetUp() override { + ScenarioTest::SetUp(); + if (!Ready()) return; + + std::string error; + m_program = CompileProgram(kSamplerArrayVertexSource, kPassthroughFragmentSource, &error); + ASSERT_NE(m_program, 0u) << error; + + const std::vector vertices = FullscreenTriangleStrip(); + m_vertexCount = static_cast(vertices.size()); + glGenVertexArrays(1, &m_vao); + glBindVertexArray(m_vao); + glGenBuffers(1, &m_vbo); + glBindBuffer(GL_ARRAY_BUFFER, m_vbo); + glBufferData(GL_ARRAY_BUFFER, GLsizeiptr(vertices.size() * sizeof(Vertex)), vertices.data(), + GL_STATIC_DRAW); + glEnableVertexAttribArray(0); + glVertexAttribPointer(0, 2, GL_FLOAT, GL_FALSE, sizeof(Vertex), reinterpret_cast(0)); + glBindVertexArray(0); + + // A solid red 2x2 texture, so the sampled colour is the same wherever the + // (deliberately degenerate) coordinates land. + const unsigned char red[] = {255, 0, 0, 255, 255, 0, 0, 255, + 255, 0, 0, 255, 255, 0, 0, 255}; + glGenTextures(1, &m_texture); + glActiveTexture(GL_TEXTURE0); + glBindTexture(GL_TEXTURE_2D, m_texture); + glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, 2, 2, 0, GL_RGBA, GL_UNSIGNED_BYTE, red); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE); + + glUseProgram(m_program); + const int samplerLocation = glGetUniformLocation(m_program, "s[1].c"); + if (samplerLocation >= 0) glUniform1i(samplerLocation, 0); + const int alphaLocation = glGetUniformLocation(m_program, "s[0].a"); + if (alphaLocation >= 0) glUniform1f(alphaLocation, 1.0f); + glUseProgram(0); + + m_target = MakeColorFbo(Gl().Width(), Gl().Height()); + ASSERT_NE(m_target.fbo, 0u) << "offscreen FBO is not framebuffer-complete"; + + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "setup left a GL error behind"; + } + + void TearDown() override { + if (!Ready()) return; + DestroyColorFbo(m_target); + if (m_texture != 0) glDeleteTextures(1, &m_texture); + if (m_vbo != 0) glDeleteBuffers(1, &m_vbo); + if (m_vao != 0) glDeleteVertexArrays(1, &m_vao); + if (m_program != 0) glDeleteProgram(m_program); + } + + Image DrawOnce() { + BindFbo(m_target); + ClearTo(0.0f, 0.0f, 0.0f, 1.0f); + glDisable(GL_DEPTH_TEST); + glDisable(GL_BLEND); + glUseProgram(m_program); + glActiveTexture(GL_TEXTURE0); + glBindTexture(GL_TEXTURE_2D, m_texture); + glBindVertexArray(m_vao); + glDrawArrays(GL_TRIANGLE_STRIP, 0, m_vertexCount); + glBindVertexArray(0); + return ReadPixels(m_target.width, m_target.height); + } + + unsigned int m_program = 0; + unsigned int m_vao = 0; + unsigned int m_vbo = 0; + unsigned int m_texture = 0; + int m_vertexCount = 0; + ColorFbo m_target; + }; + + // Reaching the assertion at all is most of the point: the shipped code SIGSEGV'd inside + // the driver on this draw. + TEST_F(PipelineFailureScenario, SamplerArrayInAStructDrawsWithoutKillingTheProcess) { + const Image drawn = DrawOnce(); + EXPECT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + EXPECT_TRUE(RegionIsMostly(drawn, 2, drawn.Width() - 3, 2, drawn.Height() - 3, "red", 0.0, + "sampler-array-in-struct draw")); + } + + // The second draw is what a poisoned cache entry cannot survive: a memoized + // VK_NULL_HANDLE is served on every subsequent lookup, so a run that dies (or silently + // stops drawing) on the second draw and not the first is exactly the "failed pipeline was + // cached" defect. + TEST_F(PipelineFailureScenario, TheSameDrawRepeatsIdenticallyWithNoPoisonedPipelineCache) { + const Image first = DrawOnce(); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "the first draw already errored"; + Gl().EndFrame(); + const Image second = DrawOnce(); + EXPECT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "the second draw errored"; + + EXPECT_TRUE(RegionIsMostly(second, 2, second.Width() - 3, 2, second.Height() - 3, "red", 0.0, + "second draw")); + EXPECT_TRUE(second == first) << "the second draw differs from the first in " + << second.ByteDiffCount(first) << " bytes - the pipeline the second " + "draw resolved is not the one the first draw used"; + } + + } // namespace +} // namespace MGITest