[Mesa-dev] [PATCH] gallivm: fix out-of-bounds access with mirror_clamp_to_edge address mode

[Jose Fonseca]

Looks great to me.

Jose

----- Original Message -----
> From: Roland Scheidegger <sroland at vmware.com>
> 
> Surprising this bug survived so long, we were missing a clamp (in the
> linear filtering version).
> (Valgrind complained a lot about invalid reads with piglit texwrap,
> I've also seen spurios failures in this test which might have
> happened due to this. Valgrind probably didn't complain before the
> alignment reduction in llvmpipe to 4x4 since the test is using tiny
> textures so the reads were still always well within allocated area.)
> While here, also do an effective clamp (after half subtraction)
> of [0,length-0.5] instead of [0, length-1] which saves an instruction
> (the filtering weight could be different due to this, but only if
> both texels point to the same max texel so it doesn't matter).
> (Both changes are borrowed from PIPE_TEX_CLAMP_TO_EDGE case.)
> 
> Note: This is a candidate for the stable branches.
> ---
>  src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c |   13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> b/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> index 7ac0029..e0a59d0 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> @@ -436,7 +436,6 @@ lp_build_sample_wrap_linear(struct
> lp_build_sample_context *bld,
>  
>     case PIPE_TEX_WRAP_MIRROR_CLAMP_TO_EDGE:
>        {
> -         LLVMValueRef min, max;
>           struct lp_build_context abs_coord_bld = bld->coord_bld;
>           abs_coord_bld.type.sign = FALSE;
>  
> @@ -450,16 +449,18 @@ lp_build_sample_wrap_linear(struct
> lp_build_sample_context *bld,
>           }
>           coord = lp_build_abs(coord_bld, coord);
>  
> -         /* clamp to [0.5, length - 0.5] */
> -         min = half;
> -         max = lp_build_sub(coord_bld, length_f, min);
> -         coord = lp_build_clamp(coord_bld, coord, min, max);
> -
> +         /* clamp to length max */
> +         coord = lp_build_min(coord_bld, coord, length_f);
> +         /* subtract 0.5 */
>           coord = lp_build_sub(coord_bld, coord, half);
> +         /* clamp to [0, length - 0.5] */
> +         coord = lp_build_max(coord_bld, coord, coord_bld->zero);
>  
>           /* convert to int, compute lerp weight */
>           lp_build_ifloor_fract(&abs_coord_bld, coord, &coord0, &weight);
>           coord1 = lp_build_add(int_coord_bld, coord0, int_coord_bld->one);
> +         /* coord1 = min(coord1, length-1) */
> +         coord1 = lp_build_min(int_coord_bld, coord1, length_minus_one);
>        }
>        break;
>  
> --
> 1.7.9.5
>