[Mesa-dev] [PATCH v2 1/2] nv50/ir: fix use-after-free in ConstantFolding::visit

[Ilia Mirkin]

On Sat, Dec 8, 2018 at 12:10 PM Karol Herbst <kherbst at redhat.com> wrote:
>
> opnd() might delete the passed in instruction, but it's used through
> i->srcExists() later in visit
>
> v2: use continue instead return

Can you verify that this change has no effect on compilation results
(at least the high-level stats) for shader-db?

>
> Signed-off-by: Karol Herbst <kherbst at redhat.com>
> ---
>  .../nouveau/codegen/nv50_ir_peephole.cpp      | 71 +++++++++++--------
>  1 file changed, 43 insertions(+), 28 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> index 202faf0746a..d7980a44ab1 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> @@ -370,7 +370,8 @@ private:
>
>     void expr(Instruction *, ImmediateValue&, ImmediateValue&);
>     void expr(Instruction *, ImmediateValue&, ImmediateValue&, ImmediateValue&);
> -   void opnd(Instruction *, ImmediateValue&, int s);
> +   /* true if i was deleted */
> +   bool opnd(Instruction *i, ImmediateValue&, int s);
>     void opnd3(Instruction *, ImmediateValue&);
>
>     void unary(Instruction *, const ImmediateValue&);
> @@ -421,11 +422,13 @@ ConstantFolding::visit(BasicBlock *bb)
>            i->src(0).getImmediate(src0) && i->src(1).getImmediate(src1))
>           expr(i, src0, src1);
>        else
> -      if (i->srcExists(0) && i->src(0).getImmediate(src0))
> -         opnd(i, src0, 0);
> -      else
> +      if (i->srcExists(0) && i->src(0).getImmediate(src0)) {
> +         if (opnd(i, src0, 0))
> +            continue;
> +      } else

Please make this whole (outer) if/else sequence have { }. Even though
technically they're not necessary anywhere, by the time it gets to
having nested ifs ... it's a good idea.