Skip to content

Commit

Permalink
Rollup merge of rust-lang#85953 - inquisitivecrystal:weak-linkat-in-f…
Browse files Browse the repository at this point in the history
…s-hardlink, 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 rust-lang#80804.

``@rustbot`` label T-libs-impl
  • Loading branch information
JohnTitor authored Jul 7, 2021
2 parents d2b04f0 + 676699c commit 91206a7
Show file tree
Hide file tree
Showing 10 changed files with 76 additions and 16 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
1 change: 1 addition & 0 deletions library/std/src/sys/unix/android.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use libc::{c_int, c_void, sighandler_t, size_t, ssize_t};
use libc::{ftruncate, pread, pwrite};

use super::weak::weak;
use super::{cvt, cvt_r};
use crate::io;

Expand Down
42 changes: 31 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,15 @@ use crate::sys::time::SystemTime;
use crate::sys::{cvt, cvt_r};
use crate::sys_common::{AsInner, FromInner};

#[cfg(all(target_os = "linux", target_env = "gnu"))]
use crate::sys::weak::syscall;
#[cfg(target_os = "macos")]
use crate::sys::weak::{syscall, weak};

use libc::{c_int, mode_t};

#[cfg(any(target_os = "macos", 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 +99,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 +114,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 +763,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 +1094,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 +1298,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
1 change: 1 addition & 0 deletions library/std/src/sys/unix/process/process_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use crate::ptr;
use crate::sys;
use crate::sys::cvt;
use crate::sys::process::process_common::*;
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 {
// Null documentation to work around #57569, since nothing else seems to work
#[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 @@ -101,7 +104,9 @@ unsafe fn fetch(name: &str) -> usize {
}

#[cfg(not(any(target_os = "linux", target_os = "android")))]
macro_rules! syscall {
// Null documentation to work around #57569, since nothing else seems to work
#[doc = ""]
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 @@ -119,9 +124,11 @@ macro_rules! syscall {
}

#[cfg(any(target_os = "linux", target_os = "android"))]
macro_rules! syscall {
#[doc = ""]
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 91206a7

Please sign in to comment.