On Tue, Mar 23, 2021 at 06:54:24PM +0100, Andrew Jones wrote:
> Checking for overflow can be difficult, but doing so may be a good
> idea to avoid difficult to debug problems. Compilers that provide
> builtins for overflow checking allow the checks to be simple
> enough that we can use them more liberally. The idea for this
> flag is to wrap a calculation that should have overflow checking,
> allowing compilers that support it to give us some extra robustness.
> For example,
>
> #ifdef COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW
> bool overflow = __builtin_mul_overflow(x, y, &z);
> assert(!overflow);
> #else
> /* Older compiler, hopefully we don't overflow... */
> z = x * y;
> #endif
>
> This is a bit ugly though, so when possible we can just use the
> predicate wrappers, which have an always-false fallback, e.g.
>
> /* Old compilers won't assert on overflow. Oh, well... */
> assert(!check_mul_overflow(x, y));
> z = x * y;
>
> Signed-off-by: Andrew Jones <drjones@xxxxxxxxxx>
> ---
>
> v2: Added predicate wrappers
>
> lib/linux/compiler.h | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/lib/linux/compiler.h b/lib/linux/compiler.h
> index 2d72f18c36e5..aa2e3710cf1d 100644
> --- a/lib/linux/compiler.h
> +++ b/lib/linux/compiler.h
> @@ -8,6 +8,39 @@
>
> #ifndef __ASSEMBLY__
>
> +#define GCC_VERSION (__GNUC__ * 10000 \
> + + __GNUC_MINOR__ * 100 \
> + + __GNUC_PATCHLEVEL__)
> +
> +#ifdef __clang__
> +#if __has_builtin(__builtin_add_overflow) && \
> + __has_builtin(__builtin_sub_overflow) && \
> + __has_builtin(__builtin_mul_overflow)
> +#define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
> +#define check_add_overflow(a, b) ({ \
> + typeof((a) + (b)) __d; \
> + __builtin_add_overflow(a, b, &__d); \
> +})
> +#define check_sub_overflow(a, b) ({ \
> + typeof((a) - (b)) __d; \
> + __builtin_sub_overflow(a, b, &__d); \
> +})
> +#define check_mul_overflow(a, b) ({ \
> + typeof((a) * (b)) __d; \
> + __builtin_mul_overflow(a, b, &__d); \
> +})
> +#endif
> +#elif GCC_VERSION >= 50100
> +#define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
> +#define check_add_overflow(a, b) __builtin_add_overflow_p(a, b, (typeof((a) + (b)))0)
> +#define check_sub_overflow(a, b) __builtin_add_overflow_p(a, b, (typeof((a) - (b)))0)
> +#define check_mul_overflow(a, b) __builtin_add_overflow_p(a, b, (typeof((a) * (b)))0)
> +#else
> +#define check_add_overflow(a, b) (0)
> +#define check_sub_overflow(a, b) (0)
> +#define check_mul_overflow(a, b) (0)
> +#endif
> +
> #include <stdint.h>
>
> #define barrier() asm volatile("" : : : "memory")
> --
> 2.26.3
>
Applied to arm/queue
https://gitlab.com/rhdrjones/kvm-unit-tests/-/commits/arm/queue
but with another fixup. When the builins are not available and we want to
fallback to always false, we also need to ensure any parameters passed to
the check_* functions do not result in unused variable warnings. To do
that, those macros have been changed to
#define check_add_overflow(a, b) ({ (void)((int)(a) == (int)(b)); 0; })
#define check_sub_overflow(a, b) ({ (void)((int)(a) == (int)(b)); 0; })
#define check_mul_overflow(a, b) ({ (void)((int)(a) == (int)(b)); 0; })
Thanks,
drew
