[Mesa-dev] [PATCH] i965/vec4: Only examine virtual_grf_end for GRF sources

[Kenneth Graunke]

On Wednesday, September 10, 2014 06:01:19 PM Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
> 
> If the source is not a GRF, it could have a register >= virtual_grf_count.
> Accessing virtual_grf_end with such a register would lead to
> out-of-bounds access.  Make sure the source is a GRF before accessing
> virtual_grf_end.
> 
> Fixes gles3conform failure in:
> 
> ES3-CTS.shaders.struct.uniform.sampler_array_vertex.
> 
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83215
> ---
>  src/mesa/drivers/dri/i965/brw_vec4_cse.cpp | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
> index a0f6e77..da3e65a 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
> @@ -224,14 +224,21 @@ vec4_visitor::opt_cse_local(bblock_t *block)
>              /* Kill any AEB entries using registers that don't get reused any
>               * more -- a sure sign they'll fail operands_match().
>               */
> -            int last_reg_use = MAX2(MAX2(virtual_grf_end[src->reg * 4 + 0],
> -                                         virtual_grf_end[src->reg * 4 + 1]),
> -                                    MAX2(virtual_grf_end[src->reg * 4 + 2],
> -                                         virtual_grf_end[src->reg * 4 + 3]));
> -            if (src->file == GRF && last_reg_use < ip) {
> -               entry->remove();
> -               ralloc_free(entry);
> -               break;
> +            if (src->file == GRF) {
> +               assert((src->reg * 4 + 0) < (virtual_grf_count * 4));
> +               assert((src->reg * 4 + 1) < (virtual_grf_count * 4));
> +               assert((src->reg * 4 + 2) < (virtual_grf_count * 4));

I'd probably drop these from the final patch.

Otherwise, (and with the "fixes test" line removed), this is:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

Thanks, Ian!  Good find!

> +               assert((src->reg * 4 + 3) < (virtual_grf_count * 4));
> +
> +               int last_reg_use = MAX2(MAX2(virtual_grf_end[src->reg * 4 + 0],
> +                                            virtual_grf_end[src->reg * 4 + 1]),
> +                                       MAX2(virtual_grf_end[src->reg * 4 + 2],
> +                                            virtual_grf_end[src->reg * 4 + 3]));
> +               if (last_reg_use < ip) {
> +                  entry->remove();
> +                  ralloc_free(entry);
> +                  break;
> +               }
>              }
>           }
>        }
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140911/96d63768/attachment.sig>