On 10/10/2015 12:34 AM, Sergey Fedorov wrote:
@@ -2936,6 +2927,10 @@ static inline void 
gen_intermediate_code_internal(AlphaCPU *cpu,
          tcg_gen_insn_start(ctx.pc);
          num_insns++;

+        if (unlikely(cpu_breakpoint_test(cs, ctx.pc, BP_ANY))) {
+            gen_excp(&ctx, EXCP_DEBUG, 0);
+            break;
+        }

Actually, control logic has changed here. The old code used a break
statement to exit from QTAILQ_FOREACH loop and continue with instruction
translation thus translating at least one instruction. The break
statement in the new code makes exit from the translation loop itself,
effectively producing zero-length TB which won't get invalidated when
clearing the breakpoint. Seems like we should remove the break statement
here and in similar cases below, right?

Why do you believe that a zero-length TB won't be cleared?
The TB still has a start address, which is contained within
a given page, which is invalidated.

Some target-*/translate.c takes care to advance the PC, but I believe that this is only required when the breakpoint instruction *itself* could span a page boundary. I.e. the TB needs to be marked to span two pages. This situation can never be true for many RISC targets.

We did discuss this exact situation during review of the patch set, though it's probably true that there are outstanding errors in some translators.


r~

Reply via email to