[Mesa-dev] [PATCH] rtasm: deal with size overflows by casting to ptrdiff_t

[Brian Paul]

On 01/21/2014 06:37 PM, Ilia Mirkin wrote:
> This was discovered as a result of the draw-elements-base-vertex-neg
> piglit test, which passes very negative offsets in, followed up by large
> indices. The nouveau code correctly adjusts the pointer, but the
> transfer code needs to do the proper inverse correction. Similarly fix
> up the SSE code to do a 64-bit multiply to compute the proper offset.
>
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
>
> With this change, nouveau passes for the draw-elements-base-vertex-neg piglit
> test with user_varrays, on a 64-bit setup both with and without
> GALLIUM_NOSSE=1. I'm pretty sure that the change should be minimal to a
> non-x86 setup since the rexw will be a no-op. I guess there will be an extra
> register use for the mov, but it shouldn't be too expensive, esp on anything
> remotely current.
>
>   src/gallium/auxiliary/translate/translate_generic.c | 2 +-
>   src/gallium/auxiliary/translate/translate_sse.c     | 8 ++++++--
>   2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/auxiliary/translate/translate_generic.c b/src/gallium/auxiliary/translate/translate_generic.c
> index 5bf97db..5ffce32 100644
> --- a/src/gallium/auxiliary/translate/translate_generic.c
> +++ b/src/gallium/auxiliary/translate/translate_generic.c
> @@ -638,7 +638,7 @@ static ALWAYS_INLINE void PIPE_CDECL generic_run_one( struct translate_generic *
>            }
>
>            src = tg->attrib[attr].input_ptr +
> -               tg->attrib[attr].input_stride * index;
> +               (ptrdiff_t)tg->attrib[attr].input_stride * index;
>
>            copy_size = tg->attrib[attr].copy_size;
>            if(likely(copy_size >= 0))
> diff --git a/src/gallium/auxiliary/translate/translate_sse.c b/src/gallium/auxiliary/translate/translate_sse.c
> index a78ea91..a72454a 100644
> --- a/src/gallium/auxiliary/translate/translate_sse.c
> +++ b/src/gallium/auxiliary/translate/translate_sse.c
> @@ -1121,7 +1121,9 @@ static boolean init_inputs( struct translate_sse *p,
>               x86_cmovcc(p->func, tmp_EAX, buf_max_index, cc_AE);
>            }
>
> -         x86_imul(p->func, tmp_EAX, buf_stride);
> +         x86_mov(p->func, p->tmp2_EDX, buf_stride);
> +         x64_rexw(p->func);
> +         x86_imul(p->func, tmp_EAX, p->tmp2_EDX);
>            x64_rexw(p->func);
>            x86_add(p->func, tmp_EAX, buf_base_ptr);
>
> @@ -1207,7 +1209,9 @@ static struct x86_reg get_buffer_ptr( struct translate_sse *p,
>         x86_cmp(p->func, ptr, buf_max_index);
>         x86_cmovcc(p->func, ptr, buf_max_index, cc_AE);
>
> -      x86_imul(p->func, ptr, buf_stride);
> +      x86_mov(p->func, p->tmp2_EDX, buf_stride);
> +      x64_rexw(p->func);
> +      x86_imul(p->func, ptr, p->tmp2_EDX);
>         x64_rexw(p->func);
>         x86_add(p->func, ptr, buf_base_ptr);
>         return ptr;
>

I'm no x86 expert, but this looks OK to me, and if it works for you...

Reviewed-by: Brian Paul <brianp at vmware.com>

Should this go into the 10.0.x branch too?