[Fix, Test] (GLImpl): validate count, instancecount, drawcount, type and range on the base-vertex draw family

MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp

@@ -382,6 +382,31 @@ namespace MobileGL::MG_Impl::GLImpl {
         }
     }
 
+    // GL 4.6 core 10.3.9: every DrawElements-family count is a sizei and "if count is negative, an
+    // INVALID_VALUE error is generated". The same sentence covers instancecount and the
+    // MultiDraw* drawcount, so one helper serves all of them; the parameter is named for the
+    // caller so the message says which argument the application actually got wrong.
+    static Bool ValidateNonNegativeDrawArgument(const char* functionName, const char* argumentName, GLsizei value) {
+        if (value >= 0) return true;
+        MG_State::pGLContext->RecordError(
+            ErrorCode::InvalidValue,
+            MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", functionName,
+                                         String(argumentName) + " must be non-negative."));
+        return false;
+    }
+
+    // GL 4.6 core 10.3.9 for DrawRangeElements*: "if end < start, an INVALID_VALUE error is
+    // generated". Both are uints, so a caller that passes -1 for start arrives here as
+    // 0xFFFFFFFF and is caught by the same comparison - which is exactly what
+    // KHR-GL4x.draw_elements_base_vertex_tests.invalid_count_argument checks.
+    static Bool ValidateDrawElementsRange(const char* functionName, GLuint start, GLuint end) {
+        if (end >= start) return true;
+        MG_State::pGLContext->RecordError(
+            ErrorCode::InvalidValue,
+            MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", functionName, "end must not be less than start."));
+        return false;
+    }
+
     // GL 4.6 core 10.9: inside a conditional block whose predicate did not pass, the drawing
     // commands, Clear, ClearBuffer* and the compute dispatches are DISCARDED. The gate sits on the
     // wrappers that ISSUE the backend call rather than at the top of each entry point, so that
@@ -836,6 +861,9 @@ namespace MobileGL::MG_Impl::GLImpl {
         if (!ValidatePrimitiveModeEnum(__func__, mode)) return;
         if (!ValidateCurrentProgramForExecution(__func__)) return;
         if (!ValidatePrimitiveModeForBackend(__func__, mode)) return;
+        if (!ValidateDrawElementsIndexType(__func__, type)) return;
+        if (!ValidateNonNegativeDrawArgument(__func__, "count", count)) return;
+        if (!ValidateDrawElementsRange(__func__, start, end)) return;
         DrawRangeElementsBaseVertex_Backend(mode, start, end, count, type, indices, basevertex);
     }
 
@@ -860,6 +888,9 @@ namespace MobileGL::MG_Impl::GLImpl {
         if (!ValidatePrimitiveModeEnum(__func__, mode)) return;
         if (!ValidateCurrentProgramForExecution(__func__)) return;
         if (!ValidatePrimitiveModeForBackend(__func__, mode)) return;
+        if (!ValidateDrawElementsIndexType(__func__, type)) return;
+        if (!ValidateNonNegativeDrawArgument(__func__, "count", count)) return;
+        if (!ValidateNonNegativeDrawArgument(__func__, "instancecount", instancecount)) return;
         DrawElementsInstancedBaseVertex_Backend(mode, count, type, indices, instancecount, basevertex);
     }
 
@@ -914,6 +945,8 @@ namespace MobileGL::MG_Impl::GLImpl {
         if (!ValidatePrimitiveModeEnum(__func__, mode)) return;
         if (!ValidateCurrentProgramForExecution(__func__)) return;
         if (!ValidatePrimitiveModeForBackend(__func__, mode)) return;
+        if (!ValidateDrawElementsIndexType(__func__, type)) return;
+        if (!ValidateNonNegativeDrawArgument(__func__, "count", count)) return;
         AccountTransformFeedbackPrimitives(mode, count);
         DrawElementsBaseVertex_Backend(mode, count, type, indices, basevertex);
     }
@@ -952,6 +985,19 @@ namespace MobileGL::MG_Impl::GLImpl {
         if (!ValidatePrimitiveModeEnum(__func__, mode)) return;
         if (!ValidateCurrentProgramForExecution(__func__)) return;
         if (!ValidatePrimitiveModeForBackend(__func__, mode)) return;
+        if (!ValidateDrawElementsIndexType(__func__, type)) return;
+        if (!ValidateNonNegativeDrawArgument(__func__, "drawcount", drawcount)) return;
+        // GL 4.6 core 10.5 defines MultiDrawElementsBaseVertex as drawcount separate
+        // DrawElementsBaseVertex calls, so each element of the count array carries the same
+        // non-negative requirement the single-draw entry point applies to its own count. The
+        // whole call is rejected before any sub-draw is issued, which is what makes the error
+        // observable at all - a driver that drew the valid prefix first would leave the
+        // framebuffer half-written.
+        if (count != nullptr) {
+            for (GLsizei draw = 0; draw < drawcount; ++draw) {
+                if (!ValidateNonNegativeDrawArgument(__func__, "every element of count", count[draw])) return;
+            }
+        }
         MultiDrawElementsBaseVertex_Backend(mode, count, type, indices, drawcount, basevertex);
     }
 

MobileGL/MG_IntegrationTest/Scenarios/MultiDrawScenario.cpp

@@ -518,5 +518,62 @@ void main() {
             ExpectSameImage(batched, unrolled, "a batch with zero-count sub-draws");
         }
 
+        // The base-vertex family's argument checks (GL 4.6 core 10.3.9). These are what
+        // KHR-GL4x.draw_elements_base_vertex_tests.invalid_* assert, and the reason the group sat
+        // NotSupported for so long hid the fact that the entry points forwarded any argument
+        // straight to the backend: a negative count reached the emulation as a huge unsigned
+        // size. Each case drains the error queue first so the assertion names the call it made.
+        TEST_F(MultiDrawScenario, BaseVertexDrawsRejectMalformedArguments) {
+            if (!Ready()) return;
+            constexpr int kPad = 0;
+            BuildScene(kPad, kQuadIndices, sizeof(kQuadIndices));
+            // A bound program and VAO are prerequisites, not decoration: the entry points check
+            // "is there something to execute" (GL_INVALID_OPERATION) before they look at any
+            // argument, so without these every case below would pass for the wrong reason.
+            glUseProgram(m_program);
+            glBindVertexArray(m_vao);
+            ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "scene setup left a GL error behind";
+
+            const auto expectError = [&](const char* what, GLenum expected) {
+                EXPECT_EQ(FirstGLError(), expected) << what;
+                // FirstGLError stops at the first one; make sure nothing else is queued so the
+                // next case starts clean.
+                while (glGetError() != GL_NO_ERROR) {
+                }
+            };
+
+            glDrawElementsBaseVertex(GL_TRIANGLES, -1, GL_UNSIGNED_INT, nullptr, 0);
+            expectError("glDrawElementsBaseVertex with a negative count", GL_INVALID_VALUE);
+
+            glDrawElementsBaseVertex(GL_TRIANGLES, 3, GL_NONE, nullptr, 0);
+            expectError("glDrawElementsBaseVertex with a non-index type", GL_INVALID_ENUM);
+
+            glDrawRangeElementsBaseVertex(GL_TRIANGLES, 3, 0, 3, GL_UNSIGNED_INT, nullptr, 0);
+            expectError("glDrawRangeElementsBaseVertex with end < start", GL_INVALID_VALUE);
+
+            // start = -1 arrives as 0xFFFFFFFF, so this is the same end < start rule seen from
+            // the other side - and it is the shape the CTS's invalid_count case actually uses.
+            glDrawRangeElementsBaseVertex(GL_TRIANGLES, static_cast<GLuint>(-1), 2, 1, GL_UNSIGNED_INT, nullptr, 0);
+            expectError("glDrawRangeElementsBaseVertex with a wrapped start", GL_INVALID_VALUE);
+
+            glDrawElementsInstancedBaseVertex(GL_TRIANGLES, 3, GL_UNSIGNED_INT, nullptr, -1, 0);
+            expectError("glDrawElementsInstancedBaseVertex with a negative instancecount", GL_INVALID_VALUE);
+
+            const GLsizei negativeCount = -1;
+            const void* offsets[1] = {reinterpret_cast<const void*>(0)};
+            const GLint baseVertices[1] = {0};
+            glMultiDrawElementsBaseVertex(GL_TRIANGLES, &negativeCount, GL_UNSIGNED_INT, offsets, 1, baseVertices);
+            expectError("glMultiDrawElementsBaseVertex with a negative element of count", GL_INVALID_VALUE);
+
+            const GLsizei validCount = 6;
+            glMultiDrawElementsBaseVertex(GL_TRIANGLES, &validCount, GL_UNSIGNED_INT, offsets, -1, baseVertices);
+            expectError("glMultiDrawElementsBaseVertex with a negative drawcount", GL_INVALID_VALUE);
+
+            // The well-formed call still has to go through, or the checks above would be
+            // indistinguishable from a blanket rejection.
+            glMultiDrawElementsBaseVertex(GL_TRIANGLES, &validCount, GL_UNSIGNED_INT, offsets, 1, baseVertices);
+            expectError("a well-formed glMultiDrawElementsBaseVertex", GL_NO_ERROR);
+        }
+
     } // namespace
 } // namespace MGITest