diff options
Diffstat (limited to 'library/std/src')
| -rw-r--r-- | library/std/src/backtrace.rs | 6 | ||||
| -rw-r--r-- | library/std/src/fs.rs | 67 | ||||
| -rw-r--r-- | library/std/src/os/wasi/io/mod.rs | 4 | ||||
| -rw-r--r-- | library/std/src/process.rs | 12 | ||||
| -rw-r--r-- | library/std/src/rt.rs | 2 | ||||
| -rw-r--r-- | library/std/src/sys/hermit/args.rs | 74 | ||||
| -rw-r--r-- | library/std/src/sys/hermit/mod.rs | 4 | ||||
| -rw-r--r-- | library/std/src/sys/solid/fs.rs | 20 | ||||
| -rw-r--r-- | library/std/src/sys/unix/fs.rs | 95 | ||||
| -rw-r--r-- | library/std/src/sys/unix/mod.rs | 38 | ||||
| -rw-r--r-- | library/std/src/sys/unix/process/process_common.rs | 2 | ||||
| -rw-r--r-- | library/std/src/sys/unix/process/process_common/tests.rs | 79 | ||||
| -rw-r--r-- | library/std/src/sys/unix/process/process_unix.rs | 72 | ||||
| -rw-r--r-- | library/std/src/sys/windows/c.rs | 3 | ||||
| -rw-r--r-- | library/std/src/sys/windows/io.rs | 11 | ||||
| -rw-r--r-- | library/std/src/sys/windows/process.rs | 6 | ||||
| -rw-r--r-- | library/std/src/sys_common/backtrace.rs | 9 | ||||
| -rw-r--r-- | library/std/src/sys_common/mutex.rs | 44 | ||||
| -rw-r--r-- | library/std/src/thread/mod.rs | 27 | ||||
| -rw-r--r-- | library/std/src/time.rs | 2 |
20 files changed, 343 insertions, 234 deletions
diff --git a/library/std/src/backtrace.rs b/library/std/src/backtrace.rs index 34b57c37635..9cb74f951dd 100644 --- a/library/std/src/backtrace.rs +++ b/library/std/src/backtrace.rs @@ -325,8 +325,7 @@ impl Backtrace { // Capture a backtrace which start just before the function addressed by // `ip` fn create(ip: usize) -> Backtrace { - // SAFETY: We don't attempt to lock this reentrantly. - let _lock = unsafe { lock() }; + let _lock = lock(); let mut frames = Vec::new(); let mut actual_start = None; unsafe { @@ -469,8 +468,7 @@ impl Capture { // Use the global backtrace lock to synchronize this as it's a // requirement of the `backtrace` crate, and then actually resolve // everything. - // SAFETY: We don't attempt to lock this reentrantly. - let _lock = unsafe { lock() }; + let _lock = lock(); for frame in self.frames.iter_mut() { let symbols = &mut frame.symbols; let frame = match &frame.frame { diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index c6c78dc3939..188ff00e1f8 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -1365,6 +1365,34 @@ impl FileTimes { impl Permissions { /// Returns `true` if these permissions describe a readonly (unwritable) file. /// + /// # Note + /// + /// This function does not take Access Control Lists (ACLs) or Unix group + /// membership into account. + /// + /// # Windows + /// + /// On Windows this returns [`FILE_ATTRIBUTE_READONLY`](https://docs.microsoft.com/en-us/windows/win32/fileio/file-attribute-constants). + /// If `FILE_ATTRIBUTE_READONLY` is set then writes to the file will fail + /// but the user may still have permission to change this flag. If + /// `FILE_ATTRIBUTE_READONLY` is *not* set then writes may still fail due + /// to lack of write permission. + /// The behavior of this attribute for directories depends on the Windows + /// version. + /// + /// # Unix (including macOS) + /// + /// On Unix-based platforms this checks if *any* of the owner, group or others + /// write permission bits are set. It does not check if the current + /// user is in the file's assigned group. It also does not check ACLs. + /// Therefore even if this returns true you may not be able to write to the + /// file, and vice versa. The [`PermissionsExt`] trait gives direct access + /// to the permission bits but also does not read ACLs. If you need to + /// accurately know whether or not a file is writable use the `access()` + /// function from libc. + /// + /// [`PermissionsExt`]: crate::os::unix::fs::PermissionsExt + /// /// # Examples /// /// ```no_run @@ -1390,8 +1418,40 @@ impl Permissions { /// using the resulting `Permission` will update file permissions to allow /// writing. /// - /// This operation does **not** modify the filesystem. To modify the - /// filesystem use the [`set_permissions`] function. + /// This operation does **not** modify the files attributes. This only + /// changes the in-memory value of these attributes for this `Permissions` + /// instance. To modify the files attributes use the [`set_permissions`] + /// function which commits these attribute changes to the file. + /// + /// # Note + /// + /// `set_readonly(false)` makes the file *world-writable* on Unix. + /// You can use the [`PermissionsExt`] trait on Unix to avoid this issue. + /// + /// It also does not take Access Control Lists (ACLs) or Unix group + /// membership into account. + /// + /// # Windows + /// + /// On Windows this sets or clears [`FILE_ATTRIBUTE_READONLY`](https://docs.microsoft.com/en-us/windows/win32/fileio/file-attribute-constants). + /// If `FILE_ATTRIBUTE_READONLY` is set then writes to the file will fail + /// but the user may still have permission to change this flag. If + /// `FILE_ATTRIBUTE_READONLY` is *not* set then the write may still fail if + /// the user does not have permission to write to the file. + /// + /// In Windows 7 and earlier this attribute prevents deleting empty + /// directories. It does not prevent modifying the directory contents. + /// On later versions of Windows this attribute is ignored for directories. + /// + /// # Unix (including macOS) + /// + /// On Unix-based platforms this sets or clears the write access bit for + /// the owner, group *and* others, equivalent to `chmod a+w <file>` + /// or `chmod a-w <file>` respectively. The latter will grant write access + /// to all users! You can use the [`PermissionsExt`] trait on Unix + /// to avoid this issue. + /// + /// [`PermissionsExt`]: crate::os::unix::fs::PermissionsExt /// /// # Examples /// @@ -1405,7 +1465,8 @@ impl Permissions { /// /// permissions.set_readonly(true); /// - /// // filesystem doesn't change + /// // filesystem doesn't change, only the in memory state of the + /// // readonly permission /// assert_eq!(false, metadata.permissions().readonly()); /// /// // just this particular `permissions`. diff --git a/library/std/src/os/wasi/io/mod.rs b/library/std/src/os/wasi/io/mod.rs index d528590d75b..57bd842a50c 100644 --- a/library/std/src/os/wasi/io/mod.rs +++ b/library/std/src/os/wasi/io/mod.rs @@ -1,6 +1,6 @@ //! WASI-specific extensions to general I/O primitives. -#![unstable(feature = "wasi_ext", issue = "71213")] +#![stable(feature = "io_safety", since = "1.63.0")] -#[unstable(feature = "wasi_ext", issue = "71213")] +#[stable(feature = "io_safety", since = "1.63.0")] pub use crate::os::fd::*; diff --git a/library/std/src/process.rs b/library/std/src/process.rs index b8249a027ad..400d25beb26 100644 --- a/library/std/src/process.rs +++ b/library/std/src/process.rs @@ -2154,8 +2154,16 @@ pub fn id() -> u32 { #[cfg_attr(not(test), lang = "termination")] #[stable(feature = "termination_trait_lib", since = "1.61.0")] #[rustc_on_unimplemented( - message = "`main` has invalid return type `{Self}`", - label = "`main` can only return types that implement `{Termination}`" + on( + all(not(bootstrap), cause = "MainFunctionType"), + message = "`main` has invalid return type `{Self}`", + label = "`main` can only return types that implement `{Termination}`" + ), + on( + bootstrap, + message = "`main` has invalid return type `{Self}`", + label = "`main` can only return types that implement `{Termination}`" + ) )] pub trait Termination { /// Is called to get the representation of the value as status code. diff --git a/library/std/src/rt.rs b/library/std/src/rt.rs index b8bcdbece0a..9c2f0c1dd3e 100644 --- a/library/std/src/rt.rs +++ b/library/std/src/rt.rs @@ -89,7 +89,7 @@ macro_rules! rtunwrap { // `src/tools/tidy/src/pal.rs` for more info. On all other platforms, `sigpipe` // has a value, but its value is ignored. // -// Even though it is an `u8`, it only ever has 3 values. These are documented in +// Even though it is an `u8`, it only ever has 4 values. These are documented in // `compiler/rustc_session/src/config/sigpipe.rs`. #[cfg_attr(test, allow(dead_code))] unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) { diff --git a/library/std/src/sys/hermit/args.rs b/library/std/src/sys/hermit/args.rs index 1c7e1dd8d57..afcae6c90ee 100644 --- a/library/std/src/sys/hermit/args.rs +++ b/library/std/src/sys/hermit/args.rs @@ -1,20 +1,37 @@ -use crate::ffi::OsString; +use crate::ffi::{c_char, CStr, OsString}; use crate::fmt; +use crate::os::unix::ffi::OsStringExt; +use crate::ptr; +use crate::sync::atomic::{ + AtomicIsize, AtomicPtr, + Ordering::{Acquire, Relaxed, Release}, +}; use crate::vec; +static ARGC: AtomicIsize = AtomicIsize::new(0); +static ARGV: AtomicPtr<*const u8> = AtomicPtr::new(ptr::null_mut()); + /// One-time global initialization. pub unsafe fn init(argc: isize, argv: *const *const u8) { - imp::init(argc, argv) -} - -/// One-time global cleanup. -pub unsafe fn cleanup() { - imp::cleanup() + ARGC.store(argc, Relaxed); + // Use release ordering here to broadcast writes by the OS. + ARGV.store(argv as *mut *const u8, Release); } /// Returns the command line arguments pub fn args() -> Args { - imp::args() + // Synchronize with the store above. + let argv = ARGV.load(Acquire); + // If argv has not been initialized yet, do not return any arguments. + let argc = if argv.is_null() { 0 } else { ARGC.load(Relaxed) }; + let args: Vec<OsString> = (0..argc) + .map(|i| unsafe { + let cstr = CStr::from_ptr(*argv.offset(i) as *const c_char); + OsStringExt::from_vec(cstr.to_bytes().to_vec()) + }) + .collect(); + + Args { iter: args.into_iter() } } pub struct Args { @@ -51,44 +68,3 @@ impl DoubleEndedIterator for Args { self.iter.next_back() } } - -mod imp { - use super::Args; - use crate::ffi::{CStr, OsString}; - use crate::os::unix::ffi::OsStringExt; - use crate::ptr; - - use crate::sys_common::mutex::StaticMutex; - - static mut ARGC: isize = 0; - static mut ARGV: *const *const u8 = ptr::null(); - static LOCK: StaticMutex = StaticMutex::new(); - - pub unsafe fn init(argc: isize, argv: *const *const u8) { - let _guard = LOCK.lock(); - ARGC = argc; - ARGV = argv; - } - - pub unsafe fn cleanup() { - let _guard = LOCK.lock(); - ARGC = 0; - ARGV = ptr::null(); - } - - pub fn args() -> Args { - Args { iter: clone().into_iter() } - } - - fn clone() -> Vec<OsString> { - unsafe { - let _guard = LOCK.lock(); - (0..ARGC) - .map(|i| { - let cstr = CStr::from_ptr(*ARGV.offset(i) as *const i8); - OsStringExt::from_vec(cstr.to_bytes().to_vec()) - }) - .collect() - } - } -} diff --git a/library/std/src/sys/hermit/mod.rs b/library/std/src/sys/hermit/mod.rs index 827d82900ea..e6534df8938 100644 --- a/library/std/src/sys/hermit/mod.rs +++ b/library/std/src/sys/hermit/mod.rs @@ -106,9 +106,7 @@ pub unsafe fn init(argc: isize, argv: *const *const u8, _sigpipe: u8) { // SAFETY: must be called only once during runtime cleanup. // NOTE: this is not guaranteed to run, for example when the program aborts. -pub unsafe fn cleanup() { - args::cleanup(); -} +pub unsafe fn cleanup() {} #[cfg(not(test))] #[no_mangle] diff --git a/library/std/src/sys/solid/fs.rs b/library/std/src/sys/solid/fs.rs index 9692222534e..6c66b93a3e1 100644 --- a/library/std/src/sys/solid/fs.rs +++ b/library/std/src/sys/solid/fs.rs @@ -175,15 +175,19 @@ impl Iterator for ReadDir { type Item = io::Result<DirEntry>; fn next(&mut self) -> Option<io::Result<DirEntry>> { - unsafe { - let mut out_dirent = MaybeUninit::uninit(); - error::SolidError::err_if_negative(abi::SOLID_FS_ReadDir( + let entry = unsafe { + let mut out_entry = MaybeUninit::uninit(); + match error::SolidError::err_if_negative(abi::SOLID_FS_ReadDir( self.inner.dirp, - out_dirent.as_mut_ptr(), - )) - .ok()?; - Some(Ok(DirEntry { entry: out_dirent.assume_init(), inner: Arc::clone(&self.inner) })) - } + out_entry.as_mut_ptr(), + )) { + Ok(_) => out_entry.assume_init(), + Err(e) if e.as_raw() == abi::SOLID_ERR_NOTFOUND => return None, + Err(e) => return Some(Err(e.as_io_error())), + } + }; + + (entry.d_name[0] != 0).then(|| Ok(DirEntry { entry, inner: Arc::clone(&self.inner) })) } } diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 3ac01e6a27d..e22b2f3340a 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -4,6 +4,15 @@ use crate::ffi::{CStr, OsStr, OsString}; use crate::fmt; use crate::io::{self, BorrowedCursor, Error, IoSlice, IoSliceMut, SeekFrom}; use crate::mem; +#[cfg(any( + target_os = "android", + target_os = "linux", + target_os = "solaris", + target_os = "fuchsia", + target_os = "redox", + target_os = "illumos" +))] +use crate::mem::MaybeUninit; use crate::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd}; use crate::path::{Path, PathBuf}; use crate::ptr; @@ -584,33 +593,69 @@ impl Iterator for ReadDir { }; } - // Only d_reclen bytes of *entry_ptr are valid, so we can't just copy the - // whole thing (#93384). Instead, copy everything except the name. - let mut copy: dirent64 = mem::zeroed(); - // Can't dereference entry_ptr, so use the local entry to get - // offsetof(struct dirent, d_name) - let copy_bytes = &mut copy as *mut _ as *mut u8; - let copy_name = &mut copy.d_name as *mut _ as *mut u8; - let name_offset = copy_name.offset_from(copy_bytes) as usize; - let entry_bytes = entry_ptr as *const u8; - let entry_name = entry_bytes.add(name_offset); - ptr::copy_nonoverlapping(entry_bytes, copy_bytes, name_offset); + // The dirent64 struct is a weird imaginary thing that isn't ever supposed + // to be worked with by value. Its trailing d_name field is declared + // variously as [c_char; 256] or [c_char; 1] on different systems but + // either way that size is meaningless; only the offset of d_name is + // meaningful. The dirent64 pointers that libc returns from readdir64 are + // allowed to point to allocations smaller _or_ LARGER than implied by the + // definition of the struct. + // + // As such, we need to be even more careful with dirent64 than if its + // contents were "simply" partially initialized data. + // + // Like for uninitialized contents, converting entry_ptr to `&dirent64` + // would not be legal. However, unique to dirent64 is that we don't even + // get to use `addr_of!((*entry_ptr).d_name)` because that operation + // requires the full extent of *entry_ptr to be in bounds of the same + // allocation, which is not necessarily the case here. + // + // Absent any other way to obtain a pointer to `(*entry_ptr).d_name` + // legally in Rust analogously to how it would be done in C, we instead + // need to make our own non-libc allocation that conforms to the weird + // imaginary definition of dirent64, and use that for a field offset + // computation. + macro_rules! offset_ptr { + ($entry_ptr:expr, $field:ident) => {{ + const OFFSET: isize = { + let delusion = MaybeUninit::<dirent64>::uninit(); + let entry_ptr = delusion.as_ptr(); + unsafe { + ptr::addr_of!((*entry_ptr).$field) + .cast::<u8>() + .offset_from(entry_ptr.cast::<u8>()) + } + }; + if true { + // Cast to the same type determined by the else branch. + $entry_ptr.byte_offset(OFFSET).cast::<_>() + } else { + #[allow(deref_nullptr)] + { + ptr::addr_of!((*ptr::null::<dirent64>()).$field) + } + } + }}; + } + + // d_name is guaranteed to be null-terminated. + let name = CStr::from_ptr(offset_ptr!(entry_ptr, d_name).cast()); + let name_bytes = name.to_bytes(); + if name_bytes == b"." || name_bytes == b".." { + continue; + } let entry = dirent64_min { - d_ino: copy.d_ino as u64, + d_ino: *offset_ptr!(entry_ptr, d_ino) as u64, #[cfg(not(any(target_os = "solaris", target_os = "illumos")))] - d_type: copy.d_type as u8, + d_type: *offset_ptr!(entry_ptr, d_type) as u8, }; - let ret = DirEntry { + return Some(Ok(DirEntry { entry, - // d_name is guaranteed to be null-terminated. - name: CStr::from_ptr(entry_name as *const _).to_owned(), + name: name.to_owned(), dir: Arc::clone(&self.inner), - }; - if ret.name_bytes() != b"." && ret.name_bytes() != b".." { - return Some(Ok(ret)); - } + })); } } } @@ -674,7 +719,10 @@ impl DirEntry { self.file_name_os_str().to_os_string() } - #[cfg(any(target_os = "linux", target_os = "emscripten", target_os = "android"))] + #[cfg(all( + any(target_os = "linux", target_os = "emscripten", target_os = "android"), + not(miri) + ))] pub fn metadata(&self) -> io::Result<FileAttr> { let fd = cvt(unsafe { dirfd(self.dir.dirp.0) })?; let name = self.name_cstr().as_ptr(); @@ -695,7 +743,10 @@ impl DirEntry { Ok(FileAttr::from_stat64(stat)) } - #[cfg(not(any(target_os = "linux", target_os = "emscripten", target_os = "android")))] + #[cfg(any( + not(any(target_os = "linux", target_os = "emscripten", target_os = "android")), + miri + ))] pub fn metadata(&self) -> io::Result<FileAttr> { lstat(&self.path()) } diff --git a/library/std/src/sys/unix/mod.rs b/library/std/src/sys/unix/mod.rs index c84e292eac1..4a9f356b63c 100644 --- a/library/std/src/sys/unix/mod.rs +++ b/library/std/src/sys/unix/mod.rs @@ -163,17 +163,27 @@ pub unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) { // See the other file for docs. NOTE: Make sure to keep them in // sync! mod sigpipe { + pub const DEFAULT: u8 = 0; pub const INHERIT: u8 = 1; pub const SIG_IGN: u8 = 2; pub const SIG_DFL: u8 = 3; } - let handler = match sigpipe { - sigpipe::INHERIT => None, - sigpipe::SIG_IGN => Some(libc::SIG_IGN), - sigpipe::SIG_DFL => Some(libc::SIG_DFL), + let (sigpipe_attr_specified, handler) = match sigpipe { + sigpipe::DEFAULT => (false, Some(libc::SIG_IGN)), + sigpipe::INHERIT => (true, None), + sigpipe::SIG_IGN => (true, Some(libc::SIG_IGN)), + sigpipe::SIG_DFL => (true, Some(libc::SIG_DFL)), _ => unreachable!(), }; + // The bootstrap compiler doesn't know about sigpipe::DEFAULT, and always passes in + // SIG_IGN. This causes some tests to fail because they expect SIGPIPE to be reset to + // default on process spawning (which doesn't happen if #[unix_sigpipe] is specified). + // Since we can't differentiate between the cases here, treat SIG_IGN as DEFAULT + // unconditionally. + if sigpipe_attr_specified && !(cfg!(bootstrap) && sigpipe == sigpipe::SIG_IGN) { + UNIX_SIGPIPE_ATTR_SPECIFIED.store(true, crate::sync::atomic::Ordering::Relaxed); + } if let Some(handler) = handler { rtassert!(signal(libc::SIGPIPE, handler) != libc::SIG_ERR); } @@ -181,6 +191,26 @@ pub unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) { } } +// This is set (up to once) in reset_sigpipe. +#[cfg(not(any( + target_os = "espidf", + target_os = "emscripten", + target_os = "fuchsia", + target_os = "horizon" +)))] +static UNIX_SIGPIPE_ATTR_SPECIFIED: crate::sync::atomic::AtomicBool = + crate::sync::atomic::AtomicBool::new(false); + +#[cfg(not(any( + target_os = "espidf", + target_os = "emscripten", + target_os = "fuchsia", + target_os = "horizon" +)))] +pub(crate) fn unix_sigpipe_attr_specified() -> bool { + UNIX_SIGPIPE_ATTR_SPECIFIED.load(crate::sync::atomic::Ordering::Relaxed) +} + // SAFETY: must be called only once during runtime cleanup. // NOTE: this is not guaranteed to run, for example when the program aborts. pub unsafe fn cleanup() { diff --git a/library/std/src/sys/unix/process/process_common.rs b/library/std/src/sys/unix/process/process_common.rs index 2834ee0ace8..848adca78c0 100644 --- a/library/std/src/sys/unix/process/process_common.rs +++ b/library/std/src/sys/unix/process/process_common.rs @@ -39,10 +39,12 @@ cfg_if::cfg_if! { // https://github.com/aosp-mirror/platform_bionic/blob/ad8dcd6023294b646e5a8288c0ed431b0845da49/libc/include/android/legacy_signal_inlines.h cfg_if::cfg_if! { if #[cfg(target_os = "android")] { + #[allow(dead_code)] pub unsafe fn sigemptyset(set: *mut libc::sigset_t) -> libc::c_int { set.write_bytes(0u8, 1); return 0; } + #[allow(dead_code)] pub unsafe fn sigaddset(set: *mut libc::sigset_t, signum: libc::c_int) -> libc::c_int { use crate::{ diff --git a/library/std/src/sys/unix/process/process_common/tests.rs b/library/std/src/sys/unix/process/process_common/tests.rs index d176b3401c0..03631e4e33b 100644 --- a/library/std/src/sys/unix/process/process_common/tests.rs +++ b/library/std/src/sys/unix/process/process_common/tests.rs @@ -31,41 +31,54 @@ macro_rules! t { ignore )] fn test_process_mask() { - unsafe { - // Test to make sure that a signal mask does not get inherited. - let mut cmd = Command::new(OsStr::new("cat")); - - let mut set = mem::MaybeUninit::<libc::sigset_t>::uninit(); - let mut old_set = mem::MaybeUninit::<libc::sigset_t>::uninit(); - t!(cvt(sigemptyset(set.as_mut_ptr()))); - t!(cvt(sigaddset(set.as_mut_ptr(), libc::SIGINT))); - t!(cvt_nz(libc::pthread_sigmask(libc::SIG_SETMASK, set.as_ptr(), old_set.as_mut_ptr()))); - - cmd.stdin(Stdio::MakePipe); - cmd.stdout(Stdio::MakePipe); - - let (mut cat, mut pipes) = t!(cmd.spawn(Stdio::Null, true)); - let stdin_write = pipes.stdin.take().unwrap(); - let stdout_read = pipes.stdout.take().unwrap(); - - t!(cvt_nz(libc::pthread_sigmask(libc::SIG_SETMASK, old_set.as_ptr(), ptr::null_mut()))); - - t!(cvt(libc::kill(cat.id() as libc::pid_t, libc::SIGINT))); - // We need to wait until SIGINT is definitely delivered. The - // easiest way is to write something to cat, and try to read it - // back: if SIGINT is unmasked, it'll get delivered when cat is - // next scheduled. - let _ = stdin_write.write(b"Hello"); - drop(stdin_write); - - // Either EOF or failure (EPIPE) is okay. - let mut buf = [0; 5]; - if let Ok(ret) = stdout_read.read(&mut buf) { - assert_eq!(ret, 0); + // Test to make sure that a signal mask *does* get inherited. + fn test_inner(mut cmd: Command) { + unsafe { + let mut set = mem::MaybeUninit::<libc::sigset_t>::uninit(); + let mut old_set = mem::MaybeUninit::<libc::sigset_t>::uninit(); + t!(cvt(sigemptyset(set.as_mut_ptr()))); + t!(cvt(sigaddset(set.as_mut_ptr(), libc::SIGINT))); + t!(cvt_nz(libc::pthread_sigmask( + libc::SIG_SETMASK, + set.as_ptr(), + old_set.as_mut_ptr() + ))); + + cmd.stdin(Stdio::MakePipe); + cmd.stdout(Stdio::MakePipe); + + let (mut cat, mut pipes) = t!(cmd.spawn(Stdio::Null, true)); + let stdin_write = pipes.stdin.take().unwrap(); + let stdout_read = pipes.stdout.take().unwrap(); + + t!(cvt_nz(libc::pthread_sigmask(libc::SIG_SETMASK, old_set.as_ptr(), ptr::null_mut()))); + + t!(cvt(libc::kill(cat.id() as libc::pid_t, libc::SIGINT))); + // We need to wait until SIGINT is definitely delivered. The + // easiest way is to write something to cat, and try to read it + // back: if SIGINT is unmasked, it'll get delivered when cat is + // next scheduled. + let _ = stdin_write.write(b"Hello"); + drop(stdin_write); + + // Exactly 5 bytes should be read. + let mut buf = [0; 5]; + let ret = t!(stdout_read.read(&mut buf)); + assert_eq!(ret, 5); + assert_eq!(&buf, b"Hello"); + + t!(cat.wait()); } - - t!(cat.wait()); } + + // A plain `Command::new` uses the posix_spawn path on many platforms. + let cmd = Command::new(OsStr::new("cat")); + test_inner(cmd); + + // Specifying `pre_exec` forces the fork/exec path. + let mut cmd = Command::new(OsStr::new("cat")); + unsafe { cmd.pre_exec(Box::new(|| Ok(()))) }; + test_inner(cmd); } #[test] diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 2ff8e600f7c..56a805cef73 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -2,7 +2,6 @@ use crate::fmt; use crate::io::{self, Error, ErrorKind}; use crate::mem; use crate::num::NonZeroI32; -use crate::ptr; use crate::sys; use crate::sys::cvt; use crate::sys::process::process_common::*; @@ -310,7 +309,7 @@ impl Command { //FIXME: Redox kernel does not support setgroups yet #[cfg(not(target_os = "redox"))] if libc::getuid() == 0 && self.get_groups().is_none() { - cvt(libc::setgroups(0, ptr::null()))?; + cvt(libc::setgroups(0, crate::ptr::null()))?; } cvt(libc::setuid(u as uid_t))?; } @@ -326,30 +325,26 @@ impl Command { // emscripten has no signal support. #[cfg(not(target_os = "emscripten"))] { - use crate::mem::MaybeUninit; - use crate::sys::cvt_nz; - // Reset signal handling so the child process starts in a - // standardized state. libstd ignores SIGPIPE, and signal-handling - // libraries often set a mask. Child processes inherit ignored - // signals and the signal mask from their parent, but most - // UNIX programs do not reset these things on their own, so we - // need to clean things up now to avoid confusing the program - // we're about to run. - let mut set = MaybeUninit::<libc::sigset_t>::uninit(); - cvt(sigemptyset(set.as_mut_ptr()))?; - cvt_nz(libc::pthread_sigmask(libc::SIG_SETMASK, set.as_ptr(), ptr::null_mut()))?; - - #[cfg(target_os = "android")] // see issue #88585 - { - let mut action: libc::sigaction = mem::zeroed(); - action.sa_sigaction = libc::SIG_DFL; - cvt(libc::sigaction(libc::SIGPIPE, &action, ptr::null_mut()))?; - } - #[cfg(not(target_os = "android"))] - { - let ret = sys::signal(libc::SIGPIPE, libc::SIG_DFL); - if ret == libc::SIG_ERR { - return Err(io::Error::last_os_error()); + // Inherit the signal mask from the parent rather than resetting it (i.e. do not call + // pthread_sigmask). + + // If #[unix_sigpipe] is specified, don't reset SIGPIPE to SIG_DFL. + // If #[unix_sigpipe] is not specified, reset SIGPIPE to SIG_DFL for backward compatibility. + // + // #[unix_sigpipe] is an opportunity to change the default here. + if !crate::sys::unix_sigpipe_attr_specified() { + #[cfg(target_os = "android")] // see issue #88585 + { + let mut action: libc::sigaction = mem::zeroed(); + action.sa_sigaction = libc::SIG_DFL; + cvt(libc::sigaction(libc::SIGPIPE, &action, crate::ptr::null_mut()))?; + } + #[cfg(not(target_os = "android"))] + { + let ret = sys::signal(libc::SIGPIPE, libc::SIG_DFL); + if ret == libc::SIG_ERR { + return Err(io::Error::last_os_error()); + } } } } @@ -411,7 +406,7 @@ impl Command { envp: Option<&CStringArray>, ) -> io::Result<Option<Process>> { use crate::mem::MaybeUninit; - use crate::sys::{self, cvt_nz}; + use crate::sys::{self, cvt_nz, unix_sigpipe_attr_specified}; if self.get_gid().is_some() || self.get_uid().is_some() @@ -531,13 +526,24 @@ impl Command { cvt_nz(libc::posix_spawnattr_setpgroup(attrs.0.as_mut_ptr(), pgroup))?; } - let mut set = MaybeUninit::<libc::sigset_t>::uninit(); - cvt(sigemptyset(set.as_mut_ptr()))?; - cvt_nz(libc::posix_spawnattr_setsigmask(attrs.0.as_mut_ptr(), set.as_ptr()))?; - cvt(sigaddset(set.as_mut_ptr(), libc::SIGPIPE))?; - cvt_nz(libc::posix_spawnattr_setsigdefault(attrs.0.as_mut_ptr(), set.as_ptr()))?; + // Inherit the signal mask from this process rather than resetting it (i.e. do not call + // posix_spawnattr_setsigmask). + + // If #[unix_sigpipe] is specified, don't reset SIGPIPE to SIG_DFL. + // If #[unix_sigpipe] is not specified, reset SIGPIPE to SIG_DFL for backward compatibility. + // + // #[unix_sigpipe] is an opportunity to change the default here. + if !unix_sigpipe_attr_specified() { + let mut default_set = MaybeUninit::<libc::sigset_t>::uninit(); + cvt(sigemptyset(default_set.as_mut_ptr()))?; + cvt(sigaddset(default_set.as_mut_ptr(), libc::SIGPIPE))?; + cvt_nz(libc::posix_spawnattr_setsigdefault( + attrs.0.as_mut_ptr(), + default_set.as_ptr(), + ))?; + flags |= libc::POSIX_SPAWN_SETSIGDEF; + } - flags |= libc::POSIX_SPAWN_SETSIGDEF | libc::POSIX_SPAWN_SETSIGMASK; cvt_nz(libc::posix_spawnattr_setflags(attrs.0.as_mut_ptr(), flags as _))?; // Make sure we synchronize access to the global `environ` resource diff --git a/library/std/src/sys/windows/c.rs b/library/std/src/sys/windows/c.rs index 917fc8e4995..be6fc2ebb7a 100644 --- a/library/std/src/sys/windows/c.rs +++ b/library/std/src/sys/windows/c.rs @@ -129,6 +129,8 @@ pub const FIONBIO: c_ulong = 0x8004667e; pub const MAX_PATH: usize = 260; +pub const FILE_TYPE_PIPE: u32 = 3; + #[repr(C)] #[derive(Copy)] pub struct WIN32_FIND_DATAW { @@ -1114,6 +1116,7 @@ extern "system" { lpFileInformation: LPVOID, dwBufferSize: DWORD, ) -> BOOL; + pub fn GetFileType(hfile: HANDLE) -> DWORD; pub fn SleepConditionVariableSRW( ConditionVariable: PCONDITION_VARIABLE, SRWLock: PSRWLOCK, diff --git a/library/std/src/sys/windows/io.rs b/library/std/src/sys/windows/io.rs index 0879ef19933..2cc34c986b9 100644 --- a/library/std/src/sys/windows/io.rs +++ b/library/std/src/sys/windows/io.rs @@ -120,6 +120,11 @@ unsafe fn handle_is_console(handle: BorrowedHandle<'_>) -> bool { } unsafe fn msys_tty_on(handle: c::HANDLE) -> bool { + // Early return if the handle is not a pipe. + if c::GetFileType(handle) != c::FILE_TYPE_PIPE { + return false; + } + const SIZE: usize = size_of::<c::FILE_NAME_INFO>() + c::MAX_PATH * size_of::<c::WCHAR>(); let mut name_info_bytes = Align8([0u8; SIZE]); let res = c::GetFileInformationByHandleEx( @@ -137,11 +142,13 @@ unsafe fn msys_tty_on(handle: c::HANDLE) -> bool { let name_ptr = name_info_bytes.0.as_ptr().offset(size_of::<c::DWORD>() as isize).cast::<u16>(); let s = core::slice::from_raw_parts(name_ptr, name_len); let name = String::from_utf16_lossy(s); + // Get the file name only. + let name = name.rsplit('\\').next().unwrap_or(&name); // This checks whether 'pty' exists in the file name, which indicates that // a pseudo-terminal is attached. To mitigate against false positives // (e.g., an actual file name that contains 'pty'), we also require that - // either the strings 'msys-' or 'cygwin-' are in the file name as well.) - let is_msys = name.contains("msys-") || name.contains("cygwin-"); + // the file name begins with either the strings 'msys-' or 'cygwin-'.) + let is_msys = name.starts_with("msys-") || name.starts_with("cygwin-"); let is_pty = name.contains("-pty"); is_msys && is_pty } diff --git a/library/std/src/sys/windows/process.rs b/library/std/src/sys/windows/process.rs index 02d5af4719a..9cbb4ef19e9 100644 --- a/library/std/src/sys/windows/process.rs +++ b/library/std/src/sys/windows/process.rs @@ -16,6 +16,7 @@ use crate::os::windows::ffi::{OsStrExt, OsStringExt}; use crate::os::windows::io::{AsHandle, AsRawHandle, BorrowedHandle, FromRawHandle, IntoRawHandle}; use crate::path::{Path, PathBuf}; use crate::ptr; +use crate::sync::Mutex; use crate::sys::args::{self, Arg}; use crate::sys::c; use crate::sys::c::NonZeroDWORD; @@ -25,7 +26,6 @@ use crate::sys::handle::Handle; use crate::sys::path; use crate::sys::pipe::{self, AnonPipe}; use crate::sys::stdio; -use crate::sys_common::mutex::StaticMutex; use crate::sys_common::process::{CommandEnv, CommandEnvs}; use crate::sys_common::IntoInner; @@ -301,9 +301,9 @@ impl Command { // // For more information, msdn also has an article about this race: // https://support.microsoft.com/kb/315939 - static CREATE_PROCESS_LOCK: StaticMutex = StaticMutex::new(); + static CREATE_PROCESS_LOCK: Mutex<()> = Mutex::new(()); - let _guard = unsafe { CREATE_PROCESS_LOCK.lock() }; + let _guard = CREATE_PROCESS_LOCK.lock(); let mut pipes = StdioPipes { stdin: None, stdout: None, stderr: None }; let null = Stdio::Null; diff --git a/library/std/src/sys_common/backtrace.rs b/library/std/src/sys_common/backtrace.rs index 31164afdc7b..8807077cb49 100644 --- a/library/std/src/sys_common/backtrace.rs +++ b/library/std/src/sys_common/backtrace.rs @@ -7,15 +7,14 @@ use crate::fmt; use crate::io; use crate::io::prelude::*; use crate::path::{self, Path, PathBuf}; -use crate::sys_common::mutex::StaticMutex; +use crate::sync::{Mutex, PoisonError}; /// Max number of frames to print. const MAX_NB_FRAMES: usize = 100; -// SAFETY: Don't attempt to lock this reentrantly. -pub unsafe fn lock() -> impl Drop { - static LOCK: StaticMutex = StaticMutex::new(); - LOCK.lock() +pub fn lock() -> impl Drop { + static LOCK: Mutex<()> = Mutex::new(()); + LOCK.lock().unwrap_or_else(PoisonError::into_inner) } /// Prints the current backtrace. diff --git a/library/std/src/sys_common/mutex.rs b/library/std/src/sys_common/mutex.rs index 7b9f7ef5487..98046f20f89 100644 --- a/library/std/src/sys_common/mutex.rs +++ b/library/std/src/sys_common/mutex.rs @@ -1,49 +1,5 @@ use crate::sys::locks as imp; -/// An OS-based mutual exclusion lock, meant for use in static variables. -/// -/// This mutex has a const constructor ([`StaticMutex::new`]), does not -/// implement `Drop` to cleanup resources, and causes UB when used reentrantly. -/// -/// This mutex does not implement poisoning. -/// -/// This is a wrapper around `imp::Mutex` that does *not* call `init()` and -/// `destroy()`. -pub struct StaticMutex(imp::Mutex); - -unsafe impl Sync for StaticMutex {} - -impl StaticMutex { - /// Creates a new mutex for use. - #[inline] - pub const fn new() -> Self { - Self(imp::Mutex::new()) - } - - /// Calls raw_lock() and then returns an RAII guard to guarantee the mutex - /// will be unlocked. - /// - /// It is undefined behaviour to call this function while locked by the - /// same thread. - #[inline] - pub unsafe fn lock(&'static self) -> StaticMutexGuard { - self.0.lock(); - StaticMutexGuard(&self.0) - } -} - -#[must_use] -pub struct StaticMutexGuard(&'static imp::Mutex); - -impl Drop for StaticMutexGuard { - #[inline] - fn drop(&mut self) { - unsafe { - self.0.unlock(); - } - } -} - /// An OS-based mutual exclusion lock. /// /// This mutex cleans up its resources in its `Drop` implementation, may safely diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index 5d4a236bd2c..05023df1bb2 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -1128,24 +1128,21 @@ impl ThreadId { } } } else { - use crate::sys_common::mutex::StaticMutex; + use crate::sync::{Mutex, PoisonError}; - // It is UB to attempt to acquire this mutex reentrantly! - static GUARD: StaticMutex = StaticMutex::new(); - static mut COUNTER: u64 = 0; + static COUNTER: Mutex<u64> = Mutex::new(0); - unsafe { - let guard = GUARD.lock(); + let mut counter = COUNTER.lock().unwrap_or_else(PoisonError::into_inner); + let Some(id) = counter.checked_add(1) else { + // in case the panic handler ends up calling `ThreadId::new()`, + // avoid reentrant lock acquire. + drop(counter); + exhausted(); + }; - let Some(id) = COUNTER.checked_add(1) else { - drop(guard); // in case the panic handler ends up calling `ThreadId::new()`, avoid reentrant lock acquire. - exhausted(); - }; - - COUNTER = id; - drop(guard); - ThreadId(NonZeroU64::new(id).unwrap()) - } + *counter = id; + drop(counter); + ThreadId(NonZeroU64::new(id).unwrap()) } } } diff --git a/library/std/src/time.rs b/library/std/src/time.rs index c7c413da060..34e18b5fa87 100644 --- a/library/std/src/time.rs +++ b/library/std/src/time.rs @@ -356,7 +356,7 @@ impl Instant { /// /// # Panics /// - /// Previous rust versions panicked when self was earlier than the current time. Currently this + /// Previous rust versions panicked when the current time was earlier than self. Currently this /// method returns a Duration of zero in that case. Future versions may reintroduce the panic. /// See [Monotonicity]. /// |