[Qemu-devel] [PATCH 0/1] Developer Conveniences - check for _Static_assert and use in QEMU_BUILD_BUG_ON

[Qemu-devel] [PATCH 0/1] Developer Conveniences - check for _Static_assert and use in QEMU_BUILD_BUG_ON

[Andreas Grapentin]

(as taken from here: http://wiki.qemu-project.org/Contribute/BiteSizedTasks)


I added a configure check for C11's _Static_assert, and based on the outcome,
QEMU_BUILD_BUG_ON will now produce slightly more readable results on failures.

Systems without C11 conforming compilers will still have the old messages, as
verified by intentionally breaking the configure check.

the following example output was generated by inverting the condition in
QEMU_BUILD_BUG_ON:

without _Static_assert:

> In file included from /home/andi/research/qemu-shared-memory/qemu/include/qemu/osdep.h:36:0,
>                  from /home/andi/research/qemu-shared-memory/qemu/qga/commands.c:13:
> /home/andi/research/qemu-shared-memory/qemu/qga/commands.c: In function ‘qmp_guest_exec_status’:
> /home/andi/research/qemu-shared-memory/qemu/include/qemu/compiler.h:89:12: error: negative width in bit-field ‘<anonymous>’
>      struct { \
>             ^
> /home/andi/research/qemu-shared-memory/qemu/include/qemu/compiler.h:96:38: note: in expansion of macro  QEMU_BUILD_BUG_ON_STRUCT’
>  #define QEMU_BUILD_BUG_ON(x) typedef QEMU_BUILD_BUG_ON_STRUCT(x) \
>                                       ^~~~~~~~~~~~~~~~~~~~~~~~
> /home/andi/research/qemu-shared-memory/qemu/include/qemu/atomic.h:146:5: note: in expansion of macro ‘QEMU_BUILD_BUG_ON’
>      QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *));   \
>      ^~~~~~~~~~~~~~~~~
> /home/andi/research/qemu-shared-memory/qemu/include/qemu/atomic.h:417:5: note: in expansion of macro ‘atomic_load_acquire’
>      atomic_load_acquire(ptr)
>      ^~~~~~~~~~~~~~~~~~~
> /home/andi/research/qemu-shared-memory/qemu/qga/commands.c:160:21: note: in expansion of macro ‘atomic_mb_read’
>      bool finished = atomic_mb_read(&gei->finished);
>                      ^~~~~~~~~~~~~~

with _Static_assert:

> In file included from /home/andi/research/qemu-shared-memory/qemu/include/qemu/osdep.h:36:0,
>                  from /home/andi/research/qemu-shared-memory/qemu/qga/commands.c:13:
> /home/andi/research/qemu-shared-memory/qemu/qga/commands.c: In function ‘qmp_guest_exec_status’:
> /home/andi/research/qemu-shared-memory/qemu/include/qemu/compiler.h:94:30: error: static assertion failed: "sizeof(*&gei->finished) > sizeof(void *)"
>  #define QEMU_BUILD_BUG_ON(x) _Static_assert((x), #x)
>                               ^
> /home/andi/research/qemu-shared-memory/qemu/include/qemu/atomic.h:146:5: note: in expansion of macro ‘QEMU_BUILD_BUG_ON’
>      QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *));   \
>      ^~~~~~~~~~~~~~~~~
> /home/andi/research/qemu-shared-memory/qemu/include/qemu/atomic.h:417:5: note: in expansion of macro ‘atomic_load_acquire’
>      atomic_load_acquire(ptr)
>      ^~~~~~~~~~~~~~~~~~~
> /home/andi/research/qemu-shared-memory/qemu/qga/commands.c:160:21: note: in expansion of macro ‘atomic_mb_read’
>      bool finished = atomic_mb_read(&gei->finished);
>                      ^~~~~~~~~~~~~~




Andreas Grapentin (1):
  added configure check for _Static_assert and updated
    QEMU_BUILD_BUG_ON(...) accordingly

 configure               | 18 ++++++++++++++++++
 include/qemu/compiler.h |  4 +++-
 2 files changed, 21 insertions(+), 1 deletion(-)

-- 
2.12.0

[Qemu-devel] [PATCH 1/1] added configure check for _Static_assert and updated QEMU_BUILD_BUG_ON(...) accordingly

[Andreas Grapentin]

---
 configure               | 18 ++++++++++++++++++
 include/qemu/compiler.h |  4 +++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 75c7c3526c..e9b33d9cf8 100755
--- a/configure
+++ b/configure
@@ -4725,6 +4725,20 @@ if compile_prog "" "" ; then
 fi
 
 ##########################################
+# check for _Static_assert()
+
+have_static_assert=no
+cat > $TMPC << EOF
+_Static_assert(1, "success.");
+int main(void) {
+    return 0;
+}
+EOF
+if compile_prog "" "" ; then
+    have_static_assert=yes
+fi
+
+##########################################
 # End of CC checks
 # After here, no more $cc or $ld runs
 
@@ -5694,6 +5708,10 @@ if test "$have_sysmacros" = "yes" ; then
   echo "CONFIG_SYSMACROS=y" >> $config_host_mak
 fi
 
+if test "$have_static_assert" = "yes" ; then
+  echo "CONFIG_STATIC_ASSERT=y" >> $config_host_mak
+fi
+
 # Hold two types of flag:
 #   CONFIG_THREAD_SETNAME_BYTHREAD  - we've got a way of setting the name on
 #                                     a thread we have a handle to
diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index e0ce9ffb28..37a65d4bb7 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -90,7 +90,9 @@
         int:(x) ? -1 : 1; \
     }
 
-#ifdef __COUNTER__
+#if defined(CONFIG_STATIC_ASSERT)
+#define QEMU_BUILD_BUG_ON(x) _Static_assert(!(x), #x)
+#elif defined(__COUNTER__)
 #define QEMU_BUILD_BUG_ON(x) typedef QEMU_BUILD_BUG_ON_STRUCT(x) \
     glue(qemu_build_bug_on__, __COUNTER__) __attribute__((unused))
 #else
-- 
2.12.0

Re: [Qemu-devel] [PATCH 0/1] Developer Conveniences - check for _Static_assert and use in QEMU_BUILD_BUG_ON

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1565 bytes --]

On 03/14/2017 09:44 AM, Andreas Grapentin wrote:
> 
> (as taken from here: http://wiki.qemu-project.org/Contribute/BiteSizedTasks)
> 

A single patch can be sent without a cover letter if desired (0/N cover
letters are only mandatory on patch series).  In fact, much of the
information you give here:

> 
> I added a configure check for C11's _Static_assert, and based on the outcome,
> QEMU_BUILD_BUG_ON will now produce slightly more readable results on failures.
> 
> Systems without C11 conforming compilers will still have the old messages, as
> verified by intentionally breaking the configure check.
> 
> the following example output was generated by inverting the condition in
> QEMU_BUILD_BUG_ON:
> 
> without _Static_assert:
> 
...

> with _Static_assert:
> 
>> In file included from /home/andi/research/qemu-shared-memory/qemu/include/qemu/osdep.h:36:0,
>>                  from /home/andi/research/qemu-shared-memory/qemu/qga/commands.c:13:
>> /home/andi/research/qemu-shared-memory/qemu/qga/commands.c: In function ‘qmp_guest_exec_status’:
>> /home/andi/research/qemu-shared-memory/qemu/include/qemu/compiler.h:94:30: error: static assertion failed: "sizeof(*&gei->finished) > sizeof(void *)"
>>  #define QEMU_BUILD_BUG_ON(x) _Static_assert((x), #x)
>>                               ^

should be in the commit message of the actual patch (right now, your 1/1
message has a rather blank commit message).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 1/1] added configure check for _Static_assert and updated QEMU_BUILD_BUG_ON(...) accordingly

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1937 bytes --]

On 03/14/2017 09:44 AM, Andreas Grapentin wrote:
> ---

Missing a Signed-off-by, so you'll need to send a v2 before we can apply it.

Subject line is long; I suggest shortening it to something like:

build: use _Static_assert in QEMU_BUILD_BUG_ON

then going into details in the rest of the commit body (see also my
comments on your 0/1 mail about content that should have been on this
commit).

>  configure               | 18 ++++++++++++++++++
>  include/qemu/compiler.h |  4 +++-
>  2 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 75c7c3526c..e9b33d9cf8 100755
> --- a/configure
> +++ b/configure
> @@ -4725,6 +4725,20 @@ if compile_prog "" "" ; then
>  fi
>  
>  ##########################################
> +# check for _Static_assert()
> +
> +have_static_assert=no
> +cat > $TMPC << EOF
> +_Static_assert(1, "success.");

trailing dot looks awkward; the test is the same if you omit it.

> +++ b/include/qemu/compiler.h
> @@ -90,7 +90,9 @@
>          int:(x) ? -1 : 1; \
>      }
>  
> -#ifdef __COUNTER__
> +#if defined(CONFIG_STATIC_ASSERT)
> +#define QEMU_BUILD_BUG_ON(x) _Static_assert(!(x), #x)

Should we add any explanatory text in addition to just the expression
being tested?  For example:

block/raw-format.c:        QEMU_BUILD_BUG_ON(BDRV_SECTOR_SIZE != 512);

A message of "static assertion failed: BDRV_SECTOR_SIZE != 512" is
tricky: it can only trip if the sector size was not 512, but normally
when I see a _Static_assert message, I assume that I should fix my code
to make the expression true, and the expression printed is already true.

Maybe _Static_assert(!(x), "not expecting " #x) is better, because it
would give a message of "static assertion failed: not expecting
BDRV_SECTOR_SIZE != 512".

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]