[libc-commits] [libc] [libc] Fix overflow check for 32 bit long time_t (PR #65394)

[Mikhail R. Gadelha via libc-commits]

https://github.com/mikhailramalho created https://github.com/llvm/llvm-project/pull/65394:

This patch fixes the overflow check in update_from_seconds, used by gmtime, gmtime_r and mktime.

In update_from_seconds, total_seconds is a int64_t and the previous overflow check for when sizeof(time_t) == 4 would check if it was < 0x80000000 and > 0x7FFFFFFF, however, this check would cause the following issues:

1. Valid negative numbers would be discarded, e.g., -1 is 0xffffffffffffffff as a int64_t, outside the range of the overflow check.

2. Some valid positive numbers would be discarded because the hex constants were being implicitly converted to int64_t, e.g., 0x80000000 would be implicitly converted to 2147483648, instead of -2147483648.

The fix for both cases was to static_cast total_seconds and the constants to time_t if sizeof(time_t) == 4. The behaviour is not changed in systems with sizeof(time_t) == 8.

>From db85022a7f7d3b0d03ce32ee51718625181a0167 Mon Sep 17 00:00:00 2001
From: "Mikhail R. Gadelha" <mikhail at igalia.com>
Date: Tue, 5 Sep 2023 12:25:44 -0300
Subject: [PATCH] [libc] Fix overflow check for 32 bit long time_t

This patch fixes the overflow check in update_from_seconds, used by
gmtime, gmtime_r and mktime.

In update_from_seconds, total_seconds is a int64_t and the previous
overflow check for when sizeof(time_t) == 4 would check if it was
< 0x80000000 and > 0x7FFFFFFF, however, this check would cause the
following issues:

1. Valid negative numbers would be discarded, e.g., -1 is 0xffffffffffffffff
   as a int64_t, outside the range of the overflow check.

2. Some valid positive numbers would be discarded because the hex
   constants were being implicitly converted to int64_t, e.g.,
   0x80000000 would be implicitly converted to 2147483648, instead of
   -2147483648.

The fix for both cases was to static_cast total_seconds and the
constants to time_t if sizeof(time_t) == 4. The behaviour is not changed
in systems with sizeof(time_t) == 8.
---
 libc/src/time/mktime.cpp     |  6 +++---
 libc/src/time/time_utils.cpp | 28 ++++++++++++++--------------
 libc/src/time/time_utils.h   |  2 +-
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/libc/src/time/mktime.cpp b/libc/src/time/mktime.cpp
index ea748ce4ab72737..3b3ea339f850f96 100644
--- a/libc/src/time/mktime.cpp
+++ b/libc/src/time/mktime.cpp
@@ -53,9 +53,9 @@ LLVM_LIBC_FUNCTION(time_t, mktime, (struct tm * tm_out)) {
 
   // Years are ints.  A 32-bit year will fit into a 64-bit time_t.
   // A 64-bit year will not.
-  static_assert(sizeof(int) == 4,
-                "ILP64 is unimplemented.  This implementation requires "
-                "32-bit integers.");
+  static_assert(
+      sizeof(int) == 4,
+      "ILP64 is unimplemented. This implementation requires 32-bit integers.");
 
   // Calculate number of months and years from tm_mon.
   int64_t month = tm_out->tm_mon;
diff --git a/libc/src/time/time_utils.cpp b/libc/src/time/time_utils.cpp
index 2cb0f071e6ce111..181ea2588b6ec98 100644
--- a/libc/src/time/time_utils.cpp
+++ b/libc/src/time/time_utils.cpp
@@ -48,20 +48,20 @@ int64_t update_from_seconds(int64_t total_seconds, struct tm *tm) {
   static const char daysInMonth[] = {31 /* Mar */, 30, 31, 30, 31, 31,
                                      30,           31, 30, 31, 31, 29};
 
-  if (sizeof(time_t) == 4) {
-    if (total_seconds < 0x80000000)
-      return time_utils::out_of_range();
-    if (total_seconds > 0x7FFFFFFF)
-      return time_utils::out_of_range();
-  } else {
-    if (total_seconds <
-            INT_MIN * static_cast<int64_t>(
-                          TimeConstants::NUMBER_OF_SECONDS_IN_LEAP_YEAR) ||
-        total_seconds >
-            INT_MAX * static_cast<int64_t>(
-                          TimeConstants::NUMBER_OF_SECONDS_IN_LEAP_YEAR))
-      return time_utils::out_of_range();
-  }
+  constexpr time_t time_min =
+      (sizeof(time_t) == 4)
+          ? static_cast<time_t>(0x80000000)
+          : INT_MIN * static_cast<int64_t>(
+                          TimeConstants::NUMBER_OF_SECONDS_IN_LEAP_YEAR);
+  constexpr time_t time_max =
+      (sizeof(time_t) == 4)
+          ? static_cast<time_t>(0x7FFFFFFF)
+          : INT_MAX * static_cast<int64_t>(
+                          TimeConstants::NUMBER_OF_SECONDS_IN_LEAP_YEAR);
+
+  time_t ts = static_cast<time_t>(total_seconds);
+  if (ts < time_min || ts > time_max)
+    return time_utils::out_of_range();
 
   int64_t seconds =
       total_seconds - TimeConstants::SECONDS_UNTIL2000_MARCH_FIRST;
diff --git a/libc/src/time/time_utils.h b/libc/src/time/time_utils.h
index 8075bfe32027ae6..424e7521504d917 100644
--- a/libc/src/time/time_utils.h
+++ b/libc/src/time/time_utils.h
@@ -92,7 +92,7 @@ extern int64_t update_from_seconds(int64_t total_seconds, struct tm *tm);
 // POSIX.1-2017 requires this.
 LIBC_INLINE time_t out_of_range() {
   libc_errno = EOVERFLOW;
-  return static_cast<time_t>(-1);
+  return TimeConstants::OUT_OF_RANGE_RETURN_VALUE;
 }
 
 LIBC_INLINE void invalid_value() { libc_errno = EINVAL; }