[Mesa-dev] [PATCH 1/4] mesa/main: fix MultiDrawElements[BaseVertex] validation of primcount

[Nicolai Hähnle]

From: Nicolai Hähnle <nicolai.haehnle at amd.com>

primcount must be a GLsizei as in the signature for MultiDrawElements
or bad things can happen.

Furthermore, an error should be flagged when primcount is negative
(plus there is an argument about whether it should be flagged when
primcount is 0, but let's worry about that separately).

Curiously, this code used to work somewhat correctly even when primcount
was negative, because the loop that checks count[i] would iterate out of
bounds and almost certainly hit a negative value at some point.

Found by an ASAN error in
GL45-CTS.gtf32.GL3Tests.draw_elements_base_vertex.draw_elements_base_vertex_primcount

Note that the OpenGL spec seems to have s/primcount/drawcount/ at some
point, and the code still reflects the old language.

Cc: mesa-stable at lists.freedesktop.org
---
 src/mesa/main/api_validate.c | 41 +++++++++++++++++++++++++++++++++++++++--
 src/mesa/main/api_validate.h |  2 +-
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
index 1e8a714..5b05301 100644
--- a/src/mesa/main/api_validate.c
+++ b/src/mesa/main/api_validate.c
@@ -716,26 +716,63 @@ _mesa_validate_DrawElements(struct gl_context *ctx,
 
 /**
  * Error checking for glMultiDrawElements().  Includes parameter checking
  * and VBO bounds checking.
  * \return GL_TRUE if OK to render, GL_FALSE if error found
  */
 GLboolean
 _mesa_validate_MultiDrawElements(struct gl_context *ctx,
                                  GLenum mode, const GLsizei *count,
                                  GLenum type, const GLvoid * const *indices,
-                                 GLuint primcount)
+                                 GLsizei primcount)
 {
-   unsigned i;
+   GLsizei i;
 
    FLUSH_CURRENT(ctx, 0);
 
+   /*
+    * Section 10.4 (Drawing Commands Using Vertex Arrays) of the OpenGL 4.5
+    * (Compatibility Profile) spec defines MultiDrawElements as equivalent to:
+    *
+    *    if (mode, drawcount, or type is invalid)
+    *       generate appropriate error
+    *    else {
+    *       for (int i = 0; i < drawcount; i++)
+    *          DrawElementsOneInstance(mode, count[i], type,
+    *                                  indices[i], 0, 0, 0);
+    *    }
+    *
+    * However, it does not say when drawcount is invalid or what the
+    * appropriate error would be.
+    *
+    * The same section does define the error condition for the similar
+    * MultiDrawElementsIndirect:
+    *
+    *    "An INVALID_VALUE error is generated if drawcount is not positive."
+    *
+    * It would make sense to apply the same condition here, but
+    *
+    * (1) this language was only introduced together with the *Indirect() draw
+    *     calls and is quite far removed textually, and
+    *
+    * (2) GL45-CTS.gtf32.GL3Tests.draw_elements_base_vertex.draw_elements_base_vertex_primcount
+    *     expects to get an error iff the value is negative.
+    *
+    * This looks like a gap in the spec, but until that is resolved, let's just
+    * follow what the CTS expects.
+    */
+   if (primcount < 0) {
+      _mesa_error(ctx, GL_INVALID_VALUE,
+                  "glMultiDrawElements(primcount=%d)", primcount);
+      return GL_FALSE;
+   }
+
    for (i = 0; i < primcount; i++) {
       if (count[i] < 0) {
          _mesa_error(ctx, GL_INVALID_VALUE,
                      "glMultiDrawElements(count)" );
          return GL_FALSE;
       }
    }
 
    if (!_mesa_valid_prim_mode(ctx, mode, "glMultiDrawElements")) {
       return GL_FALSE;
diff --git a/src/mesa/main/api_validate.h b/src/mesa/main/api_validate.h
index e94f02e..de520c9 100644
--- a/src/mesa/main/api_validate.h
+++ b/src/mesa/main/api_validate.h
@@ -50,21 +50,21 @@ _mesa_validate_DrawArrays(struct gl_context *ctx, GLenum mode, GLsizei count);
 
 extern GLboolean
 _mesa_validate_DrawElements(struct gl_context *ctx,
 			    GLenum mode, GLsizei count, GLenum type,
 			    const GLvoid *indices);
 
 extern GLboolean
 _mesa_validate_MultiDrawElements(struct gl_context *ctx,
                                  GLenum mode, const GLsizei *count,
                                  GLenum type, const GLvoid * const *indices,
-                                 GLuint primcount);
+                                 GLsizei primcount);
 
 extern GLboolean
 _mesa_validate_DrawRangeElements(struct gl_context *ctx, GLenum mode,
 				 GLuint start, GLuint end,
 				 GLsizei count, GLenum type,
 				 const GLvoid *indices);
 
 
 extern GLboolean
 _mesa_validate_DrawArraysInstanced(struct gl_context *ctx, GLenum mode, GLint first,
-- 
2.9.3