Re: [Qemu-trivial] [Qemu-devel] [PATCH v2] add macro file for coccinelle

[Markus Armbruster]

Paolo Bonzini <address@hidden> writes:

> Coccinelle chokes on some idioms from compiler.h and queue.h.
> Extract those in a macro file, to be used with "--macro-file
> scripts/cocci-macro-file.h".
>
> Signed-off-by: Paolo Bonzini <address@hidden>

I tested this as follows.  Coccinelle can report on its parsing
difficulties:

$ spatch --parse-c `git-ls-files | grep '\.c$'

Requires a generous ulimit -s, and produces tons of output.  The
interesting part here is the summary at the end:

    NB total files = 1923; perfect = 1377; pbs = 545; timeout = 0; =========> 
71%
    nb good = 964472,  nb passed = 14824 =========> 1.48% passed
    nb good = 964472,  nb bad = 23759 =========> 97.63% good or passed


Repeat with --macro-file scripts/cocci-macro-file.h.

    NB total files = 1923; perfect = 1463; pbs = 459; timeout = 0; =========> 
76%
    nb good = 910559,  nb passed = 15352 =========> 1.53% passed
    nb good = 910559,  nb bad = 77610 =========> 92.27% good or passed

The first line shows clear improvement: 86 more files are now deemed
"perfect".  I'm not sure how to read the remaining two lines.

Also interesting are the "maybe 10 most problematic tokens".  Before:

    -----------------------------------------------------------------------
    maybe 10 most problematic tokens
    -----------------------------------------------------------------------
    glue: present in 620 parsing errors
    example: 

           #define OP_32_64(x)                             \
                   glue(glue(case INDEX_op_, x), _i32):    \
                   glue(glue(case INDEX_op_, x), _i64)
    NULL: present in 507 parsing errors
    example: 
               vs->dev = (QVirtioDevice *)dev;
               g_assert(dev != NULL);
               g_assert_cmphex(vs->dev->device_type, ==, 
QVIRTIO_SCSI_DEVICE_ID);

    VMStateField: present in 502 parsing errors
    example: 
               .version_id = 1,
               .minimum_version_id = 1,
               .fields = (VMStateField[]) {
                   VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
    fields: present in 502 parsing errors
    example: 
               .version_id = 1,
               .minimum_version_id = 1,
               .fields = (VMStateField[]) {
                   VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
    minimum_version_id: present in 433 parsing errors
    example: 
               .version_id = 1,
               .minimum_version_id = 1,
               .fields = (VMStateField[]) {
                   VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
    uint32_t: present in 414 parsing errors
    example: 
                                       uint32_t *bg, uint32_t *fg,              
   \
                                       VncPalette **palette) {                  
   \
                   uint##bpp##_t *data;                                         
   \
                   uint##bpp##_t c0, c1, ci;                                    
   \
    env: present in 400 parsing errors
    example: 
               int v = float32_compare_quiet(a, b, &env->fp_status);
               set_br(env, v != float_relation_greater, br);
           }
    version_id: present in 308 parsing errors
    example: 
               .version_id = 1,
               .minimum_version_id = 1,
               .fields = (VMStateField[]) {
                   VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
    x: present in 221 parsing errors
    example: 
               Int128 a = expand(x);
               Int128 r = int128_rshift(a, n);
               g_assert_cmpuint(r.lo, ==, l);
               g_assert_cmpuint(r.hi, ==, h);
    a: present in 197 parsing errors
    example: 
               vmstate_load_state(loading, &vmstate_skipping, &obj, 2);
               g_assert(!qemu_file_get_error(loading));
               g_assert_cmpint(obj.a, ==, 10);
               g_assert_cmpint(obj.b, ==, 20);

After:

    -----------------------------------------------------------------------
    maybe 10 most problematic tokens
    -----------------------------------------------------------------------
    NULL: present in 506 parsing errors
    example: 
               vs->dev = (QVirtioDevice *)dev;
               g_assert(dev != NULL);
               g_assert_cmphex(vs->dev->device_type, ==, 
QVIRTIO_SCSI_DEVICE_ID);

    VMStateField: present in 502 parsing errors
    example: 
               .version_id = 1,
               .minimum_version_id = 1,
               .fields = (VMStateField[]) {
                   VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
    fields: present in 502 parsing errors
    example: 
               .version_id = 1,
               .minimum_version_id = 1,
               .fields = (VMStateField[]) {
                   VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
    minimum_version_id: present in 433 parsing errors
    example: 
               .version_id = 1,
               .minimum_version_id = 1,
               .fields = (VMStateField[]) {
                   VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
    env: present in 376 parsing errors
    example: 
               int v = float32_compare_quiet(a, b, &env->fp_status);
               set_br(env, v != float_relation_greater, br);
           }
    version_id: present in 308 parsing errors
    example: 
               .version_id = 1,
               .minimum_version_id = 1,
               .fields = (VMStateField[]) {
                   VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
    x: present in 221 parsing errors
    example: 
               Int128 a = expand(x);
               Int128 r = int128_rshift(a, n);
               g_assert_cmpuint(r.lo, ==, l);
               g_assert_cmpuint(r.hi, ==, h);
    a: present in 197 parsing errors
    example: 
               vmstate_load_state(loading, &vmstate_skipping, &obj, 2);
               g_assert(!qemu_file_get_error(loading));
               g_assert_cmpint(obj.a, ==, 10);
               g_assert_cmpint(obj.b, ==, 20);
    b: present in 164 parsing errors
    example: 
                   uint64_t rl, rh;
                   muls64(&rl, &rh, test_s_data[i].a, test_s_data[i].b);
                   g_assert_cmpuint(rl, ==, test_s_data[i].rl);
                   g_assert_cmpint(rh, ==, test_s_data[i].rh);
    name: present in 150 parsing errors
    example: 
               g_assert(list != NULL);
               g_assert(QTAILQ_EMPTY(&list->head));
               g_assert_cmpstr(list->name, ==, "opts_list_01");

    -----------------------------------------------------------------------

Clearly visible is the effect of your cocci-macro-file.h's glue() .

> ---
>  scripts/cocci-macro-file.h | 119 
> +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 119 insertions(+)
>  create mode 100644 scripts/cocci-macro-file.h
>
> diff --git a/scripts/cocci-macro-file.h b/scripts/cocci-macro-file.h
> new file mode 100644
> index 0000000..eceb4be
> --- /dev/null
> +++ b/scripts/cocci-macro-file.h
> @@ -0,0 +1,119 @@
> +/* Macro file for Coccinelle
> + *
> + * Copyright (C) 2015 Red Hat, Inc.
> + *
> + * Authors:
> + *  Paolo Bonzini <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or, at 
> your
> + * option, any later version.  See the COPYING file in the top-level 
> directory.
> + */
> +
> +/* Coccinelle only does limited parsing of headers, and chokes on some idioms
> + * defined in compiler.h and queue.h.  Macros that Coccinelle must know about
> + * in order to parse .c files must be in a separate macro file---which is
> + * exactly what you're staring at now.
> + *
> + * To use this file, add the "--macro-file scripts/cocci-macro-file.h" to the
> + * Coccinelle command line.
> + */
> +
> +/* From qemu/compiler.h */
> +#define QEMU_GNUC_PREREQ(maj, min) 1
> +#define QEMU_NORETURN __attribute__ ((__noreturn__))
> +#define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
> +#define QEMU_SENTINEL __attribute__((sentinel))
> +#define QEMU_ARTIFICIAL __attribute__((always_inline, artificial))
> +#define QEMU_PACKED __attribute__((gcc_struct, packed))
> +
> +#define cat(x,y) x ## y
> +#define cat2(x,y) cat(x,y)

Why not reuse glue()?

> +#define QEMU_BUILD_BUG_ON(x) \
> + typedef char cat2(qemu_build_bug_on__,__LINE__)[(x)?-1:1]
> __attribute__((unused));
> +
> +#define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m)))
> +
> +#define xglue(x, y) x ## y
> +#define glue(x, y) xglue(x, y)
> +#define stringify(s) tostring(s)
> +#define tostring(s)  #s
> +
> +#define typeof_field(type, field) typeof(((type *)0)->field)
> +#define type_check(t1,t2) ((t1*)0 - (t2*)0)
> +
[...]

Preferably with cat2() replaced by glue():
Reviewed-by: Markus Armbruster <address@hidden>