rust-lang / libc

Raw bindings to platform APIs for Rust
https://docs.rs/libc
Apache License 2.0
2.09k stars 1.04k forks source link

musl: Change `time_t` definition on 32-bit targets according to "time64" #1848

Open JohnTitor opened 4 years ago

JohnTitor commented 4 years ago

time_t is defined as c_long on musl: https://github.com/rust-lang/libc/blob/b1268e43cd34ebc95d184928296fc73740cd5162/src/unix/linux_like/linux/musl/mod.rs#L3 and c_long is i32 on 32-bit targets: https://github.com/rust-lang/libc/blob/b1268e43cd34ebc95d184928296fc73740cd5162/src/unix/linux_like/linux/musl/b32/mod.rs#L1 But the time_t definition has been changed to 64-bit by https://github.com/bminor/musl/commit/38143339646a4ccce8afe298c34467767c899f51, since musl 1.2.0. We will change our type to i64 on 32-bit targets as well at some point but we should announce it before changing. cc #1846

mahkoh commented 4 years ago

Isn't this broken either way as soon as C libraries are involved or am I missing something?

As documented in time64.html, C code compiled against >=1.2 assumes time_t = i64 whereas code compiled against <1.2 assumes time_t = i32 on 32 bit systems. If you use a C library that uses time_t in its interface (e.g. openssl), the time_t used by rust code must have the same size as in the musl headers.

abcfy2 commented 4 years ago

Any updates? I'm facing the same issue when building openssl:

   Compiling shadowsocks-rust v1.8.19 (/project)
error: linking with `arm-linux-musleabi-gcc` failed: exit code: 1
  |
  = note: "arm-linux-musleabi-gcc" "-Wl,--as-needed" "-Wl,-z,noexecstack" "-Wl,--eh-frame-hdr" "-nostartfiles" "/rust/lib/rustlib/arm-unknown-linux-musleabi/lib/self-contained/crt1.o" "/rust/lib/rustlib/arm-unknown-linux-musleabi/lib/self-contained/crti.o" "-L" "/rust/lib/rustlib/arm-unknown-linux-musleabi/lib" "-L" "/rust/lib/rustlib/arm-unknown-linux-musleabi/lib/self-contained" "/target/arm-unknown-linux-musleabi/release/deps/ssurl-b511399b1b3e9edd.ssurl.1o5mikv8-cgu.0.rcgu.o" "-o" "/target/arm-unknown-linux-musleabi/release/deps/ssurl-b511399b1b3e9edd" "-Wl,--gc-sections" "-static" "-no-pie" "-Wl,-zrelro" "-Wl,-znow" "-Wl,-O1" "-nodefaultlibs" "-L" "/target/arm-unknown-linux-musleabi/release/deps" "-L" "/target/release/deps" "-L" "/target/arm-unknown-linux-musleabi/release/build/libsodium-sys-af7e3974b41e2f39/out/installed/lib" "-L" "/target/arm-unknown-linux-musleabi/release/build/openssl-sys-ddefbd6d719725b9/out/openssl-build/install/lib" "-L" "/target/arm-unknown-linux-musleabi/release/build/ring-c1bb5f055649313e/out" "-L" "/rust/lib/rustlib/arm-unknown-linux-musleabi/lib" "-Wl,-Bstatic" "/tmp/rustc2sPxVB/libring-7676a44ad52e73ec.rlib" "/tmp/rustc2sPxVB/liblibsodium_sys-8f2b5176646fe1cb.rlib" "/tmp/rustc2sPxVB/libopenssl_sys-0f67a64037e617dd.rlib" "-Wl,--start-group" "/tmp/rustc2sPxVB/libunwind-bb2d2101beb6f64d.rlib" "/tmp/rustc2sPxVB/liblibc-ae22292566331366.rlib" "-Wl,--end-group" "/rust/lib/rustlib/arm-unknown-linux-musleabi/lib/libcompiler_builtins-1aed8fa9822aaf63.rlib" "-Wl,-Bdynamic" "/rust/lib/rustlib/arm-unknown-linux-musleabi/lib/self-contained/crtn.o"
  = note: /usr/local/bin/../lib/gcc/arm-linux-musleabi/9.2.0/../../../../arm-linux-musleabi/bin/ld: /tmp/rustc2sPxVB/libopenssl_sys-0f67a64037e617dd.rlib(drbg_lib.o): in function `RAND_DRBG_instantiate':
          drbg_lib.c:(.text.RAND_DRBG_instantiate+0x180): undefined reference to `__time64'
          /usr/local/bin/../lib/gcc/arm-linux-musleabi/9.2.0/../../../../arm-linux-musleabi/bin/ld: /tmp/rustc2sPxVB/libopenssl_sys-0f67a64037e617dd.rlib(drbg_lib.o): in function `RAND_DRBG_reseed':
          drbg_lib.c:(.text.RAND_DRBG_reseed+0x154): undefined reference to `__time64'
          /usr/local/bin/../lib/gcc/arm-linux-musleabi/9.2.0/../../../../arm-linux-musleabi/bin/ld: /tmp/rustc2sPxVB/libopenssl_sys-0f67a64037e617dd.rlib(drbg_lib.o): in function `RAND_DRBG_generate':
          drbg_lib.c:(.text.RAND_DRBG_generate+0x164): undefined reference to `__time64'
          /usr/local/bin/../lib/gcc/arm-linux-musleabi/9.2.0/../../../../arm-linux-musleabi/bin/ld: /tmp/rustc2sPxVB/libopenssl_sys-0f67a64037e617dd.rlib(rand_unix.o): in function `check_random_device':
          rand_unix.c:(.text.check_random_device+0x28): undefined reference to `__fstat_time64'
          /usr/local/bin/../lib/gcc/arm-linux-musleabi/9.2.0/../../../../arm-linux-musleabi/bin/ld: /tmp/rustc2sPxVB/libopenssl_sys-0f67a64037e617dd.rlib(rand_unix.o): in function `wait_random_seeded':
          rand_unix.c:(.text.wait_random_seeded+0x184): undefined reference to `__select_time64'
          /usr/local/bin/../lib/gcc/arm-linux-musleabi/9.2.0/../../../../arm-linux-musleabi/bin/ld: /tmp/rustc2sPxVB/libopenssl_sys-0f67a64037e617dd.rlib(rand_unix.o): in function `rand_pool_acquire_entropy':
          rand_unix.c:(.text.rand_pool_acquire_entropy+0x134): undefined reference to `__fstat_time64'
          /usr/local/bin/../lib/gcc/arm-linux-musleabi/9.2.0/../../../../arm-linux-musleabi/bin/ld: /tmp/rustc2sPxVB/libopenssl_sys-0f67a64037e617dd.rlib(rand_unix.o): in function `rand_pool_add_nonce_data':
          rand_unix.c:(.text.rand_pool_add_nonce_data+0x44): undefined reference to `__clock_gettime64'
          /usr/local/bin/../lib/gcc/arm-linux-musleabi/9.2.0/../../../../arm-linux-musleabi/bin/ld: rand_unix.c:(.text.rand_pool_add_nonce_data+0x88): undefined reference to `__gettimeofday_time64'
          /usr/local/bin/../lib/gcc/arm-linux-musleabi/9.2.0/../../../../arm-linux-musleabi/bin/ld: rand_unix.c:(.text.rand_pool_add_nonce_data+0xb0): undefined reference to `__time64'
          /usr/local/bin/../lib/gcc/arm-linux-musleabi/9.2.0/../../../../arm-linux-musleabi/bin/ld: /tmp/rustc2sPxVB/libopenssl_sys-0f67a64037e617dd.rlib(rand_unix.o): in function `rand_pool_add_additional_data':
          rand_unix.c:(.text.rand_pool_add_additional_data+0x74): undefined reference to `__clock_gettime64'
          /usr/local/bin/../lib/gcc/arm-linux-musleabi/9.2.0/../../../../arm-linux-musleabi/bin/ld: rand_unix.c:(.text.rand_pool_add_additional_data+0x9c): undefined reference to `__gettimeofday_time64'
          /usr/local/bin/../lib/gcc/arm-linux-musleabi/9.2.0/../../../../arm-linux-musleabi/bin/ld: rand_unix.c:(.text.rand_pool_add_additional_data+0xc4): undefined reference to `__time64'
          /usr/local/bin/../lib/gcc/arm-linux-musleabi/9.2.0/../../../../arm-linux-musleabi/bin/ld: /tmp/rustc2sPxVB/libopenssl_sys-0f67a64037e617dd.rlib(conf_def.o): in function `def_load_bio':
          conf_def.c:(.text.def_load_bio+0x5dc): undefined reference to `__stat_time64'
          collect2: error: ld returned 1 exit status
nagisa commented 4 years ago

The added deprecation notice is now making the libstd build to fail, as it depends on said type.

This means that libstd is either stuck on 0.2.79 for indeterminate amount of time or we need to figure out a way to support either version of musl or give users ability to specify the version of musl they target?

JohnTitor commented 4 years ago

This means that libstd is either stuck on 0.2.79 for indeterminate amount of time or we need to figure out a way to support either version of musl or give users ability to specify the version of musl they target?

It's related to #1412 but we don't have any policy for supported platforms. Yes, both ways are better than just changing the type (also cc #547).

JohnTitor commented 4 years ago

So, we currently detect FreeBSD version, like: https://github.com/rust-lang/libc/blob/4c0a7e5b7ca949088a0ba17784fb043a22e4537e/build.rs#L122-L145

I wonder we could also detect musl version and separate time_t definition.

ericonr commented 3 years ago

I wonder we could also detect musl version and separate time_t definition.

How do you deal with the cross compilation case?

arndb commented 3 years ago

Note that this is not really musl specific, the change to 64-bit time_t needs to happen on all 32-bit implementations in order to have code working beyond year 2038. glibc on riscv uses 64-bit time_t, and patches for glibc and uclibc to convert all other architectures to make this a compile-time decision are in progress.

ericonr commented 3 years ago

Ok, I created a little proof of concept for the issue: https://github.com/ericonr/rust-time64

Currently, binaries built for 32-bit targets and interacting with C code can be miscompiled.

jeff-hiner commented 3 years ago

So the following code intended to produce a timeval for external C code generates deprecation warnings on both armv7-unknown-linux-musleabihf and aarch64-unknown-linux-musl. It says that libc::time_t is a deprecated alias, but tv_sec is a time_t per POSIX and I need a way to portably cast to it. What's the intended way to do this?

let now_duration = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap();

let tv = libc::timeval {
    tv_sec: now_duration.as_secs() as libc::time_t,
    tv_usec: now_duration.subsec_micros() as libc::suseconds_t,
};
kaniini commented 3 years ago

There are no plans to continue maintaining musl 1.1 (though there might be a security patch in the case of a catastrophic bug), so I think the best path forward is to standardize on musl 1.2.

pkgw commented 2 years ago

It feels so silly to ask this, but ... what is the actual course of action here if I have FFI code that triggers this deprecation notice? Try to ensure that time_t doesn't appear in the FFI layer, i.e. add casts on the C side as appropriate to change time_t into int64_t/i64?

rawler commented 2 years ago

Echoing @pkgw here. What is the right course of action here? libc::time_t is deprecated on musl. Should I use some other type-alias? Hard-coding i64 is likely to also break on some targets, right? Should I use libc::time_t but suppress the warning?

arndb commented 2 years ago

Any use of time_t/timeval/timespec that does not match the libc-defined type will cause data corruption. On both musl and libc you can encounter both 32-bit and 64-bit types. With musl, this is purely dependent on the version of the library, i.e. musl-1.2 only allows compiling with a 64-bit time_t, while glibc allows both, depending on whether the _TIME_BITS macro was set to '64' when building the C code one is linking against, same as the _FILE_OFFSET_BITS macro for off_t and other types.

The only portable way to do this is to avoid any use of time_t, struct timeval, struct timespec, ino_t, blkcnt_t, as well as any types derived from these, such as struct stat, struct dirent, or struct rlimit when interfacing with C/C++ code.

syrel commented 1 year ago

emscripten changed time_t to 64bit in July 2022, see https://github.com/emscripten-core/emscripten/issues/17393. Probably could be another reason to reconsider this issue.

detly commented 1 month ago

The only portable way to do this is to avoid any use of time_t, struct timeval, struct timespec, ino_t, blkcnt_t, as well as any types derived from these, such as struct stat, struct dirent, or struct rlimit when interfacing with C/C++ code.

Is this advice for libc itself (or anyone planning to work on libc), or for downstream developers? Because there are plenty of C APIs that use these types, so any wrapper crates will have to use those types to interface with them. For eg. rusb wraps libusb, which uses timeval in eg. libusb_wait_for_event() and others.

So, rusb seems to have no way to avoid using timeval, and users of rusb who might know the details of their target platform can't control the size mismatch.

arndb commented 1 month ago

In a specific package like rusb, the most reliable way to address this is to ensure that the underlying C library exports both time32 and time64 symbols in the way that glibc does. This way rusb can always use the time64 interface, while C based applications can pick the symbol based on their configuration.

Having a generic solution is much harder since a glibc based system may have a mix of time32 and time64 libraries installed, making it impossible to know which types are used in a particular library. As 32-bit Linux distros are increasingly going away or migrating to time64, I see two options, neither of which is a complete fix:

a) have some build-time feature detection to probe the defaults of the compiler, making the time_t/timespec/timeval/off_t/ino_t/... types depend on the __USE_TIME_BITS64 macro defined by the C libraries that are configured for time64. This is a lot of extra work but remains unreliable because the default flags used by the compiler may not reflect what a given library API was built with.

b) Drop time32 support on 32-bit Linux/glibc rust altogether and assume that the system libraries are all built for time64, same as with musl. This fixes the glibc systems that are currently broken but in turn breaks the ones that happen to work by chance today because they have not been converted yet or never will.