Re: RFC: (Unconditionally) enable -fno-strict-overflow for kernel builds

+1, this caught us out with sendfile testing very recently :(


-a

On 30 November 2013 05:56, Konstantin Belousov <kostikbel_at_gmail.com> wrote:
> I propose to unconditionally add the switch  -fno-strict-overflow to the
> kernel compilation.  See the patch at the end of message for exact change
> proposed.
>
> What does it do. It disallows useless and counter-intuitive behaviour of
> the compiler(s) for the signed overflow. Basically, the issue is that
> the C standard left signed overflow as undefined to allow for different
> hardware implementation of signess to be used for signed arithmetic.
> De-facto, all architectures where FreeBSD works or have a chance to be
> ported, use two-complement signed integer representation, and developers
> intuition is right about it.
>
> The compiler authors take the undefined part there as a blanket to perform
> optimizations which are assuming that signed overflow cannot happen.  The
> problem with that approach is that typical checks for bounds are exactly
> the place where the overflow can happen.  Instead of making some artificial
> example, I would just point to my own r258088 and r258397.
>
> What makes the things much worse is that the behaviour is highly depended
> on the optimization level of the exact version of compiler.
>
> What other projects did in this regard. They turned the same knob
> unconditionally. I can point at least to Linux kernel and Postgresql.
> Python uses -fwrapv, which is equivalent to the -fno-strict-overflow
> on the two-complement machines.  Linux used -fwrapv before switched
> to -fno-strict-overflow.
>
> diff --git a/sys/conf/kern.mk b/sys/conf/kern.mk
> index 2939a59..6e6ba92 100644
> --- a/sys/conf/kern.mk
> +++ b/sys/conf/kern.mk
> _at__at_ -148,6 +148,12 _at__at_ INLINE_LIMIT?=     8000
>  CFLAGS+=       -ffreestanding
>
>  #
> +# Do not allow a compiler to optimize out overflow checks for signed
> +# types.
> +#
> +CFLAGS+=       -fno-strict-overflow
> +
> +#
>  # GCC SSP support
>  #
>  .if ${MK_SSP} != "no" && ${MACHINE_CPUARCH} != "ia64" && \