xtimer: set now pointer correctly in _update_short_timers() loop

sys/xtimer/xtimer_core.c

@@ -169,7 +169,8 @@ static inline void _schedule_earliest_lltimer(uint32_t now)
 }
 
 /**
- * @brief compare two timers. return true if timerA expires earlier than or equal to timerB and false otherwise.
+ * @brief   compare two timers. return true if timerA expires earlier than or
+ *          equal to timerB and false otherwise.
  */
 static bool _timer_comparison(xtimer_t* timerA, xtimer_t* timerB, uint32_t now)
 {
@@ -278,7 +279,7 @@ static inline void _update_short_timers(uint64_t *now)
 
             /* make sure we don't fire too early */
             if (timer->offset > elapsed) {
-                while(_xtimer_now() - timer->start_time < timer->offset) {}
+                while (_xtimer_now() - timer->start_time < timer->offset) {}
             }
             /* advance list */
             timer_list_head = timer->next;
@@ -292,7 +293,7 @@ static inline void _update_short_timers(uint64_t *now)
             /* assign new head */
             timer = timer_list_head;
             /* update current_time */
-            *now = _xtimer_now();
+            *now = _xtimer_now64();
         }
         else {
             timer->offset -= elapsed;

tests/xtimer_now32_overflow/Makefile

@@ -0,0 +1,8 @@
+include ../Makefile.tests_common
+
+USEMODULE += fmt
+USEMODULE += xtimer
+
+DISABLE_MODULE += auto_init_xtimer
+
+include $(RIOTBASE)/Makefile.include

tests/xtimer_now32_overflow/README.md

@@ -0,0 +1,6 @@
+Description
+===========
+
+This is a regression test for when `xtimer_now64()` becomes greater than
+`UINT32_MAX` and the internal state machine fails to update the timers state
+correctly.

tests/xtimer_now32_overflow/main.c

@@ -0,0 +1,91 @@
+/*
+ * Copyright (C) 2020 Freie Universität Berlin
+ *
+ * This file is subject to the terms and conditions of the GNU Lesser
+ * General Public License v2.1. See the file LICENSE in the top level
+ * directory for more details.
+ */
+
+/**
+ * @{
+ *
+ * @file
+ * @author  Martine Lenders <m.lenders@fu-berlin.de>
+ */
+
+#include <stdio.h>
+
+#include "fmt.h"
+#include "kernel_defines.h"
+#include "msg.h"
+#include "test_utils/expect.h"
+#include "thread.h"
+#include "xtimer.h"
+
+#define MAIN_MSG_QUEUE_SIZE     (4U)
+#define TIMERS_NUMOF            (3U)
+
+msg_t _main_msg_queue[MAIN_MSG_QUEUE_SIZE];
+static const uint64_t _timers_offsets[TIMERS_NUMOF] = {
+    /* MUST ASCEND */
+    1 * US_PER_SEC,
+    2 * US_PER_SEC,
+    3 * US_PER_SEC,
+};
+
+int main(void)
+{
+    xtimer_t timers[TIMERS_NUMOF];
+    msg_t msgs[TIMERS_NUMOF];
+    uint64_t start;
+
+    expect(ARRAY_SIZE(_timers_offsets) == TIMERS_NUMOF);
+    msg_init_queue(_main_msg_queue, MAIN_MSG_QUEUE_SIZE);
+    /* ensure that xtimer_now64() is greater than UINT32_MAX */
+    _xtimer_current_time = (2LLU << 32U);
+    xtimer_init();
+    print_str("Setting ");
+    print_u32_dec(TIMERS_NUMOF);
+    print_str(" timers:\n");
+    for (unsigned i = 0; i < TIMERS_NUMOF; i++) {
+        msgs[i].content.value = i;
+        print_str(" #");
+        print_u32_dec(i);
+        print_str(" in ");
+        print_u64_dec(_timers_offsets[i]);
+        print_str(" usec\n");
+    }
+    print_str("now=");
+    start = xtimer_now64().ticks64;
+    print_u64_dec(start);
+    print_str("\n");
+    expect(start > UINT32_MAX);
+    /* set timers after all were printed for better timing */
+    for (unsigned i = 0; i < TIMERS_NUMOF; i++) {
+        xtimer_set_msg64(&timers[i], _timers_offsets[i], &msgs[i],
+                         thread_getpid());
+        expect(timers[i].long_start_time > 0);
+    }
+    while (1) {
+        msg_t msg;
+
+        msg_receive(&msg);
+        print_str("#");
+        print_u32_dec(msg.content.value);
+        print_str(":now=");
+        print_u64_dec((uint64_t)xtimer_now64().ticks64);
+        print_str("\n");
+        for (unsigned i = 0; i <= msg.content.value; i++) {
+            /* all fired timers expired */
+            expect(timers[i].long_start_time == 0);
+        }
+        for (unsigned i = (msg.content.value + 1); i <= TIMERS_NUMOF; i++) {
+            /* upper half of remaing_timers' start_time stays above 0 as it is
+             * based on xtimer_now64() during the timer's callback execution */
+            expect(timers[i].long_start_time > 0);
+        }
+    }
+    return 0;
+}
+
+/** @} */

tests/xtimer_now32_overflow/tests/01-run.py

@@ -0,0 +1,48 @@
+#!/usr/bin/env python3
+
+# Copyright (C) 2018 Freie Universität Berlin
+#
+# This file is subject to the terms and conditions of the GNU Lesser
+# General Public License v2.1. See the file LICENSE in the top level
+# directory for more details.
+
+import sys
+import time
+from testrunner import run
+
+
+def _get_largest_timeout_difference(timeouts):
+    next_timeout = min(timeouts)
+    # get largest difference between all timeouts
+    timeout = max(a - b for a in timeouts for b in timeouts)
+    # check if smallest timeout (difference to 0) is the largest difference
+    if timeout < next_timeout:
+        timeout = next_timeout
+    return timeout
+
+
+def testfunc(child):
+    timers = {}
+    child.expect(r"Setting (\d+) timers:")
+    timers_numof = int(child.match.group(1))
+    for i in range(timers_numof):
+        child.expect(r" #(\d+) in (\d+) usec")
+        assert i == int(child.match.group(1))
+        timers[i] = int(child.match.group(2))
+    assert timers_numof == len(timers)
+    check_time = int(time.time())
+    child.expect(r"now=(\d+)")
+    offset = int(child.match.group(1))
+    # get largest possible timeout for expects below
+    timeout = _get_largest_timeout_difference(timers.values()) / (10**6) + 1
+    for i in range(timers_numof):
+        child.expect(r"#(\d):now=(\d+)", timeout=timeout)
+        t = int(child.match.group(1))
+        now = int(child.match.group(2))
+        assert (int(time.time()) - check_time) >= (timers[t] / 10**6)
+        expected = timers[t] + offset
+        assert expected <= now
+
+
+if __name__ == "__main__":
+    sys.exit(run(testfunc))