On 04/10/2017 02:13 PM, Peter Maydell wrote:
> We now test for "are we singlestepping" in several places and
> it's not a trivial check because we need to care about both
> architectural singlestep and QEMU gdbstub singlestep. We're
> also about to add another place that needs to make this check,
> so pull the condition out into a function.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> target/arm/translate.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index 87fd702..f28c4ca 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -309,6 +309,17 @@ static void gen_singlestep_exception(DisasContext *s)
> }
> }
>
> +static inline bool is_singlestepping(DisasContext *s)
> +{
> + /* Return true if we are singlestepping either because of
> + * architectural singlestep or QEMU gdbstub singlestep. This does
> + * not include the command line '-singlestep' mode which is rather
> + * misnamed as it only means "one instruction per TB" and doesn't
> + * affect the code we generate.
> + */
This comment is very welcome :)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> + return s->singlestep_enabled || s->ss_active;
> +}
> +
> static void gen_smul_dual(TCGv_i32 a, TCGv_i32 b)
> {
> TCGv_i32 tmp1 = tcg_temp_new_i32();
> @@ -4104,7 +4115,7 @@ static inline void gen_goto_tb(DisasContext *s, int n, target_ulong dest)
>
> static inline void gen_jmp (DisasContext *s, uint32_t dest)
> {
> - if (unlikely(s->singlestep_enabled || s->ss_active)) {
> + if (unlikely(is_singlestepping(s))) {
> /* An indirect jump so that we still trigger the debug exception. */
> if (s->thumb)
> dest |= 1;
> @@ -11970,9 +11981,8 @@ void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb)
> ((dc->pc >= next_page_start - 3) && insn_crosses_page(env, dc));
>
> } while (!dc->is_jmp && !tcg_op_buf_full() &&
> - !cs->singlestep_enabled &&
> + !is_singlestepping(dc) &&
> !singlestep &&
> - !dc->ss_active &&
> !end_of_page &&
> num_insns < max_insns);
>
> @@ -11989,7 +11999,7 @@ void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb)
> instruction was a conditional branch or trap, and the PC has
> already been written. */
> gen_set_condexec(dc);
> - if (unlikely(cs->singlestep_enabled || dc->ss_active)) {
> + if (unlikely(is_singlestepping(dc))) {
> /* Unconditional and "condition passed" instruction codepath. */
> switch (dc->is_jmp) {
> case DISAS_SWI:
> @@ -12067,7 +12077,7 @@ void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb)
> /* "Condition failed" instruction codepath for the branch/trap insn */
> gen_set_label(dc->condlabel);
> gen_set_condexec(dc);
> - if (unlikely(cs->singlestep_enabled || dc->ss_active)) {
> + if (unlikely(is_singlestepping(dc))) {
> gen_set_pc_im(dc, dc->pc);
> gen_singlestep_exception(dc);
> } else {
>