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

[Brian Paul]

On 01/12/2016 02:49 AM, Jose Fonseca wrote:
> 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?

I would suggest that too.

-Brian


>
> 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
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev