[Mesa-dev] [PATCH] nir/lower_vec_to_movs: Better report channels handled by insert_mov

[Jason Ekstrand]

This fixes two issues.  First, we had a use-after-free in the case where
the instruction got deleted and we tried to return mov->dest.write_mask.
Second, in the case where we are doing a self-mov of a register, we delete
those channels that are moved to themselves from the write-mask.  This
means that those channels aren't reported as being handled even though they
are.  We now stash off the write-mask before remove unneeded channels so
that they still get reported as handled.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94073
---
 src/compiler/nir/nir_lower_vec_to_movs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/compiler/nir/nir_lower_vec_to_movs.c 
b/src/compiler/nir/nir_lower_vec_to_movs.c
index 06d6279..f51cede 100644
--- a/src/compiler/nir/nir_lower_vec_to_movs.c
+++ b/src/compiler/nir/nir_lower_vec_to_movs.c
@@ -83,6 +83,8 @@ insert_mov(nir_alu_instr *vec, unsigned start_idx, nir_shader 
*shader)
   }
}
 
+   unsigned channels_handled = mov->dest.write_mask;
+
/* In some situations (if the vecN is involved in a phi-web), we can end
 * up with a mov from a register to itself.  Some of those channels may end
 * up doing nothing and there's no reason to have them as part of the mov.
@@ -103,7 +105,7 @@ insert_mov(nir_alu_instr *vec, unsigned start_idx, 
nir_shader *shader)
   ralloc_free(mov);
}
 
-   return mov->dest.write_mask;
+   return channels_handled;
 }
 
 static bool
-- 
2.5.0.400.gff86faf

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] nir/lower_vec_to_movs: Better report channels handled by insert_mov

[Jason Ekstrand]

On Wed, Feb 10, 2016 at 1:47 PM, Matt Turner  wrote:

> On Wed, Feb 10, 2016 at 1:27 PM, Jason Ekstrand 
> wrote:
> > This fixes two issues.  First, we had a use-after-free in the case where
> > the instruction got deleted and we tried to return mov->dest.write_mask.
> > Second, in the case where we are doing a self-mov of a register, we
> delete
> > those channels that are moved to themselves from the write-mask.  This
> > means that those channels aren't reported as being handled even though
> they
> > are.  We now stash off the write-mask before remove unneeded channels so
> > that they still get reported as handled.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94073
> > ---
> >  src/compiler/nir/nir_lower_vec_to_movs.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/compiler/nir/nir_lower_vec_to_movs.c
> b/src/compiler/nir/nir_lower_vec_to_movs.c
> > index 06d6279..f51cede 100644
> > --- a/src/compiler/nir/nir_lower_vec_to_movs.c
> > +++ b/src/compiler/nir/nir_lower_vec_to_movs.c
> > @@ -83,6 +83,8 @@ insert_mov(nir_alu_instr *vec, unsigned start_idx,
> nir_shader *shader)
> >}
> > }
> >
> > +   unsigned channels_handled = mov->dest.write_mask;
> > +
> > /* In some situations (if the vecN is involved in a phi-web), we can
> end
> >  * up with a mov from a register to itself.  Some of those channels
> may end
> >  * up doing nothing and there's no reason to have them as part of
> the mov.
> > @@ -103,7 +105,7 @@ insert_mov(nir_alu_instr *vec, unsigned start_idx,
> nir_shader *shader)
> >ralloc_free(mov);
> > }
> >
> > -   return mov->dest.write_mask;
> > +   return channels_handled;
> >  }
>
> Yup. I totally missed the very obvious use-after-free in 8dcbca5.
>
> Reviewed-by: Matt Turner 
>

Thanks!


>
> I'd tag this for stable as well since it fixes a WebGL conformance test.
>

Yup.  Will do.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] nir/lower_vec_to_movs: Better report channels handled by insert_mov

[Matt Turner]

On Wed, Feb 10, 2016 at 1:27 PM, Jason Ekstrand  wrote:
> This fixes two issues.  First, we had a use-after-free in the case where
> the instruction got deleted and we tried to return mov->dest.write_mask.
> Second, in the case where we are doing a self-mov of a register, we delete
> those channels that are moved to themselves from the write-mask.  This
> means that those channels aren't reported as being handled even though they
> are.  We now stash off the write-mask before remove unneeded channels so
> that they still get reported as handled.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94073
> ---
>  src/compiler/nir/nir_lower_vec_to_movs.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/compiler/nir/nir_lower_vec_to_movs.c 
> b/src/compiler/nir/nir_lower_vec_to_movs.c
> index 06d6279..f51cede 100644
> --- a/src/compiler/nir/nir_lower_vec_to_movs.c
> +++ b/src/compiler/nir/nir_lower_vec_to_movs.c
> @@ -83,6 +83,8 @@ insert_mov(nir_alu_instr *vec, unsigned start_idx, 
> nir_shader *shader)
>}
> }
>
> +   unsigned channels_handled = mov->dest.write_mask;
> +
> /* In some situations (if the vecN is involved in a phi-web), we can end
>  * up with a mov from a register to itself.  Some of those channels may 
> end
>  * up doing nothing and there's no reason to have them as part of the mov.
> @@ -103,7 +105,7 @@ insert_mov(nir_alu_instr *vec, unsigned start_idx, 
> nir_shader *shader)
>ralloc_free(mov);
> }
>
> -   return mov->dest.write_mask;
> +   return channels_handled;
>  }

Yup. I totally missed the very obvious use-after-free in 8dcbca5.

Reviewed-by: Matt Turner 

I'd tag this for stable as well since it fixes a WebGL conformance test.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev