Redo array element checking for vertex array buffers.

Now, compute ctx->Array._MaxElement as the min of enabled array's max element.
Test against ctx->Array._MaxElement in glDrawArrays/Elements.
Note: testing in glArrayElement not done yet.
Only do element checking if ctx->Const.CheckArrayBounds is set.

5 files changed, 198 insertions, 88 deletions

diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
index 23a8012a30..f6193aec55 100644
--- a/src/mesa/main/api_validate.c
+++ b/src/mesa/main/api_validate.c
@@ -26,7 +26,6 @@
 #include "glheader.h"
 #include "api_validate.h"
 #include "context.h"
-#include "image.h"  /* for _mesa_sizeof_type() */
 #include "imports.h"
 #include "mtypes.h"
 #include "state.h"
@@ -46,7 +45,7 @@ _mesa_validate_DrawElements(GLcontext *ctx,
    }
 
    if (mode > GL_POLYGON) {
-      _mesa_error(ctx, GL_INVALID_ENUM, "glDrawArrays(mode)" );
+      _mesa_error(ctx, GL_INVALID_ENUM, "glDrawElements(mode)" );
       return GL_FALSE;
    }
 
@@ -59,13 +58,38 @@ _mesa_validate_DrawElements(GLcontext *ctx,
    }
 
    if (ctx->NewState)
-      _mesa_update_state( ctx );
-
-   if (ctx->Array.Vertex.Enabled
-       || (ctx->VertexProgram.Enabled && ctx->Array.VertexAttrib[0].Enabled))
-      return GL_TRUE;
-   else
-      return GL_FALSE;
+      _mesa_update_state(ctx);
+
+   /* Always need vertex positions */
+   if (!ctx->Array.Vertex.Enabled
+       && !(ctx->VertexProgram.Enabled && ctx->Array.VertexAttrib[0].Enabled))
+      return GL_FALSE;
+
+   if (ctx->Const.CheckArrayBounds) {
+      /* find max array index */
+      GLuint max = 0;
+      GLint i;
+      if (type == GL_UNSIGNED_INT) {
+         for (i = 0; i < count; i++)
+            if (((GLuint *) indices)[i] > max)
+               max = ((GLuint *) indices)[i];
+      }
+      else if (type == GL_UNSIGNED_SHORT) {
+         for (i = 0; i < count; i++)
+            if (((GLushort *) indices)[i] > max)
+               max = ((GLushort *) indices)[i];
+      }
+      else {
+         ASSERT(type == GL_UNSIGNED_BYTE);
+         for (i = 0; i < count; i++)
+            if (((GLubyte *) indices)[i] > max)
+               max = ((GLubyte *) indices)[i];
+      }
+      if (max >= ctx->Array._MaxElement) {
+         /* the max element is out of bounds of one or more enabled arrays */
+         return GL_FALSE;
+      }
+   }
 
    return GL_TRUE;
 }
@@ -81,12 +105,12 @@ _mesa_validate_DrawRangeElements(GLcontext *ctx, GLenum mode,
 
    if (count <= 0) {
       if (count < 0)
-	 _mesa_error(ctx, GL_INVALID_VALUE, "glDrawElements(count)" );
+	 _mesa_error(ctx, GL_INVALID_VALUE, "glDrawRangeElements(count)" );
       return GL_FALSE;
    }
 
    if (mode > GL_POLYGON) {
-      _mesa_error(ctx, GL_INVALID_ENUM, "glDrawArrays(mode)" );
+      _mesa_error(ctx, GL_INVALID_ENUM, "glDrawRangeElements(mode)" );
       return GL_FALSE;
    }
 
@@ -98,39 +122,46 @@ _mesa_validate_DrawRangeElements(GLcontext *ctx, GLenum mode,
    if (type != GL_UNSIGNED_INT &&
        type != GL_UNSIGNED_BYTE &&
        type != GL_UNSIGNED_SHORT) {
-      _mesa_error(ctx, GL_INVALID_ENUM, "glDrawElements(type)" );
+      _mesa_error(ctx, GL_INVALID_ENUM, "glDrawRangeElements(type)" );
       return GL_FALSE;
    }
 
    if (ctx->NewState)
-      _mesa_update_state( ctx );
-
-   if (ctx->Array.Vertex.Enabled
-       || (ctx->VertexProgram.Enabled && ctx->Array.VertexAttrib[0].Enabled))
-      return GL_TRUE;
-   else
-      return GL_FALSE;
-}
-
-
-/**
- * Helper routine for validating vertex array data to be sure the given
- * element lies within the legal range (i.e. vertex buffer object).
- */
-static INLINE GLboolean
-validate(GLcontext *ctx, GLint attribArray,
-         const struct gl_client_array *array, GLint element)
-{
-   if (ctx->VertexProgram.Enabled
-       && attribArray >= 0
-       && ctx->Array.VertexAttrib[attribArray].Enabled) {
-      if (element >= ctx->Array.VertexAttrib[attribArray]._MaxElement)
-         return GL_FALSE;
-   }
-   else if (array && array->Enabled) {
-      if (element >= array->_MaxElement)
+      _mesa_update_state(ctx);
+
+   /* Always need vertex positions */
+   if (!ctx->Array.Vertex.Enabled
+       && !(ctx->VertexProgram.Enabled && ctx->Array.VertexAttrib[0].Enabled))
+      return GL_FALSE;
+
+   if (ctx->Const.CheckArrayBounds) {
+      /* Find max array index.
+       * We don't trust the user's start and end values.
+       */
+      GLuint max = 0;
+      GLint i;
+      if (type == GL_UNSIGNED_INT) {
+         for (i = 0; i < count; i++)
+            if (((GLuint *) indices)[i] > max)
+               max = ((GLuint *) indices)[i];
+      }
+      else if (type == GL_UNSIGNED_SHORT) {
+         for (i = 0; i < count; i++)
+            if (((GLushort *) indices)[i] > max)
+               max = ((GLushort *) indices)[i];
+      }
+      else {
+         ASSERT(type == GL_UNSIGNED_BYTE);
+         for (i = 0; i < count; i++)
+            if (((GLubyte *) indices)[i] > max)
+               max = ((GLubyte *) indices)[i];
+      }
+      if (max >= ctx->Array._MaxElement) {
+         /* the max element is out of bounds of one or more enabled arrays */
          return GL_FALSE;
+      }
    }
+
    return GL_TRUE;
 }
 
@@ -143,7 +174,6 @@ GLboolean
 _mesa_validate_DrawArrays(GLcontext *ctx,
 			  GLenum mode, GLint start, GLsizei count)
 {
-   GLint i;
    ASSERT_OUTSIDE_BEGIN_END_WITH_RETVAL(ctx, GL_FALSE);
 
    if (count < 0) {
@@ -157,59 +187,17 @@ _mesa_validate_DrawArrays(GLcontext *ctx,
    }
 
    if (ctx->NewState)
-      _mesa_update_state( ctx );
-
-   /* Either the conventional vertex position array, or the 0th
-    * generic vertex attribute array is required to be enabled.
-    */
-   if (ctx->VertexProgram.Enabled
-       && ctx->Array.VertexAttrib[VERT_ATTRIB_POS].Enabled) {
-      if (start + count >= ctx->Array.VertexAttrib[VERT_ATTRIB_POS]._MaxElement)
-         return GL_FALSE;
-   }
-   else if (ctx->Array.Vertex.Enabled) {
-      if (start + count >= ctx->Array.Vertex._MaxElement)
-         return GL_FALSE;
-   }
-   else {
-      /* no vertex position array! */
-      return GL_FALSE;
-   }
-
-   /*
-    * OK, now check all the other enabled arrays to be sure the elements
-    * are in bounds.
-    */
-   if (!validate(ctx, VERT_ATTRIB_WEIGHT, NULL, start + count))
-      return GL_FALSE;
+      _mesa_update_state(ctx);
 
-   if (!validate(ctx, VERT_ATTRIB_NORMAL, &ctx->Array.Normal, start + count))
+   /* Always need vertex positions */
+   if (!ctx->Array.Vertex.Enabled
+       && !(ctx->VertexProgram.Enabled && ctx->Array.VertexAttrib[0].Enabled))
       return GL_FALSE;
 
-   if (!validate(ctx, VERT_ATTRIB_COLOR0, &ctx->Array.Color, start + count))
-      return GL_FALSE;
-
-   if (!validate(ctx, VERT_ATTRIB_COLOR1, &ctx->Array.SecondaryColor, start + count))
-      return GL_FALSE;
-
-   if (!validate(ctx, VERT_ATTRIB_FOG, &ctx->Array.FogCoord, start + count))
-      return GL_FALSE;
-
-   if (!validate(ctx, VERT_ATTRIB_SIX, NULL, start + count))
-      return GL_FALSE;
-
-   if (!validate(ctx, VERT_ATTRIB_SEVEN, &ctx->Array.FogCoord, start + count))
-      return GL_FALSE;
-
-   for (i = 0; i < MAX_TEXTURE_COORD_UNITS; i++)
-      if (!validate(ctx, VERT_ATTRIB_TEX0 + i, &ctx->Array.TexCoord[i], start + count))
+   if (ctx->Const.CheckArrayBounds) {
+      if (start + count > ctx->Array._MaxElement)
          return GL_FALSE;
-
-   if (!validate(ctx, -1, &ctx->Array.Index, start + count))
-      return GL_FALSE;
-
-   if (!validate(ctx, -1, &ctx->Array.EdgeFlag, start + count))
-      return GL_FALSE;
+   }
 
    return GL_TRUE;
 }
diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 51abf86072..f524045532 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -1062,6 +1062,15 @@ _mesa_init_constants( GLcontext *ctx )
    ctx->Const.MaxProgramMatrices = MAX_PROGRAM_MATRICES;
    ctx->Const.MaxProgramMatrixStackDepth = MAX_PROGRAM_MATRIX_STACK_DEPTH;
 
+   /* If we're running in the X server, do bounds checking to prevent
+    * segfaults and server crashes!
+    */
+#if defined(XFree86LOADER) && defined(IN_MODULE)
+   ctx->Const.CheckArrayBounds = GL_TRUE;
+#else
+   ctx->Const.CheckArrayBounds = GL_FALSE;
+#endif
+
    ASSERT(ctx->Const.MaxTextureUnits == MAX2(ctx->Const.MaxTextureImageUnits, ctx->Const.MaxTextureCoordUnits));
 }
 
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 2692cbb7e3..ea8ae2dc41 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -1329,6 +1329,7 @@ struct gl_array_attrib {
    struct gl_buffer_object *ArrayBufferObj;
    struct gl_buffer_object *ElementArrayBufferObj;
 #endif
+   GLuint _MaxElement;          /* Min of all enabled array's maxes */
 };
 
 
@@ -1720,6 +1721,8 @@ struct gl_constants
    /* vertex or fragment program */
    GLuint MaxProgramMatrices;
    GLuint MaxProgramMatrixStackDepth;
+   /* vertex array / buffer object bounds checking */
+   GLboolean CheckArrayBounds;
 };
 
 
diff --git a/src/mesa/main/state.c b/src/mesa/main/state.c
index 2922828d22..24e7847a28 100644
--- a/src/mesa/main/state.c
+++ b/src/mesa/main/state.c
@@ -60,6 +60,7 @@
 #include "imports.h"
 #include "light.h"
 #include "lines.h"
+#include "macros.h"
 #include "matrix.h"
 #if FEATURE_ARB_occlusion_query
 #include "occlude.h"
@@ -752,6 +753,110 @@ update_program( GLcontext *ctx )
 }
 
 
+/**
+ * Update state dependent on vertex arrays.
+ */
+static void
+update_arrays( GLcontext *ctx )
+{
+   GLuint i, min;
+
+   /* find min of _MaxElement values for all enabled arrays */
+
+   /* 0 */
+   if (ctx->VertexProgram.Enabled
+       && ctx->Array.VertexAttrib[VERT_ATTRIB_POS].Enabled) {
+      min = ctx->Array.VertexAttrib[VERT_ATTRIB_POS]._MaxElement;
+   }
+   else if (ctx->Array.Vertex.Enabled) {
+      min = ctx->Array.Vertex._MaxElement;
+   }
+   else {
+      /* can't draw anything without vertex positions! */
+      min = 0;
+   }
+
+   /* 1 */
+   if (ctx->VertexProgram.Enabled
+       && ctx->Array.VertexAttrib[VERT_ATTRIB_WEIGHT].Enabled) {
+      min = MIN2(min, ctx->Array.VertexAttrib[VERT_ATTRIB_WEIGHT]._MaxElement);
+   }
+   /* no conventional vertex weight array */
+
+   /* 2 */
+   if (ctx->VertexProgram.Enabled
+       && ctx->Array.VertexAttrib[VERT_ATTRIB_NORMAL].Enabled) {
+      min = MIN2(min, ctx->Array.VertexAttrib[VERT_ATTRIB_NORMAL]._MaxElement);
+   }
+   else if (ctx->Array.Normal.Enabled) {
+      min = MIN2(min, ctx->Array.Normal._MaxElement);
+   }
+
+   /* 3 */
+   if (ctx->VertexProgram.Enabled
+       && ctx->Array.VertexAttrib[VERT_ATTRIB_COLOR0].Enabled) {
+      min = MIN2(min, ctx->Array.VertexAttrib[VERT_ATTRIB_COLOR0]._MaxElement);
+   }
+   else if (ctx->Array.Color.Enabled) {
+      min = MIN2(min, ctx->Array.Color._MaxElement);
+   }
+
+   /* 4 */
+   if (ctx->VertexProgram.Enabled
+       && ctx->Array.VertexAttrib[VERT_ATTRIB_COLOR1].Enabled) {
+      min = MIN2(min, ctx->Array.VertexAttrib[VERT_ATTRIB_COLOR1]._MaxElement);
+   }
+   else if (ctx->Array.SecondaryColor.Enabled) {
+      min = MIN2(min, ctx->Array.SecondaryColor._MaxElement);
+   }
+
+   /* 5 */
+   if (ctx->VertexProgram.Enabled
+       && ctx->Array.VertexAttrib[VERT_ATTRIB_FOG].Enabled) {
+      min = MIN2(min, ctx->Array.VertexAttrib[VERT_ATTRIB_FOG]._MaxElement);
+   }
+   else if (ctx->Array.FogCoord.Enabled) {
+      min = MIN2(min, ctx->Array.FogCoord._MaxElement);
+   }
+
+   /* 6 */
+   if (ctx->VertexProgram.Enabled
+       && ctx->Array.VertexAttrib[VERT_ATTRIB_SIX].Enabled) {
+      min = MIN2(min, ctx->Array.VertexAttrib[VERT_ATTRIB_SIX]._MaxElement);
+   }
+
+   /* 7 */
+   if (ctx->VertexProgram.Enabled
+       && ctx->Array.VertexAttrib[VERT_ATTRIB_SEVEN].Enabled) {
+      min = MIN2(min, ctx->Array.VertexAttrib[VERT_ATTRIB_SEVEN]._MaxElement);
+   }
+
+   /* 8..15 */
+   for (i = VERT_ATTRIB_TEX0; i < VERT_ATTRIB_MAX; i++) {
+      if (ctx->VertexProgram.Enabled
+          && ctx->Array.VertexAttrib[i].Enabled) {
+         min = MIN2(min, ctx->Array.VertexAttrib[i]._MaxElement);
+      }
+      else if (i - VERT_ATTRIB_TEX0 < ctx->Const.MaxTextureCoordUnits
+               && ctx->Array.TexCoord[i - VERT_ATTRIB_TEX0].Enabled) {
+         min = MIN2(min, ctx->Array.TexCoord[i - VERT_ATTRIB_TEX0]._MaxElement);
+      }
+   }
+
+   if (ctx->Array.Index.Enabled) {
+      min = MIN2(min, ctx->Array.Index._MaxElement);
+   }
+
+   if (ctx->Array.EdgeFlag.Enabled) {
+      min = MIN2(min, ctx->Array.EdgeFlag._MaxElement);
+   }
+
+   /* _MaxElement is one past the last legal array element */
+   ctx->Array._MaxElement = min;
+}
+
+
+
 /*
  * If __GLcontextRec::NewState is non-zero then this function \b must be called
  * before rendering any primitive.  Basically, function pointers and
@@ -793,6 +898,9 @@ void _mesa_update_state( GLcontext *ctx )
    if (new_state & _NEW_PROGRAM)
       update_program( ctx );
 
+   if (new_state & _NEW_ARRAY)
+      update_arrays( ctx );
+
    /* ctx->_NeedEyeCoords is now up to date.
     *
     * If the truth value of this variable has changed, update for the
diff --git a/src/mesa/tnl/t_imm_api.c b/src/mesa/tnl/t_imm_api.c
index b4e67d8f02..52c20285cc 100644
--- a/src/mesa/tnl/t_imm_api.c
+++ b/src/mesa/tnl/t_imm_api.c
@@ -1129,6 +1129,8 @@ _tnl_EvalPoint2( GLint i, GLint j )
 
 /* Need to use the default array-elt outside begin/end for strict
  * conformance.
+ * XXX If ctx->Const.CheckArrayBounds is true, we need to test i against
+ * ctx->Array._MaxElement
  */
 #define ARRAY_ELT( IM, i )			\
 {						\