gettimeofday seconds rollover problem?

[Ian Caddy]

Hi,

I had a look at this as well this morning and am concerned.  In our 
application, exactly the same thing happens as in Chris's example.  I 
had a quick look through the rest of our application and can't find the 
same problem in any other place, although sometimes it is a bit hard 
with the optimisation moving the code around alot.  That is with -O3.

I also built a debug version this morning (-O0) and the same problem 
does not appear in the gettimeofday code.

On another issue we are looking into a another Coldfire compiler bug 
with gcc 3.2.3 which is listed here:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=8309

Basically in some of our code, uint16 are used, and if local it is 
possible that they won't get incremented properly on Coldfire 
processors, and possibly modifying other variables instead on any 
optimisation level greater than 0.

We have tried to build newer versions of GCC (3.4.5), but haven't been 
able to get a magic combination of GCC, binutils and newlib to all work 
together nicely and build working code for us, so have gone back to 
using 3.2.3 with a patch that is described here:

http://gcc.gnu.org/ml/gcc/2004-05/msg01357.html

for reload.c which seems to have fixed our problem for us.

We will look more into this issue as well.

regards,

Ian Caddy


Chris Johns wrote:
> Joel Sherrill wrote:
>>
>> The seconds and nanoseconds values are grabbed/computed with interrupts
>> disabled so I don't think that would be the problem.
> 
> No they are not and it is the problem. The interrupts are being enable 
> just after they are being disabled. Here is the code:
> 
> 00000000 <gettimeofday>:
> gettimeofday():
> ../../../../../head/cpukit/libcsupport/src/__gettod.c:50
>    0:   4e56 0000       linkw %fp,#0
>    4:   206e 0008       moveal %fp@(8),%a0
> ../../../../../head/cpukit/libcsupport/src/__gettod.c:55
>    8:   4a88            tstl %a0
>    a:   6736            beqs 42 <gettimeofday+0x42>
> ../../../../../head/cpukit/libcsupport/src/__gettod.c:67
>    c:   203c 0000 0700  movel #1792,%d0
>   12:   2200            movel %d0,%d1
>   14:   40c0            movew %sr,%d0
>   16:   8280            orl %d0,%d1
>   18:   46c1            movew %d1,%sr
>                         ^^^^^^^^^^^^^ DISABLED
> ../../../../../head/cpukit/libcsupport/src/__gettod.c:70
>   1a:   46c0            movew %d0,%sr
>                         ^^^^^^^^^^^^^ ENABLED
> 
> The remainder of the code is the reading of the time:
> 
> ../../../../../head/cpukit/libcsupport/src/__gettod.c:72
>   1c:   2039 0000 0000  movel 0 <gettimeofday>,%d0
>   22:   0680 21da e500  addil #567993600,%d0
>   28:   2080            movel %d0,%a0@
> ../../../../../head/cpukit/libcsupport/src/__gettod.c:73
>   2a:   2239 0000 0000  movel 0 <gettimeofday>,%d1
>   30:   2039 0000 0000  movel 0 <gettimeofday>,%d0
>   36:   4c00 1800       mulsl %d0,%d1
>   3a:   2141 0004       movel %d1,%a0@(4)
>   3e:   4280            clrl %d0
>   40:   6010            bras 52 <gettimeofday+0x52>
> ../../../../../head/cpukit/libcsupport/src/__gettod.c:56
>   42:   4eb9 0000 0000  jsr 0 <gettimeofday>
>   48:   2040            moveal %d0,%a0
>   4a:   20bc 0000 000e  movel #14,%a0@
>   50:   70ff            moveq #-1,%d0
>   52:   4e5e            unlk %fp
>   54:   4e75            rts
> 
> I wonder where else in the kernel this is happening and to which other 
> targets. I see the i386 also has problems. In the i386 only one value is 
> being read with interrupts masked:
> 
>   rtems_interrupt_disable(level);
>    c:   9c                      pushf
>    d:   fa                      cli
>    e:   58                      pop    %eax
>  cpukit/libcsupport/src/__gettod.c:69
>     seconds      = _TOD_Seconds_since_epoch;
>     microseconds = _TOD_Current.ticks;
>    f:   8b 15 18 00 00 00       mov    0x18,%edx
>                                 ^^^^^^^^^^^^^^^^ ONLY ONE READ
> cpukit/libcsupport/src/__gettod.c:70
>   rtems_interrupt_enable(level);
>   15:   50                      push   %eax
>   16:   9d                      popf
> cpukit/libcsupport/src/__gettod.c:72
>   tp->tv_sec  = seconds + POSIX_TIME_SECONDS_1970_THROUGH_1988;
>   17:   a1 00 00 00 00          mov    0x0,%eax
>                                 ^^^^^^^^^^^^^^^ THE OTHER READ
>   1c:   05 00 e5 da 21          add    $0x21dae500,%eax
>   21:   89 01                   mov    %eax,(%ecx)
> 
>> The asm volatile statements
>> should provide sync points for the compiler if a partial read of 
>> memory were
>> an issue. 
> 
> If this is the case it would seem we have a problem with our asm 
> statements or the compiler has a bug.
> 
> Regards
> Chris
> 

-- 
Ian Caddy
Goanna Technologies Pty Ltd
+61 8 9221 1860