From a20708d1bc05466f0389f1bd1475420535636075 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Tue, 27 Aug 2024 11:26:36 -0700 Subject: [PATCH 1/3] Support UTIME_OMIT/UTIME_NOW in futimens Fixes: #22348 --- src/library_syscall.js | 31 ++++++++--- src/struct_info.json | 4 +- src/struct_info_generated.json | 2 + src/struct_info_generated_wasm64.json | 2 + test/utime/test_futimens.c | 77 ++++++++++++++++++++++++--- 5 files changed, 101 insertions(+), 15 deletions(-) diff --git a/src/library_syscall.js b/src/library_syscall.js index c87c708643878..6e7e984690ab5 100644 --- a/src/library_syscall.js +++ b/src/library_syscall.js @@ -56,11 +56,11 @@ var SyscallsLibrary = { var mtime = stat.mtime.getTime(); var ctime = stat.ctime.getTime(); {{{ makeSetValue('buf', C_STRUCTS.stat.st_atim.tv_sec, 'Math.floor(atime / 1000)', 'i64') }}}; - {{{ makeSetValue('buf', C_STRUCTS.stat.st_atim.tv_nsec, '(atime % 1000) * 1000', SIZE_TYPE) }}}; + {{{ makeSetValue('buf', C_STRUCTS.stat.st_atim.tv_nsec, '(atime % 1000) * 1000 * 1000', SIZE_TYPE) }}}; {{{ makeSetValue('buf', C_STRUCTS.stat.st_mtim.tv_sec, 'Math.floor(mtime / 1000)', 'i64') }}}; - {{{ makeSetValue('buf', C_STRUCTS.stat.st_mtim.tv_nsec, '(mtime % 1000) * 1000', SIZE_TYPE) }}}; + {{{ makeSetValue('buf', C_STRUCTS.stat.st_mtim.tv_nsec, '(mtime % 1000) * 1000 * 1000', SIZE_TYPE) }}}; {{{ makeSetValue('buf', C_STRUCTS.stat.st_ctim.tv_sec, 'Math.floor(ctime / 1000)', 'i64') }}}; - {{{ makeSetValue('buf', C_STRUCTS.stat.st_ctim.tv_nsec, '(ctime % 1000) * 1000', SIZE_TYPE) }}}; + {{{ makeSetValue('buf', C_STRUCTS.stat.st_ctim.tv_nsec, '(ctime % 1000) * 1000 * 1000', SIZE_TYPE) }}}; {{{ makeSetValue('buf', C_STRUCTS.stat.st_ino, 'stat.ino', 'i64') }}}; return 0; }, @@ -978,19 +978,34 @@ var SyscallsLibrary = { assert(flags === 0); #endif path = SYSCALLS.calculateAt(dirfd, path, true); + var now = Date.now(), atime, mtime; if (!times) { - var atime = Date.now(); - var mtime = atime; + atime = now; + mtime = now; } else { var seconds = {{{ makeGetValue('times', C_STRUCTS.timespec.tv_sec, 'i53') }}}; var nanoseconds = {{{ makeGetValue('times', C_STRUCTS.timespec.tv_nsec, 'i32') }}}; - atime = (seconds*1000) + (nanoseconds/(1000*1000)); + if (nanoseconds == {{{ cDefs.UTIME_NOW }}}) { + atime = now; + } else if (nanoseconds == {{{ cDefs.UTIME_OMIT }}}) { + atime = -1; + } else { + atime = (seconds*1000) + (nanoseconds/(1000*1000)); + } times += {{{ C_STRUCTS.timespec.__size__ }}}; seconds = {{{ makeGetValue('times', C_STRUCTS.timespec.tv_sec, 'i53') }}}; nanoseconds = {{{ makeGetValue('times', C_STRUCTS.timespec.tv_nsec, 'i32') }}}; - mtime = (seconds*1000) + (nanoseconds/(1000*1000)); + if (nanoseconds == {{{ cDefs.UTIME_NOW }}}) { + mtime = now; + } else if (nanoseconds == {{{ cDefs.UTIME_OMIT }}}) { + mtime = -1; + } else { + mtime = (seconds*1000) + (nanoseconds/(1000*1000)); + } + } + if (mtime != -1 || atime != -1) { + FS.utime(path, atime, mtime); } - FS.utime(path, atime, mtime); return 0; }, __syscall_fallocate__i53abi: true, diff --git a/src/struct_info.json b/src/struct_info.json index 525c96c28b8f1..9403baf0329f7 100644 --- a/src/struct_info.json +++ b/src/struct_info.json @@ -33,7 +33,9 @@ "S_IFCHR", "S_IRUSR", "S_IRGRP", - "S_IROTH" + "S_IROTH", + "UTIME_OMIT", + "UTIME_NOW" ], "structs": { "stat": [ diff --git a/src/struct_info_generated.json b/src/struct_info_generated.json index d1fa6f32447b9..086b5e305e210 100644 --- a/src/struct_info_generated.json +++ b/src/struct_info_generated.json @@ -409,6 +409,8 @@ "TIOCSPGRP": 21520, "TIOCSWINSZ": 21524, "TZNAME_MAX": 16, + "UTIME_NOW": 1073741823, + "UTIME_OMIT": 1073741822, "UUID_TYPE_DCE_RANDOM": 4, "UUID_VARIANT_DCE": 1, "W_OK": 2, diff --git a/src/struct_info_generated_wasm64.json b/src/struct_info_generated_wasm64.json index 8995b4452909f..ddb899dc20aff 100644 --- a/src/struct_info_generated_wasm64.json +++ b/src/struct_info_generated_wasm64.json @@ -409,6 +409,8 @@ "TIOCSPGRP": 21520, "TIOCSWINSZ": 21524, "TZNAME_MAX": 16, + "UTIME_NOW": 1073741823, + "UTIME_OMIT": 1073741822, "UUID_TYPE_DCE_RANDOM": 4, "UUID_VARIANT_DCE": 1, "W_OK": 2, diff --git a/test/utime/test_futimens.c b/test/utime/test_futimens.c index 2f4d373d26fb7..01930a67f48ba 100644 --- a/test/utime/test_futimens.c +++ b/test/utime/test_futimens.c @@ -12,6 +12,7 @@ #include #include #include +#include #include @@ -31,6 +32,23 @@ void setup() { symlink("file", "folder/file-link"); } +void check_times(int fd, struct timespec* expected, int tollerance) { + struct stat s; + int rtn = fstatat(fd, "", &s, AT_EMPTY_PATH); + assert(rtn == 0); + printf("atime: tv_sec=%lld tv_nsec=%ld\n", s.st_atim.tv_sec, s.st_atim.tv_nsec); + printf("mtime: tv_sec=%lld tv_nsec=%ld\n", s.st_mtim.tv_sec, s.st_mtim.tv_nsec); + printf("expected atime: tv_sec=%lld tv_nsec=%ld\n", expected[0].tv_sec, expected[0].tv_nsec); + printf("expected mtime: tv_sec=%lld tv_nsec=%ld\n", expected[1].tv_sec, expected[1].tv_nsec); + if (tollerance) { + assert(llabs(expected[0].tv_sec - s.st_atim.tv_sec) <= tollerance); + assert(llabs(expected[1].tv_sec - s.st_mtim.tv_sec) <= tollerance); + } else { + assert(expected[0].tv_sec == s.st_atim.tv_sec); + assert(expected[1].tv_sec == s.st_mtim.tv_sec); + } +} + void test() { int err; struct stat s; @@ -54,13 +72,60 @@ void test() { assert(s.st_blocks == 1); #endif - struct timespec origTimes[2]; - origTimes[0].tv_sec = (time_t)s.st_atime; - origTimes[0].tv_nsec = origTimes[0].tv_sec * 1000; - origTimes[1].tv_sec = (time_t)s.st_mtime; - origTimes[1].tv_nsec = origTimes[1].tv_sec * 1000; - err = futimens(fd, origTimes); + struct timespec times[2]; + times[0].tv_sec = s.st_atim.tv_sec; + times[0].tv_nsec = s.st_atim.tv_nsec; + times[1].tv_sec = s.st_mtim.tv_sec; + times[1].tv_nsec = s.st_mtim.tv_nsec; + + // set the timestampe to the current value + err = futimens(fd, times); + assert(!err); + check_times(fd, times, 0); + + // UTIME_OMIT means that the timeval is ignored, so + // this call should do nothing. + struct timespec newtimes[2]; + newtimes[0].tv_sec = 42; + newtimes[0].tv_nsec = UTIME_OMIT; + newtimes[1].tv_sec = 42; + newtimes[1].tv_nsec = UTIME_OMIT; + err = futimens(fd, newtimes); + assert(!err); + check_times(fd, times, 0); + + // Setting just one of the two times to UTIME_OMIT means + // the other should be honored. + newtimes[0].tv_sec = 41; + newtimes[0].tv_nsec = UTIME_OMIT; + newtimes[1].tv_sec = 42; + newtimes[1].tv_nsec = 88; + err = futimens(fd, newtimes); + assert(!err); + + times[0].tv_sec = 42; + times[0].tv_nsec = 88; + times[1].tv_sec = 42; + times[1].tv_nsec = 88; + check_times(fd, times, 0); + + // UTIME_NOW means use the current date and ignore the seconds value + newtimes[0].tv_sec = 99; + newtimes[0].tv_nsec = UTIME_NOW; + newtimes[1].tv_sec = 99; + newtimes[1].tv_nsec = UTIME_NOW; + err = futimens(fd, newtimes); + assert(!err); + + struct timespec now; + err = clock_gettime(CLOCK_REALTIME, &now); + printf("now: %lld %ld\n", now.tv_sec, now.tv_nsec); assert(!err); + times[0].tv_sec = now.tv_sec; + times[0].tv_nsec = now.tv_nsec; + times[1].tv_sec = now.tv_sec; + times[1].tv_nsec = now.tv_nsec; + check_times(fd, times, 1); close(fd); From 7ccb2fa3164f3845d6f248fad99aa45882ed5dbd Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Tue, 27 Aug 2024 14:45:34 -0700 Subject: [PATCH 2/3] wasmfs --- system/lib/wasmfs/syscalls.cpp | 24 +++++++++++++++++------- test/utime/test_futimens.c | 7 +++++++ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/system/lib/wasmfs/syscalls.cpp b/system/lib/wasmfs/syscalls.cpp index 6d974bafcd576..4bba107d8f0f9 100644 --- a/system/lib/wasmfs/syscalls.cpp +++ b/system/lib/wasmfs/syscalls.cpp @@ -353,10 +353,6 @@ static timespec ms_to_timespec(double ms) { return ts; } -static double timespec_to_ms(timespec ts) { - return double(ts.tv_sec) * 1000 + double(ts.tv_nsec) / (1000 * 1000); -} - int __syscall_newfstatat(int dirfd, intptr_t path, intptr_t buf, int flags) { // Only accept valid flags. if (flags & ~(AT_EMPTY_PATH | AT_NO_AUTOMOUNT | AT_SYMLINK_NOFOLLOW)) { @@ -1123,6 +1119,16 @@ int __syscall_readlinkat(int dirfd, return bytes; } +static double timespec_to_ms(timespec ts) { + if (ts.tv_nsec == UTIME_OMIT) { + return INFINITY; + } + if (ts.tv_nsec == UTIME_NOW) { + return emscripten_date_now(); + } + return double(ts.tv_sec) * 1000 + double(ts.tv_nsec) / (1000 * 1000); +} + // TODO: Test this with non-AT_FDCWD values. int __syscall_utimensat(int dirFD, intptr_t path_, intptr_t times_, int flags) { const char* path = (const char*)path_; @@ -1148,7 +1154,7 @@ int __syscall_utimensat(int dirFD, intptr_t path_, intptr_t times_, int flags) { // TODO: Check for write access to the file (see man page for specifics). double aTime, mTime; - if (times == NULL) { + if (times == nullptr) { aTime = mTime = emscripten_date_now(); } else { aTime = timespec_to_ms(times[0]); @@ -1156,8 +1162,12 @@ int __syscall_utimensat(int dirFD, intptr_t path_, intptr_t times_, int flags) { } auto locked = parsed.getFile()->locked(); - locked.setATime(aTime); - locked.setMTime(mTime); + if (aTime != INFINITY) { + locked.setATime(aTime); + } + if (mTime != INFINITY) { + locked.setMTime(mTime); + } return 0; } diff --git a/test/utime/test_futimens.c b/test/utime/test_futimens.c index 01930a67f48ba..60982537bd394 100644 --- a/test/utime/test_futimens.c +++ b/test/utime/test_futimens.c @@ -85,6 +85,7 @@ void test() { // UTIME_OMIT means that the timeval is ignored, so // this call should do nothing. + printf("check double UTIME_OMIT...\n"); struct timespec newtimes[2]; newtimes[0].tv_sec = 42; newtimes[0].tv_nsec = UTIME_OMIT; @@ -96,6 +97,7 @@ void test() { // Setting just one of the two times to UTIME_OMIT means // the other should be honored. + printf("check single UTIME_OMIT...\n"); newtimes[0].tv_sec = 41; newtimes[0].tv_nsec = UTIME_OMIT; newtimes[1].tv_sec = 42; @@ -103,13 +105,18 @@ void test() { err = futimens(fd, newtimes); assert(!err); +#if defined(__EMSCRIPTEN__) && !defined(WASMFS) + // The trandiation emscripten FS only supports single timestamp so both + // mtime and atime will always be the same. times[0].tv_sec = 42; times[0].tv_nsec = 88; +#endif times[1].tv_sec = 42; times[1].tv_nsec = 88; check_times(fd, times, 0); // UTIME_NOW means use the current date and ignore the seconds value + printf("check single UTIME_NOW...\n"); newtimes[0].tv_sec = 99; newtimes[0].tv_nsec = UTIME_NOW; newtimes[1].tv_sec = 99; From dd782da4e9f1529ffd0167799fdbe6b803fbc517 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Tue, 27 Aug 2024 15:34:49 -0700 Subject: [PATCH 3/3] feedback --- src/library_syscall.js | 3 +++ test/test_core.py | 1 + test/utime/test_futimens.c | 14 +++++++------- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/library_syscall.js b/src/library_syscall.js index 6e7e984690ab5..2ae0b8f23652b 100644 --- a/src/library_syscall.js +++ b/src/library_syscall.js @@ -1003,6 +1003,9 @@ var SyscallsLibrary = { mtime = (seconds*1000) + (nanoseconds/(1000*1000)); } } + // -1 here means UTIME_OMIT was passed. FS.utime tables the max of these + // two values and sets the timestamp to that single value. If both were + // set to UTIME_OMIT then we can skip the call completely. if (mtime != -1 || atime != -1) { FS.utime(path, atime, mtime); } diff --git a/test/test_core.py b/test/test_core.py index 896f3c27eddad..6f0e1f4ce2bd4 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -5654,6 +5654,7 @@ def test_libgen(self): def test_utime(self): self.do_runf('utime/test_utime.c', 'success') + @also_with_noderawfs def test_futimens(self): self.do_runf('utime/test_futimens.c', 'success') diff --git a/test/utime/test_futimens.c b/test/utime/test_futimens.c index 60982537bd394..562e957f071d4 100644 --- a/test/utime/test_futimens.c +++ b/test/utime/test_futimens.c @@ -32,7 +32,7 @@ void setup() { symlink("file", "folder/file-link"); } -void check_times(int fd, struct timespec* expected, int tollerance) { +void check_times(int fd, struct timespec* expected, int tolerance) { struct stat s; int rtn = fstatat(fd, "", &s, AT_EMPTY_PATH); assert(rtn == 0); @@ -40,9 +40,9 @@ void check_times(int fd, struct timespec* expected, int tollerance) { printf("mtime: tv_sec=%lld tv_nsec=%ld\n", s.st_mtim.tv_sec, s.st_mtim.tv_nsec); printf("expected atime: tv_sec=%lld tv_nsec=%ld\n", expected[0].tv_sec, expected[0].tv_nsec); printf("expected mtime: tv_sec=%lld tv_nsec=%ld\n", expected[1].tv_sec, expected[1].tv_nsec); - if (tollerance) { - assert(llabs(expected[0].tv_sec - s.st_atim.tv_sec) <= tollerance); - assert(llabs(expected[1].tv_sec - s.st_mtim.tv_sec) <= tollerance); + if (tolerance) { + assert(llabs(expected[0].tv_sec - s.st_atim.tv_sec) <= tolerance); + assert(llabs(expected[1].tv_sec - s.st_mtim.tv_sec) <= tolerance); } else { assert(expected[0].tv_sec == s.st_atim.tv_sec); assert(expected[1].tv_sec == s.st_mtim.tv_sec); @@ -67,7 +67,7 @@ void test() { assert(s.st_rdev == 0); assert(s.st_size == 8); assert(s.st_ctime); -#ifdef __EMSCRIPTEN__ +#if defined(__EMSCRIPTEN__) && !defined(NODERAWFS) assert(s.st_blksize == 4096); assert(s.st_blocks == 1); #endif @@ -105,8 +105,8 @@ void test() { err = futimens(fd, newtimes); assert(!err); -#if defined(__EMSCRIPTEN__) && !defined(WASMFS) - // The trandiation emscripten FS only supports single timestamp so both +#if defined(__EMSCRIPTEN__) && !defined(WASMFS) && !defined(NODERAWFS) + // The original emscripten FS (in JS) only supports a single timestamp so both // mtime and atime will always be the same. times[0].tv_sec = 42; times[0].tv_nsec = 88;