Date: Tue, 2 May 2017 11:34:38 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Nick Hibma <nick@van-laarhoven.org> Cc: FreeBSD Current Mailing List <freebsd-current@FreeBSD.ORG> Subject: Re: Compiler optimisation bug Message-ID: <20170502083438.GA1622@kib.kiev.ua> In-Reply-To: <3626A28E-9DF5-4775-BAAB-CC6C36D2CD01@van-laarhoven.org> References: <3626A28E-9DF5-4775-BAAB-CC6C36D2CD01@van-laarhoven.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, May 02, 2017 at 10:26:17AM +0200, Nick Hibma wrote: > There is a bug in sbin/dhclient.c for large expiry values on 32 bit platforms where time_t is a uint32_t (bug #218980, https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=218980). It is caused by a compiler optimisation that removes an if-statement. The code below shows the following output, clearly showing that the optimised case provides a different answer: > > > % cc -O2 main.c -o main.a && ./main.a > no opt: 0x7fffffff > with opt: 0xfffffffe > rephrased: 0x7fffffff > > The code is as follows: > > % cat main.c > #include <stdio.h> > #include <sys/time.h> > #define TIME_MAX 2147483647 > > time_t a = TIME_MAX; > time_t b = TIME_MAX; > > time_t > add_noopt(time_t a, time_t b) __attribute__((optnone)) > { > a += b; > if (a < b) > a = TIME_MAX; This is a canonical example of the undefined behaviour. Compiler authors consider that they have a blanket there, because the C standard left signed integer overflow as undefined behaviour, to allow implementation using native signed representation. Instead, they abuse the permit to gain 0.5% in some unimportant benchmarks. > return a; > } > > time_t > add_withopt(time_t a, time_t b) > { > a += b; > if (a < b) > a = TIME_MAX; > return a; > } > > time_t > add_rephrased(time_t a, time_t b) > { > if (a < 0 || a > TIME_MAX - b) > a = TIME_MAX; > else > a += b; > return a; > } > > int > main(int argc, char **argv) > { > printf("no opt: 0x%08x\n", add_noopt(a, b)); > printf("with opt: 0x%08x\n", add_withopt(a, b)); > printf("rephrased: 0x%08x\n", add_rephrased(a, b)); > } > > Should this be reported to the clang folks? Or is this to be expected when abusing integer overflows this way? You will get an answer that this is expected. Add -fwrapv compiler flag to make signed arithmetic behave in a way different from the mine-field, or remove the code. For kernel, we use -fwrapv. > > Also: The underlying problem is the fact that time_t is a 32 bit signed int. Any pointers as to a general method of resolving these time_t issues? > > Regards, > > Nick Hibma > nick@van-laarhoven.org > > -- Open Source: We stand on the shoulders of giants. > > >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170502083438.GA1622>