Skip to content

Use libc::SYS_futex_time64 on riscv32 #485

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.

Sign up for GitHub

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

Open
kraj wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use libc::SYS_futex_time64 on riscv32 #485

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions core/src/thread_parker/linux.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,13 @@ impl ThreadParker {
.as_ref()
.map(|ts_ref| ts_ref as *const _)
.unwrap_or(ptr::null());
#[cfg(not(all(target_arch = "riscv32", target_env = "musl")))]
Copy link
Owner

@Amanieu Amanieu Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this specific to musl? Shouldn't this affect all riscv32 targets?

Copy link
Author

@kraj kraj Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this specific to musl? Shouldn't this affect all riscv32 targets?

glibc seems to alias SYS_futex to 64bit syscall, unlike musl

Copy link
Owner

@Amanieu Amanieu Jul 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still incorrect on musl because libc currently defines timespec as using 32-bit time while the syscall expects a 64-bit timespec:

src/unix/linux_like/linux/musl/mod.rs:pub type time_t = c_long;

Proper support for riscv32 would require some work in libc to support these types.

Copy link
Author

@kraj kraj Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats a rust libc bug you have uncovered :) because unlike glibc time_t is always 64bit on musl for all architectures ( both 32bit and 64bit) so that value should perhaps use dedicated 64bit type instead of c_ulong

Copy link
Author

@kraj kraj Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what musl has

arch/aarch64/bits/alltypes.h.in:#define _Int64 long
arch/arm/bits/alltypes.h.in:#define _Int64 long long
arch/i386/bits/alltypes.h.in:#define _Int64 long long
arch/loongarch64/bits/alltypes.h.in:#define _Int64 long
arch/m68k/bits/alltypes.h.in:#define _Int64 long long
arch/microblaze/bits/alltypes.h.in:#define _Int64 long long
arch/mips/bits/alltypes.h.in:#define _Int64 long long
arch/mips64/bits/alltypes.h.in:#define _Int64 long
arch/mipsn32/bits/alltypes.h.in:#define _Int64 long long
arch/or1k/bits/alltypes.h.in:#define _Int64 long long
arch/powerpc/bits/alltypes.h.in:#define _Int64 long long
arch/powerpc64/bits/alltypes.h.in:#define _Int64 long
arch/riscv32/bits/alltypes.h.in:#define _Int64 long long
arch/riscv64/bits/alltypes.h.in:#define _Int64 long
arch/s390x/bits/alltypes.h.in:#define _Int64 long
arch/sh/bits/alltypes.h.in:#define _Int64 long long
arch/x32/bits/alltypes.h.in:#define _Int64 long long
arch/x86_64/bits/alltypes.h.in:#define _Int64 long
include/alltypes.h.in:TYPEDEF _Int64 time_t;

Copy link
Owner

@Amanieu Amanieu Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the libc bug needs to be addressed before this PR can be merged.

Copy link
Author

@kraj kraj Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see dependency with libc bug for this PR

Copy link
Author

@kraj kraj Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have created a PR for rust libc as well
rust-lang/libc#4581

let futex_num = libc::SYS_futex;
#[cfg(all(target_arch = "riscv32", target_env = "musl"))]
let futex_num = libc::SYS_futex_time64;
let r = unsafe {
libc::syscall(
libc::SYS_futex,
futex_num,
&self.futex,
libc::FUTEX_WAIT | libc::FUTEX_PRIVATE_FLAG,
1,
Expand All @@ -137,8 +141,13 @@ impl super::UnparkHandleT for UnparkHandle {
unsafe fn unpark(self) {
// The thread data may have been freed at this point, but it doesn't
// matter since the syscall will just return EFAULT in that case.
#[cfg(not(all(target_arch = "riscv32", target_env = "musl",)))]
let futex_num = libc::SYS_futex;
#[cfg(all(target_arch = "riscv32", target_env = "musl",))]
let futex_num = libc::SYS_futex_time64;

let r = libc::syscall(
libc::SYS_futex,
futex_num,
self.futex,
libc::FUTEX_WAKE | libc::FUTEX_PRIVATE_FLAG,
1,
Expand Down
Loading