* [Qemu-devel] [PATCH v2] use _Static_assert in QEMU_BUILD_BUG_ON
@ 2017-03-14 16:59 Andreas Grapentin
2017-03-14 17:43 ` Eric Blake
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Andreas Grapentin @ 2017-03-14 16:59 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, eblake, Andreas Grapentin
QEMU_BUILD_BUG_ON should use C11's _Static_assert, if the compiler supports it,
to provide more readable messages on failure.
We check for _Static_assert in configure, and set CONFIG_STATIC_ASSERT
accordingly. QEMU_BUILD_BUG_ON invokes _Static_assert if CONFIG_STATIC_ASSERT
is defined, and reverts to the old way otherwise.
That way, systems without C11 conforming compiler 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 /qemu/include/qemu/osdep.h:36:0,
> from /qemu/qga/commands.c:13:
> /qemu/qga/commands.c: In function ‘qmp_guest_exec_status’:
> /qemu/include/qemu/compiler.h:89:12: error: negative width in bit-field ‘<anonymous>’
> struct { \
> ^
> /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) \
> ^~~~~~~~~~~~~~~~~~~~~~~~
> /qemu/include/qemu/atomic.h:146:5: note: in expansion of macro ‘QEMU_BUILD_BUG_ON’
> QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \
> ^~~~~~~~~~~~~~~~~
> /qemu/include/qemu/atomic.h:417:5: note: in expansion of macro ‘atomic_load_acquire’
> atomic_load_acquire(ptr)
> ^~~~~~~~~~~~~~~~~~~
> /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 /qemu/include/qemu/osdep.h:36:0,
> from /qemu/qga/commands.c:13:
> /qemu/qga/commands.c: In function ‘qmp_guest_exec_status’:
> /qemu/include/qemu/compiler.h:94:30: error: static assertion failed: "not expecting: sizeof(*&gei->finished) > sizeof(void *)"
> #define QEMU_BUILD_BUG_ON(x) _Static_assert((x), #x)
> ^
> /qemu/include/qemu/atomic.h:146:5: note: in expansion of macro ‘QEMU_BUILD_BUG_ON’
> QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \
> ^~~~~~~~~~~~~~~~~
> /qemu/include/qemu/atomic.h:417:5: note: in expansion of macro ‘atomic_load_acquire’
> atomic_load_acquire(ptr)
> ^~~~~~~~~~~~~~~~~~~
> /qemu/qga/commands.c:160:21: note: in expansion of macro ‘atomic_mb_read’
> bool finished = atomic_mb_read(&gei->finished);
> ^~~~~~~~~~~~~~
Signed-off-by: Andreas Grapentin <andreas@grapentin.org>
---
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), "not expecting: " #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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH v2] use _Static_assert in QEMU_BUILD_BUG_ON
2017-03-14 16:59 [Qemu-devel] [PATCH v2] use _Static_assert in QEMU_BUILD_BUG_ON Andreas Grapentin
@ 2017-03-14 17:43 ` Eric Blake
2017-03-14 18:38 ` Andreas Grapentin
2017-03-14 22:02 ` Richard Henderson
2017-04-23 17:24 ` Michael Tokarev
2 siblings, 1 reply; 5+ messages in thread
From: Eric Blake @ 2017-03-14 17:43 UTC (permalink / raw)
To: Andreas Grapentin, qemu-devel; +Cc: qemu-trivial
[-- Attachment #1: Type: text/plain, Size: 1796 bytes --]
On 03/14/2017 11:59 AM, Andreas Grapentin wrote:
> QEMU_BUILD_BUG_ON should use C11's _Static_assert, if the compiler supports it,
> to provide more readable messages on failure.
>
> with _Static_assert:
>
>> In file included from /qemu/include/qemu/osdep.h:36:0,
>> from /qemu/qga/commands.c:13:
>> /qemu/qga/commands.c: In function ‘qmp_guest_exec_status’:
>> /qemu/include/qemu/compiler.h:94:30: error: static assertion failed: "not expecting: sizeof(*&gei->finished) > sizeof(void *)"
>> #define QEMU_BUILD_BUG_ON(x) _Static_assert((x), #x)
Manual copy-and-paste? Because this is missing the ! present in the
actual definition...
>> ^
>> /qemu/include/qemu/atomic.h:146:5: note: in expansion of macro ‘QEMU_BUILD_BUG_ON’
>> QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \
>> ^~~~~~~~~~~~~~~~~
>> /qemu/include/qemu/atomic.h:417:5: note: in expansion of macro ‘atomic_load_acquire’
>> atomic_load_acquire(ptr)
>> ^~~~~~~~~~~~~~~~~~~
>> /qemu/qga/commands.c:160:21: note: in expansion of macro ‘atomic_mb_read’
>> bool finished = atomic_mb_read(&gei->finished);
>> ^~~~~~~~~~~~~~
>
> Signed-off-by: Andreas Grapentin <andreas@grapentin.org>
> ---
> +++ 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), "not expecting: " #x)
...here.
But that's trivial, if the maintainer would like to correct the commit
message.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
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 --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2] use _Static_assert in QEMU_BUILD_BUG_ON
2017-03-14 17:43 ` Eric Blake
@ 2017-03-14 18:38 ` Andreas Grapentin
0 siblings, 0 replies; 5+ messages in thread
From: Andreas Grapentin @ 2017-03-14 18:38 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, qemu-trivial
[-- Attachment #1: Type: text/plain, Size: 2413 bytes --]
The missing ! is because I generated the error messages by inverting the
condition and then forgot to change it back. I can send a v3 if you
like.
Thanks for being patient with me.
-A
On Tue, Mar 14, 2017 at 12:43:56PM -0500, Eric Blake wrote:
> On 03/14/2017 11:59 AM, Andreas Grapentin wrote:
> > QEMU_BUILD_BUG_ON should use C11's _Static_assert, if the compiler supports it,
> > to provide more readable messages on failure.
>
> >
> > with _Static_assert:
> >
> >> In file included from /qemu/include/qemu/osdep.h:36:0,
> >> from /qemu/qga/commands.c:13:
> >> /qemu/qga/commands.c: In function ‘qmp_guest_exec_status’:
> >> /qemu/include/qemu/compiler.h:94:30: error: static assertion failed: "not expecting: sizeof(*&gei->finished) > sizeof(void *)"
> >> #define QEMU_BUILD_BUG_ON(x) _Static_assert((x), #x)
>
> Manual copy-and-paste? Because this is missing the ! present in the
> actual definition...
>
> >> ^
> >> /qemu/include/qemu/atomic.h:146:5: note: in expansion of macro ‘QEMU_BUILD_BUG_ON’
> >> QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \
> >> ^~~~~~~~~~~~~~~~~
> >> /qemu/include/qemu/atomic.h:417:5: note: in expansion of macro ‘atomic_load_acquire’
> >> atomic_load_acquire(ptr)
> >> ^~~~~~~~~~~~~~~~~~~
> >> /qemu/qga/commands.c:160:21: note: in expansion of macro ‘atomic_mb_read’
> >> bool finished = atomic_mb_read(&gei->finished);
> >> ^~~~~~~~~~~~~~
> >
> > Signed-off-by: Andreas Grapentin <andreas@grapentin.org>
> > ---
>
> > +++ 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), "not expecting: " #x)
>
> ...here.
>
> But that's trivial, if the maintainer would like to correct the commit
> message.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
>
> --
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
--
------------------------------------------------------------------------------
my GPG Public Key: https://files.grapentin.org/.gpg/public.key
------------------------------------------------------------------------------
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2] use _Static_assert in QEMU_BUILD_BUG_ON
2017-03-14 16:59 [Qemu-devel] [PATCH v2] use _Static_assert in QEMU_BUILD_BUG_ON Andreas Grapentin
2017-03-14 17:43 ` Eric Blake
@ 2017-03-14 22:02 ` Richard Henderson
2017-04-23 17:24 ` Michael Tokarev
2 siblings, 0 replies; 5+ messages in thread
From: Richard Henderson @ 2017-03-14 22:02 UTC (permalink / raw)
To: Andreas Grapentin, qemu-devel; +Cc: qemu-trivial
On 03/15/2017 02:59 AM, Andreas Grapentin wrote:
> QEMU_BUILD_BUG_ON should use C11's _Static_assert, if the compiler supports it,
> to provide more readable messages on failure.
>
> We check for _Static_assert in configure, and set CONFIG_STATIC_ASSERT
> accordingly. QEMU_BUILD_BUG_ON invokes _Static_assert if CONFIG_STATIC_ASSERT
> is defined, and reverts to the old way otherwise.
>
> That way, systems without C11 conforming compiler 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 /qemu/include/qemu/osdep.h:36:0,
>> from /qemu/qga/commands.c:13:
>> /qemu/qga/commands.c: In function ‘qmp_guest_exec_status’:
>> /qemu/include/qemu/compiler.h:89:12: error: negative width in bit-field ‘<anonymous>’
>> struct { \
>> ^
>> /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) \
>> ^~~~~~~~~~~~~~~~~~~~~~~~
>> /qemu/include/qemu/atomic.h:146:5: note: in expansion of macro ‘QEMU_BUILD_BUG_ON’
>> QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \
>> ^~~~~~~~~~~~~~~~~
>> /qemu/include/qemu/atomic.h:417:5: note: in expansion of macro ‘atomic_load_acquire’
>> atomic_load_acquire(ptr)
>> ^~~~~~~~~~~~~~~~~~~
>> /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 /qemu/include/qemu/osdep.h:36:0,
>> from /qemu/qga/commands.c:13:
>> /qemu/qga/commands.c: In function ‘qmp_guest_exec_status’:
>> /qemu/include/qemu/compiler.h:94:30: error: static assertion failed: "not expecting: sizeof(*&gei->finished) > sizeof(void *)"
>> #define QEMU_BUILD_BUG_ON(x) _Static_assert((x), #x)
>> ^
>> /qemu/include/qemu/atomic.h:146:5: note: in expansion of macro ‘QEMU_BUILD_BUG_ON’
>> QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \
>> ^~~~~~~~~~~~~~~~~
>> /qemu/include/qemu/atomic.h:417:5: note: in expansion of macro ‘atomic_load_acquire’
>> atomic_load_acquire(ptr)
>> ^~~~~~~~~~~~~~~~~~~
>> /qemu/qga/commands.c:160:21: note: in expansion of macro ‘atomic_mb_read’
>> bool finished = atomic_mb_read(&gei->finished);
>> ^~~~~~~~~~~~~~
>
> Signed-off-by: Andreas Grapentin <andreas@grapentin.org>
> ---
> 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), "not expecting: " #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
>
Reviewed-by: Richard Henderson <rth@twiddle.net>
r~
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH v2] use _Static_assert in QEMU_BUILD_BUG_ON
2017-03-14 16:59 [Qemu-devel] [PATCH v2] use _Static_assert in QEMU_BUILD_BUG_ON Andreas Grapentin
2017-03-14 17:43 ` Eric Blake
2017-03-14 22:02 ` Richard Henderson
@ 2017-04-23 17:24 ` Michael Tokarev
2 siblings, 0 replies; 5+ messages in thread
From: Michael Tokarev @ 2017-04-23 17:24 UTC (permalink / raw)
To: Andreas Grapentin, qemu-devel; +Cc: qemu-trivial, eblake
14.03.2017 19:59, Andreas Grapentin wrote:
> QEMU_BUILD_BUG_ON should use C11's _Static_assert, if the compiler supports it,
> to provide more readable messages on failure.
...
Applied to -trivial, with trivial commit comment fix. Thanks!
/mjt
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-04-23 17:25 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-14 16:59 [Qemu-devel] [PATCH v2] use _Static_assert in QEMU_BUILD_BUG_ON Andreas Grapentin
2017-03-14 17:43 ` Eric Blake
2017-03-14 18:38 ` Andreas Grapentin
2017-03-14 22:02 ` Richard Henderson
2017-04-23 17:24 ` Michael Tokarev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).