[Mesa-dev] [RFC 3/7] nir: coverity unitialized pointer read

[Rob Clark]

On Thu, May 19, 2016 at 4:42 PM, Matt Turner <mattst88 at gmail.com> wrote:
> On Wed, May 18, 2016 at 8:54 AM, Rob Clark <robdclark at gmail.com> wrote:
>> From: Rob Clark <robclark at freedesktop.org>
>>
>> Not sure how coverity arrives at the conclusion that we can read comp[j]
>> unitialized (around line 204), other than not being aware that ncomp is
>> greater than 1 so it won't underflow in the 'if (tex->is_array)' case.
>> ---
>>  src/compiler/nir/nir_lower_tex.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/src/compiler/nir/nir_lower_tex.c b/src/compiler/nir/nir_lower_tex.c
>> index a080475..c05d48b 100644
>> --- a/src/compiler/nir/nir_lower_tex.c
>> +++ b/src/compiler/nir/nir_lower_tex.c
>> @@ -177,6 +177,12 @@ saturate_src(nir_builder *b, nir_tex_instr *tex, unsigned sat_mask)
>>        /* split src into components: */
>>        nir_ssa_def *comp[4];
>>
>> +      /* NOTE: coord_components won't be >4 or <1 but coverity doesn't
>> +       * know this:
>> +       */
>
> I'd drop the comment. git blame will allow us to figure out why the
> assume() is there if needed.
>
>> +      assume(tex->coord_components < ARRAY_SIZE(comp));
>> +      assume(tex->coord_components >= 1);
>
> I think the second one is sufficient, since part of the path involves
> ncomp-- I think that it believes coord_components can be zero so
> subtracting 1 will produce UINT_MAX.
>
> With the comment and the first assume() dropped,

fwiw, first assume() was mostly because I was surprised that coverity
wasn't also upset about coord_components>=4 case.  (Not sure if it
just doesn't bother warning about that sort of thing?  Since I'm not
entirely sure how it could figure that out otherwise.. maybe it really
is that clever..)

BR,
-R

> Reviewed-by: Matt Turner <mattst88 at gmail.com>