* [PATCH] target/arm: Fix temp double-free in sve ldr/str
2020-07-06 18:52 [PULL 0/2] tcg patch queue Richard Henderson
@ 2020-07-06 18:52 ` Richard Henderson
2020-07-06 18:52 ` [PULL 1/2] tcg/ppc: Sanitize immediate shifts Richard Henderson
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Richard Henderson @ 2020-07-06 18:52 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
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(-)
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)
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PULL 1/2] tcg/ppc: Sanitize immediate shifts
2020-07-06 18:52 [PULL 0/2] tcg patch queue Richard Henderson
2020-07-06 18:52 ` [PATCH] target/arm: Fix temp double-free in sve ldr/str Richard Henderson
@ 2020-07-06 18:52 ` Richard Henderson
2020-07-06 18:52 ` [PULL 2/2] tcg: Fix do_nonatomic_op_* vs signed operations Richard Henderson
2020-07-08 15:33 ` [PULL 0/2] tcg patch queue Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Richard Henderson @ 2020-07-06 18:52 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Catherine A. Frederick
From: "Catherine A. Frederick" <chocola@animebitch.es>
Sanitize shift constants so that shift operations with
large constants don't generate invalid instructions.
Signed-off-by: Catherine A. Frederick <chocola@animebitch.es>
Message-Id: <20200607211100.22858-1-agrecascino123@gmail.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
tcg/ppc/tcg-target.inc.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index 7da67086c6..c8d1e765d9 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -2610,21 +2610,24 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
case INDEX_op_shl_i32:
if (const_args[2]) {
- tcg_out_shli32(s, args[0], args[1], args[2]);
+ /* Limit immediate shift count lest we create an illegal insn. */
+ tcg_out_shli32(s, args[0], args[1], args[2] & 31);
} else {
tcg_out32(s, SLW | SAB(args[1], args[0], args[2]));
}
break;
case INDEX_op_shr_i32:
if (const_args[2]) {
- tcg_out_shri32(s, args[0], args[1], args[2]);
+ /* Limit immediate shift count lest we create an illegal insn. */
+ tcg_out_shri32(s, args[0], args[1], args[2] & 31);
} else {
tcg_out32(s, SRW | SAB(args[1], args[0], args[2]));
}
break;
case INDEX_op_sar_i32:
if (const_args[2]) {
- tcg_out32(s, SRAWI | RS(args[1]) | RA(args[0]) | SH(args[2]));
+ /* Limit immediate shift count lest we create an illegal insn. */
+ tcg_out32(s, SRAWI | RS(args[1]) | RA(args[0]) | SH(args[2] & 31));
} else {
tcg_out32(s, SRAW | SAB(args[1], args[0], args[2]));
}
@@ -2696,14 +2699,16 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
case INDEX_op_shl_i64:
if (const_args[2]) {
- tcg_out_shli64(s, args[0], args[1], args[2]);
+ /* Limit immediate shift count lest we create an illegal insn. */
+ tcg_out_shli64(s, args[0], args[1], args[2] & 63);
} else {
tcg_out32(s, SLD | SAB(args[1], args[0], args[2]));
}
break;
case INDEX_op_shr_i64:
if (const_args[2]) {
- tcg_out_shri64(s, args[0], args[1], args[2]);
+ /* Limit immediate shift count lest we create an illegal insn. */
+ tcg_out_shri64(s, args[0], args[1], args[2] & 63);
} else {
tcg_out32(s, SRD | SAB(args[1], args[0], args[2]));
}
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PULL 2/2] tcg: Fix do_nonatomic_op_* vs signed operations
2020-07-06 18:52 [PULL 0/2] tcg patch queue Richard Henderson
2020-07-06 18:52 ` [PATCH] target/arm: Fix temp double-free in sve ldr/str Richard Henderson
2020-07-06 18:52 ` [PULL 1/2] tcg/ppc: Sanitize immediate shifts Richard Henderson
@ 2020-07-06 18:52 ` Richard Henderson
2020-07-08 15:33 ` [PULL 0/2] tcg patch queue Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Richard Henderson @ 2020-07-06 18:52 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, LIU Zhiwei
The smin/smax/umin/umax operations require the operands to be
properly sign extended. Do not drop the MO_SIGN bit from the
load, and additionally extend the val input.
Reviewed-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
Reported-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20200701165646.1901320-1-richard.henderson@linaro.org>
---
tcg/tcg-op.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index e60b74fb82..4b8a473fad 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -3189,8 +3189,9 @@ static void do_nonatomic_op_i32(TCGv_i32 ret, TCGv addr, TCGv_i32 val,
memop = tcg_canonicalize_memop(memop, 0, 0);
- tcg_gen_qemu_ld_i32(t1, addr, idx, memop & ~MO_SIGN);
- gen(t2, t1, val);
+ tcg_gen_qemu_ld_i32(t1, addr, idx, memop);
+ tcg_gen_ext_i32(t2, val, memop);
+ gen(t2, t1, t2);
tcg_gen_qemu_st_i32(t2, addr, idx, memop);
tcg_gen_ext_i32(ret, (new_val ? t2 : t1), memop);
@@ -3232,8 +3233,9 @@ static void do_nonatomic_op_i64(TCGv_i64 ret, TCGv addr, TCGv_i64 val,
memop = tcg_canonicalize_memop(memop, 1, 0);
- tcg_gen_qemu_ld_i64(t1, addr, idx, memop & ~MO_SIGN);
- gen(t2, t1, val);
+ tcg_gen_qemu_ld_i64(t1, addr, idx, memop);
+ tcg_gen_ext_i64(t2, val, memop);
+ gen(t2, t1, t2);
tcg_gen_qemu_st_i64(t2, addr, idx, memop);
tcg_gen_ext_i64(ret, (new_val ? t2 : t1), memop);
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PULL 0/2] tcg patch queue
2020-07-06 18:52 [PULL 0/2] tcg patch queue Richard Henderson
` (2 preceding siblings ...)
2020-07-06 18:52 ` [PULL 2/2] tcg: Fix do_nonatomic_op_* vs signed operations Richard Henderson
@ 2020-07-08 15:33 ` Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2020-07-08 15:33 UTC (permalink / raw)
To: Richard Henderson; +Cc: QEMU Developers
On Mon, 6 Jul 2020 at 19:52, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The following changes since commit eb6490f544388dd24c0d054a96dd304bc7284450:
>
> Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200703' into staging (2020-07-04 16:08:41 +0100)
>
> are available in the Git repository at:
>
> https://github.com/rth7680/qemu.git tags/pull-tcg-20200706
>
> for you to fetch changes up to 852f933e482518797f7785a2e017a215b88df815:
>
> tcg: Fix do_nonatomic_op_* vs signed operations (2020-07-06 10:58:19 -0700)
>
> ----------------------------------------------------------------
> Fix for ppc shifts
> Fix for non-parallel atomic ops
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread