diff --git a/MobileGL/MG_Impl/Pipe/ProgramEmit.h b/MobileGL/MG_Impl/Pipe/ProgramEmit.h index d6f91363..7a128a83 100644 --- a/MobileGL/MG_Impl/Pipe/ProgramEmit.h +++ b/MobileGL/MG_Impl/Pipe/ProgramEmit.h @@ -344,7 +344,7 @@ namespace MobileGL::MG_Pipe { for (Uint loc = 0; loc <= maxUniformLoc; ++loc) { // The same guard CacheResourceLocations uses one level down: a location with no // uniform behind it has an empty name and must not index the type tables. - if (program.GetUniformName(loc).empty()) continue; + if (!program.HasUniformLocationTables(loc) || program.GetUniformName(loc).empty()) continue; const Int unit = program.GetUniformSamplerOrImageUnitIndex(loc); if (unit < 0) continue; if (m_samplerUnits.size() >= kMGPipeMaxProgramSamplerUnits) { diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h index 392373b8..aca04f0e 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h @@ -895,6 +895,16 @@ namespace MobileGL::MG_State::GLState { return Artifacts().uniformSamplerOrImageUnitIndex[location]; } + // True when |location| indexes the per-location tables. GetMaxUniformLocation() alone + // cannot say so: a link that fails clears the tables and leaves it at 0, which reads + // the same as "one location", while the program's previous executable - and its + // pipe handle - stays in use (GL 4.6 core 7.3). + Bool HasUniformLocationTables(Uint location) const { + const auto& artifacts = Artifacts(); + return location < artifacts.uniformIndexInTProgram.size() && + location < artifacts.uniformSamplerOrImageUnitIndex.size(); + } + Bool GetDeleteStatus() const { return m_deleteStatus; } Bool GetLinkStatus() const { return Artifacts().linkStatus; } // GL_PROGRAM_BINARY_RETRIEVABLE_HINT. MobileGL exposes no program binary format diff --git a/MobileGL/MG_Test/Pipe/ProgramEmitTest.cpp b/MobileGL/MG_Test/Pipe/ProgramEmitTest.cpp index 2dc67915..7dc53cb1 100644 --- a/MobileGL/MG_Test/Pipe/ProgramEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/ProgramEmitTest.cpp @@ -919,6 +919,35 @@ void main() { gl_Position = u_a + u_b + u_c + u_d; } } } + // KHR-GL46.sepshaderobjs.StateInteraction crashed the client here. A relink that fails + // clears the per-location tables but leaves GetMaxUniformLocation() at 0, while the + // previous executable - and the CSO naming it - stays in use, so the next validate point + // walked location 0 straight off an empty array. + TEST(ProgramEmit, BindingsOfAProgramWhoseRelinkFailedReadNoLocationTables) { + EmitterScope scope; + const GLuint name = MakeVsFsProgram(); + const SharedPtr& program = Ctx().GetProgramObject(name); + ASSERT_TRUE(program); + Uint64 bytes = 0; + const MGPipeHandle cso = Emitter().AcquireShaderCso(*program, bytes); + ASSERT_FALSE(MGPipeHandleIsNull(cso)); + EXPECT_TRUE(program->HasUniformLocationTables(0)) << "a linked program must own its tables"; + + Vector attached; + for (const auto& shader : program->GetAttachedShaders()) attached.push_back(shader->GetExternalIndex()); + for (const GLuint shader : attached) GL::DetachShader(name, shader); + GL::LinkProgram(name); + GLint linked = GL_TRUE; + GL::GetProgramiv(name, GL_LINK_STATUS, &linked); + ASSERT_EQ(linked, GL_FALSE) << "a program with no shaders must fail to link"; + ASSERT_FALSE(program->HasUniformLocationTables(0)) << "the failed link must clear the tables"; + + const Uint64 before = Emitter().ProgramBindingsSetCount(); + Emitter().EmitProgramBindings(*program, cso); + EXPECT_EQ(Emitter().ProgramBindingsSetCount(), before + 1); + EXPECT_EQ(Emitter().LastProgramBindings().SamplerUnitCount, 0u); + } + // The applier's half: whole-set replacement in all three tails, and a re-issued create // drops them because they are INDICES INTO the archive it has just replaced. TEST(ProgramEmit, TheThreeBindingTailsAreWholeSetsAndAReIssuedCreateDropsThem) {