[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-arm] [PATCH] target-*: Get rid of "PC advancement" trick
|
From: |
Sergey Fedorov |
|
Subject: |
[Qemu-arm] [PATCH] target-*: Get rid of "PC advancement" trick |
|
Date: |
Thu, 10 Dec 2015 21:47:24 +0300 |
The "PC advancement" trick was used just after recognizing that a
breakpoint exception was going to be generated. This trick has had two
points:
1. Guarantee that tb->size isn't zero: there are many places where it's
expected to be non-zero. In fact, that is even stated in the comment
for this field.
2. Try to satisfy disassembler's check for instruction length. To this
end, PC advancement was done for estimated instruction length, but
actually, didn't work properly in variable-instruction-length cases.
Substitute this trick with checking for TB size at the end of
translation. If we get an empty TB then just set tb->size to 1 and skip
disassembling. Setting tb->size to 1 is enough to get correct behaviour,
whereas an empty TB doesn't obviously need to be disassembled.
Suggested-by: Peter Maydell <address@hidden>
Signed-off-by: Sergey Fedorov <address@hidden>
---
target-alpha/translate.c | 15 ++++++++++-----
target-arm/translate-a64.c | 16 ++++++++++------
target-arm/translate.c | 18 ++++++++++--------
target-cris/translate.c | 15 ++++++++++-----
target-i386/translate.c | 15 ++++++++++-----
target-lm32/translate.c | 15 ++++++++++-----
target-m68k/translate.c | 15 ++++++++++-----
target-microblaze/translate.c | 15 ++++++++++-----
target-mips/translate.c | 15 ++++++++++-----
target-moxie/translate.c | 15 ++++++++++-----
target-openrisc/translate.c | 15 ++++++++++-----
target-ppc/translate.c | 15 ++++++++++-----
target-s390x/translate.c | 15 ++++++++++-----
target-sh4/translate.c | 15 ++++++++++-----
target-unicore32/translate.c | 15 ++++++++++-----
target-xtensa/translate.c | 15 ++++++++++-----
16 files changed, 160 insertions(+), 84 deletions(-)
diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index 9909c70..d27f504 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -2917,11 +2917,6 @@ void gen_intermediate_code(CPUAlphaState *env, struct
TranslationBlock *tb)
if (unlikely(cpu_breakpoint_test(cs, ctx.pc, BP_ANY))) {
ret = gen_excp(&ctx, EXCP_DEBUG, 0);
- /* The address covered by the breakpoint must be included in
- [tb->pc, tb->pc + tb->size) in order to for it to be
- properly cleared -- thus we increment the PC here so that
- the logic setting tb->size below does the right thing. */
- ctx.pc += 4;
break;
}
if (num_insns == max_insns && (tb->cflags & CF_LAST_IO)) {
@@ -2983,6 +2978,16 @@ void gen_intermediate_code(CPUAlphaState *env, struct
TranslationBlock *tb)
gen_tb_end(tb, num_insns);
+ if (ctx.pc == pc_start) {
+ /* We have got an empty TB but there are many places where tb->size is
+ * expected it to be non-zero. Just setting it to 1 is enough to get
+ * correct behaviour.
+ */
+ tb->size = 1;
+ tb->icount = 0;
+ return;
+ }
+
tb->size = ctx.pc - pc_start;
tb->icount = num_insns;
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 14e8131..11719f8 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -11097,12 +11097,6 @@ void gen_intermediate_code_a64(ARMCPU *cpu,
TranslationBlock *tb)
dc->is_jmp = DISAS_UPDATE;
} else {
gen_exception_internal_insn(dc, 0, EXCP_DEBUG);
- /* The address covered by the breakpoint must be
- included in [tb->pc, tb->pc + tb->size) in order
- to for it to be properly cleared -- thus we
- increment the PC here so that the logic setting
- tb->size below does the right thing. */
- dc->pc += 4;
goto done_generating;
}
break;
@@ -11213,6 +11207,16 @@ void gen_intermediate_code_a64(ARMCPU *cpu,
TranslationBlock *tb)
done_generating:
gen_tb_end(tb, num_insns);
+ if (dc->pc == pc_start) {
+ /* We have got an empty TB but there are many places where tb->size is
+ * expected it to be non-zero. Just setting it to 1 is enough to get
+ * correct behaviour.
+ */
+ tb->size = 1;
+ tb->icount = 0;
+ return;
+ }
+
#ifdef DEBUG_DISAS
if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
qemu_log("----------------\n");
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 5d22879..07d5d02 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -11381,14 +11381,6 @@ void gen_intermediate_code(CPUARMState *env,
TranslationBlock *tb)
dc->is_jmp = DISAS_UPDATE;
} else {
gen_exception_internal_insn(dc, 0, EXCP_DEBUG);
- /* The address covered by the breakpoint must be
- included in [tb->pc, tb->pc + tb->size) in order
- to for it to be properly cleared -- thus we
- increment the PC here so that the logic setting
- tb->size below does the right thing. */
- /* TODO: Advance PC by correct instruction length to
- * avoid disassembler error messages */
- dc->pc += 2;
goto done_generating;
}
break;
@@ -11583,6 +11575,16 @@ void gen_intermediate_code(CPUARMState *env,
TranslationBlock *tb)
done_generating:
gen_tb_end(tb, num_insns);
+ if (dc->pc == pc_start) {
+ /* We have got an empty TB but there are many places where tb->size is
+ * expected it to be non-zero. Just setting it to 1 is enough to get
+ * correct behaviour.
+ */
+ tb->size = 1;
+ tb->icount = 0;
+ return;
+ }
+
#ifdef DEBUG_DISAS
if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
qemu_log("----------------\n");
diff --git a/target-cris/translate.c b/target-cris/translate.c
index 2d710cc..b3274a3 100644
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -3166,11 +3166,6 @@ void gen_intermediate_code(CPUCRISState *env, struct
TranslationBlock *tb)
tcg_gen_movi_tl(env_pc, dc->pc);
t_gen_raise_exception(EXCP_DEBUG);
dc->is_jmp = DISAS_UPDATE;
- /* The address covered by the breakpoint must be included in
- [tb->pc, tb->pc + tb->size) in order to for it to be
- properly cleared -- thus we increment the PC here so that
- the logic setting tb->size below does the right thing. */
- dc->pc += 2;
break;
}
@@ -3293,6 +3288,16 @@ void gen_intermediate_code(CPUCRISState *env, struct
TranslationBlock *tb)
}
gen_tb_end(tb, num_insns);
+ if (dc->pc == pc_start) {
+ /* We have got an empty TB but there are many places where tb->size is
+ * expected it to be non-zero. Just setting it to 1 is enough to get
+ * correct behaviour.
+ */
+ tb->size = 1;
+ tb->icount = 0;
+ return;
+ }
+
tb->size = dc->pc - pc_start;
tb->icount = num_insns;
diff --git a/target-i386/translate.c b/target-i386/translate.c
index a3dd167..b6f4855 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7987,11 +7987,6 @@ void gen_intermediate_code(CPUX86State *env,
TranslationBlock *tb)
tb->flags & HF_RF_MASK
? BP_GDB : BP_ANY))) {
gen_debug(dc, pc_ptr - dc->cs_base);
- /* The address covered by the breakpoint must be included in
- [tb->pc, tb->pc + tb->size) in order to for it to be
- properly cleared -- thus we increment the PC here so that
- the logic setting tb->size below does the right thing. */
- pc_ptr += 1;
goto done_generating;
}
if (num_insns == max_insns && (tb->cflags & CF_LAST_IO)) {
@@ -8046,6 +8041,16 @@ void gen_intermediate_code(CPUX86State *env,
TranslationBlock *tb)
done_generating:
gen_tb_end(tb, num_insns);
+ if (pc_ptr == pc_start) {
+ /* We have got an empty TB but there are many places where tb->size is
+ * expected it to be non-zero. Just setting it to 1 is enough to get
+ * correct behaviour.
+ */
+ tb->size = 1;
+ tb->icount = 0;
+ return;
+ }
+
#ifdef DEBUG_DISAS
if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
int disas_flags;
diff --git a/target-lm32/translate.c b/target-lm32/translate.c
index fa5b0b9..ef150bf 100644
--- a/target-lm32/translate.c
+++ b/target-lm32/translate.c
@@ -1078,11 +1078,6 @@ void gen_intermediate_code(CPULM32State *env, struct
TranslationBlock *tb)
tcg_gen_movi_tl(cpu_pc, dc->pc);
t_gen_raise_exception(dc, EXCP_DEBUG);
dc->is_jmp = DISAS_UPDATE;
- /* The address covered by the breakpoint must be included in
- [tb->pc, tb->pc + tb->size) in order to for it to be
- properly cleared -- thus we increment the PC here so that
- the logic setting tb->size below does the right thing. */
- dc->pc += 4;
break;
}
@@ -1131,6 +1126,16 @@ void gen_intermediate_code(CPULM32State *env, struct
TranslationBlock *tb)
gen_tb_end(tb, num_insns);
+ if (dc->pc == pc_start) {
+ /* We have got an empty TB but there are many places where tb->size is
+ * expected it to be non-zero. Just setting it to 1 is enough to get
+ * correct behaviour.
+ */
+ tb->size = 1;
+ tb->icount = 0;
+ return;
+ }
+
tb->size = dc->pc - pc_start;
tb->icount = num_insns;
diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index 41ae2c6..583a455 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -3004,11 +3004,6 @@ void gen_intermediate_code(CPUM68KState *env,
TranslationBlock *tb)
if (unlikely(cpu_breakpoint_test(cs, dc->pc, BP_ANY))) {
gen_exception(dc, dc->pc, EXCP_DEBUG);
dc->is_jmp = DISAS_JUMP;
- /* The address covered by the breakpoint must be included in
- [tb->pc, tb->pc + tb->size) in order to for it to be
- properly cleared -- thus we increment the PC here so that
- the logic setting tb->size below does the right thing. */
- dc->pc += 2;
break;
}
@@ -3053,6 +3048,16 @@ void gen_intermediate_code(CPUM68KState *env,
TranslationBlock *tb)
}
gen_tb_end(tb, num_insns);
+ if (dc->pc == pc_start) {
+ /* We have got an empty TB but there are many places where tb->size is
+ * expected it to be non-zero. Just setting it to 1 is enough to get
+ * correct behaviour.
+ */
+ tb->size = 1;
+ tb->icount = 0;
+ return;
+ }
+
#ifdef DEBUG_DISAS
if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
qemu_log("----------------\n");
diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c
index 154b9d6..4a4beb7 100644
--- a/target-microblaze/translate.c
+++ b/target-microblaze/translate.c
@@ -1693,11 +1693,6 @@ void gen_intermediate_code(CPUMBState *env, struct
TranslationBlock *tb)
if (unlikely(cpu_breakpoint_test(cs, dc->pc, BP_ANY))) {
t_gen_raise_exception(dc, EXCP_DEBUG);
dc->is_jmp = DISAS_UPDATE;
- /* The address covered by the breakpoint must be included in
- [tb->pc, tb->pc + tb->size) in order to for it to be
- properly cleared -- thus we increment the PC here so that
- the logic setting tb->size below does the right thing. */
- dc->pc += 4;
break;
}
@@ -1803,6 +1798,16 @@ void gen_intermediate_code(CPUMBState *env, struct
TranslationBlock *tb)
}
gen_tb_end(tb, num_insns);
+ if (dc->pc == pc_start) {
+ /* We have got an empty TB but there are many places where tb->size is
+ * expected it to be non-zero. Just setting it to 1 is enough to get
+ * correct behaviour.
+ */
+ tb->size = 1;
+ tb->icount = 0;
+ return;
+ }
+
tb->size = dc->pc - pc_start;
tb->icount = num_insns;
diff --git a/target-mips/translate.c b/target-mips/translate.c
index 5626647..488603b 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -19627,11 +19627,6 @@ void gen_intermediate_code(CPUMIPSState *env, struct
TranslationBlock *tb)
save_cpu_state(&ctx, 1);
ctx.bstate = BS_BRANCH;
gen_helper_raise_exception_debug(cpu_env);
- /* The address covered by the breakpoint must be included in
- [tb->pc, tb->pc + tb->size) in order to for it to be
- properly cleared -- thus we increment the PC here so that
- the logic setting tb->size below does the right thing. */
- ctx.pc += 4;
goto done_generating;
}
@@ -19722,6 +19717,16 @@ void gen_intermediate_code(CPUMIPSState *env, struct
TranslationBlock *tb)
done_generating:
gen_tb_end(tb, num_insns);
+ if (ctx.pc == pc_start) {
+ /* We have got an empty TB but there are many places where tb->size is
+ * expected it to be non-zero. Just setting it to 1 is enough to get
+ * correct behaviour.
+ */
+ tb->size = 1;
+ tb->icount = 0;
+ return;
+ }
+
tb->size = ctx.pc - pc_start;
tb->icount = num_insns;
diff --git a/target-moxie/translate.c b/target-moxie/translate.c
index 6dedcb7..e682735 100644
--- a/target-moxie/translate.c
+++ b/target-moxie/translate.c
@@ -848,11 +848,6 @@ void gen_intermediate_code(CPUMoxieState *env, struct
TranslationBlock *tb)
tcg_gen_movi_i32(cpu_pc, ctx.pc);
gen_helper_debug(cpu_env);
ctx.bstate = BS_EXCP;
- /* The address covered by the breakpoint must be included in
- [tb->pc, tb->pc + tb->size) in order to for it to be
- properly cleared -- thus we increment the PC here so that
- the logic setting tb->size below does the right thing. */
- ctx.pc += 2;
goto done_generating;
}
@@ -890,6 +885,16 @@ void gen_intermediate_code(CPUMoxieState *env, struct
TranslationBlock *tb)
done_generating:
gen_tb_end(tb, num_insns);
+ if (ctx.pc == pc_start) {
+ /* We have got an empty TB but there are many places where tb->size is
+ * expected it to be non-zero. Just setting it to 1 is enough to get
+ * correct behaviour.
+ */
+ tb->size = 1;
+ tb->icount = 0;
+ return;
+ }
+
tb->size = ctx.pc - pc_start;
tb->icount = num_insns;
}
diff --git a/target-openrisc/translate.c b/target-openrisc/translate.c
index 606490a..762e82e 100644
--- a/target-openrisc/translate.c
+++ b/target-openrisc/translate.c
@@ -1665,11 +1665,6 @@ void gen_intermediate_code(CPUOpenRISCState *env, struct
TranslationBlock *tb)
tcg_gen_movi_tl(cpu_pc, dc->pc);
gen_exception(dc, EXCP_DEBUG);
dc->is_jmp = DISAS_UPDATE;
- /* The address covered by the breakpoint must be included in
- [tb->pc, tb->pc + tb->size) in order to for it to be
- properly cleared -- thus we increment the PC here so that
- the logic setting tb->size below does the right thing. */
- dc->pc += 4;
break;
}
@@ -1736,6 +1731,16 @@ void gen_intermediate_code(CPUOpenRISCState *env, struct
TranslationBlock *tb)
gen_tb_end(tb, num_insns);
+ if (dc->pc == pc_start) {
+ /* We have got an empty TB but there are many places where tb->size is
+ * expected it to be non-zero. Just setting it to 1 is enough to get
+ * correct behaviour.
+ */
+ tb->size = 1;
+ tb->icount = 0;
+ return;
+ }
+
tb->size = dc->pc - pc_start;
tb->icount = num_insns;
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 41a7258..ba83e08 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -11488,11 +11488,6 @@ void gen_intermediate_code(CPUPPCState *env, struct
TranslationBlock *tb)
if (unlikely(cpu_breakpoint_test(cs, ctx.nip, BP_ANY))) {
gen_debug_exception(ctxp);
- /* The address covered by the breakpoint must be included in
- [tb->pc, tb->pc + tb->size) in order to for it to be
- properly cleared -- thus we increment the PC here so that
- the logic setting tb->size below does the right thing. */
- ctx.nip += 4;
break;
}
@@ -11589,6 +11584,16 @@ void gen_intermediate_code(CPUPPCState *env, struct
TranslationBlock *tb)
}
gen_tb_end(tb, num_insns);
+ if (ctx.nip == pc_start) {
+ /* We have got an empty TB but there are many places where tb->size is
+ * expected it to be non-zero. Just setting it to 1 is enough to get
+ * correct behaviour.
+ */
+ tb->size = 1;
+ tb->icount = 0;
+ return;
+ }
+
tb->size = ctx.nip - pc_start;
tb->icount = num_insns;
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index c79a2cb..09421ad 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -5360,11 +5360,6 @@ void gen_intermediate_code(CPUS390XState *env, struct
TranslationBlock *tb)
if (unlikely(cpu_breakpoint_test(cs, dc.pc, BP_ANY))) {
status = EXIT_PC_STALE;
do_debug = true;
- /* The address covered by the breakpoint must be included in
- [tb->pc, tb->pc + tb->size) in order to for it to be
- properly cleared -- thus we increment the PC here so that
- the logic setting tb->size below does the right thing. */
- dc.pc += 2;
break;
}
@@ -5417,6 +5412,16 @@ void gen_intermediate_code(CPUS390XState *env, struct
TranslationBlock *tb)
gen_tb_end(tb, num_insns);
+ if (dc.pc == pc_start) {
+ /* We have got an empty TB but there are many places where tb->size is
+ * expected it to be non-zero. Just setting it to 1 is enough to get
+ * correct behaviour.
+ */
+ tb->size = 1;
+ tb->icount = 0;
+ return;
+ }
+
tb->size = dc.pc - pc_start;
tb->icount = num_insns;
diff --git a/target-sh4/translate.c b/target-sh4/translate.c
index 7bc6216..ed2fa57 100644
--- a/target-sh4/translate.c
+++ b/target-sh4/translate.c
@@ -1855,11 +1855,6 @@ void gen_intermediate_code(CPUSH4State * env, struct
TranslationBlock *tb)
tcg_gen_movi_i32(cpu_pc, ctx.pc);
gen_helper_debug(cpu_env);
ctx.bstate = BS_BRANCH;
- /* The address covered by the breakpoint must be included in
- [tb->pc, tb->pc + tb->size) in order to for it to be
- properly cleared -- thus we increment the PC here so that
- the logic setting tb->size below does the right thing. */
- ctx.pc += 2;
break;
}
@@ -1908,6 +1903,16 @@ void gen_intermediate_code(CPUSH4State * env, struct
TranslationBlock *tb)
gen_tb_end(tb, num_insns);
+ if (ctx.pc == pc_start) {
+ /* We have got an empty TB but there are many places where tb->size is
+ * expected it to be non-zero. Just setting it to 1 is enough to get
+ * correct behaviour.
+ */
+ tb->size = 1;
+ tb->icount = 0;
+ return;
+ }
+
tb->size = ctx.pc - pc_start;
tb->icount = num_insns;
diff --git a/target-unicore32/translate.c b/target-unicore32/translate.c
index d2f92f0..b02783f 100644
--- a/target-unicore32/translate.c
+++ b/target-unicore32/translate.c
@@ -1917,11 +1917,6 @@ void gen_intermediate_code(CPUUniCore32State *env,
TranslationBlock *tb)
gen_set_pc_im(dc->pc);
gen_exception(EXCP_DEBUG);
dc->is_jmp = DISAS_JUMP;
- /* The address covered by the breakpoint must be included in
- [tb->pc, tb->pc + tb->size) in order to for it to be
- properly cleared -- thus we increment the PC here so that
- the logic setting tb->size below does the right thing. */
- dc->pc += 4;
goto done_generating;
}
@@ -2017,6 +2012,16 @@ void gen_intermediate_code(CPUUniCore32State *env,
TranslationBlock *tb)
done_generating:
gen_tb_end(tb, num_insns);
+ if (dc->pc == pc_start) {
+ /* We have got an empty TB but there are many places where tb->size is
+ * expected it to be non-zero. Just setting it to 1 is enough to get
+ * correct behaviour.
+ */
+ tb->size = 1;
+ tb->icount = 0;
+ return;
+ }
+
#ifdef DEBUG_DISAS
if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
qemu_log("----------------\n");
diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
index 06b0163..6e5f8ba 100644
--- a/target-xtensa/translate.c
+++ b/target-xtensa/translate.c
@@ -3088,11 +3088,6 @@ void gen_intermediate_code(CPUXtensaState *env,
TranslationBlock *tb)
tcg_gen_movi_i32(cpu_pc, dc.pc);
gen_exception(&dc, EXCP_DEBUG);
dc.is_jmp = DISAS_UPDATE;
- /* The address covered by the breakpoint must be included in
- [tb->pc, tb->pc + tb->size) in order to for it to be
- properly cleared -- thus we increment the PC here so that
- the logic setting tb->size below does the right thing. */
- dc.pc += 2;
break;
}
@@ -3146,6 +3141,16 @@ void gen_intermediate_code(CPUXtensaState *env,
TranslationBlock *tb)
}
gen_tb_end(tb, insn_count);
+ if (dc.pc == pc_start) {
+ /* We have got an empty TB but there are many places where tb->size is
+ * expected it to be non-zero. Just setting it to 1 is enough to get
+ * correct behaviour.
+ */
+ tb->size = 1;
+ tb->icount = 0;
+ return;
+ }
+
#ifdef DEBUG_DISAS
if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
qemu_log("----------------\n");
--
1.9.1
- [Qemu-arm] [PATCH] target-*: Get rid of "PC advancement" trick,
Sergey Fedorov <=