[Mesa-dev] [PATCH] draw: fix key comparison with uninitialized value

[Jose Fonseca]

On 12/01/16 02:11, sroland at vmware.com wrote:
> From: Roland Scheidegger <sroland at vmware.com>
>
> Discovered by accident, valgrind was complaining (could have possibly caused
> us to create redundant geometry shader variants).

Great fine.

> ---
>   src/gallium/auxiliary/draw/draw_llvm.c | 3 +++
>   src/gallium/auxiliary/draw/draw_llvm.h | 5 +++++
>   2 files changed, 8 insertions(+)
>
> diff --git a/src/gallium/auxiliary/draw/draw_llvm.c b/src/gallium/auxiliary/draw/draw_llvm.c
> index f25dafe..faa6e40 100644
> --- a/src/gallium/auxiliary/draw/draw_llvm.c
> +++ b/src/gallium/auxiliary/draw/draw_llvm.c
> @@ -2315,6 +2315,9 @@ draw_gs_llvm_make_variant_key(struct draw_llvm *llvm, char *store)
>
>      key = (struct draw_gs_llvm_variant_key *)store;
>
> +   assert(offsetof(struct draw_gs_llvm_variant_key, samplers[0]) == 4);
> +   key->pad1 = 0;
> +
>      key->num_outputs = draw_total_gs_outputs(llvm->draw);
>
>      /* All variants of this shader will have the same value for
> diff --git a/src/gallium/auxiliary/draw/draw_llvm.h b/src/gallium/auxiliary/draw/draw_llvm.h
> index f617a29..67f2170 100644
> --- a/src/gallium/auxiliary/draw/draw_llvm.h
> +++ b/src/gallium/auxiliary/draw/draw_llvm.h
> @@ -332,6 +332,11 @@ struct draw_gs_llvm_variant_key
>      unsigned nr_samplers:8;
>      unsigned nr_sampler_views:8;
>      unsigned num_outputs:8;
> +   /*
> +    * it is important there are no holes in this struct
> +    * (and all padding gets zeroed). 32 bit pad should be enough...
> +    */
> +   unsigned pad1:8;
>
>      struct draw_sampler_static_state samplers[1];
>   };
>

The change looks OK, but I wonder if it wouldn't be more future proof to 
just memset the whole structure to zero?

And then ensure we always use memcpy to copy, memcmp to compare.  I 
believe this is the pattern we've been following elsewhere.

Either way,

Reviewed-by: Jose Fonseca <jfonseca at vmware.com>

Jose