Auto merge of #90846 - cuviper:weak, r=dtolnay

Refactor weak symbols in std::sys::unix

This makes a few changes to the weak symbol macros in `sys::unix`:

- `dlsym!` is added to keep the functionality for runtime `dlsym`
  lookups, like for `__pthread_get_minstack@GLIBC_PRIVATE` that we don't
  want to show up in ELF symbol tables.
- `weak!` now uses `#[linkage = "extern_weak"]` symbols, so its runtime
  behavior is just a simple null check. This is also used by `syscall!`.
  - On non-ELF targets (macos/ios) where that linkage is not known to
    behave, `weak!` is just an alias to `dlsym!` for the old behavior.
- `raw_syscall!` is added to always call `libc::syscall` on linux and
  android, for cases like `clone3` that have no known libc wrapper.

The new `weak!` linkage does mean that you'll get versioned symbols if
you build with a newer glibc, like `WEAK DEFAULT UND statx@GLIBC_2.28`.
This might seem problematic, but old non-weak symbols can tie the build
to new versions too, like `dlsym@GLIBC_2.34` from their recent library
unification. If you build with an old glibc like `dist-x86_64-linux`
does, you'll still get unversioned `WEAK DEFAULT UND statx`, which may
be resolved based on the runtime glibc.

I also found a few functions that don't need to be weak anymore:

- Android can directly use `ftruncate64`, `pread64`, and `pwrite64`, as
  these were added in API 12, and our baseline is API 14.
- Linux can directly use `splice`, added way back in glibc 2.5 and
  similarly old musl. Android only added it in API 21 though.

Author: bors

library/std/src/sys/unix/android.rs

@@ -18,11 +18,9 @@
 
 #![cfg(target_os = "android")]
 
-use libc::{c_int, c_void, sighandler_t, size_t, ssize_t};
-use libc::{ftruncate, pread, pwrite};
+use libc::{c_int, sighandler_t};
 
-use super::{cvt, cvt_r, weak::weak};
-use crate::io;
+use super::weak::weak;
 
 // The `log2` and `log2f` functions apparently appeared in android-18, or at
 // least you can see they're not present in the android-17 header [1] and they
@@ -81,87 +79,3 @@ pub unsafe fn signal(signum: c_int, handler: sighandler_t) -> sighandler_t {
     let f = f.expect("neither `signal` nor `bsd_signal` symbols found");
     f(signum, handler)
 }
-
-// The `ftruncate64` symbol apparently appeared in android-12, so we do some
-// dynamic detection to see if we can figure out whether `ftruncate64` exists.
-//
-// If it doesn't we just fall back to `ftruncate`, generating an error for
-// too-large values.
-#[cfg(target_pointer_width = "32")]
-pub fn ftruncate64(fd: c_int, size: u64) -> io::Result<()> {
-    weak!(fn ftruncate64(c_int, i64) -> c_int);
-
-    unsafe {
-        match ftruncate64.get() {
-            Some(f) => cvt_r(|| f(fd, size as i64)).map(drop),
-            None => {
-                if size > i32::MAX as u64 {
-                    Err(io::Error::new_const(io::ErrorKind::InvalidInput, &"cannot truncate >2GB"))
-                } else {
-                    cvt_r(|| ftruncate(fd, size as i32)).map(drop)
-                }
-            }
-        }
-    }
-}
-
-#[cfg(target_pointer_width = "64")]
-pub fn ftruncate64(fd: c_int, size: u64) -> io::Result<()> {
-    unsafe { cvt_r(|| ftruncate(fd, size as i64)).map(drop) }
-}
-
-#[cfg(target_pointer_width = "32")]
-pub unsafe fn cvt_pread64(
-    fd: c_int,
-    buf: *mut c_void,
-    count: size_t,
-    offset: i64,
-) -> io::Result<ssize_t> {
-    use crate::convert::TryInto;
-    weak!(fn pread64(c_int, *mut c_void, size_t, i64) -> ssize_t);
-    pread64.get().map(|f| cvt(f(fd, buf, count, offset))).unwrap_or_else(|| {
-        if let Ok(o) = offset.try_into() {
-            cvt(pread(fd, buf, count, o))
-        } else {
-            Err(io::Error::new_const(io::ErrorKind::InvalidInput, &"cannot pread >2GB"))
-        }
-    })
-}
-
-#[cfg(target_pointer_width = "32")]
-pub unsafe fn cvt_pwrite64(
-    fd: c_int,
-    buf: *const c_void,
-    count: size_t,
-    offset: i64,
-) -> io::Result<ssize_t> {
-    use crate::convert::TryInto;
-    weak!(fn pwrite64(c_int, *const c_void, size_t, i64) -> ssize_t);
-    pwrite64.get().map(|f| cvt(f(fd, buf, count, offset))).unwrap_or_else(|| {
-        if let Ok(o) = offset.try_into() {
-            cvt(pwrite(fd, buf, count, o))
-        } else {
-            Err(io::Error::new_const(io::ErrorKind::InvalidInput, &"cannot pwrite >2GB"))
-        }
-    })
-}
-
-#[cfg(target_pointer_width = "64")]
-pub unsafe fn cvt_pread64(
-    fd: c_int,
-    buf: *mut c_void,
-    count: size_t,
-    offset: i64,
-) -> io::Result<ssize_t> {
-    cvt(pread(fd, buf, count, offset))
-}
-
-#[cfg(target_pointer_width = "64")]
-pub unsafe fn cvt_pwrite64(
-    fd: c_int,
-    buf: *const c_void,
-    count: size_t,
-    offset: i64,
-) -> io::Result<ssize_t> {
-    cvt(pwrite(fd, buf, count, offset))
-}

library/std/src/sys/unix/fd.rs

@@ -99,30 +99,18 @@ impl FileDesc {
     }
 
     pub fn read_at(&self, buf: &mut [u8], offset: u64) -> io::Result<usize> {
-        #[cfg(target_os = "android")]
-        use super::android::cvt_pread64;
-
-        #[cfg(not(target_os = "android"))]
-        unsafe fn cvt_pread64(
-            fd: c_int,
-            buf: *mut c_void,
-            count: usize,
-            offset: i64,
-        ) -> io::Result<isize> {
-            #[cfg(not(target_os = "linux"))]
-            use libc::pread as pread64;
-            #[cfg(target_os = "linux")]
-            use libc::pread64;
-            cvt(pread64(fd, buf, count, offset))
-        }
+        #[cfg(not(any(target_os = "linux", target_os = "android")))]
+        use libc::pread as pread64;
+        #[cfg(any(target_os = "linux", target_os = "android"))]
+        use libc::pread64;
 
         unsafe {
-            cvt_pread64(
+            cvt(pread64(
                 self.as_raw_fd(),
                 buf.as_mut_ptr() as *mut c_void,
                 cmp::min(buf.len(), READ_LIMIT),
                 offset as i64,
-            )
+            ))
             .map(|n| n as usize)
         }
     }
@@ -161,30 +149,18 @@ impl FileDesc {
     }
 
     pub fn write_at(&self, buf: &[u8], offset: u64) -> io::Result<usize> {
-        #[cfg(target_os = "android")]
-        use super::android::cvt_pwrite64;
-
-        #[cfg(not(target_os = "android"))]
-        unsafe fn cvt_pwrite64(
-            fd: c_int,
-            buf: *const c_void,
-            count: usize,
-            offset: i64,
-        ) -> io::Result<isize> {
-            #[cfg(not(target_os = "linux"))]
-            use libc::pwrite as pwrite64;
-            #[cfg(target_os = "linux")]
-            use libc::pwrite64;
-            cvt(pwrite64(fd, buf, count, offset))
-        }
+        #[cfg(not(any(target_os = "linux", target_os = "android")))]
+        use libc::pwrite as pwrite64;
+        #[cfg(any(target_os = "linux", target_os = "android"))]
+        use libc::pwrite64;
 
         unsafe {
-            cvt_pwrite64(
+            cvt(pwrite64(
                 self.as_raw_fd(),
                 buf.as_ptr() as *const c_void,
                 cmp::min(buf.len(), READ_LIMIT),
                 offset as i64,
-            )
+            ))
             .map(|n| n as usize)
         }
     }

library/std/src/sys/unix/fs.rs

@@ -46,8 +46,8 @@ use libc::fstatat64;
 use libc::readdir_r as readdir64_r;
 #[cfg(target_os = "android")]
 use libc::{
-    dirent as dirent64, fstat as fstat64, fstatat as fstatat64, lseek64, lstat as lstat64,
-    open as open64, stat as stat64,
+    dirent as dirent64, fstat as fstat64, fstatat as fstatat64, ftruncate64, lseek64,
+    lstat as lstat64, off64_t, open as open64, stat as stat64,
 };
 #[cfg(not(any(
     target_os = "linux",
@@ -835,16 +835,10 @@ impl File {
     }
 
     pub fn truncate(&self, size: u64) -> io::Result<()> {
-        #[cfg(target_os = "android")]
-        return crate::sys::android::ftruncate64(self.as_raw_fd(), size);
-
-        #[cfg(not(target_os = "android"))]
-        {
-            use crate::convert::TryInto;
-            let size: off64_t =
-                size.try_into().map_err(|e| io::Error::new(io::ErrorKind::InvalidInput, e))?;
-            cvt_r(|| unsafe { ftruncate64(self.as_raw_fd(), size) }).map(drop)
-        }
+        use crate::convert::TryInto;
+        let size: off64_t =
+            size.try_into().map_err(|e| io::Error::new(io::ErrorKind::InvalidInput, e))?;
+        cvt_r(|| unsafe { ftruncate64(self.as_raw_fd(), size) }).map(drop)
     }
 
     pub fn read(&self, buf: &mut [u8]) -> io::Result<usize> {
@@ -1154,7 +1148,7 @@ pub fn link(original: &Path, link: &Path) -> io::Result<()> {
         } 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
+            // to check. `linkat` is preferable to `link` because 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);

library/std/src/sys/unix/kernel_copy.rs

@@ -614,6 +614,9 @@ fn sendfile_splice(mode: SpliceMode, reader: RawFd, writer: RawFd, len: u64) ->
     static HAS_SENDFILE: AtomicBool = AtomicBool::new(true);
     static HAS_SPLICE: AtomicBool = AtomicBool::new(true);
 
+    // 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! {
         fn splice(
             srcfd: libc::c_int,
@@ -625,6 +628,9 @@ fn sendfile_splice(mode: SpliceMode, reader: RawFd, writer: RawFd, len: u64) ->
         ) -> libc::ssize_t
     }
 
+    #[cfg(target_os = "linux")]
+    use libc::splice;
+
     match mode {
         SpliceMode::Sendfile if !HAS_SENDFILE.load(Ordering::Relaxed) => {
             return CopyResult::Fallback(0);

library/std/src/sys/unix/net.rs

@@ -501,7 +501,7 @@ impl FromRawFd for Socket {
 // res_init unconditionally, we call it only when we detect we're linking
 // against glibc version < 2.26. (That is, when we both know its needed and
 // believe it's thread-safe).
-#[cfg(all(target_env = "gnu", not(target_os = "vxworks")))]
+#[cfg(all(target_os = "linux", target_env = "gnu"))]
 fn on_resolver_failure() {
     use crate::sys;
 
@@ -513,5 +513,5 @@ fn on_resolver_failure() {
     }
 }
 
-#[cfg(any(not(target_env = "gnu"), target_os = "vxworks"))]
+#[cfg(not(all(target_os = "linux", target_env = "gnu")))]
 fn on_resolver_failure() {}

library/std/src/sys/unix/os.rs

@@ -2,7 +2,7 @@
 
 #![allow(unused_imports)] // lots of cfg code here
 
-#[cfg(all(test, target_env = "gnu"))]
+#[cfg(test)]
 mod tests;
 
 use crate::os::unix::prelude::*;
@@ -636,30 +636,22 @@ pub fn getppid() -> u32 {
     unsafe { libc::getppid() as u32 }
 }
 
-#[cfg(all(target_env = "gnu", not(target_os = "vxworks")))]
+#[cfg(all(target_os = "linux", target_env = "gnu"))]
 pub fn glibc_version() -> Option<(usize, usize)> {
-    if let Some(Ok(version_str)) = glibc_version_cstr().map(CStr::to_str) {
-        parse_glibc_version(version_str)
-    } else {
-        None
-    }
-}
-
-#[cfg(all(target_env = "gnu", not(target_os = "vxworks")))]
-fn glibc_version_cstr() -> Option<&'static CStr> {
-    weak! {
-        fn gnu_get_libc_version() -> *const libc::c_char
+    extern "C" {
+        fn gnu_get_libc_version() -> *const libc::c_char;
     }
-    if let Some(f) = gnu_get_libc_version.get() {
-        unsafe { Some(CStr::from_ptr(f())) }
+    let version_cstr = unsafe { CStr::from_ptr(gnu_get_libc_version()) };
+    if let Ok(version_str) = version_cstr.to_str() {
+        parse_glibc_version(version_str)
     } else {
         None
     }
 }
 
 // Returns Some((major, minor)) if the string is a valid "x.y" version,
 // ignoring any extra dot-separated parts. Otherwise return None.
-#[cfg(all(target_env = "gnu", not(target_os = "vxworks")))]
+#[cfg(all(target_os = "linux", target_env = "gnu"))]
 fn parse_glibc_version(version: &str) -> Option<(usize, usize)> {
     let mut parsed_ints = version.split('.').map(str::parse::<usize>).fuse();
     match (parsed_ints.next(), parsed_ints.next()) {

library/std/src/sys/unix/os/tests.rs

@@ -1,14 +1,12 @@
-use super::*;
-
 #[test]
-#[cfg(not(target_os = "vxworks"))]
+#[cfg(all(target_os = "linux", target_env = "gnu"))]
 fn test_glibc_version() {
     // This mostly just tests that the weak linkage doesn't panic wildly...
-    glibc_version();
+    super::glibc_version();
 }
 
 #[test]
-#[cfg(not(target_os = "vxworks"))]
+#[cfg(all(target_os = "linux", target_env = "gnu"))]
 fn test_parse_glibc_version() {
     let cases = [
         ("0.0", Some((0, 0))),
@@ -20,6 +18,6 @@ fn test_parse_glibc_version() {
         ("foo.1", None),
     ];
     for &(version_str, parsed) in cases.iter() {
-        assert_eq!(parsed, parse_glibc_version(version_str));
+        assert_eq!(parsed, super::parse_glibc_version(version_str));
     }
 }

library/std/src/sys/unix/process/process_unix.rs

@@ -13,7 +13,7 @@ use crate::sys::process::process_common::*;
 use crate::os::linux::process::PidFd;
 
 #[cfg(target_os = "linux")]
-use crate::sys::weak::syscall;
+use crate::sys::weak::raw_syscall;
 
 #[cfg(any(
     target_os = "macos",
@@ -162,7 +162,7 @@ impl Command {
             cgroup: u64,
         }
 
-        syscall! {
+        raw_syscall! {
             fn clone3(cl_args: *mut clone_args, len: libc::size_t) -> libc::c_long
         }
 

library/std/src/sys/unix/thread.rs

@@ -7,7 +7,9 @@ use crate::ptr;
 use crate::sys::{os, stack_overflow};
 use crate::time::Duration;
 
-#[cfg(any(target_os = "linux", target_os = "solaris", target_os = "illumos"))]
+#[cfg(all(target_os = "linux", target_env = "gnu"))]
+use crate::sys::weak::dlsym;
+#[cfg(any(target_os = "solaris", target_os = "illumos"))]
 use crate::sys::weak::weak;
 #[cfg(not(any(target_os = "l4re", target_os = "vxworks", target_os = "espidf")))]
 pub const DEFAULT_MIN_STACK_SIZE: usize = 2 * 1024 * 1024;
@@ -627,20 +629,21 @@ pub mod guard {
 // We need that information to avoid blowing up when a small stack
 // is created in an application with big thread-local storage requirements.
 // See #6233 for rationale and details.
-#[cfg(target_os = "linux")]
-#[allow(deprecated)]
+#[cfg(all(target_os = "linux", target_env = "gnu"))]
 fn min_stack_size(attr: *const libc::pthread_attr_t) -> usize {
-    weak!(fn __pthread_get_minstack(*const libc::pthread_attr_t) -> libc::size_t);
+    // 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);
 
     match __pthread_get_minstack.get() {
         None => libc::PTHREAD_STACK_MIN,
         Some(f) => unsafe { f(attr) },
     }
 }
 
-// No point in looking up __pthread_get_minstack() on non-glibc
-// platforms.
-#[cfg(all(not(target_os = "linux"), not(target_os = "netbsd")))]
+// No point in looking up __pthread_get_minstack() on non-glibc platforms.
+#[cfg(all(not(all(target_os = "linux", target_env = "gnu")), not(target_os = "netbsd")))]
 fn min_stack_size(_: *const libc::pthread_attr_t) -> usize {
     libc::PTHREAD_STACK_MIN
 }

library/std/src/sys/unix/weak.rs

@@ -6,47 +6,92 @@
 //! detection.
 //!
 //! One option to use here is weak linkage, but that is unfortunately only
-//! really workable on Linux. Hence, use dlsym to get the symbol value at
+//! really workable with ELF. Otherwise, use dlsym to get the symbol value at
 //! runtime. This is also done for compatibility with older versions of glibc,
 //! and to avoid creating dependencies on GLIBC_PRIVATE symbols. It assumes that
 //! we've been dynamically linked to the library the symbol comes from, but that
 //! is currently always the case for things like libpthread/libc.
 //!
 //! A long time ago this used weak linkage for the __pthread_get_minstack
 //! symbol, but that caused Debian to detect an unnecessarily strict versioned
-//! dependency on libc6 (#23628).
+//! dependency on libc6 (#23628) because it is GLIBC_PRIVATE. We now use `dlsym`
+//! for a runtime lookup of that symbol to avoid the ELF versioned dependency.
 
 // There are a variety of `#[cfg]`s controlling which targets are involved in
 // each instance of `weak!` and `syscall!`. Rather than trying to unify all of
 // that, we'll just allow that some unix targets don't use this module at all.
 #![allow(dead_code, unused_macros)]
 
 use crate::ffi::CStr;
-use crate::marker;
+use crate::marker::PhantomData;
 use crate::mem;
 use crate::sync::atomic::{self, AtomicUsize, Ordering};
 
+// We can use true weak linkage on ELF targets.
+#[cfg(not(any(target_os = "macos", target_os = "ios")))]
 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'));
+        let ref $name: ExternWeak<unsafe extern "C" fn($($t),*) -> $ret> = {
+            extern "C" {
+                #[linkage = "extern_weak"]
+                static $name: *const libc::c_void;
+            }
+            #[allow(unused_unsafe)]
+            ExternWeak::new(unsafe { $name })
+        };
     )
 }
 
-pub struct Weak<F> {
+// On non-ELF targets, use the dlsym approximation of weak linkage.
+#[cfg(any(target_os = "macos", target_os = "ios"))]
+pub(crate) use self::dlsym as weak;
+
+pub(crate) struct ExternWeak<F> {
+    weak_ptr: *const libc::c_void,
+    _marker: PhantomData<F>,
+}
+
+impl<F> ExternWeak<F> {
+    #[inline]
+    pub(crate) fn new(weak_ptr: *const libc::c_void) -> Self {
+        ExternWeak { weak_ptr, _marker: PhantomData }
+    }
+}
+
+impl<F> ExternWeak<F> {
+    #[inline]
+    pub(crate) fn get(&self) -> Option<F> {
+        unsafe {
+            if self.weak_ptr.is_null() {
+                None
+            } else {
+                Some(mem::transmute_copy::<*const libc::c_void, F>(&self.weak_ptr))
+            }
+        }
+    }
+}
+
+pub(crate) macro dlsym {
+    (fn $name:ident($($t:ty),*) -> $ret:ty) => (
+        static DLSYM: DlsymWeak<unsafe extern "C" fn($($t),*) -> $ret> =
+            DlsymWeak::new(concat!(stringify!($name), '\0'));
+        let $name = &DLSYM;
+    )
+}
+
+pub(crate) struct DlsymWeak<F> {
     name: &'static str,
     addr: AtomicUsize,
-    _marker: marker::PhantomData<F>,
+    _marker: PhantomData<F>,
 }
 
-impl<F> Weak<F> {
-    pub const fn new(name: &'static str) -> Weak<F> {
-        Weak { name, addr: AtomicUsize::new(1), _marker: marker::PhantomData }
+impl<F> DlsymWeak<F> {
+    pub(crate) const fn new(name: &'static str) -> Self {
+        DlsymWeak { name, addr: AtomicUsize::new(1), _marker: PhantomData }
     }
 
-    pub fn get(&self) -> Option<F> {
-        assert_eq!(mem::size_of::<F>(), mem::size_of::<usize>());
+    #[inline]
+    pub(crate) fn get(&self) -> Option<F> {
         unsafe {
             // Relaxed is fine here because we fence before reading through the
             // pointer (see the comment below).
@@ -82,6 +127,8 @@ impl<F> Weak<F> {
     // Cold because it should only happen during first-time initalization.
     #[cold]
     unsafe fn initialize(&self) -> Option<F> {
+        assert_eq!(mem::size_of::<F>(), mem::size_of::<usize>());
+
         let val = fetch(self.name);
         // This synchronizes with the acquire fence in `get`.
         self.addr.store(val, Ordering::Release);
@@ -105,14 +152,12 @@ unsafe fn fetch(name: &str) -> usize {
 pub(crate) macro syscall {
     (fn $name:ident($($arg_name:ident: $t:ty),*) -> $ret:ty) => (
         unsafe fn $name($($arg_name: $t),*) -> $ret {
-            use super::os;
-
             weak! { fn $name($($t),*) -> $ret }
 
             if let Some(fun) = $name.get() {
                 fun($($arg_name),*)
             } else {
-                os::set_errno(libc::ENOSYS);
+                super::os::set_errno(libc::ENOSYS);
                 -1
             }
         }
@@ -123,18 +168,17 @@ pub(crate) macro 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::*;
-
             weak! { fn $name($($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),*)
             } else {
+                // This looks like a hack, but concat_idents only accepts idents
+                // (not paths).
+                use libc::*;
+
                 syscall(
                     concat_idents!(SYS_, $name),
                     $($arg_name),*
@@ -143,3 +187,19 @@ 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 {
+            // This looks like a hack, but concat_idents only accepts idents
+            // (not paths).
+            use libc::*;
+
+            syscall(
+                concat_idents!(SYS_, $name),
+                $($arg_name),*
+            ) as $ret
+        }
+    )
+}