[PATCH] udhcpd: Handle auto_time timeout overflow

Rich Felker dalias at libc.org
Tue Jan 27 21:39:46 UTC 2015


On Tue, Jan 27, 2015 at 10:28:26PM +0100, Tim Hentenaar wrote:
> On Tue, Jan 27, 2015 at 09:51:29PM +0100, Denys Vlasenko wrote:
> > Hmm, I think it's a sign-extension bug. Can you try replacing
> > 
> > tv.tv_sec = timeout_end - monotonic_sec();
> > 
> > with
> > 
> > tv.tv_sec = (int)(timeout_end - monotonic_sec());
> > 
> > I suspect this will fix the behavior.
> 
> When I make that change, I get:
> 
>     movq    $0, -872(%rbp)  #, tv.tv_usec
>     subl    %eax, %ecx  # D.8486, D.8486
>     testl   %r14d, %r14d    #
>     movslq  %ecx, %rax  # D.8486,
>     movq    %rax, -880(%rbp)    # D.8494, tv.tv_sec
>     je  .L101   #,
>     testq   %rax, %rax  # D.8494
>     jle .L192   #,
> 
> Hmm... Looking at the assembly before the change, it's moving eax -> edx
> instead of sign-extending, while here (with the explicit cast) it
> sign-extends the result. It then generates the proper jump instruction
> to boot.
> 
> Perhaps it wrongly assumes that since the operands for the subtraction
> are 32-bit unsigned integers, that the result will be also unsigned. Then,
> the sign-extension gets optimized away. Then, making the cast explicit
> forces gcc to sign-extend the result.

This is not a wrong assumption. The result of unsigned subtraction is
unsigned, and it's computed via modular arithmetic.

The problem is that the semantics of the C code as written are wrong,
not anything weird done by the compiler in translating it.

Rich


More information about the busybox mailing list