diff mbox

[v2] target-*: Advance pc after recognizing a breakpoint

Message ID 1445637617-18300-1-git-send-email-rth@twiddle.net
State New
Headers show

Commit Message

Richard Henderson Oct. 23, 2015, 10 p.m. UTC
Some targets already had this within their logic, but make sure
it's present for all targets.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
Version 2 updates the language as discussed in the followup in v1.

Peter, in that followup you mentioned that we ought to just use +1
for all targets.  I was about to do that when I noticed the comment
in the arm32 port about it being better to use the correct insn size
in order to avoid warnings from the disassembler.  So I left my good
estimates as-is from v1.


r~
---
 target-alpha/translate.c      | 5 +++++
 target-arm/translate-a64.c    | 7 +++++--
 target-arm/translate.c        | 7 +++++--
 target-cris/translate.c       | 5 +++++
 target-i386/translate.c       | 5 +++++
 target-lm32/translate.c       | 5 +++++
 target-m68k/translate.c       | 5 +++++
 target-microblaze/translate.c | 5 +++++
 target-mips/translate.c       | 6 ++++--
 target-moxie/translate.c      | 5 +++++
 target-openrisc/translate.c   | 5 +++++
 target-ppc/translate.c        | 5 +++++
 target-s390x/translate.c      | 5 +++++
 target-sh4/translate.c        | 5 +++++
 target-sparc/translate.c      | 2 +-
 target-unicore32/translate.c  | 8 +++++---
 target-xtensa/translate.c     | 5 +++++
 17 files changed, 80 insertions(+), 10 deletions(-)

Comments

Peter Maydell Oct. 26, 2015, 11:47 a.m. UTC | #1
On 23 October 2015 at 23:00, Richard Henderson <rth@twiddle.net> wrote:
> Some targets already had this within their logic, but make sure
> it's present for all targets.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> Version 2 updates the language as discussed in the followup in v1.
>
> Peter, in that followup you mentioned that we ought to just use +1
> for all targets.  I was about to do that when I noticed the comment
> in the arm32 port about it being better to use the correct insn size
> in order to avoid warnings from the disassembler.  So I left my good
> estimates as-is from v1.

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

though really we should shut up the disassembler some other way
somehow..."pick the insn length" doesn't work for variable-insn-length
CPUs.

thanks
-- PMM
diff mbox

Patch

diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index f936d1b..87950c6 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -2917,6 +2917,11 @@  void gen_intermediate_code(CPUAlphaState *env, struct TranslationBlock *tb)
 
         if (unlikely(cpu_breakpoint_test(cs, ctx.pc, BP_ANY))) {
             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)) {
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 19f9d8d..83b8376 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -11096,8 +11096,11 @@  void gen_intermediate_code_a64(ARMCPU *cpu, TranslationBlock *tb)
                         dc->is_jmp = DISAS_UPDATE;
                     } else {
                         gen_exception_internal_insn(dc, 0, EXCP_DEBUG);
-                        /* Advance PC so that clearing the breakpoint will
-                           invalidate this TB.  */
+                        /* 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;
                     }
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 9f1d740..ee631af 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -11348,8 +11348,11 @@  void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb)
                         dc->is_jmp = DISAS_UPDATE;
                     } else {
                         gen_exception_internal_insn(dc, 0, EXCP_DEBUG);
-                        /* Advance PC so that clearing the breakpoint will
-                           invalidate this TB.  */
+                        /* 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;
diff --git a/target-cris/translate.c b/target-cris/translate.c
index 964845c..2d710cc 100644
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -3166,6 +3166,11 @@  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;
         }
 
diff --git a/target-i386/translate.c b/target-i386/translate.c
index ef10e68..17f6601 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7942,6 +7942,11 @@  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)) {
diff --git a/target-lm32/translate.c b/target-lm32/translate.c
index c61ad0f..fa5b0b9 100644
--- a/target-lm32/translate.c
+++ b/target-lm32/translate.c
@@ -1078,6 +1078,11 @@  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;
         }
 
diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index 5995cce..41ae2c6 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -3004,6 +3004,11 @@  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;
         }
 
diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c
index a9c5010..154b9d6 100644
--- a/target-microblaze/translate.c
+++ b/target-microblaze/translate.c
@@ -1693,6 +1693,11 @@  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;
         }
 
diff --git a/target-mips/translate.c b/target-mips/translate.c
index 897839c..a10bfa3 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -19594,8 +19594,10 @@  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);
-            /* Include the breakpoint location or the tb won't
-             * be flushed when it must be.  */
+            /* 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;
         }
diff --git a/target-moxie/translate.c b/target-moxie/translate.c
index f84841e..6dedcb7 100644
--- a/target-moxie/translate.c
+++ b/target-moxie/translate.c
@@ -848,6 +848,11 @@  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;
         }
 
diff --git a/target-openrisc/translate.c b/target-openrisc/translate.c
index b66fde1..606490a 100644
--- a/target-openrisc/translate.c
+++ b/target-openrisc/translate.c
@@ -1665,6 +1665,11 @@  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;
         }
 
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index c2bc1a7..1d759c8 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -11488,6 +11488,11 @@  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;
         }
 
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 05d51fe..c79a2cb 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -5360,6 +5360,11 @@  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;
         }
 
diff --git a/target-sh4/translate.c b/target-sh4/translate.c
index f764bc2..7bc6216 100644
--- a/target-sh4/translate.c
+++ b/target-sh4/translate.c
@@ -1855,6 +1855,11 @@  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;
         }
 
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index b59742a..41a3319 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -5247,6 +5247,7 @@  void gen_intermediate_code(CPUSPARCState * env, TranslationBlock * tb)
             tcg_gen_insn_start(dc->pc, dc->npc);
         }
         num_insns++;
+        last_pc = dc->pc;
 
         if (unlikely(cpu_breakpoint_test(cs, dc->pc, BP_ANY))) {
             if (dc->pc != pc_start) {
@@ -5262,7 +5263,6 @@  void gen_intermediate_code(CPUSPARCState * env, TranslationBlock * tb)
             gen_io_start();
         }
 
-        last_pc = dc->pc;
         insn = cpu_ldl_code(env, dc->pc);
 
         disas_sparc_insn(dc, insn);
diff --git a/target-unicore32/translate.c b/target-unicore32/translate.c
index 48f89fb..d2f92f0 100644
--- a/target-unicore32/translate.c
+++ b/target-unicore32/translate.c
@@ -1917,9 +1917,11 @@  void gen_intermediate_code(CPUUniCore32State *env, TranslationBlock *tb)
             gen_set_pc_im(dc->pc);
             gen_exception(EXCP_DEBUG);
             dc->is_jmp = DISAS_JUMP;
-            /* Advance PC so that clearing the breakpoint will
-               invalidate this TB.  */
-            dc->pc += 2; /* FIXME */
+            /* 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;
         }
 
diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
index aa0c527..06b0163 100644
--- a/target-xtensa/translate.c
+++ b/target-xtensa/translate.c
@@ -3088,6 +3088,11 @@  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;
         }