Hi Denys,

On Sun, Jan 25, 2015 at 09:58:50PM +0100, Denys Vlasenko wrote:
> I don't understand why current code does not reach write_leases().
> If tv.tv_sec is negative, we should not reach select() here -
>
>                 retval = 0;
>                 if (!server_config.auto_time || tv.tv_sec > 0) {
>                         retval = select(max_sock + 1, &rfds, NULL, NULL,
>                                         server_config.auto_time ? &tv : NULL);
>                 }
>                 if (retval == 0) {
>                         write_leases();
>                         goto continue_with_autotime;
>                 }
>
> and therefore, retval == 0 and we should reach write_leases().
>
> What am I missing?

I wondered the same thing at first. Here's the relevent code as
generated by GCC. Interestingly, the "tv_sec > 0" check seems to get
optimized out (I'm compiling with -O2), and select() gets called as long
as tv_sec is non-zero. Looks almost as if it's treating tv_sec as
unsigned...

In my .config, I've got:

  CONFIG_EXTRA_CFLAGS="-O2 -fPIE -pie"

Code generated with -O2:
        ; bb_common_bufsiz1+0x34 = server_config.auto_time -> %r15d
   177d7:       44 8b 3d 96 ec 26 00    mov    0x26ec96(%rip),%r15d
   177de:       89 c3                   mov    %eax,%ebx
   177e0:       45 85 ff                test   %r15d,%r15d
   177e3:       75 26                   jne    1780b <udhcpd_main+0x30b>

   ; if (!server_config.auto_time) { select(...); }
   177e5:       45 31 c0                xor    %r8d,%r8d
   177e8:       48 8b b5 60 fc ff ff    mov    -0x3a0(%rbp),%rsi
   177ef:       8d 7b 01                lea    0x1(%rbx),%edi
   177f2:       31 c9                   xor    %ecx,%ecx
   177f4:       31 d2                   xor    %edx,%edx
   177f6:       e8 b5 08 ff ff          callq  80b0 <select@plt>

   ; if (retval == 0)
   ;     write leases
   177fb:       85 c0                   test   %eax,%eax
   177fd:       75 4a                   jne    17849 <udhcpd_main+0x349>

   177ff:       e8 e5 88 03 00          callq  500e9 <write_leases>
   17804:       eb 85                   jmp    1778b <udhcpd_main+0x28b>
   17806:       e8 85 09 ff ff          callq  8190 <__stack_chk_fail@plt>

   ; if (server_config.auto_time) {
   1780b:       0f 1f 44 00 00          nopl   0x0(%rax,%rax,1)
   17810:       e8 3b 2c ff ff          callq  a450 <monotonic_sec>

   ; timeout_end -> %ecx
   17815:       8b 8d 68 fc ff ff       mov    -0x398(%rbp),%ecx

   ; server_config.auto_time -> %r14d
   1781b:       44 8b 35 52 ec 26 00    mov    0x26ec52(%rip),%r14d

   ; tv.tv_usec = 0
   17822:       48 c7 85 98 fc ff ff    movq   $0x0,-0x368(%rbp)
   17829:       00 00 00 00

   ; tv.tv_sec = timeout_end - monotonic_time()
   1782d:       29 c1                   sub    %eax,%ecx
   1782f:       45 85 f6                test   %r14d,%r14d
   17832:       48 89 8d 90 fc ff ff    mov    %rcx,-0x370(%rbp)

   ; if (!server_config.auto_time)
   ;     go do select() with NULL timeout
   17839:       74 aa                   je     177e5 <udhcpd_main+0x2e5>

   ; if (!tv.tv_sec)
   ;     go write leases
   1783b:       48 85 c9                test   %rcx,%rcx
   1783e:       74 bf                   je     177ff <udhcpd_main+0x2ff>

   ; Setup timeout param for select() and go do it
   17840:       4c 8d 85 90 fc ff ff    lea    -0x370(%rbp),%r8
   17847:       eb 9f                   jmp    177e8 <udhcpd_main+0x2e8>

Recompiling with -O0 yields:

   ; If (!server_config.auto_time)
   3055f:       48 8d 05 da 4e 28 00    lea    0x284eda(%rip),%rax
   30566:       8b 40 34                mov    0x34(%rax),%eax
   30569:       85 c0                   test   %eax,%eax
   3056b:       74 0c                   je     30579 <udhcpd_main+0x446>

   ; if !server_config.auto_time && tv.tv_sec <= 0 then write leases
   3056d:       48 8b 85 b0 fc ff ff    mov    -0x350(%rbp),%rax
   30574:       48 85 c0                test   %rax,%rax
   30577:       7e 44                   jle    305bd <udhcpd_main+0x48a>

   ; if (!server_config.auto_time)
   ;    go set timeout to NULL
   30579:       48 8d 05 c0 4e 28 00    lea    0x284ec0(%rip),%rax
   30580:       8b 40 34                mov    0x34(%rax),%eax
   30583:       85 c0                   test   %eax,%eax
   30585:       74 09                   je     30590 <udhcpd_main+0x45d>

   ; else: use tv as timeout for select()
   30587:       48 8d 85 b0 fc ff ff    lea    -0x350(%rbp),%rax
   3058e:       eb 05                   jmp    30595 <udhcpd_main+0x462>

   ; timeout = NULL
   30590:       b8 00 00 00 00          mov    $0x0,%eax

   ; Setup select parameters
   30595:       8b 95 58 fc ff ff       mov    -0x3a8(%rbp),%edx
   3059b:       8d 7a 01                lea    0x1(%rdx),%edi
   3059e:       48 8d b5 f0 fc ff ff    lea    -0x310(%rbp),%rsi
   305a5:       49 89 c0                mov    %rax,%r8
   305a8:       b9 00 00 00 00          mov    $0x0,%ecx
   305ad:       ba 00 00 00 00          mov    $0x0,%edx
   305b2:       e8 99 7b fd ff          callq  8150 <select@plt>
   305b7:       89 85 40 fc ff ff       mov    %eax,-0x3c0(%rbp)

   ; if (reval == 0)
   ;    write leases
   305bd:       83 bd 40 fc ff ff 00    cmpl   $0x0,-0x3c0(%rbp)
   305c4:       75 0a                   jne    305d0 <udhcpd_main+0x49d>
   305c6:       e8 55 13 00 00          callq  31920 <write_leases>
   305cb:       e9 ef fe ff ff          jmpq   304bf <udhcpd_main+0x38c>

With my patch applied, the proper check gets generated with -O2:

   ; tv.tv_sec = timeout_end - monotonic_sec()
   1782b:       29 c2                   sub    %eax,%edx
   1782d:       89 d0                   mov    %edx,%eax
   1782f:       48 89 85 90 fc ff ff    mov    %rax,-0x370(%rbp)

   ; if ((unsigned) tv.tv_sec > server_config.auto_time)
   17836:       8b 05 38 ec 26 00       mov    0x26ec38(%rip),%eax
   1783c:       39 c2                   cmp    %eax,%edx
   1783e:       76 0b                   jbe    1784b <udhcpd_main+0x34b>

   ; tv.tv_sec = 0
   17840:       48 c7 85 90 fc ff ff    movq   $0x0,-0x370(%rbp)
   17847:       00 00 00 00

   ; if (!server_config.auto_time)
   ;    do select()
   1784b:       85 c0                   test   %eax,%eax
   1784d:       74 98                   je     177e7 <udhcpd_main+0x2e7>

   ; If tv.tv_sec <= 0, write leases
   1784f:       48 83 bd 90 fc ff ff    cmpq   $0x0,-0x370(%rbp)
   17856:       00
   17857:       7e a8                   jle    17801 <udhcpd_main+0x301>

Here's the version of GCC (and target) I'm using:

Target: x86_64-gentoo-linux-uclibc
gcc version 4.8.3 (Gentoo Hardened 4.8.3 p1.1, pie-0.5.9)

Tim

Attachment: pgpRY4A3Qsjtx.pgp
Description: PGP signature

_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to