gettimeofday seconds rollover problem?

[Chris Johns]

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