Skip to content

Reduce monomorphisation bloat in small_c_string #121101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 16 additions & 13 deletions library/std/src/sys/pal/common/small_c_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,28 @@ const NUL_ERR: io::Error =
io::const_io_error!(io::ErrorKind::InvalidInput, "file name contained an unexpected NUL byte");

#[inline]
pub fn run_path_with_cstr<T, F>(path: &Path, f: F) -> io::Result<T>
where
F: FnOnce(&CStr) -> io::Result<T>,
{
pub fn run_path_with_cstr<T>(path: &Path, f: &dyn Fn(&CStr) -> io::Result<T>) -> io::Result<T> {
run_with_cstr(path.as_os_str().as_encoded_bytes(), f)
}

#[inline]
pub fn run_with_cstr<T, F>(bytes: &[u8], f: F) -> io::Result<T>
where
F: FnOnce(&CStr) -> io::Result<T>,
{
pub fn run_with_cstr<T>(bytes: &[u8], f: &dyn Fn(&CStr) -> io::Result<T>) -> io::Result<T> {
// Dispatch and dyn erase the closure type to prevent mono bloat.
// See https://github.com/rust-lang/rust/pull/121101.
if bytes.len() >= MAX_STACK_ALLOCATION {
return run_with_cstr_allocating(bytes, f);
run_with_cstr_allocating(bytes, f)
} else {
unsafe { run_with_cstr_stack(bytes, f) }
}
}

/// # Safety
///
/// `bytes` must have a length less than `MAX_STACK_ALLOCATION`.
unsafe fn run_with_cstr_stack<T>(
bytes: &[u8],
f: &dyn Fn(&CStr) -> io::Result<T>,
) -> io::Result<T> {
let mut buf = MaybeUninit::<[u8; MAX_STACK_ALLOCATION]>::uninit();
let buf_ptr = buf.as_mut_ptr() as *mut u8;

Expand All @@ -47,10 +53,7 @@ where

#[cold]
#[inline(never)]
fn run_with_cstr_allocating<T, F>(bytes: &[u8], f: F) -> io::Result<T>
where
F: FnOnce(&CStr) -> io::Result<T>,
{
fn run_with_cstr_allocating<T>(bytes: &[u8], f: &dyn Fn(&CStr) -> io::Result<T>) -> io::Result<T> {
match CString::new(bytes) {
Ok(s) => f(&s),
Err(_) => Err(NUL_ERR),
Expand Down
12 changes: 6 additions & 6 deletions library/std/src/sys/pal/common/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use core::iter::repeat;
#[test]
fn stack_allocation_works() {
let path = Path::new("abc");
let result = run_path_with_cstr(path, |p| {
let result = run_path_with_cstr(path, &|p| {
assert_eq!(p, &*CString::new(path.as_os_str().as_encoded_bytes()).unwrap());
Ok(42)
});
Expand All @@ -17,14 +17,14 @@ fn stack_allocation_works() {
#[test]
fn stack_allocation_fails() {
let path = Path::new("ab\0");
assert!(run_path_with_cstr::<(), _>(path, |_| unreachable!()).is_err());
assert!(run_path_with_cstr::<()>(path, &|_| unreachable!()).is_err());
}

#[test]
fn heap_allocation_works() {
let path = repeat("a").take(384).collect::<String>();
let path = Path::new(&path);
let result = run_path_with_cstr(path, |p| {
let result = run_path_with_cstr(path, &|p| {
assert_eq!(p, &*CString::new(path.as_os_str().as_encoded_bytes()).unwrap());
Ok(42)
});
Expand All @@ -36,15 +36,15 @@ fn heap_allocation_fails() {
let mut path = repeat("a").take(384).collect::<String>();
path.push('\0');
let path = Path::new(&path);
assert!(run_path_with_cstr::<(), _>(path, |_| unreachable!()).is_err());
assert!(run_path_with_cstr::<()>(path, &|_| unreachable!()).is_err());
}

#[bench]
fn bench_stack_path_alloc(b: &mut test::Bencher) {
let path = repeat("a").take(383).collect::<String>();
let p = Path::new(&path);
b.iter(|| {
run_path_with_cstr(p, |cstr| {
run_path_with_cstr(p, &|cstr| {
black_box(cstr);
Ok(())
})
Expand All @@ -57,7 +57,7 @@ fn bench_heap_path_alloc(b: &mut test::Bencher) {
let path = repeat("a").take(384).collect::<String>();
let p = Path::new(&path);
b.iter(|| {
run_path_with_cstr(p, |cstr| {
run_path_with_cstr(p, &|cstr| {
black_box(cstr);
Ok(())
})
Expand Down
4 changes: 2 additions & 2 deletions library/std/src/sys/pal/hermit/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ impl OpenOptions {

impl File {
pub fn open(path: &Path, opts: &OpenOptions) -> io::Result<File> {
run_path_with_cstr(path, |path| File::open_c(&path, opts))
run_path_with_cstr(path, &|path| File::open_c(&path, opts))
}

pub fn open_c(path: &CStr, opts: &OpenOptions) -> io::Result<File> {
Expand Down Expand Up @@ -421,7 +421,7 @@ pub fn readdir(_p: &Path) -> io::Result<ReadDir> {
}

pub fn unlink(path: &Path) -> io::Result<()> {
run_path_with_cstr(path, |path| cvt(unsafe { abi::unlink(path.as_ptr()) }).map(|_| ()))
run_path_with_cstr(path, &|path| cvt(unsafe { abi::unlink(path.as_ptr()) }).map(|_| ()))
}

pub fn rename(_old: &Path, _new: &Path) -> io::Result<()> {
Expand Down
8 changes: 4 additions & 4 deletions library/std/src/sys/pal/solid/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ pub fn env() -> Env {
pub fn getenv(k: &OsStr) -> Option<OsString> {
// environment variables with a nul byte can't be set, so their value is
// always None as well
run_with_cstr(k.as_bytes(), |k| {
run_with_cstr(k.as_bytes(), &|k| {
let _guard = env_read_lock();
let v = unsafe { libc::getenv(k.as_ptr()) } as *const libc::c_char;

Expand All @@ -190,16 +190,16 @@ pub fn getenv(k: &OsStr) -> Option<OsString> {
}

pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
run_with_cstr(k.as_bytes(), |k| {
run_with_cstr(v.as_bytes(), |v| {
run_with_cstr(k.as_bytes(), &|k| {
run_with_cstr(v.as_bytes(), &|v| {
let _guard = ENV_LOCK.write();
cvt_env(unsafe { libc::setenv(k.as_ptr(), v.as_ptr(), 1) }).map(drop)
})
})
}

pub fn unsetenv(n: &OsStr) -> io::Result<()> {
run_with_cstr(n.as_bytes(), |nbuf| {
run_with_cstr(n.as_bytes(), &|nbuf| {
let _guard = ENV_LOCK.write();
cvt_env(unsafe { libc::unsetenv(nbuf.as_ptr()) }).map(drop)
})
Expand Down
42 changes: 21 additions & 21 deletions library/std/src/sys/pal/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1118,7 +1118,7 @@ impl OpenOptions {

impl File {
pub fn open(path: &Path, opts: &OpenOptions) -> io::Result<File> {
run_path_with_cstr(path, |path| File::open_c(path, opts))
run_path_with_cstr(path, &|path| File::open_c(path, opts))
}

pub fn open_c(path: &CStr, opts: &OpenOptions) -> io::Result<File> {
Expand Down Expand Up @@ -1394,7 +1394,7 @@ impl DirBuilder {
}

pub fn mkdir(&self, p: &Path) -> io::Result<()> {
run_path_with_cstr(p, |p| cvt(unsafe { libc::mkdir(p.as_ptr(), self.mode) }).map(|_| ()))
run_path_with_cstr(p, &|p| cvt(unsafe { libc::mkdir(p.as_ptr(), self.mode) }).map(|_| ()))
}

pub fn set_mode(&mut self, mode: u32) {
Expand Down Expand Up @@ -1575,7 +1575,7 @@ impl fmt::Debug for File {
}

pub fn readdir(path: &Path) -> io::Result<ReadDir> {
let ptr = run_path_with_cstr(path, |p| unsafe { Ok(libc::opendir(p.as_ptr())) })?;
let ptr = run_path_with_cstr(path, &|p| unsafe { Ok(libc::opendir(p.as_ptr())) })?;
if ptr.is_null() {
Err(Error::last_os_error())
} else {
Expand All @@ -1586,27 +1586,27 @@ pub fn readdir(path: &Path) -> io::Result<ReadDir> {
}

pub fn unlink(p: &Path) -> io::Result<()> {
run_path_with_cstr(p, |p| cvt(unsafe { libc::unlink(p.as_ptr()) }).map(|_| ()))
run_path_with_cstr(p, &|p| cvt(unsafe { libc::unlink(p.as_ptr()) }).map(|_| ()))
}

pub fn rename(old: &Path, new: &Path) -> io::Result<()> {
run_path_with_cstr(old, |old| {
run_path_with_cstr(new, |new| {
run_path_with_cstr(old, &|old| {
run_path_with_cstr(new, &|new| {
cvt(unsafe { libc::rename(old.as_ptr(), new.as_ptr()) }).map(|_| ())
})
})
}

pub fn set_perm(p: &Path, perm: FilePermissions) -> io::Result<()> {
run_path_with_cstr(p, |p| cvt_r(|| unsafe { libc::chmod(p.as_ptr(), perm.mode) }).map(|_| ()))
run_path_with_cstr(p, &|p| cvt_r(|| unsafe { libc::chmod(p.as_ptr(), perm.mode) }).map(|_| ()))
}

pub fn rmdir(p: &Path) -> io::Result<()> {
run_path_with_cstr(p, |p| cvt(unsafe { libc::rmdir(p.as_ptr()) }).map(|_| ()))
run_path_with_cstr(p, &|p| cvt(unsafe { libc::rmdir(p.as_ptr()) }).map(|_| ()))
}

pub fn readlink(p: &Path) -> io::Result<PathBuf> {
run_path_with_cstr(p, |c_path| {
run_path_with_cstr(p, &|c_path| {
let p = c_path.as_ptr();

let mut buf = Vec::with_capacity(256);
Expand Down Expand Up @@ -1635,16 +1635,16 @@ pub fn readlink(p: &Path) -> io::Result<PathBuf> {
}

pub fn symlink(original: &Path, link: &Path) -> io::Result<()> {
run_path_with_cstr(original, |original| {
run_path_with_cstr(link, |link| {
run_path_with_cstr(original, &|original| {
run_path_with_cstr(link, &|link| {
cvt(unsafe { libc::symlink(original.as_ptr(), link.as_ptr()) }).map(|_| ())
})
})
}

pub fn link(original: &Path, link: &Path) -> io::Result<()> {
run_path_with_cstr(original, |original| {
run_path_with_cstr(link, |link| {
run_path_with_cstr(original, &|original| {
run_path_with_cstr(link, &|link| {
cfg_if::cfg_if! {
if #[cfg(any(target_os = "vxworks", target_os = "redox", target_os = "android", target_os = "espidf", target_os = "horizon", target_os = "vita"))] {
// VxWorks, Redox and ESP-IDF lack `linkat`, so use `link` instead. POSIX leaves
Expand Down Expand Up @@ -1678,7 +1678,7 @@ pub fn link(original: &Path, link: &Path) -> io::Result<()> {
}

pub fn stat(p: &Path) -> io::Result<FileAttr> {
run_path_with_cstr(p, |p| {
run_path_with_cstr(p, &|p| {
cfg_has_statx! {
if let Some(ret) = unsafe { try_statx(
libc::AT_FDCWD,
Expand All @@ -1697,7 +1697,7 @@ pub fn stat(p: &Path) -> io::Result<FileAttr> {
}

pub fn lstat(p: &Path) -> io::Result<FileAttr> {
run_path_with_cstr(p, |p| {
run_path_with_cstr(p, &|p| {
cfg_has_statx! {
if let Some(ret) = unsafe { try_statx(
libc::AT_FDCWD,
Expand All @@ -1716,7 +1716,7 @@ pub fn lstat(p: &Path) -> io::Result<FileAttr> {
}

pub fn canonicalize(p: &Path) -> io::Result<PathBuf> {
let r = run_path_with_cstr(p, |path| unsafe {
let r = run_path_with_cstr(p, &|path| unsafe {
Ok(libc::realpath(path.as_ptr(), ptr::null_mut()))
})?;
if r.is_null() {
Expand Down Expand Up @@ -1879,7 +1879,7 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
// Opportunistically attempt to create a copy-on-write clone of `from`
// using `fclonefileat`.
if HAS_FCLONEFILEAT.load(Ordering::Relaxed) {
let clonefile_result = run_path_with_cstr(to, |to| {
let clonefile_result = run_path_with_cstr(to, &|to| {
cvt(unsafe { fclonefileat(reader.as_raw_fd(), libc::AT_FDCWD, to.as_ptr(), 0) })
});
match clonefile_result {
Expand Down Expand Up @@ -1925,7 +1925,7 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
}

pub fn chown(path: &Path, uid: u32, gid: u32) -> io::Result<()> {
run_path_with_cstr(path, |path| {
run_path_with_cstr(path, &|path| {
cvt(unsafe { libc::chown(path.as_ptr(), uid as libc::uid_t, gid as libc::gid_t) })
.map(|_| ())
})
Expand All @@ -1937,15 +1937,15 @@ pub fn fchown(fd: c_int, uid: u32, gid: u32) -> io::Result<()> {
}

pub fn lchown(path: &Path, uid: u32, gid: u32) -> io::Result<()> {
run_path_with_cstr(path, |path| {
run_path_with_cstr(path, &|path| {
cvt(unsafe { libc::lchown(path.as_ptr(), uid as libc::uid_t, gid as libc::gid_t) })
.map(|_| ())
})
}

#[cfg(not(any(target_os = "fuchsia", target_os = "vxworks")))]
pub fn chroot(dir: &Path) -> io::Result<()> {
run_path_with_cstr(dir, |dir| cvt(unsafe { libc::chroot(dir.as_ptr()) }).map(|_| ()))
run_path_with_cstr(dir, &|dir| cvt(unsafe { libc::chroot(dir.as_ptr()) }).map(|_| ()))
}

pub use remove_dir_impl::remove_dir_all;
Expand Down Expand Up @@ -2140,7 +2140,7 @@ mod remove_dir_impl {
if attr.file_type().is_symlink() {
crate::fs::remove_file(p)
} else {
run_path_with_cstr(p, |p| remove_dir_all_recursive(None, &p))
run_path_with_cstr(p, &|p| remove_dir_all_recursive(None, &p))
}
}

Expand Down
10 changes: 5 additions & 5 deletions library/std/src/sys/pal/unix/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ pub fn chdir(_p: &path::Path) -> io::Result<()> {

#[cfg(not(target_os = "espidf"))]
pub fn chdir(p: &path::Path) -> io::Result<()> {
let result = run_path_with_cstr(p, |p| unsafe { Ok(libc::chdir(p.as_ptr())) })?;
let result = run_path_with_cstr(p, &|p| unsafe { Ok(libc::chdir(p.as_ptr())) })?;
if result == 0 { Ok(()) } else { Err(io::Error::last_os_error()) }
}

Expand Down Expand Up @@ -643,7 +643,7 @@ pub fn env() -> Env {
pub fn getenv(k: &OsStr) -> Option<OsString> {
// environment variables with a nul byte can't be set, so their value is
// always None as well
run_with_cstr(k.as_bytes(), |k| {
run_with_cstr(k.as_bytes(), &|k| {
let _guard = env_read_lock();
let v = unsafe { libc::getenv(k.as_ptr()) } as *const libc::c_char;

Expand All @@ -661,16 +661,16 @@ pub fn getenv(k: &OsStr) -> Option<OsString> {
}

pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
run_with_cstr(k.as_bytes(), |k| {
run_with_cstr(v.as_bytes(), |v| {
run_with_cstr(k.as_bytes(), &|k| {
run_with_cstr(v.as_bytes(), &|v| {
let _guard = ENV_LOCK.write();
cvt(unsafe { libc::setenv(k.as_ptr(), v.as_ptr(), 1) }).map(drop)
})
})
}

pub fn unsetenv(n: &OsStr) -> io::Result<()> {
run_with_cstr(n.as_bytes(), |nbuf| {
run_with_cstr(n.as_bytes(), &|nbuf| {
let _guard = ENV_LOCK.write();
cvt(unsafe { libc::unsetenv(nbuf.as_ptr()) }).map(drop)
})
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/pal/wasi/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,7 @@ fn open_at(fd: &WasiFd, path: &Path, opts: &OpenOptions) -> io::Result<File> {
/// Note that this can fail if `p` doesn't look like it can be opened relative
/// to any pre-opened file descriptor.
fn open_parent(p: &Path) -> io::Result<(ManuallyDrop<WasiFd>, PathBuf)> {
run_path_with_cstr(p, |p| {
run_path_with_cstr(p, &|p| {
let mut buf = Vec::<u8>::with_capacity(512);
loop {
unsafe {
Expand Down
10 changes: 5 additions & 5 deletions library/std/src/sys/pal/wasi/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ pub fn getcwd() -> io::Result<PathBuf> {
}

pub fn chdir(p: &path::Path) -> io::Result<()> {
let result = run_path_with_cstr(p, |p| unsafe { Ok(libc::chdir(p.as_ptr())) })?;
let result = run_path_with_cstr(p, &|p| unsafe { Ok(libc::chdir(p.as_ptr())) })?;
match result == (0 as libc::c_int) {
true => Ok(()),
false => Err(io::Error::last_os_error()),
Expand Down Expand Up @@ -227,7 +227,7 @@ pub fn env() -> Env {
pub fn getenv(k: &OsStr) -> Option<OsString> {
// environment variables with a nul byte can't be set, so their value is
// always None as well
run_with_cstr(k.as_bytes(), |k| {
run_with_cstr(k.as_bytes(), &|k| {
let _guard = env_read_lock();
let v = unsafe { libc::getenv(k.as_ptr()) } as *const libc::c_char;

Expand All @@ -245,16 +245,16 @@ pub fn getenv(k: &OsStr) -> Option<OsString> {
}

pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
run_with_cstr(k.as_bytes(), |k| {
run_with_cstr(v.as_bytes(), |v| unsafe {
run_with_cstr(k.as_bytes(), &|k| {
run_with_cstr(v.as_bytes(), &|v| unsafe {
let _guard = env_write_lock();
cvt(libc::setenv(k.as_ptr(), v.as_ptr(), 1)).map(drop)
})
})
}

pub fn unsetenv(n: &OsStr) -> io::Result<()> {
run_with_cstr(n.as_bytes(), |nbuf| unsafe {
run_with_cstr(n.as_bytes(), &|nbuf| unsafe {
let _guard = env_write_lock();
cvt(libc::unsetenv(nbuf.as_ptr())).map(drop)
})
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys_common/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ impl<'a> TryFrom<(&'a str, u16)> for LookupHost {
fn try_from((host, port): (&'a str, u16)) -> io::Result<LookupHost> {
init();

run_with_cstr(host.as_bytes(), |c_host| {
run_with_cstr(host.as_bytes(), &|c_host| {
let mut hints: c::addrinfo = unsafe { mem::zeroed() };
hints.ai_socktype = c::SOCK_STREAM;
let mut res = ptr::null_mut();
Expand Down
Loading