Re: Compiler optimisation bug
>> 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. Thanks, that was what I expected. I searched for -fwrapv and found similar comments. The code has been rewritten to not depend on overflow for its checks, so it works properly with any sized time_t (assuming that it is an integer though :). I'll commit it after feedback. Nick signature.asc Description: Message signed with OpenPGP
Re: Compiler optimisation bug
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: 0x7fff
> with opt: 0xfffe
> rephrased: 0x7fff
>
> The code is as follows:
>
> % cat main.c
> #include
> #include
> #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
> n...@van-laarhoven.org
>
> -- Open Source: We stand on the shoulders of giants.
>
>
>
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Compiler optimisation bug
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: 0x7fff
with opt: 0xfffe
rephrased: 0x7fff
The code is as follows:
% cat main.c
#include
#include
#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;
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?
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
n...@van-laarhoven.org
-- Open Source: We stand on the shoulders of giants.
signature.asc
Description: Message signed with OpenPGP
