[Mesa-dev] [PATCH 2/4] vbo: Ignore invalid element ranges where 'start' is past the end.

[Brian Paul]

On 02/08/2012 06:08 AM, Kenneth Graunke wrote:
> Some applications, such as Regnum Online, appear to pass invalid
> start/end values to glDrawRangeElements.  In particular, the 'start'
> index sometimes exceeds the maximum array element.  This is clearly
> invalid behavior, and although the spec isn't clear, seems to result
> in undefined, implementation-specific behavior.
>
> This patch takes the conservative approach and simply ignores the range,
> while issuing a warning indicating that the application is broken and
> should be fixed.
>
> NOTE: This is a candidate for release branches.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=45214
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44701
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41152
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=40361
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=28138
> Signed-off-by: Kenneth Graunke<kenneth at whitecape.org>
> ---
>   src/mesa/vbo/vbo_exec_array.c |   25 +++++++++++++++++++++++++
>   1 files changed, 25 insertions(+), 0 deletions(-)
>
> diff --git a/src/mesa/vbo/vbo_exec_array.c b/src/mesa/vbo/vbo_exec_array.c
> index ec4cb4f..c26a8cd 100644
> --- a/src/mesa/vbo/vbo_exec_array.c
> +++ b/src/mesa/vbo/vbo_exec_array.c
> @@ -858,6 +858,7 @@ vbo_exec_DrawRangeElementsBaseVertex(GLenum mode,
>   				     const GLvoid *indices,
>   				     GLint basevertex)
>   {
> +   static GLuint warnCount = 0;
>      GET_CURRENT_CONTEXT(ctx);
>
>      if (MESA_VERBOSE&  VERBOSE_DRAW)
> @@ -870,6 +871,30 @@ vbo_exec_DrawRangeElementsBaseVertex(GLenum mode,
>                                             type, indices, basevertex ))
>         return;
>
> +   if ((int)(start + basevertex)<  0 ||

I think this would be better as:

       if ((int) start + basevertex < 0 ||

The intermediate sum of start and basevertex is unsigned.


> +       start + basevertex>= ctx->Array.ArrayObj->_MaxElement) {
> +      /* The application requested we draw using a range of indices that's
> +       * outside the bounds of the current VBO.  This is invalid and appears
> +       * to give undefined results.  The safest thing to do is to simply
> +       * ignore the range, in case the application botched their range tracking
> +       * but did provide valid indices.  Also issue a warning indicating that
> +       * the application is broken.
> +       */
> +      if (warnCount++<  10) {
> +         _mesa_warning(ctx, "glDrawRangeElements(start %u, end %u, "
> +                       "basevertex %d, count %d, type 0x%x, indices=%p):\n"
> +                       "\tstart is out of bounds (max=%u); ignoring range.\n"
> +                       "\tThis should be fixed in the application.",
> +                       start, end, basevertex, count, type, indices,
> +                       ctx->Array.ArrayObj->_MaxElement - 1);
> +      }
> +
> +      /* Just do an ordinary glDrawElementsBaseVertex(). */
> +      vbo_validated_drawrangeelements(ctx, mode, GL_FALSE, ~0, ~0,

Shouldn't that be FALSE, 0, ~0 ?

> +                                      count, type, indices, basevertex, 1);
> +      return;
> +   }
> +
>      /* NOTE: It's important that 'end' is a reasonable value.
>       * in _tnl_draw_prims(), we use end to determine how many vertices
>       * to transform.  If it's too large, we can unnecessarily split prims

Reviewed-by: Brian Paul <brianp at vmware.com>