[Piglit] [PATCH] Fix out of bounds read in the AMD_performance_monitor measure test.

[Ian Romanick]

On 09/19/2013 03:41 PM, Kenneth Graunke wrote:
> If the last counter returned is a 32-bit value, reading a uint64_t might
> go past the end of the buffer.  So, delay reading the 64-bit value until
> we know the type is 64-bit.
> 
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

Some semi-random thoughts below...

> ---
>  tests/spec/amd_performance_monitor/measure.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/spec/amd_performance_monitor/measure.c b/tests/spec/amd_performance_monitor/measure.c
> index bc39c84..61c98b1 100644
> --- a/tests/spec/amd_performance_monitor/measure.c
> +++ b/tests/spec/amd_performance_monitor/measure.c
> @@ -194,7 +194,6 @@ test_basic_measurement(unsigned group)
>  		/* Counter values */
>  		uint32_t u32 = p[2];
>  		float f = ((float *) p)[2];
> -		uint64_t u64 = ((uint64_t *) p)[1];
>  
>  		/* Query results */
>  		GLenum counter_type = GL_NONE;
> @@ -226,6 +225,7 @@ test_basic_measurement(unsigned group)
>  			break;
>  		}
>  		case GL_UNSIGNED_INT64_AMD: {
> +			uint64_t u64 = ((uint64_t *) p)[1];

This looks so weird to me, given that the others use [2].  The
alternative is also a face only a mother could love:

        uint64_t u64 = *(uint64_t *)(p + 2);

Of course, then the other usages could follow the same pattern:

 		/* Counter values */
 		uint32_t u32 = *(uint32_t *)(p + 2);
 		float f = *(float *)(p + 2);

What do you think?

>  			verify(u64 >= range[0]);
>  			verify(u64 <= range[1]);
>  			break;
>