[Mesa-dev] [PATCH] [RFC] program: fix out of bounds array accesses and other bad things
Noticed a warning: array subscript is above array bounds given at one of the existing sanity-check asserts. Turns out all the arrays of strings haven't matched the corresponding enum values in a while, if ever. I didn't know the proper names for any of these and couldn't find them in the base specs aside from result.pointsize in ARB_vertex_program, so I just filled in the enum's value as was done with other slots. Also add four STATIC_ASSERT()s to be sure and catch future additions or bumps to MAX_VARYING/etc again, and some more non-static asserts where there weren't any before. (Note, the fragment enum that corresponded to result.color(half) was removed in 8d475822e6e19fa79719c856a2db5b6a205db1b9.) --- src/mesa/program/prog_print.c | 73 + 1 files changed, 59 insertions(+), 14 deletions(-) diff --git a/src/mesa/program/prog_print.c b/src/mesa/program/prog_print.c index e9bf3aa..dfb5793 100644 --- a/src/mesa/program/prog_print.c +++ b/src/mesa/program/prog_print.c @@ -95,15 +95,15 @@ arb_input_attrib_string(GLint index, GLenum progType) /* * These strings should match the VERT_ATTRIB_x and FRAG_ATTRIB_x tokens. */ - const char *vertAttribs[] = { + static const char *const vertAttribs[] = { vertex.position, vertex.weight, vertex.normal, vertex.color.primary, vertex.color.secondary, vertex.fogcoord, - vertex.(six), - vertex.(seven), + vertex.(six), /* VERT_ATTRIB_COLOR_INDEX */ + vertex.(seven), /* VERT_ATTRIB_EDGEFLAG */ vertex.texcoord[0], vertex.texcoord[1], vertex.texcoord[2], @@ -112,6 +112,7 @@ arb_input_attrib_string(GLint index, GLenum progType) vertex.texcoord[5], vertex.texcoord[6], vertex.texcoord[7], + vertex.(sixteen), /* VERT_ATTRIB_POINT_SIZE */ vertex.attrib[0], vertex.attrib[1], vertex.attrib[2], @@ -127,9 +128,9 @@ arb_input_attrib_string(GLint index, GLenum progType) vertex.attrib[12], vertex.attrib[13], vertex.attrib[14], - vertex.attrib[15] + vertex.attrib[15] /* MAX_VARYING = 16 */ }; - const char *fragAttribs[] = { + static const char *const fragAttribs[] = { fragment.position, fragment.color.primary, fragment.color.secondary, @@ -142,6 +143,10 @@ arb_input_attrib_string(GLint index, GLenum progType) fragment.texcoord[5], fragment.texcoord[6], fragment.texcoord[7], + fragment.(twelve), /* FRAG_ATTRIB_FACE */ + fragment.(thirteen), /* FRAG_ATTRIB_PNTC */ + fragment.(fourteen), /* FRAG_ATTRIB_CLIP_DIST0 */ + fragment.(fifteen), /* FRAG_ATTRIB_CLIP_DIST1 */ fragment.varying[0], fragment.varying[1], fragment.varying[2], @@ -149,18 +154,31 @@ arb_input_attrib_string(GLint index, GLenum progType) fragment.varying[4], fragment.varying[5], fragment.varying[6], - fragment.varying[7] + fragment.varying[7], + fragment.varying[8], + fragment.varying[9], + fragment.varying[10], + fragment.varying[11], + fragment.varying[12], + fragment.varying[13], + fragment.varying[14], + fragment.varying[15] /* MAX_VARYING = 16 */ }; /* sanity checks */ + STATIC_ASSERT(Elements(vertAttribs) == VERT_ATTRIB_MAX); + STATIC_ASSERT(Elements(fragAttribs) == FRAG_ATTRIB_MAX); assert(strcmp(vertAttribs[VERT_ATTRIB_TEX0], vertex.texcoord[0]) == 0); assert(strcmp(vertAttribs[VERT_ATTRIB_GENERIC15], vertex.attrib[15]) == 0); + assert(strcmp(fragAttribs[FRAG_ATTRIB_TEX0], fragment.texcoord[0]) == 0); + assert(strcmp(fragAttribs[FRAG_ATTRIB_VAR0+15], fragment.varying[15]) == 0); if (progType == GL_VERTEX_PROGRAM_ARB) { assert(index Elements(vertAttribs)); return vertAttribs[index]; } else { + assert(progType == GL_FRAGMENT_PROGRAM_ARB); assert(index Elements(fragAttribs)); return fragAttribs[index]; } @@ -213,7 +231,7 @@ arb_output_attrib_string(GLint index, GLenum progType) /* * These strings should match the VERT_RESULT_x and FRAG_RESULT_x tokens. */ - const char *vertResults[] = { + static const char *const vertResults[] = { result.position, result.color.primary, result.color.secondary, @@ -226,6 +244,13 @@ arb_output_attrib_string(GLint index, GLenum progType) result.texcoord[5], result.texcoord[6], result.texcoord[7], + result.pointsize, /* VERT_RESULT_PSIZ */ + result.(thirteen), /* VERT_RESULT_BFC0 */ + result.(fourteen), /* VERT_RESULT_BFC1 */ + result.(fifteen), /* VERT_RESULT_EDGE */ + result.(sixteen), /* VERT_RESULT_CLIP_VERTEX */ + result.(seventeen), /* VERT_RESULT_CLIP_DIST0 */ + result.(eighteen), /* VERT_RESULT_CLIP_DIST1 */ result.varying[0], result.varying[1], result.varying[2], @@ -233,23 +258,43 @@ arb_output_attrib_string(GLint
Re: [Mesa-dev] [PATCH] [RFC] program: fix out of bounds array accesses and other bad things
On 12/09/2011 06:54 AM, nobled wrote: Noticed a warning: array subscript is above array bounds given at one of the existing sanity-check asserts. Turns out all the arrays of strings haven't matched the corresponding enum values in a while, if ever. I didn't know the proper names for any of these and couldn't find them in the base specs aside from result.pointsize in ARB_vertex_program, so I just filled in the enum's value as was done with other slots. Also add four STATIC_ASSERT()s to be sure and catch future additions or bumps to MAX_VARYING/etc again, and some more non-static asserts where there weren't any before. (Note, the fragment enum that corresponded to result.color(half) was removed in 8d475822e6e19fa79719c856a2db5b6a205db1b9.) --- src/mesa/program/prog_print.c | 73 + 1 files changed, 59 insertions(+), 14 deletions(-) diff --git a/src/mesa/program/prog_print.c b/src/mesa/program/prog_print.c index e9bf3aa..dfb5793 100644 --- a/src/mesa/program/prog_print.c +++ b/src/mesa/program/prog_print.c @@ -95,15 +95,15 @@ arb_input_attrib_string(GLint index, GLenum progType) /* * These strings should match the VERT_ATTRIB_x and FRAG_ATTRIB_x tokens. */ - const char *vertAttribs[] = { + static const char *const vertAttribs[] = { vertex.position, vertex.weight, vertex.normal, vertex.color.primary, vertex.color.secondary, vertex.fogcoord, - vertex.(six), - vertex.(seven), + vertex.(six), /* VERT_ATTRIB_COLOR_INDEX */ + vertex.(seven), /* VERT_ATTRIB_EDGEFLAG */ vertex.texcoord[0], vertex.texcoord[1], vertex.texcoord[2], @@ -112,6 +112,7 @@ arb_input_attrib_string(GLint index, GLenum progType) vertex.texcoord[5], vertex.texcoord[6], vertex.texcoord[7], + vertex.(sixteen), /* VERT_ATTRIB_POINT_SIZE */ vertex.attrib[0], vertex.attrib[1], vertex.attrib[2], @@ -127,9 +128,9 @@ arb_input_attrib_string(GLint index, GLenum progType) vertex.attrib[12], vertex.attrib[13], vertex.attrib[14], - vertex.attrib[15] + vertex.attrib[15] /* MAX_VARYING = 16 */ }; - const char *fragAttribs[] = { + static const char *const fragAttribs[] = { fragment.position, fragment.color.primary, fragment.color.secondary, @@ -142,6 +143,10 @@ arb_input_attrib_string(GLint index, GLenum progType) fragment.texcoord[5], fragment.texcoord[6], fragment.texcoord[7], + fragment.(twelve), /* FRAG_ATTRIB_FACE */ + fragment.(thirteen), /* FRAG_ATTRIB_PNTC */ + fragment.(fourteen), /* FRAG_ATTRIB_CLIP_DIST0 */ + fragment.(fifteen), /* FRAG_ATTRIB_CLIP_DIST1 */ fragment.varying[0], fragment.varying[1], fragment.varying[2], @@ -149,18 +154,31 @@ arb_input_attrib_string(GLint index, GLenum progType) fragment.varying[4], fragment.varying[5], fragment.varying[6], - fragment.varying[7] + fragment.varying[7], + fragment.varying[8], + fragment.varying[9], + fragment.varying[10], + fragment.varying[11], + fragment.varying[12], + fragment.varying[13], + fragment.varying[14], + fragment.varying[15] /* MAX_VARYING = 16 */ }; /* sanity checks */ + STATIC_ASSERT(Elements(vertAttribs) == VERT_ATTRIB_MAX); + STATIC_ASSERT(Elements(fragAttribs) == FRAG_ATTRIB_MAX); assert(strcmp(vertAttribs[VERT_ATTRIB_TEX0], vertex.texcoord[0]) == 0); assert(strcmp(vertAttribs[VERT_ATTRIB_GENERIC15], vertex.attrib[15]) == 0); + assert(strcmp(fragAttribs[FRAG_ATTRIB_TEX0], fragment.texcoord[0]) == 0); + assert(strcmp(fragAttribs[FRAG_ATTRIB_VAR0+15], fragment.varying[15]) == 0); if (progType == GL_VERTEX_PROGRAM_ARB) { assert(index Elements(vertAttribs)); return vertAttribs[index]; } else { + assert(progType == GL_FRAGMENT_PROGRAM_ARB); assert(index Elements(fragAttribs)); return fragAttribs[index]; } @@ -213,7 +231,7 @@ arb_output_attrib_string(GLint index, GLenum progType) /* * These strings should match the VERT_RESULT_x and FRAG_RESULT_x tokens. */ - const char *vertResults[] = { + static const char *const vertResults[] = { result.position, result.color.primary, result.color.secondary, @@ -226,6 +244,13 @@ arb_output_attrib_string(GLint index, GLenum progType) result.texcoord[5], result.texcoord[6], result.texcoord[7], + result.pointsize, /* VERT_RESULT_PSIZ */ + result.(thirteen), /* VERT_RESULT_BFC0 */ + result.(fourteen), /* VERT_RESULT_BFC1 */ + result.(fifteen), /* VERT_RESULT_EDGE */ + result.(sixteen), /* VERT_RESULT_CLIP_VERTEX */ + result.(seventeen), /* VERT_RESULT_CLIP_DIST0 */ + result.(eighteen), /* VERT_RESULT_CLIP_DIST1 */ result.varying[0],
Re: [Mesa-dev] [PATCH] [RFC] program: fix out of bounds array accesses and other bad things
On Fri, Dec 9, 2011 at 8:54 AM, nobled nob...@dreamwidth.org wrote: Can you set your name in git? It makes the log look a lot better. git config --global user.name Firstname Lastname ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev