Re: [Qemu-devel] [PATCH 2/2] tcg: add TB sanity checking

[Aurelien Jarno <aurelien@aurel32.net>]

On Fri, Sep 21, 2012 at 04:18:08AM +0400, Max Filippov wrote:
> Do a sanity checking pass on the intermediate code.
> Check that goto_tb indices are either 0 or 1 and used at most once per
> TB.
> 
> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
> ---
>  tcg/tcg.c |   69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 69 insertions(+), 0 deletions(-)
> 
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index b8a1bec..cdd1975 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -1454,6 +1454,71 @@ static void check_regs(TCGContext *s)
>  }
>  #endif
>  
> +#ifdef CONFIG_DEBUG_TCG
> +static void tcg_sanity_check(TCGContext *s)
> +{
> +    const uint16_t *opc_ptr;
> +    const TCGArg *args;
> +    TCGArg arg;
> +    TCGOpcode c;
> +    int nb_oargs, nb_iargs, nb_cargs, error_count = 0;
> +    const TCGOpDef *def;
> +    unsigned goto_tb_slots[2] = {0};
> +
> +    opc_ptr = gen_opc_buf;
> +    args = gen_opparam_buf;
> +    while (opc_ptr < gen_opc_ptr) {
> +        c = *opc_ptr++;
> +        def = &tcg_op_defs[c];
> +        if (c == INDEX_op_call) {
> +            TCGArg arg;
> +
> +            /* variable number of arguments */
> +            arg = *args++;
> +            nb_oargs = arg >> 16;
> +            nb_iargs = arg & 0xffff;
> +            nb_cargs = def->nb_cargs;
> +        } else {
> +            if (c == INDEX_op_nopn) {
> +                /* variable number of arguments */
> +                nb_cargs = *args;
> +                nb_oargs = 0;
> +                nb_iargs = 0;
> +            } else {
> +                nb_oargs = def->nb_oargs;
> +                nb_iargs = def->nb_iargs;
> +                nb_cargs = def->nb_cargs;
> +            }
> +        }
> +
> +        switch (c) {
> +        case INDEX_op_goto_tb:
> +            arg = args[0];
> +            if (arg != 0 && arg != 1) {
> +                qemu_log("TB ERROR: wrong goto_tb slot index: %"TCG_PRIlx"\n",
> +                        arg);
> +                ++error_count;
> +            } else {
> +                ++goto_tb_slots[arg];
> +                if (goto_tb_slots[arg] > 1) {
> +                    qemu_log("TB ERROR: multiple goto_tb(%"TCG_PRIlx")\n", arg);
> +                    ++error_count;
> +                }
> +            }
> +            break;
> +
> +        default:
> +            break;
> +        }
> +
> +        args += nb_iargs + nb_oargs + nb_cargs;
> +    }
> +    if (error_count) {
> +        qemu_log("\n");
> +    }
> +}
> +#endif
> +
>  static void temp_allocate_frame(TCGContext *s, int temp)
>  {
>      TCGTemp *ts;
> @@ -2082,6 +2147,10 @@ static inline int tcg_gen_code_common(TCGContext *s, uint8_t *gen_code_buf,
>      }
>  #endif
>  
> +#ifdef CONFIG_DEBUG_TCG
> +    tcg_sanity_check(s);
> +#endif
> +
>      tcg_reg_alloc_start(s);
>  
>      s->code_buf = gen_code_buf;

I think this is better address in the patch from Richard Henderson.


-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net