[Mesa-dev] [PATCH] mesa: fixes for DrawRangeElements[BaseVertex] index validation

[Roland Scheidegger]

in check_index_bounds the comparison needs to be "greater equal" since
contrary to the name _MaxElement is the count of the array.
In vbo_exec_DrawRangeElementsBaseVertex, take into account the basevertex.
As far as I can tell it is completely ok (though maybe stupid) to have
start/end of 100/199, with _MaxElement being 100, if the basevertex
is -100 (since the start/end are prior to adding basevertex). The opposite
is also true (0/99 is not ok if _MaxElement is 100 and and basevertex is 100).
Previously we would have issued a warning in such cases and worse probably
"fixed" end to a bogus value.
Totally untested, found by code inspection when looking at bug 40361 (which
seems unrelated after all).
---
 src/mesa/main/api_validate.c  |    2 +-
 src/mesa/vbo/vbo_exec_array.c |   30 +++++++++++++++++++-----------
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
index b6871d0..1ae491f 100644
--- a/src/mesa/main/api_validate.c
+++ b/src/mesa/main/api_validate.c
@@ -187,7 +187,7 @@ check_index_bounds(struct gl_context *ctx, GLsizei count, GLenum type,
    vbo_get_minmax_indices(ctx, &prim, &ib, &min, &max, 1);
 
    if ((int)(min + basevertex) < 0 ||
-       max + basevertex > ctx->Array.ArrayObj->_MaxElement) {
+       max + basevertex >= ctx->Array.ArrayObj->_MaxElement) {
       /* the max element is out of bounds of one or more enabled arrays */
       _mesa_warning(ctx, "glDrawElements() index=%u is out of bounds (max=%u)",
                     max, ctx->Array.ArrayObj->_MaxElement);
diff --git a/src/mesa/vbo/vbo_exec_array.c b/src/mesa/vbo/vbo_exec_array.c
index 9861b21..3e0fe20 100644
--- a/src/mesa/vbo/vbo_exec_array.c
+++ b/src/mesa/vbo/vbo_exec_array.c
@@ -885,17 +885,18 @@ vbo_exec_DrawRangeElementsBaseVertex(GLenum mode,
       end = MIN2(end, 0xffff);
    }
 
-   if (end >= ctx->Array.ArrayObj->_MaxElement) {
+   if ((int)(start + basevertex) < 0 ||
+       end + basevertex >= ctx->Array.ArrayObj->_MaxElement) {
       /* the max element is out of bounds of one or more enabled arrays */
       warnCount++;
 
       if (warnCount < 10) {
-         _mesa_warning(ctx, "glDraw[Range]Elements(start %u, end %u, count %d, "
-                       "type 0x%x, indices=%p)\n"
+         _mesa_warning(ctx, "glDrawRangeElements[BaseVertex](start %u, end %u, count %d, "
+                       "type 0x%x, indices=%p, base %d)\n"
                        "\tend is out of bounds (max=%u)  "
                        "Element Buffer %u (size %d)\n"
                        "\tThis should probably be fixed in the application.",
-                       start, end, count, type, indices,
+                       start, end, count, type, indices, basevertex,
                        ctx->Array.ArrayObj->_MaxElement - 1,
                        ctx->Array.ArrayObj->ElementArrayBufferObj->Name,
                        (int) ctx->Array.ArrayObj->ElementArrayBufferObj->Size);
@@ -913,14 +914,14 @@ vbo_exec_DrawRangeElementsBaseVertex(GLenum mode,
       if (0) {
          GLuint max = _mesa_max_buffer_index(ctx, count, type, indices,
                                              ctx->Array.ArrayObj->ElementArrayBufferObj);
-         if (max >= ctx->Array.ArrayObj->_MaxElement) {
+         if (max + basevertex >= ctx->Array.ArrayObj->_MaxElement) {
             if (warnCount < 10) {
-               _mesa_warning(ctx, "glDraw[Range]Elements(start %u, end %u, "
-                             "count %d, type 0x%x, indices=%p)\n"
+               _mesa_warning(ctx, "glDrawRangeElements[BaseVertex](start %u, end %u, "
+                             "count %d, type 0x%x, indices=%p, base %d)\n"
                              "\tindex=%u is out of bounds (max=%u)  "
                              "Element Buffer %u (size %d)\n"
                              "\tSkipping the glDrawRangeElements() call",
-                             start, end, count, type, indices, max,
+                             start, end, count, type, indices, basevertex, max,
                              ctx->Array.ArrayObj->_MaxElement - 1,
                              ctx->Array.ArrayObj->ElementArrayBufferObj->Name,
                              (int) ctx->Array.ArrayObj->ElementArrayBufferObj->Size);
@@ -928,13 +929,20 @@ vbo_exec_DrawRangeElementsBaseVertex(GLenum mode,
          }
          /* XXX we could also find the min index and compare to 'start'
           * to see if start is correct.  But it's more likely to get the
-          * upper bound wrong.
+          * upper bound wrong, at least for the non-BaseVertex variant...
           */
       }
 
       /* Set 'end' to the max possible legal value */
       assert(ctx->Array.ArrayObj->_MaxElement >= 1);
-      end = ctx->Array.ArrayObj->_MaxElement - 1;
+      /* XXX Could have positive uint overflow for both start and end too. */
+      if ((int)(ctx->Array.ArrayObj->_MaxElement - 1 - basevertex) < 0) {
+         return;
+      }
+      end = ctx->Array.ArrayObj->_MaxElement - 1 - basevertex;
+      if ((int)(start + basevertex) < 0) {
+         start = -basevertex;
+      }
 
       if (end < start) {
          return;
@@ -942,7 +950,7 @@ vbo_exec_DrawRangeElementsBaseVertex(GLenum mode,
    }
 
    if (0) {
-      printf("glDraw[Range]Elements{,BaseVertex}"
+      printf("glDrawRangeElements{,BaseVertex}"
 	     "(start %u, end %u, type 0x%x, count %d) ElemBuf %u, "
 	     "base %d\n",
 	     start, end, type, count,
-- 
1.7.1