Use 64 bits for time_t by sbc100 · Pull Request #17401 · emscripten-core/emscripten

sbc100 · 2022-07-08T18:09:40Z

This brings us back in line with upstream musl. The change to 32-bit
was only recently made in #16966. The reason we made this change was
made was because we had certain C library calls that were implemented in
JS that returned time_t. Since returning 64-bit values from JS
functions is not always easy (we don't always have WASM_BIGINT
available) that simplest solution was to define time_t to 32-bit which
doesn't have issues at the JS boundary.

However, in the intervening time many of the time_t-returning function
have been moved into native code (See #16606 and #16439) with only two
remaining: _mktime_js and _timegm_js. So this change redefines just
those two functions to return int while keeping time_t itself as
64-bit.

Fixes: #17393

kripken

Should we mention this in the changelog? The type change is user-visible and people might need changes as in those tests changed in this PR?

This brings us back in line with upstream musl. The change to 32-bit was only recently made in #16966. The reason we made this change was made was because we had certain C library calls that were implemented in JS that returned `time_t`. Since returning 64-bit values from JS functions is not always easy (we don't always have WASM_BIGINT available) that simplest solution was to define `time_t` to 32-bit which doesn't have issues at the JS boundary. However, in the intervening time many of the `time_t`-returning function have been moved into native code (See #16606 and #16439) with only two remaining: _mktime_js and _timegm_js. So this change redefines just those two functions to return `int` while keeping `time_t` itself as 64-bit. Fixes: #17393

Co-authored-by: Alon Zakai <azakai@google.com>

hoodmane · 2022-07-08T20:37:33Z

Thanks!

This brings us back in line with upstream musl. The change to 32-bit was only recently made in emscripten-core#16966. The reason we made this change was made was because we had certain C library calls that were implemented in JS that returned `time_t`. Since returning 64-bit values from JS functions is not always easy (we don't always have WASM_BIGINT available) that simplest solution was to define `time_t` to 32-bit which doesn't have issues at the JS boundary. However, in the intervening time many of the `time_t`-returning function have been moved into native code (See emscripten-core#16606 and emscripten-core#16439) with only two remaining: _mktime_js and _timegm_js. So this change redefines just those two functions to return `int` while keeping `time_t` itself as 64-bit. Fixes: emscripten-core#17393

Since July 8, 2022: emscripten-core/emscripten#17401 https://github.com/emscripten-core/emscripten/blob/main/system/lib/libc/musl/arch/emscripten/bits/alltypes.h?plain=1#L83

sbc100 · 2022-07-08T18:09:40Z

This brings us back in line with upstream musl. The change to 32-bit
was only recently made in #16966. The reason we made this change was
made was because we had certain C library calls that were implemented in
JS that returned time_t. Since returning 64-bit values from JS
functions is not always easy (we don't always have WASM_BIGINT
available) that simplest solution was to define time_t to 32-bit which
doesn't have issues at the JS boundary.

However, in the intervening time many of the time_t-returning function
have been moved into native code (See #16606 and #16439) with only two
remaining: _mktime_js and _timegm_js. So this change redefines just
those two functions to return int while keeping time_t itself as
64-bit.

Fixes: #17393

kripken

Should we mention this in the changelog? The type change is user-visible and people might need changes as in those tests changed in this PR?

This brings us back in line with upstream musl. The change to 32-bit was only recently made in #16966. The reason we made this change was made was because we had certain C library calls that were implemented in JS that returned `time_t`. Since returning 64-bit values from JS functions is not always easy (we don't always have WASM_BIGINT available) that simplest solution was to define `time_t` to 32-bit which doesn't have issues at the JS boundary. However, in the intervening time many of the `time_t`-returning function have been moved into native code (See #16606 and #16439) with only two remaining: _mktime_js and _timegm_js. So this change redefines just those two functions to return `int` while keeping `time_t` itself as 64-bit. Fixes: #17393

Co-authored-by: Alon Zakai <azakai@google.com>

hoodmane · 2022-07-08T20:37:33Z

Thanks!

This brings us back in line with upstream musl. The change to 32-bit was only recently made in emscripten-core#16966. The reason we made this change was made was because we had certain C library calls that were implemented in JS that returned `time_t`. Since returning 64-bit values from JS functions is not always easy (we don't always have WASM_BIGINT available) that simplest solution was to define `time_t` to 32-bit which doesn't have issues at the JS boundary. However, in the intervening time many of the `time_t`-returning function have been moved into native code (See emscripten-core#16606 and emscripten-core#16439) with only two remaining: _mktime_js and _timegm_js. So this change redefines just those two functions to return `int` while keeping `time_t` itself as 64-bit. Fixes: emscripten-core#17393

Since July 8, 2022: emscripten-core/emscripten#17401 https://github.com/emscripten-core/emscripten/blob/main/system/lib/libc/musl/arch/emscripten/bits/alltypes.h?plain=1#L83

sbc100 · 2022-07-08T18:09:40Z

This brings us back in line with upstream musl. The change to 32-bit
was only recently made in #16966. The reason we made this change was
made was because we had certain C library calls that were implemented in
JS that returned time_t. Since returning 64-bit values from JS
functions is not always easy (we don't always have WASM_BIGINT
available) that simplest solution was to define time_t to 32-bit which
doesn't have issues at the JS boundary.

However, in the intervening time many of the time_t-returning function
have been moved into native code (See #16606 and #16439) with only two
remaining: _mktime_js and _timegm_js. So this change redefines just
those two functions to return int while keeping time_t itself as
64-bit.

Fixes: #17393

kripken

Should we mention this in the changelog? The type change is user-visible and people might need changes as in those tests changed in this PR?

This brings us back in line with upstream musl. The change to 32-bit was only recently made in #16966. The reason we made this change was made was because we had certain C library calls that were implemented in JS that returned `time_t`. Since returning 64-bit values from JS functions is not always easy (we don't always have WASM_BIGINT available) that simplest solution was to define `time_t` to 32-bit which doesn't have issues at the JS boundary. However, in the intervening time many of the `time_t`-returning function have been moved into native code (See #16606 and #16439) with only two remaining: _mktime_js and _timegm_js. So this change redefines just those two functions to return `int` while keeping `time_t` itself as 64-bit. Fixes: #17393

Co-authored-by: Alon Zakai <azakai@google.com>

hoodmane · 2022-07-08T20:37:33Z

Thanks!

This brings us back in line with upstream musl. The change to 32-bit was only recently made in emscripten-core#16966. The reason we made this change was made was because we had certain C library calls that were implemented in JS that returned `time_t`. Since returning 64-bit values from JS functions is not always easy (we don't always have WASM_BIGINT available) that simplest solution was to define `time_t` to 32-bit which doesn't have issues at the JS boundary. However, in the intervening time many of the `time_t`-returning function have been moved into native code (See emscripten-core#16606 and emscripten-core#16439) with only two remaining: _mktime_js and _timegm_js. So this change redefines just those two functions to return `int` while keeping `time_t` itself as 64-bit. Fixes: emscripten-core#17393

Since July 8, 2022: emscripten-core/emscripten#17401 https://github.com/emscripten-core/emscripten/blob/main/system/lib/libc/musl/arch/emscripten/bits/alltypes.h?plain=1#L83

sbc100 · 2022-07-08T18:09:40Z

This brings us back in line with upstream musl. The change to 32-bit
was only recently made in #16966. The reason we made this change was
made was because we had certain C library calls that were implemented in
JS that returned time_t. Since returning 64-bit values from JS
functions is not always easy (we don't always have WASM_BIGINT
available) that simplest solution was to define time_t to 32-bit which
doesn't have issues at the JS boundary.

However, in the intervening time many of the time_t-returning function
have been moved into native code (See #16606 and #16439) with only two
remaining: _mktime_js and _timegm_js. So this change redefines just
those two functions to return int while keeping time_t itself as
64-bit.

Fixes: #17393

kripken

Should we mention this in the changelog? The type change is user-visible and people might need changes as in those tests changed in this PR?

This brings us back in line with upstream musl. The change to 32-bit was only recently made in #16966. The reason we made this change was made was because we had certain C library calls that were implemented in JS that returned `time_t`. Since returning 64-bit values from JS functions is not always easy (we don't always have WASM_BIGINT available) that simplest solution was to define `time_t` to 32-bit which doesn't have issues at the JS boundary. However, in the intervening time many of the `time_t`-returning function have been moved into native code (See #16606 and #16439) with only two remaining: _mktime_js and _timegm_js. So this change redefines just those two functions to return `int` while keeping `time_t` itself as 64-bit. Fixes: #17393

Co-authored-by: Alon Zakai <azakai@google.com>

hoodmane · 2022-07-08T20:37:33Z

Thanks!

This brings us back in line with upstream musl. The change to 32-bit was only recently made in emscripten-core#16966. The reason we made this change was made was because we had certain C library calls that were implemented in JS that returned `time_t`. Since returning 64-bit values from JS functions is not always easy (we don't always have WASM_BIGINT available) that simplest solution was to define `time_t` to 32-bit which doesn't have issues at the JS boundary. However, in the intervening time many of the `time_t`-returning function have been moved into native code (See emscripten-core#16606 and emscripten-core#16439) with only two remaining: _mktime_js and _timegm_js. So this change redefines just those two functions to return `int` while keeping `time_t` itself as 64-bit. Fixes: emscripten-core#17393

Since July 8, 2022: emscripten-core/emscripten#17401 https://github.com/emscripten-core/emscripten/blob/main/system/lib/libc/musl/arch/emscripten/bits/alltypes.h?plain=1#L83

sbc100 · 2022-07-08T18:09:40Z

This brings us back in line with upstream musl. The change to 32-bit
was only recently made in #16966. The reason we made this change was
made was because we had certain C library calls that were implemented in
JS that returned time_t. Since returning 64-bit values from JS
functions is not always easy (we don't always have WASM_BIGINT
available) that simplest solution was to define time_t to 32-bit which
doesn't have issues at the JS boundary.

However, in the intervening time many of the time_t-returning function
have been moved into native code (See #16606 and #16439) with only two
remaining: _mktime_js and _timegm_js. So this change redefines just
those two functions to return int while keeping time_t itself as
64-bit.

Fixes: #17393

kripken

Should we mention this in the changelog? The type change is user-visible and people might need changes as in those tests changed in this PR?

This brings us back in line with upstream musl. The change to 32-bit was only recently made in #16966. The reason we made this change was made was because we had certain C library calls that were implemented in JS that returned `time_t`. Since returning 64-bit values from JS functions is not always easy (we don't always have WASM_BIGINT available) that simplest solution was to define `time_t` to 32-bit which doesn't have issues at the JS boundary. However, in the intervening time many of the `time_t`-returning function have been moved into native code (See #16606 and #16439) with only two remaining: _mktime_js and _timegm_js. So this change redefines just those two functions to return `int` while keeping `time_t` itself as 64-bit. Fixes: #17393

Co-authored-by: Alon Zakai <azakai@google.com>

hoodmane · 2022-07-08T20:37:33Z

Thanks!

This brings us back in line with upstream musl. The change to 32-bit was only recently made in emscripten-core#16966. The reason we made this change was made was because we had certain C library calls that were implemented in JS that returned `time_t`. Since returning 64-bit values from JS functions is not always easy (we don't always have WASM_BIGINT available) that simplest solution was to define `time_t` to 32-bit which doesn't have issues at the JS boundary. However, in the intervening time many of the `time_t`-returning function have been moved into native code (See emscripten-core#16606 and emscripten-core#16439) with only two remaining: _mktime_js and _timegm_js. So this change redefines just those two functions to return `int` while keeping `time_t` itself as 64-bit. Fixes: emscripten-core#17393

Since July 8, 2022: emscripten-core/emscripten#17401 https://github.com/emscripten-core/emscripten/blob/main/system/lib/libc/musl/arch/emscripten/bits/alltypes.h?plain=1#L83

sbc100 · 2022-07-08T18:09:40Z

This brings us back in line with upstream musl. The change to 32-bit
was only recently made in #16966. The reason we made this change was
made was because we had certain C library calls that were implemented in
JS that returned time_t. Since returning 64-bit values from JS
functions is not always easy (we don't always have WASM_BIGINT
available) that simplest solution was to define time_t to 32-bit which
doesn't have issues at the JS boundary.

However, in the intervening time many of the time_t-returning function
have been moved into native code (See #16606 and #16439) with only two
remaining: _mktime_js and _timegm_js. So this change redefines just
those two functions to return int while keeping time_t itself as
64-bit.

Fixes: #17393

kripken

Should we mention this in the changelog? The type change is user-visible and people might need changes as in those tests changed in this PR?

This brings us back in line with upstream musl. The change to 32-bit was only recently made in #16966. The reason we made this change was made was because we had certain C library calls that were implemented in JS that returned `time_t`. Since returning 64-bit values from JS functions is not always easy (we don't always have WASM_BIGINT available) that simplest solution was to define `time_t` to 32-bit which doesn't have issues at the JS boundary. However, in the intervening time many of the `time_t`-returning function have been moved into native code (See #16606 and #16439) with only two remaining: _mktime_js and _timegm_js. So this change redefines just those two functions to return `int` while keeping `time_t` itself as 64-bit. Fixes: #17393

Co-authored-by: Alon Zakai <azakai@google.com>

hoodmane · 2022-07-08T20:37:33Z

Thanks!

This brings us back in line with upstream musl. The change to 32-bit was only recently made in emscripten-core#16966. The reason we made this change was made was because we had certain C library calls that were implemented in JS that returned `time_t`. Since returning 64-bit values from JS functions is not always easy (we don't always have WASM_BIGINT available) that simplest solution was to define `time_t` to 32-bit which doesn't have issues at the JS boundary. However, in the intervening time many of the `time_t`-returning function have been moved into native code (See emscripten-core#16606 and emscripten-core#16439) with only two remaining: _mktime_js and _timegm_js. So this change redefines just those two functions to return `int` while keeping `time_t` itself as 64-bit. Fixes: emscripten-core#17393

Since July 8, 2022: emscripten-core/emscripten#17401 https://github.com/emscripten-core/emscripten/blob/main/system/lib/libc/musl/arch/emscripten/bits/alltypes.h?plain=1#L83

sbc100 · 2022-07-08T18:09:40Z

This brings us back in line with upstream musl. The change to 32-bit
was only recently made in #16966. The reason we made this change was
made was because we had certain C library calls that were implemented in
JS that returned time_t. Since returning 64-bit values from JS
functions is not always easy (we don't always have WASM_BIGINT
available) that simplest solution was to define time_t to 32-bit which
doesn't have issues at the JS boundary.

However, in the intervening time many of the time_t-returning function
have been moved into native code (See #16606 and #16439) with only two
remaining: _mktime_js and _timegm_js. So this change redefines just
those two functions to return int while keeping time_t itself as
64-bit.

Fixes: #17393

kripken

Should we mention this in the changelog? The type change is user-visible and people might need changes as in those tests changed in this PR?

This brings us back in line with upstream musl. The change to 32-bit was only recently made in #16966. The reason we made this change was made was because we had certain C library calls that were implemented in JS that returned `time_t`. Since returning 64-bit values from JS functions is not always easy (we don't always have WASM_BIGINT available) that simplest solution was to define `time_t` to 32-bit which doesn't have issues at the JS boundary. However, in the intervening time many of the `time_t`-returning function have been moved into native code (See #16606 and #16439) with only two remaining: _mktime_js and _timegm_js. So this change redefines just those two functions to return `int` while keeping `time_t` itself as 64-bit. Fixes: #17393

Co-authored-by: Alon Zakai <azakai@google.com>

hoodmane · 2022-07-08T20:37:33Z

Thanks!

This brings us back in line with upstream musl. The change to 32-bit was only recently made in emscripten-core#16966. The reason we made this change was made was because we had certain C library calls that were implemented in JS that returned `time_t`. Since returning 64-bit values from JS functions is not always easy (we don't always have WASM_BIGINT available) that simplest solution was to define `time_t` to 32-bit which doesn't have issues at the JS boundary. However, in the intervening time many of the `time_t`-returning function have been moved into native code (See emscripten-core#16606 and emscripten-core#16439) with only two remaining: _mktime_js and _timegm_js. So this change redefines just those two functions to return `int` while keeping `time_t` itself as 64-bit. Fixes: emscripten-core#17393

Since July 8, 2022: emscripten-core/emscripten#17401 https://github.com/emscripten-core/emscripten/blob/main/system/lib/libc/musl/arch/emscripten/bits/alltypes.h?plain=1#L83

sbc100 · 2022-07-08T18:09:40Z

This brings us back in line with upstream musl. The change to 32-bit
was only recently made in #16966. The reason we made this change was
made was because we had certain C library calls that were implemented in
JS that returned time_t. Since returning 64-bit values from JS
functions is not always easy (we don't always have WASM_BIGINT
available) that simplest solution was to define time_t to 32-bit which
doesn't have issues at the JS boundary.

However, in the intervening time many of the time_t-returning function
have been moved into native code (See #16606 and #16439) with only two
remaining: _mktime_js and _timegm_js. So this change redefines just
those two functions to return int while keeping time_t itself as
64-bit.

Fixes: #17393

kripken

Should we mention this in the changelog? The type change is user-visible and people might need changes as in those tests changed in this PR?

This brings us back in line with upstream musl. The change to 32-bit was only recently made in #16966. The reason we made this change was made was because we had certain C library calls that were implemented in JS that returned `time_t`. Since returning 64-bit values from JS functions is not always easy (we don't always have WASM_BIGINT available) that simplest solution was to define `time_t` to 32-bit which doesn't have issues at the JS boundary. However, in the intervening time many of the `time_t`-returning function have been moved into native code (See #16606 and #16439) with only two remaining: _mktime_js and _timegm_js. So this change redefines just those two functions to return `int` while keeping `time_t` itself as 64-bit. Fixes: #17393

Co-authored-by: Alon Zakai <azakai@google.com>

hoodmane · 2022-07-08T20:37:33Z

Thanks!

This brings us back in line with upstream musl. The change to 32-bit was only recently made in emscripten-core#16966. The reason we made this change was made was because we had certain C library calls that were implemented in JS that returned `time_t`. Since returning 64-bit values from JS functions is not always easy (we don't always have WASM_BIGINT available) that simplest solution was to define `time_t` to 32-bit which doesn't have issues at the JS boundary. However, in the intervening time many of the `time_t`-returning function have been moved into native code (See emscripten-core#16606 and emscripten-core#16439) with only two remaining: _mktime_js and _timegm_js. So this change redefines just those two functions to return `int` while keeping `time_t` itself as 64-bit. Fixes: emscripten-core#17393

Since July 8, 2022: emscripten-core/emscripten#17401 https://github.com/emscripten-core/emscripten/blob/main/system/lib/libc/musl/arch/emscripten/bits/alltypes.h?plain=1#L83

sbc100 requested a review from kripken July 8, 2022 18:09

kripken reviewed Jul 8, 2022
View reviewed changes

Comment threadsystem/lib/libc/emscripten_time.c

sbc100 force-pushed the restore_64bit_time_t branch from 15da934 to cd78df6Compare July 8, 2022 19:05

sbc100 changed the title ~~Restore time_t and other core types to 64bit~~Restore time_t to 64bitJul 8, 2022

sbc100 force-pushed the restore_64bit_time_t branch from cd78df6 to c0b6ba6Compare July 8, 2022 19:14

sbc100 force-pushed the restore_64bit_time_t branch from c0b6ba6 to 8ae0b3eCompare July 8, 2022 19:16

sbc100 changed the title ~~Restore time_t to 64bit~~Use 64 bits for time_tJul 8, 2022

kripken reviewed Jul 8, 2022
View reviewed changes

Comment threadsystem/lib/libc/emscripten_time.c Outdated

kripken approved these changes Jul 8, 2022
View reviewed changes

Update system/lib/libc/emscripten_time.c
e7becc5
Co-authored-by: Alon Zakai <azakai@google.com>

sbc100 merged commit c8857a6 into mainJul 8, 2022

sbc100 deleted the restore_64bit_time_t branch July 8, 2022 20:28

juj mentioned this pull request Aug 15, 2022
was change in structure padding intended? #17640
Closed

sternenseemann mentioned this pull request Jan 7, 2023
haskell.compiler.ghcjs has broken NixOS/nixpkgs#208812
Closed

pavelsavara mentioned this pull request May 9, 2023
[release/7.0-staging] [browser] fix job queue timespan calculation dotnet/runtime#85784
Merged

mmarczell-graphisoft mentioned this pull request Jun 23, 2023
gmtime is still 32 bit (wrapping to 1900 after 2038) #19694
Closed

This was referenced Jul 20, 2023
ASYNCIFY=1 emscripten_sleep broken #19872
Closed
emscripten time_t has wrong size rust-lang/libc#3306
Closed

sbc100 mentioned this pull request Aug 16, 2023
Link error after #17401 merge #20049
Open

hoodmane mentioned this pull request Jan 30, 2024
Fix: time_t is 64 bits on Emscripten rust-lang/libc#3569
Closed

juj mentioned this pull request Aug 20, 2025
Possibly fix flaky test seen to error in wasm2js3.test_stat_chmod_wasmfs. #24989
Merged