arithmetic overflow in VirtualizeTimerC

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Connect two TimerMilliC's (virtualized from the same HW timer) to a module. 
Upon firing, they should each toggle a separate LED.

2. Have that module start them both, one with t0=612 and dt=64, the other with 
t0=347, dt=256 (other values will also work, but this is an example)

3. Look at the LEDs.

What is the expected output? What do you see instead?
LED0 should toggle very fast, while LED1 toggles 4 times per second. LED1 
toggles much more than 4 times per second. 

The attached files show the behavior. defining the flag TIMER_BUG will use the 
version of fireTimers that has the bug, leaving it undefined will introduce the 
fix (check that now > timer->t0 ). 

The bare serial printf library (zipped) can be used to look at the way the 
virtual timers are set up (just connect to the mote with picocom or whatever). 
If you want to use this, uncomment the other lines in the makefile.

Tested on telosb.

Original issue reported on code.google.com by carlso...@gmail.com on 7 Feb 2012 at 7:03

Attachments:

• VirtualizeTimerC.nc
• Makefile
• TestP.nc
• TestAppC.nc
• serialprintf.zip

[GoogleCodeExporter]

It's been on my to-do list for a few months to investigate this issue further. 
We have observed the virtual timers causing the mcu to run flat out due to 
unnecessary firing/reposting, and had the oscilloscope hooked up to the LEDs to 
give us a better view of things. Unfortunately we got pulled off the task 
before we had a chance to nail down the exact location of the problem - we only 
got as far as "it's somewhere in the logic in VirtualizeTimerC.nc".

My immediate reaction to this fix is "what about the case where 'now' has just 
overflowed back to zero? won't that result in lost timer signals?" TEP102 
explicitly says that t0 is always assumed to be in the past, so there should 
not be a need to test against now at that point. I quote:
  "The time t0 is always assumed to be in the past. A value of t0 numerically greater than the current time (returned by getNow()) represents a time from before the last wraparound."

I'm not convinced that the attached fix is correct, but I'm very happy to see a 
reproducible test case exposing the issue (without the need for a CRO)!

Original comment by jmatts...@dius.com.au on 7 Feb 2012 at 11:42

[GoogleCodeExporter]

Fair point re: intentional overflows. If t0 is close to the rollover point, and 
we start handling the subtimer's fire just after 'now' has rolled over, then 
this would result in that virtual timer effectively stopping. 

Additionally, while not in the TEP, the docs for Timer.startPeriodicAt indicate 
that the intended behavior when t0 is in the past is for several events to fire 
until the timer "catches up." This modification does break that behavior (so 
applications that rely on it would not work as expected). 

I'll put this on the back burner and see if I can think of a better solution.

Original comment by carlso...@gmail.com on 8 Feb 2012 at 2:26