gettimeofday seconds rollover problem?

Thomas Doerfler Thomas.Doerfler at imd-systems.de
Thu Feb 23 09:11:23 UTC 2006


Hi,

wow, we really got a problem here. According to Andrews quoting, GCC can 
move around "rtems_disable_interrupt" and "rtems_enable_interrupt" in 
the current implementation (a macro that contains "volatile" asm), and 
therefore the usual way to block a short critical section does not work 
reliable.

Even making the _TOD* vairables volatile (which should be done anyway, I 
think), will not fix this problem, because their usage might still be 
moved out of the irq-disabled section.

One solution might be to reimplement rtems_dis/enable_interrupt as a 
(non-inline) function. Then GCC will have no idea what is going on in 
these functions, they might even change e.g. the global _TOD* variables 
(even when they are not volatile), and therefore GCC would be forced to 
read these variables exactly at the location specified in source code.

The advantage of this solution would be, that ALL critical sections 
useing rtems_dis/enable_interrupt would be save again. The disadvantage 
would be, that these statements will be executed slower due to the 
subroutine call and the register save/restore operations needed.

Any comments on my sugestion?

wkr,
Thomas.


Andrew Sinclair wrote:
> It seems to me that the assembler statements in m68k.h for enabling and
> disabling interrupts are never going to work with gcc optimisation.
> 
> Have a look at this article.
> 
> http://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Extended-Asm
>  
> Quoting 
> "
> The volatile keyword indicates that the instruction has important
> side-effects. GCC will not delete a volatile asm if it is reachable.
> (The instruction can still be deleted if GCC can prove that control-flow
> will never reach the location of the instruction.) Note that even a
> volatile asm instruction can be moved relative to other code, including
> across jump instructions. For example, on many targets there is a system
> register which can be set to control the rounding mode of floating point
> operations. You might try setting it with a volatile asm, like this
> PowerPC example: 
> 
>             asm volatile("mtfsf 255,%0" : : "f" (fpenv));
>             sum = x + y;
> 
> This will not work reliably, as the compiler may move the addition back
> before the volatile asm. To make it work you need to add an artificial
> dependency to the asm referencing a variable in the code you don't want
> moved, for example: 
> 
>          asm volatile ("mtfsf 255,%1" : "=X"(sum): "f"(fpenv));
>          sum = x + y;
> "
> 
> Nice trick, but how can we make it work for our situation? 
> 
> I am looking at setting values (eg. "memory") in the clobber field to
> prevent movement of the instruction. 
> 
> -----Original Message-----
> From: Ian Caddy [mailto:ianc at goanna.iinet.net.au] 
> Sent: Thursday, 23 February 2006 3:01 PM
> To: Andrew Sinclair
> Cc: Chris Johns; Joel Sherrill; rtems-users
> Subject: Re: gettimeofday seconds rollover problem?
> 
> Hmmm... looks like I will have to look deeper into our application.  I
> had a quick look through this morning, but on some functions such as the
> one you mentioned, the optimisation makes it a bit hard to follow on a
> quick run through.
> 
> I confirm the problem for rtems_clock_get, although in our case, one of
> the parameters is inside the disable interrupts, while the other part is
> not.  This is just as bad as both not being in there.  Again, I would
> say that changing tmp to be a volatile will fix the problem.
> 
> Throughout the rest of today I will try to look thoroughly through our
> application to see if I can find anymore and compile a list.
> 
> regards,
> 
> Ian Caddy
> 
> 
> Andrew Sinclair wrote:
> 
>>It looks like a similar issue exists with rtems_clock_get.
>>
>>I just wonder where else it is an issue...
>>
>>
>>-----Original Message-----
>>From: Ian Caddy [mailto:ianc at goanna.iinet.net.au]
>>Sent: Thursday, 23 February 2006 1:46 PM
>>To: Chris Johns
>>Cc: Joel Sherrill; Andrew Sinclair; rtems-users
>>Subject: Re: gettimeofday seconds rollover problem?
>>
>>On further analysis, it looks like the compiler is optimising out the 
>>two local variables, seconds and microseconds in gettimeofday.
>>
>>The reason for this is that the interrupt macros do not use these 
>>variables and so the compiler has no reason to ensure they are created
> 
> 
>>in the interrupt disabled state, since the compiler does not know that
> 
> 
>>was what we were trying to do.
>>
>>The simple solution is to make these two local variables volatile 
>>which ensures no optimisation of them:
>>
>>int gettimeofday(
>>   struct timeval  *tp,
>>   struct timezone *tzp
>>)
>>{
>>   rtems_interrupt_level level;
>>   volatile rtems_unsigned32      seconds;
>>   volatile rtems_unsigned32      microseconds;
>>
>>
>>This then creates the correct code, when -O3 is used:
>>
>>   rtems_interrupt_disable(level);
>>  8083f70:	203c 0000 0700 	movel #1792,%d0
>>  8083f76:	40c1           	movew %sr,%d1
>>  8083f78:	8081           	orl %d1,%d0
>>  8083f7a:	46c0           	movew %d0,%sr
>>     seconds      = _TOD_Seconds_since_epoch;
>>  8083f7c:	2279 0810 f4e2 	moveal 810f4e2
>><_TOD_Seconds_since_epoch>,%a1
>>  8083f82:	2f49 0004      	movel %a1,%sp@(4)
>>     microseconds = _TOD_Current.ticks;
>>  8083f86:	2eb9 0810 f5c6 	movel 810f5c6 <_TOD_Current+0x18>,%sp@
>>   rtems_interrupt_enable(level);
>>  8083f8c:	46c1           	movew %d1,%sr
>>
>>   tp->tv_sec  = seconds + POSIX_TIME_SECONDS_1970_THROUGH_1988;
>>  8083f8e:	222f 0004      	movel %sp@(4),%d1
>>  8083f92:	0681 21da e500 	addil #567993600,%d1
>>  8083f98:	2081           	movel %d1,%a0@
>>
>>
>>I hope this helps.
>>
>>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
>>
>>
>>
> 
> 
> --
> Ian Caddy
> Goanna Technologies Pty Ltd
> +61 8 9221 1860
> 
> 
> 


-- 
--------------------------------------------
IMD Ingenieurbuero fuer Microcomputertechnik
Thomas Doerfler           Herbststrasse 8
D-82178 Puchheim          Germany
email:    Thomas.Doerfler at imd-systems.de
PGP public key available at:
      http://www.imd-systems.de/pgpkey_en.html



More information about the users mailing list