From fa0f6693d0b2dd449fe606f6b1da8c9d529cd14d Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 11 Aug 2026 02:59:10 -0400 Subject: [PATCH] [Fix, Test] (MG_State, MG_Impl): reflection-backed glGetProgramiv queries answer zero instead of dereferencing a null TProgram --- .../GLState/ProgramState/ProgramObject.h | 21 ++++++-- .../MG_Test/Program/ProgramInterfaceTest.cpp | 49 +++++++++++++++++++ 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h index a4a03cea..a888266e 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h @@ -516,12 +516,27 @@ namespace MobileGL::MG_State::GLState { Artifacts().infoLog = "No program binary format is supported."; } Bool GetValidateStatus() const { return m_validateStatus; } - Int GetActiveAtomicCounterCount() const { return Artifacts().program->getNumAtomicCounters(); } - Int GetActiveAttributesCount() const { return Artifacts().program->getNumPipeInputs(); } + // Artifacts().program is null until a link produces reflection, and glGetProgramiv is + // perfectly legal on a program that never linked (GL 4.6 sec. 7.3: the queried state is + // simply its initial value, zero). Dereferencing it there took the process down with a + // SIGSEGV inside glslang::TProgram::getNumPipeInputs - KHR-GL30.api.coverage does exactly + // this after a failed glGetAttribLocation, and reached it as soon as the CopyTexImage2D + // throw ahead of it stopped killing the run first. + Int GetActiveAtomicCounterCount() const { + const auto& program = Artifacts().program; + return program ? program->getNumAtomicCounters() : 0; + } + Int GetActiveAttributesCount() const { + const auto& program = Artifacts().program; + return program ? program->getNumPipeInputs() : 0; + } // GL-visible uniform blocks only: the synthesized MGL_GLOBAL_UBO the relaxed parse // materializes for default-block uniforms is filtered out by DoReflection. Int GetActiveUniformBlocksCount() const { return static_cast(Artifacts().glBlockIndexToTProgram.size()); } - GLuint GetComputeLocalSize(Uint dim) const { return Artifacts().program->getLocalSize(static_cast(dim)); } + GLuint GetComputeLocalSize(Uint dim) const { + const auto& program = Artifacts().program; + return program ? program->getLocalSize(static_cast(dim)) : 0; + } Int GetActiveAttributesMaxLength() const { return Artifacts().attribInNameMaxLength; } Int GetActiveUniformBlocksMaxNameLength() const { return Artifacts().uniformBlockNameMaxLength; } Uint GetUniformBlockIndex(const char* name) const { diff --git a/MobileGL/MG_Test/Program/ProgramInterfaceTest.cpp b/MobileGL/MG_Test/Program/ProgramInterfaceTest.cpp index b4a3e64a..73fbee8b 100644 --- a/MobileGL/MG_Test/Program/ProgramInterfaceTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramInterfaceTest.cpp @@ -1101,6 +1101,55 @@ void main() { color = u + v; } EXPECT_EQ(TakeError(), GL_NO_ERROR); } + // ---------------------------------------------------- queries on an unlinked program ---- + // glGetProgramiv is legal on a program that has never linked - GL 4.6 sec. 7.3 says the + // queried state simply has its initial value - but the reflection-backed pnames read + // Artifacts().program, which is null until a link produces one. That dereference was a + // SIGSEGV inside glslang::TProgram::getNumPipeInputs, and KHR-GL30.api.coverage walks into it + // (it queries GL_ACTIVE_ATTRIBUTES right after a glGetAttribLocation that failed). It only + // became reachable once the glCopyTexImage2D throw ahead of it in the same case stopped + // killing the run first. + TEST_F(ProgramInterfaceTest, ReflectionQueriesOnAnUnlinkedProgramAnswerZero) { + const GLuint neverLinked = CreateProgram(); + ASSERT_NE(neverLinked, 0u); + ClearErrors(); + + for (const GLenum pname : {GL_ACTIVE_ATTRIBUTES, GL_ACTIVE_ATTRIBUTE_MAX_LENGTH, GL_ACTIVE_UNIFORMS, + GL_ACTIVE_UNIFORM_MAX_LENGTH, GL_ACTIVE_UNIFORM_BLOCKS, + GL_ACTIVE_ATOMIC_COUNTER_BUFFERS}) { + GLint value = -1; + GetProgramiv(neverLinked, pname, &value); + ClearErrors(); + EXPECT_GE(value, 0) << "pname 0x" << std::hex << pname << " left its output untouched"; + } + + // A program that was linked and FAILED is the shape api.coverage actually hits. + const GLuint brokenSource = MakeProgram("#version 430\nvoid main() { this is not glsl }\n", kSimpleFs); + LinkProgram(brokenSource); + ClearErrors(); + GLint linked = GL_TRUE; + GetProgramiv(brokenSource, GL_LINK_STATUS, &linked); + ASSERT_EQ(linked, GL_FALSE) << "the shader was supposed to fail to compile"; + ClearErrors(); + + GLint attributes = -1; + GetProgramiv(brokenSource, GL_ACTIVE_ATTRIBUTES, &attributes); + ClearErrors(); + EXPECT_EQ(attributes, 0); + + // GL_COMPUTE_WORK_GROUP_SIZE is GL_INVALID_OPERATION on a program that has not linked (GL + // 4.6 sec. 7.13), so it is allowed to leave the output alone - but it still reaches + // GetComputeLocalSize(), and it may not do so through a null reflection. + GLint localSize[3] = {-1, -1, -1}; + GetProgramiv(brokenSource, GL_COMPUTE_WORK_GROUP_SIZE, localSize); + const GLenum computeError = TakeError(); + ClearErrors(); + EXPECT_TRUE(computeError == GL_INVALID_OPERATION || (localSize[0] == 0 && localSize[1] == 0 && + localSize[2] == 0)) + << "either the query is refused, or it answers the initial value - never both untouched " + "and unreported"; + } + // ------------------------------------------------------------- length on every path ---- // glGetProgramResourceiv's *length is the caller's only signal for how many entries params // holds, and callers are entitled to leave it uninitialised: the CTS declares `GLsizei