Re: SOLVED: Problem with -fno-strict-overflow (was: Re: RFC: (Unconditionally) enable -fno-strict-overflow for kernel builds)

2013-12-19 Thread Konstantin Belousov
On Thu, Dec 19, 2013 at 10:16:16AM +0100, Stefan Esser wrote:
> Am 30.11.2013 14:56, schrieb Konstantin Belousov:
> > 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.
> 
> Hi Konstantin,
> 
> you may put back -fno-strict-overflow after I found and fixed the
> problem uncovered by enabling it in -CURRENT (SVN rev. 259609).
> 
> The problem was an overflow in the conversion of timeout values to
> sbintine, which lead to negative values being detected with
> -fno-strict-overflow, while the compiler performed the signedness
> test before the multiplication, without that option.
> 
> I found that timeout values of more than 1000 years were requested
> by some programs, which are now capped at 68 years (the maximum that
> can be represented by sbintime, 2^31 seconds).
> 
> So, -fno-strict-overflow has already proved itself to be useful
> in uncovering a bug that would have been hard to find, otherwise.

Feel free to restore the commit, I have no plans to do this.


pgpjaTcm6YfbP.pgp
Description: PGP signature


Re: SOLVED: Problem with -fno-strict-overflow (was: Re: RFC: (Unconditionally) enable -fno-strict-overflow for kernel builds)

2013-12-19 Thread Oliver Pinter
On 12/19/13, Stefan Esser  wrote:
> Am 30.11.2013 14:56, schrieb Konstantin Belousov:
>> 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.
>
> Hi Konstantin,
>
> you may put back -fno-strict-overflow after I found and fixed the
> problem uncovered by enabling it in -CURRENT (SVN rev. 259609).
>
> The problem was an overflow in the conversion of timeout values to
> sbintine, which lead to negative values being detected with
> -fno-strict-overflow, while the compiler performed the signedness
> test before the multiplication, without that option.
>
> I found that timeout values of more than 1000 years were requested
> by some programs, which are now capped at 68 years (the maximum that
> can be represented by sbintime, 2^31 seconds).
>
> So, -fno-strict-overflow has already proved itself to be useful
> in uncovering a bug that would have been hard to find, otherwise.
>

I have a plan, to port this or like this plugin to llvm/clang in the
near future:

http://www.grsecurity.net/~ephox/overflow_plugin/

> Regards, STefan
> ___
> freebsd-current@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
>
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


SOLVED: Problem with -fno-strict-overflow (was: Re: RFC: (Unconditionally) enable -fno-strict-overflow for kernel builds)

2013-12-19 Thread Stefan Esser
Am 30.11.2013 14:56, schrieb Konstantin Belousov:
> 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.

Hi Konstantin,

you may put back -fno-strict-overflow after I found and fixed the
problem uncovered by enabling it in -CURRENT (SVN rev. 259609).

The problem was an overflow in the conversion of timeout values to
sbintine, which lead to negative values being detected with
-fno-strict-overflow, while the compiler performed the signedness
test before the multiplication, without that option.

I found that timeout values of more than 1000 years were requested
by some programs, which are now capped at 68 years (the maximum that
can be represented by sbintime, 2^31 seconds).

So, -fno-strict-overflow has already proved itself to be useful
in uncovering a bug that would have been hard to find, otherwise.

Regards, STefan
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


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

2013-12-01 Thread Dimitry Andric
On 01 Dec 2013, at 01:33, Adrian Chadd  wrote:
> On 30 November 2013 15:25, Dimitry Andric  wrote:
...
>> Basically, if you rely on undefined behavior, you are inventing your own
>> de facto language, which is *not* C.  That is fine with me, but let's
>> not pretend the FreeBSD kernel is written in C then. :-)
> 
> Are you able to have clang/llvm/gcc tell us where/when code is relying
> on undefined behaviour? So we can, like, fix them?

Not in the most general sense, since that would amount to solving the
halting problem.  But there are some tools that can help quite a lot.  I
guess Coverity can already cover quite a lot of cases, and there is also
the STACK tool from MIT:

http://css.csail.mit.edu/stack/

It would be really nice to have this in ports.

Another mechanism is run-time detection, e.g. the undefined behavior
sanitizer and other sanitizers:

http://clang.llvm.org/docs/UsersManual.html#controlling-code-generation

some of which have also been ported to gcc, see:

http://gcc.gnu.org/gcc-4.9/changes.html

However, these have not been completely ported to FreeBSD yet, and come
at a (sometimes large) run-time cost.  Still a lot less than valgrind,
though. :-)

Also, for use in the kernel, the run-time support would have to be
ported separately to the kernel environment.  


> If there was a way to lint this stuff then yes, please lint it.
> 
> Otherwise we don't have the tools to know whether we're doing sane
> things or not.
> 
> (Same with things like strict aliasing..)

Yes, the comparison with strict aliasing is spot-on.  A lot of code has
been written that is not aliasing safe, and if it is too much effort to
fix it, using -fno-strict-aliasing is a reasonable workaround.

Note this option can prevent a lot of very useful optimizations, but if
you do not particularly care (for example if you are waiting for slow
hardware anyway), it is fine to use it.

-Dimitry



signature.asc
Description: Message signed with OpenPGP using GPGMail


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

2013-12-01 Thread dt71

Konstantin Belousov wrote, On 11/30/2013 13:56:

The compiler authors take the undefined part there as a blanket to perform
optimizations which are assuming that signed overflow cannot happen.


Personally, when I first heard about such assumptions, it was inspiring to 
write code in a way that automatically gives the compiler certain ``overflow 
cannot happer'' (for example, because the input values given are always small) 
hints, such as turning some uses of ``unsigned int'' (where a negative value 
logically doesn't make sense, such as in a size/length value) into ``signed 
int''. However, I quickly found that this way of thinking leads to 
counter-production: coding becomes slower, the resulting code becomes less 
readable, while the performance gain remains questionable. It would be much 
better if hints could be given to the compiler using assert() (which would have 
effect even in non-debug mode). How do others feel?

Konstantin Belousov wrote, On 12/01/2013 08:59:

It is written in C, but no useful program can be written in the pure
standard C.  We must rely on the assumptions about underlying architecture,
and compiler must provide sane access to the features of the underlying
architecture to be useful.


But what behavior do you want for signed arithmetic? Modular, saturating, or some other? 
And how do you signal that? Or maybe you just want to check for (signed/unsigned) integer 
overflow (which can't be done "cleanly and efficiently" in C), in which case 
someone should write a check_add_overflow() function...
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


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

2013-11-30 Thread Konstantin Belousov
On Sat, Nov 30, 2013 at 04:33:17PM -0800, Adrian Chadd wrote:
> On 30 November 2013 15:25, Dimitry Andric  wrote:
> > On 30 Nov 2013, at 14:56, Konstantin Belousov  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.
> >
> > I think this is quite a misrepresentation.  Any C compiler is free to do
> > whatever it wants whenever it encounters undefined behavior.  Some
> > behavior is undefined in the C standards, so compilers can do a better
> > job at optimization.
Sure. And we are free to call such compiler useless and moronic. E.g.
the standard explicitely marks any code implementing malloc-like
allocator and VM 'forbidden', since it cannot be done without calling
undefined behaviour. Should we stop writing the kernel or libc ?

> >
> > If the optimized code fails to do what the programmer thinks it does, it
> > is almost always the programmer's fault, excluding actual compiler bugs
> > (which are unavoidable, as all software has bugs).
> >
> > Basically, if you rely on undefined behavior, you are inventing your own
> > de facto language, which is *not* C.  That is fine with me, but let's
> > not pretend the FreeBSD kernel is written in C then. :-)
It is written in C, but no useful program can be written in the pure
standard C.  We must rely on the assumptions about underlying architecture,
and compiler must provide sane access to the features of the underlying
architecture to be useful.

Just to list a few,
- ILP32 or LP64;
- ABI;
- flat address space with arbitrary arithmetic on the data pointers;
- code as data, in the same address space (yes, we patch code at
  runtime);
- non-regular memory, both with additional side-effects when accessed,
  like register mappings (this cannot be modelled with volatile, think
  about e.g. address space switch caused by register access);
  and side-effect free memory with non-WC semantic.
This can be continued infinitely.

Why _sane_ implementation of signed arithmetic for 2-complement machine,
which is just one item in the list above, is tolerated to be crippled ?
Esp. since it causes (god forbids) *security* problems systematically ?

> 
> Are you able to have clang/llvm/gcc tell us where/when code is relying
> on undefined behaviour? So we can, like, fix them?
No, it is impossible. This is similar to the -fno-strict-aliasing.
Compiler is sometimes able to note that it cheat on you by applying
the undefined behaviour card, but this is not coded in the compiler
systematically. And, the biggest problem, routine changes in the
optimizer cause different places to become the victim. So the fact that
your code is not flagged today does not assure that it would be not
crippled tomorrow.

> 
> If there was a way to lint this stuff then yes, please lint it.
> 
> Otherwise we don't have the tools to know whether we're doing sane
> things or not.
> 
> (Same with things like strict aliasing..)
We do not have the tools, and indeed, same as with strict aliasing.



pgpSzxypyojez.pgp
Description: PGP signature


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

2013-11-30 Thread Peter Wemm
On Sat, Nov 30, 2013 at 8:38 PM, Eitan Adler  wrote:
> On Sat, Nov 30, 2013 at 11:26 PM, Peter Wemm  wrote:
>> On Sat, Nov 30, 2013 at 4:33 PM, Adrian Chadd  wrote:
>> [..]
>>> Are you able to have clang/llvm/gcc tell us where/when code is relying
>>> on undefined behaviour? So we can, like, fix them?
>>
>> It wasn't all that long ago that we had this wonderful thing called
>> -Werror and had a clean kernel build.
>>
>> The problem is that gcc and clang have different warning sets.  I seem
>> to recall we had -Werror on for gcc and off for clang.  IMHO it would
>> be more useful to do it the other way around.
>
> Not all cases can be caught by static analysis.  They would all be
> caught be the integer sanitizer.  However, these have not yet been
> ported to FreeBSD.
>

I also missed the  -Wno-error-tautological-compare setting. Oops.

I personally tweak my builds a little so that:

  CC ../../../kern/kern_acct.c
  CC ../../../kern/kern_clock.c
WARNING: kern_clock.c: enum pmc_event has too many values: 1669 > 1023
  CC ../../../kern/kern_condvar.c
  CC ../../../kern/kern_conf.c
  CC ../../../kern/kern_cons.c
  CC ../../../kern/kern_cpu.c
  CC ../../../kern/kern_cpuset.c
../../../kern/kern_cpuset.c:637:16: warning: comparison of unsigned
expression < 0 is always false [-Wtautological-compare]
for (i = 0; i < (_NCPUWORDS - 1); i++) {
~ ^ 
1 warning generated.
  CC ../../../kern/kern_context.c
  CC ../../../kern/kern_descrip.c
  CC ../../../kern/kern_dtrace.c

Warnings stand out nicely that way.

The diff is along these lines:

--- kern.pre.mk(revision 258784)
+++ kern.pre.mk(working copy)
@@ -126,12 +126,12 @@
 # Optional linting. This can be overridden in /etc/make.conf.
 LINTFLAGS=${LINTOBJKERNFLAGS}

-NORMAL_C= ${CC} -c ${CFLAGS} ${WERROR} ${PROF} ${.IMPSRC}
-NORMAL_S= ${CC} -c ${ASM_CFLAGS} ${WERROR} ${.IMPSRC}
+NORMAL_C= @echo "  CC ${.IMPSRC}" ; ${CC} -c ${CFLAGS} ${WERROR}
${PROF} ${.IMPSRC}
+NORMAL_S= @echo "  AS ${.IMPSRC}" ; ${CC} -c ${ASM_CFLAGS} ${WERROR} ${.IMPSRC}
 PROFILE_C= ${CC} -c ${CFLAGS} ${WERROR} ${.IMPSRC}
-NORMAL_C_NOWERROR= ${CC} -c ${CFLAGS} ${PROF} ${.IMPSRC}
+NORMAL_C_NOWERROR= @echo "  CC_NOWERROR ${.IMPSRC}" ; ${CC} -c
${CFLAGS} ${PROF} ${.IMPSRC}
...

Unfortunately that interferes with my usual use of 'make -s' - silent.
-- 
Peter Wemm - pe...@wemm.org; pe...@freebsd.org; pe...@yahoo-inc.com; KI6FJV
UTF-8: for when a ' just won\342\200\231t do.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


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

2013-11-30 Thread Eitan Adler
On Sat, Nov 30, 2013 at 11:26 PM, Peter Wemm  wrote:
> On Sat, Nov 30, 2013 at 4:33 PM, Adrian Chadd  wrote:
> [..]
>> Are you able to have clang/llvm/gcc tell us where/when code is relying
>> on undefined behaviour? So we can, like, fix them?
>
> It wasn't all that long ago that we had this wonderful thing called
> -Werror and had a clean kernel build.
>
> The problem is that gcc and clang have different warning sets.  I seem
> to recall we had -Werror on for gcc and off for clang.  IMHO it would
> be more useful to do it the other way around.

Not all cases can be caught by static analysis.  They would all be
caught be the integer sanitizer.  However, these have not yet been
ported to FreeBSD.





-- 
Eitan Adler
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


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

2013-11-30 Thread Peter Wemm
On Sat, Nov 30, 2013 at 4:33 PM, Adrian Chadd  wrote:
[..]
> Are you able to have clang/llvm/gcc tell us where/when code is relying
> on undefined behaviour? So we can, like, fix them?

It wasn't all that long ago that we had this wonderful thing called
-Werror and had a clean kernel build.

The problem is that gcc and clang have different warning sets.  I seem
to recall we had -Werror on for gcc and off for clang.  IMHO it would
be more useful to do it the other way around.

-- 
Peter Wemm - pe...@wemm.org; pe...@freebsd.org; pe...@yahoo-inc.com; KI6FJV
UTF-8: for when a ' just won\342\200\231t do.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


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

2013-11-30 Thread dt71

Adrian Chadd wrote, On 12/01/2013 01:33:

Are you able to have clang/llvm/gcc tell us where/when code is relying
on undefined behaviour? So we can, like, fix them?


Well, there's -ftrapv.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


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

2013-11-30 Thread Adrian Chadd
On 30 November 2013 15:25, Dimitry Andric  wrote:
> On 30 Nov 2013, at 14:56, Konstantin Belousov  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.
>
> I think this is quite a misrepresentation.  Any C compiler is free to do
> whatever it wants whenever it encounters undefined behavior.  Some
> behavior is undefined in the C standards, so compilers can do a better
> job at optimization.
>
> If the optimized code fails to do what the programmer thinks it does, it
> is almost always the programmer's fault, excluding actual compiler bugs
> (which are unavoidable, as all software has bugs).
>
> Basically, if you rely on undefined behavior, you are inventing your own
> de facto language, which is *not* C.  That is fine with me, but let's
> not pretend the FreeBSD kernel is written in C then. :-)

Are you able to have clang/llvm/gcc tell us where/when code is relying
on undefined behaviour? So we can, like, fix them?

If there was a way to lint this stuff then yes, please lint it.

Otherwise we don't have the tools to know whether we're doing sane
things or not.

(Same with things like strict aliasing..)


-adrian
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


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

2013-11-30 Thread Dimitry Andric
On 30 Nov 2013, at 14:56, Konstantin Belousov  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.

I think this is quite a misrepresentation.  Any C compiler is free to do
whatever it wants whenever it encounters undefined behavior.  Some
behavior is undefined in the C standards, so compilers can do a better
job at optimization.

If the optimized code fails to do what the programmer thinks it does, it
is almost always the programmer's fault, excluding actual compiler bugs
(which are unavoidable, as all software has bugs).

Basically, if you rely on undefined behavior, you are inventing your own
de facto language, which is *not* C.  That is fine with me, but let's
not pretend the FreeBSD kernel is written in C then. :-)


> 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.

Of course it is: the behavior is undefined, so the compiler is free to
randomly do anything.  Garbage in, garbage out.


> 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.

If this makes the life of kernel developers easier, so be it.  But I
still feel this is a bit of a cop-out, and we could also just fix the
bugs instead.

-Dimitry



signature.asc
Description: Message signed with OpenPGP using GPGMail


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

2013-11-30 Thread Poul-Henning Kamp
In message 
, Adrian Chadd writes:

>> The compiler authors take the undefined part there as a blanket to perform
>> optimizations which are assuming that signed overflow cannot happen.

That's sufficient explanation for me to support your proposal.

-- 
Poul-Henning Kamp   | UNIX since Zilog Zeus 3.20
p...@freebsd.org | TCP/IP since RFC 956
FreeBSD committer   | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


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

2013-11-30 Thread Adrian Chadd
+1, this caught us out with sendfile testing very recently :(


-a

On 30 November 2013 05:56, Konstantin Belousov  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
> @@ -148,6 +148,12 @@ 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" && \
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


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

2013-11-30 Thread Konstantin Belousov
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
@@ -148,6 +148,12 @@ 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" && \


pgpoK5Q5rquAX.pgp
Description: PGP signature