[Piglit] [PATCH 1/3] Make atomic counter tests more robust against race conditions.

[Jordan Justen]

Series Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

On 2015-10-21 01:11:25, Kenneth Graunke wrote:
> From: Chris Wilson <chris at chris-wilson.co.uk>
> 
> Drivers with synchronization bugs could generate errors such as:
> 
> Probe value at (0)
>   Expected: 0x00000001
>   Observed: 0x00000001
> 
> This is because the initial comparison and the print statement access
> the buffer at different times, leading to the following scenario.
> 
> 1. Comparison reads wrong value from the buffer (test properly fails).
> 2. Right value finally lands in buffer.
> 3. Error message prints the current value...which is the not the value
>    used by the comparison.  Instead, it's the expected value, so the
>    message makes no sense.
> 
> [Ken imported this patch from Chris Wilson's Bugzilla comment linked
>  below and wrote a commit message.]
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91298
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  tests/spec/arb_shader_atomic_counters/common.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> Chris, I made you the author on this patch since it's your code.
> It didn't look like a patch ever hit the mailing list, so I went
> ahead and put one together...
> 
> diff --git a/tests/spec/arb_shader_atomic_counters/common.c b/tests/spec/arb_shader_atomic_counters/common.c
> index b966009..c732699 100644
> --- a/tests/spec/arb_shader_atomic_counters/common.c
> +++ b/tests/spec/arb_shader_atomic_counters/common.c
> @@ -35,6 +35,7 @@ atomic_counters_probe_buffer(unsigned base, unsigned count,
>          uint32_t *p = glMapBufferRange(
>                  GL_ATOMIC_COUNTER_BUFFER, base * sizeof(uint32_t),
>                  count * sizeof(uint32_t), GL_MAP_READ_BIT);
> +        bool pass = true;
>          unsigned i;
>  
>          if (!p) {
> @@ -43,17 +44,18 @@ atomic_counters_probe_buffer(unsigned base, unsigned count,
>          }
>  
>          for (i = 0; i < count; ++i) {
> -                if (p[i] != expected[i]) {
> +                uint32_t found = p[i];
> +                if (found != expected[i]) {
>                          printf("Probe value at (%i)\n", i);
>                          printf("  Expected: 0x%08x\n", expected[i]);
> -                        printf("  Observed: 0x%08x\n", p[i]);
> -                        glUnmapBuffer(GL_ATOMIC_COUNTER_BUFFER);
> -                        return false;
> +                        printf("  Observed: 0x%08x\n", found);
> +                        pass = false;
> +                        break;
>                  }
>          }
>  
>          glUnmapBuffer(GL_ATOMIC_COUNTER_BUFFER);
> -        return true;
> +        return pass;
>  }
>  
>  bool
> -- 
> 2.6.1
> 
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit