From dc38d87505f4b1e381f143b4f34704df6a24e57e Mon Sep 17 00:00:00 2001 From: Aris Merchant Date: Wed, 2 Jun 2021 22:53:03 -0700 Subject: [PATCH 1/4] Fix linker error This makes `fs::hard_link` use weak! for some platforms, thereby preventing a linker error. --- library/std/src/sys/unix/fs.rs | 41 +++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index bec0d5898ac92..4bbf334fc13d2 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -14,6 +14,12 @@ use crate::sys_common::{AsInner, FromInner}; use libc::{c_int, mode_t}; +#[cfg(any( + target_os = "macos", + target_os = "ios", + all(target_os = "linux", target_env = "gnu") +))] +use libc::c_char; #[cfg(any(target_os = "linux", target_os = "emscripten", target_os = "android"))] use libc::dirfd; #[cfg(any(target_os = "linux", target_os = "emscripten"))] @@ -92,7 +98,7 @@ cfg_has_statx! {{ // Default `stat64` contains no creation time. unsafe fn try_statx( fd: c_int, - path: *const libc::c_char, + path: *const c_char, flags: i32, mask: u32, ) -> Option> { @@ -107,7 +113,7 @@ cfg_has_statx! {{ syscall! { fn statx( fd: c_int, - pathname: *const libc::c_char, + pathname: *const c_char, flags: c_int, mask: libc::c_uint, statxbuf: *mut libc::statx @@ -756,7 +762,7 @@ impl File { cfg_has_statx! { if let Some(ret) = unsafe { try_statx( fd, - b"\0" as *const _ as *const libc::c_char, + b"\0" as *const _ as *const c_char, libc::AT_EMPTY_PATH | libc::AT_STATX_SYNC_AS_STAT, libc::STATX_ALL, ) } { @@ -1087,15 +1093,28 @@ pub fn link(original: &Path, link: &Path) -> io::Result<()> { let link = cstr(link)?; cfg_if::cfg_if! { if #[cfg(any(target_os = "vxworks", target_os = "redox", target_os = "android"))] { - // VxWorks, Redox, and old versions of Android lack `linkat`, so use - // `link` instead. POSIX leaves it implementation-defined whether - // `link` follows symlinks, so rely on the `symlink_hard_link` test - // in library/std/src/fs/tests.rs to check the behavior. + // VxWorks and Redox lack `linkat`, so use `link` instead. POSIX leaves + // it implementation-defined whether `link` follows symlinks, so rely on the + // `symlink_hard_link` test in library/std/src/fs/tests.rs to check the behavior. + // Android has `linkat` on newer versions, but we happen to know `link` + // always has the correct behavior, so it's here as well. cvt(unsafe { libc::link(original.as_ptr(), link.as_ptr()) })?; + } else if #[cfg(target_os = "macos")] { + // On MacOS, older versions (<=10.9) lack support for linkat while newer + // versions have it. We want to use linkat if it is available, so we use weak! + // to check. `linkat` is preferable to `link` ecause it gives us a flag to + // specify how symlinks should be handled. We pass 0 as the flags argument, + // meaning it shouldn't follow symlinks. + weak!(fn linkat(c_int, *const c_char, c_int, *const c_char, c_int) -> c_int); + + if let Some(f) = linkat.get() { + cvt(unsafe { f(libc::AT_FDCWD, original.as_ptr(), libc::AT_FDCWD, link.as_ptr(), 0) })?; + } else { + cvt(unsafe { libc::link(original.as_ptr(), link.as_ptr()) })?; + }; } else { - // Use `linkat` with `AT_FDCWD` instead of `link` as `linkat` gives - // us a flag to specify how symlinks should be handled. Pass 0 as - // the flags argument, meaning don't follow symlinks. + // Where we can, use `linkat` instead of `link`; see the comment above + // this one for details on why. cvt(unsafe { libc::linkat(libc::AT_FDCWD, original.as_ptr(), libc::AT_FDCWD, link.as_ptr(), 0) })?; } } @@ -1278,7 +1297,7 @@ pub fn copy(from: &Path, to: &Path) -> io::Result { fn fclonefileat( srcfd: libc::c_int, dst_dirfd: libc::c_int, - dst: *const libc::c_char, + dst: *const c_char, flags: libc::c_int ) -> libc::c_int } From 5022c0638dfbb83e0b38ce90f79d0fd6a4d295d6 Mon Sep 17 00:00:00 2001 From: Aris Merchant <22333129+inquisitivecrystal@users.noreply.github.com> Date: Wed, 7 Jul 2021 00:23:29 -0700 Subject: [PATCH 2/4] Update docs for `fs::hard_link` --- library/std/src/fs.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index a9ce814e2ec18..fb928ea55985f 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -1736,8 +1736,11 @@ pub fn copy, Q: AsRef>(from: P, to: Q) -> io::Result { /// /// # Platform-specific behavior /// -/// This function currently corresponds to the `linkat` function with no flags -/// on Unix and the `CreateHardLink` function on Windows. +/// This function currently corresponds the `CreateHardLink` function on Windows. +/// On most Unix systems, it corresponds to the `linkat` function with no flags. +/// On Android, VxWorks, and Redox, it instead corresponds to the `link` function. +/// On MacOS, it uses the `linkat` function if it is available, but on very old +/// systems where `linkat` is not available, `link` is selected at runtime instead. /// Note that, this [may change in the future][changes]. /// /// [changes]: io#platform-specific-behavior From fd0cb0cdc21dd9c06025277d772108f8d42cb25f Mon Sep 17 00:00:00 2001 From: Aris Merchant <22333129+inquisitivecrystal@users.noreply.github.com> Date: Mon, 5 Jul 2021 20:28:10 -0700 Subject: [PATCH 3/4] Change `weak!` and `linkat!` to macros 2.0 `weak!` is needed in a test in another module. With macros 1.0, importing `weak!` would require reordering module declarations in `std/src/lib.rs`, which is a bit too evil. --- library/std/src/sys/unix/android.rs | 2 +- library/std/src/sys/unix/fs.rs | 9 +++++++++ library/std/src/sys/unix/kernel_copy.rs | 1 + library/std/src/sys/unix/os.rs | 3 +++ library/std/src/sys/unix/process/process_unix.rs | 8 ++++++++ library/std/src/sys/unix/rand.rs | 4 ++++ library/std/src/sys/unix/thread.rs | 2 ++ library/std/src/sys/unix/weak.rs | 13 ++++++++++--- 8 files changed, 38 insertions(+), 4 deletions(-) diff --git a/library/std/src/sys/unix/android.rs b/library/std/src/sys/unix/android.rs index cf6aa31b7cfe3..6a46525f682c4 100644 --- a/library/std/src/sys/unix/android.rs +++ b/library/std/src/sys/unix/android.rs @@ -21,7 +21,7 @@ use libc::{c_int, c_void, sighandler_t, size_t, ssize_t}; use libc::{ftruncate, pread, pwrite}; -use super::{cvt, cvt_r}; +use super::{cvt, cvt_r, weak::weak}; use crate::io; // The `log2` and `log2f` functions apparently appeared in android-18, or at diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 4bbf334fc13d2..5c8c94971c33c 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -12,6 +12,15 @@ use crate::sys::time::SystemTime; use crate::sys::{cvt, cvt_r}; use crate::sys_common::{AsInner, FromInner}; +#[cfg(any( + all(target_os = "linux", target_env = "gnu"), + target_os = "macos", + target_os = "ios", +))] +use crate::sys::weak::syscall; +#[cfg(target_os = "macos")] +use crate::sys::weak::weak; + use libc::{c_int, mode_t}; #[cfg(any( diff --git a/library/std/src/sys/unix/kernel_copy.rs b/library/std/src/sys/unix/kernel_copy.rs index 9687576bb6aeb..a6b43229ba6b7 100644 --- a/library/std/src/sys/unix/kernel_copy.rs +++ b/library/std/src/sys/unix/kernel_copy.rs @@ -61,6 +61,7 @@ use crate::process::{ChildStderr, ChildStdin, ChildStdout}; use crate::ptr; use crate::sync::atomic::{AtomicBool, AtomicU8, Ordering}; use crate::sys::cvt; +use crate::sys::weak::syscall; use libc::{EBADF, EINVAL, ENOSYS, EOPNOTSUPP, EOVERFLOW, EPERM, EXDEV}; #[cfg(test)] diff --git a/library/std/src/sys/unix/os.rs b/library/std/src/sys/unix/os.rs index 71fec79347a87..d3c874edf2dc4 100644 --- a/library/std/src/sys/unix/os.rs +++ b/library/std/src/sys/unix/os.rs @@ -23,6 +23,9 @@ use crate::sys::memchr; use crate::sys_common::rwlock::{StaticRWLock, StaticRWLockReadGuard}; use crate::vec; +#[cfg(all(target_env = "gnu", not(target_os = "vxworks")))] +use crate::sys::weak::weak; + use libc::{c_char, c_int, c_void}; const TMPBUF_SZ: usize = 128; diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index ed55e1aa715ae..c888dd0d87d8e 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -9,6 +9,14 @@ use crate::sys; use crate::sys::cvt; use crate::sys::process::process_common::*; +#[cfg(any( + target_os = "macos", + target_os = "freebsd", + all(target_os = "linux", target_env = "gnu"), + all(target_os = "linux", target_env = "musl"), +))] +use crate::sys::weak::weak; + #[cfg(target_os = "vxworks")] use libc::RTP_ID as pid_t; diff --git a/library/std/src/sys/unix/rand.rs b/library/std/src/sys/unix/rand.rs index 44f9eabc319a0..32895001a65ba 100644 --- a/library/std/src/sys/unix/rand.rs +++ b/library/std/src/sys/unix/rand.rs @@ -25,6 +25,9 @@ mod imp { use crate::fs::File; use crate::io::Read; + #[cfg(any(target_os = "linux", target_os = "android"))] + use crate::sys::weak::syscall; + #[cfg(any(target_os = "linux", target_os = "android"))] fn getrandom(buf: &mut [u8]) -> libc::ssize_t { // A weak symbol allows interposition, e.g. for perf measurements that want to @@ -108,6 +111,7 @@ mod imp { use crate::fs::File; use crate::io::Read; use crate::sys::os::errno; + use crate::sys::weak::weak; use libc::{c_int, c_void, size_t}; fn getentropy_fill_bytes(v: &mut [u8]) -> bool { diff --git a/library/std/src/sys/unix/thread.rs b/library/std/src/sys/unix/thread.rs index df2ba0a8bc8e6..879d716052497 100644 --- a/library/std/src/sys/unix/thread.rs +++ b/library/std/src/sys/unix/thread.rs @@ -7,6 +7,8 @@ use crate::ptr; use crate::sys::{os, stack_overflow}; use crate::time::Duration; +#[cfg(any(target_os = "linux", target_os = "solaris", target_os = "illumos"))] +use crate::sys::weak::weak; #[cfg(not(any(target_os = "l4re", target_os = "vxworks")))] pub const DEFAULT_MIN_STACK_SIZE: usize = 2 * 1024 * 1024; #[cfg(target_os = "l4re")] diff --git a/library/std/src/sys/unix/weak.rs b/library/std/src/sys/unix/weak.rs index 432fe4c33bcc4..cad8be6d289ee 100644 --- a/library/std/src/sys/unix/weak.rs +++ b/library/std/src/sys/unix/weak.rs @@ -26,8 +26,11 @@ use crate::marker; use crate::mem; use crate::sync::atomic::{self, AtomicUsize, Ordering}; -macro_rules! weak { +// Temporary null documentation to work around #57569 until the fix is beta +#[cfg_attr(bootstrap, doc = "")] +pub(crate) macro weak { (fn $name:ident($($t:ty),*) -> $ret:ty) => ( + #[allow(non_upper_case_globals)] static $name: crate::sys::weak::Weak $ret> = crate::sys::weak::Weak::new(concat!(stringify!($name), '\0')); ) @@ -100,8 +103,10 @@ unsafe fn fetch(name: &str) -> usize { libc::dlsym(libc::RTLD_DEFAULT, name.as_ptr()) as usize } +// Temporary null documentation to work around #57569 until the fix is beta +#[cfg_attr(bootstrap, doc = "")] #[cfg(not(any(target_os = "linux", target_os = "android")))] -macro_rules! syscall { +pub(crate) macro syscall { (fn $name:ident($($arg_name:ident: $t:ty),*) -> $ret:ty) => ( unsafe fn $name($($arg_name: $t),*) -> $ret { use super::os; @@ -118,10 +123,12 @@ macro_rules! syscall { ) } +#[cfg_attr(bootstrap, doc = "")] #[cfg(any(target_os = "linux", target_os = "android"))] -macro_rules! syscall { +pub(crate) macro syscall { (fn $name:ident($($arg_name:ident: $t:ty),*) -> $ret:ty) => ( unsafe fn $name($($arg_name:$t),*) -> $ret { + use weak; // This looks like a hack, but concat_idents only accepts idents // (not paths). use libc::*; From 5999a5fbdc91ac07d4103095ed532d8cd4d3443b Mon Sep 17 00:00:00 2001 From: Aris Merchant <22333129+inquisitivecrystal@users.noreply.github.com> Date: Mon, 5 Jul 2021 20:44:55 -0700 Subject: [PATCH 4/4] Make tests pass on old macos On old macos systems, `fs::hard_link()` will follow symlinks. This changes the test `symlink_hard_link` to exit early on these systems, so that tests can pass. --- library/std/src/fs/tests.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/library/std/src/fs/tests.rs b/library/std/src/fs/tests.rs index 127b7bf34a3db..080b4b5d87f26 100644 --- a/library/std/src/fs/tests.rs +++ b/library/std/src/fs/tests.rs @@ -19,6 +19,10 @@ use crate::os::unix::fs::symlink as symlink_junction; use crate::os::windows::fs::{symlink_dir, symlink_file}; #[cfg(windows)] use crate::sys::fs::symlink_junction; +#[cfg(target_os = "macos")] +use crate::sys::weak::weak; +#[cfg(target_os = "macos")] +use libc::{c_char, c_int}; macro_rules! check { ($e:expr) => { @@ -79,6 +83,17 @@ pub fn got_symlink_permission(tmpdir: &TempDir) -> bool { } } +#[cfg(target_os = "macos")] +fn able_to_not_follow_symlinks_while_hard_linking() -> bool { + weak!(fn linkat(c_int, *const c_char, c_int, *const c_char, c_int) -> c_int); + linkat.get().is_some() +} + +#[cfg(not(target_os = "macos"))] +fn able_to_not_follow_symlinks_while_hard_linking() -> bool { + return true; +} + #[test] fn file_test_io_smoke_test() { let message = "it's alright. have a good time"; @@ -1347,6 +1362,9 @@ fn symlink_hard_link() { if !got_symlink_permission(&tmpdir) { return; }; + if !able_to_not_follow_symlinks_while_hard_linking() { + return; + } // Create "file", a file. check!(fs::File::create(tmpdir.join("file")));