On Thu, May 19, 2016 at 4:42 PM, Matt Turner <matts...@gmail.com> wrote: > On Wed, May 18, 2016 at 8:54 AM, Rob Clark <robdcl...@gmail.com> wrote: >> From: Rob Clark <robcl...@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 <matts...@gmail.com> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
