From 1e8d4661e62170e9fe93fbdc22b191a63c21f403 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 20 Aug 2026 05:10:38 -0400 Subject: [PATCH] [Fix, Test] (GLImpl, MG_Test): raise a draw's mode INVALID_ENUM before the no-current-program guard --- .../MG_Impl/GLImpl/Drawing/GL_Drawing.cpp | 38 +++++++++++++++++-- .../MG_Test/State/NegativeApiErrorsTest.cpp | 35 +++++++++++++++++ 2 files changed, 69 insertions(+), 4 deletions(-) diff --git a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp index 121af025..408e08fb 100644 --- a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp +++ b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp @@ -162,11 +162,23 @@ namespace MobileGL::MG_Impl::GLImpl { } } + // The `mode` INVALID_ENUM in isolation, so a draw entry point can raise it BEFORE any of the + // state-dependent INVALID_OPERATIONs below. GL 4.6 core 10.4 makes a bad mode INVALID_ENUM + // unconditionally, while "no current program" is not even a spec-listed draw error - it is + // MobileGL's own null-dereference guard - so it must never shadow the enum check + // (KHR-GL31.api.coverage calls glDrawArraysInstanced/glDrawElementsInstanced with mode + // GL_POINTS-1 against a bare context and pins GL_INVALID_ENUM). + static Bool ValidatePrimitiveModeEnum(const char* functionName, GLenum mode) { + if (IsAcceptedPrimitiveMode(mode)) return true; + + MG_State::pGLContext->RecordError( + ErrorCode::InvalidEnum, + MakeUnique("MG_Impl/GLImpl", functionName, "mode is not an accepted primitive type.")); + return false; + } + static Bool ValidatePrimitiveModeForBackend(const char* functionName, GLenum mode) { - if (!IsAcceptedPrimitiveMode(mode)) { - MG_State::pGLContext->RecordError( - ErrorCode::InvalidEnum, - MakeUnique("MG_Impl/GLImpl", functionName, "mode is not an accepted primitive type.")); + if (!ValidatePrimitiveModeEnum(functionName, mode)) { return false; } @@ -607,12 +619,14 @@ namespace MobileGL::MG_Impl::GLImpl { } void MultiDrawElementsIndirect(GLenum mode, GLenum type, const void* indirect, GLsizei drawcount, GLsizei stride) { + if (!ValidatePrimitiveModeEnum(__func__, mode)) return; if (!ValidateCurrentProgramForExecution(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; MultiDrawElementsIndirect_Backend(mode, type, indirect, drawcount, stride); } void MultiDrawArraysIndirect(GLenum mode, const void* indirect, GLsizei drawcount, GLsizei stride) { + if (!ValidatePrimitiveModeEnum(__func__, mode)) return; if (!ValidateCurrentProgramForExecution(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; MultiDrawArraysIndirect_Backend(mode, indirect, drawcount, stride); @@ -726,12 +740,14 @@ namespace MobileGL::MG_Impl::GLImpl { void DrawRangeElementsBaseVertex(GLenum mode, GLuint start, GLuint end, GLsizei count, GLenum type, const void* indices, GLint basevertex) { + if (!ValidatePrimitiveModeEnum(__func__, mode)) return; if (!ValidateCurrentProgramForExecution(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; DrawRangeElementsBaseVertex_Backend(mode, start, end, count, type, indices, basevertex); } void DrawRangeElements(GLenum mode, GLuint start, GLuint end, GLsizei count, GLenum type, const void* indices) { + if (!ValidatePrimitiveModeEnum(__func__, mode)) return; if (!ValidateCurrentProgramForExecution(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; DrawRangeElements_Backend(mode, start, end, count, type, indices); @@ -739,6 +755,7 @@ namespace MobileGL::MG_Impl::GLImpl { void DrawElementsInstancedBaseVertexBaseInstance(GLenum mode, GLsizei count, GLenum type, const void* indices, GLsizei instancecount, GLint basevertex, GLuint baseinstance) { + if (!ValidatePrimitiveModeEnum(__func__, mode)) return; if (!ValidateCurrentProgramForExecution(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; DrawElementsInstancedBaseVertexBaseInstance_Backend(mode, count, type, indices, instancecount, basevertex, @@ -747,6 +764,7 @@ namespace MobileGL::MG_Impl::GLImpl { void DrawElementsInstancedBaseVertex(GLenum mode, GLsizei count, GLenum type, const void* indices, GLsizei instancecount, GLint basevertex) { + if (!ValidatePrimitiveModeEnum(__func__, mode)) return; if (!ValidateCurrentProgramForExecution(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; DrawElementsInstancedBaseVertex_Backend(mode, count, type, indices, instancecount, basevertex); @@ -754,18 +772,21 @@ namespace MobileGL::MG_Impl::GLImpl { void DrawElementsInstancedBaseInstance(GLenum mode, GLsizei count, GLenum type, const void* indices, GLsizei instancecount, GLuint baseinstance) { + if (!ValidatePrimitiveModeEnum(__func__, mode)) return; if (!ValidateCurrentProgramForExecution(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; DrawElementsInstancedBaseInstance_Backend(mode, count, type, indices, instancecount, baseinstance); } void DrawElementsInstanced(GLenum mode, GLsizei count, GLenum type, const void* indices, GLsizei instancecount) { + if (!ValidatePrimitiveModeEnum(__func__, mode)) return; if (!ValidateCurrentProgramForExecution(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; DrawElementsInstanced_Backend(mode, count, type, indices, instancecount); } void DrawElementsIndirect(GLenum mode, GLenum type, const void* indirect) { + if (!ValidatePrimitiveModeEnum(__func__, mode)) return; if (!ValidateCurrentProgramForExecution(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; if (!ValidateDrawElementsIndexType(__func__, type)) return; @@ -775,18 +796,21 @@ namespace MobileGL::MG_Impl::GLImpl { void DrawArraysInstancedBaseInstance(GLenum mode, GLint first, GLsizei count, GLsizei instancecount, GLuint baseinstance) { + if (!ValidatePrimitiveModeEnum(__func__, mode)) return; if (!ValidateCurrentProgramForExecution(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; DrawArraysInstancedBaseInstance_Backend(mode, first, count, instancecount, baseinstance); } void DrawArraysInstanced(GLenum mode, GLint first, GLsizei count, GLsizei instancecount) { + if (!ValidatePrimitiveModeEnum(__func__, mode)) return; if (!ValidateCurrentProgramForExecution(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; DrawArraysInstanced_Backend(mode, first, count, instancecount); } void DrawArraysIndirect(GLenum mode, const void* indirect) { + if (!ValidatePrimitiveModeEnum(__func__, mode)) return; if (!ValidateCurrentProgramForExecution(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; if (!ValidateIndirectDrawSource(__func__, indirect, kDrawArraysIndirectCommandBytes)) return; @@ -794,6 +818,7 @@ namespace MobileGL::MG_Impl::GLImpl { } void DrawElementsBaseVertex(GLenum mode, GLsizei count, GLenum type, const void* indices, GLint basevertex) { + if (!ValidatePrimitiveModeEnum(__func__, mode)) return; if (!ValidateCurrentProgramForExecution(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; AccountTransformFeedbackPrimitives(mode, count); @@ -801,6 +826,7 @@ namespace MobileGL::MG_Impl::GLImpl { } void DrawArrays(GLenum mode, GLint first, GLsizei count) { + if (!ValidatePrimitiveModeEnum(__func__, mode)) return; if (!ValidateCurrentProgramForExecution(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; AccountTransformFeedbackPrimitives(mode, count); @@ -808,6 +834,7 @@ namespace MobileGL::MG_Impl::GLImpl { } void MultiDrawArrays(GLenum mode, const GLint* first, const GLsizei* count, GLsizei drawcount) { + if (!ValidatePrimitiveModeEnum(__func__, mode)) return; if (!ValidateCurrentProgramForExecution(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; if (drawcount < 0) { @@ -821,6 +848,7 @@ namespace MobileGL::MG_Impl::GLImpl { void MultiDrawElements(GLenum mode, const GLsizei* count, GLenum type, const void* const* indices, GLsizei drawcount) { + if (!ValidatePrimitiveModeEnum(__func__, mode)) return; if (!ValidateCurrentProgramForExecution(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; MultiDrawElements_Backend(mode, count, type, indices, drawcount); @@ -828,6 +856,7 @@ namespace MobileGL::MG_Impl::GLImpl { void MultiDrawElementsBaseVertex(GLenum mode, const GLsizei* count, GLenum type, const void* const* indices, GLsizei drawcount, const GLint* basevertex) { + if (!ValidatePrimitiveModeEnum(__func__, mode)) return; if (!ValidateCurrentProgramForExecution(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; MultiDrawElementsBaseVertex_Backend(mode, count, type, indices, drawcount, basevertex); @@ -838,6 +867,7 @@ namespace MobileGL::MG_Impl::GLImpl { } void DrawElements(GLenum mode, GLsizei count, GLenum type, const void* indices) { + if (!ValidatePrimitiveModeEnum(__func__, mode)) return; if (!ValidateCurrentProgramForExecution(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; AccountTransformFeedbackPrimitives(mode, count); diff --git a/MobileGL/MG_Test/State/NegativeApiErrorsTest.cpp b/MobileGL/MG_Test/State/NegativeApiErrorsTest.cpp index 9426a7f2..50ee18b2 100644 --- a/MobileGL/MG_Test/State/NegativeApiErrorsTest.cpp +++ b/MobileGL/MG_Test/State/NegativeApiErrorsTest.cpp @@ -19,6 +19,8 @@ // * KHR-GL43.compute_shader.api-indirect / .api-program. // * KHR-GLxx.texture_storage.compressed_data - compressed formats on TEXTURE_3D. // * KHR-GL32.api.coverage - glFenceSync's condition/flags and glWaitSync's flags/timeout. +// * KHR-GL31.api.coverage - a draw's mode INVALID_ENUM has to outrank MobileGL's own +// no-current-program guard. // Plus the indexed-getter parity RC-7b is about: glGetBooleani_v / glGetInteger64i_v / // glGetFloati_v / glGetDoublei_v must answer every pname glGetIntegeri_v answers. // @@ -504,4 +506,37 @@ void main() { g_color = vec4(1); } DeleteSync(sync); EXPECT_EQ(GetError(), GL_NO_ERROR); } + + // KHR-GL31.api.coverage's first two calls are glDrawArraysInstanced / glDrawElementsInstanced + // with mode GL_POINTS-1 against a context that has no program and no VAO bound, and they must + // answer GL_INVALID_ENUM. MobileGL's own "there is no current program" guard - which the spec + // does not list as a draw error at all - used to run first and shadowed the enum check with + // GL_INVALID_OPERATION. Nothing here reaches a backend: the mode is rejected before the guard. + TEST_F(NegativeApiErrorsTest, BadPrimitiveModeOutranksTheNoProgramGuard) { + DrainErrors(); + // Exactly what the coverage test passes: GL_POINTS is 0, so this is 0xFFFFFFFF. + constexpr GLenum kBadMode = static_cast(GL_POINTS - 1); + + RunRows({ + {"glDrawArraysInstanced with an unaccepted mode", [] { DrawArraysInstanced(kBadMode, 0, 3, 4); }, + GL_INVALID_ENUM}, + {"glDrawElementsInstanced with an unaccepted mode", + [] { DrawElementsInstanced(kBadMode, 3, GL_UNSIGNED_INT, nullptr, 4); }, GL_INVALID_ENUM}, + {"glDrawArrays with an unaccepted mode", [] { DrawArrays(kBadMode, 0, 3); }, GL_INVALID_ENUM}, + {"glDrawElements with an unaccepted mode", + [] { DrawElements(kBadMode, 3, GL_UNSIGNED_INT, nullptr); }, GL_INVALID_ENUM}, + {"glMultiDrawArrays with an unaccepted mode", + [] { MultiDrawArrays(kBadMode, nullptr, nullptr, 0); }, GL_INVALID_ENUM}, + {"glDrawRangeElements with an unaccepted mode", + [] { DrawRangeElements(kBadMode, 0, 2, 3, GL_UNSIGNED_INT, nullptr); }, GL_INVALID_ENUM}, + {"glDrawElementsIndirect with an unaccepted mode", + [] { DrawElementsIndirect(kBadMode, GL_UNSIGNED_INT, nullptr); }, GL_INVALID_ENUM}, + {"glDrawArraysIndirect with an unaccepted mode", [] { DrawArraysIndirect(kBadMode, nullptr); }, + GL_INVALID_ENUM}, + // A mode the enum check accepts falls through to the guard, so the INVALID_OPERATION + // that used to win is still raised for the calls it is actually about. + {"glDrawArrays with a legal mode and no program bound", [] { DrawArrays(GL_TRIANGLES, 0, 3); }, + GL_INVALID_OPERATION}, + }); + } } // namespace