Skip to content

Commit

Permalink
Auto merge of #85953 - inquisitivecrystal:weak-linkat-in-fs-hardlink,…
Browse files Browse the repository at this point in the history
… r=joshtriplett

Fix linker error

Currently, `fs::hard_link` determines whether platforms have `linkat` based on the OS, and uses `link` if they don't. However, this heuristic does not work well if a platform provides `linkat` on newer versions but not on older ones. On old MacOS, this currently causes a linking error.

This commit fixes `fs::hard_link` by telling it to use `weak!` on macOS. This means that, on  that operating system, we now check for `linkat` at runtime and use `link` if it is not available.

Fixes #80804.

`@rustbot` label T-libs-impl
  • Loading branch information
bors committed Jul 10, 2021
2 parents 432e145 + 5999a5f commit dfd7b8d
Show file tree
Hide file tree
Showing 10 changed files with 91 additions and 17 deletions.
7 changes: 5 additions & 2 deletions library/std/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1736,8 +1736,11 @@ pub fn copy<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) -> io::Result<u64> {
///
/// # 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
Expand Down
18 changes: 18 additions & 0 deletions library/std/src/fs/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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")));
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/unix/android.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
50 changes: 39 additions & 11 deletions library/std/src/sys/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,23 @@ 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(
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"))]
Expand Down Expand Up @@ -92,7 +107,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<io::Result<FileAttr>> {
Expand All @@ -107,7 +122,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
Expand Down Expand Up @@ -756,7 +771,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,
) } {
Expand Down Expand Up @@ -1087,15 +1102,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) })?;
}
}
Expand Down Expand Up @@ -1278,7 +1306,7 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
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
}
Expand Down
1 change: 1 addition & 0 deletions library/std/src/sys/unix/kernel_copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
3 changes: 3 additions & 0 deletions library/std/src/sys/unix/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 8 additions & 0 deletions library/std/src/sys/unix/process/process_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
4 changes: 4 additions & 0 deletions library/std/src/sys/unix/rand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions library/std/src/sys/unix/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
13 changes: 10 additions & 3 deletions library/std/src/sys/unix/weak.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<unsafe extern "C" fn($($t),*) -> $ret> =
crate::sys::weak::Weak::new(concat!(stringify!($name), '\0'));
)
Expand Down Expand Up @@ -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;
Expand All @@ -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::*;
Expand Down

0 comments on commit dfd7b8d

Please sign in to comment.