[PATCH v2 0/1] MR5024: msvcrt: Adjust _gmtime64_s() accepted time limits.
Fixes a crash in Microsoft Flight Simulator (which passes some currently invalid values to ucrtbase._gmtime64() and doesn't expect NULL result). It looks like any msvcrt version on modern Windows accepts wider range of values. Meanwhile, the exact range is different for ucrtbase vs older CRT versions (the exact values for ucrtbase also vary before Win10 1909). Also, older msvcrt.dll matches what we have in Wine now. So the patch keeps old msvcrt behavior, adds universally correct limits for msvcrt 70-120 and the latest (Win10 1909+) for ucrtbase. -- v2: msvcrt: Adjust _gmtime64_s() accepted time limits. https://gitlab.winehq.org/wine/wine/-/merge_requests/5024
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/msvcr120/tests/msvcr120.c | 78 ++++++++++++++++++++++++++++++++++ dlls/msvcrt/tests/time.c | 77 +++++++++++++++++++++++++++++++++ dlls/msvcrt/time.c | 15 ++++++- dlls/ucrtbase/tests/misc.c | 75 ++++++++++++++++++++++++++++++++ include/msvcrt/time.h | 3 ++ 5 files changed, 247 insertions(+), 1 deletion(-) diff --git a/dlls/msvcr120/tests/msvcr120.c b/dlls/msvcr120/tests/msvcr120.c index c76db6582cf..656004f4826 100644 --- a/dlls/msvcr120/tests/msvcr120.c +++ b/dlls/msvcr120/tests/msvcr120.c @@ -35,6 +35,8 @@ #include <locale.h> +#define _MAX__TIME64_T (((__time64_t)0x00000007 << 32) | 0x93406FFF) + #ifdef __i386__ #include "pshpack1.h" struct thiscall_thunk @@ -226,6 +228,10 @@ static wctrans_t (__cdecl *p_wctrans)(const char*); static wint_t (__cdecl *p_towctrans)(wint_t, wctrans_t); static int (__cdecl *p_strcmp)(const char *, const char *); static int (__cdecl *p_strncmp)(const char *, const char *, size_t); +static struct tm* (__cdecl *p_gmtime32)(__time32_t*); +static errno_t (__cdecl *p_gmtime32_s)(struct tm*, __time32_t*); +static struct tm* (__cdecl *p_gmtime64)(__time64_t*); +static errno_t (__cdecl *p_gmtime64_s)(struct tm*, __time64_t*); /* make sure we use the correct errno */ #undef errno @@ -296,6 +302,10 @@ static BOOL init(void) p_errno = (void*)GetProcAddress(module, "_errno"); p_wcreate_locale = (void*)GetProcAddress(module, "_wcreate_locale"); p_free_locale = (void*)GetProcAddress(module, "_free_locale"); + p_gmtime64 = (void*)GetProcAddress(module, "_gmtime64"); + p_gmtime64_s = (void*)GetProcAddress(module, "_gmtime64_s"); + p_gmtime32 = (void*)GetProcAddress(module, "_gmtime32"); + p_gmtime32_s = (void*)GetProcAddress(module, "_gmtime32_s"); SET(p_wctype, "wctype"); SET(p_fegetenv, "fegetenv"); SET(p_fesetenv, "fesetenv"); @@ -1714,6 +1724,73 @@ static void test_strcmp(void) ok( ret == 0, "wrong ret %d\n", ret ); } +static void test_gmtime64(void) +{ + struct tm *ptm, tm; + __time64_t t; + int ret; + + t = -1; + memset(&tm, 0xcc, sizeof(tm)); + ptm = p_gmtime64(&t); + ok(!!ptm, "got NULL.\n"); + ret = p_gmtime64_s(&tm, &t); + ok(!ret, "got %d.\n", ret); + ok(tm.tm_year == 69 && tm.tm_hour == 23 && tm.tm_min == 59 && tm.tm_sec == 59, "got %d, %d, %d, %d.\n", + tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); + + t = -43200; + memset(&tm, 0xcc, sizeof(tm)); + ptm = p_gmtime64(&t); + ok(!!ptm, "got NULL.\n"); + ret = p_gmtime64_s(&tm, &t); + ok(!ret, "got %d.\n", ret); + ok(tm.tm_year == 69 && tm.tm_hour == 12 && tm.tm_min == 0 && tm.tm_sec == 0, "got %d, %d, %d, %d.\n", + tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); + ptm = p_gmtime32((__time32_t *)&t); + ok(!!ptm, "got NULL.\n"); + memset(&tm, 0xcc, sizeof(tm)); + ret = p_gmtime32_s(&tm, (__time32_t *)&t); + ok(!ret, "got %d.\n", ret); + todo_wine_if(tm.tm_year == 69 && tm.tm_hour == 12) + ok(tm.tm_year == 70 && tm.tm_hour == -12 && tm.tm_min == 0 && tm.tm_sec == 0, "got %d, %d, %d, %d.\n", + tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); + + t = -43201; + ptm = p_gmtime64(&t); + ok(!ptm, "got non-NULL.\n"); + memset(&tm, 0xcc, sizeof(tm)); + ret = p_gmtime64_s(&tm, &t); + ok(ret == EINVAL, "got %d.\n", ret); + ok(tm.tm_year == -1 && tm.tm_hour == -1 && tm.tm_min == -1 && tm.tm_sec == -1, "got %d, %d, %d, %d.\n", + tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); + ptm = p_gmtime32((__time32_t *)&t); + ok(!ptm, "got NULL.\n"); + memset(&tm, 0xcc, sizeof(tm)); + ret = p_gmtime32_s(&tm, (__time32_t *)&t); + ok(ret == EINVAL, "got %d.\n", ret); + ok(tm.tm_year == -1 && tm.tm_hour == -1 && tm.tm_min == -1 && tm.tm_sec == -1, "got %d, %d, %d, %d.\n", + tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); + + t = _MAX__TIME64_T + 46800; + memset(&tm, 0xcc, sizeof(tm)); + ptm = p_gmtime64(&t); + ok(!!ptm, "got NULL.\n"); + ret = p_gmtime64_s(&tm, &t); + ok(!ret, "got %d.\n", ret); + ok(tm.tm_year == 1101 && tm.tm_hour == 20 && tm.tm_min == 59 && tm.tm_sec == 59, "got %d, %d, %d, %d.\n", + tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); + + t = _MAX__TIME64_T + 46801; + ptm = p_gmtime64(&t); + ok(!ptm, "got non-NULL.\n"); + memset(&tm, 0xcc, sizeof(tm)); + ret = p_gmtime64_s(&tm, &t); + ok(ret == EINVAL, "got %d.\n", ret); + ok(tm.tm_year == -1 && tm.tm_hour == -1 && tm.tm_min == -1 && tm.tm_sec == -1, "got %d, %d, %d, %d.\n", + tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); +} + START_TEST(msvcr120) { if (!init()) return; @@ -1738,4 +1815,5 @@ START_TEST(msvcr120) test_CurrentContext(); test_StructuredTaskCollection(); test_strcmp(); + test_gmtime64(); } diff --git a/dlls/msvcrt/tests/time.c b/dlls/msvcrt/tests/time.c index 4ed17c940a9..aba911f30a4 100644 --- a/dlls/msvcrt/tests/time.c +++ b/dlls/msvcrt/tests/time.c @@ -55,6 +55,8 @@ static __time32_t (__cdecl *p_mkgmtime32)(struct tm*); static struct tm* (__cdecl *p_gmtime32)(__time32_t*); static struct tm* (__cdecl *p_gmtime)(time_t*); static errno_t (__cdecl *p_gmtime32_s)(struct tm*, __time32_t*); +static struct tm* (__cdecl *p_gmtime64)(__time64_t*); +static errno_t (__cdecl *p_gmtime64_s)(struct tm*, __time64_t*); static errno_t (__cdecl *p_strtime_s)(char*,size_t); static errno_t (__cdecl *p_strdate_s)(char*,size_t); static errno_t (__cdecl *p_localtime32_s)(struct tm*, __time32_t*); @@ -76,6 +78,8 @@ static void init(void) p_gmtime32 = (void*)GetProcAddress(hmod, "_gmtime32"); p_gmtime = (void*)GetProcAddress(hmod, "gmtime"); p_gmtime32_s = (void*)GetProcAddress(hmod, "_gmtime32_s"); + p_gmtime64 = (void*)GetProcAddress(hmod, "_gmtime64"); + p_gmtime64_s = (void*)GetProcAddress(hmod, "_gmtime64_s"); p_mkgmtime32 = (void*)GetProcAddress(hmod, "_mkgmtime32"); p_strtime_s = (void*)GetProcAddress(hmod, "_strtime_s"); p_strdate_s = (void*)GetProcAddress(hmod, "_strdate_s"); @@ -208,6 +212,78 @@ static void test_gmtime(void) } } +static void test_gmtime64(void) +{ + struct tm *ptm, tm; + __time64_t t; + int ret; + + t = -1; + memset(&tm, 0xcc, sizeof(tm)); + ptm = p_gmtime64(&t); + if (!ptm) + { + skip("Old gmtime64 limits, skipping tests.\n"); + return; + } + ok(!!ptm, "got NULL.\n"); + ret = p_gmtime64_s(&tm, &t); + ok(!ret, "got %d.\n", ret); + ok(tm.tm_year == 69 && tm.tm_hour == 23 && tm.tm_min == 59 && tm.tm_sec == 59, "got %d, %d, %d, %d.\n", + tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); + + t = -43200; + memset(&tm, 0xcc, sizeof(tm)); + ptm = p_gmtime64(&t); + ok(!!ptm, "got NULL.\n"); + ret = p_gmtime64_s(&tm, &t); + ok(!ret, "got %d.\n", ret); + ok(tm.tm_year == 69 && tm.tm_hour == 12 && tm.tm_min == 0 && tm.tm_sec == 0, "got %d, %d, %d, %d.\n", + tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); + ptm = p_gmtime32((__time32_t *)&t); + ok(!!ptm, "got NULL.\n"); + memset(&tm, 0xcc, sizeof(tm)); + ret = p_gmtime32_s(&tm, (__time32_t *)&t); + ok(!ret, "got %d.\n", ret); + todo_wine_if(tm.tm_year == 69 && tm.tm_hour == 12) + ok(tm.tm_year == 70 && tm.tm_hour == -12 && tm.tm_min == 0 && tm.tm_sec == 0, "got %d, %d, %d, %d.\n", + tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); + + t = -43201; + ptm = p_gmtime64(&t); + ok(!ptm, "got non-NULL.\n"); + memset(&tm, 0xcc, sizeof(tm)); + ret = p_gmtime64_s(&tm, &t); + ok(ret == EINVAL, "got %d.\n", ret); + ok(tm.tm_year == -1 && tm.tm_hour == -1 && tm.tm_min == -1 && tm.tm_sec == -1, "got %d, %d, %d, %d.\n", + tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); + ptm = p_gmtime32((__time32_t *)&t); + ok(!ptm, "got NULL.\n"); + memset(&tm, 0xcc, sizeof(tm)); + ret = p_gmtime32_s(&tm, (__time32_t *)&t); + ok(ret == EINVAL, "got %d.\n", ret); + ok(tm.tm_year == -1 && tm.tm_hour == -1 && tm.tm_min == -1 && tm.tm_sec == -1, "got %d, %d, %d, %d.\n", + tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); + + t = _MAX__TIME64_T + 46800; + memset(&tm, 0xcc, sizeof(tm)); + ptm = p_gmtime64(&t); + ok(!!ptm, "got NULL.\n"); + ret = p_gmtime64_s(&tm, &t); + ok(!ret, "got %d.\n", ret); + ok(tm.tm_year == 1101 && tm.tm_hour == 20 && tm.tm_min == 59 && tm.tm_sec == 59, "got %d, %d, %d, %d.\n", + tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); + + t = _MAX__TIME64_T + 46801; + ptm = p_gmtime64(&t); + ok(!ptm, "got non-NULL.\n"); + memset(&tm, 0xcc, sizeof(tm)); + ret = p_gmtime64_s(&tm, &t); + ok(ret == EINVAL, "got %d.\n", ret); + ok(tm.tm_year == -1 && tm.tm_hour == -1 && tm.tm_min == -1 && tm.tm_sec == -1, "got %d, %d, %d, %d.\n", + tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); +} + static void test_mktime(void) { TIME_ZONE_INFORMATION tzinfo; @@ -963,6 +1039,7 @@ START_TEST(time) test_strftime(); test_ctime(); test_gmtime(); + test_gmtime64(); test_mktime(); test_localtime(); test_strdate(); diff --git a/dlls/msvcrt/time.c b/dlls/msvcrt/time.c index 56b80e84105..7a110e53455 100644 --- a/dlls/msvcrt/time.c +++ b/dlls/msvcrt/time.c @@ -66,6 +66,17 @@ static const int MAX_SECONDS = 60; static const int MAX_SECONDS = 59; #endif +#if _MSVCR_VER == 0 +#define MIN_GMTIME64_TIME 0 +#define MAX_GMTIME64_TIME _MAX__TIME64_T +#elif _MSVCR_VER >= 140 +#define MIN_GMTIME64_TIME -43200 +#define MAX_GMTIME64_TIME (_MAX__TIME64_T + 1605600) +#else +#define MIN_GMTIME64_TIME -43200 +#define MAX_GMTIME64_TIME (_MAX__TIME64_T + 46800) +#endif + static inline BOOL IsLeapYear(int Year) { return Year % 4 == 0 && (Year % 100 != 0 || Year % 400 == 0); @@ -457,7 +468,9 @@ int CDECL _gmtime64_s(struct tm *res, const __time64_t *secs) SYSTEMTIME st; ULONGLONG time; - if (!res || !secs || *secs < 0 || *secs > _MAX__TIME64_T) { + TRACE("res %p, secs %p (%I64d).\n", res, secs, secs ? *secs : 0); + + if (!res || !secs || *secs < MIN_GMTIME64_TIME || *secs > MAX_GMTIME64_TIME) { if (res) { write_invalid_msvcrt_tm(res); } diff --git a/dlls/ucrtbase/tests/misc.c b/dlls/ucrtbase/tests/misc.c index 86cdec88108..ff1ed20f2f8 100644 --- a/dlls/ucrtbase/tests/misc.c +++ b/dlls/ucrtbase/tests/misc.c @@ -113,6 +113,8 @@ _se_translator_function __cdecl _set_se_translator(_se_translator_function func) void** __cdecl __current_exception(void); int* __cdecl __processing_throw(void); +#define _MAX__TIME64_T (((__time64_t)0x00000007 << 32) | 0x93406FFF) + static void test__initialize_onexit_table(void) { _onexit_table_t table, table2; @@ -1632,6 +1634,78 @@ static void test_rewind_i386_abi(void) } #endif +static void test_gmtime64(void) +{ + struct tm *ptm, tm; + __time64_t t; + int ret; + + t = -1; + memset(&tm, 0xcc, sizeof(tm)); + ptm = _gmtime64(&t); + ok(!!ptm, "got NULL.\n"); + ret = _gmtime64_s(&tm, &t); + ok(!ret, "got %d.\n", ret); + ok(tm.tm_year == 69 && tm.tm_hour == 23 && tm.tm_min == 59 && tm.tm_sec == 59, "got %d, %d, %d, %d.\n", + tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); + + t = -43200; + memset(&tm, 0xcc, sizeof(tm)); + ptm = _gmtime64(&t); + ok(!!ptm, "got NULL.\n"); + ret = _gmtime64_s(&tm, &t); + ok(!ret, "got %d.\n", ret); + ok(tm.tm_year == 69 && tm.tm_hour == 12 && tm.tm_min == 0 && tm.tm_sec == 0, "got %d, %d, %d, %d.\n", + tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); + ptm = _gmtime32((__time32_t *)&t); + ok(!!ptm, "got NULL.\n"); + memset(&tm, 0xcc, sizeof(tm)); + ret = _gmtime32_s(&tm, (__time32_t *)&t); + ok(!ret, "got %d.\n", ret); + todo_wine_if(tm.tm_year == 69 && tm.tm_hour == 12) + ok(tm.tm_year == 70 && tm.tm_hour == -12 && tm.tm_min == 0 && tm.tm_sec == 0, "got %d, %d, %d, %d.\n", + tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); + + t = -43201; + ptm = _gmtime64(&t); + ok(!ptm, "got non-NULL.\n"); + memset(&tm, 0xcc, sizeof(tm)); + ret = _gmtime64_s(&tm, &t); + ok(ret == EINVAL, "got %d.\n", ret); + ok(tm.tm_year == -1 && tm.tm_hour == -1 && tm.tm_min == -1 && tm.tm_sec == -1, "got %d, %d, %d, %d.\n", + tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); + ptm = _gmtime32((__time32_t *)&t); + ok(!ptm, "got NULL.\n"); + memset(&tm, 0xcc, sizeof(tm)); + ret = _gmtime32_s(&tm, (__time32_t *)&t); + ok(ret == EINVAL, "got %d.\n", ret); + ok(tm.tm_year == -1 && tm.tm_hour == -1 && tm.tm_min == -1 && tm.tm_sec == -1, "got %d, %d, %d, %d.\n", + tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); + + t = _MAX__TIME64_T + 1605600; + memset(&tm, 0xcc, sizeof(tm)); + ptm = _gmtime64(&t); + ok(!!ptm || broken(!ptm) /* before Win10 1909 */, "got NULL.\n"); + if (!ptm) + { + win_skip("Old gmtime64 limits, skipping tests.\n"); + return; + } + ret = _gmtime64_s(&tm, &t); + ok(!ret, "got %d.\n", ret); + ok(tm.tm_year == 1101 && tm.tm_hour == 21 && tm.tm_min == 59 && tm.tm_sec == 59, "got %d, %d, %d, %d.\n", + tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); + + t = _MAX__TIME64_T + 1605601; + ptm = _gmtime64(&t); + ok(!ptm, "got non-NULL.\n"); + memset(&tm, 0xcc, sizeof(tm)); + ret = _gmtime64_s(&tm, &t); + ok(ret == EINVAL, "got %d.\n", ret); + ok(tm.tm_year == -1 && tm.tm_hour == -1 && tm.tm_min == -1 && tm.tm_sec == -1, "got %d, %d, %d, %d.\n", + tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); +} + START_TEST(misc) { int arg_c; @@ -1677,4 +1751,5 @@ START_TEST(misc) #if defined(__i386__) test_rewind_i386_abi(); #endif + test_gmtime64(); } diff --git a/include/msvcrt/time.h b/include/msvcrt/time.h index e1d914fea0d..4d79d066184 100644 --- a/include/msvcrt/time.h +++ b/include/msvcrt/time.h @@ -80,7 +80,10 @@ _ACRTIMP errno_t __cdecl _ctime64_s(char*,size_t,const __time64_t*); _ACRTIMP double __cdecl _difftime32(__time32_t,__time32_t); _ACRTIMP double __cdecl _difftime64(__time64_t,__time64_t); _ACRTIMP struct tm* __cdecl _gmtime32(const __time32_t*); +_ACRTIMP int __cdecl _gmtime32_s(struct tm *res, const __time32_t *secs); _ACRTIMP struct tm* __cdecl _gmtime64(const __time64_t*); +_ACRTIMP int __cdecl _gmtime64_s(struct tm *res, const __time64_t *secs); + _ACRTIMP struct tm* __cdecl _localtime32(const __time32_t*); _ACRTIMP errno_t __cdecl _localtime32_s(struct tm*, const __time32_t*); _ACRTIMP struct tm* __cdecl _localtime64(const __time64_t*); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5024
v2: - move _gmtime64_s() definition to include/msvcrt/time.h; - use win_skip() in ucrtbase/tests/misc.c; - remove added (time) debug channel and log to default (msvcrt) and in _gmtime64_s(); - remove a spurious newline; - add tests for _gmtime32 / _gmtime32_s with the same negative values and remove checks for negative values in implementation. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5024#note_60075
Piotr Caban (@piotr) commented about dlls/msvcrt/time.c:
static const int MAX_SECONDS = 59; #endif
+#if _MSVCR_VER == 0 +#define MIN_GMTIME64_TIME 0 +#define MAX_GMTIME64_TIME _MAX__TIME64_T +#elif _MSVCR_VER >= 140 +#define MIN_GMTIME64_TIME -43200 +#define MAX_GMTIME64_TIME (_MAX__TIME64_T + 1605600) +#else +#define MIN_GMTIME64_TIME -43200 +#define MAX_GMTIME64_TIME (_MAX__TIME64_T + 46800) +#endif Shouldn't we update the _MAX__TIME64_T value instead? It looks like the ranges could be also updated in other functions that use this (I didn't check if exact values match between _gmtime64, _wctime64 and _localtime64). Taking in account that the values change between dll version we probably don't have to support exactly the same range as native.
A quick test shows that _MAX__TIME64_T is not correct for _wctime64 in ucrtbase. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5024#note_60190
On Tue Feb 6 11:11:11 2024 +0000, Piotr Caban wrote:
Shouldn't we update the _MAX__TIME64_T value instead? It looks like the ranges could be also updated in other functions that use this (I didn't check if exact values match between _gmtime64, _wctime64 and _localtime64). Taking in account that the values change between dll version we probably don't have to support exactly the same range as native. A quick test shows that _MAX__TIME64_T is not correct for _wctime64 in ucrtbase. I checked with _localtime64_s() on ucrtbase and it doesn't allow negative values. Maximum time is current _MAX__TIME64_T + 1555200 (vs 1605600 for _gmtime64_s), which is 14 hours difference and doesn't match neither negative offset of _gmtime64_s nor my local time difference to GMT (either side).
So do you suggest updating _MAX__TIME64_T constant universally, for all the functions and CRT versions? Maybe we should keep old msvcrt result at least, and then drop test for CRT140 and follow up to date ucrtbase _gmtime64_s value? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5024#note_60255
On Tue Feb 6 15:51:32 2024 +0000, Paul Gofman wrote:
I checked with _localtime64_s() on ucrtbase and it doesn't allow negative values. Maximum time is current _MAX__TIME64_T + 1555200 (vs 1605600 for _gmtime64_s), which is 14 hours difference and doesn't match neither negative offset of _gmtime64_s nor my local time difference to GMT (either side). So do you suggest updating _MAX__TIME64_T constant universally, for all the functions and CRT versions? Maybe we should keep old msvcrt result at least, and then drop test for CRT140 and follow up to date ucrtbase _gmtime64_s value? That's a shame, I was hoping that there will be correlation between maximum values. I'm OK with current patch in this case.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5024#note_60314
This merge request was approved by Piotr Caban. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5024
participants (3)
-
Paul Gofman -
Paul Gofman (＠gofman)
-
Piotr Caban (＠piotr)