Re: [Qemu-devel] [PATCH v2][RESEND] target-arm: cleanup internal resource leaks
On Thu, Oct 22, 2009 at 1:17 PM, juha.riihim...@nokia.com wrote:
From: Juha Riihimäki juha.riihim...@nokia.com
Revised patch for getting rid of tcg temporary variable leaks in
target-arm/translate.c. This version also includes the leak patch for
gen_set_cpsr macro, now converted as a static inline function, which I
sent earlier as a separate patch on top of this patch.
Signed-off-by: Juha Riihimäki juha.riihim...@nokia.com
---
target-arm/translate.c | 116 ---
1 files changed, 89 insertions(+), 27 deletions(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index e56082b..813f661 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -184,7 +184,12 @@ static void store_reg(DisasContext *s, int reg, TCGv var)
#define gen_uxtb16(var) gen_helper_uxtb16(var, var)
-#define gen_set_cpsr(var, mask) gen_helper_cpsr_write(var,
tcg_const_i32(mask))
+static inline void gen_set_cpsr(TCGv var, uint32_t mask)
+{
+ TCGv tmp_mask = tcg_const_i32(mask);
+ gen_helper_cpsr_write(var, tmp_mask);
+ tcg_temp_free_i32(tmp_mask);
+}
/* Set NZCV flags from the high 4 bits of var. */
#define gen_set_nzcv(var) gen_set_cpsr(var, CPSR_NZCV)
@@ -287,6 +292,7 @@ static TCGv_i64 gen_mulu_i64_i32(TCGv a, TCGv b)
tcg_gen_extu_i32_i64(tmp2, b);
dead_tmp(b);
tcg_gen_mul_i64(tmp1, tmp1, tmp2);
+ tcg_temp_free_i64(tmp2);
return tmp1;
}
@@ -300,6 +306,7 @@ static TCGv_i64 gen_muls_i64_i32(TCGv a, TCGv b)
tcg_gen_ext_i32_i64(tmp2, b);
dead_tmp(b);
tcg_gen_mul_i64(tmp1, tmp1, tmp2);
+ tcg_temp_free_i64(tmp2);
return tmp1;
}
@@ -312,9 +319,11 @@ static void gen_mull(TCGv a, TCGv b)
tcg_gen_extu_i32_i64(tmp1, a);
tcg_gen_extu_i32_i64(tmp2, b);
tcg_gen_mul_i64(tmp1, tmp1, tmp2);
+ tcg_temp_free_i64(tmp2);
tcg_gen_trunc_i64_i32(a, tmp1);
tcg_gen_shri_i64(tmp1, tmp1, 32);
tcg_gen_trunc_i64_i32(b, tmp1);
+ tcg_temp_free_i64(tmp1);
}
/* Signed 32x32-64 multiply. */
@@ -326,9 +335,11 @@ static void gen_imull(TCGv a, TCGv b)
tcg_gen_ext_i32_i64(tmp1, a);
tcg_gen_ext_i32_i64(tmp2, b);
tcg_gen_mul_i64(tmp1, tmp1, tmp2);
+ tcg_temp_free_i64(tmp2);
tcg_gen_trunc_i64_i32(a, tmp1);
tcg_gen_shri_i64(tmp1, tmp1, 32);
tcg_gen_trunc_i64_i32(b, tmp1);
+ tcg_temp_free_i64(tmp1);
}
/* Swap low and high halfwords. */
@@ -542,11 +553,13 @@ static void gen_arm_parallel_addsub(int op1, int op2,
TCGv a, TCGv b)
tmp = tcg_temp_new_ptr();
tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
PAS_OP(s)
+ tcg_temp_free_ptr(tmp);
break;
case 5:
tmp = tcg_temp_new_ptr();
tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
PAS_OP(u)
+ tcg_temp_free_ptr(tmp);
break;
#undef gen_pas_helper
#define gen_pas_helper(name) glue(gen_helper_,name)(a, a, b)
@@ -587,11 +600,13 @@ static void gen_thumb2_parallel_addsub(int op1, int
op2, TCGv a, TCGv b)
tmp = tcg_temp_new_ptr();
tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
PAS_OP(s)
+ tcg_temp_free_ptr(tmp);
break;
case 4:
tmp = tcg_temp_new_ptr();
tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
PAS_OP(u)
+ tcg_temp_free_ptr(tmp);
break;
#undef gen_pas_helper
#define gen_pas_helper(name) glue(gen_helper_,name)(a, a, b)
@@ -995,10 +1010,12 @@ static inline void gen_vfp_tosiz(int dp)
#define VFP_GEN_FIX(name) \
static inline void gen_vfp_##name(int dp, int shift) \
{ \
+ TCGv tmp_shift = tcg_const_i32(shift); \
if (dp) \
- gen_helper_vfp_##name##d(cpu_F0d, cpu_F0d, tcg_const_i32(shift),
cpu_env);\
+ gen_helper_vfp_##name##d(cpu_F0d, cpu_F0d, tmp_shift, cpu_env);\
else \
- gen_helper_vfp_##name##s(cpu_F0s, cpu_F0s, tcg_const_i32(shift),
cpu_env);\
+ gen_helper_vfp_##name##s(cpu_F0s, cpu_F0s, tmp_shift, cpu_env);\
+ tcg_temp_free_i32(tmp_shift); \
}
VFP_GEN_FIX(tosh)
VFP_GEN_FIX(tosl)
@@ -2399,7 +2416,7 @@ static int disas_dsp_insn(CPUState *env, DisasContext
*s, uint32_t insn)
instruction is not defined. */
static int disas_cp_insn(CPUState *env, DisasContext *s, uint32_t insn)
{
- TCGv tmp;
+ TCGv tmp, tmp2;
uint32_t rd = (insn 12) 0xf;
uint32_t cp = (insn 8) 0xf;
if (IS_USER(s)) {
@@ -2411,14 +2428,18 @@ static int disas_cp_insn(CPUState *env, DisasContext
*s, uint32_t insn)
return 1;
gen_set_pc_im(s-pc);
tmp = new_tmp();
- gen_helper_get_cp(tmp, cpu_env, tcg_const_i32(insn));
+ tmp2 = tcg_const_i32(insn);
+ gen_helper_get_cp(tmp, cpu_env, tmp2);
+ tcg_temp_free(tmp2);
store_reg(s, rd, tmp);
} else {
if (!env-cp[cp].cp_write)
return 1;
Re: [Qemu-devel] [PATCH v2][RESEND] target-arm: cleanup internal resource leaks
On Oct 26, 2009, at 12:46, ext Laurent Desnogues wrote:
@@ -5511,8 +5539,9 @@ static int disas_neon_data_insn(CPUState *
env, DisasContext *s, uint32_t insn)
tcg_gen_movi_i32(tmp, 0);
}
tmp2 = neon_load_reg(rm, 0);
-gen_helper_neon_tbl(tmp2, tmp2, tmp, tcg_const_i32
(rn),
-tcg_const_i32(n));
+tmp4 = tcg_const_i32(rn);
+tmp5 = tcg_const_i32(n);
+gen_helper_neon_tbl(tmp2, tmp2, tmp, tmp4, tmp5);
dead_tmp(tmp);
if (insn (1 6)) {
tmp = neon_load_reg(rd, 1);
@@ -5521,8 +5550,9 @@ static int disas_neon_data_insn(CPUState *
env, DisasContext *s, uint32_t insn)
tcg_gen_movi_i32(tmp, 0);
}
tmp3 = neon_load_reg(rm, 1);
-gen_helper_neon_tbl(tmp3, tmp3, tmp, tcg_const_i32
(rn),
-tcg_const_i32(n));
+gen_helper_neon_tbl(tmp3, tmp3, tmp, tmp4, tmp5);
+dead_tmp(tmp5);
+dead_tmp(tmp4);
I missed this mistake when I acked the patch: you're using
dead_tmp instead of tcg_temp_free... One more reason
to drop dead_tmp :-)
Indeed. I'll fix it, didn't notice it either because I'm using other
new_xxx/dead_xxx helpers myself to catch resource leaks. I should work
around that to get more similar code and eliminate this kind of issues.
The bug will cause false resource leak reports, the actual freeing of
the temporary is done correctly. Since this patch seems to have
already been applied, I'll just provide a patch that fixes this
specific error in the mainline instead of reposting a fixed version of
the original patch.
Regards,
Juha
Re: [Qemu-devel] [PATCH v2][RESEND] target-arm: cleanup internal resource leaks
On Thu, Oct 22, 2009 at 2:17 PM, juha.riihim...@nokia.com wrote:
From: Juha Riihimäki juha.riihim...@nokia.com
Revised patch for getting rid of tcg temporary variable leaks in
target-arm/translate.c. This version also includes the leak patch for
gen_set_cpsr macro, now converted as a static inline function, which I
sent earlier as a separate patch on top of this patch.
Signed-off-by: Juha Riihimäki juha.riihim...@nokia.com
Acked-by: Laurent Desnogues laurent.desnog...@gmail.com
---
target-arm/translate.c | 116 ---
1 files changed, 89 insertions(+), 27 deletions(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index e56082b..813f661 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -184,7 +184,12 @@ static void store_reg(DisasContext *s, int reg, TCGv var)
#define gen_uxtb16(var) gen_helper_uxtb16(var, var)
-#define gen_set_cpsr(var, mask) gen_helper_cpsr_write(var,
tcg_const_i32(mask))
+static inline void gen_set_cpsr(TCGv var, uint32_t mask)
+{
+ TCGv tmp_mask = tcg_const_i32(mask);
+ gen_helper_cpsr_write(var, tmp_mask);
+ tcg_temp_free_i32(tmp_mask);
+}
/* Set NZCV flags from the high 4 bits of var. */
#define gen_set_nzcv(var) gen_set_cpsr(var, CPSR_NZCV)
@@ -287,6 +292,7 @@ static TCGv_i64 gen_mulu_i64_i32(TCGv a, TCGv b)
tcg_gen_extu_i32_i64(tmp2, b);
dead_tmp(b);
tcg_gen_mul_i64(tmp1, tmp1, tmp2);
+ tcg_temp_free_i64(tmp2);
return tmp1;
}
@@ -300,6 +306,7 @@ static TCGv_i64 gen_muls_i64_i32(TCGv a, TCGv b)
tcg_gen_ext_i32_i64(tmp2, b);
dead_tmp(b);
tcg_gen_mul_i64(tmp1, tmp1, tmp2);
+ tcg_temp_free_i64(tmp2);
return tmp1;
}
@@ -312,9 +319,11 @@ static void gen_mull(TCGv a, TCGv b)
tcg_gen_extu_i32_i64(tmp1, a);
tcg_gen_extu_i32_i64(tmp2, b);
tcg_gen_mul_i64(tmp1, tmp1, tmp2);
+ tcg_temp_free_i64(tmp2);
tcg_gen_trunc_i64_i32(a, tmp1);
tcg_gen_shri_i64(tmp1, tmp1, 32);
tcg_gen_trunc_i64_i32(b, tmp1);
+ tcg_temp_free_i64(tmp1);
}
/* Signed 32x32-64 multiply. */
@@ -326,9 +335,11 @@ static void gen_imull(TCGv a, TCGv b)
tcg_gen_ext_i32_i64(tmp1, a);
tcg_gen_ext_i32_i64(tmp2, b);
tcg_gen_mul_i64(tmp1, tmp1, tmp2);
+ tcg_temp_free_i64(tmp2);
tcg_gen_trunc_i64_i32(a, tmp1);
tcg_gen_shri_i64(tmp1, tmp1, 32);
tcg_gen_trunc_i64_i32(b, tmp1);
+ tcg_temp_free_i64(tmp1);
}
/* Swap low and high halfwords. */
@@ -542,11 +553,13 @@ static void gen_arm_parallel_addsub(int op1, int op2,
TCGv a, TCGv b)
tmp = tcg_temp_new_ptr();
tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
PAS_OP(s)
+ tcg_temp_free_ptr(tmp);
break;
case 5:
tmp = tcg_temp_new_ptr();
tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
PAS_OP(u)
+ tcg_temp_free_ptr(tmp);
break;
#undef gen_pas_helper
#define gen_pas_helper(name) glue(gen_helper_,name)(a, a, b)
@@ -587,11 +600,13 @@ static void gen_thumb2_parallel_addsub(int op1, int
op2, TCGv a, TCGv b)
tmp = tcg_temp_new_ptr();
tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
PAS_OP(s)
+ tcg_temp_free_ptr(tmp);
break;
case 4:
tmp = tcg_temp_new_ptr();
tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
PAS_OP(u)
+ tcg_temp_free_ptr(tmp);
break;
#undef gen_pas_helper
#define gen_pas_helper(name) glue(gen_helper_,name)(a, a, b)
@@ -995,10 +1010,12 @@ static inline void gen_vfp_tosiz(int dp)
#define VFP_GEN_FIX(name) \
static inline void gen_vfp_##name(int dp, int shift) \
{ \
+ TCGv tmp_shift = tcg_const_i32(shift); \
if (dp) \
- gen_helper_vfp_##name##d(cpu_F0d, cpu_F0d, tcg_const_i32(shift),
cpu_env);\
+ gen_helper_vfp_##name##d(cpu_F0d, cpu_F0d, tmp_shift, cpu_env);\
else \
- gen_helper_vfp_##name##s(cpu_F0s, cpu_F0s, tcg_const_i32(shift),
cpu_env);\
+ gen_helper_vfp_##name##s(cpu_F0s, cpu_F0s, tmp_shift, cpu_env);\
+ tcg_temp_free_i32(tmp_shift); \
}
VFP_GEN_FIX(tosh)
VFP_GEN_FIX(tosl)
@@ -2399,7 +2416,7 @@ static int disas_dsp_insn(CPUState *env, DisasContext
*s, uint32_t insn)
instruction is not defined. */
static int disas_cp_insn(CPUState *env, DisasContext *s, uint32_t insn)
{
- TCGv tmp;
+ TCGv tmp, tmp2;
uint32_t rd = (insn 12) 0xf;
uint32_t cp = (insn 8) 0xf;
if (IS_USER(s)) {
@@ -2411,14 +2428,18 @@ static int disas_cp_insn(CPUState *env, DisasContext
*s, uint32_t insn)
return 1;
gen_set_pc_im(s-pc);
tmp = new_tmp();
- gen_helper_get_cp(tmp, cpu_env, tcg_const_i32(insn));
+ tmp2 = tcg_const_i32(insn);
+ gen_helper_get_cp(tmp, cpu_env, tmp2);
+ tcg_temp_free(tmp2);
store_reg(s, rd, tmp);
} else {
if
