[Fix, Test] (GLImpl, MG_Test): raise a draw's mode INVALID_ENUM before the no-current-program guard

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<GenericErrorInfo>("MG_Impl/GLImpl", functionName, "mode is not an accepted primitive type."));
+        return false;
+    }
+
     static Bool ValidatePrimitiveModeForBackend(const char* functionName, GLenum mode) {
-        if (!IsAcceptedPrimitiveMode(mode)) {
+        if (!ValidatePrimitiveModeEnum(functionName, mode)) {
-            MG_State::pGLContext->RecordError(
-                ErrorCode::InvalidEnum,
-                MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", functionName, "mode is not an accepted primitive type."));
             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);

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<GLenum>(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