| Message ID | 20200611144529.8873-3-peter.maydell@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/arm: Convert 2-reg-scalar to decodetree | expand |
On 6/11/20 7:45 AM, Peter Maydell wrote: > In commit 37bfce81b10450071 we accidentally introduced a leak of a TCG > temporary in do_2shift_env_64(); free it. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > My test setup wasn't looking for temporary-leak warnings (they are > not as easy to get at as they used to be because they only appear > if you enable qemu_log tracing for some other purpose). This is the > only one that snuck through, though. > --- > target/arm/translate-neon.inc.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c > index 7c4888a80c9..f2c241a87e9 100644 > --- a/target/arm/translate-neon.inc.c > +++ b/target/arm/translate-neon.inc.c > @@ -1329,6 +1329,7 @@ static bool do_2shift_env_64(DisasContext *s, arg_2reg_shift *a, > neon_load_reg64(tmp, a->vm + pass); > fn(tmp, cpu_env, tmp, constimm); > neon_store_reg64(tmp, a->vd + pass); > + tcg_temp_free_i64(tmp); Huh. I thought all the a32 stores magically freed their inputs. I guess that's just the general-purpose registers. Anyway, Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On Thu, 11 Jun 2020 at 16:43, Richard Henderson <richard.henderson@linaro.org> wrote: > > On 6/11/20 7:45 AM, Peter Maydell wrote: > > In commit 37bfce81b10450071 we accidentally introduced a leak of a TCG > > temporary in do_2shift_env_64(); free it. > > > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > --- > > My test setup wasn't looking for temporary-leak warnings (they are > > not as easy to get at as they used to be because they only appear > > if you enable qemu_log tracing for some other purpose). This is the > > only one that snuck through, though. > > --- > > target/arm/translate-neon.inc.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c > > index 7c4888a80c9..f2c241a87e9 100644 > > --- a/target/arm/translate-neon.inc.c > > +++ b/target/arm/translate-neon.inc.c > > @@ -1329,6 +1329,7 @@ static bool do_2shift_env_64(DisasContext *s, arg_2reg_shift *a, > > neon_load_reg64(tmp, a->vm + pass); > > fn(tmp, cpu_env, tmp, constimm); > > neon_store_reg64(tmp, a->vd + pass); > > + tcg_temp_free_i64(tmp); > > Huh. I thought all the a32 stores magically freed their inputs. I guess > that's just the general-purpose registers. Confusingly, neon_load_reg()/neon_store_reg() do the "create new TCG temp for the load, free the store input" thing, but neon_load_reg64()/neon_store_reg64()/neon_load_reg32()/neon_store_reg32() do not. I'd kind of like to get rid of this weird proliferation of load and store functions, but neon_load_reg()/neon_store_reg() are also the only ones which take a (regno, pass) pair of inputs, so it's not a completely trivial substitution. thanks -- PMM
diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c index 7c4888a80c9..f2c241a87e9 100644 --- a/target/arm/translate-neon.inc.c +++ b/target/arm/translate-neon.inc.c @@ -1329,6 +1329,7 @@ static bool do_2shift_env_64(DisasContext *s, arg_2reg_shift *a, neon_load_reg64(tmp, a->vm + pass); fn(tmp, cpu_env, tmp, constimm); neon_store_reg64(tmp, a->vd + pass); + tcg_temp_free_i64(tmp); } tcg_temp_free_i64(constimm); return true;
In commit 37bfce81b10450071 we accidentally introduced a leak of a TCG temporary in do_2shift_env_64(); free it. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- My test setup wasn't looking for temporary-leak warnings (they are not as easy to get at as they used to be because they only appear if you enable qemu_log tracing for some other purpose). This is the only one that snuck through, though. --- target/arm/translate-neon.inc.c | 1 + 1 file changed, 1 insertion(+)