[PATCH 0/1] MR5024: msvcrt: Adjust _gmtime64_s() accepted time limits.

[Paul Gofman (＠gofman)]

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.

[PATCH 1/1] msvcrt: Adjust _gmtime64_s() accepted time limits.

[Paul Gofman]

From: Paul Gofman

---
 dlls/msvcr120/tests/msvcr120.c | 60 ++++++++++++++++++++++++++++++++
 dlls/msvcrt/tests/time.c       | 62 ++++++++++++++++++++++++++++++++++
 dlls/msvcrt/time.c             | 30 +++++++++++++++-
 dlls/ucrtbase/tests/misc.c     | 62 ++++++++++++++++++++++++++++++++++
 4 files changed, 213 insertions(+), 1 deletion(-)

diff --git a/dlls/msvcr120/tests/msvcr120.c b/dlls/msvcr120/tests/msvcr120.c
index c76db6582cf..f506a9d7717 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,9 @@ 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_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 +301,8 @@ 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");
     SET(p_wctype, "wctype");
     SET(p_fegetenv, "fegetenv");
     SET(p_fesetenv, "fesetenv");
@@ -1714,6 +1721,58 @@ 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);
+
+    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);
+
+    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 +1797,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..18ab23c4f9e 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,63 @@ 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);
+
+    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);
+
+    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 +1024,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..0758f53fb6d 100644
--- a/dlls/msvcrt/time.c
+++ b/dlls/msvcrt/time.c
@@ -35,6 +35,7 @@
 #include "wine/debug.h"

WINE_DEFAULT_DEBUG_CHANNEL(msvcrt);
+WINE_DECLARE_DEBUG_CHANNEL(time);

#undef _ctime32
 #undef _difftime32
@@ -66,6 +67,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);
@@ -452,12 +464,13 @@ int CDECL _localtime32_s(struct tm *time, const __time32_t *secs)
  */
 int CDECL _gmtime64_s(struct tm *res, const __time64_t *secs)
 {
+
     int i;
     FILETIME ft;
     SYSTEMTIME st;
     ULONGLONG time;

-    if (!res || !secs || *secs < 0 || *secs > _MAX__TIME64_T) {
+    if (!res || !secs || *secs < MIN_GMTIME64_TIME || *secs > MAX_GMTIME64_TIME) {
         if (res) {
             write_invalid_msvcrt_tm(res);
         }
@@ -497,6 +510,8 @@ struct tm* CDECL _gmtime64(const __time64_t *secs)
 {
     thread_data_t * const data = msvcrt_get_thread_data();

+    TRACE_(time)("secs %p, %I64d.\n", secs, secs ? *secs : 0);
+
     if(!data->time_buffer)
         data->time_buffer = malloc(sizeof(struct tm));

@@ -513,6 +528,13 @@ int CDECL _gmtime32_s(struct tm *res, const __time32_t *secs)
     __time64_t secs64;

if(secs) {
+        if (*secs < 0)
+        {
+            if (res)
+                write_invalid_msvcrt_tm(res);
+            *_errno() = EINVAL;
+            return EINVAL;
+        }
         secs64 = *secs;
         return _gmtime64_s(res, &secs64);
     }
@@ -529,6 +551,12 @@ struct tm* CDECL _gmtime32(const __time32_t* secs)
     if(!secs)
         return NULL;

+    if (*secs < 0)
+    {
+        *_errno() = EINVAL;
+        return NULL;
+    }
+
     secs64 = *secs;
     return _gmtime64( &secs64 );
 }
diff --git a/dlls/ucrtbase/tests/misc.c b/dlls/ucrtbase/tests/misc.c
index 86cdec88108..4e3fe1f35e8 100644
--- a/dlls/ucrtbase/tests/misc.c
+++ b/dlls/ucrtbase/tests/misc.c
@@ -113,6 +113,10 @@ _se_translator_function __cdecl _set_se_translator(_se_translator_function func)
 void** __cdecl __current_exception(void);
 int* __cdecl __processing_throw(void);

+_ACRTIMP int __cdecl _gmtime64_s(struct tm *res, const __time64_t *secs);
+
+#define _MAX__TIME64_T     (((__time64_t)0x00000007 << 32) | 0x93406FFF)
+
 static void test__initialize_onexit_table(void)
 {
     _onexit_table_t table, table2;
@@ -1632,6 +1636,63 @@ 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);
+
+    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);
+
+    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)
+    {
+        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 +1738,5 @@ START_TEST(misc)
 #if defined(__i386__)
     test_rewind_i386_abi();
 #endif
+    test_gmtime64();
 }

[Piotr Caban (＠piotr)]

Piotr Caban (@piotr) commented about dlls/ucrtbase/tests/misc.c:
> void** __cdecl __current_exception(void);
>  int* __cdecl __processing_throw(void);
>
> +_ACRTIMP int __cdecl _gmtime64_s(struct tm *res, const __time64_t *secs);

It should be added to appropriate header.

[Piotr Caban (＠piotr)]

Piotr Caban (@piotr) commented about dlls/ucrtbase/tests/misc.c:
> - 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);
> -
> - 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)
> - {
> -
>    skip("Old gmtime64 limits, skipping tests.\n");

```suggestion:-0+0
        win_skip("Old gmtime64 limits, skipping tests.\n");
```

[Piotr Caban (＠piotr)]

Piotr Caban (@piotr) commented about dlls/msvcrt/time.c:
> #include "wine/debug.h"
>
> WINE_DEFAULT_DEBUG_CHANNEL(msvcrt);
> +WINE_DECLARE_DEBUG_CHANNEL(time);

You probably forgot to remove `time` debug channel.

[Piotr Caban (＠piotr)]

Piotr Caban (@piotr) commented about dlls/msvcrt/time.c:
>  if(!secs)
>      return NULL;
>
> - if (*secs < 0)
> - {
> -
>    *_errno() = EINVAL;
> -
>    return NULL;
> - }

The function succeeds for `*secs == -1` in ucrtbase (I didn't test other versions).

[Piotr Caban (＠piotr)]

Piotr Caban (@piotr) commented about dlls/msvcrt/time.c:
> */
>  int CDECL _gmtime64_s(struct tm *res, const __time64_t *secs)
>  {
> -

```suggestion:-0+0
```

[Paul Gofman (＠gofman)]

On Mon Feb  5 19:12:25 2024 +0000, Piotr Caban wrote:
> The function succeeds for `*secs == -1` in ucrtbase (I didn't test other versions).

I am reproducing this result with ucrtbase only when adding ported test_gmtime() to ucrtbase/tests/misc.c in the end, with invalid parameter handler set in the previous tests. The attached test (on top of the present patchset) succeeds for me on Windows 10. This test has _set_invalid_parameter_handler(NULL); before the added tests. So unless I am missing something this specific behaviour with *secs < 0 succeeding is related to the presence of invalid parameter handler and not to actual acceptance of *secs < 0 result. We are currently missing such handling in ucrtbase time functions but that looks unrelated?

[test.patch](/uploads/fb6429ea32d7760cab1ee9d82bc22129/test.patch)

[Paul Gofman (＠gofman)]

On Mon Feb  5 20:11:42 2024 +0000, Paul Gofman wrote:
> I am reproducing this result with ucrtbase only when adding ported
> test_gmtime() to ucrtbase/tests/misc.c in the end, with invalid
> parameter handler set in the previous tests. The attached test (on top
> of the present patchset) succeeds for me on Windows 10. This test has
> _set_invalid_parameter_handler(NULL); before the added tests. So unless
> I am missing something this specific behaviour with *secs < 0 succeeding
> is related to the presence of invalid parameter handler and not to
> actual acceptance of *secs < 0 result. We are currently missing such
> handling in ucrtbase time functions but that looks unrelated?
> [test.patch](/uploads/fb6429ea32d7760cab1ee9d82bc22129/test.patch)

Sorry, it looks like I my test just crashes on  = _gmtime32(NULL); without the handler, and _gmtime32 may actually succeed, I need to look more.