gettimeofday seconds rollover problem?

[Joel Sherrill]

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

This is the C code we are looking at the assembly for, correct?

  rtems_interrupt_disable(level);
    seconds      = _TOD_Seconds_since_epoch;
    microseconds = _TOD_Current.ticks;
  rtems_interrupt_enable(level);

and the assembly inlines for that are:

#define m68k_disable_interrupts( _level ) \
   do { register uint32_t   _tmpsr = 0x0700; \
        asm volatile ( "move.w %%sr,%0\n\t" \
                       "or.l   %0,%1\n\t" \
                       "move.w %1,%%sr" \
                       : "=d" (_level), "=d"(_tmpsr) : "1"(_tmpsr) ); \
   } while( 0 )
#define m68k_enable_interrupts( _level ) \
  asm volatile ( "move.w  %0,%%sr " : : "d" (_level));

Which compiler did you check that with? 

> Regards
> Chris