64 bit time_t

[hoodmane]

As discussed in pyodide/pyodide#2841, 2038 is only 16 years from now so it would be great to be able to represent dates further into the future as unix timestamps. In interest of this, we would like to be able to opt into a 64 bit time_t (or have it as the default).

[tiran]

I believe that the change broke some stat or utime() related APIs in Emscripten. CPython's test suite is failing on EMSDK 3.1.6 and TOT, e.g.

 ======================================================================
FAIL: test_utime (test.test_os.UtimeTests.test_utime)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/cpython-wasm-test/cpython-wasm-test/cpython/Lib/test/test_os.py", line 805, in test_utime
    self._test_utime(set_time)
  File "/home/runner/work/cpython-wasm-test/cpython-wasm-test/cpython/Lib/test/test_os.py", line 793, in _test_utime
    self.assertAlmostEqual(st.st_atime, atime_ns * 1e-9, delta=1e-6)
AssertionError: 1.1182583010295808e+17 != 1.002003 within 1e-06 delta (1.1182583010295808e+17 difference)

[sbc100]

Ha, that's ironic/unfortunate given that its was python folks who wanted this changed. Any ideas why it would be failing?

[tiran]

Not yet, but I have a gut feeling that the utimensat syscall code should be changed to use bigint and 64bit ints.

emscripten/src/library_syscall.js

Lines 957 to 978 in 8c047b1

	__syscall_utimensat__sig: 'iippi',
	__syscall_utimensat: function(dirfd, path, times, flags) {
	path = SYSCALLS.getStr(path);
	#if ASSERTIONS
	assert(flags === 0);
	#endif
	path = SYSCALLS.calculateAt(dirfd, path, true);
	if (!times) {
	var atime = Date.now();
	var mtime = atime;
	} else {
	var seconds = {{{ makeGetValue('times', C_STRUCTS.timespec.tv_sec, 'i32') }}};
	var nanoseconds = {{{ makeGetValue('times', C_STRUCTS.timespec.tv_nsec, 'i32') }}};
	atime = (seconds*1000) + (nanoseconds/(1000*1000));
	times += {{{ C_STRUCTS.timespec.__size__ }}};
	seconds = {{{ makeGetValue('times', C_STRUCTS.timespec.tv_sec, 'i32') }}};
	nanoseconds = {{{ makeGetValue('times', C_STRUCTS.timespec.tv_nsec, 'i32') }}};
	mtime = (seconds*1000) + (nanoseconds/(1000*1000));
	}
	FS.utime(path, atime, mtime);
	return 0;
	},

[tiran]

timespec struct definition depends on size of time_t

system/lib/libc/musl/include/alltypes.h.in:STRUCT timespec { time_t tv_sec; int :8*(sizeof(time_t)-sizeof(long))*(__BYTE_ORDER==4321); long tv_nsec; int :8*(sizeof(time_t)-sizeof(long))*(__BYTE_ORDER!=4321); };

[sbc100]

Hmm.. it looks like the JS filesystem layer assumes timestamps are JS numbers, so we will not be able fully represent i64 times. Do you know if its reasonable for the value passed to utimens to be truncated such that value returns by stat that follows it looks some precision?

If not then I think I might just revert this change since 32-bit time_t is more compatible with the JS layer (which represents time as JS number (double) number of seconds): https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/getTime

[sbc100]

Hmm.. looks like it may be ok to truncate: https://www.qnx.com/developers/docs/7.1/#com.qnx.doc.neutrino.lib_ref/topic/u/utimensat.html:

"The time that's actually recorded depends on the resolution in the filesystem's data structures. For example, no matter what you put in the tv_nsec field, the time that a FAT filesystem records uses a 2-second granularity. The recorded time is the greatest value supported by the filesystem that isn't greater than the specified time."

[sbc100]

Still, I'm not sure it work the extra cost in code complexity and output side to have to deal with i64 here rather than i32.

[sbc100]

Playing around with linux is looks like its currently capping tv_sec at 0x37fffffff (2446-05-10 15:38:53.000000000). Specifying anything bigger than this results in stat returning that value... so I guess its fine if we cap ours at 2^53-1.

[tiran]

>>> datetime.datetime.fromtimestamp(2**53 - 1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: year 285428751 is out of range

Yeah, a cap at 2^53-1 looks fine to me, too! 👍

[sbc100]

Hopefully #17459 will fix you failure

[hoodmane]

Thanks @sbc100, @tiran!

[tiran]

I have created a GH repo with daily GHA to run smoke tests of EMSDK tip-of-tree with CPython 3.11 and main branch, https://github.com/tiran/cpython-wasm-test/actions

[tiran]

Thanks! The PR fixed most broken tests of CPython's test suite. The functions localtime_js and _gmtime_js need some attention, too. They still parse time_t as i32. Python expects that localtime() fails with EOVERFLOW when the input is out of supported range.

https://github.com/tiran/cpython-wasm-test/runs/7403750856

[tiran]

Fixed in #17471 . The PR also adds some tests for edge cases.

[tiran]

Good news! CPython's test suite is passing again with Emscripten tot-upstream.