From 2f9bfeb73f3df4fb5a807f3ee571aee700edc0a3 Mon Sep 17 00:00:00 2001 From: JP Hutchins Date: Thu, 24 Sep 2026 11:56:20 -0700 Subject: [PATCH 1/5] sys: time_units: evaluate conversion arguments once The conversion macros are expressions so that constant arguments give integer constant expressions for static initializers, see commit 8f5d0791bfc5 ("sys/time_units.h: Convert time conversion to macros"). Three of the leaf expressions name an operand more than once: - z_tmcvt_int_div_32() reads the input for its range check and again for the division. - z_tmcvt_gen_64_slow() reads it for the quotient and again for the remainder. - z_tmcvt_gen_32() and z_tmcvt_gen_64_slow() read the source frequency up to four times, which is a system call with CONFIG_TIMER_READS_ITS_FREQUENCY_AT_RUNTIME. A function call in the argument is made that many times, and two reads of a running clock can disagree. At 32768 Hz ticks, k_ticks_to_ns_floor64(k_uptime_ticks()) takes the slow path and returns a time one second early when the reads straddle a second boundary. k_uptime_seconds() calls k_uptime_ticks() twice in every configuration. Wrap the three leaves in z_tmcvt_choose(). When every operand is a compile-time constant it selects the existing expression, so the result is still an integer constant expression; otherwise the operands are passed once to an always-inline function that evaluates the same expression. C uses __builtin_choose_expr(): with ?:, a dead arm that calls a function makes GCC report a file scope array sized by the result as variably modified. C++ uses ?:, since constant evaluation ignores the dead arm. Toolchains without the builtins keep the expression form. GCC does not treat a const variable as constant here, so it can no longer be the argument in a static initializer. A scan of the tree found no such use. Tested with the tests/unit/time_units cases added later in this series, run at 1000, 10000 and 32768 Hz ticks and with 32-bit timeouts, and host builds with GCC and Clang counting evaluations of the argument for all conversion macros. The full series was also built on native_sim for tests/kernel, tests/lib/cpp, tests/drivers/build_all and tests/posix. Assisted-by: Claude:claude-opus-5-5 --- include/zephyr/sys/time_units.h | 75 +++++++++++++++++++++++++++------ 1 file changed, 61 insertions(+), 14 deletions(-) diff --git a/include/zephyr/sys/time_units.h b/include/zephyr/sys/time_units.h index 642ddf008fb4..376ecb46029c 100644 --- a/include/zephyr/sys/time_units.h +++ b/include/zephyr/sys/time_units.h @@ -130,6 +130,25 @@ static inline unsigned int z_impl_sys_clock_hw_cycles_per_sec_runtime_get(void) * the results may be used to initialize global variables. */ +/* Constant operands take the expression form, which stays an integer constant + * expression; others are evaluated once as function arguments. + */ +#if defined(__cplusplus) && HAS_BUILTIN(__builtin_constant_p) +#define z_tmcvt_const_arg(__x) __builtin_constant_p(__x) +#define z_tmcvt_choose(__is_const, __expr, __once) ((__is_const) ? (__expr) : (__once)) +#elif !defined(__cplusplus) && HAS_BUILTIN(__builtin_constant_p) && \ + HAS_BUILTIN(__builtin_choose_expr) +#define z_tmcvt_const_arg(__x) __builtin_constant_p(__x) +#define z_tmcvt_choose(__is_const, __expr, __once) __builtin_choose_expr(__is_const, __expr, __once) +#else +#define z_tmcvt_const_arg(__x) true +#define z_tmcvt_choose(__is_const, __expr, __once) (__expr) +#endif + +#define z_tmcvt_is_const(__t, __from_hz, __to_hz) \ + (z_tmcvt_const_arg(__t) && z_tmcvt_const_arg(__from_hz) && \ + z_tmcvt_const_arg(__to_hz)) + /* true if the conversion is the identity */ #define z_tmcvt_is_identity(__from_hz, __to_hz) \ ((__to_hz) == (__from_hz)) @@ -182,29 +201,45 @@ static inline unsigned int z_impl_sys_clock_hw_cycles_per_sec_runtime_get(void) 0) /* Integer division 32-bit conversion */ +#define z_tmcvt_int_div_32_expr(__t, __off, __divisor) \ + ((uint64_t) (__t) <= 0xffffffffU - (__off) ? \ + (uint32_t) (((__t) + (__off)) / (__divisor)) : \ + (uint32_t) (((uint64_t) (__t) + (__off)) / (__divisor))) + +static ALWAYS_INLINE uint32_t z_tmcvt_int_div_32_once(uint64_t t, uint32_t off, uint32_t divisor) +{ + return z_tmcvt_int_div_32_expr(t, off, divisor); +} + #define z_tmcvt_int_div_32(__t, __from_hz, __to_hz, __round_up, __round_off) \ - ((uint64_t) (__t) <= 0xffffffffU - \ - z_tmcvt_off_div(__from_hz, __to_hz, __round_up, __round_off) ? \ - ((uint32_t)(((__t) + \ - z_tmcvt_off_div(__from_hz, __to_hz, \ - __round_up, __round_off)) / \ - z_tmcvt_divisor(__from_hz, __to_hz))) \ - : \ - (uint32_t) (((uint64_t) (__t) + \ - z_tmcvt_off_div(__from_hz, __to_hz, \ - __round_up, __round_off)) / \ - z_tmcvt_divisor(__from_hz, __to_hz)) \ - ) + z_tmcvt_choose(z_tmcvt_const_arg(__t), \ + z_tmcvt_int_div_32_expr(__t, \ + z_tmcvt_off_div(__from_hz, __to_hz, __round_up, __round_off), \ + z_tmcvt_divisor(__from_hz, __to_hz)), \ + z_tmcvt_int_div_32_once(__t, \ + z_tmcvt_off_div(__from_hz, __to_hz, __round_up, __round_off), \ + z_tmcvt_divisor(__from_hz, __to_hz))) /* Integer multiplication 32-bit conversion */ #define z_tmcvt_int_mul_32(__t, __from_hz, __to_hz) \ ((uint32_t) ((__t)*((__to_hz) / (__from_hz)))) /* General 32-bit conversion */ -#define z_tmcvt_gen_32(__t, __from_hz, __to_hz, __round_up, __round_off) \ +#define z_tmcvt_gen_32_expr(__t, __from_hz, __to_hz, __round_up, __round_off) \ ((uint32_t) (((uint64_t) (__t)*(__to_hz) + \ z_tmcvt_off_gen(__from_hz, __to_hz, __round_up, __round_off)) / (__from_hz))) +static ALWAYS_INLINE uint32_t z_tmcvt_gen_32_once(uint64_t t, uint32_t from_hz, uint32_t to_hz, + bool round_up, bool round_off) +{ + return z_tmcvt_gen_32_expr(t, from_hz, to_hz, round_up, round_off); +} + +#define z_tmcvt_gen_32(__t, __from_hz, __to_hz, __round_up, __round_off) \ + z_tmcvt_choose(z_tmcvt_is_const(__t, __from_hz, __to_hz), \ + z_tmcvt_gen_32_expr(__t, __from_hz, __to_hz, __round_up, __round_off), \ + z_tmcvt_gen_32_once(__t, __from_hz, __to_hz, __round_up, __round_off)) + /* Integer division 64-bit conversion */ #define z_tmcvt_int_div_64(__t, __from_hz, __to_hz, __round_up, __round_off) \ (((uint64_t) (__t) + z_tmcvt_off_div(__from_hz, __to_hz, \ @@ -221,11 +256,23 @@ static inline unsigned int z_impl_sys_clock_hw_cycles_per_sec_runtime_get(void) z_tmcvt_off_gen(__from_hz, __to_hz, __round_up, __round_off)) / (__from_hz)) /* Slow 64-bit conversion. This avoids overflowing the multiply */ -#define z_tmcvt_gen_64_slow(__t, __from_hz, __to_hz, __round_up, __round_off) \ +#define z_tmcvt_gen_64_slow_expr(__t, __from_hz, __to_hz, __round_up, __round_off) \ ((((uint64_t) (__t) / (__from_hz))*(__to_hz)) + \ (((((uint64_t) (__t) % (__from_hz))*(__to_hz)) + \ z_tmcvt_off_gen(__from_hz, __to_hz, __round_up, __round_off)) / (__from_hz))) +static ALWAYS_INLINE uint64_t z_tmcvt_gen_64_slow_once(uint64_t t, uint32_t from_hz, + uint32_t to_hz, bool round_up, + bool round_off) +{ + return z_tmcvt_gen_64_slow_expr(t, from_hz, to_hz, round_up, round_off); +} + +#define z_tmcvt_gen_64_slow(__t, __from_hz, __to_hz, __round_up, __round_off) \ + z_tmcvt_choose(z_tmcvt_is_const(__t, __from_hz, __to_hz), \ + z_tmcvt_gen_64_slow_expr(__t, __from_hz, __to_hz, __round_up, __round_off), \ + z_tmcvt_gen_64_slow_once(__t, __from_hz, __to_hz, __round_up, __round_off)) + /* General 64-bit conversion. Uses one of the two above macros */ #define z_tmcvt_gen_64(__t, __from_hz, __to_hz, __round_up, __round_off) \ (z_tmcvt_use_fast_algo(__from_hz, __to_hz) ? \ From ba7029c536dd2237bc9f4705aadea6780dadae0b Mon Sep 17 00:00:00 2001 From: JP Hutchins Date: Thu, 24 Sep 2026 11:56:20 -0700 Subject: [PATCH 2/5] sys: clock: evaluate timeout arguments once Z_TIMEOUT_MS(), Z_TIMEOUT_US(), Z_TIMEOUT_NS(), Z_TIMEOUT_CYC() and Z_TIMEOUT_MS_TICKS(), and therefore K_MSEC() and the other K_*() timeout macros, clamp negative arguments with MAX(t, 0). That evaluates the argument once for the comparison and again for the result. When the argument reads a clock, a deadline that passes between the two reads lets a negative value through the clamp. K_MSEC(next_cycle - k_uptime_get()) in subsys/net/lib/mqtt_sn/mqtt_sn.c then yields K_TICKS_FOREVER at 1000 Hz ticks, and about 1.8e16 ticks at 32768 Hz. Clamp through z_tmcvt_choose(): constant arguments keep MAX(t, 0), so the macros still produce constant expressions, and other arguments go through z_timeout_clamp_once(). It takes int64_t, so a runtime unsigned argument of 2^63 or more now clamps to 0 instead of passing through; that is more than 292 years in nanoseconds. Tested with the tests/unit/time_units cases added later in this series, run at 1000, 10000 and 32768 Hz ticks and with 32-bit timeouts, and host builds with GCC and Clang counting evaluations of the argument of each Z_TIMEOUT_*() macro. Assisted-by: Claude:claude-opus-5-5 --- include/zephyr/sys/clock.h | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/include/zephyr/sys/clock.h b/include/zephyr/sys/clock.h index 997f514a012b..72f346e3eff2 100644 --- a/include/zephyr/sys/clock.h +++ b/include/zephyr/sys/clock.h @@ -125,18 +125,25 @@ typedef struct { #define Z_TIMEOUT_TICKS(t) ((k_timeout_t)Z_TIMEOUT_TICKS_INIT(t)) #define Z_FOREVER Z_TIMEOUT_TICKS(K_TICKS_FOREVER) +static ALWAYS_INLINE int64_t z_timeout_clamp_once(int64_t t) +{ + return MAX(t, 0); +} + +#define Z_TIMEOUT_CLAMP(t) z_tmcvt_choose(z_tmcvt_const_arg(t), MAX(t, 0), z_timeout_clamp_once(t)) + #ifdef CONFIG_TIMEOUT_64BIT -#define Z_TIMEOUT_MS(t) Z_TIMEOUT_TICKS((k_ticks_t)k_ms_to_ticks_ceil64(MAX(t, 0))) -#define Z_TIMEOUT_US(t) Z_TIMEOUT_TICKS((k_ticks_t)k_us_to_ticks_ceil64(MAX(t, 0))) -#define Z_TIMEOUT_NS(t) Z_TIMEOUT_TICKS((k_ticks_t)k_ns_to_ticks_ceil64(MAX(t, 0))) -#define Z_TIMEOUT_CYC(t) Z_TIMEOUT_TICKS((k_ticks_t)k_cyc_to_ticks_ceil64(MAX(t, 0))) -#define Z_TIMEOUT_MS_TICKS(t) ((k_ticks_t)k_ms_to_ticks_ceil64(MAX(t, 0))) +#define Z_TIMEOUT_MS(t) Z_TIMEOUT_TICKS((k_ticks_t)k_ms_to_ticks_ceil64(Z_TIMEOUT_CLAMP(t))) +#define Z_TIMEOUT_US(t) Z_TIMEOUT_TICKS((k_ticks_t)k_us_to_ticks_ceil64(Z_TIMEOUT_CLAMP(t))) +#define Z_TIMEOUT_NS(t) Z_TIMEOUT_TICKS((k_ticks_t)k_ns_to_ticks_ceil64(Z_TIMEOUT_CLAMP(t))) +#define Z_TIMEOUT_CYC(t) Z_TIMEOUT_TICKS((k_ticks_t)k_cyc_to_ticks_ceil64(Z_TIMEOUT_CLAMP(t))) +#define Z_TIMEOUT_MS_TICKS(t) ((k_ticks_t)k_ms_to_ticks_ceil64(Z_TIMEOUT_CLAMP(t))) #else -#define Z_TIMEOUT_MS(t) Z_TIMEOUT_TICKS((k_ticks_t)k_ms_to_ticks_ceil32(MAX(t, 0))) -#define Z_TIMEOUT_US(t) Z_TIMEOUT_TICKS((k_ticks_t)k_us_to_ticks_ceil32(MAX(t, 0))) -#define Z_TIMEOUT_NS(t) Z_TIMEOUT_TICKS((k_ticks_t)k_ns_to_ticks_ceil32(MAX(t, 0))) -#define Z_TIMEOUT_CYC(t) Z_TIMEOUT_TICKS((k_ticks_t)k_cyc_to_ticks_ceil32(MAX(t, 0))) -#define Z_TIMEOUT_MS_TICKS(t) ((k_ticks_t)k_ms_to_ticks_ceil32(MAX(t, 0))) +#define Z_TIMEOUT_MS(t) Z_TIMEOUT_TICKS((k_ticks_t)k_ms_to_ticks_ceil32(Z_TIMEOUT_CLAMP(t))) +#define Z_TIMEOUT_US(t) Z_TIMEOUT_TICKS((k_ticks_t)k_us_to_ticks_ceil32(Z_TIMEOUT_CLAMP(t))) +#define Z_TIMEOUT_NS(t) Z_TIMEOUT_TICKS((k_ticks_t)k_ns_to_ticks_ceil32(Z_TIMEOUT_CLAMP(t))) +#define Z_TIMEOUT_CYC(t) Z_TIMEOUT_TICKS((k_ticks_t)k_cyc_to_ticks_ceil32(Z_TIMEOUT_CLAMP(t))) +#define Z_TIMEOUT_MS_TICKS(t) ((k_ticks_t)k_ms_to_ticks_ceil32(Z_TIMEOUT_CLAMP(t))) #endif /* Converts between absolute timeout expiration values (packed into From 7bd0e8dd660374736753c567b2df24e3abb4cb4b Mon Sep 17 00:00:00 2001 From: JP Hutchins Date: Thu, 24 Sep 2026 11:56:21 -0700 Subject: [PATCH 3/5] sys: time_units: evaluate SYS_TIMEOUT_MS argument once SYS_TIMEOUT_MS_INIT() compares its argument with SYS_FOREVER_MS and then converts it, so it evaluates the argument at least twice. Route non-constant arguments through z_sys_timeout_ms_ticks_once(). The expression form compares in the argument's type, so a uint32_t UINT32_MAX means forever. The function compares against 0 ? (ms) : SYS_FOREVER_MS, which converts SYS_FOREVER_MS to that type without evaluating ms, and keeps the same behavior. Tested with the tests/unit/time_units cases added later in this series, run at 1000, 10000 and 32768 Hz ticks and with 32-bit timeouts, including SYS_FOREVER_MS in int32_t, uint32_t, int64_t and uint64_t arguments. Assisted-by: Claude:claude-opus-5-5 --- include/zephyr/sys/clock.h | 6 ++++++ include/zephyr/sys/time_units.h | 5 +++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/include/zephyr/sys/clock.h b/include/zephyr/sys/clock.h index 72f346e3eff2..2be90a12cde8 100644 --- a/include/zephyr/sys/clock.h +++ b/include/zephyr/sys/clock.h @@ -146,6 +146,12 @@ static ALWAYS_INLINE int64_t z_timeout_clamp_once(int64_t t) #define Z_TIMEOUT_MS_TICKS(t) ((k_ticks_t)k_ms_to_ticks_ceil32(Z_TIMEOUT_CLAMP(t))) #endif +/* forever_ms is SYS_FOREVER_MS converted to the type of the SYS_TIMEOUT_MS() argument */ +static ALWAYS_INLINE k_ticks_t z_sys_timeout_ms_ticks_once(int64_t ms, int64_t forever_ms) +{ + return (ms == forever_ms) ? K_TICKS_FOREVER : Z_TIMEOUT_MS_TICKS(ms); +} + /* Converts between absolute timeout expiration values (packed into * the negative space below K_TICKS_FOREVER) and (non-negative) delta * timeout values. If the result of Z_TICK_ABS(t) is >= 0, then the diff --git a/include/zephyr/sys/time_units.h b/include/zephyr/sys/time_units.h index 376ecb46029c..8ef004fb1781 100644 --- a/include/zephyr/sys/time_units.h +++ b/include/zephyr/sys/time_units.h @@ -42,8 +42,9 @@ extern "C" { * converted from milliseconds. */ #define SYS_TIMEOUT_MS_INIT(ms) \ - Z_TIMEOUT_TICKS_INIT((ms) == SYS_FOREVER_MS ? \ - K_TICKS_FOREVER : Z_TIMEOUT_MS_TICKS(ms)) + Z_TIMEOUT_TICKS_INIT(z_tmcvt_choose(z_tmcvt_const_arg(ms), \ + ((ms) == SYS_FOREVER_MS ? K_TICKS_FOREVER : Z_TIMEOUT_MS_TICKS(ms)), \ + z_sys_timeout_ms_ticks_once(ms, 0 ? (ms) : SYS_FOREVER_MS))) /** @brief System-wide macro to convert milliseconds to kernel timeouts */ From a8eb9598cb2f1f4efb46c3862c0f69f3a9acee68 Mon Sep 17 00:00:00 2001 From: JP Hutchins Date: Thu, 24 Sep 2026 11:56:21 -0700 Subject: [PATCH 4/5] tests: unit: time_units: test single evaluation of arguments Count evaluations of the argument through every 32- and 64-bit conversion path, with a runtime source frequency, and through Z_TIMEOUT_*() and SYS_TIMEOUT_MS(). Check that each runtime result matches the constant form, and that SYS_FOREVER_MS is recognized in signed, unsigned, 32- and 64-bit arguments. Two cases feed a clock that advances between reads. One passes a deadline between the clamp's comparison and its result. The other straddles a second boundary on the slow 64-bit path. BUILD_ASSERT(), a file scope array bound and static initializers check that constant arguments still give constant expressions. New scenarios run the suite at 1000 and 32768 Hz ticks and with 32-bit timeouts; the slow path only runs at 32768 Hz. The new cases fail without the preceding commits and pass with them. Assisted-by: Claude:claude-opus-5-5 --- tests/unit/time_units/single_eval.c | 205 ++++++++++++++++++++++++++++ tests/unit/time_units/tests.yaml | 9 ++ 2 files changed, 214 insertions(+) create mode 100644 tests/unit/time_units/single_eval.c diff --git a/tests/unit/time_units/single_eval.c b/tests/unit/time_units/single_eval.c new file mode 100644 index 000000000000..eb2b27e834cb --- /dev/null +++ b/tests/unit/time_units/single_eval.c @@ -0,0 +1,205 @@ +/* + * Copyright The Zephyr Project Contributors + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include + +#include +#include +#include +#include + +BUILD_ASSERT(k_ms_to_ticks_ceil32(1) == DIV_ROUND_UP(CONFIG_SYS_CLOCK_TICKS_PER_SEC, 1000)); +BUILD_ASSERT(k_ticks_to_ns_floor64(CONFIG_SYS_CLOCK_TICKS_PER_SEC) == NSEC_PER_SEC); + +static const k_timeout_t static_timeout = Z_TIMEOUT_MS(100); +static const k_timeout_t static_forever = SYS_TIMEOUT_MS(SYS_FOREVER_MS); +static const uint32_t static_ticks = z_tmcvt_32(32768, 32768, 1000, true, true, false); +static uint8_t static_array[k_ms_to_ticks_ceil32(1)]; + +static int evaluations; + +static uint64_t counted(uint64_t value) +{ + evaluations++; + return value; +} + +static int64_t counted_signed(int64_t value) +{ + evaluations++; + return value; +} + +static uint32_t counted_hz(uint32_t hz) +{ + evaluations++; + return hz; +} + +#define ASSERT_ONCE(convert, value) \ + do { \ + evaluations = 0; \ + const uint64_t runtime = convert(counted(value)); \ + \ + zassert_equal(evaluations, 1, #convert "(" #value ") evaluated %d times", \ + evaluations); \ + zassert_equal(runtime, convert(value), #convert "(" #value ")"); \ + } while (false) + +#define DIV_32_FLOOR(t) z_tmcvt_32(t, 10000, 1000, true, false, false) +#define DIV_32_NEAR(t) z_tmcvt_32(t, 10000, 1000, true, false, true) +#define DIV_32_CEIL(t) z_tmcvt_32(t, 10000, 1000, true, true, false) +#define MUL_32(t) z_tmcvt_32(t, 1000, 10000, true, true, false) +#define GEN_32(t) z_tmcvt_32(t, 32768, 1000, true, true, false) +#define DIV_64_CEIL(t) z_tmcvt_64(t, 10000, 1000, true, true, false) +#define MUL_64(t) z_tmcvt_64(t, 1000, 10000, true, true, false) +#define FAST_64(t) z_tmcvt_64(t, 32768, 1000, true, true, false) +#define SLOW_64(t) z_tmcvt_64(t, 32768, 1000000000, true, false, false) +#define RUNTIME_64(t) z_tmcvt_64(t, 32768, 1000000000, false, false, true) + +#define ASSERT_ONCE_ALL_PATHS(value) \ + ASSERT_ONCE(DIV_32_FLOOR, value); \ + ASSERT_ONCE(DIV_32_NEAR, value); \ + ASSERT_ONCE(DIV_32_CEIL, value); \ + ASSERT_ONCE(MUL_32, value); \ + ASSERT_ONCE(GEN_32, value); \ + ASSERT_ONCE(DIV_64_CEIL, value); \ + ASSERT_ONCE(MUL_64, value); \ + ASSERT_ONCE(FAST_64, value); \ + ASSERT_ONCE(SLOW_64, value); \ + ASSERT_ONCE(RUNTIME_64, value) + +ZTEST(time_units, test_constant_arguments_stay_constant_expressions) +{ + zassert_equal(static_timeout.ticks, k_ms_to_ticks_ceil64(100)); + zassert_equal(static_forever.ticks, K_TICKS_FOREVER); + zassert_equal(static_ticks, 1000); + zassert_equal(sizeof(static_array), k_ms_to_ticks_ceil32(1)); +} + +ZTEST(time_units, test_conversion_paths_evaluate_argument_once) +{ + ASSERT_ONCE_ALL_PATHS(0); + ASSERT_ONCE_ALL_PATHS(1); + ASSERT_ONCE_ALL_PATHS(32767); + ASSERT_ONCE_ALL_PATHS(32768); + ASSERT_ONCE_ALL_PATHS(0xfffffff6); + ASSERT_ONCE_ALL_PATHS(0xfffffff7); + ASSERT_ONCE_ALL_PATHS(0xffffffff); + ASSERT_ONCE_ALL_PATHS(0x100000000); +} + +ZTEST(time_units, test_runtime_frequency_evaluated_once) +{ + evaluations = 0; + const uint64_t ns = + z_tmcvt_64(counted(98304), counted_hz(32768), 1000000000, false, false, true); + + zassert_equal(evaluations, 2); + zassert_equal(ns, 3000000000); + + evaluations = 0; + const uint32_t ms = z_tmcvt_32(counted(98304), counted_hz(32768), 1000, false, true, false); + + zassert_equal(evaluations, 2); + zassert_equal(ms, 3000); +} + +ZTEST(time_units, test_timeout_macros_evaluate_argument_once) +{ + evaluations = 0; + const k_timeout_t ms = Z_TIMEOUT_MS(counted_signed(250)); + + zassert_equal(evaluations, 1); + zassert_equal(ms.ticks, Z_TIMEOUT_MS(250).ticks); + + evaluations = 0; + const k_timeout_t us = Z_TIMEOUT_US(counted_signed(250)); + + zassert_equal(evaluations, 1); + zassert_equal(us.ticks, Z_TIMEOUT_US(250).ticks); + + evaluations = 0; + const k_timeout_t ns = Z_TIMEOUT_NS(counted_signed(250)); + + zassert_equal(evaluations, 1); + zassert_equal(ns.ticks, Z_TIMEOUT_NS(250).ticks); + + evaluations = 0; + const k_timeout_t negative = Z_TIMEOUT_MS(counted_signed(-5)); + + zassert_equal(evaluations, 1); + zassert_equal(negative.ticks, 0); +} + +ZTEST(time_units, test_sys_timeout_ms_evaluates_argument_once) +{ + evaluations = 0; + const k_timeout_t ms = SYS_TIMEOUT_MS(counted_signed(250)); + + zassert_equal(evaluations, 1); + zassert_equal(ms.ticks, SYS_TIMEOUT_MS(250).ticks); + + evaluations = 0; + const k_timeout_t forever = SYS_TIMEOUT_MS(counted_signed(SYS_FOREVER_MS)); + + zassert_equal(evaluations, 1); + zassert_equal(forever.ticks, K_TICKS_FOREVER); +} + +ZTEST(time_units, test_sys_timeout_ms_forever_in_argument_type) +{ + zassert_equal(SYS_TIMEOUT_MS((int32_t)counted_signed(-1)).ticks, K_TICKS_FOREVER); + zassert_equal(SYS_TIMEOUT_MS((uint32_t)counted(UINT32_MAX)).ticks, K_TICKS_FOREVER); + zassert_equal(SYS_TIMEOUT_MS(counted_signed(-1)).ticks, K_TICKS_FOREVER); + zassert_equal(SYS_TIMEOUT_MS(counted(UINT64_MAX)).ticks, K_TICKS_FOREVER); + zassert_equal(SYS_TIMEOUT_MS((uint16_t)counted(UINT16_MAX)).ticks, + SYS_TIMEOUT_MS((uint16_t)UINT16_MAX).ticks); + zassert_equal(SYS_TIMEOUT_MS((uint32_t)counted(UINT32_MAX - 1)).ticks, + SYS_TIMEOUT_MS((uint32_t)(UINT32_MAX - 1)).ticks); +} + +static int reads; +static int64_t read_sequence[2]; + +static int64_t read_clock(void) +{ + return read_sequence[reads++ % ARRAY_SIZE(read_sequence)]; +} + +/* A deadline that passes between two reads of the clock must not bypass the + * clamp; at 1000 Hz a -1 ms argument is K_TICKS_FOREVER. + */ +ZTEST(time_units, test_timeout_clamp_uses_a_single_clock_read) +{ + const int64_t now = 1000000; + const int64_t deadline = now + 1; + + reads = 0; + read_sequence[0] = now; + read_sequence[1] = deadline + 1; + const k_timeout_t timeout = Z_TIMEOUT_MS(deadline - read_clock()); + + zassert_equal(reads, 1); + zassert_equal(timeout.ticks, Z_TIMEOUT_MS(1).ticks); +} + +/* Quotient and remainder of the slow 64-bit path come from one read, so reads + * that straddle a second boundary cannot lose a whole second. + */ +ZTEST(time_units, test_slow_path_uses_a_single_clock_read) +{ + const int64_t hz = CONFIG_SYS_CLOCK_TICKS_PER_SEC; + + reads = 0; + read_sequence[0] = 1000 * hz - 1; + read_sequence[1] = 1000 * hz; + const uint64_t ns = k_ticks_to_ns_floor64(read_clock()); + + zassert_equal(reads, 1); + zassert_equal(ns, k_ticks_to_ns_floor64(1000 * CONFIG_SYS_CLOCK_TICKS_PER_SEC - 1)); +} diff --git a/tests/unit/time_units/tests.yaml b/tests/unit/time_units/tests.yaml index 4624ce7143b8..75b63782462d 100644 --- a/tests/unit/time_units/tests.yaml +++ b/tests/unit/time_units/tests.yaml @@ -3,3 +3,12 @@ common: type: unit tests: utilities.time_units.z_tmcvt: {} + utilities.time_units.ticks_1000: + extra_configs: + - CONFIG_SYS_CLOCK_TICKS_PER_SEC=1000 + utilities.time_units.ticks_32768: + extra_configs: + - CONFIG_SYS_CLOCK_TICKS_PER_SEC=32768 + utilities.time_units.timeout_32bit: + extra_configs: + - CONFIG_TIMEOUT_64BIT=n From 9f8f1dfc1888f0978cd6ba994b395288c092352a Mon Sep 17 00:00:00 2001 From: JP Hutchins Date: Thu, 24 Sep 2026 11:56:22 -0700 Subject: [PATCH 5/5] doc: releases: note single evaluation of time conversion arguments Add a release note for the time unit conversion and timeout macros evaluating a non-constant argument once. Add a migration note for GCC rejecting a const variable as their argument in a static initializer. Assisted-by: Claude:claude-opus-5-5 --- doc/releases/migration-guide-4.5.rst | 6 ++++++ doc/releases/release-notes-4.5.rst | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/doc/releases/migration-guide-4.5.rst b/doc/releases/migration-guide-4.5.rst index cfdf8b94324c..0dfc35d787f1 100644 --- a/doc/releases/migration-guide-4.5.rst +++ b/doc/releases/migration-guide-4.5.rst @@ -133,6 +133,12 @@ Kernel be used as futex address. The error -EINVAL can no longer happen on futex operations. +* With GCC, the argument of a time unit conversion macro such as :c:macro:`K_MSEC` or + :c:macro:`k_ms_to_ticks_ceil32` used in a static initializer must be an integer constant + expression. A ``const`` variable, which GCC folded as an extension, is now rejected with + ``initializer element is not constant``. Use a macro, an enumerator or, with + :kconfig:option:`CONFIG_STD_C23`, a ``constexpr`` object instead. + Boards ****** diff --git a/doc/releases/release-notes-4.5.rst b/doc/releases/release-notes-4.5.rst index c0821b13517f..5a3e6dba4ca6 100644 --- a/doc/releases/release-notes-4.5.rst +++ b/doc/releases/release-notes-4.5.rst @@ -2094,6 +2094,13 @@ Other notable changes failure. Use :c:func:`k_thread_cpu_pin` to reassign a thread to a different CPU. + * With GCC and Clang, the time unit conversion macros (``k__to__()``), + :c:macro:`K_MSEC`, :c:macro:`K_USEC`, :c:macro:`K_NSEC`, :c:macro:`K_CYC`, + :c:macro:`K_SECONDS` and :c:macro:`SYS_TIMEOUT_MS` evaluate an argument that is not a + compile-time constant exactly once. An argument such as ``deadline - k_uptime_get()`` was + evaluated up to four times, and reads of a running clock could disagree within one + conversion. Constant arguments still produce integer constant expressions. + * Timer * With :kconfig:option:`CONFIG_SYSTEM_CLOCK_SLOPPY_IDLE` enabled, a driver may no