-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gmtime is still 32 bit (wrapping to 1900 after 2038) #19694
Comments
JS Number can hold up to 53bits, so that should be plenty. Any idea where the remaining bug lies.. it not in the definition of time_t.. |
I found the source of the problem: emscripten/system/lib/libc/emscripten_internal.h Lines 38 to 43 in eab65a0
I wonder if this is actually urgent to fix? i.e. does real world code care about this today? Fixing would likely involve a slight code size bump for all users. |
The reason I ask is that in a few months when we have wasm-bigint enabled by default this should be trivial to fix. |
Seems like there is no reason to pass a pointer here. I noticed this while investigating #19694.
Seems like there is no reason to pass a pointer here. I noticed this while investigating #19694.
Seems like there is no reason to pass a pointer here. I noticed this while investigating #19694.
Good find. Sgtm to wait for WASM_BIGINT by default - that will surely happen before 2038! 🤣 |
Actually it looks like we can fix without regressing code size for most programs.. |
Seems like there is no reason to pass a pointer here. I noticed this while investigating #19694.
Seems like there is no reason to pass a pointer here. I noticed this while investigating #19694.
Seems like there is no reason to pass a pointer here. I noticed this while investigating #19694.
Parts of the time.h / ctime API still don't support 64 bit time, as demonstrated by the following snippet:
It prints
Related:
#17393, #17401, #17471
I understand the reasons behind this with JS Number being less than 64 bit, but I still think it's a bug.
Version of emscripten/emsdk:
But I believe the bug is present in latest too.
The text was updated successfully, but these errors were encountered: