On 02/22/2017 11:04 AM, Nicolai Hähnle wrote: > From: Nicolai Hähnle <[email protected]> > > 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: [email protected] > --- > 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."
There is blanket coverage for this. Section 2.3.1 (Errors) of the OpenGL 4.5 Core Profile spec says: If a negative number is provided where an argument of type sizei or sizeiptr is specified, an INVALID_VALUE error is generated. > + * > + * 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. The CTS is also expecting this (from the same section of the spec): In other cases, there are no side effects unless otherwise noted; the command which generates the error is ignored so that it has no effect on GL state or framebuffer contents. If some count[i] would generate an error, the whole command must have no side effects. With the comment changed to reference the spec quotes, this patch is Reviewed-by: Ian Romanick <[email protected]> > + */ > + 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, > _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
