Re: [Qemu-devel] [PATCH 3/7] trace: Add event "guest_inst_info_before"

[Richard Henderson <richard.henderson@linaro.org>]

On 09/10/2017 09:23 AM, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
>  accel/tcg/translator.c |   18 ++++++++++++++++++
>  trace-events           |    9 +++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
> index 287d27b4f7..6598931171 100644
> --- a/accel/tcg/translator.c
> +++ b/accel/tcg/translator.c
> @@ -70,6 +70,8 @@ void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
>  
>      while (true) {
>          target_ulong pc_insn = db->pc_next;
> +        TCGv_i32 insn_size_tcg = 0;
> +        int insn_size_opcode_idx;

Initializing a TCGv_i32 is wrong.
And surely insn_size_opcode is surely uninitialized?

> +        if (TRACE_GUEST_INST_INFO_BEFORE_EXEC_ENABLED) {
> +            insn_size_tcg = tcg_temp_new_i32();
> +            insn_size_opcode_idx = tcg_op_buf_count();
> +            tcg_gen_movi_i32(insn_size_tcg, 0xdeadbeef);
> +
> +            trace_guest_inst_info_before_tcg(
> +                cpu, tcg_ctx.tcg_env, pc_insn, insn_size_tcg);
> +
> +            tcg_temp_free_i32(insn_size_tcg);

There's no reason you can't declare insn_size_tcg right here and avoid the
incorrect initialization above.

Is there a reason to have both "guest_insn" and "guest_insn_info"?


r~