From b30ce119d63a0c2a1af3cd63d73b84992d2d0e4d Mon Sep 17 00:00:00 2001 From: Chris Friedt Date: Fri, 16 May 2025 11:24:06 -0400 Subject: [PATCH 1/5] timeutil: correct position of doxygen end-group comment swap the relative position of the __cplusplus closing bracket and the doxygen end-group comment. Signed-off-by: Chris Friedt --- include/zephyr/sys/timeutil.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/zephyr/sys/timeutil.h b/include/zephyr/sys/timeutil.h index 333ff76a2efb..138f89a883cf 100644 --- a/include/zephyr/sys/timeutil.h +++ b/include/zephyr/sys/timeutil.h @@ -301,12 +301,12 @@ int timeutil_sync_local_from_ref(const struct timeutil_sync_state *tsp, */ int32_t timeutil_sync_skew_to_ppb(float skew); -#ifdef __cplusplus -} -#endif - /** * @} */ +#ifdef __cplusplus +} +#endif + #endif /* ZEPHYR_INCLUDE_SYS_TIMEUTIL_H_ */ From 87a3b1b3b465e2e5a547d6f0ecd6e6ea18367835 Mon Sep 17 00:00:00 2001 From: Chris Friedt Date: Fri, 16 May 2025 11:28:24 -0400 Subject: [PATCH 2/5] sys: timeutil: add utility functions for struct timespec Add a number of utility functions for manipulating struct timespec. * timespec_add() * timespec_compare() * timespec_equal() * timespec_is_valid() * timespec_negate() * timespec_normalize() * timespec_sub() If the `__builtin_add_overflow()` function is available, then the API is mostly branchless, which should provide decent performance on systems with an instruction cache and branch prediction. Otherwise, manually written alternatives exist that are also perhaps more readable. Signed-off-by: Chris Friedt --- include/zephyr/sys/timeutil.h | 294 ++++++++++++++++++++++++++++++++++ 1 file changed, 294 insertions(+) diff --git a/include/zephyr/sys/timeutil.h b/include/zephyr/sys/timeutil.h index 138f89a883cf..e7cfb6737627 100644 --- a/include/zephyr/sys/timeutil.h +++ b/include/zephyr/sys/timeutil.h @@ -21,8 +21,15 @@ #ifndef ZEPHYR_INCLUDE_SYS_TIMEUTIL_H_ #define ZEPHYR_INCLUDE_SYS_TIMEUTIL_H_ +#include +#include +#include +#include #include +#include +#include +#include #include #ifdef __cplusplus @@ -301,6 +308,293 @@ int timeutil_sync_local_from_ref(const struct timeutil_sync_state *tsp, */ int32_t timeutil_sync_skew_to_ppb(float skew); +/** + * @} + */ + +/** + * @defgroup timeutil_timespec_apis Timespec Utility APIs + * @ingroup timeutil_apis + * @{ + */ + +/** + * @brief Check if a timespec is valid. + * + * Check if a timespec is valid (i.e. normalized) by ensuring that the `tv_nsec` field is in the + * range `[0, NSEC_PER_SEC-1]`. + * + * @note @p ts must not be `NULL`. + * + * @param ts the timespec to check + * + * @return `true` if the timespec is valid, otherwise `false`. + */ +static inline bool timespec_is_valid(const struct timespec *ts) +{ + __ASSERT_NO_MSG(ts != NULL); + + return (ts->tv_nsec >= 0) && (ts->tv_nsec < NSEC_PER_SEC); +} + +#if (defined(CONFIG_SPEED_OPTIMIZATIONS) && HAS_BUILTIN(__builtin_add_overflow)) || \ + defined(__DOXYGEN__) +/** + * @brief Normalize a timespec so that the `tv_nsec` field is in valid range. + * + * Normalize a timespec by adjusting the `tv_sec` and `tv_nsec` fields so that the `tv_nsec` field + * is in the range `[0, NSEC_PER_SEC-1]`. This is achieved by converting nanoseconds to seconds and + * accumulating seconds in either the positive direction when `tv_nsec` > `NSEC_PER_SEC`, or in the + * negative direction when `tv_nsec` < 0. + * + * In pseudocode, normalization can be done as follows: + * ```python + * if ts.tv_nsec >= NSEC_PER_SEC: + * sec = ts.tv_nsec / NSEC_PER_SEC + * ts.tv_sec += sec + * ts.tv_nsec -= sec * NSEC_PER_SEC + * elif ts.tv_nsec < 0: + * # div_round_up(abs(ts->tv_nsec), NSEC_PER_SEC) + * sec = (NSEC_PER_SEC - ts.tv_nsec - 1) / NSEC_PER_SEC + * ts.tv_sec -= sec; + * ts.tv_nsec += sec * NSEC_PER_SEC; + * ``` + * + * @note There are two cases where the normalization can result in integer overflow. These can + * be extrapolated to not simply overflowing the `tv_sec` field by one second, but also by any + * realizable multiple of `NSEC_PER_SEC`. + * + * 1. When `tv_nsec` is negative and `tv_sec` is already most negative. + * 2. When `tv_nsec` is greater-or-equal to `NSEC_PER_SEC` and `tv_sec` is already most positive. + * + * If the operation would result in integer overflow, return value is `false`. + * + * @note @p ts must be non-`NULL`. + * + * @param ts the timespec to be normalized + * + * @return `true` if the operation completes successfully, otherwise `false`. + */ +static inline bool timespec_normalize(struct timespec *ts) +{ + __ASSERT_NO_MSG(ts != NULL); + + int sign = (ts->tv_nsec >= 0) - (ts->tv_nsec < 0); + int64_t sec = (ts->tv_nsec >= (long)NSEC_PER_SEC) * (ts->tv_nsec / (long)NSEC_PER_SEC) + + ((ts->tv_nsec < 0) && (ts->tv_nsec != LONG_MIN)) * + DIV_ROUND_UP((unsigned long)-ts->tv_nsec, (long)NSEC_PER_SEC) + + (ts->tv_nsec == LONG_MIN) * ((LONG_MAX / NSEC_PER_SEC) + 1); + bool overflow = __builtin_add_overflow(ts->tv_sec, sign * sec, &ts->tv_sec); + + ts->tv_nsec -= sign * (long)NSEC_PER_SEC * sec; + + if (!overflow) { + __ASSERT_NO_MSG(timespec_is_valid(ts)); + } + + return !overflow; +} + +/** + * @brief Add one timespec to another + * + * This function sums the two timespecs pointed to by @p a and @p b and stores the result in the + * timespce pointed to by @p a. + * + * If the operation would result in integer overflow, return value is `false`. + * + * @note @p a and @p b must be non-`NULL` and normalized. + * + * @param a the timespec which is added to + * @param b the timespec to be added + * + * @return `true` if the operation was successful, otherwise `false`. + */ +static inline bool timespec_add(struct timespec *a, const struct timespec *b) +{ + __ASSERT_NO_MSG((a != NULL) && timespec_is_valid(a)); + __ASSERT_NO_MSG((b != NULL) && timespec_is_valid(b)); + + return !__builtin_add_overflow(a->tv_sec, b->tv_sec, &a->tv_sec) && + !__builtin_add_overflow(a->tv_nsec, b->tv_nsec, &a->tv_nsec) && + timespec_normalize(a); +} + +/** + * @brief Negate a timespec object + * + * Negate the timespec object pointed to by @p ts and store the result in the same + * memory location. + * + * If the operation would result in integer overflow, return value is `false`. + * + * @param ts The timespec object to negate. + * + * @return `true` of the operation is successful, otherwise `false`. + */ +static inline bool timespec_negate(struct timespec *ts) +{ + __ASSERT_NO_MSG((ts != NULL) && timespec_is_valid(ts)); + + return !__builtin_sub_overflow(0LL, ts->tv_sec, &ts->tv_sec) && + !__builtin_sub_overflow(0L, ts->tv_nsec, &ts->tv_nsec) && timespec_normalize(ts); +} +#else + +static inline bool timespec_normalize(struct timespec *ts) +{ + __ASSERT_NO_MSG(ts != NULL); + + long sec; + + if (ts->tv_nsec >= (long)NSEC_PER_SEC) { + sec = ts->tv_nsec / (long)NSEC_PER_SEC; + } else if (ts->tv_nsec < 0) { + if ((sizeof(ts->tv_nsec) == sizeof(uint32_t)) && (ts->tv_nsec == LONG_MIN)) { + sec = DIV_ROUND_UP(LONG_MAX / NSEC_PER_USEC, USEC_PER_SEC); + } else { + sec = DIV_ROUND_UP((unsigned long)-ts->tv_nsec, NSEC_PER_SEC); + } + } else { + sec = 0; + } + + if ((ts->tv_nsec < 0) && (ts->tv_sec < 0) && (ts->tv_sec - INT64_MIN < sec)) { + /* + * When `tv_nsec` is negative and `tv_sec` is already most negative, + * further subtraction would cause integer overflow. + */ + return false; + } + + if ((ts->tv_nsec >= (long)NSEC_PER_SEC) && (ts->tv_sec > 0) && + (INT64_MAX - ts->tv_sec < sec)) { + /* + * When `tv_nsec` is >= `NSEC_PER_SEC` and `tv_sec` is already most + * positive, further addition would cause integer overflow. + */ + return false; + } + + if (ts->tv_nsec >= (long)NSEC_PER_SEC) { + ts->tv_sec += sec; + ts->tv_nsec -= sec * (long)NSEC_PER_SEC; + } else if (ts->tv_nsec < 0) { + ts->tv_sec -= sec; + ts->tv_nsec += sec * (long)NSEC_PER_SEC; + } else { + /* no change: SonarQube was complaining */ + } + + __ASSERT_NO_MSG(timespec_is_valid(ts)); + + return true; +} + +static inline bool timespec_add(struct timespec *a, const struct timespec *b) +{ + __ASSERT_NO_MSG((a != NULL) && timespec_is_valid(a)); + __ASSERT_NO_MSG((b != NULL) && timespec_is_valid(b)); + + if ((a->tv_sec < 0) && (b->tv_sec < 0) && (INT64_MIN - a->tv_sec > b->tv_sec)) { + /* negative integer overflow would occur */ + return false; + } + + if ((a->tv_sec > 0) && (b->tv_sec > 0) && (INT64_MAX - a->tv_sec < b->tv_sec)) { + /* positive integer overflow would occur */ + return false; + } + + a->tv_sec += b->tv_sec; + a->tv_nsec += b->tv_nsec; + + return timespec_normalize(a); +} + +static inline bool timespec_negate(struct timespec *ts) +{ + __ASSERT_NO_MSG((ts != NULL) && timespec_is_valid(ts)); + + if (ts->tv_sec == INT64_MIN) { + /* -INT64_MIN > INT64_MAX, so +ve integer overflow would occur */ + return false; + } + + ts->tv_sec = -ts->tv_sec; + ts->tv_nsec = -ts->tv_nsec; + + return timespec_normalize(ts); +} +#endif + +/** + * @brief Subtract one timespec from another + * + * This function subtracts the timespec pointed to by @p b from the timespec pointed to by @p a and + * stores the result in the timespce pointed to by @p a. + * + * If the operation would result in integer overflow, return value is `false`. + * + * @note @p a and @p b must be non-`NULL`. + * + * @param a the timespec which is subtracted from + * @param b the timespec to be subtracted + * + * @return `true` if the operation is successful, otherwise `false`. + */ +static inline bool timespec_sub(struct timespec *a, const struct timespec *b) +{ + __ASSERT_NO_MSG(a != NULL); + __ASSERT_NO_MSG(b != NULL); + + struct timespec neg = *b; + + return timespec_negate(&neg) && timespec_add(a, &neg); +} + +/** + * @brief Compare two timespec objects + * + * This function compares two timespec objects pointed to by @p a and @p b. + * + * @note @p a and @p b must be non-`NULL` and normalized. + * + * @param a the first timespec to compare + * @param b the second timespec to compare + * + * @return -1, 0, or +1 if @a a is less than, equal to, or greater than @a b, respectively. + */ +static inline int timespec_compare(const struct timespec *a, const struct timespec *b) +{ + __ASSERT_NO_MSG((a != NULL) && timespec_is_valid(a)); + __ASSERT_NO_MSG((b != NULL) && timespec_is_valid(b)); + + return (((a->tv_sec == b->tv_sec) && (a->tv_nsec < b->tv_nsec)) * -1) + + (((a->tv_sec == b->tv_sec) && (a->tv_nsec > b->tv_nsec)) * 1) + + ((a->tv_sec < b->tv_sec) * -1) + ((a->tv_sec > b->tv_sec)); +} + +/** + * @brief Check if two timespec objects are equal + * + * This function checks if the two timespec objects pointed to by @p a and @p b are equal. + * + * @note @p a and @p b must be non-`NULL` are not required to be normalized. + * + * @param a the first timespec to compare + * @param b the second timespec to compare + * + * @return true if the two timespec objects are equal, otherwise false. + */ +static inline bool timespec_equal(const struct timespec *a, const struct timespec *b) +{ + __ASSERT_NO_MSG(a != NULL); + __ASSERT_NO_MSG(b != NULL); + + return (a->tv_sec == b->tv_sec) && (a->tv_nsec == b->tv_nsec); +} + /** * @} */ From d98018c6a3aa9ef47e0db61b359f661608ccce59 Mon Sep 17 00:00:00 2001 From: Chris Friedt Date: Fri, 16 May 2025 07:01:21 -0400 Subject: [PATCH 3/5] tests: lib: timeutil: add timespec util testsuite Add a timespec util testsuite. This should have reasonably high enough coverage to be useful. I would have preferred to add this as an architecture-independent unit test (for the unit_testing platform) under tests/unit/timeutil but there is an inconsistency about the size of time_t on the unit_testing and native_sim/native platforms. On every other platform supported by Zephyr, time_t is 64-bits. However, on those platforms, time_t is only 32-bits. Signed-off-by: Chris Friedt --- tests/lib/timespec_util/CMakeLists.txt | 8 + tests/lib/timespec_util/prj.conf | 1 + tests/lib/timespec_util/src/main.c | 271 +++++++++++++++++++++++++ tests/lib/timespec_util/testcase.yaml | 36 ++++ 4 files changed, 316 insertions(+) create mode 100644 tests/lib/timespec_util/CMakeLists.txt create mode 100644 tests/lib/timespec_util/prj.conf create mode 100644 tests/lib/timespec_util/src/main.c create mode 100644 tests/lib/timespec_util/testcase.yaml diff --git a/tests/lib/timespec_util/CMakeLists.txt b/tests/lib/timespec_util/CMakeLists.txt new file mode 100644 index 000000000000..e436a632ba04 --- /dev/null +++ b/tests/lib/timespec_util/CMakeLists.txt @@ -0,0 +1,8 @@ +# SPDX-License-Identifier: Apache-2.0 + +cmake_minimum_required(VERSION 3.20.0) +find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE}) +project(timespec_util) + +FILE(GLOB app_sources src/main.c) +target_sources(app PRIVATE ${app_sources}) diff --git a/tests/lib/timespec_util/prj.conf b/tests/lib/timespec_util/prj.conf new file mode 100644 index 000000000000..9467c2926896 --- /dev/null +++ b/tests/lib/timespec_util/prj.conf @@ -0,0 +1 @@ +CONFIG_ZTEST=y diff --git a/tests/lib/timespec_util/src/main.c b/tests/lib/timespec_util/src/main.c new file mode 100644 index 000000000000..c0d406b65243 --- /dev/null +++ b/tests/lib/timespec_util/src/main.c @@ -0,0 +1,271 @@ +/* + * Copyright 2025 Tenstorrent AI ULC + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include + +#include +#include +#include +#include + +BUILD_ASSERT(sizeof(time_t) == sizeof(int64_t), "time_t must be 64-bit"); +BUILD_ASSERT(sizeof(((struct timespec *)0)->tv_sec) == sizeof(int64_t), "tv_sec must be 64-bit"); + +/* need NSEC_PER_SEC to be signed for the purposes of this testsuite */ +#undef NSEC_PER_SEC +#define NSEC_PER_SEC 1000000000L + +#undef CORRECTABLE +#define CORRECTABLE true + +#undef UNCORRECTABLE +#define UNCORRECTABLE false + +/* + * test spec for simple timespec validation + * + * If a timespec is expected to be valid, then invalid_ts and valid_ts are equal. + * + * If a timespec is expected to be invalid, then invalid_ts and valid_ts are not equal. + */ +struct ts_test_spec { + struct timespec invalid_ts; + struct timespec valid_ts; + bool expect_valid; + bool correctable; +}; + +#define DECL_VALID_TS_TEST(sec, nsec) \ + { \ + .invalid_ts = {(sec), (nsec)}, \ + .valid_ts = {(sec), (nsec)}, \ + .expect_valid = true, \ + .correctable = false, \ + } + +/* + * Invalid timespecs can usually be converted to valid ones by adding or subtracting + * multiples of `NSEC_PER_SEC` from tv_nsec, and incrementing or decrementing tv_sec + * proportionately, unless doing so would result in signed integer overflow. + * + * There are two particular corner cases: + * 1. making tv_sec more negative would overflow the tv_sec field in the negative direction + * 1. making tv_sec more positive would overflow the tv_sec field in the positive direction + */ +#define DECL_INVALID_TS_TEST(invalid_sec, invalid_nsec, valid_sec, valid_nsec, is_correctable) \ + { \ + .valid_ts = {(valid_sec), (valid_nsec)}, \ + .invalid_ts = {(invalid_sec), (invalid_nsec)}, \ + .expect_valid = false, \ + .correctable = (is_correctable), \ + } + +/* + * Easily verifiable tests for both valid and invalid timespecs. + */ +static const struct ts_test_spec ts_tests[] = { + /* Valid cases */ + DECL_VALID_TS_TEST(0, 0), + DECL_VALID_TS_TEST(0, 1), + DECL_VALID_TS_TEST(1, 0), + DECL_VALID_TS_TEST(1, 1), + DECL_VALID_TS_TEST(1, NSEC_PER_SEC - 1), + DECL_VALID_TS_TEST(-1, 0), + DECL_VALID_TS_TEST(-1, 1), + DECL_VALID_TS_TEST(-1, 0), + DECL_VALID_TS_TEST(-1, 1), + DECL_VALID_TS_TEST(-1, NSEC_PER_SEC - 1), + DECL_VALID_TS_TEST(INT64_MIN, 0), + DECL_VALID_TS_TEST(INT64_MIN, NSEC_PER_SEC - 1), + DECL_VALID_TS_TEST(INT64_MAX, 0), + DECL_VALID_TS_TEST(INT64_MAX, NSEC_PER_SEC - 1), + + /* Correctable, invalid cases */ + DECL_INVALID_TS_TEST(0, -2 * NSEC_PER_SEC + 1, -2, 1, CORRECTABLE), + DECL_INVALID_TS_TEST(0, -2 * NSEC_PER_SEC - 1, -3, NSEC_PER_SEC - 1, CORRECTABLE), + DECL_INVALID_TS_TEST(0, -NSEC_PER_SEC - 1, -2, NSEC_PER_SEC - 1, CORRECTABLE), + DECL_INVALID_TS_TEST(0, -1, -1, NSEC_PER_SEC - 1, CORRECTABLE), + DECL_INVALID_TS_TEST(0, NSEC_PER_SEC, 1, 0, CORRECTABLE), + DECL_INVALID_TS_TEST(0, NSEC_PER_SEC + 1, 1, 0, CORRECTABLE), + DECL_INVALID_TS_TEST(1, -1, 0, NSEC_PER_SEC - 1, CORRECTABLE), + DECL_INVALID_TS_TEST(1, NSEC_PER_SEC, 2, 0, CORRECTABLE), + DECL_INVALID_TS_TEST(-1, -1, -2, NSEC_PER_SEC - 1, CORRECTABLE), + DECL_INVALID_TS_TEST(0, NSEC_PER_SEC, 1, 0, CORRECTABLE), + DECL_INVALID_TS_TEST(1, -1, 0, NSEC_PER_SEC - 1, CORRECTABLE), + DECL_INVALID_TS_TEST(1, NSEC_PER_SEC, 2, 0, CORRECTABLE), + DECL_INVALID_TS_TEST(INT64_MIN, NSEC_PER_SEC, INT64_MIN + 1, 0, CORRECTABLE), + DECL_INVALID_TS_TEST(INT64_MAX, -1, INT64_MAX - 1, NSEC_PER_SEC - 1, CORRECTABLE), + DECL_INVALID_TS_TEST(0, LONG_MIN, LONG_MAX / NSEC_PER_SEC, 145224192, CORRECTABLE), + DECL_INVALID_TS_TEST(0, LONG_MAX, LONG_MAX / NSEC_PER_SEC, LONG_MAX % NSEC_PER_SEC, + CORRECTABLE), + + /* Uncorrectable, invalid cases */ + DECL_INVALID_TS_TEST(INT64_MIN + 2, -2 * NSEC_PER_SEC - 1, 0, 0, UNCORRECTABLE), + DECL_INVALID_TS_TEST(INT64_MIN + 1, -NSEC_PER_SEC - 1, 0, 0, UNCORRECTABLE), + DECL_INVALID_TS_TEST(INT64_MIN + 1, -NSEC_PER_SEC - 1, 0, 0, UNCORRECTABLE), + DECL_INVALID_TS_TEST(INT64_MIN, -1, 0, 0, UNCORRECTABLE), + DECL_INVALID_TS_TEST(INT64_MAX, NSEC_PER_SEC, 0, 0, UNCORRECTABLE), + DECL_INVALID_TS_TEST(INT64_MAX - 1, 2 * NSEC_PER_SEC, 0, 0, UNCORRECTABLE), +}; + +ZTEST(timeutil_api, test_timespec_is_valid) +{ + ARRAY_FOR_EACH(ts_tests, i) { + const struct ts_test_spec *const tspec = &ts_tests[i]; + bool valid = timespec_is_valid(&tspec->invalid_ts); + + zexpect_equal(valid, tspec->expect_valid, + "%d: timespec_is_valid({%ld, %ld}) = %s, expected true", i, + tspec->valid_ts.tv_sec, tspec->valid_ts.tv_nsec, + tspec->expect_valid ? "false" : "true"); + } +} + +ZTEST(timeutil_api, test_timespec_normalize) +{ + ARRAY_FOR_EACH(ts_tests, i) { + bool different; + bool overflow; + const struct ts_test_spec *const tspec = &ts_tests[i]; + struct timespec norm = tspec->invalid_ts; + + overflow = !timespec_normalize(&norm); + zexpect_not_equal(tspec->expect_valid || tspec->correctable, overflow, + "%d: timespec_normalize({%ld, %ld}) %s, unexpectedly", i, + (long)tspec->invalid_ts.tv_sec, (long)tspec->invalid_ts.tv_nsec, + tspec->correctable ? "failed" : "succeeded"); + + if (!tspec->expect_valid && tspec->correctable) { + different = !timespec_equal(&tspec->invalid_ts, &norm); + zexpect_true(different, "%d: {%ld, %ld} and {%ld, %ld} are unexpectedly %s", + i, tspec->invalid_ts.tv_sec, tspec->invalid_ts.tv_nsec, + norm.tv_sec, tspec->valid_ts.tv_sec, + (tspec->expect_valid || tspec->correctable) ? "different" + : "equal"); + } + } +} + +ZTEST(timeutil_api, test_timespec_add) +{ + bool overflow; + struct timespec actual; + const struct atspec { + struct timespec a; + struct timespec b; + struct timespec result; + bool expect; + } tspecs[] = { + /* non-overflow cases */ + {.a = {0, 0}, .b = {0, 0}, .result = {0, 0}, .expect = false}, + {.a = {1, 1}, .b = {1, 1}, .result = {2, 2}, .expect = false}, + {.a = {-1, 1}, .b = {-1, 1}, .result = {-2, 2}, .expect = false}, + {.a = {-1, NSEC_PER_SEC - 1}, .b = {0, 1}, .result = {0, 0}, .expect = false}, + /* overflow cases */ + {.a = {INT64_MAX, 0}, .b = {1, 0}, .result = {0}, .expect = true}, + {.a = {INT64_MIN, 0}, .b = {-1, 0}, .result = {0}, .expect = true}, + {.a = {INT64_MAX, NSEC_PER_SEC - 1}, .b = {1, 1}, .result = {0}, .expect = true}, + {.a = {INT64_MIN, NSEC_PER_SEC - 1}, .b = {-1, 0}, .result = {0}, .expect = true}, + }; + + ARRAY_FOR_EACH(tspecs, i) { + const struct atspec *const tspec = &tspecs[i]; + + actual = tspec->a; + overflow = !timespec_add(&actual, &tspec->b); + + zexpect_equal(overflow, tspec->expect, + "%d: timespec_add({%ld, %ld}, {%ld, %ld}) %s, unexpectedly", i, + tspec->a.tv_sec, tspec->a.tv_nsec, tspec->b.tv_sec, tspec->b.tv_nsec, + tspec->expect ? "succeeded" : "failed"); + + if (!tspec->expect) { + zexpect_equal(timespec_equal(&actual, &tspec->result), true, + "%d: {%ld, %ld} and {%ld, %ld} are unexpectedly different", i, + actual.tv_sec, actual.tv_nsec, tspec->result.tv_sec, + tspec->result.tv_nsec); + } + } +} + +ZTEST(timeutil_api, test_timespec_negate) +{ + struct ntspec { + struct timespec ts; + struct timespec result; + bool expect_failure; + } tspecs[] = { + /* non-overflow cases */ + {.ts = {0, 0}, .result = {0, 0}, .expect_failure = false}, + {.ts = {1, 1}, .result = {-2, NSEC_PER_SEC - 1}, .expect_failure = false}, + {.ts = {-1, 1}, .result = {0, NSEC_PER_SEC - 1}, .expect_failure = false}, + {.ts = {INT64_MAX, 0}, .result = {INT64_MIN + 1, 0}, .expect_failure = false}, + /* overflow cases */ + {.ts = {INT64_MIN, 0}, .result = {0}, .expect_failure = true}, + }; + + ARRAY_FOR_EACH(tspecs, i) { + bool overflow; + const struct ntspec *const tspec = &tspecs[i]; + struct timespec actual = tspec->ts; + + overflow = !timespec_negate(&actual); + zexpect_equal(overflow, tspec->expect_failure, + "%d: timespec_negate({%ld, %ld}) %s, unexpectedly", i, + tspec->ts.tv_sec, tspec->ts.tv_nsec, + tspec->expect_failure ? "did not overflow" : "overflowed"); + + if (!tspec->expect_failure) { + zexpect_true(timespec_equal(&actual, &tspec->result), + "%d: {%ld, %ld} and {%ld, %ld} are unexpectedly different", i, + actual.tv_sec, actual.tv_nsec, tspec->result.tv_sec, + tspec->result.tv_nsec); + } + } +} + +ZTEST(timeutil_api, test_timespec_sub) +{ + struct timespec a = {0}; + struct timespec b = {0}; + + zexpect_true(timespec_sub(&a, &b)); +} + +ZTEST(timeutil_api, test_timespec_compare) +{ + struct timespec a; + struct timespec b; + + a = (struct timespec){0}; + b = (struct timespec){0}; + zexpect_equal(timespec_compare(&a, &b), 0); + + a = (struct timespec){-1}; + b = (struct timespec){0}; + zexpect_equal(timespec_compare(&a, &b), -1); + + a = (struct timespec){1}; + b = (struct timespec){0}; + zexpect_equal(timespec_compare(&a, &b), 1); +} + +ZTEST(timeutil_api, test_timespec_equal) +{ + struct timespec a; + struct timespec b; + + a = (struct timespec){0}; + b = (struct timespec){0}; + zexpect_true(timespec_equal(&a, &b)); + + a = (struct timespec){-1}; + b = (struct timespec){0}; + zexpect_false(timespec_equal(&a, &b)); +} + +ZTEST_SUITE(timeutil_api, NULL, NULL, NULL, NULL, NULL); diff --git a/tests/lib/timespec_util/testcase.yaml b/tests/lib/timespec_util/testcase.yaml new file mode 100644 index 000000000000..d69c5bb755fb --- /dev/null +++ b/tests/lib/timespec_util/testcase.yaml @@ -0,0 +1,36 @@ +# FIXME: this should be under tests/unit/timeutil but will not work due to #90029 +common: + filter: not CONFIG_NATIVE_LIBC + tags: + - timeutils + # 1 tier0 platform per supported architecture + platform_key: + - arch + - simulation + integration_platforms: + - native_sim/native/64 +tests: + libraries.timespec_utils: {} + libraries.timespec_utils.speed: + extra_configs: + - CONFIG_SPEED_OPTIMIZATIONS=y + libraries.timespec_utils.armclang_std_libc: + toolchain_allow: armclang + extra_configs: + - CONFIG_ARMCLANG_STD_LIBC=y + libraries.timespec_utils.arcmwdtlib: + toolchain_allow: arcmwdt + extra_configs: + - CONFIG_ARCMWDT_LIBC=y + libraries.timespec_utils.minimal: + extra_configs: + - CONFIG_MINIMAL_LIBC=y + libraries.timespec_utils.newlib: + filter: TOOLCHAIN_HAS_NEWLIB == 1 + extra_configs: + - CONFIG_NEWLIB_LIBC=y + libraries.timespec_utils.picolibc: + tags: picolibc + filter: CONFIG_PICOLIBC_SUPPORTED + extra_configs: + - CONFIG_PICOLIBC=y From 816164e08f4c553ba509eda3d7f80181fb9d24bc Mon Sep 17 00:00:00 2001 From: Chris Friedt Date: Fri, 16 May 2025 07:44:47 -0400 Subject: [PATCH 4/5] posix: timers: use newly added timespec util functions Use the newly added timespec util functions to manipulate and compare timespec structures with overflow detection. Signed-off-by: Chris Friedt --- lib/posix/options/clock_common.c | 18 +++++++++++------- lib/posix/options/posix_clock.h | 9 ++------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/lib/posix/options/clock_common.c b/lib/posix/options/clock_common.c index daacd3c7b801..c8271cf185dd 100644 --- a/lib/posix/options/clock_common.c +++ b/lib/posix/options/clock_common.c @@ -15,6 +15,7 @@ #include #include #include +#include /* * `k_uptime_get` returns a timestamp based on an always increasing @@ -82,11 +83,9 @@ int z_clock_gettime(clockid_t clock_id, struct timespec *ts) /* For ns 32 bit conversion can be used since its smaller than 1sec. */ ts->tv_nsec = (int32_t)k_ticks_to_ns_floor32(nremainder); - ts->tv_sec += base.tv_sec; - ts->tv_nsec += base.tv_nsec; - if (ts->tv_nsec >= NSEC_PER_SEC) { - ts->tv_sec++; - ts->tv_nsec -= NSEC_PER_SEC; + if (unlikely(!timespec_normalize(ts)) || unlikely(!timespec_add(ts, &base))) { + errno = EOVERFLOW; + return -1; } return 0; @@ -101,7 +100,7 @@ int z_clock_settime(clockid_t clock_id, const struct timespec *tp) return -1; } - if (tp->tv_nsec < 0 || tp->tv_nsec >= NSEC_PER_SEC) { + if (!timespec_is_valid(tp)) { errno = EINVAL; return -1; } @@ -112,6 +111,11 @@ int z_clock_settime(clockid_t clock_id, const struct timespec *tp) base.tv_sec = delta / NSEC_PER_SEC; base.tv_nsec = delta % NSEC_PER_SEC; + if (unlikely(!timespec_normalize(&base))) { + errno = EOVERFLOW; + return -1; + } + SYS_SEM_LOCK(&rt_clock_base_lock) { rt_clock_base = base; } @@ -137,7 +141,7 @@ int z_clock_nanosleep(clockid_t clock_id, int flags, const struct timespec *rqtp return -1; } - if ((rqtp->tv_sec < 0) || (rqtp->tv_nsec < 0) || (rqtp->tv_nsec >= NSEC_PER_SEC)) { + if ((rqtp->tv_sec < 0) || !timespec_is_valid(rqtp)) { errno = EINVAL; return -1; } diff --git a/lib/posix/options/posix_clock.h b/lib/posix/options/posix_clock.h index 31fd829c1478..86b4e2e7eb22 100644 --- a/lib/posix/options/posix_clock.h +++ b/lib/posix/options/posix_clock.h @@ -16,6 +16,7 @@ #include #include #include +#include #ifdef __cplusplus extern "C" { @@ -23,12 +24,6 @@ extern "C" { /** @cond INTERNAL_HIDDEN */ -static inline bool timespec_is_valid(const struct timespec *ts) -{ - __ASSERT_NO_MSG(ts != NULL); - return (ts->tv_nsec >= 0) && (ts->tv_nsec < NSEC_PER_SEC); -} - static inline int64_t ts_to_ns(const struct timespec *ts) { return ts->tv_sec * NSEC_PER_SEC + ts->tv_nsec; @@ -47,7 +42,7 @@ static inline void tv_to_ts(const struct timeval *tv, struct timespec *ts) static inline bool tp_ge(const struct timespec *a, const struct timespec *b) { - return ts_to_ns(a) >= ts_to_ns(b); + return timespec_compare(a, b) >= 0; } static inline int64_t tp_diff(const struct timespec *a, const struct timespec *b) From d6487e91baf78f4031500283c79559e4cf7a4f41 Mon Sep 17 00:00:00 2001 From: Chris Friedt Date: Fri, 16 May 2025 11:20:31 -0400 Subject: [PATCH 5/5] doc: kernel: timeutil: add documentation for timespec apis Add documentation in the timeutil group for recently added functions for validating, comparing, and manipulating `struct timespec` objects. Signed-off-by: Chris Friedt --- doc/kernel/timeutil.rst | 111 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/doc/kernel/timeutil.rst b/doc/kernel/timeutil.rst index 203d52ea9afd..049e4f47a7cf 100644 --- a/doc/kernel/timeutil.rst +++ b/doc/kernel/timeutil.rst @@ -28,6 +28,7 @@ The time utilities API supports: * :ref:`converting between time representations ` * :ref:`synchronizing and aligning time scales ` +* :ref:`comparing, adding, and subtracting representations ` For terminology and concepts that support these functions see :ref:`timeutil_concepts`. @@ -106,6 +107,90 @@ process: .. doxygengroup:: timeutil_sync_apis +.. _timeutil_manip: + +``timespec`` Manipulation +========================= + +Checking the validity of a ``timespec`` can be done with :c:func:`timespec_is_valid`. + +.. code-block:: c + + struct timespec ts = { + .tv_sec = 0, + .tv_nsec = -1, /* out of range! */ + }; + + if (!timespec_is_valid(&ts)) { + /* error-handing code */ + } + +In some cases, invalid ``timespec`` objects may be re-normalized using +:c:func:`timespec_normalize`. + +.. code-block:: c + + if (!timespec_normalize(&ts)) { + /* error-handling code */ + } + + /* ts should be normalized */ + __ASSERT(timespec_is_valid(&ts) == true, "expected normalized timespec"); + +It is possible to compare two ``timespec`` objects for equality using :c:func:`timespec_equal`. + +.. code-block:: c + + if (timespec_equal(then, now)) { + /* time is up! */ + } + +It is possible to compare and fully order (valid) ``timespec`` objects using +:c:func:`timespec_compare`. + +.. code-block:: c + + int cmp = timespec_compare(a, b); + + switch (cmp) { + case 0: + /* a == b */ + break; + case -1: + /* a < b */ + break; + case +1: + /* a > b */ + break; + } + +It is possible to add, subtract, and negate ``timespec`` objects using +:c:func:`timespec_add`, :c:func:`timespec_sub`, and :c:func:`timespec_negate`, +respectively. Like :c:func:`timespec_normalize`, these functions will output +a normalized ``timespec`` when doing so would not result in overflow. +On success, these functions return ``true``. If overflow would occur, the +functions return ``false``. + +.. code-block:: c + + /* a += b */ + if (!timespec_add(&a, &b)) { + /* overflow */ + } + + /* a -= b */ + if (!timespec_sub(&a, &b)) { + /* overflow */ + } + + /* a = -a */ + if (!timespec_negate(&a)) { + /* overflow */ + } + +.. doxygengroup:: timeutil_timespec_apis + + .. _timeutil_concepts: Concepts Underlying Time Support in Zephyr @@ -236,3 +321,29 @@ The mechanism used to populate synchronization points is not relevant: it may involve reading from a local high-precision RTC peripheral, exchanging packets over a network using a protocol like NTP or PTP, or processing NMEA messages received a GPS with or without a 1pps signal. + +``timespec`` Concepts +===================== + +Originally from POSIX, ``struct timespec`` has been a part of the C standard +since C11. The definition of ``struct timespec`` is as shown below. + +.. code-block:: c + + struct timespec { + time_t tv_sec; /* seconds */ + long tv_nsec; /* nanoseconds */ + }; + +.. _note: + + The C standard does not define the size of ``time_t``. However, Zephyr + uses 64-bits for ``time_t``. The ``long`` type is required to be at least + 32-bits, but usually matches the word size of the architecture. Both + elements of ``struct timespec`` are signed integers. ``time_t`` is defined + to be 64-bits both for historical reasons and to be robust enough to + represent times in the future. + +The ``tv_nsec`` field is only valid with values in the range ``[0, 999999999]``. The +``tv_sec`` field is the number of seconds since the epoch. If ``struct timespec`` is +used to express a difference, the ``tv_sec`` field may fall into a negative range.