alarm: fix new alarm insertion when one of the alarms overflows

Fixes #466

When inserting an alarm into the virtual alarm queue, comparing two
alarms needs to account for alarms that may wrap beyond the clock
overflow. Simply comparing computed expirations against the new
alarm's reference isn't sufficient, as the current alarm may overflow,
resulting in an expiration that is earlies by absolute value, but
should still expire later.

examples/tests/multi_alarm_simple_overflow_test/Makefile

@@ -0,0 +1,11 @@
+# Makefile for user application
+
+# Specify this directory relative to the current application.
+TOCK_USERLAND_BASE_DIR = ../../..
+
+# Which files to compile.
+C_SRCS := $(wildcard *.c)
+
+# Include userland master makefile. Contains rules and flags for actually
+# building the application.
+include $(TOCK_USERLAND_BASE_DIR)/AppMakefile.mk

examples/tests/multi_alarm_simple_overflow_test/README.md

@@ -0,0 +1,13 @@
+# Test Multiple Alarms (With Overflow)
+
+This tests the virtual alarms available to userspace. It sets two
+alarms, first one that overflows the alarm, such that it's expiration
+is small in absolute value (but should shouldn't fire until after the
+clock overflows) and one after 1s. When successful, the second (1s)
+alarm should fire after 1 second, while the second alarm should fire
+after the clock overflows (approximately 7 minutes if the clock is at
+32kHz).
+
+If the virtual alarm library is buggy, this test might fail by
+scheduling the 1 second alarm _after_ the longer wrapping alarm, and
+it won't fire immediately.

examples/tests/multi_alarm_simple_overflow_test/main.c

@@ -0,0 +1,26 @@
+#include <stdio.h>
+#include <stdlib.h>
+
+#include <libtock-sync/services/alarm.h>
+
+static void event_cb(uint32_t now, uint32_t expiration, void* ud) {
+  int i = (int)ud;
+  printf("%d %lu %lu\n", i, now, expiration);
+}
+
+int main(void) {
+  libtock_alarm_t t1;
+  libtock_alarm_t t2;
+
+  uint32_t now;
+  libtock_alarm_command_read(&now);
+
+  uint32_t overflow_ms = libtock_alarm_ticks_to_ms(UINT32_MAX);
+
+  libtock_alarm_in_ms(overflow_ms, event_cb, (void*)1, &t1);
+  libtock_alarm_in_ms(1000, event_cb, (void*)2, &t2);
+
+  while (1) {
+    yield();
+  }
+}

examples/tests/multi_alarm_simple_test/main.c

@@ -5,7 +5,7 @@
 
 static void event_cb(uint32_t now, uint32_t expiration, void* ud) {
   int i = (int)ud;
-  printf("%d %ld %ld\n", i, now, expiration);
+  printf("%d %lu %lu\n", i, now, expiration);
 }
 
 int main(void) {

libtock/services/alarm.c

@@ -5,11 +5,6 @@
 
 #define MAX_TICKS UINT32_MAX
 
-// Returns 0 if a <= b < c, 1 otherwise
-static int within_range(uint32_t a, uint32_t b, uint32_t c) {
-  return (b - a) < (b - c);
-}
-
 /** \brief Convert milliseconds to clock ticks
  *
  * WARNING: This function will assert if the output
@@ -40,7 +35,7 @@ static uint32_t ms_to_ticks(uint32_t ms) {
   return ticks;
 }
 
-static uint32_t ticks_to_ms(uint32_t ticks) {
+uint32_t libtock_alarm_ticks_to_ms(uint32_t ticks) {
   // `ticks_to_ms`'s conversion will be accurate to within the range
   // 0 to 1 milliseconds less than the exact conversion
   // (true millisecond conversion - [0,1) milliseconds).
@@ -75,19 +70,44 @@ static uint32_t ticks_to_ms(uint32_t ticks) {
 static libtock_alarm_ticks_t* root = NULL;
 
 static void root_insert(libtock_alarm_ticks_t* alarm) {
+  // We want to insert the new alarm just before an existing alarm
+  // that should expire next after it, as `alarm_upcall` breaks as
+  // soon as it finds an alarm that should not fire yet. To do so, we
+  // need to account for an empty list, a non-empty list where the new
+  // alarm should be last, as well as computing relative expirations
+  // when alarm expirations may overflow the clock 0 or 1 times.
+
   if (root == NULL) {
     root       = alarm;
     root->next = NULL;
     root->prev = NULL;
     return;
   }
 
+  // Compute the tick value at which the new alarm will expire.
+  uint32_t new_expiration = alarm->reference + alarm->dt;
+  // Determine whether the clock overflows before the new alarm
+  // expires. Because ticks are 32-bit, a clock can overflow at most
+  // once before a 32-bit alarm fires.
+  bool new_overflows = alarm->reference > UINT32_MAX - alarm->dt;
+
   libtock_alarm_ticks_t** cur = &root;
   libtock_alarm_ticks_t* prev = NULL;
   while (*cur != NULL) {
+    // Compute the tick value at which this alarm will expire.
     uint32_t cur_expiration = (*cur)->reference + (*cur)->dt;
-    uint32_t new_expiration = alarm->reference + alarm->dt;
-    if (!within_range(alarm->reference, cur_expiration, new_expiration)) {
+    // Determine whether the clock overflows before this alarm
+    // expires.
+    bool cur_overflows = (*cur)->reference > UINT32_MAX - (*cur)->dt;
+
+    // This alarm happens after the new alarm if:
+    // - neither expirations overflow and this expiration value is larger than the new expiration
+    // - both overflow and this expiration value is larger than the new expiration
+    // - or, this alarm overflows but the new one doesn't
+    //
+    // If the new alarm overflows and this alarm doesn't, this alarm
+    // happens _before_ the new alarm.
+    if (!(!cur_overflows && new_overflows) && ((cur_overflows && !new_overflows) || cur_expiration > new_expiration)) {
       // insert before
       libtock_alarm_ticks_t* tmp = *cur;
       *cur        = alarm;
@@ -231,7 +251,7 @@ int libtock_alarm_in_ms(uint32_t ms, libtock_alarm_callback cb, void* opaque, li
   // schedule multiple alarms to reach the full length. We calculate the number of full overflows
   // and the remainder ticks to reach the target length of time. The overflows use the
   // `overflow_callback` for each intermediate overflow.
-  const uint32_t max_ticks_in_ms = ticks_to_ms(MAX_TICKS);
+  const uint32_t max_ticks_in_ms = libtock_alarm_ticks_to_ms(MAX_TICKS);
   if (ms > max_ticks_in_ms) {
     // overflows_left is the number of intermediate alarms that need to be scheduled to reach the target
     // dt_ms. After the alarm in this block is scheduled, we have this many overflows left (hence the reason

libtock/services/alarm.h

@@ -31,6 +31,10 @@ extern "C" {
 // - `arg3` (`opaque`): An arbitrary user pointer passed back to the callback.
 typedef void (*libtock_alarm_callback)(uint32_t, uint32_t, void*);
 
+/** \brief Utility function for converting hardware-specific tics to milliseconds
+ */
+uint32_t libtock_alarm_ticks_to_ms(uint32_t);
+
 /** \brief Opaque handle to a single-shot alarm.
  *
  * An opaque handle to an alarm created by `alarm_at` or `alarm_in`. Memory