Patchwork [2/2] tcg: add TB sanity checking

login
register
mail settings
Submitter Max Filippov
Date Sept. 21, 2012, 12:18 a.m.
Message ID <1348186688-29410-3-git-send-email-jcmvbkbc@gmail.com>
Download mbox | patch
Permalink /patch/185527/
State New
Headers show

Comments

Max Filippov - Sept. 21, 2012, 12:18 a.m.
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(-)
Michael Tokarev - Sept. 21, 2012, 8:37 a.m.
On 21.09.2012 04:18, Max Filippov wrote:

> diff --git a/tcg/tcg.c b/tcg/tcg.c

> +#ifdef CONFIG_DEBUG_TCG
> +static void tcg_sanity_check(TCGContext *s)

#ifndef CONFIG_DEBUG_TCG
#define tcg_sanity_check(s) /*empty*/
#else
static void tcg_sanity_check(TCGContext *s)

> +{
[]
> +}
> +#endif

> @@ -2082,6 +2147,10 @@ static inline int tcg_gen_code_common(TCGContext *s, uint8_t *gen_code_buf,
> +#ifdef CONFIG_DEBUG_TCG
> +    tcg_sanity_check(s);
> +#endif

And here we can drop the #ifdef.  FWIW.

/mjt
Aurelien Jarno - Sept. 22, 2012, 2:55 p.m.
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.

Patch

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;