Mesa (main): util/timespec: Return overflow from timespec_add_[mn]sec()

[GitLab Mirror]

Module: Mesa
Branch: main
Commit: 0ba22b203dbc152958ce4c1a1b842498a24dba3d
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=0ba22b203dbc152958ce4c1a1b842498a24dba3d

Author: Jason Ekstrand <jason.ekstrand at collabora.com>
Date:   Thu Mar 31 10:44:56 2022 -0500

util/timespec: Return overflow from timespec_add_[mn]sec()

To avoid altering any currently existing callers, we continue on with
the calculation regardless of overflow.  This also matches the behavior
of GCC's __builtin_add_overflow().

Reviewed-By: Mike Blumenkrantz <michael.blumenkrantz at gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/15651>

---

 src/util/tests/timespec_test.cpp | 41 +++++++++++++++++++++++++++++++++-------
 src/util/timespec.h              | 30 +++++++++++++++++++++++------
 2 files changed, 58 insertions(+), 13 deletions(-)

diff --git a/src/util/tests/timespec_test.cpp b/src/util/tests/timespec_test.cpp
index 5005506f9fd..4fad1884b30 100644
--- a/src/util/tests/timespec_test.cpp
+++ b/src/util/tests/timespec_test.cpp
@@ -27,6 +27,8 @@
 
 #include "util/timespec.h"
 
+#include <limits>
+
 TEST(timespec_test, timespec_add)
 {
    struct timespec a, b, r;
@@ -114,41 +116,66 @@ TEST(timespec_test, millihz_to_nsec)
    EXPECT_EQ(millihz_to_nsec(60000), 16666666);
 }
 
+TEST(timespec_test, time_t_max)
+{
+   /* The TIME_T_MAX macro assumes it's no more than 64 bits */
+   EXPECT_LE(sizeof(time_t), sizeof(uint64_t));
+
+   time_t t = TIME_T_MAX;
+   EXPECT_EQ((uint64_t)t, (uint64_t)TIME_T_MAX);
+
+   /* Since the tests are C++ code, we have std::numeric_limits */
+   EXPECT_EQ(std::numeric_limits<time_t>::max(), TIME_T_MAX);
+}
+
 TEST(timespec_test, timespec_add_nsec)
 {
    struct timespec a, r;
 
    a.tv_sec = 0;
    a.tv_nsec = NSEC_PER_SEC - 1;
-   timespec_add_nsec(&r, &a, 1);
+   EXPECT_FALSE(timespec_add_nsec(&r, &a, 1));
    EXPECT_EQ(1, r.tv_sec);
    EXPECT_EQ(0, r.tv_nsec);
 
-   timespec_add_nsec(&r, &a, 2);
+   EXPECT_FALSE(timespec_add_nsec(&r, &a, 2));
    EXPECT_EQ(1, r.tv_sec);
    EXPECT_EQ(1, r.tv_nsec);
 
-   timespec_add_nsec(&r, &a, (NSEC_PER_SEC * 2ULL));
+   EXPECT_FALSE(timespec_add_nsec(&r, &a, (NSEC_PER_SEC * 2ULL)));
    EXPECT_EQ(2, r.tv_sec);
    EXPECT_EQ(NSEC_PER_SEC - 1, r.tv_nsec);
 
-   timespec_add_nsec(&r, &a, (NSEC_PER_SEC * 2ULL) + 2);
+   EXPECT_FALSE(timespec_add_nsec(&r, &a, (NSEC_PER_SEC * 2ULL) + 2));
    EXPECT_EQ(r.tv_sec, 3);
    EXPECT_EQ(r.tv_nsec, 1);
 
    r.tv_sec = 4;
    r.tv_nsec = 0;
-   timespec_add_nsec(&r, &r, NSEC_PER_SEC + 10ULL);
+   EXPECT_FALSE(timespec_add_nsec(&r, &r, NSEC_PER_SEC + 10ULL));
    EXPECT_EQ(5, r.tv_sec);
    EXPECT_EQ(10, r.tv_nsec);
 
-   timespec_add_nsec(&r, &r, (NSEC_PER_SEC * 3ULL) - 9ULL);
+   EXPECT_FALSE(timespec_add_nsec(&r, &r, (NSEC_PER_SEC * 3ULL) - 9ULL));
    EXPECT_EQ(8, r.tv_sec);
    EXPECT_EQ(1, r.tv_nsec);
 
-   timespec_add_nsec(&r, &r, (NSEC_PER_SEC * 7ULL) + (NSEC_PER_SEC - 1ULL));
+   EXPECT_FALSE(timespec_add_nsec(&r, &r, (NSEC_PER_SEC * 7ULL) +
+                                          (NSEC_PER_SEC - 1ULL)));
    EXPECT_EQ(16, r.tv_sec);
    EXPECT_EQ(0, r.tv_nsec);
+
+   a.tv_sec = TIME_T_MAX;
+   a.tv_nsec = 0;
+   EXPECT_TRUE(timespec_add_nsec(&r, &a, UINT64_MAX));
+
+   a.tv_sec = TIME_T_MAX;
+   a.tv_nsec = 0;
+   EXPECT_TRUE(timespec_add_nsec(&r, &a, NSEC_PER_SEC));
+
+   a.tv_sec = TIME_T_MAX;
+   a.tv_nsec = NSEC_PER_SEC / 2;
+   EXPECT_TRUE(timespec_add_nsec(&r, &a, NSEC_PER_SEC / 2));
 }
 
 TEST(timespec_test, timespec_add_msec)
diff --git a/src/util/timespec.h b/src/util/timespec.h
index fbc3a89bd11..83411090ffa 100644
--- a/src/util/timespec.h
+++ b/src/util/timespec.h
@@ -37,6 +37,8 @@
 #include <time.h>
 #include <stdbool.h>
 
+#include "macros.h"
+
 #define NSEC_PER_SEC 1000000000
 
 /**
@@ -77,26 +79,41 @@ timespec_sub(struct timespec *r,
    }
 }
 
+#define TIME_T_MAX \
+   ((time_t)(((time_t)-1) > 0 ? u_uintN_max(sizeof(time_t) * 8) : \
+                                u_intN_max(sizeof(time_t) * 8)))
+
 /**
  * Add a nanosecond value to a timespec
  *
  * \param r[out] result: a + b
  * \param a[in] base operand as timespec
  * \param b[in] operand in nanoseconds
+ * \return true if the calculation overflowed
  */
-static inline void
+static inline bool
 timespec_add_nsec(struct timespec *r, const struct timespec *a, uint64_t b)
 {
-   r->tv_sec = a->tv_sec + (b / NSEC_PER_SEC);
-   r->tv_nsec = a->tv_nsec + (b % NSEC_PER_SEC);
+   uint64_t b_sec = b / NSEC_PER_SEC;
+   long b_nsec = b % NSEC_PER_SEC;
+   bool overflow = (b_sec > (uint64_t)TIME_T_MAX) ||
+                   ((uint64_t)a->tv_sec > (uint64_t)TIME_T_MAX - b_sec);
+
+   r->tv_sec = (uint64_t)a->tv_sec + b_sec;
+   r->tv_nsec = (uint64_t)a->tv_nsec + b_nsec;
 
    if (r->tv_nsec >= NSEC_PER_SEC) {
-      r->tv_sec++;
+      if (r->tv_sec >= TIME_T_MAX)
+         overflow = true;
+      r->tv_sec = (uint64_t)r->tv_sec + 1ull;
       r->tv_nsec -= NSEC_PER_SEC;
    } else if (r->tv_nsec < 0) {
+      assert(overflow);
       r->tv_sec--;
       r->tv_nsec += NSEC_PER_SEC;
    }
+
+   return overflow;
 }
 
 /**
@@ -105,11 +122,12 @@ timespec_add_nsec(struct timespec *r, const struct timespec *a, uint64_t b)
  * \param r[out] result: a + b
  * \param a[in] base operand as timespec
  * \param b[in] operand in milliseconds
+ * \return true if the calculation overflowed
  */
-static inline void
+static inline bool
 timespec_add_msec(struct timespec *r, const struct timespec *a, uint64_t b)
 {
-   timespec_add_nsec(r, a, b * 1000000);
+   return timespec_add_nsec(r, a, b * 1000000) || b > (UINT64_MAX / 1000000);
 }
 
 /**