Skip to content

Commit

Permalink
fs: fix utime/futime timestamp rounding errors
Browse files Browse the repository at this point in the history 
`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>
  • Loading branch information
@bnoordhuis @vtjnash
bnoordhuis and vtjnash committed Nov 6, 2020
1 parent b2339e1 commit 7b41272
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 40 deletions.
18 changes: 10 additions & 8 deletions src/unix/fs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,24 +212,29 @@ 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;

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;
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);
Expand Down Expand Up @@ -1010,9 +1015,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);
Expand Down
31 changes: 17 additions & 14 deletions src/win/fs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,17 +92,17 @@
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_INT(filetime) \
((filetime) - 116444736 * BILLION)

#define FILETIME_TO_TIME_T(filetime) \
(FILETIME_TO_UINT(filetime) / (10u * MILLIONu))
(FILETIME_TO_INT(filetime) / (10 * MILLION))

#define FILETIME_TO_TIME_NS(filetime, secs) \
((FILETIME_TO_UINT(filetime) - (secs * (uint64_t) 10 * MILLIONu)) * 100U)
((FILETIME_TO_INT(filetime) - (secs) * 10 * MILLION) * 100U)

#define FILETIME_TO_TIMESPEC(ts, filetime) \
do { \
Expand All @@ -112,10 +112,9 @@

#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'/')
Expand Down Expand Up @@ -1791,10 +1790,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);
FILETIME_TO_TIMESPEC(statbuf->st_atim,
file_info.BasicInformation.LastAccessTime.QuadPart);
FILETIME_TO_TIMESPEC(statbuf->st_ctim,
file_info.BasicInformation.ChangeTime.QuadPart);
FILETIME_TO_TIMESPEC(statbuf->st_mtim,
file_info.BasicInformation.LastWriteTime.QuadPart);
FILETIME_TO_TIMESPEC(statbuf->st_birthtim,
file_info.BasicInformation.CreationTime.QuadPart);

statbuf->st_ino = file_info.InternalInformation.IndexNumber.QuadPart;

Expand Down
36 changes: 20 additions & 16 deletions test/task.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
38 changes: 36 additions & 2 deletions test/test-fs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -821,8 +821,11 @@ 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);
ASSERT_DOUBLE_EQ(s->st_atim.tv_sec + (s->st_atim.tv_nsec / 1000000000.0),
atime);

ASSERT_DOUBLE_EQ(s->st_mtim.tv_sec + (s->st_mtim.tv_nsec / 1000000000.0),
mtime);

uv_fs_req_cleanup(&req);
}
Expand Down Expand Up @@ -2561,6 +2564,37 @@ TEST_IMPL(fs_utime) {
}


TEST_IMPL(fs_utime_round) {
const char path[] = "test_file";
double atime;
double mtime;
uv_fs_t req;
int r;

#ifdef __MVS__
RETURN_SKIP("futime on z/os does not like pre-epoch timestamps");
#endif

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.0; /* 1969-07-20T02:56:00.00Z */
ASSERT_EQ(0, uv_fs_utime(NULL, &req, path, atime, mtime, NULL));
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;
Expand Down
2 changes: 2 additions & 0 deletions test/test-list.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -995,6 +996,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)
Expand Down

0 comments on commit 7b41272

Please sign in to comment.