From a7bafc0afc3dba988a7bf85198c493b20523eaa0 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Wed, 26 Mar 2025 15:23:47 +0100 Subject: [PATCH] Change the syntax of the internal `weak!` macro Change the syntax to include parameter names and a trailing semicolon. Motivation: - Mirror the `syscall!` macro. - Allow rustfmt to format it (when wrapped in parentheses). - For better documentation (having the parameter names available in the source code is a bit nicer). - Allow future improvements to this macro where we can sometimes use the symbol directly when it's statically known to be available. --- library/std/src/sys/fs/unix.rs | 16 +++--- library/std/src/sys/pal/unix/fd.rs | 52 ++++++++++++++----- library/std/src/sys/pal/unix/kernel_copy.rs | 16 +++--- .../std/src/sys/pal/unix/stack_overflow.rs | 30 ++++++++--- library/std/src/sys/pal/unix/thread.rs | 13 +++-- library/std/src/sys/pal/unix/time.rs | 7 ++- library/std/src/sys/pal/unix/weak.rs | 40 ++++++++------ library/std/src/sys/process/unix/unix.rs | 42 ++++++++------- library/std/src/sys/random/linux.rs | 8 +-- 9 files changed, 144 insertions(+), 80 deletions(-) diff --git a/library/std/src/sys/fs/unix.rs b/library/std/src/sys/fs/unix.rs index 7c3ed8029f7dd..60e6559fa6b99 100644 --- a/library/std/src/sys/fs/unix.rs +++ b/library/std/src/sys/fs/unix.rs @@ -155,15 +155,15 @@ cfg_has_statx! {{ enum STATX_STATE{ Unknown = 0, Present, Unavailable } static STATX_SAVED_STATE: AtomicU8 = AtomicU8::new(STATX_STATE::Unknown as u8); - syscall! { + syscall!( fn statx( fd: c_int, pathname: *const c_char, flags: c_int, mask: libc::c_uint, - statxbuf: *mut libc::statx - ) -> c_int - } + statxbuf: *mut libc::statx, + ) -> c_int; + ); let statx_availability = STATX_SAVED_STATE.load(Ordering::Relaxed); if statx_availability == STATX_STATE::Unavailable as u8 { @@ -1540,7 +1540,9 @@ impl File { let times = [to_timespec(times.accessed)?, to_timespec(times.modified)?]; // futimens requires Android API level 19 cvt(unsafe { - weak!(fn futimens(c_int, *const libc::timespec) -> c_int); + weak!( + fn futimens(fd: c_int, times: *const libc::timespec) -> c_int; + ); match futimens.get() { Some(futimens) => futimens(self.as_raw_fd(), times.as_ptr()), None => return Err(io::const_error!( @@ -1556,7 +1558,9 @@ impl File { use crate::sys::{time::__timespec64, weak::weak}; // Added in glibc 2.34 - weak!(fn __futimens64(libc::c_int, *const __timespec64) -> libc::c_int); + weak!( + fn __futimens64(fd: c_int, times: *const __timespec64) -> c_int; + ); if let Some(futimens64) = __futimens64.get() { let to_timespec = |time: Option| time.map(|time| time.t.to_timespec64()) diff --git a/library/std/src/sys/pal/unix/fd.rs b/library/std/src/sys/pal/unix/fd.rs index f03c440e30e12..2ec8d01c13f46 100644 --- a/library/std/src/sys/pal/unix/fd.rs +++ b/library/std/src/sys/pal/unix/fd.rs @@ -232,14 +232,14 @@ impl FileDesc { // implementation if `preadv` is not available. #[cfg(all(target_os = "android", target_pointer_width = "64"))] pub fn read_vectored_at(&self, bufs: &mut [IoSliceMut<'_>], offset: u64) -> io::Result { - super::weak::syscall! { + super::weak::syscall!( fn preadv( fd: libc::c_int, iovec: *const libc::iovec, n_iovec: libc::c_int, - offset: off64_t - ) -> isize - } + offset: off64_t, + ) -> isize; + ); let ret = cvt(unsafe { preadv( @@ -257,7 +257,14 @@ impl FileDesc { // and its metadata from LLVM IR. #[no_sanitize(cfi)] pub fn read_vectored_at(&self, bufs: &mut [IoSliceMut<'_>], offset: u64) -> io::Result { - super::weak::weak!(fn preadv64(libc::c_int, *const libc::iovec, libc::c_int, off64_t) -> isize); + super::weak::weak!( + fn preadv64( + fd: libc::c_int, + iovec: *const libc::iovec, + n_iovec: libc::c_int, + offset: off64_t, + ) -> isize; + ); match preadv64.get() { Some(preadv) => { @@ -286,7 +293,14 @@ impl FileDesc { // use "weak" linking. #[cfg(target_vendor = "apple")] pub fn read_vectored_at(&self, bufs: &mut [IoSliceMut<'_>], offset: u64) -> io::Result { - super::weak::weak!(fn preadv(libc::c_int, *const libc::iovec, libc::c_int, off64_t) -> isize); + super::weak::weak!( + fn preadv( + fd: libc::c_int, + iovec: *const libc::iovec, + n_iovec: libc::c_int, + offset: off64_t, + ) -> isize; + ); match preadv.get() { Some(preadv) => { @@ -428,14 +442,14 @@ impl FileDesc { // implementation if `pwritev` is not available. #[cfg(all(target_os = "android", target_pointer_width = "64"))] pub fn write_vectored_at(&self, bufs: &[IoSlice<'_>], offset: u64) -> io::Result { - super::weak::syscall! { + super::weak::syscall!( fn pwritev( fd: libc::c_int, iovec: *const libc::iovec, n_iovec: libc::c_int, - offset: off64_t - ) -> isize - } + offset: off64_t, + ) -> isize; + ); let ret = cvt(unsafe { pwritev( @@ -450,7 +464,14 @@ impl FileDesc { #[cfg(all(target_os = "android", target_pointer_width = "32"))] pub fn write_vectored_at(&self, bufs: &[IoSlice<'_>], offset: u64) -> io::Result { - super::weak::weak!(fn pwritev64(libc::c_int, *const libc::iovec, libc::c_int, off64_t) -> isize); + super::weak::weak!( + fn pwritev64( + fd: libc::c_int, + iovec: *const libc::iovec, + n_iovec: libc::c_int, + offset: off64_t, + ) -> isize; + ); match pwritev64.get() { Some(pwritev) => { @@ -479,7 +500,14 @@ impl FileDesc { // use "weak" linking. #[cfg(target_vendor = "apple")] pub fn write_vectored_at(&self, bufs: &[IoSlice<'_>], offset: u64) -> io::Result { - super::weak::weak!(fn pwritev(libc::c_int, *const libc::iovec, libc::c_int, off64_t) -> isize); + super::weak::weak!( + fn pwritev( + fd: libc::c_int, + iovec: *const libc::iovec, + n_iovec: libc::c_int, + offset: off64_t, + ) -> isize; + ); match pwritev.get() { Some(pwritev) => { diff --git a/library/std/src/sys/pal/unix/kernel_copy.rs b/library/std/src/sys/pal/unix/kernel_copy.rs index bbf29f3252341..d42a7e2a7fc51 100644 --- a/library/std/src/sys/pal/unix/kernel_copy.rs +++ b/library/std/src/sys/pal/unix/kernel_copy.rs @@ -604,16 +604,16 @@ pub(super) fn copy_regular_files(reader: RawFd, writer: RawFd, max_len: u64) -> _ => true, }; - syscall! { + syscall!( fn copy_file_range( fd_in: libc::c_int, off_in: *mut libc::loff_t, fd_out: libc::c_int, off_out: *mut libc::loff_t, len: libc::size_t, - flags: libc::c_uint - ) -> libc::ssize_t - } + flags: libc::c_uint, + ) -> libc::ssize_t; + ); fn probe_copy_file_range_support() -> u8 { // In some cases, we cannot determine availability from the first @@ -727,16 +727,16 @@ fn sendfile_splice(mode: SpliceMode, reader: RawFd, writer: RawFd, len: u64) -> // Android builds use feature level 14, but the libc wrapper for splice is // gated on feature level 21+, so we have to invoke the syscall directly. #[cfg(target_os = "android")] - syscall! { + syscall!( fn splice( srcfd: libc::c_int, src_offset: *const i64, dstfd: libc::c_int, dst_offset: *const i64, len: libc::size_t, - flags: libc::c_int - ) -> libc::ssize_t - } + flags: libc::c_int, + ) -> libc::ssize_t; + ); #[cfg(target_os = "linux")] use libc::splice; diff --git a/library/std/src/sys/pal/unix/stack_overflow.rs b/library/std/src/sys/pal/unix/stack_overflow.rs index 4bd0cedd44ccb..34b3948e3f671 100644 --- a/library/std/src/sys/pal/unix/stack_overflow.rs +++ b/library/std/src/sys/pal/unix/stack_overflow.rs @@ -424,18 +424,32 @@ mod imp { let pages = PAGES.get_or_init(|| { use crate::sys::weak::dlsym; - dlsym!(fn sysctlbyname(*const libc::c_char, *mut libc::c_void, *mut libc::size_t, *const libc::c_void, libc::size_t) -> libc::c_int); + dlsym!( + fn sysctlbyname( + name: *const libc::c_char, + oldp: *mut libc::c_void, + oldlenp: *mut libc::size_t, + newp: *const libc::c_void, + newlen: libc::size_t, + ) -> libc::c_int; + ); let mut guard: usize = 0; let mut size = size_of_val(&guard); let oid = c"security.bsd.stack_guard_page"; match sysctlbyname.get() { - Some(fcn) if unsafe { - fcn(oid.as_ptr(), - (&raw mut guard).cast(), - &raw mut size, - ptr::null_mut(), - 0) == 0 - } => guard, + Some(fcn) + if unsafe { + fcn( + oid.as_ptr(), + (&raw mut guard).cast(), + &raw mut size, + ptr::null_mut(), + 0, + ) == 0 + } => + { + guard + } _ => 1, } }); diff --git a/library/std/src/sys/pal/unix/thread.rs b/library/std/src/sys/pal/unix/thread.rs index bb34c2fabe559..9078dd1c23166 100644 --- a/library/std/src/sys/pal/unix/thread.rs +++ b/library/std/src/sys/pal/unix/thread.rs @@ -193,11 +193,12 @@ impl Thread { // and its metadata from LLVM IR. #[no_sanitize(cfi)] pub fn set_name(name: &CStr) { - weak! { + weak!( fn pthread_setname_np( - libc::pthread_t, *const libc::c_char - ) -> libc::c_int - } + thread: libc::pthread_t, + name: *const libc::c_char, + ) -> libc::c_int; + ); if let Some(f) = pthread_setname_np.get() { #[cfg(target_os = "nto")] @@ -762,7 +763,9 @@ unsafe fn min_stack_size(attr: *const libc::pthread_attr_t) -> usize { // We use dlsym to avoid an ELF version dependency on GLIBC_PRIVATE. (#23628) // We shouldn't really be using such an internal symbol, but there's currently // no other way to account for the TLS size. - dlsym!(fn __pthread_get_minstack(*const libc::pthread_attr_t) -> libc::size_t); + dlsym!( + fn __pthread_get_minstack(attr: *const libc::pthread_attr_t) -> libc::size_t; + ); match __pthread_get_minstack.get() { None => libc::PTHREAD_STACK_MIN, diff --git a/library/std/src/sys/pal/unix/time.rs b/library/std/src/sys/pal/unix/time.rs index c0a3044660b7e..b8469b1681f03 100644 --- a/library/std/src/sys/pal/unix/time.rs +++ b/library/std/src/sys/pal/unix/time.rs @@ -123,7 +123,12 @@ impl Timespec { // __clock_gettime64 was added to 32-bit arches in glibc 2.34, // and it handles both vDSO calls and ENOSYS fallbacks itself. - weak!(fn __clock_gettime64(libc::clockid_t, *mut __timespec64) -> libc::c_int); + weak!( + fn __clock_gettime64( + clockid: libc::clockid_t, + tp: *mut __timespec64, + ) -> libc::c_int; + ); if let Some(clock_gettime64) = __clock_gettime64.get() { let mut t = MaybeUninit::uninit(); diff --git a/library/std/src/sys/pal/unix/weak.rs b/library/std/src/sys/pal/unix/weak.rs index ce3f66a83748b..e7f4e005cc48c 100644 --- a/library/std/src/sys/pal/unix/weak.rs +++ b/library/std/src/sys/pal/unix/weak.rs @@ -29,7 +29,7 @@ use crate::{mem, ptr}; // We can use true weak linkage on ELF targets. #[cfg(all(unix, not(target_vendor = "apple")))] pub(crate) macro weak { - (fn $name:ident($($t:ty),*) -> $ret:ty) => ( + (fn $name:ident($($param:ident : $t:ty),* $(,)?) -> $ret:ty;) => ( let ref $name: ExternWeak $ret> = { unsafe extern "C" { #[linkage = "extern_weak"] @@ -62,10 +62,16 @@ impl ExternWeak { } pub(crate) macro dlsym { - (fn $name:ident($($t:ty),*) -> $ret:ty) => ( - dlsym!(fn $name($($t),*) -> $ret, stringify!($name)); + (fn $name:ident($($param:ident : $t:ty),* $(,)?) -> $ret:ty;) => ( + dlsym!( + #[link_name = stringify!($name)] + fn $name($($param : $t),*) -> $ret; + ); ), - (fn $name:ident($($t:ty),*) -> $ret:ty, $sym:expr) => ( + ( + #[link_name = $sym:expr] + fn $name:ident($($param:ident : $t:ty),* $(,)?) -> $ret:ty; + ) => ( static DLSYM: DlsymWeak $ret> = DlsymWeak::new(concat!($sym, '\0')); let $name = &DLSYM; @@ -143,15 +149,15 @@ unsafe fn fetch(name: &str) -> *mut libc::c_void { #[cfg(not(any(target_os = "linux", target_os = "android")))] pub(crate) macro syscall { - (fn $name:ident($($arg_name:ident: $t:ty),*) -> $ret:ty) => ( + (fn $name:ident($($param:ident : $t:ty),* $(,)?) -> $ret:ty;) => ( // FIXME(#115199): Rust currently omits weak function definitions // and its metadata from LLVM IR. #[no_sanitize(cfi)] - unsafe fn $name($($arg_name: $t),*) -> $ret { - weak! { fn $name($($t),*) -> $ret } + unsafe fn $name($($param: $t),*) -> $ret { + weak!(fn $name($($param: $t),*) -> $ret;); if let Some(fun) = $name.get() { - fun($($arg_name),*) + fun($($param),*) } else { super::os::set_errno(libc::ENOSYS); -1 @@ -162,16 +168,18 @@ pub(crate) macro syscall { #[cfg(any(target_os = "linux", target_os = "android"))] pub(crate) macro syscall { - (fn $name:ident($($arg_name:ident: $t:ty),*) -> $ret:ty) => ( - unsafe fn $name($($arg_name:$t),*) -> $ret { - weak! { fn $name($($t),*) -> $ret } + ( + fn $name:ident($($param:ident : $t:ty),* $(,)?) -> $ret:ty; + ) => ( + unsafe fn $name($($param: $t),*) -> $ret { + weak!(fn $name($($param: $t),*) -> $ret;); // Use a weak symbol from libc when possible, allowing `LD_PRELOAD` // interposition, but if it's not found just use a raw syscall. if let Some(fun) = $name.get() { - fun($($arg_name),*) + fun($($param),*) } else { - libc::syscall(libc::${concat(SYS_, $name)}, $($arg_name),*) as $ret + libc::syscall(libc::${concat(SYS_, $name)}, $($param),*) as $ret } } ) @@ -179,9 +187,9 @@ pub(crate) macro syscall { #[cfg(any(target_os = "linux", target_os = "android"))] pub(crate) macro raw_syscall { - (fn $name:ident($($arg_name:ident: $t:ty),*) -> $ret:ty) => ( - unsafe fn $name($($arg_name:$t),*) -> $ret { - libc::syscall(libc::${concat(SYS_, $name)}, $($arg_name),*) as $ret + (fn $name:ident($($param:ident : $t:ty),* $(,)?) -> $ret:ty;) => ( + unsafe fn $name($($param: $t),*) -> $ret { + libc::syscall(libc::${concat(SYS_, $name)}, $($param),*) as $ret } ) } diff --git a/library/std/src/sys/process/unix/unix.rs b/library/std/src/sys/process/unix/unix.rs index 42542f81b6545..191a09c8da913 100644 --- a/library/std/src/sys/process/unix/unix.rs +++ b/library/std/src/sys/process/unix/unix.rs @@ -461,18 +461,20 @@ impl Command { if #[cfg(target_os = "linux")] { use crate::sys::weak::weak; - weak! { + weak!( fn pidfd_spawnp( - *mut libc::c_int, - *const libc::c_char, - *const libc::posix_spawn_file_actions_t, - *const libc::posix_spawnattr_t, - *const *mut libc::c_char, - *const *mut libc::c_char - ) -> libc::c_int - } + pidfd: *mut libc::c_int, + path: *const libc::c_char, + file_actions: *const libc::posix_spawn_file_actions_t, + attrp: *const libc::posix_spawnattr_t, + argv: *const *mut libc::c_char, + envp: *const *mut libc::c_char, + ) -> libc::c_int; + ); - weak! { fn pidfd_getpid(libc::c_int) -> libc::c_int } + weak!( + fn pidfd_getpid(pidfd: libc::c_int) -> libc::c_int; + ); static PIDFD_SUPPORTED: AtomicU8 = AtomicU8::new(0); const UNKNOWN: u8 = 0; @@ -593,19 +595,19 @@ impl Command { // https://pubs.opengroup.org/onlinepubs/9799919799/functions/posix_spawn_file_actions_addchdir.html. // The _np version is more widely available, though, so try that first. - weak! { + weak!( fn posix_spawn_file_actions_addchdir_np( - *mut libc::posix_spawn_file_actions_t, - *const libc::c_char - ) -> libc::c_int - } + file_actions: *mut libc::posix_spawn_file_actions_t, + path: *const libc::c_char, + ) -> libc::c_int; + ); - weak! { + weak!( fn posix_spawn_file_actions_addchdir( - *mut libc::posix_spawn_file_actions_t, - *const libc::c_char - ) -> libc::c_int - } + file_actions: *mut libc::posix_spawn_file_actions_t, + path: *const libc::c_char, + ) -> libc::c_int; + ); posix_spawn_file_actions_addchdir_np .get() diff --git a/library/std/src/sys/random/linux.rs b/library/std/src/sys/random/linux.rs index e3cb79285cd15..c0591ec0c1527 100644 --- a/library/std/src/sys/random/linux.rs +++ b/library/std/src/sys/random/linux.rs @@ -73,13 +73,13 @@ fn getrandom(mut bytes: &mut [u8], insecure: bool) { // A weak symbol allows interposition, e.g. for perf measurements that want to // disable randomness for consistency. Otherwise, we'll try a raw syscall. // (`getrandom` was added in glibc 2.25, musl 1.1.20, android API level 28) - syscall! { + syscall!( fn getrandom( buffer: *mut libc::c_void, length: libc::size_t, - flags: libc::c_uint - ) -> libc::ssize_t - } + flags: libc::c_uint, + ) -> libc::ssize_t; + ); static GETRANDOM_AVAILABLE: AtomicBool = AtomicBool::new(true); static GRND_INSECURE_AVAILABLE: AtomicBool = AtomicBool::new(true);