On Thu, Oct 01, 2015 at 04:59:35PM +0100, Michel Thierry wrote:
> We tried to fix this in commit fdc454c1484a ("drm/i915: Prevent out of
> range pt in gen6_for_each_pde").
>
> But the static analyzer still complains that, just before we break due
> to "iter < I915_PDES", we do "pt = (pd)->page_table[iter]" with an
> iter value that is bigger than I915_PDES. Of course, this isn't really
> a problem since no one uses pt outside the macro. Still, every single
> new usage of the macro will create a new issue for us to mark as a
> false positive.
>
> Also, Paulo re-started the discussion a while ago [1], but didn't end up
> implemented.
>
> In order to "solve" this "problem", this patch takes the ideas from
> Chris and Dave, but that check would change the desired behavior of the
> code, because the object (for example pdp->page_directory[iter]) can be
> null during init/alloc, and C would take this as false, breaking the for
> loop immediately.
>
> This has been already verified with "static analysis tools".
>
> [1]http://lists.freedesktop.org/archives/intel-gfx/2015-June/068548.html
>
> Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Dave Gordon <david.s.gordon@xxxxxxxxx>
> Signed-off-by: Michel Thierry <michel.thierry@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.h | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 9fbb07d..94f8344 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -394,7 +394,9 @@ struct i915_hw_ppgtt {
> */
> #define gen6_for_each_pde(pt, pd, start, length, temp, iter) \
> for (iter = gen6_pde_index(start); \
> - pt = (pd)->page_table[iter], length > 0 && iter < I915_PDES; \
> + pt = (length > 0 && iter < I915_PDES) ? \
> + (pd)->page_table[iter] : NULL, \
> + length > 0 && iter < I915_PDES; \
length > 0 && iter < I915_PDES ? (pt = (pd)->page_table[iter]) : 0,
as the compiler wouldn't be able to CSE it otherwise (I think).
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
