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

Reply via email to