[PATCH] target/arm: fix TCG leak for fcvt half->double

Alex Bennée posted 1 patch 5 years, 9 months ago
Test docker-quick@centos7 passed
Test FreeBSD passed
Test docker-mingw@fedora passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200131153439.26027-1-alex.bennee@linaro.org
target/arm/translate-a64.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] target/arm: fix TCG leak for fcvt half->double
Posted by Alex Bennée 5 years, 9 months ago
When support for the AHP flag was added we inexplicably only freed the
new temps in one of the two legs. Move those tcg_temp_free to the same
level as the allocation to fix that leak.

Fixes: 486624fcd3eac
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 target/arm/translate-a64.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 96a5be2b372..766a03335bf 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -5778,8 +5778,6 @@ static void handle_fp_fcvt(DisasContext *s, int opcode,
             TCGv_i32 tcg_rd = tcg_temp_new_i32();
             gen_helper_vfp_fcvt_f16_to_f32(tcg_rd, tcg_rn, tcg_fpst, tcg_ahp);
             write_fp_sreg(s, rd, tcg_rd);
-            tcg_temp_free_ptr(tcg_fpst);
-            tcg_temp_free_i32(tcg_ahp);
             tcg_temp_free_i32(tcg_rd);
         } else {
             /* Half to double */
@@ -5789,6 +5787,8 @@ static void handle_fp_fcvt(DisasContext *s, int opcode,
             tcg_temp_free_i64(tcg_rd);
         }
         tcg_temp_free_i32(tcg_rn);
+        tcg_temp_free_ptr(tcg_fpst);
+        tcg_temp_free_i32(tcg_ahp);
         break;
     }
     default:
-- 
2.20.1


Re: [PATCH] target/arm: fix TCG leak for fcvt half->double
Posted by Philippe Mathieu-Daudé 5 years, 9 months ago
On 1/31/20 4:34 PM, Alex Bennée wrote:
> When support for the AHP flag was added we inexplicably only freed the
> new temps in one of the two legs. Move those tcg_temp_free to the same
> level as the allocation to fix that leak.

Probably too much code refactoring :/

> 
> Fixes: 486624fcd3eac

Maybe:
Reported-by: Peter Maydell <peter.maydell@linaro.org>

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   target/arm/translate-a64.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index 96a5be2b372..766a03335bf 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -5778,8 +5778,6 @@ static void handle_fp_fcvt(DisasContext *s, int opcode,
>               TCGv_i32 tcg_rd = tcg_temp_new_i32();
>               gen_helper_vfp_fcvt_f16_to_f32(tcg_rd, tcg_rn, tcg_fpst, tcg_ahp);
>               write_fp_sreg(s, rd, tcg_rd);
> -            tcg_temp_free_ptr(tcg_fpst);
> -            tcg_temp_free_i32(tcg_ahp);
>               tcg_temp_free_i32(tcg_rd);
>           } else {
>               /* Half to double */
> @@ -5789,6 +5787,8 @@ static void handle_fp_fcvt(DisasContext *s, int opcode,
>               tcg_temp_free_i64(tcg_rd);
>           }
>           tcg_temp_free_i32(tcg_rn);
> +        tcg_temp_free_ptr(tcg_fpst);
> +        tcg_temp_free_i32(tcg_ahp);
>           break;
>       }
>       default:
> 


Re: [PATCH] target/arm: fix TCG leak for fcvt half->double
Posted by Peter Maydell 5 years, 9 months ago
On Fri, 31 Jan 2020 at 16:50, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> On 1/31/20 4:34 PM, Alex Bennée wrote:
> > When support for the AHP flag was added we inexplicably only freed the
> > new temps in one of the two legs. Move those tcg_temp_free to the same
> > level as the allocation to fix that leak.
>
> Probably too much code refactoring :/
>
> >
> > Fixes: 486624fcd3eac
>
> Maybe:
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
>
> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks; applied to master as that will let me apply the
tracing pullreq without having to suppress or ignore the
warning.

-- PMM