* [PATCH v2] coverity: provide Coverity-friendly MIN_CONST and MAX_CONST
@ 2020-06-29 16:28 Eric Blake
2020-06-30 15:01 ` Philippe Mathieu-Daudé
2020-06-30 15:39 ` Paolo Bonzini
0 siblings, 2 replies; 3+ messages in thread
From: Eric Blake @ 2020-06-29 16:28 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, pbonzini
Coverity has problems seeing through __builtin_choose_expr, which
result in it abandoning analysis of later functions that utilize a
definition that used MIN_CONST or MAX_CONST, such as in qemu-file.c:
50 DECLARE_BITMAP(may_free, MAX_IOV_SIZE);
CID 1429992 (#1 of 1): Unrecoverable parse warning (PARSE_ERROR)1.
expr_not_constant: expression must have a constant value
As has been done in the past (see 07d66672), it's okay to dumb things
down when compiling for static analyzers. (Of course, now the
syntax-checker has a false positive on our reference to
__COVERITY__...)
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Fixes: CID 1429992, CID 1429995, CID 1429997, CID 1429999
Signed-off-by: Eric Blake <eblake@redhat.com>
---
Improvements over Paolo's v1:
- proper use of ()
- add comment explaining the COVERITY section
- add indentation for easier read of #if/#else flow
include/qemu/osdep.h | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 0d26a1b9bd07..0fc206ae6154 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -250,7 +250,8 @@ extern int daemon(int, int);
* Note that neither form is usable as an #if condition; if you truly
* need to write conditional code that depends on a minimum or maximum
* determined by the pre-processor instead of the compiler, you'll
- * have to open-code it.
+ * have to open-code it. Sadly, Coverity is severely confused by the
+ * constant variants, so we have to dumb things down there.
*/
#undef MIN
#define MIN(a, b) \
@@ -258,22 +259,28 @@ extern int daemon(int, int);
typeof(1 ? (a) : (b)) _a = (a), _b = (b); \
_a < _b ? _a : _b; \
})
-#define MIN_CONST(a, b) \
- __builtin_choose_expr( \
- __builtin_constant_p(a) && __builtin_constant_p(b), \
- (a) < (b) ? (a) : (b), \
- ((void)0))
#undef MAX
#define MAX(a, b) \
({ \
typeof(1 ? (a) : (b)) _a = (a), _b = (b); \
_a > _b ? _a : _b; \
})
-#define MAX_CONST(a, b) \
+
+#ifdef __COVERITY__
+# define MIN_CONST(a, b) ((a) < (b) ? (a) : (b))
+# define MAX_CONST(a, b) ((a) > (b) ? (a) : (b))
+#else
+# define MIN_CONST(a, b) \
+ __builtin_choose_expr( \
+ __builtin_constant_p(a) && __builtin_constant_p(b), \
+ (a) < (b) ? (a) : (b), \
+ ((void)0))
+# define MAX_CONST(a, b) \
__builtin_choose_expr( \
__builtin_constant_p(a) && __builtin_constant_p(b), \
(a) > (b) ? (a) : (b), \
((void)0))
+#endif
/*
* Minimum function that returns zero only if both values are zero.
--
2.27.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v2] coverity: provide Coverity-friendly MIN_CONST and MAX_CONST
2020-06-29 16:28 [PATCH v2] coverity: provide Coverity-friendly MIN_CONST and MAX_CONST Eric Blake
@ 2020-06-30 15:01 ` Philippe Mathieu-Daudé
2020-06-30 15:39 ` Paolo Bonzini
1 sibling, 0 replies; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-30 15:01 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: peter.maydell, pbonzini
On 6/29/20 6:28 PM, Eric Blake wrote:
> Coverity has problems seeing through __builtin_choose_expr, which
> result in it abandoning analysis of later functions that utilize a
> definition that used MIN_CONST or MAX_CONST, such as in qemu-file.c:
>
> 50 DECLARE_BITMAP(may_free, MAX_IOV_SIZE);
>
> CID 1429992 (#1 of 1): Unrecoverable parse warning (PARSE_ERROR)1.
> expr_not_constant: expression must have a constant value
>
> As has been done in the past (see 07d66672), it's okay to dumb things
> down when compiling for static analyzers. (Of course, now the
> syntax-checker has a false positive on our reference to
> __COVERITY__...)
>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Fixes: CID 1429992, CID 1429995, CID 1429997, CID 1429999
> Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>
> Improvements over Paolo's v1:
> - proper use of ()
> - add comment explaining the COVERITY section
> - add indentation for easier read of #if/#else flow
>
> include/qemu/osdep.h | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 0d26a1b9bd07..0fc206ae6154 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -250,7 +250,8 @@ extern int daemon(int, int);
> * Note that neither form is usable as an #if condition; if you truly
> * need to write conditional code that depends on a minimum or maximum
> * determined by the pre-processor instead of the compiler, you'll
> - * have to open-code it.
> + * have to open-code it. Sadly, Coverity is severely confused by the
> + * constant variants, so we have to dumb things down there.
> */
> #undef MIN
> #define MIN(a, b) \
> @@ -258,22 +259,28 @@ extern int daemon(int, int);
> typeof(1 ? (a) : (b)) _a = (a), _b = (b); \
> _a < _b ? _a : _b; \
> })
> -#define MIN_CONST(a, b) \
> - __builtin_choose_expr( \
> - __builtin_constant_p(a) && __builtin_constant_p(b), \
> - (a) < (b) ? (a) : (b), \
> - ((void)0))
> #undef MAX
> #define MAX(a, b) \
> ({ \
> typeof(1 ? (a) : (b)) _a = (a), _b = (b); \
> _a > _b ? _a : _b; \
> })
> -#define MAX_CONST(a, b) \
> +
> +#ifdef __COVERITY__
> +# define MIN_CONST(a, b) ((a) < (b) ? (a) : (b))
> +# define MAX_CONST(a, b) ((a) > (b) ? (a) : (b))
> +#else
> +# define MIN_CONST(a, b) \
> + __builtin_choose_expr( \
> + __builtin_constant_p(a) && __builtin_constant_p(b), \
> + (a) < (b) ? (a) : (b), \
> + ((void)0))
> +# define MAX_CONST(a, b) \
> __builtin_choose_expr( \
> __builtin_constant_p(a) && __builtin_constant_p(b), \
> (a) > (b) ? (a) : (b), \
> ((void)0))
> +#endif
>
> /*
> * Minimum function that returns zero only if both values are zero.
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2] coverity: provide Coverity-friendly MIN_CONST and MAX_CONST
2020-06-29 16:28 [PATCH v2] coverity: provide Coverity-friendly MIN_CONST and MAX_CONST Eric Blake
2020-06-30 15:01 ` Philippe Mathieu-Daudé
@ 2020-06-30 15:39 ` Paolo Bonzini
1 sibling, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2020-06-30 15:39 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: peter.maydell
On 29/06/20 18:28, Eric Blake wrote:
> Coverity has problems seeing through __builtin_choose_expr, which
> result in it abandoning analysis of later functions that utilize a
> definition that used MIN_CONST or MAX_CONST, such as in qemu-file.c:
>
> 50 DECLARE_BITMAP(may_free, MAX_IOV_SIZE);
>
> CID 1429992 (#1 of 1): Unrecoverable parse warning (PARSE_ERROR)1.
> expr_not_constant: expression must have a constant value
>
> As has been done in the past (see 07d66672), it's okay to dumb things
> down when compiling for static analyzers. (Of course, now the
> syntax-checker has a false positive on our reference to
> __COVERITY__...)
>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Fixes: CID 1429992, CID 1429995, CID 1429997, CID 1429999
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>
> Improvements over Paolo's v1:
> - proper use of ()
> - add comment explaining the COVERITY section
> - add indentation for easier read of #if/#else flow
>
> include/qemu/osdep.h | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 0d26a1b9bd07..0fc206ae6154 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -250,7 +250,8 @@ extern int daemon(int, int);
> * Note that neither form is usable as an #if condition; if you truly
> * need to write conditional code that depends on a minimum or maximum
> * determined by the pre-processor instead of the compiler, you'll
> - * have to open-code it.
> + * have to open-code it. Sadly, Coverity is severely confused by the
> + * constant variants, so we have to dumb things down there.
> */
> #undef MIN
> #define MIN(a, b) \
> @@ -258,22 +259,28 @@ extern int daemon(int, int);
> typeof(1 ? (a) : (b)) _a = (a), _b = (b); \
> _a < _b ? _a : _b; \
> })
> -#define MIN_CONST(a, b) \
> - __builtin_choose_expr( \
> - __builtin_constant_p(a) && __builtin_constant_p(b), \
> - (a) < (b) ? (a) : (b), \
> - ((void)0))
> #undef MAX
> #define MAX(a, b) \
> ({ \
> typeof(1 ? (a) : (b)) _a = (a), _b = (b); \
> _a > _b ? _a : _b; \
> })
> -#define MAX_CONST(a, b) \
> +
> +#ifdef __COVERITY__
> +# define MIN_CONST(a, b) ((a) < (b) ? (a) : (b))
> +# define MAX_CONST(a, b) ((a) > (b) ? (a) : (b))
> +#else
> +# define MIN_CONST(a, b) \
> + __builtin_choose_expr( \
> + __builtin_constant_p(a) && __builtin_constant_p(b), \
> + (a) < (b) ? (a) : (b), \
> + ((void)0))
> +# define MAX_CONST(a, b) \
> __builtin_choose_expr( \
> __builtin_constant_p(a) && __builtin_constant_p(b), \
> (a) > (b) ? (a) : (b), \
> ((void)0))
> +#endif
>
> /*
> * Minimum function that returns zero only if both values are zero.
>
Queued, thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-06-30 15:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-06-29 16:28 [PATCH v2] coverity: provide Coverity-friendly MIN_CONST and MAX_CONST Eric Blake
2020-06-30 15:01 ` Philippe Mathieu-Daudé
2020-06-30 15:39 ` Paolo Bonzini
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).