mesa: Delete VAO _MaxElement code and index buffer bounds checking.

Fredrik's implementation of ARB_vertex_attrib_binding introduced new
gl_vertex_attrib_array and gl_vertex_buffer_binding structures, and
converted Mesa's older gl_client_array to be derived state.  Ultimately,
we'd like to drop gl_client_array and use those structures directly.

One hitch is that gl_client_array::_MaxElement doesn't correspond to
either structure (unlike every other field), so we'd have to figure out
where to store it.  The _MaxElement computation uses values from both
structures, so it doesn't really belong in either place.  We could put
it in the VAO, but we'd have to pass it around everywhere.

It turns out that it's only used when ctx->Const.CheckArrayBounds is
set, which is only set by the (rarely used) classic swrast driver.
It appears that drivers/x11 used to set it as well, which was intended
to avoid segmentation faults on out-of-bounds memory access in the X
server (probably for indirect GLX clients).  However, ajax deleted that
code in 2010 (commit 1ccef926be46dce3b6b5c76e812e2fae4e205ce7).

The bounds checking apparently doesn't actually work, either.  Non-VBO
attributes arbitrarily set _MaxElement to 2 * 1000 * 1000 * 1000.
vbo_save_draw and vbo_exec_draw remark /* ??? */ when setting it, and
the i965 code contains a comment noting that _MaxElement is often bogus.

Given that the code is complex, rarely used, and dubiously functional,
it doesn't seem worth maintaining going forward.  This patch drops it.

This will probably mean the classic swrast driver may begin crashing on
out of bounds vertex buffer access in some cases, but I believe that is
allowed by OpenGL (and probably happened for non-VBO accesses anyway).
There do not appear to be any Piglit regressions, either.

Signed-off-by: Kenneth Graunke <[email protected]>
Reviewed-by: Brian Paul <[email protected]>
Reviewed-by: Eric Anholt <[email protected]>
Acked-by: Roland Scheidegger <[email protected]>

15 files changed, 9 insertions, 206 deletions

diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c b/src/mesa/drivers/dri/i965/brw_draw_upload.c
index 2162624cede..5a12439e8f3 100644
--- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
@@ -502,10 +502,7 @@ brw_prepare_vertices(struct brw_context *brw)
 	 /* This is a common place to reach if the user mistakenly supplies
 	  * a pointer in place of a VBO offset.  If we just let it go through,
 	  * we may end up dereferencing a pointer beyond the bounds of the
-	  * GTT.  We would hope that the VBO's max_index would save us, but
-	  * Mesa appears to hand us min/max values not clipped to the
-	  * array object's _MaxElement, and _MaxElement frequently appears
-	  * to be wrong anyway.
+	  * GTT.
 	  *
 	  * The VBO spec allows application termination in this case, and it's
 	  * probably a service to the poor programmer to do so rather than
diff --git a/src/mesa/drivers/dri/swrast/swrast.c b/src/mesa/drivers/dri/swrast/swrast.c
index e28991b0cb6..e8a2c12d71c 100644
--- a/src/mesa/drivers/dri/swrast/swrast.c
+++ b/src/mesa/drivers/dri/swrast/swrast.c
@@ -772,9 +772,6 @@ dri_create_context(gl_api api,
 
     driContextSetFlags(mesaCtx, flags);
 
-    /* do bounds checking to prevent segfaults and server crashes! */
-    mesaCtx->Const.CheckArrayBounds = GL_TRUE;
-
     /* create module contexts */
     _swrast_CreateContext( mesaCtx );
     _vbo_CreateContext( mesaCtx );
diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
index 8f0b1998dad..51a3d1f01ce 100644
--- a/src/mesa/main/api_validate.c
+++ b/src/mesa/main/api_validate.c
@@ -160,47 +160,6 @@ check_valid_to_render(struct gl_context *ctx, const char *function)
 
 
 /**
- * Do bounds checking on array element indexes.  Check that the vertices
- * pointed to by the indices don't lie outside buffer object bounds.
- * \return GL_TRUE if OK, GL_FALSE if any indexed vertex goes is out of bounds
- */
-static GLboolean
-check_index_bounds(struct gl_context *ctx, GLsizei count, GLenum type,
-		   const GLvoid *indices, GLint basevertex)
-{
-   struct _mesa_prim prim;
-   struct _mesa_index_buffer ib;
-   GLuint min, max;
-
-   /* Only the X Server needs to do this -- otherwise, accessing outside
-    * array/BO bounds allows application termination.
-    */
-   if (!ctx->Const.CheckArrayBounds)
-      return GL_TRUE;
-
-   memset(&prim, 0, sizeof(prim));
-   prim.count = count;
-
-   memset(&ib, 0, sizeof(ib));
-   ib.type = type;
-   ib.ptr = indices;
-   ib.obj = ctx->Array.VAO->IndexBufferObj;
-
-   vbo_get_minmax_indices(ctx, &prim, &ib, &min, &max, 1);
-
-   if ((int)(min + basevertex) < 0 ||
-       max + basevertex >= ctx->Array.VAO->_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.VAO->_MaxElement);
-      return GL_FALSE;
-   }
-
-   return GL_TRUE;
-}
-
-
-/**
  * Is 'mode' a valid value for glBegin(), glDrawArrays(), glDrawElements(),
  * etc?  The set of legal values depends on whether geometry shaders/programs
  * are supported.
@@ -453,9 +412,6 @@ _mesa_validate_DrawElements(struct gl_context *ctx,
          return GL_FALSE;
    }
 
-   if (!check_index_bounds(ctx, count, type, indices, basevertex))
-      return GL_FALSE;
-
    if (count == 0)
       return GL_FALSE;
 
@@ -517,12 +473,6 @@ _mesa_validate_MultiDrawElements(struct gl_context *ctx,
       }
    }
 
-   for (i = 0; i < primcount; i++) {
-      if (!check_index_bounds(ctx, count[i], type, indices[i],
-                              basevertex ? basevertex[i] : 0))
-         return GL_FALSE;
-   }
-
    return GL_TRUE;
 }
 
@@ -588,9 +538,6 @@ _mesa_validate_DrawRangeElements(struct gl_context *ctx, GLenum mode,
          return GL_FALSE;
    }
 
-   if (!check_index_bounds(ctx, count, type, indices, basevertex))
-      return GL_FALSE;
-
    if (count == 0)
       return GL_FALSE;
 
@@ -623,11 +570,6 @@ _mesa_validate_DrawArrays(struct gl_context *ctx,
    if (!check_valid_to_render(ctx, "glDrawArrays"))
       return GL_FALSE;
 
-   if (ctx->Const.CheckArrayBounds) {
-      if (start + count > (GLint) ctx->Array.VAO->_MaxElement)
-         return GL_FALSE;
-   }
-
    /* From the GLES3 specification, section 2.14.2 (Transform Feedback
     * Primitive Capture):
     *
@@ -692,11 +634,6 @@ _mesa_validate_DrawArraysInstanced(struct gl_context *ctx, GLenum mode, GLint fi
    if (!check_valid_to_render(ctx, "glDrawArraysInstanced(invalid to render)"))
       return GL_FALSE;
 
-   if (ctx->Const.CheckArrayBounds) {
-      if (first + count > (GLint) ctx->Array.VAO->_MaxElement)
-         return GL_FALSE;
-   }
-
    /* From the GLES3 specification, section 2.14.2 (Transform Feedback
     * Primitive Capture):
     *
@@ -791,9 +728,6 @@ _mesa_validate_DrawElementsInstanced(struct gl_context *ctx,
    if (count == 0)
       return GL_FALSE;
 
-   if (!check_index_bounds(ctx, count, type, indices, basevertex))
-      return GL_FALSE;
-
    return GL_TRUE;
 }
 
diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
index 1ea319a746e..0d77b112b31 100644
--- a/src/mesa/main/arrayobj.c
+++ b/src/mesa/main/arrayobj.c
@@ -291,52 +291,6 @@ remove_array_object( struct gl_context *ctx, struct gl_vertex_array_object *obj
 }
 
 
-
-/**
- * Helper for _mesa_update_vao_max_element().
- * \return  min(vao->_VertexAttrib[*]._MaxElement).
- */
-static GLuint
-compute_max_element(struct gl_vertex_array_object *vao, GLbitfield64 enabled)
-{
-   GLuint min = ~((GLuint)0);
-
-   while (enabled) {
-      struct gl_client_array *client_array;
-      GLint attrib = ffsll(enabled) - 1;
-      enabled ^= BITFIELD64_BIT(attrib);
-
-      client_array = &vao->_VertexAttrib[attrib];
-      assert(client_array->Enabled);
-      _mesa_update_array_max_element(client_array);
-      min = MIN2(min, client_array->_MaxElement);
-   }
-
-   return min;
-}
-
-
-/**
- * Examine vertex arrays to update the gl_vertex_array_object::_MaxElement field.
- */
-void
-_mesa_update_vao_max_element(struct gl_context *ctx,
-                                      struct gl_vertex_array_object *vao)
-{
-   GLbitfield64 enabled;
-
-   if (!ctx->VertexProgram._Current ||
-       ctx->VertexProgram._Current == ctx->VertexProgram._TnlProgram) {
-      enabled = _mesa_array_object_get_enabled_ff(vao);
-   } else {
-      enabled = _mesa_array_object_get_enabled_arb(vao);
-   }
-
-   /* _MaxElement is one past the last legal array element */
-   vao->_MaxElement = compute_max_element(vao, enabled);
-}
-
-
 /**
  * Updates the derived gl_client_arrays when a gl_vertex_attrib_array
  * or a gl_vertex_buffer_binding has changed.
diff --git a/src/mesa/main/arrayobj.h b/src/mesa/main/arrayobj.h
index d72761db15f..1819cd12a2b 100644
--- a/src/mesa/main/arrayobj.h
+++ b/src/mesa/main/arrayobj.h
@@ -75,10 +75,6 @@ _mesa_initialize_vao(struct gl_context *ctx,
 
 
 extern void
-_mesa_update_vao_max_element(struct gl_context *ctx,
-                             struct gl_vertex_array_object *vao);
-
-extern void
 _mesa_update_vao_client_arrays(struct gl_context *ctx,
                                struct gl_vertex_array_object *vao);
 
diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
index 2e289b6f1cb..ef98ba7fd39 100644
--- a/src/mesa/main/attrib.c
+++ b/src/mesa/main/attrib.c
@@ -1458,7 +1458,6 @@ copy_array_object(struct gl_context *ctx,
    /* _Enabled must be the same than on push */
    dest->_Enabled = src->_Enabled;
    dest->NewArrays = src->NewArrays;
-   dest->_MaxElement = src->_MaxElement;
 }
 
 /**
diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 4c2c32e7b7a..682b9c79718 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -628,9 +628,6 @@ _mesa_init_constants(struct gl_constants *consts, gl_api api)
     */
    consts->VertexID_is_zero_based = false;
 
-   /* CheckArrayBounds is overriden by drivers/x11 for X server */
-   consts->CheckArrayBounds = GL_FALSE;
-
    /* GL_ARB_draw_buffers */
    consts->MaxDrawBuffers = MAX_DRAW_BUFFERS;
 
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 0d50be842fb..553a216672a 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -1526,7 +1526,6 @@ struct gl_client_array
    GLuint _ElementSize;         /**< size of each element in bytes */
 
    struct gl_buffer_object *BufferObj;/**< GL_ARB_vertex_buffer_object */
-   GLuint _MaxElement;          /**< max element index into array buffer + 1 */
 };
 
 
@@ -1629,12 +1628,6 @@ struct gl_vertex_array_object
    /** Mask of VERT_BIT_* values indicating changed/dirty arrays */
    GLbitfield64 NewArrays;
 
-   /**
-    * Min of all enabled arrays' _MaxElement.  When arrays reside inside VBOs
-    * we can determine the max legal (in bounds) glDrawElements array index.
-    */
-   GLuint _MaxElement;
-
    /** The index buffer (also known as the element array buffer in OpenGL). */
    struct gl_buffer_object *IndexBufferObj;
 };
@@ -3432,9 +3425,6 @@ struct gl_constants
       GLuint PrimitivesWritten;
    } QueryCounterBits;
 
-   /** vertex array / buffer object bounds checking */
-   GLboolean CheckArrayBounds;
-
    GLuint MaxDrawBuffers;    /**< GL_ARB_draw_buffers */
 
    GLuint MaxColorAttachments;   /**< GL_EXT_framebuffer_object */
diff --git a/src/mesa/main/state.c b/src/mesa/main/state.c
index c122c16aae3..80287c47062 100644
--- a/src/mesa/main/state.c
+++ b/src/mesa/main/state.c
@@ -419,11 +419,6 @@ _mesa_update_state_locked( struct gl_context *ctx )
    if (new_state & _NEW_ARRAY)
       _mesa_update_vao_client_arrays(ctx, ctx->Array.VAO);
 
-   if (ctx->Const.CheckArrayBounds &&
-       new_state & (_NEW_ARRAY | _NEW_PROGRAM | _NEW_BUFFER_OBJECT)) {
-      _mesa_update_vao_max_element(ctx, ctx->Array.VAO);
-   }
-
  out:
    new_prog_state |= update_program_constants(ctx);
 
diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
index 7d169f9d49b..ead78649db8 100644
--- a/src/mesa/main/varray.c
+++ b/src/mesa/main/varray.c
@@ -1867,7 +1867,6 @@ _mesa_copy_client_array(struct gl_context *ctx,
    dst->InstanceDivisor = src->InstanceDivisor;
    dst->_ElementSize = src->_ElementSize;
    _mesa_reference_buffer_object(ctx, &dst->BufferObj, src->BufferObj);
-   dst->_MaxElement = src->_MaxElement;
 }
 
 void
@@ -1911,11 +1910,10 @@ print_array(const char *name, GLint index, const struct gl_client_array *array)
       printf("  %s[%d]: ", name, index);
    else
       printf("  %s: ", name);
-   printf("Ptr=%p, Type=0x%x, Size=%d, ElemSize=%u, Stride=%d, Buffer=%u(Size %lu), MaxElem=%u\n",
+   printf("Ptr=%p, Type=0x%x, Size=%d, ElemSize=%u, Stride=%d, Buffer=%u(Size %lu)\n",
 	  array->Ptr, array->Type, array->Size,
 	  array->_ElementSize, array->StrideB,
-	  array->BufferObj->Name, (unsigned long) array->BufferObj->Size,
-	  array->_MaxElement);
+	  array->BufferObj->Name, (unsigned long) array->BufferObj->Size);
 }
 
 
@@ -1928,8 +1926,6 @@ _mesa_print_arrays(struct gl_context *ctx)
    struct gl_vertex_array_object *vao = ctx->Array.VAO;
    GLuint i;
 
-   _mesa_update_vao_max_element(ctx, vao);
-
    printf("Array Object %u\n", vao->Name);
    if (vao->_VertexAttrib[VERT_ATTRIB_POS].Enabled)
       print_array("Vertex", -1, &vao->_VertexAttrib[VERT_ATTRIB_POS]);
@@ -1943,7 +1939,6 @@ _mesa_print_arrays(struct gl_context *ctx)
    for (i = 0; i < VERT_ATTRIB_GENERIC_MAX; i++)
       if (vao->_VertexAttrib[VERT_ATTRIB_GENERIC(i)].Enabled)
          print_array("Attrib", i, &vao->_VertexAttrib[VERT_ATTRIB_GENERIC(i)]);
-   printf("  _MaxElement = %u\n", vao->_MaxElement);
 }
 
 
diff --git a/src/mesa/main/varray.h b/src/mesa/main/varray.h
index d5d8b363d43..4e4bd5f80a7 100644
--- a/src/mesa/main/varray.h
+++ b/src/mesa/main/varray.h
@@ -34,39 +34,6 @@
 struct gl_client_array;
 struct gl_context;
 
-
-/**
- * Compute the index of the last array element that can be safely accessed in
- * a vertex array.  We can really only do this when the array lives in a VBO.
- * The array->_MaxElement field will be updated.
- * Later in glDrawArrays/Elements/etc we can do some bounds checking.
- */
-static inline void
-_mesa_update_array_max_element(struct gl_client_array *array)
-{
-   assert(array->Enabled);
-
-   if (array->BufferObj->Name) {
-      GLsizeiptrARB offset = (GLsizeiptrARB) array->Ptr;
-      GLsizeiptrARB bufSize = (GLsizeiptrARB) array->BufferObj->Size;
-
-      if (offset < bufSize) {
-         const GLuint stride = array->StrideB ?
-                                 array->StrideB : array->_ElementSize;
-         array->_MaxElement = (bufSize - offset + stride
-                                  - array->_ElementSize) / stride;
-      }
-      else {
-	 array->_MaxElement = 0;
-      }
-   }
-   else {
-      /* user-space array, no idea how big it is */
-      array->_MaxElement = 2 * 1000 * 1000 * 1000; /* just a big number */
-   }
-}
-
-
 /**
  * Returns a pointer to the vertex attribute data in a client array,
  * or the offset into the vertex buffer for an array that resides in
diff --git a/src/mesa/vbo/vbo_exec_array.c b/src/mesa/vbo/vbo_exec_array.c
index 22557e16844..111321bd7f2 100644
--- a/src/mesa/vbo/vbo_exec_array.c
+++ b/src/mesa/vbo/vbo_exec_array.c
@@ -1032,7 +1032,12 @@ vbo_exec_DrawRangeElementsBaseVertex(GLenum mode,
 {
    static GLuint warnCount = 0;
    GLboolean index_bounds_valid = GL_TRUE;
-   GLuint max_element;
+
+   /* This is only useful to catch invalid values in the "end" parameter
+    * like ~0.
+    */
+   GLuint max_element = 2 * 1000 * 1000 * 1000; /* just a big number */
+
    GET_CURRENT_CONTEXT(ctx);
 
    if (MESA_VERBOSE & VERBOSE_DRAW)
@@ -1045,25 +1050,6 @@ vbo_exec_DrawRangeElementsBaseVertex(GLenum mode,
                                           type, indices, basevertex ))
       return;
 
-   if (ctx->Const.CheckArrayBounds) {
-      /* _MaxElement was computed, so we can use it.
-       * This path is used for drivers which need strict bounds checking.
-       */
-      max_element = ctx->Array.VAO->_MaxElement;
-   }
-   else {
-      /* Generally, hardware drivers don't need to know the buffer bounds
-       * if all vertex attributes are in VBOs.
-       * However, if none of vertex attributes are in VBOs, _MaxElement
-       * is always set to some random big number anyway, so bounds checking
-       * is mostly useless.
-       *
-       * This is only useful to catch invalid values in the "end" parameter
-       * like ~0.
-       */
-      max_element = 2 * 1000 * 1000 * 1000; /* just a big number */
-   }
-
    if ((int) end + basevertex < 0 ||
        start + basevertex >= max_element) {
       /* The application requested we draw using a range of indices that's
diff --git a/src/mesa/vbo/vbo_exec_draw.c b/src/mesa/vbo/vbo_exec_draw.c
index c454c6943b4..362cc10405c 100644
--- a/src/mesa/vbo/vbo_exec_draw.c
+++ b/src/mesa/vbo/vbo_exec_draw.c
@@ -159,7 +159,6 @@ vbo_exec_bind_arrays( struct gl_context *ctx )
    struct vbo_context *vbo = vbo_context(ctx);
    struct vbo_exec_context *exec = &vbo->exec;
    struct gl_client_array *arrays = exec->vtx.arrays;
-   const GLuint count = exec->vtx.vert_count;
    const GLuint *map;
    GLuint attr;
    GLbitfield64 varying_inputs = 0x0;
@@ -241,7 +240,6 @@ vbo_exec_bind_arrays( struct gl_context *ctx )
          _mesa_reference_buffer_object(ctx,
                                        &arrays[attr].BufferObj,
                                        exec->vtx.bufferobj);
-	 arrays[attr]._MaxElement = count; /* ??? */
 
          varying_inputs |= VERT_BIT(attr);
       }
diff --git a/src/mesa/vbo/vbo_save_draw.c b/src/mesa/vbo/vbo_save_draw.c
index 89fd30e0fb1..d0521d79485 100644
--- a/src/mesa/vbo/vbo_save_draw.c
+++ b/src/mesa/vbo/vbo_save_draw.c
@@ -203,7 +203,6 @@ static void vbo_bind_vertex_list(struct gl_context *ctx,
          _mesa_reference_buffer_object(ctx,
                                        &arrays[attr].BufferObj,
                                        node->vertex_store->bufferobj);
-	 arrays[attr]._MaxElement = node->count; /* ??? */
 	 
 	 assert(arrays[attr].BufferObj->Name);
 
diff --git a/src/mesa/vbo/vbo_split_copy.c b/src/mesa/vbo/vbo_split_copy.c
index 719ad651a89..ca752e81060 100644
--- a/src/mesa/vbo/vbo_split_copy.c
+++ b/src/mesa/vbo/vbo_split_copy.c
@@ -533,7 +533,6 @@ replay_init( struct copy_context *copy )
       dst->Integer = src->Integer;
       dst->BufferObj = ctx->Shared->NullBufferObj;
       dst->_ElementSize = src->_ElementSize;
-      dst->_MaxElement = copy->dstbuf_size; /* may be less! */
 
       offset += copy->varying[i].size;
    }