| Message ID | 20200702175605.1987125-1-richard.henderson@linaro.org |
|---|---|
| State | Accepted |
| Commit | 4b4dc9750a0aa0b9766bd755bf6512a84744ce8a |
| Headers | show |
| Series | target/arm: Fix temp double-free in sve ldr/str | expand |
On 7/2/20 7:56 PM, Richard Henderson wrote: > The temp that gets assigned to clean_addr has been allocated with > new_tmp_a64, which means that it will be freed at the end of the > instruction. Freeing it earlier leads to assertion failure. > > The loop creates a complication, in which we allocate a new local > temp, which does need freeing, and the final code path is shared > between the loop and non-loop. > > Fix this complication by adding new_tmp_a64_local so that the new > local temp is freed at the end, and can be treated exactly like > the non-loop path. > > Fixes: bba87d0a0f4 > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/translate-a64.h | 1 + > target/arm/translate-a64.c | 6 ++++++ > target/arm/translate-sve.c | 8 ++------ > 3 files changed, 9 insertions(+), 6 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
On Thu, 2 Jul 2020 at 18:56, Richard Henderson <richard.henderson@linaro.org> wrote: > > The temp that gets assigned to clean_addr has been allocated with > new_tmp_a64, which means that it will be freed at the end of the > instruction. Freeing it earlier leads to assertion failure. > > The loop creates a complication, in which we allocate a new local > temp, which does need freeing, and the final code path is shared > between the loop and non-loop. > > Fix this complication by adding new_tmp_a64_local so that the new > local temp is freed at the end, and can be treated exactly like > the non-loop path. > > Fixes: bba87d0a0f4 > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Applied to target-arm.next, thanks. -- PMM
diff --git a/target/arm/translate-a64.h b/target/arm/translate-a64.h index 49e4865918..647f0c74f6 100644 --- a/target/arm/translate-a64.h +++ b/target/arm/translate-a64.h @@ -30,6 +30,7 @@ void unallocated_encoding(DisasContext *s); } while (0) TCGv_i64 new_tmp_a64(DisasContext *s); +TCGv_i64 new_tmp_a64_local(DisasContext *s); TCGv_i64 new_tmp_a64_zero(DisasContext *s); TCGv_i64 cpu_reg(DisasContext *s, int reg); TCGv_i64 cpu_reg_sp(DisasContext *s, int reg); diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index 73d753f11f..8c0764957c 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -461,6 +461,12 @@ TCGv_i64 new_tmp_a64(DisasContext *s) return s->tmp_a64[s->tmp_a64_count++] = tcg_temp_new_i64(); } +TCGv_i64 new_tmp_a64_local(DisasContext *s) +{ + assert(s->tmp_a64_count < TMP_A64_MAX); + return s->tmp_a64[s->tmp_a64_count++] = tcg_temp_local_new_i64(); +} + TCGv_i64 new_tmp_a64_zero(DisasContext *s) { TCGv_i64 t = new_tmp_a64(s); diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c index f318ca265f..08f0fd15b2 100644 --- a/target/arm/translate-sve.c +++ b/target/arm/translate-sve.c @@ -4372,9 +4372,8 @@ static void do_ldr(DisasContext *s, uint32_t vofs, int len, int rn, int imm) /* Copy the clean address into a local temp, live across the loop. */ t0 = clean_addr; - clean_addr = tcg_temp_local_new_i64(); + clean_addr = new_tmp_a64_local(s); tcg_gen_mov_i64(clean_addr, t0); - tcg_temp_free_i64(t0); gen_set_label(loop); @@ -4422,7 +4421,6 @@ static void do_ldr(DisasContext *s, uint32_t vofs, int len, int rn, int imm) tcg_gen_st_i64(t0, cpu_env, vofs + len_align); tcg_temp_free_i64(t0); } - tcg_temp_free_i64(clean_addr); } /* Similarly for stores. */ @@ -4463,9 +4461,8 @@ static void do_str(DisasContext *s, uint32_t vofs, int len, int rn, int imm) /* Copy the clean address into a local temp, live across the loop. */ t0 = clean_addr; - clean_addr = tcg_temp_local_new_i64(); + clean_addr = new_tmp_a64_local(s); tcg_gen_mov_i64(clean_addr, t0); - tcg_temp_free_i64(t0); gen_set_label(loop); @@ -4509,7 +4506,6 @@ static void do_str(DisasContext *s, uint32_t vofs, int len, int rn, int imm) } tcg_temp_free_i64(t0); } - tcg_temp_free_i64(clean_addr); } static bool trans_LDR_zri(DisasContext *s, arg_rri *a)
The temp that gets assigned to clean_addr has been allocated with new_tmp_a64, which means that it will be freed at the end of the instruction. Freeing it earlier leads to assertion failure. The loop creates a complication, in which we allocate a new local temp, which does need freeing, and the final code path is shared between the loop and non-loop. Fix this complication by adding new_tmp_a64_local so that the new local temp is freed at the end, and can be treated exactly like the non-loop path. Fixes: bba87d0a0f4 Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/translate-a64.h | 1 + target/arm/translate-a64.c | 6 ++++++ target/arm/translate-sve.c | 8 ++------ 3 files changed, 9 insertions(+), 6 deletions(-) -- 2.25.1