From c384da2f47f4f6cd9c941cccf0a031b743654cde Mon Sep 17 00:00:00 2001 From: "Emilio G. Cota" Date: Tue, 13 Mar 2018 14:51:53 -0400 Subject: [PATCH] tcg: convert tb->cflags reads to tb_cflags(tb) Convert all existing readers of tb->cflags to tb_cflags, so that we use atomic_read and therefore avoid undefined behaviour in C11. Note that the remaining setters/getters of the field are protected by tb_lock, and therefore do not need conversion. Luckily all readers access the field via 'tb->cflags' (so no foo.cflags, bar->cflags in the code base), which makes the conversion easily scriptable: FILES=$(git grep 'tb->cflags' target include/exec/gen-icount.h \ accel/tcg/translator.c | cut -f1 -d':' | sort | uniq) perl -pi -e 's/([^.>])tb->cflags/$1tb_cflags(tb)/g' $FILES perl -pi -e 's/([a-z->.]*)(->|\.)tb->cflags/tb_cflags($1$2tb)/g' $FILES Then manually fixed the few errors that checkpatch reported. Compile-tested for all targets. Backports commit c5a49c63fa26e8825ad101dfe86339ae4c216539 from qemu --- qemu/accel/tcg/translator.c | 4 +- qemu/include/exec/gen-icount.h | 4 +- qemu/target/arm/translate-a64.c | 7 +-- qemu/target/arm/translate.c | 6 +-- qemu/target/i386/translate.c | 16 +++--- qemu/target/m68k/translate.c | 2 +- qemu/target/mips/translate.c | 91 ++++++++++++++++++++------------- qemu/target/sparc/translate.c | 20 +++++--- 8 files changed, 89 insertions(+), 61 deletions(-) diff --git a/qemu/accel/tcg/translator.c b/qemu/accel/tcg/translator.c index fb3eed07..078deda6 100644 --- a/qemu/accel/tcg/translator.c +++ b/qemu/accel/tcg/translator.c @@ -47,7 +47,7 @@ void translator_loop(const TranslatorOps *ops, DisasContextBase *db, db->uc->block_full = false; /* Instruction counting */ - max_insns = db->tb->cflags & CF_COUNT_MASK; + max_insns = tb_cflags(db->tb) & CF_COUNT_MASK; if (max_insns == 0) { max_insns = CF_COUNT_MASK; } @@ -121,7 +121,7 @@ void translator_loop(const TranslatorOps *ops, DisasContextBase *db, update db->pc_next and db->is_jmp to indicate what should be done next -- either exiting this loop or locate the start of the next instruction. */ - if (db->num_insns == max_insns && (db->tb->cflags & CF_LAST_IO)) { + if (db->num_insns == max_insns && (tb_cflags(db->tb) & CF_LAST_IO)) { /* Accept I/O on the last instruction. */ //gen_io_start(); ops->translate_insn(db, cpu); diff --git a/qemu/include/exec/gen-icount.h b/qemu/include/exec/gen-icount.h index 076dc70e..5bc68cef 100644 --- a/qemu/include/exec/gen-icount.h +++ b/qemu/include/exec/gen-icount.h @@ -22,7 +22,7 @@ static inline void gen_tb_start(TCGContext *tcg_ctx, TranslationBlock *tb) tcg_temp_free_i32(tcg_ctx, flag); #if 0 - if (!(tb->cflags & CF_USE_ICOUNT)) + if (!(tb_cflags(tb) & CF_USE_ICOUNT)) return; } @@ -53,7 +53,7 @@ static inline void gen_tb_end(TCGContext *tcg_ctx, TranslationBlock *tb, int num tcg_gen_exit_tb(tcg_ctx, (uintptr_t)tb + TB_EXIT_REQUESTED); #if 0 - if (tb->cflags & CF_USE_ICOUNT) { + if (tb_cflags(tb) & CF_USE_ICOUNT) { /* Update the num_insn immediate parameter now that we know * the actual insn count. */ tcg_set_insn_param(tcg_ctx->icount_start_insn, 1, num_insns); diff --git a/qemu/target/arm/translate-a64.c b/qemu/target/arm/translate-a64.c index 6099046a..ce94c5d2 100644 --- a/qemu/target/arm/translate-a64.c +++ b/qemu/target/arm/translate-a64.c @@ -374,7 +374,8 @@ static inline bool use_goto_tb(DisasContext *s, int n, uint64_t dest) /* No direct tb linking with singlestep (either QEMU's or the ARM * debug architecture kind) or deterministic io */ - if (s->base.singlestep_enabled || s->ss_active || (s->base.tb->cflags & CF_LAST_IO)) { + if (s->base.singlestep_enabled || s->ss_active || + (tb_cflags(s->base.tb) & CF_LAST_IO)) { return false; } @@ -1756,7 +1757,7 @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, // Unicorn: if'd out #if 0 - if ((s->base.tb->cflags & CF_USE_ICOUNT) && (ri->type & ARM_CP_IO)) { + if ((tb_cflags(s->base.tb) & CF_USE_ICOUNT) && (ri->type & ARM_CP_IO)) { gen_io_start(); } #endif @@ -1788,7 +1789,7 @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, } } - if ((s->base.tb->cflags & CF_USE_ICOUNT) && (ri->type & ARM_CP_IO)) { + if ((tb_cflags(s->base.tb) & CF_USE_ICOUNT) && (ri->type & ARM_CP_IO)) { /* I/O operations must end the TB here (whether read or write) */ // Unicorn: commented out //gen_io_end(); diff --git a/qemu/target/arm/translate.c b/qemu/target/arm/translate.c index 11f349e1..45615843 100644 --- a/qemu/target/arm/translate.c +++ b/qemu/target/arm/translate.c @@ -8083,7 +8083,7 @@ static int disas_coproc_insn(DisasContext *s, uint32_t insn) // Unicorn: if'd out #if 0 - if ((s->base.tb->cflags & CF_USE_ICOUNT) && (ri->type & ARM_CP_IO)) { + if ((tb_cflags(s->base.tb) & CF_USE_ICOUNT) && (ri->type & ARM_CP_IO)) { gen_io_start(); } #endif @@ -8175,7 +8175,7 @@ static int disas_coproc_insn(DisasContext *s, uint32_t insn) } } - if ((s->base.tb->cflags & CF_USE_ICOUNT) && (ri->type & ARM_CP_IO)) { + if ((tb_cflags(s->base.tb) & CF_USE_ICOUNT) && (ri->type & ARM_CP_IO)) { /* I/O operations must end the TB here (whether read or write) */ // Unicorn: commented out //gen_io_end(); @@ -12772,7 +12772,7 @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) DisasContext *dc = container_of(dcbase, DisasContext, base); TCGContext *tcg_ctx = cpu->uc->tcg_ctx; - if (dc->base.tb->cflags & CF_LAST_IO && dc->condjmp) { + if (tb_cflags(dc->base.tb) & CF_LAST_IO && dc->condjmp) { /* FIXME: This can theoretically happen with self-modifying code. */ cpu_abort(cpu, "IO on conditional branch instruction"); } diff --git a/qemu/target/i386/translate.c b/qemu/target/i386/translate.c index 860255ec..57ceb9d0 100644 --- a/qemu/target/i386/translate.c +++ b/qemu/target/i386/translate.c @@ -8232,12 +8232,12 @@ case 0x101: } gen_update_cc_op(s); gen_jmp_im(s, pc_start - s->cs_base); - if (s->base.tb->cflags & CF_USE_ICOUNT) { + if (tb_cflags(s->base.tb) & CF_USE_ICOUNT) { // Unicorn: commented out //gen_io_start(); } gen_helper_rdtscp(tcg_ctx, cpu_env); - if (s->base.tb->cflags & CF_USE_ICOUNT) { + if (tb_cflags(s->base.tb) & CF_USE_ICOUNT) { // Unicorn: commented out //gen_io_end(); gen_jmp(s, s->pc - s->cs_base); @@ -8605,7 +8605,7 @@ case 0x101: if (b & 2) { // Unicorn: if'd out #if 0 - if (s->base.tb->cflags & CF_USE_ICOUNT) { + if (tb_cflags(s->base.tb) & CF_USE_ICOUNT) { gen_io_start(); } #endif @@ -8615,7 +8615,7 @@ case 0x101: // Unicorn: if'd out #if 0 - if (s->base.tb->cflags & CF_USE_ICOUNT) { + if (tb_cflags(s->base.tb) & CF_USE_ICOUNT) { gen_io_end(); } #endif @@ -8624,14 +8624,14 @@ case 0x101: } else { // Unicorn: if'd out #if 0 - if (s->base.tb->cflags & CF_USE_ICOUNT) { + if (tb_cflags(s->base.tb) & CF_USE_ICOUNT) { gen_io_start(); } #endif gen_helper_read_crN(tcg_ctx, cpu_T0, cpu_env, tcg_const_i32(tcg_ctx, reg)); gen_op_mov_reg_v(tcg_ctx, ot, rm, cpu_T0); #if 0 - if (s->base.tb->cflags & CF_USE_ICOUNT) { + if (tb_cflags(s->base.tb) & CF_USE_ICOUNT) { gen_io_end(); } #endif @@ -9190,7 +9190,7 @@ static int i386_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cpu, record/replay modes and there will always be an additional step for ecx=0 when icount is enabled. */ - dc->repz_opt = !dc->jmp_opt && !(dc->base.tb->cflags & CF_USE_ICOUNT); + dc->repz_opt = !dc->jmp_opt && !(tb_cflags(dc->base.tb) & CF_USE_ICOUNT); #if 0 /* check addseg logic */ if (!dc->addseg && (dc->vm86 || !dc->pe || !dc->code32)) @@ -9263,7 +9263,7 @@ static void i386_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) the flag and abort the translation to give the irqs a chance to happen */ dc->base.is_jmp = DISAS_TOO_MANY; - } else if ((dc->base.tb->cflags & CF_USE_ICOUNT) + } else if ((tb_cflags(dc->base.tb) & CF_USE_ICOUNT) && ((dc->base.pc_next & TARGET_PAGE_MASK) != ((dc->base.pc_next + TARGET_MAX_INSN_SIZE - 1) & TARGET_PAGE_MASK) diff --git a/qemu/target/m68k/translate.c b/qemu/target/m68k/translate.c index 106ace46..3f8af3cb 100644 --- a/qemu/target/m68k/translate.c +++ b/qemu/target/m68k/translate.c @@ -6262,7 +6262,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb) dc->done_mac = 0; dc->writeback_mask = 0; num_insns = 0; - max_insns = tb->cflags & CF_COUNT_MASK; + max_insns = tb_cflags(tb) & CF_COUNT_MASK; if (max_insns == 0) { max_insns = CF_COUNT_MASK; } diff --git a/qemu/target/mips/translate.c b/qemu/target/mips/translate.c index 7d2c4311..e5cc4817 100644 --- a/qemu/target/mips/translate.c +++ b/qemu/target/mips/translate.c @@ -5402,13 +5402,18 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel) switch (sel) { case 0: /* Mark as an IO operation because we read the time. */ - //if (ctx->tb->cflags & CF_USE_ICOUNT) { - // gen_io_start(); - //} + // Unicorn: if'd out + #if 0 + if (tb_cflags(ctx->tb) & CF_USE_ICOUNT) { + gen_io_start(); + } + #endif gen_helper_mfc0_count(tcg_ctx, arg, tcg_ctx->cpu_env); - //if (ctx->tb->cflags & CF_USE_ICOUNT) { - // gen_io_end(); - //} + #if 0 + if (tb_cflags(ctx->tb) & CF_USE_ICOUNT) { + gen_io_end(); + } + #endif /* Break the TB to be able to take timer interrupts immediately after reading count. BS_STOP isn't sufficient, we need to ensure we break completely out of translated code. */ @@ -5811,9 +5816,12 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel) if (sel != 0) check_insn(ctx, ISA_MIPS32); - //if (ctx->tb->cflags & CF_USE_ICOUNT) { - // gen_io_start(); - //} + // Unicorn: if'd out +#if 0 + if (tb_cflags(ctx->tb) & CF_USE_ICOUNT) { + gen_io_start(); + } +#endif switch (reg) { case 0: @@ -6477,14 +6485,15 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel) } (void)rn; /* avoid a compiler warning */ LOG_DISAS("mtc0 %s (reg %d sel %d)\n", rn, reg, sel); + /* For simplicity assume that all writes can cause interrupts. */ - //if (ctx->tb->cflags & CF_USE_ICOUNT) { - // gen_io_end(); - // /* BS_STOP isn't sufficient, we need to ensure we break out of - // * translated code to check for pending interrupts. */ - // gen_save_pc(ctx, ctx->pc + 4); - // ctx->bstate = BS_EXCP; - //} + if (tb_cflags(ctx->tb) & CF_USE_ICOUNT) { + //gen_io_end(); + /* BS_STOP isn't sufficient, we need to ensure we break out of + * translated code to check for pending interrupts. */ + gen_save_pc(ctx, ctx->pc + 4); + ctx->bstate = BS_EXCP; + } return; cp0_unimplemented: @@ -6757,13 +6766,17 @@ static void gen_dmfc0(DisasContext *ctx, TCGv arg, int reg, int sel) switch (sel) { case 0: /* Mark as an IO operation because we read the time. */ - //if (ctx->tb->cflags & CF_USE_ICOUNT) { - // gen_io_start(); - //} + #if 0 + if (tb_cflags(ctx->tb) & CF_USE_ICOUNT) { + gen_io_start(); + } + #endif gen_helper_mfc0_count(tcg_ctx, arg, tcg_ctx->cpu_env); - //if (ctx->tb->cflags & CF_USE_ICOUNT) { - // gen_io_end(); - //} + #if 0 + if (tb_cflags(ctx->tb) & CF_USE_ICOUNT) { + gen_io_end(); + } + #endif /* Break the TB to be able to take timer interrupts immediately after reading count. BS_STOP isn't sufficient, we need to ensure we break completely out of translated code. */ @@ -7152,9 +7165,12 @@ static void gen_dmtc0(DisasContext *ctx, TCGv arg, int reg, int sel) if (sel != 0) check_insn(ctx, ISA_MIPS64); - //if (ctx->tb->cflags & CF_USE_ICOUNT) { - // gen_io_start(); - //} + // Unicorn: if'd out +#if 0 + if (tb_cflags(ctx->tb) & CF_USE_ICOUNT) { + gen_io_start(); + } +#endif switch (reg) { case 0: @@ -10860,13 +10876,13 @@ static void gen_rdhwr(DisasContext *ctx, int rt, int rd, int sel) case 2: // Unicorn: if'd out #if 0 - if (ctx->tb->cflags & CF_USE_ICOUNT) { + if (tb_cflags(ctx->tb) & CF_USE_ICOUNT) { gen_io_start(); } #endif gen_helper_rdhwr_cc(tcg_ctx, t0, tcg_ctx->cpu_env); #if 0 - if (ctx->tb->cflags & CF_USE_ICOUNT) { + if (tb_cflags(ctx->tb) & CF_USE_ICOUNT) { gen_io_end(); } #endif @@ -20424,7 +20440,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb) MO_UNALN : MO_ALIGN; num_insns = 0; - max_insns = tb->cflags & CF_COUNT_MASK; + max_insns = tb_cflags(tb) & CF_COUNT_MASK; if (max_insns == 0) { max_insns = CF_COUNT_MASK; } @@ -20474,10 +20490,12 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb) goto done_generating; } - // Unicorn: Commented out - //if (num_insns == max_insns && (tb->cflags & CF_LAST_IO)) { - // gen_io_start(); - //} + // Unicorn: If'd out +#if 0 + if (num_insns == max_insns && (tb_cflags(tb) & CF_LAST_IO)) { + gen_io_start(); + } +#endif // Unicorn: end address tells us to stop emulation if (ctx.pc == ctx.uc->addr_end) { @@ -20572,9 +20590,12 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb) block_full = true; } - //if (tb->cflags & CF_LAST_IO) { - // gen_io_end(); - //} + // Unicorn: commented out +#if 0 + if (tb_cflags(tb) & CF_LAST_IO) { + gen_io_end(); + } +#endif if (cs->singlestep_enabled && ctx.bstate != BS_BRANCH) { save_cpu_state(&ctx, ctx.bstate != BS_EXCP); gen_helper_raise_exception_debug(tcg_ctx, tcg_ctx->cpu_env); diff --git a/qemu/target/sparc/translate.c b/qemu/target/sparc/translate.c index d3f3b946..ac5c1a33 100644 --- a/qemu/target/sparc/translate.c +++ b/qemu/target/sparc/translate.c @@ -5963,7 +5963,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock * tb) goto done_generating; } - max_insns = tb->cflags & CF_COUNT_MASK; + max_insns = tb_cflags(tb) & CF_COUNT_MASK; if (max_insns == 0) { max_insns = CF_COUNT_MASK; } @@ -6013,9 +6013,12 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock * tb) goto exit_gen_loop; } - //if (num_insns == max_insns && (tb->cflags & CF_LAST_IO)) { - // gen_io_start(); - //} + // Unicorn: if'd out +#if 0 + if (num_insns == max_insns && (tb_cflags(tb) & CF_LAST_IO)) { + gen_io_start(); + } +#endif // Unicorn: end address tells us to stop emulation if (dc->pc == dc->uc->addr_end) { @@ -6052,9 +6055,12 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock * tb) block_full = true; exit_gen_loop: - //if (tb->cflags & CF_LAST_IO) { - // gen_io_end(); - //} + // Unicorn: if'd out +#if 0 + if (tb_cflags(tb) & CF_LAST_IO) { + gen_io_end(); + } +#endif if (!dc->is_br) { if (dc->pc != DYNAMIC_PC && (dc->npc != DYNAMIC_PC && dc->npc != JUMP_PC)) {