From 064f16269c4eeee071d01fae71133ad6f0b6852a Mon Sep 17 00:00:00 2001 From: Garo Bournoutian Date: Thu, 25 Jan 2024 11:25:39 -0800 Subject: [PATCH] Implement POSIX time, gmtime_r portably (#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 --- base/time/time_now_starboard.cc | 1 - starboard/build/config/modular/BUILD.gn | 2 + starboard/client_porting/poem/eztime_poem.h | 19 ---------- starboard/elf_loader/exported_symbols.cc | 5 +-- .../nplb/posix_compliance/posix_time_test.cc | 38 ++++++++++++++++++- .../shared/modular/posix_time_wrappers.cc | 33 +++++++++++++++- .../shared/modular/posix_time_wrappers.h | 31 ++++++++++++--- .../posix_emu/include/posix_force_include.h | 3 ++ starboard/shared/win32/posix_emu/time.cc | 7 ++++ .../api_leak_detector/api_leak_detector.py | 1 + third_party/boringssl/BUILD.gn | 1 - .../config/starboard/openssl/opensslconf.h | 14 +++---- .../boringssl/src/crypto/asn1/internal.h | 5 --- third_party/boringssl/src/ssl/ssl_session.cc | 4 -- third_party/musl/BUILD.gn | 3 ++ third_party/musl/include/time.h | 2 + .../musl/src/starboard/time/gmtime_r.c | 28 ++++++++++++++ third_party/musl/src/starboard/time/time.c | 15 ++++++-- .../src/google/protobuf/stubs/time.cc | 4 -- 19 files changed, 159 insertions(+), 57 deletions(-) create mode 100644 third_party/musl/src/starboard/time/gmtime_r.c diff --git a/base/time/time_now_starboard.cc b/base/time/time_now_starboard.cc index 8332e5bf4683..9e40e42b0442 100644 --- 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 index b335b334b298..b8a864db52ed 100644 --- 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 index a1a295fc4af0..04d0393e03df 100644 --- 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 index 469cca0f5d1e..3bffb8d9f02b 100644 --- a/starboard/elf_loader/exported_symbols.cc +++ b/starboard/elf_loader/exported_symbols.cc @@ -16,7 +16,6 @@ #include #include -#include #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(&__wrap_clock_gettime); map_["gettimeofday"] = reinterpret_cast(&__wrap_gettimeofday); + map_["time"] = reinterpret_cast(&__wrap_time); + map_["gmtime_r"] = reinterpret_cast(&__wrap_gmtime_r); map_["mmap"] = reinterpret_cast(&__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 index ae79072c0e68..5c41b0236d50 100644 --- a/starboard/nplb/posix_compliance/posix_time_test.cc +++ b/starboard/nplb/posix_compliance/posix_time_test.cc @@ -14,6 +14,8 @@ #if SB_API_VERSION >= 16 +#include +#include #include "starboard/common/time.h" #include "testing/gtest/include/gtest/gtest.h" @@ -21,6 +23,21 @@ 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(time_s) * 1'000'000; + int64_t gettimeofday_us = + (static_cast(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 index 42b93a52e1e0..d95586a3e29e 100644 --- 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(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(*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 index 2e0afb49f038..373327329c11 100644 --- 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 index 89fa748df0a0..3b8a807c4fbf 100644 --- 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 index b86b00523e8e..865d72afda86 100644 --- 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 index a38869d9b5cc..cef89e99f812 100755 --- 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 index 58a78e34ab82..7031a645154a 100644 --- 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 index 046d2d3c535d..4c31b5e9c153 100644 --- 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 index f951ec376213..a4bd34e5b27e 100644 --- a/third_party/boringssl/src/crypto/asn1/internal.h +++ b/third_party/boringssl/src/crypto/asn1/internal.h @@ -63,11 +63,6 @@ #include -#if defined(OPENSSL_SYS_STARBOARD) -#include -#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 index cbb20012b446..1ede94f91944 100644 --- 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 index 38b8b3c2a819..0cb46e2af569 100644 --- 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 index ddf7a5f952ea..9d31ccac67f8 100644 --- 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); diff --git a/third_party/musl/src/starboard/time/gmtime_r.c b/third_party/musl/src/starboard/time/gmtime_r.c new file mode 100644 index 000000000000..7e89690add12 --- /dev/null +++ b/third_party/musl/src/starboard/time/gmtime_r.c @@ -0,0 +1,28 @@ +#if SB_API_VERSION < 16 + +#include + +#include "starboard/client_porting/eztime/eztime.h" + +struct tm *gmtime_r(const time_t *restrict t, struct tm *restrict tm) { + if (!t || !tm) { + return NULL; + } + EzTimeT ezt = (EzTimeT)*t; + EzTimeExploded ezte; + if (EzTimeTExplodeUTC(&ezt, &ezte) == NULL) { + return NULL; + } + tm->tm_sec = ezte.tm_sec; + tm->tm_min = ezte.tm_min; + tm->tm_hour = ezte.tm_hour; + tm->tm_mday = ezte.tm_mday; + tm->tm_mon = ezte.tm_mon; + tm->tm_year = ezte.tm_year; + tm->tm_wday = ezte.tm_wday; + tm->tm_yday = ezte.tm_yday; + tm->tm_isdst = ezte.tm_isdst; + return tm; +} + +#endif // SB_API_VERSION < 16 diff --git a/third_party/musl/src/starboard/time/time.c b/third_party/musl/src/starboard/time/time.c index db40849b8cb0..da7654d49c5c 100644 --- a/third_party/musl/src/starboard/time/time.c +++ b/third_party/musl/src/starboard/time/time.c @@ -1,11 +1,18 @@ -#include - #if SB_API_VERSION < 16 -#include "starboard/client_porting/eztime/eztime.h" +#include + +#include "starboard/time.h" time_t time(time_t *t) { - return EzTimeTGetNow(t); + int64_t posix_us = SbTimeToPosix(SbTimeGetNow()); + int64_t posix_s = posix_us >= 0 ? posix_us / 1000000 + : (posix_us - 1000000 + 1) / 1000000; + time_t time_s = (time_t)posix_s; + if (t) { + *t = time_s; + } + return time_s; } #endif // SB_API_VERSION < 16 diff --git a/third_party/protobuf/src/google/protobuf/stubs/time.cc b/third_party/protobuf/src/google/protobuf/stubs/time.cc index 615d433615b6..49c0412c1766 100644 --- a/third_party/protobuf/src/google/protobuf/stubs/time.cc +++ b/third_party/protobuf/src/google/protobuf/stubs/time.cc @@ -5,10 +5,6 @@ #include #include -#if defined(STARBOARD) -#include "starboard/client_porting/poem/eztime_poem.h" -#endif - namespace google { namespace protobuf { namespace internal {