fs: fix utime/futime timestamp rounding errors

`uv_fs_utime()` and `uv_fs_futime()` receive the timestamp as
a `double` and then convert it to `struct timeval` or `struct timespec`
where necessary but the calculation for the sub-second part exhibited
rounding errors for dates in the deep past or the far-flung future,
causing the timestamps to be off by sometimes over half a second on
unix, or to be reinterpreted as unsigned and end up off by more than
just sign but many also decades.

Fixes: nodejs/node#32369 (partially)
PR-URL: libuv#2747
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

src/unix/fs.c

@@ -212,24 +212,37 @@ static ssize_t uv__fs_fdatasync(uv_fs_t* req) {
 UV_UNUSED(static struct timespec uv__fs_to_timespec(double time)) {
   struct timespec ts;
   ts.tv_sec  = time;
-  ts.tv_nsec = (uint64_t)(time * 1000000) % 1000000 * 1000;
+  ts.tv_nsec = (time - ts.tv_sec) * 1e9;
+
+ /* TODO(bnoordhuis) Remove this. utimesat() has nanosecond resolution but we
+  * stick to microsecond resolution for the sake of consistency with other
+  * platforms. I'm the original author of this compatibility hack but I'm
+  * less convinced it's useful nowadays.
+  */
+  ts.tv_nsec -= ts.tv_nsec % 1000;
+
+  if (ts.tv_nsec < 0) {
+    ts.tv_nsec += 1e9;
+    ts.tv_sec -= 1;
+  }
   return ts;
 }
 
 UV_UNUSED(static struct timeval uv__fs_to_timeval(double time)) {
   struct timeval tv;
   tv.tv_sec  = time;
-  tv.tv_usec = (uint64_t)(time * 1000000) % 1000000;
+  tv.tv_usec = (time - tv.tv_sec) * 1e6;
+  if (tv.tv_usec < 0) {
+    tv.tv_usec += 1e6;
+    tv.tv_sec -= 1;
+  }
   return tv;
 }
 
 static ssize_t uv__fs_futime(uv_fs_t* req) {
 #if defined(__linux__)                                                        \
     || defined(_AIX71)                                                        \
     || defined(__HAIKU__)
-  /* utimesat() has nanosecond resolution but we stick to microseconds
-   * for the sake of consistency with other platforms.
-   */
   struct timespec ts[2];
   ts[0] = uv__fs_to_timespec(req->atime);
   ts[1] = uv__fs_to_timespec(req->mtime);
@@ -1010,9 +1023,6 @@ static ssize_t uv__fs_utime(uv_fs_t* req) {
     || defined(_AIX71)                                                         \
     || defined(__sun)                                                          \
     || defined(__HAIKU__)
-  /* utimesat() has nanosecond resolution but we stick to microseconds
-   * for the sake of consistency with other platforms.
-   */
   struct timespec ts[2];
   ts[0] = uv__fs_to_timespec(req->atime);
   ts[1] = uv__fs_to_timespec(req->mtime);

src/win/fs.c

@@ -92,30 +92,24 @@
     return;                                                                 \
   }
 
-#define MILLIONu (1000U * 1000U)
-#define BILLIONu (1000U * 1000U * 1000U)
+#define MILLION ((int64_t) 1000 * 1000)
+#define BILLION ((int64_t) 1000 * 1000 * 1000)
 
-#define FILETIME_TO_UINT(filetime)                                          \
-   (*((uint64_t*) &(filetime)) - (uint64_t) 116444736 * BILLIONu)
-
-#define FILETIME_TO_TIME_T(filetime)                                        \
-   (FILETIME_TO_UINT(filetime) / (10u * MILLIONu))
-
-#define FILETIME_TO_TIME_NS(filetime, secs)                                 \
-   ((FILETIME_TO_UINT(filetime) - (secs * (uint64_t) 10 * MILLIONu)) * 100U)
-
-#define FILETIME_TO_TIMESPEC(ts, filetime)                                  \
-   do {                                                                     \
-     (ts).tv_sec = (long) FILETIME_TO_TIME_T(filetime);                     \
-     (ts).tv_nsec = (long) FILETIME_TO_TIME_NS(filetime, (ts).tv_sec);      \
-   } while(0)
+static void uv__filetime_to_timespec(uv_timespec_t *ts, int64_t filetime) {
+  filetime -= 116444736 * BILLION;
+  ts->tv_sec = (long) (filetime / (10 * MILLION));
+  ts->tv_nsec = (long) ((filetime - ts->tv_sec * 10 * MILLION) * 100U);
+  if (ts->tv_nsec < 0) {
+    ts->tv_sec -= 1;
+    ts->tv_nsec += 1e9;
+  }
+}
 
 #define TIME_T_TO_FILETIME(time, filetime_ptr)                              \
   do {                                                                      \
-    uint64_t bigtime = ((uint64_t) ((time) * (uint64_t) 10 * MILLIONu)) +   \
-                       (uint64_t) 116444736 * BILLIONu;                     \
-    (filetime_ptr)->dwLowDateTime = bigtime & 0xFFFFFFFF;                   \
-    (filetime_ptr)->dwHighDateTime = bigtime >> 32;                         \
+    int64_t bigtime = ((time) * 10 * MILLION + 116444736 * BILLION);        \
+    (filetime_ptr)->dwLowDateTime = (uint64_t) bigtime & 0xFFFFFFFF;        \
+    (filetime_ptr)->dwHighDateTime = (uint64_t) bigtime >> 32;              \
   } while(0)
 
 #define IS_SLASH(c) ((c) == L'\\' || (c) == L'/')
@@ -1791,10 +1785,14 @@ INLINE static int fs__stat_handle(HANDLE handle, uv_stat_t* statbuf,
     statbuf->st_mode |= (_S_IREAD | _S_IWRITE) | ((_S_IREAD | _S_IWRITE) >> 3) |
                         ((_S_IREAD | _S_IWRITE) >> 6);
 
-  FILETIME_TO_TIMESPEC(statbuf->st_atim, file_info.BasicInformation.LastAccessTime);
-  FILETIME_TO_TIMESPEC(statbuf->st_ctim, file_info.BasicInformation.ChangeTime);
-  FILETIME_TO_TIMESPEC(statbuf->st_mtim, file_info.BasicInformation.LastWriteTime);
-  FILETIME_TO_TIMESPEC(statbuf->st_birthtim, file_info.BasicInformation.CreationTime);
+  uv__filetime_to_timespec(&statbuf->st_atim,
+                           file_info.BasicInformation.LastAccessTime.QuadPart);
+  uv__filetime_to_timespec(&statbuf->st_ctim,
+                           file_info.BasicInformation.ChangeTime.QuadPart);
+  uv__filetime_to_timespec(&statbuf->st_mtim,
+                           file_info.BasicInformation.LastWriteTime.QuadPart);
+  uv__filetime_to_timespec(&statbuf->st_birthtim,
+                           file_info.BasicInformation.CreationTime.QuadPart);
 
   statbuf->st_ino = file_info.InternalInformation.IndexNumber.QuadPart;
 

test/task.h

@@ -196,22 +196,26 @@ typedef enum {
   }                                                            \
  } while (0)
 
-#define ASSERT_INT_BASE(a, operator, b, type, conv)          \
- ASSERT_BASE(a, operator, b, type, conv)
-
-#define ASSERT_EQ(a, b) ASSERT_INT_BASE(a, ==, b, int64_t, PRId64)
-#define ASSERT_GE(a, b) ASSERT_INT_BASE(a, >=, b, int64_t, PRId64)
-#define ASSERT_GT(a, b) ASSERT_INT_BASE(a, >, b, int64_t, PRId64)
-#define ASSERT_LE(a, b) ASSERT_INT_BASE(a, <=, b, int64_t, PRId64)
-#define ASSERT_LT(a, b) ASSERT_INT_BASE(a, <, b, int64_t, PRId64)
-#define ASSERT_NE(a, b) ASSERT_INT_BASE(a, !=, b, int64_t, PRId64)
-
-#define ASSERT_UINT64_EQ(a, b) ASSERT_INT_BASE(a, ==, b, uint64_t, PRIu64)
-#define ASSERT_UINT64_GE(a, b) ASSERT_INT_BASE(a, >=, b, uint64_t, PRIu64)
-#define ASSERT_UINT64_GT(a, b) ASSERT_INT_BASE(a, >, b, uint64_t, PRIu64)
-#define ASSERT_UINT64_LE(a, b) ASSERT_INT_BASE(a, <=, b, uint64_t, PRIu64)
-#define ASSERT_UINT64_LT(a, b) ASSERT_INT_BASE(a, <, b, uint64_t, PRIu64)
-#define ASSERT_UINT64_NE(a, b) ASSERT_INT_BASE(a, !=, b, uint64_t, PRIu64)
+#define ASSERT_EQ(a, b) ASSERT_BASE(a, ==, b, int64_t, PRId64)
+#define ASSERT_GE(a, b) ASSERT_BASE(a, >=, b, int64_t, PRId64)
+#define ASSERT_GT(a, b) ASSERT_BASE(a, >, b, int64_t, PRId64)
+#define ASSERT_LE(a, b) ASSERT_BASE(a, <=, b, int64_t, PRId64)
+#define ASSERT_LT(a, b) ASSERT_BASE(a, <, b, int64_t, PRId64)
+#define ASSERT_NE(a, b) ASSERT_BASE(a, !=, b, int64_t, PRId64)
+
+#define ASSERT_UINT64_EQ(a, b) ASSERT_BASE(a, ==, b, uint64_t, PRIu64)
+#define ASSERT_UINT64_GE(a, b) ASSERT_BASE(a, >=, b, uint64_t, PRIu64)
+#define ASSERT_UINT64_GT(a, b) ASSERT_BASE(a, >, b, uint64_t, PRIu64)
+#define ASSERT_UINT64_LE(a, b) ASSERT_BASE(a, <=, b, uint64_t, PRIu64)
+#define ASSERT_UINT64_LT(a, b) ASSERT_BASE(a, <, b, uint64_t, PRIu64)
+#define ASSERT_UINT64_NE(a, b) ASSERT_BASE(a, !=, b, uint64_t, PRIu64)
+
+#define ASSERT_DOUBLE_EQ(a, b) ASSERT_BASE(a, ==, b, double, "f")
+#define ASSERT_DOUBLE_GE(a, b) ASSERT_BASE(a, >=, b, double, "f")
+#define ASSERT_DOUBLE_GT(a, b) ASSERT_BASE(a, >, b, double, "f")
+#define ASSERT_DOUBLE_LE(a, b) ASSERT_BASE(a, <=, b, double, "f")
+#define ASSERT_DOUBLE_LT(a, b) ASSERT_BASE(a, <, b, double, "f")
+#define ASSERT_DOUBLE_NE(a, b) ASSERT_BASE(a, !=, b, double, "f")
 
 #define ASSERT_STR_EQ(a, b) \
   ASSERT_BASE_STR(strcmp(a, b) == 0, a, == , b, char*, "s")

test/test-fs.c

@@ -821,8 +821,29 @@ static void check_utime(const char* path,
   ASSERT(req.result == 0);
   s = &req.statbuf;
 
-  ASSERT(s->st_atim.tv_sec + (s->st_atim.tv_nsec / 1000000000.0) == atime);
-  ASSERT(s->st_mtim.tv_sec + (s->st_mtim.tv_nsec / 1000000000.0) == mtime);
+  if (s->st_atim.tv_nsec == 0 && s->st_mtim.tv_nsec == 0) {
+    /*
+     * Test sub-second timestamps only when supported (such as Windows with
+     * NTFS). Some other platforms support sub-second timestamps, but that
+     * support is filesystem-dependent. Notably OS X (HFS Plus) does NOT
+     * support sub-second timestamps.
+     */
+#ifdef _WIN32
+    ASSERT_DOUBLE_EQ(atime, (long) atime);
+    ASSERT_DOUBLE_EQ(mtime, (long) atime);
+#endif
+    ASSERT_EQ(s->st_atim.tv_sec, (long) atime);
+    ASSERT_EQ(s->st_mtim.tv_sec, (long) mtime);
+  } else {
+    double st_atim;
+    double st_mtim;
+    ASSERT_DOUBLE_GE(s->st_atim.tv_nsec, 0);
+    ASSERT_DOUBLE_GE(s->st_mtim.tv_nsec, 0);
+    st_atim = s->st_atim.tv_sec + s->st_atim.tv_nsec / 1e9;
+    st_mtim = s->st_mtim.tv_sec + s->st_mtim.tv_nsec / 1e9;
+    ASSERT_DOUBLE_EQ(st_atim, atime);
+    ASSERT_DOUBLE_EQ(st_mtim, mtime);
+  }
 
   uv_fs_req_cleanup(&req);
 }
@@ -2523,16 +2544,7 @@ TEST_IMPL(fs_utime) {
   uv_fs_req_cleanup(&req);
   uv_fs_close(loop, &req, r, NULL);
 
-  atime = mtime = 400497753; /* 1982-09-10 11:22:33 */
-
-  /*
-   * Test sub-second timestamps only on Windows (assuming NTFS). Some other
-   * platforms support sub-second timestamps, but that support is filesystem-
-   * dependent. Notably OS X (HFS Plus) does NOT support sub-second timestamps.
-   */
-#ifdef _WIN32
-  mtime += 0.444;            /* 1982-09-10 11:22:33.444 */
-#endif
+  atime = mtime = 400497753.25; /* 1982-09-10 11:22:33.25 */
 
   r = uv_fs_utime(NULL, &req, path, atime, mtime, NULL);
   ASSERT(r == 0);
@@ -2541,7 +2553,7 @@ TEST_IMPL(fs_utime) {
 
   check_utime(path, atime, mtime, /* test_lutime */ 0);
 
-  atime = mtime = 1291404900; /* 2010-12-03 20:35:00 - mees <3 */
+  atime = mtime = 1291404900.25; /* 2010-12-03 20:35:00.25 - mees <3 */
   checkme.path = path;
   checkme.atime = atime;
   checkme.mtime = mtime;
@@ -2561,6 +2573,41 @@ TEST_IMPL(fs_utime) {
 }
 
 
+TEST_IMPL(fs_utime_round) {
+  const char path[] = "test_file";
+  double atime;
+  double mtime;
+  uv_fs_t req;
+  int r;
+
+  loop = uv_default_loop();
+  unlink(path);
+  r = uv_fs_open(NULL, &req, path, O_RDWR | O_CREAT, S_IWUSR | S_IRUSR, NULL);
+  ASSERT_GE(r, 0);
+  ASSERT_GE(req.result, 0);
+  uv_fs_req_cleanup(&req);
+  ASSERT_EQ(0, uv_fs_close(loop, &req, r, NULL));
+
+  atime = mtime = -14245440.25;  /* 1969-07-20T02:56:00.25Z */
+
+  r = uv_fs_utime(NULL, &req, path, atime, mtime, NULL);
+#if (defined(_AIX) && !defined(_AIX71)) ||                                    \
+     defined(__MVS__) ||                                                      \
+     defined(__PASE__)
+  ASSERT(r == UV_EINVAL);
+  RETURN_SKIP("utime on z/OS, IBM i PASE, and AIX versions below 7.1 does not like pre-epoch timestamps");
+#endif
+  ASSERT_EQ(0, r);
+  ASSERT_EQ(0, req.result);
+  uv_fs_req_cleanup(&req);
+  check_utime(path, atime, mtime, /* test_lutime */ 0);
+  unlink(path);
+
+  MAKE_VALGRIND_HAPPY();
+  return 0;
+}
+
+
 #ifdef _WIN32
 TEST_IMPL(fs_stat_root) {
   int r;
@@ -2614,16 +2661,7 @@ TEST_IMPL(fs_futime) {
   uv_fs_req_cleanup(&req);
   uv_fs_close(loop, &req, r, NULL);
 
-  atime = mtime = 400497753; /* 1982-09-10 11:22:33 */
-
-  /*
-   * Test sub-second timestamps only on Windows (assuming NTFS). Some other
-   * platforms support sub-second timestamps, but that support is filesystem-
-   * dependent. Notably OS X (HFS Plus) does NOT support sub-second timestamps.
-   */
-#ifdef _WIN32
-  mtime += 0.444;            /* 1982-09-10 11:22:33.444 */
-#endif
+  atime = mtime = 400497753.25; /* 1982-09-10 11:22:33.25 */
 
   r = uv_fs_open(NULL, &req, path, O_RDWR, 0, NULL);
   ASSERT(r >= 0);
@@ -2700,11 +2738,7 @@ TEST_IMPL(fs_lutime) {
   uv_fs_req_cleanup(&req);
 
   /* Test the synchronous version. */
-  atime = mtime = 400497753; /* 1982-09-10 11:22:33 */
-
-#ifdef _WIN32
-  mtime += 0.444;            /* 1982-09-10 11:22:33.444 */
-#endif
+  atime = mtime = 400497753.25; /* 1982-09-10 11:22:33.25 */
 
   checkme.atime = atime;
   checkme.mtime = mtime;

test/test-list.h

@@ -360,6 +360,7 @@ TEST_DECLARE   (fs_open_flags)
 TEST_DECLARE   (fs_fd_hash)
 #endif
 TEST_DECLARE   (fs_utime)
+TEST_DECLARE   (fs_utime_round)
 TEST_DECLARE   (fs_futime)
 TEST_DECLARE   (fs_lutime)
 TEST_DECLARE   (fs_file_open_append)
@@ -996,6 +997,7 @@ TASK_LIST_START
 #endif
   TEST_ENTRY  (fs_chown)
   TEST_ENTRY  (fs_utime)
+  TEST_ENTRY  (fs_utime_round)
   TEST_ENTRY  (fs_futime)
   TEST_ENTRY  (fs_lutime)
   TEST_ENTRY  (fs_readlink)