Implement POSIX time, gmtime_r portably (youtube#2238)

- Add unit tests - Add Windows emulation for gmtime_r - Remove eztime_poem usage in boringssl, protobuf - Remove unused redefinitions from eztime_poem b/320398326 Test-On-Device: true
TyHolc · Jan 25, 2024 · 064f162 · 064f162
1 parent 5868085
commit 064f162
Show file tree

Hide file tree

Showing 19 changed files with 159 additions and 57 deletions.
diff --git a/base/time/time_now_starboard.cc b/base/time/time_now_starboard.cc
@@ -19,7 +19,6 @@
 #include "base/time/time_override.h"
 #include "build/build_config.h"
 
-#include "starboard/client_porting/poem/eztime_poem.h"
 #include "starboard/common/log.h"
 #include "starboard/common/time.h"
 #include "starboard/types.h"

diff --git a/starboard/build/config/modular/BUILD.gn b/starboard/build/config/modular/BUILD.gn
@@ -182,6 +182,8 @@ config("modular") {
     ldflags += [
       "-Wl,--wrap=clock_gettime",
       "-Wl,--wrap=gettimeofday",
+      "-Wl,--wrap=time",
+      "-Wl,--wrap=gmtime_r",
       "-Wl,--wrap=mmap",
     ]
   }

diff --git a/starboard/client_porting/poem/eztime_poem.h b/starboard/client_porting/poem/eztime_poem.h
@@ -24,29 +24,10 @@
 
 #if !defined(POEM_NO_EMULATION)
 
-#undef time_t
-#define time_t EzTimeT
-
-#undef tm
-#define tm EzTimeExploded
-
-#undef timeval
-#define timeval EzTimeValue
-
-#undef gettimeofday
-#define gettimeofday(a, b) EzTimeValueGetNow(a, b)
-#undef gmtime_r
-#define gmtime_r(a, b) EzTimeTExplodeUTC(a, b)
 #undef localtime_r
 #define localtime_r(a, b) EzTimeTExplodeLocal(a, b)
 #undef mktime
 #define mktime(x) EzTimeTImplodeLocal(x)
-#undef time
-#define time(x) EzTimeTGetNow(x)
-#undef timegm
-#define timegm(x) EzTimeTImplodeUTC(x)
-#undef timelocal
-#define timelocal(x) EzTimeTImplodeLocal(x)
 
 #endif  // POEM_NO_EMULATION
 

diff --git a/starboard/elf_loader/exported_symbols.cc b/starboard/elf_loader/exported_symbols.cc
@@ -16,7 +16,6 @@
 
 #include <stdlib.h>
 #include <sys/mman.h>
-#include <time.h>
 
 #include "starboard/accessibility.h"
 #include "starboard/audio_sink.h"
@@ -417,8 +416,6 @@ ExportedSymbols::ExportedSymbols() {
   REGISTER_SYMBOL(posix_memalign);
   REGISTER_SYMBOL(free);
   REGISTER_SYMBOL(vsscanf);
-  REGISTER_SYMBOL(time);
-  REGISTER_SYMBOL(mmap);
   REGISTER_SYMBOL(mprotect);
   REGISTER_SYMBOL(munmap);
   REGISTER_SYMBOL(msync);
@@ -430,6 +427,8 @@ ExportedSymbols::ExportedSymbols() {
   // TODO: b/316603042 - Detect via NPLB and only add the wrapper if needed.
   map_["clock_gettime"] = reinterpret_cast<const void*>(&__wrap_clock_gettime);
   map_["gettimeofday"] = reinterpret_cast<const void*>(&__wrap_gettimeofday);
+  map_["time"] = reinterpret_cast<const void*>(&__wrap_time);
+  map_["gmtime_r"] = reinterpret_cast<const void*>(&__wrap_gmtime_r);
   map_["mmap"] = reinterpret_cast<const void*>(&__wrap_mmap);
 
   REGISTER_SYMBOL(sprintf);

diff --git a/starboard/nplb/posix_compliance/posix_time_test.cc b/starboard/nplb/posix_compliance/posix_time_test.cc
@@ -14,13 +14,30 @@
 
 #if SB_API_VERSION >= 16
 
+#include <sys/time.h>
+#include <time.h>
 #include "starboard/common/time.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace starboard {
 namespace nplb {
 namespace {
 
+TEST(PosixTimeTest, TimeMatchesGettimeofday) {
+  time_t other_time_s = 0;
+  time_t time_s = time(&other_time_s);  // Seconds since Unix epoch.
+  struct timeval tv;
+  gettimeofday(&tv, NULL);  // Microseconds since Unix epoch.
+
+  EXPECT_EQ(time_s, other_time_s);
+
+  int64_t time_us = static_cast<int64_t>(time_s) * 1'000'000;
+  int64_t gettimeofday_us =
+      (static_cast<int64_t>(tv.tv_sec) * 1'000'000) + tv.tv_usec;
+  // Values should be within 1 second of each other.
+  EXPECT_NEAR(time_us, gettimeofday_us, 1'000'000);
+}
+
 TEST(PosixTimeTest, TimeIsKindOfSane) {
   int64_t now_usec = CurrentPosixTime();
 
@@ -50,7 +67,7 @@ TEST(PosixTimeTest, HasDecentResolution) {
   }
 }
 
-TEST(PosixTimeTest, MonotonickIsMonotonic) {
+TEST(PosixTimeTest, MonotonicIsMonotonic) {
   const int kTrials = 100;
   for (int trial = 0; trial < kTrials; ++trial) {
     int64_t timerStart = CurrentPosixTime();
@@ -77,6 +94,25 @@ TEST(PosixTimeTest, MonotonickIsMonotonic) {
   }
 }
 
+TEST(PosixTimeTest, GmtimeR) {
+  time_t timer = 1722468779;  // Wed 2024-07-31 23:32:59 UTC.
+  struct tm result;
+  memset(&result, 0, sizeof(result));
+
+  struct tm* retval = NULL;
+  retval = gmtime_r(&timer, &result);
+  EXPECT_EQ(retval, &result);
+  EXPECT_EQ(result.tm_year, 2024 - 1900);  // Year since 1900.
+  EXPECT_EQ(result.tm_mon, 7 - 1);         // Zero-indexed.
+  EXPECT_EQ(result.tm_mday, 31);
+  EXPECT_EQ(result.tm_hour, 23);
+  EXPECT_EQ(result.tm_min, 32);
+  EXPECT_EQ(result.tm_sec, 59);
+  EXPECT_EQ(result.tm_wday, 3);    // Wednesday, 0==Sunday.
+  EXPECT_EQ(result.tm_yday, 212);  // Zero-indexed; 2024 is a leap year.
+  EXPECT_EQ(result.tm_isdst, 0);   // GMT/UTC never has DST (even in July).
+}
+
 }  // namespace
 }  // namespace nplb
 }  // namespace starboard

diff --git a/starboard/shared/modular/posix_time_wrappers.cc b/starboard/shared/modular/posix_time_wrappers.cc
@@ -16,7 +16,7 @@
 
 #include "starboard/log.h"
 
-int __wrap_clock_gettime(int /*clockid_t */ musl_clock_id,
+int __wrap_clock_gettime(int /* clockid_t */ musl_clock_id,
                          struct musl_timespec* mts) {
   if (!mts) {
     return -1;
@@ -59,3 +59,34 @@ int __wrap_gettimeofday(struct musl_timeval* mtv, void* tzp) {
   mtv->tv_usec = tv.tv_usec;
   return retval;
 }
+
+int64_t __wrap_time(int64_t* /* time_t* */ musl_tloc) {
+  time_t t = time(NULL);  // The type from platform toolchain (may be 32-bits).
+  int64_t retval = static_cast<int64_t>(t);
+  if (musl_tloc) {
+    *musl_tloc = retval;
+  }
+  return retval;
+}
+
+struct musl_tm* __wrap_gmtime_r(const int64_t* /* time_t* */ musl_timer,
+                                struct musl_tm* musl_result) {
+  if (!musl_timer || !musl_result) {
+    return NULL;
+  }
+  time_t t = static_cast<time_t>(*musl_timer);  // Platform type may be 32-bits.
+  struct tm tm;  // The type from platform toolchain.
+  if (!gmtime_r(&t, &tm)) {
+    return NULL;
+  }
+  musl_result->tm_sec = tm.tm_sec;
+  musl_result->tm_min = tm.tm_min;
+  musl_result->tm_hour = tm.tm_hour;
+  musl_result->tm_mday = tm.tm_mday;
+  musl_result->tm_mon = tm.tm_mon;
+  musl_result->tm_year = tm.tm_year;
+  musl_result->tm_wday = tm.tm_wday;
+  musl_result->tm_yday = tm.tm_yday;
+  musl_result->tm_isdst = tm.tm_isdst;
+  return musl_result;
+}
diff --git a/starboard/shared/modular/posix_time_wrappers.h b/starboard/shared/modular/posix_time_wrappers.h
@@ -72,34 +72,53 @@ extern "C" {
 #define __SB_BYTE_ORDER 1234
 #endif
 #if SB_IS(ARCH_ARM64) || SB_IS(ARCH_X64)
-#define __SB_LONG_TYPE int64_t
+#define __MUSL_LONG_TYPE int64_t
 #else
-#define __SB_LONG_TYPE int32_t
+#define __MUSL_LONG_TYPE int32_t
 #endif
 // Note, these structs need to ABI match the musl definitions.
 struct musl_timespec {
   int64_t /* time_t */ tv_sec;
-  int : 8 * (sizeof(int64_t) - sizeof(__SB_LONG_TYPE)) *
+  int : 8 * (sizeof(int64_t) - sizeof(__MUSL_LONG_TYPE)) *
       (__SB_BYTE_ORDER == 4321);
-  __SB_LONG_TYPE /* long */ tv_nsec;
-  int : 8 * (sizeof(int64_t) - sizeof(__SB_LONG_TYPE)) *
+  __MUSL_LONG_TYPE /* long */ tv_nsec;
+  int : 8 * (sizeof(int64_t) - sizeof(__MUSL_LONG_TYPE)) *
       (__SB_BYTE_ORDER != 4321);
 };
 struct musl_timeval {
   int64_t /* time_t */ tv_sec;
   int64_t /* suseconds_t */ tv_usec;
 };
+struct musl_tm {
+  int32_t /* int */ tm_sec;
+  int32_t /* int */ tm_min;
+  int32_t /* int */ tm_hour;
+  int32_t /* int */ tm_mday;
+  int32_t /* int */ tm_mon;
+  int32_t /* int */ tm_year;
+  int32_t /* int */ tm_wday;
+  int32_t /* int */ tm_yday;
+  int32_t /* int */ tm_isdst;
+  __MUSL_LONG_TYPE /* long */ __tm_gmtoff;
+  const char* __tm_zone;
+};
 // Copying macro constants from //third_party/musl/include/time.h
 #define MUSL_CLOCK_REALTIME 0
 #define MUSL_CLOCK_MONOTONIC 1
 #define MUSL_CLOCK_PROCESS_CPUTIME_ID 2
 #define MUSL_CLOCK_THREAD_CPUTIME_ID 3
 
-SB_EXPORT int __wrap_clock_gettime(int /*clockid_t */ musl_clock_id,
+SB_EXPORT int __wrap_clock_gettime(int /* clockid_t */ musl_clock_id,
                                    struct musl_timespec* mts);
 
 SB_EXPORT int __wrap_gettimeofday(struct musl_timeval* mtv, void* tzp);
 
+SB_EXPORT int64_t __wrap_time(int64_t* /* time_t* */ musl_tloc);
+
+SB_EXPORT struct musl_tm* __wrap_gmtime_r(
+    const int64_t* /* time_t* */ musl_timer,
+    struct musl_tm* musl_result);
+
 #ifdef __cplusplus
 }  // extern "C"
 #endif

diff --git a/starboard/shared/win32/posix_emu/include/posix_force_include.h b/starboard/shared/win32/posix_emu/include/posix_force_include.h
@@ -40,6 +40,9 @@ extern "C" {
 // https://pubs.opengroup.org/onlinepubs/9699919799/functions/clock_gettime.html
 int clock_gettime(clockid_t clock_id, struct timespec* tp);
 
+// https://pubs.opengroup.org/onlinepubs/000095399/functions/gmtime_r.html
+struct tm* gmtime_r(const time_t* timer, struct tm* result);
+
 int posix_memalign(void** res, size_t alignment, size_t size);
 
 #ifdef __cplusplus

diff --git a/starboard/shared/win32/posix_emu/time.cc b/starboard/shared/win32/posix_emu/time.cc
@@ -97,3 +97,10 @@ extern "C" int clock_gettime(clockid_t clock_id, struct timespec* tp) {
   }
   return -1;
 }
+
+extern "C" struct tm* gmtime_r(const time_t* timer, struct tm* result) {
+  if (gmtime_s(result, timer) != 0) {
+    return NULL;
+  }
+  return result;
+}
diff --git a/starboard/tools/api_leak_detector/api_leak_detector.py b/starboard/tools/api_leak_detector/api_leak_detector.py
@@ -91,6 +91,7 @@
     'clock_gettime',
     'free',
     'gettimeofday',
+    'gmtime_r',
     'malloc',
     'posix_memalign',
     'realloc',

diff --git a/third_party/boringssl/BUILD.gn b/third_party/boringssl/BUILD.gn
@@ -192,7 +192,6 @@ if (!use_cobalt_customizations) {
     public -= [ "src/include/openssl/opensslconf.h" ]
     public_deps = [
       "//starboard:starboard_headers_only",
-      "//starboard/client_porting/eztime",
     ]
     configs -= [ "//starboard/build/config:size" ]
     configs += [ "//starboard/build/config:speed" ]

diff --git a/third_party/boringssl/src/config/starboard/openssl/opensslconf.h b/third_party/boringssl/src/config/starboard/openssl/opensslconf.h
@@ -2,7 +2,6 @@
 /* WARNING: Edited heavily by hand, based on lbshell config. Meant for all
  * starboard platforms. */
 
-#include "starboard/client_porting/eztime/eztime.h"
 #include "starboard/common/log.h"
 #include "starboard/configuration.h"
 #include "starboard/file.h"
@@ -224,10 +223,9 @@
 
 // Types that need to be ported.
 
-// Use EzTime simulated POSIX types.
-#define OPENSSL_port_tm EzTimeExploded
-#define OPENSSL_port_time_t EzTimeT
-#define OPENSSL_port_timeval EzTimeValue
+// Resolve shim types as POSIX types.
+#define OPENSSL_port_time_t time_t
+#define OPENSSL_port_timeval struct timeval
 
 // Definitions for system calls that may need to be overridden.
 #define OPENSSL_port_free free
@@ -239,14 +237,14 @@
 #define OPENSSL_port_abort SbSystemBreakIntoDebugger
 #define OPENSSL_port_assert(x) SB_DCHECK(x)
 #define OPENSSL_port_getenv(x) NULL
-#define OPENSSL_port_gettimeofday EzTimeValueGetNow
-#define OPENSSL_port_gmtime_r EzTimeTExplodeUTC
+#define OPENSSL_port_gettimeofday gettimeofday
+#define OPENSSL_port_gmtime_r gmtime_r
 #define OPENSSL_port_printf SbLogFormatF
 #define OPENSSL_port_printferr SbLogFormatF
 #define OPENSSL_port_strcasecmp SbStringCompareNoCase
 #define OPENSSL_port_strerror(x) ""
 #define OPENSSL_port_strncasecmp SbStringCompareNoCaseN
-#define OPENSSL_port_time EzTimeTGetNow
+#define OPENSSL_port_time time
 
 // Variables that need to be ported.
 #define OPENSSL_port_errno SbSystemGetLastError()

diff --git a/third_party/boringssl/src/crypto/asn1/internal.h b/third_party/boringssl/src/crypto/asn1/internal.h
@@ -63,11 +63,6 @@
 
 #include <openssl/asn1.h>
 
-#if defined(OPENSSL_SYS_STARBOARD)
-#include <stdlib.h>
-#include "starboard/client_porting/poem/eztime_poem.h"
-#endif  // defined(OPENSSL_SYS_STARBOARD)
-
 #if defined(__cplusplus)
 extern "C" {
 #endif

diff --git a/third_party/boringssl/src/ssl/ssl_session.cc b/third_party/boringssl/src/ssl/ssl_session.cc
@@ -946,13 +946,9 @@ ssl_session_st::ssl_session_st(const SSL_X509_METHOD *method)
       is_quic(false),
       has_application_settings(false) {
   CRYPTO_new_ex_data(&ex_data);
-#ifdef STARBOARD
-  time = OPENSSL_port_time(nullptr);
-#else
   // OPENSSL_port_time can't be used here because the name conflict between
   // the variable and system call needs to be resolved.
   time = ::time(nullptr);
-#endif
 }
 
 ssl_session_st::~ssl_session_st() {

diff --git a/third_party/musl/BUILD.gn b/third_party/musl/BUILD.gn
@@ -397,6 +397,8 @@ static_library("c_internal") {
     "src/starboard/sys/time/gettimeofday.c",
     "src/starboard/time/__tz.c",
     "src/starboard/time/clock_gettime.c",
+    "src/starboard/time/gmtime_r.c",
+    "src/starboard/time/time.c",
     "src/stdio/__toread.c",
     "src/stdio/__uflow.c",
     "src/stdio/fprintf.c",
@@ -525,6 +527,7 @@ static_library("c_internal") {
 
   deps = [
     "//starboard:starboard_headers_only",
+    "//starboard/client_porting/eztime",
     "//starboard/common:common_headers_only",
   ]
 }

diff --git a/third_party/musl/include/time.h b/third_party/musl/include/time.h
@@ -144,7 +144,9 @@ __REDIR(timespec_get, __timespec_get_time64);
 #if defined(_POSIX_SOURCE) || defined(_POSIX_C_SOURCE) \
  || defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) \
  || defined(_BSD_SOURCE)
+#if !defined(STARBOARD)
 __REDIR(gmtime_r, __gmtime64_r);
+#endif  // !defined(STARBOARD)
 __REDIR(localtime_r, __localtime64_r);
 __REDIR(ctime_r, __ctime64_r);
 __REDIR(nanosleep, __nanosleep_time64);